Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-30 Thread GunChleoc
I'm not a big Java thing, but I guess that's one thing they did better, that you have to declare explicitly which exceptions can be thrown by a function. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-29 Thread SirVer
I hate exceptions. For everything. They add a second control flow - but it is invisible. Your functions can return a value - or they might throw an exception. The first thing is visible in its signature, the second is not. At the call site you cannot tell which exceptions might come your way.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-29 Thread GunChleoc
I guess the reason for disallowing Exceptions is to make sure that nobody starts programming by exception - I have no issues with Exceptions as long as they are used sensibly, i.e. for error handling rather than for code path decisions. https://en.wikipedia.org/wiki/Coding_by_exception --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread SirVer
you are right, we should probably codify our style somewhere - since we are only using parts of the google c++ style. we have a long history of boost usage in Widelands, so we continue using it. We also have used exceptions a lot in the past, so we also continue using them. Were we a

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread Teppo Mäenpää
I will take note for future. Thanks. I cannot resist asking: The Google C++ style guide disallows boost, with exceptions; lexical_cast is not mentioned in the list of allowed uses. How strictly should one follow the style guide? --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread GunChleoc
One more comment for the future: You could use boost::lexical_cast instead of "int ival = strtol(value.c_str(), nullptr, 10);" -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 -- Your team Widelands Develop

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread Teppo Mäenpää
Forgot the commit message. I guess bunnybot is not offended by repeated requests. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread Teppo Mäenpää
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Commit Message changed to: Bug 1574379: Forester prefers good soil, and is thus more efficient. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread Teppo Mäenpää
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Commit Message changed to: Bug 1574379: Forester prefers good soil. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread Teppo Mäenpää
I understood the Jan14th comments so that it is okay to merge this. @bunnybot merge Curious to see what happens. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-27 Thread bunnybot
Continuous integration builds have changed state: Travis build 3102. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/334024010. Appveyor build 2909. State: success. Details:

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-26 Thread bunnybot
Continuous integration builds have changed state: Travis build 3092. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/333588431. Appveyor build 2899. State: success. Details:

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-24 Thread bunnybot
Continuous integration builds have changed state: Travis build 3089. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/332953711. Appveyor build 2896. State: success. Details:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-24 Thread Teppo Mäenpää
intended to say "comment" in the above. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-23 Thread bunnybot
Continuous integration builds have changed state: Travis build 3086. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/332469214. Appveyor build 2893. State: success. Details:

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-22 Thread bunnybot
Continuous integration builds have changed state: Travis build 3083. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/331668194. Appveyor build 2890. State: success. Details:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
Got the random error, again. I guess bunnybot will not merge now. If we cannot alter the way the infra works, would it be possible to work around? What do the scripts look like ? If the errors are truly random, one could try command || (sleep 10 && command) || $(sleep 10 && command) or, in this

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Commit Message changed to: For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 -- Your team Widelands Developers

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Status: Approved => Needs review For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 -- Your team Widela

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 -- Your team Widela

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Commit Message changed to: Forester prefers good soil for planting. Fixes bug #1574379. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
The proposal to merge lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands has been updated. Commit Message changed to: Forester prefers good soil for planting. Fixes but #1574379. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread kaputtnik
Thanks :-) Travis has some problems fetching the packages. There is nothing we can do against this, afaik. You should add the commit message, bunnybot uses it when merging into trunk. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
Side note: Yes, it is gone. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit. ___ Mailing list:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
I have been pretty unlucky with Travis and appveyor. A new commit just to merge trunk sounds scary already. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread kaputtnik
> Side note: WuiPlotArea::register_plot_data leaks memory according to > valgrind, is this known? That should be fixed with http://bazaar.launchpad.net/~widelands-dev/widelands/trunk/revision/8557 Can you merge trunk and test again? --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-21 Thread Teppo Mäenpää
Played more. No problems. IMO, merge-quality again. Side note: WuiPlotArea::register_plot_data leaks memory according to valgrind, is this known? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-20 Thread Teppo Mäenpää
Tried to get the assert for ~2 hours, without success. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1574379-forester-wit.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-19 Thread Teppo Mäenpää
I currently mostly write code using language, where single equal sign is a comparison instead of assignment. A side-effect is that "return a = b" looks very boolean to my eyes.. About the cache_entry > kInvalidForesterEntry: You are right, this is more robust, even if it looks weird. I'll add

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-16 Thread Klaus Halfmann
> return forester_cache[mi] = correct_val; < forester_cache[mi] = correct_val;" < return correct_val; Well any compiler with some omptimization should optimize this away. Maybe som bad habit from the old days of C programming :-) > The comparison (cache_entry > kInvalidForesterEntry) looks

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-14 Thread bunnybot
Continuous integration builds have changed state: Travis build 3053. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/328793965. Appveyor build 2861. State: success. Details:

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-14 Thread Teppo Mäenpää
Hello, Klaus, Since you asked: Does the line like "return forester_cache[mi] = correct_val;" bring any performance benefits? I know it is valid and all, but the old way (separate assignment and returning) is more intuitive. Not everybody reading the source are professionals, especially C/C++

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-14 Thread Klaus Halfmann
Review: Approve rerview, compile debug, test Hello Teppo, I did some small changes, I hope you agree. I assume this will improve efiicency of tree planting quite a lot, thus chnageing the balance of existing maps. To stick to the ideas of widelands a "tree planter" should become a "power

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-14 Thread Klaus Halfmann
dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-13 Thread bunnybot
/_widelands_dev_widelands_bug_1574379_forester_wit-2859. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-13 Thread Teppo Mäenpää
requested in the bug description. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-13 Thread Teppo Mäenpää
feedback! No particular reason to hurry -- we are not close to any deadlines AFAIK. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bu

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-13 Thread Klaus Halfmann
ur team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscri

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-13 Thread Teppo Mäenpää
elands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe :

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

2018-01-13 Thread Teppo Mäenpää
osed merge of lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands. === modified file 'data/tribes/workers/atlanteans/forester/init.lua' --- data/tribes/workers/atlanteans/forester/init.lua 2017-02-12 09:10:57 + +++ data/tribes/workers/atlanteans/forester/init.lua 2018-01-1