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

2014-08-01 Thread GunChleoc
I have put some questions in NOCOM comments that I need help with -- https://code.launchpad.net/~widelands-dev/widelands/bug-1348795/+merge/228430 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1348795. ___

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

2014-08-01 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-566729 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #566729 in widelands: Add gametime display to replays https://bugs.launchpad.net/widelands/+bug/566729 For more details, see

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

2014-08-02 Thread GunChleoc
Now fully localized filenames for replays. Ready for merging now. Please update the .pot catalogs after merging. -- https://code.launchpad.net/~widelands-dev/widelands/bug-566729/+merge/229302 Your team Widelands Developers is requested to review the proposed merge of

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

2014-08-02 Thread GunChleoc
I will try doing the redesign for the metadata. At the moment, if the filename doesn't conform to the datetime format, the filename is shown as is. At to blitting the gametime to the editor, it will count from 0, just like in the game. No harm done, or does this need to go? --

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

2014-08-02 Thread GunChleoc
I think a way to test this would be to play the 2nd Empire campaign until you hit the objective obj_better_material_2. Then don't build any coal mines but charcoal kilns only and see if you can complete the objectve by that. If you can, everything's fine. --

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

2014-08-02 Thread GunChleoc
I have now added data to game_preload_data_packet, but it is always empty and I have no idea why. -- https://code.launchpad.net/~widelands-dev/widelands/bug-566729/+merge/229302 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-566729.

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

2014-08-03 Thread GunChleoc
Review: Approve LGTM -- https://code.launchpad.net/~widelands-dev/widelands/furnace/+merge/227810 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/furnace. ___ Mailing list: https://launchpad.net/~widelands-dev Post

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

2014-08-05 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/string_fixes into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/string_fixes/+merge/229598 1 stringfix -- https://code.launchpad.net

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

2014-08-05 Thread GunChleoc
Since this fix is both small + critical, I will merge myself so we can get this in. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1351620/+merge/229361 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1351620 into

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

2014-09-07 Thread GunChleoc
P.S.you can of course expand the new notification struct with any parameters that the AI needs for its decision making, like time sent etc - you could also add counters into the AI itself as to how often a message has been triggered for a particular production site, etc... if it's a mine, I

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

2014-09-08 Thread GunChleoc
I'm not sure where to put Game_Preload_Data_Packet::get_localized_display_title The only common included header that would marginally make sense is logic/game_controller.h. Or a new h/cpp for this? base/time_string.h doesn't make sense either, because we would have a reference to

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

2014-09-08 Thread GunChleoc
I don't think that you need to analyze how exactly mines get their resources - once the message is triggered, it means that the mine isn't profitable anymore. That's all the AI needs to know. A human player would look into enhancing/dismantling the mine as soon as the message is triggered,

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

2014-09-08 Thread GunChleoc
No worries, the code isn't going anywhere ;) -- https://code.launchpad.net/~widelands-dev/widelands/bug-566729/+merge/229302 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-566729. ___ Mailing list:

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

2014-09-08 Thread GunChleoc
Review: Resubmit Done :) -- https://code.launchpad.net/~widelands-dev/widelands/boost_format/+merge/229366 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/boost_format. ___ Mailing list:

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

2014-09-09 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-988831 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #988831 in widelands: Remove message expiry feature. https://bugs.launchpad.net/widelands/+bug/988831 For more details, see

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1343297_2 into lp:widelands

2014-09-11 Thread GunChleoc
2 questions then: what does the _t in TypeAndLevel_t mean? We also have some _s stuff in the typedefs. I also have trouble with a rule for everything - a simple check for underscore won't do, because of the uint16_t in definitions like std::pairuint16_t, uint16_t. Also, some typedefs are

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1343297_2 into lp:widelands

2014-09-13 Thread GunChleoc
Sounds good to me: I'll switch everything to using, which will make for an easier code check rule afterwards -- https://code.launchpad.net/~widelands-dev/widelands/bug-1343297_2/+merge/234183 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1343297_2.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1343297_2 into lp:widelands

2014-09-15 Thread GunChleoc
Done. I also added an exception to the CamelCase codecheck rule to avoid the false positives. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1343297_2/+merge/234183 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1343297_2.

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

2014-09-15 Thread GunChleoc
Review: Resubmit All fixed. Diff comments: === modified file 'src/game_io/game_loader.cc' --- src/game_io/game_loader.cc2014-07-28 14:23:03 + +++ src/game_io/game_loader.cc2014-09-11 17:46:54 + @@ -32,7 +32,6 @@ #include game_io/game_player_info_data_packet.h

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

2014-09-15 Thread GunChleoc
Review: Resubmit Diff comments are addressed -- https://code.launchpad.net/~widelands-dev/widelands/bug-1366725/+merge/233750 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1366725. ___ Mailing list:

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

2014-09-18 Thread GunChleoc
Nits are fixed. Could you please merge the Launtpad translations update first, in case there are new/updated translations for the console help? -- https://code.launchpad.net/~widelands-dev/widelands/bug-978175/+merge/234790 Your team Widelands Developers is subscribed to branch

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

2014-09-18 Thread GunChleoc
Good catch! Bug fixed. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1366725/+merge/233750 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1366725. ___ Mailing list:

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

2014-09-23 Thread GunChleoc
Fixed. We need to leave the bug open though, because more instances of not enough space have been reported. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1371905/+merge/235439 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1371905.

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

2014-09-24 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1174075 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1174075 in widelands: Clarify meaning of icons in editor terrain preview https://bugs.launchpad.net/widelands/+bug/1174075

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

2014-09-24 Thread GunChleoc
Will do -- https://code.launchpad.net/~widelands-dev/widelands/bug-1371905/+merge/235439 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1371905. ___ Mailing list: https://launchpad.net/~widelands-dev Post to

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

2014-09-25 Thread GunChleoc
Question for one of the nits - see diff comment. Diff comments: === modified file 'src/editor/ui_menus/editor_tool_set_terrain_options_menu.cc' --- src/editor/ui_menus/editor_tool_set_terrain_options_menu.cc 2014-09-10 14:08:25 + +++

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

2014-09-25 Thread GunChleoc
Are you using green to indicate buildings allowed? I don't know what it's used for. F.e. 'Water (Desert): Water Unpassable Trees' what about this layout: Water (Desert): water, unpassable, trees I'm actually not sure if native speakers would prefer title case here or not. Is 'Desert' in

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

2014-09-26 Thread GunChleoc
It is probably waiting on SirVer to find the time to review it ;) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1366725/+merge/233750 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1366725. ___

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

2014-09-26 Thread GunChleoc
SirVer is too busy - generally not just now. I communicated with him and he suggested that also other developers should review/approve merge requests. (I had my AI branch in mind specifically) So I can approve this one as this looks a simple one, and SirVer has looked at it before, and I

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1371905_2 into lp:widelands

2014-09-27 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1371905_2 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1371905 in widelands: Localized strings doesn'tfit in the available place https://bugs.launchpad.net/widelands/+bug

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

2014-09-27 Thread GunChleoc
Are you using green to indicate buildings allowed? I don't know what it's used for. It seems to me that it belongs to house icon, anyway it is confusing for user Can somebody who actually knows that this is for give us a hint? Yes, it's one of the original terrain types. These

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

2014-09-27 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/production_program_stringfix into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/production_program_stringfix/+merge/236230 Small

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

2014-09-29 Thread GunChleoc
It only is called when the production program starts with Skipped when. I didn't figure out why. I'm also not sure that I could get these string o screen at all, because it immediately switched to an alternative progrm without sleeping first. If you wish me to investigate further, I can add

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

2014-09-29 Thread GunChleoc
I overlooked that we already have the hotkeys 1-9 in use. I willcome up with different hotkeys. Or even better, but probably more work for you - remove that flag on the bottom and add new column that would contain a particular flag for the message. This column would need a header and eat up

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

2014-09-29 Thread GunChleoc
OK, I added some sleep commands to make sure I see these, but these methods don't seem to be involved with printing anything into the tooltips. I guess there is a generic action called someplace just to move to the next step with the when keyword, and that this action also creates its negation

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

2014-09-29 Thread GunChleoc
I think the word we're looking for here is arable. Will go change :) I am not happy with the switched to vecrot proposed in the diff comments, could I please have feedback on that before we merge this? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1174075/+merge/235763 Your team

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

2014-09-29 Thread GunChleoc
Exactly. I grepped for when, and it only appears in these 2 Atlantean buildings. The low occurrence is the reason I didn't catch this in the first place, because I usually test with Barbarians (less mouse clicks to start the game :P) --

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

2014-09-29 Thread GunChleoc
I found some letters that are included somewhere in the English strings for the hotkeys. Since we don't have localizable hotkeys, we will have to live with other languages not matching. But we have thet problem anyway with all hotkeys that are letters. - Why not simply write Hotkey: %s? That

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1371905_2 into lp:widelands

2014-09-29 Thread GunChleoc
This should work: const std::vectorUI::Button* get_buttons() { return m_buttons; } It does - The mistake I made is that I thought I had to match the member variable type. Will commit and merge now :) Use clang_format over all your changes :). Seriously, when we get clang_format to being

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

2014-09-29 Thread GunChleoc
I was just wondering if I had messed anything up. We were both working on trunk at the same time. I have a question though - just trying to understand this cmake stuff a bit: Why is it WL_INSTALL_DATADIR in CMakeLists.txt , but INSTALL_DATADIR without the WL_ in src/wlapplication.cc? --

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

2014-09-29 Thread GunChleoc
So, what are the preferred hotkeys for you guys: 0-5, or the letters I'm using right now? -- https://code.launchpad.net/~widelands-dev/widelands/bug-987510/+merge/236231 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-987510 into

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

2014-09-29 Thread GunChleoc
Thanks! I did do a bzr pull, but you pushed something while I was busy solving conflicts, compiling and testing - will use shelve in the future :) -- https://code.launchpad.net/~widelands-dev/widelands/fix_setup_searchpaths/+merge/236410 Your team Widelands Developers is requested to review the

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

2014-09-29 Thread GunChleoc
Will do the numbers then. The discinct message types are needed elsewhere, see the diff comment for details. Diff comments: === modified file 'pics/message_archive.png' Binary files pics/message_archive.png 2010-01-14 18:22:23 + and pics/message_archive.png 2014-09-29 15:04:07 +

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

2014-09-30 Thread GunChleoc
Review: Resubmit All done. Hotkeys work as SirVer described. -- https://code.launchpad.net/~widelands-dev/widelands/bug-987510/+merge/236231 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-987510. ___ Mailing

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

2014-09-30 Thread GunChleoc
Review: Approve One nit: _Sow and harvest grapes. One cannot sow grapes - I suggest: _Plant grapevines and harvest grapes. Otherwise LGTM :) -- https://code.launchpad.net/~widelands-dev/widelands/bug-669699/+merge/236490 Your team Widelands Developers is subscribed to branch

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

2014-10-03 Thread GunChleoc
This doesn't compile, you need to change a line (see diff comment). Otherwise, I did a bit of testing. The paths already work for me without this fix, bue it doesn't make things worse either. Diff comments: === modified file 'src/wlapplication.cc' --- src/wlapplication.cc 2014-09-20

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

2014-10-04 Thread GunChleoc
I just did some testing and have an improvement suggestion for the next AI branch. Training sites are built before weapons production, and weapons production is built before the mountains have been reached to build some mines. Suggestion: For each production site, get descr()-inputs(). Then

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

2014-10-04 Thread GunChleoc
The proposal to merge lp:~widelands-dev/widelands/fix_setup_searchpaths into lp:widelands has been updated. Status: Needs review = Approved For more details, see: https://code.launchpad.net/~widelands-dev/widelands/fix_setup_searchpaths/+merge/236410 --

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

2014-10-05 Thread GunChleoc
abundant road - is a road not intensivelly used and is not necessary, start and ends of the road are connected via other roads. (hence that traversing on roads, if adjacent flags A and B are connected via two routes, one of them can be cancelled) How about calling it dispensable road?

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

2014-10-06 Thread GunChleoc
I find word reference really useful to make sure I picked the right word - you can get both English synonyms and English definitions there. I also like to click back and forth in a bilingual dictionary to find the right term, which I then double-check in word reference. This is how I came up

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

2014-10-11 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1371062 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1371062 in widelands: Add confirmation dialog to exit game https://bugs.launchpad.net/widelands/+bug/1371062 For more

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

2014-10-12 Thread GunChleoc
Review: Needs Fixing All fixed now. BTW the compilation failed because the DISALLOW_COPY_AND_ASSIGN had the wrong object - SirVer doesn't have time to wait for the compiler when doing code reviews, so I always compile again when addressing his comments. --

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

2014-10-12 Thread GunChleoc
Review: Resubmit OOps, I meant to resubmit -- https://code.launchpad.net/~widelands-dev/widelands/bug-1378801/+merge/237599 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1378801. ___ Mailing list:

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

2014-10-12 Thread GunChleoc
The problem is with the test suite. I can't track it down, so it probably lies with the binaries somewhere. I decided to allow lower packet versions in some places to keep the tests working, and there was no change in the packet content, just the packet number. I don't know how to fix the

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

2014-10-12 Thread GunChleoc
I tried that as well, but I can't avoid having multiple windows being generated, which is behaviour you pointed out that we don't want. So, not making it unique implies the current behaviour. So, I need a +1 on that before I change the code again. --

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

2014-10-13 Thread GunChleoc
Resaving maps with --nozip doesn't work for the test suite maps, it kills them. So, will have to create all of them from scratch. Maybe someone with a big screen and practice with the editor could do that? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1291904/+merge/237128 Your

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

2014-10-13 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/wui_won_message into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/wui_won_message/+merge/238111 Fixed space issues in game summary

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

2014-10-13 Thread GunChleoc
Review: Resubmit OK, done. I think. The confirmation box now also kills the Game Options Menu. If that isn't wanted behaviour, I can easily change that - it's just 1 line of code to remove. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1371062/+merge/238048 Your team Widelands

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

2014-10-15 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1169445 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1169445 in widelands: Commandline options 1/0 = true/false on win32 https://bugs.launchpad.net/widelands/+bug/1169445

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

2014-10-17 Thread GunChleoc
Multiplayer load game is done now. Still can't reproduce Tibor's bug. -- https://code.launchpad.net/~widelands-dev/widelands/i18nfixes/+merge/238137 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/i18nfixes. ___

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

2014-10-17 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/fix_building_statistics into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/fix_building_statistics/+merge/238699 Two small string

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

2014-10-17 Thread GunChleoc
Let's merge https://code.launchpad.net/~widelands-dev/widelands/new-tutorials first. I'll then fix this up afterwards and send a resubmit. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1298301/+merge/236966 Your team Widelands Developers is requested to review the proposed merge of

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

2014-10-18 Thread GunChleoc
This looks like a basic directory for Widelands in general is not found, so it can't be connected to my changes. I guess your instinct here is correct. You could try not installing, just compiling and running. -- https://code.launchpad.net/~widelands-dev/widelands/i18nfixes/+merge/238137 Your

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

2014-10-18 Thread GunChleoc
Review: Needs Fixing I have proofread and fixed some stuff and then playtested the tutorials. We still have the following issues to resolve: Economy: function _try_add_objective doesn't exist. What was it supposed to do? Basic control: 1. We should bzr move campaigns/tutorial01.wmf to Basic

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

2014-10-18 Thread GunChleoc
This fixed another crash. Did you check that everything still works? This has never crashed so far. Normally, the given panels should all exist. Yes, I did check - at least I didn't notice any adverse effects. And why did you only write about sentries in tut01? The player can build other

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

2014-10-18 Thread GunChleoc
I still can't reproduce this bug, so I don't know how to fix it. I will be without my dev machine for a week now anyway. Do you you have time to go bug hunting? -- https://code.launchpad.net/~widelands-dev/widelands/i18nfixes/+merge/238137 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~hjd/widelands/mark-scan-build-false-positives into lp:widelands

2014-10-18 Thread GunChleoc
For the pointer is null case, would an assert that the object != nullptr do the job? -- https://code.launchpad.net/~hjd/widelands/mark-scan-build-false-positives/+merge/238794 Your team Widelands Developers is requested to review the proposed merge of

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

2014-10-19 Thread GunChleoc
2. How about checking for just 1 of the quarries to complete? The user needs to connect the roads for both of them, so we can assume that if he can manage one, he will manage the other. 3. I had another look at the function. Since it only needs a random empty field, hard coding the start

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

2014-10-25 Thread GunChleoc
Fixed the road building mode + panel problem - you simply didn't sleep long enough. Good point about proofreading the descriptions, I had forgotten those. -- https://code.launchpad.net/~widelands-dev/widelands/new-tutorials/+merge/238682 Your team Widelands Developers is subscribed to branch

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

2014-10-25 Thread GunChleoc
@Tibor: Since I can't reproduce this myself, could you please check if the crash is still happening for you after my last commit? -- https://code.launchpad.net/~widelands-dev/widelands/i18nfixes/+merge/238137 Your team Widelands Developers is subscribed to branch

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

2014-10-26 Thread GunChleoc
I really get no crash. And I tried both with an English language interface (no checkbox) and with a translated language interface (with checkbox). Working off your stack trace, I tried something else. How is it now? If it still crashes, does it crash for Single Player - New Game and Editor -

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

2014-10-27 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1385859 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1385859/+merge/239705 To avoid occasional crashes, c_str

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

2014-10-28 Thread GunChleoc
Yes, compile.sh builds locales in the local tree. -- https://code.launchpad.net/~widelands-dev/widelands/reducing-paths/+merge/239645 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/reducing-paths into lp:widelands.

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

2014-10-28 Thread GunChleoc
Review: Approve I think we are done here. Could somebody please check my part of the code and do some additional playtesting? I still need to create the gettext catalog files, but I'll do that during the merge, so we won't use any translations. I will have to run msgmerge quite a lot ;) --

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

2014-10-28 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/SDL2 into lp:widelands. Requested reviews: SirVer (sirver) Related bugs: Bug #1380048 in widelands: Port to SDL2 https://bugs.launchpad.net/widelands/+bug/1380048 For more details, see: https://code.launchpad.net/~widelands-dev

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

2014-10-28 Thread GunChleoc
Thanks for the fix. I'll merge this tomorrow. -- https://code.launchpad.net/~widelands-dev/widelands/new-tutorials/+merge/238682 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/new-tutorials. ___ Mailing list:

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

2014-10-29 Thread GunChleoc
1. We would need to extend the Lua interface for that. Let's keep it as TODO(wl-zocker) for now. Open a new bug? 2. No. Comment removed 3. I had a look at the code and added if possible to the text. It is how the option works, but the columns might be produced in a different economy (not the

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/debian-sdl2-packages into lp:~widelands-dev/widelands/debian

2014-10-29 Thread GunChleoc
The rotozoom header is still used in a coulple of files. I have created a new bug for this: https://bugs.launchpad.net/widelands/+bug/1387317 -- https://code.launchpad.net/~widelands-dev/widelands/debian-sdl2-packages/+merge/239904 Your team Widelands Developers is subscribed to branch

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

2014-10-30 Thread GunChleoc
Unless Sirver finds some nits to pick in this branch, I'd be fine with separate merges. This would keep the code reviews to smaller chunks. The only thing that stands out for me here it that renaming built_mat_ to build_material_ will make the code more readable. --

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

2014-10-31 Thread GunChleoc
Review: Resubmit I have finished integrating wl-zocker's changes, so this is ready for review again. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1298301/+merge/236966 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1298301.

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

2014-11-01 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1388166 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1388166 in widelands: Cannot open savegame of build 18 with bzr 7237 https://bugs.launchpad.net/widelands/+bug/1388166

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

2014-11-01 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1388028 into lp:widelands. Requested reviews: TiborB (tiborb95) Related bugs: Bug #1388028 in widelands: Unable to load saved game https://bugs.launchpad.net/widelands/+bug/1388028 For more details, see: https

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

2014-11-01 Thread GunChleoc
really, so if you think it's not necessary, we can revert to using genstats[j - 1] rather than player_attackable.at(j - 1). I just thought it can't hurt to leave it in. 01/11/2014 20:09, sgrìobh TiborB: GunChleoc - this is not about zero division but about out-of-range - I think. Do you think

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

2014-11-01 Thread GunChleoc
Do we really need the if (genstats.size()j) { As I said, the try/catch was just something I added while searching, and the real fix for this particular crash was the check for genstats.at(j - 1).miltary_strength.empty()) I don't mind either way though. I expect that these cases will mostly

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

2014-11-01 Thread GunChleoc
I fixed the compiler error. The warning about switch (order_index) { should still be fixed. -- https://code.launchpad.net/~widelands-dev/widelands/glsl/+merge/240363 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/glsl into lp:widelands.

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

2014-11-02 Thread GunChleoc
Review: Approve Tested on Ubuntu 14.04 and is working. So, LGTM except for the missing default case in the switch statement. -- https://code.launchpad.net/~widelands-dev/widelands/glsl/+merge/240363 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/glsl.

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

2014-11-02 Thread GunChleoc
I don't know. Let's stay on the safe side and use the exception in combination with your log output and defaults. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1388028/+merge/240357 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1388028.

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

2014-11-03 Thread GunChleoc
Review: Resubmit Done -- https://code.launchpad.net/~widelands-dev/widelands/bug-1388166/+merge/240355 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1388166. ___ Mailing list:

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

2014-11-03 Thread GunChleoc
Ready for review now -- https://code.launchpad.net/~widelands-dev/widelands/fonts/+merge/238608 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fonts into lp:widelands. ___ Mailing list:

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

2014-11-04 Thread GunChleoc
At the moment, FullscreenMenuOptions::add_languages_to_list both in runk ad in the fonts branch are using the localedir entry fro the config file. This way, when a new language gets added that doen't have an entry in the languages config, it might be available to the user - I have not tested

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

2014-11-11 Thread GunChleoc
This actually makes me wonder, is there any reason why the languages file (or multiple files in the suggested fonts branch) is placed in txts instead of locale? All the translations are stored there, so that's the first place I would look for a list of languages to add my language code to.

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

2014-11-11 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/stringfixes into lp:widelands. Requested reviews: wl-zocker (wl-zocker) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/stringfixes/+merge/241392 Various string fixes, mainly for the new tutorials

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

2014-11-11 Thread GunChleoc
Following from this discussion: https://code.launchpad.net/~widelands-dev/widelands/reducing-paths/+merge/239645 Where do we want the language config files? They don't really belong in txts. locale is out IMO, because it's a generated resource. po isn't included in binary packages, so it's not

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

2014-11-11 Thread GunChleoc
Review: Approve I have given this a quick test on Ubuntu and it seems to work fine :) -- https://code.launchpad.net/~widelands-dev/widelands/glsl1/+merge/241179 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/glsl1.

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

2014-11-12 Thread GunChleoc
I think the repetition is OK, because it's a type of error message when the user had closed a window that he shouldn't have. I like to be consistent with those (just like with button names etc) - I think it makes it easier for the user. --

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

2014-11-13 Thread GunChleoc
Review: Approve LGTM The main po dire is a generated directory too though. I am fine with having the file there, but I am unsure if our tools can cope with a file living there. Also, the file should be a Lua file ideally since we are moving in this direction right now - but that is a

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

2014-11-13 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands-media/message_icons into lp:widelands-media. Requested reviews: Widelands Media Developers (widelands-media-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands-media/message_icons/+merge/241671 Added

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

2014-11-13 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1392406 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1392406 in widelands: Confirmation dialog when leaving the editor although Ctrl has been pressed https://bugs.launchpad.net

[Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/css-table into lp:widelands-website

2014-11-18 Thread GunChleoc
The proposal to merge lp:~widelands-dev/widelands-website/css-table into lp:widelands-website has been updated. Description changed to: Improved table layout - patch by kaputtnik. https://wl.widelands.org/forum/topic/1581/ For more details, see:

[Widelands-dev] [Merge] lp:~widelands-dev/widelands-website/css-table into lp:widelands-website

2014-11-18 Thread GunChleoc
GunChleoc has proposed merging lp:~widelands-dev/widelands-website/css-table into lp:widelands-website. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands-website/css-table/+merge/242062 Improved table layout

<    1   2   3   4   5   6   7   8   9   10   >