Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-08-13 Thread GunChleoc
Review: Approve LGTM :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/371171 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/workarea-fixes. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-23 Thread Benedikt Straub
inputqueues @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/workarea-fixes. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-23 Thread GunChleoc
Review: Approve Retested and still working :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/workarea-fixes. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread GunChleoc
Sorry for the comment spam tested and working nicely. I am also happy with the colors and borders :) -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342 Your team Widelands Developers is requested to review the proposed merge of

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread GunChleoc
2 comments for code readability Diff comments: > > === modified file 'src/logic/map_objects/tribes/tribes.cc' > --- src/logic/map_objects/tribes/tribes.cc2019-05-29 06:24:42 + > +++ src/logic/map_objects/tribes/tribes.cc2019-06-05 14:12:33 + > @@ -335,6 +335,21 @@ >

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread GunChleoc
Don't worry, kaputtnik, we can't always agree on everything and your opinion is just as valid as everybody else's -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342 Your team Widelands Developers is requested to review the proposed merge of

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread kaputtnik
ok, ok :-) Sorry for the noise. Will be quiet now. -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/workarea-fixes into lp:widelands.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread GunChleoc
OK, Lua entries it is. And I am in favor of this feature too - it will come in handy, especially for less-experienced players. I think we could have an explanation in the basic control tutorial, after the quarries have been placed and before messages get introduced. Don't do this in trunk or

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread hessenfarmer
It was definitely wanted by more then one player. I like it too. Especially if it shows good or bad coverage. I think the basic tutorial should bhe the place to explain this as early as possible, cause it will be already there. --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread Benedikt Straub
Good idea, will implement that :) > On the other side: Is it worth the work for a feature > - which is hard to explain I can just add an explanation in a tutorial. Would the economy tut or bar01 be the better place for it? > - which was wanted by a single player I would not have implemented

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread hessenfarmer
Definitions look good from my side so far. one additional question / demand? Would it be possible to distinguish between wanted overlaps (e.g. forester and lumberjack shown in greenish colour) and unwanted overlaps (e.g. farm and forester shown in red shadows) --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread Benedikt Straub
> Hm on second thought - the beekeeper -> fields rule might be actually covered > by rule 4 The farm plants only barley_field_tiny, but the flowering attrib belongs to barley_field_medium. We would have to check not only whether they plant the same attrib, but also whether one plants something

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread GunChleoc
Hm on second thought - the beekeeper -> fields rule might be actually covered by rule 4, so that would be an argument in favor of the rule-based approach, because we already found a bug in this branch ;) -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368342 Your

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread kaputtnik
While i am not convinced by this feature at all, i agree with Gun. The lua files are already full of tables and options, which make it hard to understand for beginners (modders). On the other side: Is it worth the work for a feature - which is hard to explain - which was wanted by a single

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-05 Thread GunChleoc
As pointed out by hessenfarmer in https://bugs.launchpad.net/widelands/+bug/1830345/comments/17, The beekeeper would be interested in farms and berries too. So, let's keep the Lua design and add those to the building. A rule-based approach would get very complicated with that. --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-06-04 Thread GunChleoc
I'm still in favor of deducing these in the engine so that we won't need to maintain these Lua definitions, which will be prone to bugs. Criteria could be: 1. If both buildings mine the same resource 2. If both buildings plant an immovable 3. If both buildings collect an immovable 4. If one

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-05-31 Thread Benedikt Straub
This time it´s just the inputqueues @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368083 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/workarea-fixes. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-05-31 Thread Benedikt Straub
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368083 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/workarea-fixes. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-05-31 Thread Benedikt Straub
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368083 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/workarea-fixes. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-05-30 Thread GunChleoc
Review: Approve :P Looks good though :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368083 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/workarea-fixes. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-05-30 Thread Benedikt Straub
Done. The diff is now almost 50% longer :P -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368083 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/workarea-fixes into lp:widelands.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/workarea-fixes into lp:widelands

2019-05-30 Thread GunChleoc
Code LGTM and it's all working well :) Could you make the surrounding line thicker? It's too thin to be noticeable, really. -- https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368083 Your team Widelands Developers is requested to review the proposed merge of