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

2013-07-27 Thread Teppo Mäenpää
typo: Without this check - Without this patch -- https://code.launchpad.net/~widelands-dev/widelands/soldierselect_radiobutton/+merge/176784 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/soldierselect_radiobutton.

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

2013-07-27 Thread cghislai
I figured it out - by saving the file_index given by the map object saver, objects serial are loaded correctly through the map object loader. Works great :) -- https://code.launchpad.net/~widelands-dev/widelands/bug1186906/+merge/177204 Your team Widelands Developers is requested to review the

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

2013-07-27 Thread SirVer
I agree that the new buttons look really good, and that their placement is good as well. Thanks! Glad that you like it that way too. However, start and stop buttons have now appeared to the lower left corner. Good catch - I messed that up. I added something along the lines of your patch and

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/soldierselect_radiobutton into lp:widelands

2013-07-27 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/soldierselect_radiobutton into lp:widelands has been updated. Status: Needs review = Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/soldierselect_radiobutton/+merge/176784 --

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/fishing into lp:widelands

2013-07-27 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/fishing into lp:widelands has been updated. Status: Needs review = Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/fishing/+merge/177237 --

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

2013-07-27 Thread SirVer
Code lgtm. I just did some nit fixes and merged. -- https://code.launchpad.net/~widelands-dev/widelands/fishing/+merge/177237 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fishing into lp:widelands.

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands

2013-07-27 Thread cghislai
cghislai has proposed merging lp:~widelands-dev/widelands/lua_mapview_persistence into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1203329 in widelands: Possible to trigger a crash by saving between story dialogs

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

2013-07-27 Thread SirVer
Just a general though that crossed my mind: we have many 'manual' test cases in the last few days (like the one you describe in your last sentence). How about starting a test suite with lua scenarios that can be automatically run (like ts.wmf)? they would set up a starting condition (a bunch of

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

2013-07-27 Thread SirVer
Review: Needs Fixing -- https://code.launchpad.net/~widelands-dev/widelands/bug1205010/+merge/177137 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug1205010. ___ Mailing list:

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

2013-07-27 Thread SirVer
Okay, the code lgtm. What I do not like is that the messages are only archived when the message window is opened. E.g.: if you have no messages and a quarry runs out of stone. You burn this quarry - after this you open the message window (which is marked as having new messages), but you see

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

2013-07-27 Thread cghislai
Indeed a proper test suite would be nice. and each bug fixed should have, if possible, a test written to check for regression. But as you said some handler function would be required. I may try to implement that, but Im don't think that should be ready for b18. Maybe we could open a blueprint

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

2013-07-27 Thread SirVer
Review: Needs Fixing I think this is the wrong approach - the LuaClass does not have any data members, so it should not persist anything at all. The MapView (c++ class) should save its data to save games one day, so that you have the same view and so on when loading. For the Lua fix here,

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

2013-07-27 Thread SirVer
Indeed a proper test suite would be nice. and each bug fixed should have, if possible, a test written to check for regression. But as you said some handler function would be required. I think starting out with a directory of .wmf files and run each in order from a shell script (i.e. always