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

2019-04-09 Thread Klaus Halfmann
Review, found so far: * restructered the sound directory wihht more subdirectories * Help now shows --nosoundStarts the game with sound disabled. * Using FXset one could have different soundscapes for testing or scenarions, interesting * I assuem the random must be a _local_ random, as

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

2019-04-07 Thread Klaus Halfmann
I played the first 3 Tutorial on this branhc and everyhting was fine. When hanging around in the config I get > Songset: Loaded song "music/menu_00.ogg" again and again, is this intended, or should this happen only once I will try to do a code review. But this seems to be a _bigger_ task :-) --

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/vector-float-warnings into lp:widelands

2019-03-24 Thread Klaus Halfmann
OOps, you are right and its a compiler error: > ../src/wui/mapview.h:83:13: error: expected unqualified-id before 'const' > bool near(const View& other) const { ^ But I have no idea what the problem ist, did we hit some reserved indentifier for WIN/GCC? -- https://code.launc

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

2019-03-24 Thread Klaus Halfmann
Is this for R20? -- https://code.launchpad.net/~nordfriese/widelands/workareas/+merge/364266 Your team Widelands Developers is requested to review the proposed merge of lp:~nordfriese/widelands/workareas into lp:widelands. ___ Mailing list: https://lau

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/vector-float-warnings into lp:widelands

2019-03-24 Thread Klaus Halfmann
Review: Approve compile, test, review compiled, warnings are gone. Jumped around a bit in the WideWorldMap, all fine. Anyrthing else we must check? -- https://code.launchpad.net/~widelands-dev/widelands/vector-float-warnings/+merge/365003 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/vector-float-warnings into lp:widelands

2019-03-24 Thread Klaus Halfmann
Code looks good. will copile this and zoom around a bit. -- https://code.launchpad.net/~widelands-dev/widelands/vector-float-warnings/+merge/365003 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/vector-float-warnings into lp:widelands. __

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1819311-port-spaces-heap-use-after-free into lp:widelands

2019-03-24 Thread Klaus Halfmann
Review: Approve compile, review, test Fixed that bug, lets have it. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1819311-port-spaces-heap-use-after-free/+merge/365004 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1819311-por

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

2019-03-10 Thread Klaus Halfmann
Review: Approve code review Code looks fine, will not do any further tests. -- https://code.launchpad.net/~widelands-dev/widelands/valgrind/+merge/364213 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/valgrind. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1818494-reset-zoom-crash into lp:widelands

2019-03-10 Thread Klaus Halfmann
Review: Approve code reieww, compile, testing Found a (related?) Bug at #1819311 before I coould do the > 500 undos :-) OTOH from the code review it sould not harm. I will do more tests with this huge (nice) map, lets see what we will find. -- https://code.launchpad.net/~widelands-dev/widelands

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1818494-reset-zoom-crash into lp:widelands

2019-03-10 Thread Klaus Halfmann
Wow, we should get the Map(s) this user created :-) To reproduce the first problem we should edit the biggest maxium possible map with as many objects a possible and zomm out and in like mad. Never restart to get the overflow in that undo stack?. An in a normal game this could happen, too? Can

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1807156-heap-use-after-free into lp:widelands

2019-03-05 Thread Klaus Halfmann
I quick game for some 40 minutes, found nothing, will try to produce this in some coop game perhaps. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1807156-heap-use-after-free/+merge/363584 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1807156-

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1807156-heap-use-after-free into lp:widelands

2019-03-05 Thread Klaus Halfmann
Review: Needs Information OK, his is te code that will (new in R20) add the Helpbutton so the correct helpwindow for the building in progress will be shown. _parent is the same as igbase() which is used at the very start of the function, so I have no idea how this should be come null? Teh origi

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1807156-heap-use-after-free into lp:widelands

2019-03-05 Thread Klaus Halfmann
OK merged trunk. I get some ../src/wlapplication.cc:1476:30: warning: unused exception parameter 'e' [-Wunused-exception-parameter] } catch (const FileError& e) { will now use a debugger to poke into that code. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1807156-heap-use

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

2019-03-04 Thread Klaus Halfmann
Review: Approve @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/frisian_music/+merge/350429 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/frisian_music. ___ Mailing list: https://launchpad.ne

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

2019-03-04 Thread Klaus Halfmann
Will add some metasata and thanks -- https://code.launchpad.net/~widelands-dev/widelands/frisian_music/+merge/350429 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/frisian_music. ___ Mailing list: https://launchpad.n

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1807156-heap-use-after-free into lp:widelands

2019-02-24 Thread Klaus Halfmann
Mhh, in case (building_in_lambda == nullptr) you return; In case (parent_ != nullptr) you fall through. This is in create_capsbuttons() but I dont see this in the stacktrace? Any idea how to test this? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1807156-heap-use-after-free/+merge

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

2019-02-23 Thread Klaus Halfmann
compiled found no warnings we an care for -- https://code.launchpad.net/~widelands-dev/widelands/compiler_warnings_clang_201902/+merge/363580 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/compiler_warnings_clang_201902. ___

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

2019-02-23 Thread Klaus Halfmann
Review: Approve review. Thanks for copying the rights to 2019. Found only these changes and removal of some empty lines. will noth compile this -- https://code.launchpad.net/~widelands-dev/widelands/update_copyright_year_2019/+merge/363579 Your team Widelands Developers is subscribed to branch

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

2019-02-23 Thread Klaus Halfmann
Review: Approve review. Thanks for catching these warings. Will fetch and buid this even though you alreaday want to merge this. -- https://code.launchpad.net/~widelands-dev/widelands/compiler_warnings_clang_201902/+merge/363580 Your team Widelands Developers is subscribed to branch lp:~widelan

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

2019-02-17 Thread Klaus Halfmann
Us tesing with OSX a) requireed n) if yes where can I downlaod the artifacts from https://travis-ci.org/widelands/widelands/builds/493702657 or such? -- https://code.launchpad.net/~widelands-dev/widelands/appveyor-fix/+merge/363152 Your team Widelands Developers is requested to review the propose

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/robust-file-saving into lp:widelands

2019-02-16 Thread Klaus Halfmann
Review: Resubmit Uhm, why dont we get another build? I foundd soum compile warnigns about the new JSON Objects not having virtual DTors. but that is some other story. -- https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718 Your team Widelands Developers is subscr

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

2019-02-10 Thread Klaus Halfmann
Review: Approve listen Listend to the Sound at: https://bazaar.launchpad.net/~widelands-dev/widelands/scotty_the_scout/view/head:/data/music/ingame_27.ogg fine for me. -- https://code.launchpad.net/~widelands-dev/widelands/scotty_the_scout/+merge/362943 Your team Widelands Developers is subscri

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

2019-02-10 Thread Klaus Halfmann
Uhm, I dont find any changes for #1421942, #1487887, #1530240, #1547909 , #1530398 Bu only soem pluar in the wincondition. Can we get the relations correct? The actual fix is ok :-) -- https://code.launchpad.net/~widelands-dev/widelands/string-fixes/+merge/362944 Your team Widelands Developers

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1800364-reset-economy-counter into lp:widelands

2019-02-09 Thread Klaus Halfmann
OK, last_economy_serial_ becomes different for replays and causes desnycs. And restarting a game may result in desyncs as well. OK, I always try to keep my roads connected, so this may explain why this does not hit me that often. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1800364-

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1800364-reset-economy-counter into lp:widelands

2019-02-09 Thread Klaus Halfmann
Review: Approve review That code is straight forard. But please detail why this should fix some desyncs? (We had some nice Multiplayer games upto 2 vs 2 tese days but had some crashes from such desyncs.) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1800364-reset-economy-counter/

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

2019-02-09 Thread Klaus Halfmann
Review: Approve code review OK, for the Reeview: This branch substitutes a streaming JSON aproach with a Tree Object model. Streaming is normally better as of resource usage, but in this case this does not matter. If kaputtnik likes it his way, its all fine with me. (Solange nix kaputt geht ;-)

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1814372-lua-gametype into lp:widelands

2019-02-03 Thread Klaus Halfmann
Review: Approve review, compile Straighht forward, should not affect any current change but can be user for later lua scripts. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1814372-lua-gametype/+merge/362638 Your team Widelands Developers is subscribed to branch lp

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1814372-lua-gametype into lp:widelands

2019-02-02 Thread Klaus Halfmann
Mhh, I do not see any code changes and cannot pull the branch, whats the Problem? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1814372-lua-gametype/+merge/362638 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1814372-lua-

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-30 Thread Klaus Halfmann
Review: Approve compile, testplay We played this to the End on a 3vs3 Game on some large Map and had no problems whatsover. This way I finally learned Tonis game-name. Traviss had an error with apt.llvm.org port one build only. @bunnbot merge -- https://code.launchpad.net/~widelands-dev/widel

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/robust-file-saving into lp:widelands

2019-01-25 Thread Klaus Halfmann
Review: Resubmit merge Arty can, you take another look? Maybe my merge broke it lets try to compile this at leasst -- https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/robu

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/robust-file-saving into lp:widelands

2019-01-22 Thread Klaus Halfmann
Hello Arty I try to fix these conflicts, but maybe I will need you help? -- https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/robust-file-saving.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/robust-file-saving into lp:widelands

2019-01-19 Thread Klaus Halfmann
Review: Approve reaprove I think should get this in now, I dont think it will break anything. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/robust-file-sa

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

2019-01-16 Thread Klaus Halfmann
Review: Approve review compile, testplay Travis has only one (Timeout) Problem with GCC_VERSION="4.8" BUILD_TYPE="Debug" appveyor. has Issue in Configuration: Debug No artifacts found matching 'Widelands-_widelands_dev_widelands_campaignselect_box-4193-Debug-x64.exe' path strip -sv %APPVEYOR_BU

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

2019-01-13 Thread Klaus Halfmann
Anything I can test for this? -- https://code.launchpad.net/~widelands-dev/widelands/json_writer/+merge/357908 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/json_writer. ___ Mailing list: https://launchpad.net/~wide

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

2019-01-13 Thread Klaus Halfmann
* Bug #627361: Show available campaigns and missions greyed out Fine for me except: - after finishing the last available mission widelands should present a message like "Thank you for playing all available missions of widelands." Mhh, All excpet frisisans show "unimplemened" as last Sce

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

2019-01-13 Thread Klaus Halfmann
Review: Needs Fixing review, compile Findings: compiles locally on OSX. src/logic/campaign_visibility.h/.c was move to src/ui_fsmenu/campaigns.h/c campaigns.conf -> campaigns.ua (to speed upp reading, I assume) Many reefactorings around campains and scenarios Travis fails with Could

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811030-desync-ai into lp:widelands

2019-01-12 Thread Klaus Halfmann
as travis had only unrelated Issues with OSX / homebrew: @bunnybot merge force kapputnik: that savegame from #1800366 shoud be to old, but it smells just like thes situation I had. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811030-desync-ai/+merge/361689 Your team Widelands Deve

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811030-desync-ai into lp:widelands

2019-01-12 Thread Klaus Halfmann
Review: Approve compile test Tested this now, Notabilis hint was 100% correct, I got a desync right at the start. This can go in. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811030-desync-ai/+merge/361689 Your team Widelands Developers is subscribed to branch l

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811030-desync-ai into lp:widelands

2019-01-11 Thread Klaus Halfmann
Review: Approve review That code looks perfectly clear and should fix the bug. Nonetheless I will try to reproduce it in the next days. So this requires a Network game with at least two Humans and one AI player. Anyone else with that branch on Disk? Travis has only problems on OSX with homebrew,

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1796364-blinking-buildings into lp:widelands

2019-01-05 Thread Klaus Halfmann
A Debug builds now shows this, too: https://travis-ci.org/widelands/widelands/jobs/475766582 widelands: /home/travis/build/widelands/widelands/src/logic/map_objects/draw_text.h:38: TextToDraw operator~(TextToDraw): Assertion `result >= 0' failed. -- https://code.launchpad.net/~widelands-dev/wid

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1796364-blinking-buildings into lp:widelands

2019-01-05 Thread Klaus Halfmann
Oops, Benedikt is playing in Frisian/Platdütsch that complicates the Issue :-) I pulled the branch again and will try tommorow, hopefully merging trunk fixed the Assertion. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1796364-blinking-buildings/+merge/359348 Your team Widelands Deve

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1796364-blinking-buildings into lp:widelands

2019-01-05 Thread Klaus Halfmann
Review: Abstain debug I attached a Debuugger and starte to play my latest youtube Video: https://www.magentacloud.de/share/tu4ayusx.k Fie_: FRI01_H50_End.wgf The asser ht me amost immediatedly when Itried to open the Map. The TextToDraw Enum was 3 which is out of range, hence the assert. Gu

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1796364-blinking-buildings into lp:widelands

2019-01-05 Thread Klaus Halfmann
Review: Needs Fixing testplay Oops was killed with: Assertion failed: (result >= 0), function operator~, file ../src/logic/map_objects/draw_text.h, line 38. will ty to reproduce this in a debugger. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1796364-blinking-buildings/+merge/3593

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1796364-blinking-buildings into lp:widelands

2019-01-05 Thread Klaus Halfmann
Uhm, the savegame fails me: Assertion failed: (result >= 0), function operator~, file ../src/logic/map_objects/draw_text.h, line 38. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1796364-blinking-buildings/+merge/359348 Your team Widelands Developers is requested to review the propo

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1796364-blinking-buildings into lp:widelands

2019-01-05 Thread Klaus Halfmann
Hmm I don get these travis Errors, one looks like a timeout, next one hapens only att a pecific compile? /home/travis/build/widelands/widelands/doc/sphinx/source/autogen_ai_hints.rst:117: Definition list ends without a blank line; unexpected unindent. Will try to test this anyway -- https://code

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/robust-file-saving into lp:widelands

2018-12-25 Thread Klaus Halfmann
Review: Approve compile, test, debug OK, I did some tests with normal saves and some ways to break it. Experience tells me that Windows is more picky about such errors (as it locks open files). I was unable to reproduce all the errors. But anway, basic Saving is ok and nothing is broken. The code

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/robust-file-saving into lp:widelands

2018-12-24 Thread Klaus Halfmann
Hello artydent, I checked out this code an attached a debugger, I will try to gain some coverage an this way understand it. Gun: shall this become part of R20 or is this is "only" an improvemnet? Artydent: Im am using OSX, anything special that I should look for? -- https://code.launchpad.net/~w

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1800814-update-script into lp:widelands

2018-11-07 Thread Klaus Halfmann
Review: Approve compile.ch / 2 x update.sh Works as desigends, lets have it. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1800814-update-script/+merge/358419 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1800814-update-scrip

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1800814-update-script into lp:widelands

2018-11-07 Thread Klaus Halfmann
Had to manully inject code from number_of_cpus branch. code is straight foreward. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1800814-update-script/+merge/358419 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1800814-upd

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-net-error-game-connect into lp:widelands

2018-10-21 Thread Klaus Halfmann
Review: Approve review, compile, debug, test Complied this and did a "poor mans coverage" in the debugger. I was able to hit about 50% of the changed code. Hitting the rest would need tweaking of the metaserver or such. But this is all reasonable for me. @bunnybot mergre -- https://code.launchp

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1759857-territorial-lord into lp:widelands

2018-10-14 Thread Klaus Halfmann
Uhhm, is there a way to debug lua inside widelands? Checking that code just by reading is a pain. I will play some games with territoral time lord, will this be enough? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1759857-territorial-lord/+merge/355860 Your team Widelands Developers

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

2018-10-06 Thread Klaus Halfmann
Review: Approve OK, the portraits work as designed. But as we already have a real Islnad could we have some +/- real Perons portrait, too? Like https://de.wikipedia.org/wiki/Aldgisl Or from here: https://www.youtube.com/watch?v=cVZjSD6_WIQ Or from some local Museum? Fell free to merge, thou

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

2018-10-06 Thread Klaus Halfmann
Looks like my youtube video reminded you of something ;-) Will check this now. -- https://code.launchpad.net/~widelands-dev/widelands/fri-portraits/+merge/356221 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/fri-portraits into lp:wideland

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1791426-multiplayer-map-change into lp:widelands

2018-10-06 Thread Klaus Halfmann
Review: Approve compile, review, test Reproduced it on trunk, found it fixed here. Looks like these monster switches in gameclient.cc deserve a refactoring :-) One Comment inline. All fine for me. @bunnybot merge Diff comments: > === modified file 'src/network/gameclient.cc' > --- src/networ

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

2018-10-06 Thread Klaus Halfmann
The proposal to merge lp:~widelands-dev/widelands/bug_1794339_center_wo_parent into lp:widelands has been updated. Description changed to: Assert that Window has a parent instead of crashing. Keep d->modal as parent window in GameClient::run() and set to nullptr as late as possible. This shoul

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

2018-10-06 Thread Klaus Halfmann
The proposal to merge lp:~widelands-dev/widelands/bug_1794339_center_wo_parent into lp:widelands has been updated. Commit message changed to: Assert null access in Window::center_to_parent(), keep parent window in GameClient::run() For more details, see: https://code.launchpad.net/~widelands-d

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

2018-10-05 Thread Klaus Halfmann
Ok made this an assert -- https://code.launchpad.net/~widelands-dev/widelands/bug_1794339_center_wo_parent/+merge/355885 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug_1794339_center_wo_parent into lp:widelands. ___

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

2018-10-02 Thread Klaus Halfmann
Oops, I pushed to :parent, which was my local trunk. Now I see that I used tabs instead of spaces, uhm. Can earliest fix this tomorrow, perhpas. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1794339_center_wo_parent/+merge/355885 Your team Widelands Developers is requested to review t

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

2018-09-30 Thread Klaus Halfmann
Review: Approve review, compile, testcases OK, only one warning left. src/graphic/image_io.cc:99:6: warning: disabled expansion of recursive macro [-Wdisabled-macro-expansion] if (setjmp(png_jmpbuf(png_ptr))) { // NOLINT I see no sematic changes. Ran 42 tests in 1196.210s all fine.

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

2018-09-30 Thread Klaus Halfmann
Review: Approve review OK this just makes the compiler happy, will compile this after merging with bug-1699852-compiler-warning. WHich should actually get rid of all warnings (Wow!). -- https://code.launchpad.net/~widelands-dev/widelands/compiler_warnings_200809/+merge/355875 Your team Widelan

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1699852-compiler-warning into lp:widelands

2018-09-30 Thread Klaus Halfmann
Review: Approve review, compile No real changes, so I wont do any further tests. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1699852-compiler-warning/+merge/355884 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1699852-comp

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1699852-compiler-warning into lp:widelands

2018-09-30 Thread Klaus Halfmann
I still get these: .../src/logic/map_objects/tribes/ship.cc:705:3: warning: fallthrough annotation in unreachable code [-Wimplicit-fallthrough] FALLS_THROUGH; ../src/ui_basic/fileview_panel.h:50:23: warning: non-static data member 'style_' of 'FileViewPanel' shadows

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

2018-09-30 Thread Klaus Halfmann
Klaus Halfmann has proposed merging lp:~widelands-dev/widelands/bug_1794339_center_wo_parent into lp:widelands. Commit message: Avoid null access in Window::center_to_parent(), keep parent window in GameClient::run() Requested reviews: Widelands Developers (widelands-dev) Related bugs

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

2018-09-30 Thread Klaus Halfmann
The proposal to merge lp:~widelands-dev/widelands/bug_1794339_center_wo_parent into lp:widelands has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug_1794339_center_wo_parent/+merge/355873 -- Your team Widelands

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

2018-09-30 Thread Klaus Halfmann
This works for me, I dont think it will break anything, well. -- https://code.launchpad.net/~widelands-dev/widelands/bug_1794339_center_wo_parent/+merge/355873 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug_1794339_center_wo_parent into

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1699852-compiler-warning into lp:widelands

2018-09-29 Thread Klaus Halfmann
Thanks for fixing this one, syntax and usage of templates in C++ still give me a headache. I will try to review it,but I am running out of time for this weekend. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1699852-compiler-warning/+merge/355884 Your team Widelands Developers is re

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

2018-09-29 Thread Klaus Halfmann
In my case the root cause was a savegame incompatibilty. void GameClient::disconnect in network/gameclient.cc creates the messagebox as: UI::WLMessageBox mmb(d->modal, d is a GameClientImpl* and modal is explicily set as nullpointer in the CTor. modal is always set to the currently show win

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

2018-09-29 Thread Klaus Halfmann
Klaus Halfmann has proposed merging lp:~widelands-dev/widelands/bug_1794339_center_wo_parent into lp:widelands. Commit message: Avoid null access in Window::center_to_parent() Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1794339 in widelands: "segfault jo

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/per-level-soldier-anims into lp:widelands

2018-09-28 Thread Klaus Halfmann
Gun/Benedikt we are actually at feature freeze and this _is_ a new feature. I have no idea about the risks involved and the testing needed for this. So I propose defering this to R21. What do you think? -- https://code.launchpad.net/~widelands-dev/widelands/per-level-soldier-anims/+merge/354929 Y

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine into lp:widelands

2018-09-28 Thread Klaus Halfmann
Review: Approve compile, review, testplay (osx, win10, linux) Played this with kapputnik yesterday for about an hour or such. We both finally got the Idea about the Map and how smuggling works :-) We had no desnycs whatsover so this can go in. Gun: Do you want to open Smugglers for all tribes in

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine into lp:widelands

2018-09-28 Thread Klaus Halfmann
Played this on OSX/localhost only, stating on the last save, no Issues. Gun: perhpas we need som real world unreliable network to do more test? I will try to come online this weekend perhpas we can do some smuggling? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1751440-smugglers-de

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine into lp:widelands

2018-09-28 Thread Klaus Halfmann
OK after cleaning my confusion about the two builds I will try a 2xWIN/ 2xOSX Smugglers game. ... some hours later ... Did so and it was quite ok until I ran into #1794965 I think this is unrelated. But I may not be able to continue testing. I will try other variants, later -- https://code.lau

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine into lp:widelands

2018-09-27 Thread Klaus Halfmann
When trying to Debug WIndows I gita Problem with system.out, as described in: #1398536 I will try to debug this anyway -- https://code.launchpad.net/~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine/+merge/354747 Your team Widelands Developers is requested to review the

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine into lp:widelands

2018-09-27 Thread Klaus Halfmann
OK after cleaning my confusion about the two builds I will try a 2xWIN/ 2xOSX Smugglers game. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1751440-smugglers-desync-single-coroutine/+merge/354747 Your team Widelands Developers is requested to review the proposed merge of lp:~widela

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-858292-military-influence into lp:widelands

2018-09-27 Thread Klaus Halfmann
OK cleanup_playerimmovables_area was duplicate code and moved to do_conquer_area(). -- https://code.launchpad.net/~widelands-dev/widelands/bug-858292-military-influence/+merge/353834 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-858

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-858292-military-influence into lp:widelands

2018-09-26 Thread Klaus Halfmann
Mhh, don understand that (single) failure on travis: CMake Error at /usr/local/cmake-3.9.2/share/cmake-3.9/Modules/CMakeDetermineCCompiler.cmake:48 (message): Could not find compiler set in environment variable CC: -- https://code.launchpad.net/~widelands-dev/widelands/bug-858292-military-inf

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-858292-military-influence into lp:widelands

2018-09-26 Thread Klaus Halfmann
Mhh, why can you remove that code and fix it via that other code? I must take a complete look at this logic. I will not try to use this replay, but try to reproduce it, which will become quite tricky. Any tips how I can provoke this fast and easy? One nit inline. Diff comments: > === modified

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1746270-rolling-autosave-file-permissions into lp:widelands

2018-09-26 Thread Klaus Halfmann
Review: Approve testplay on windows OK, tested along the original Bug, works as desigend. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1746270-rolling-autosave-file-permissions/+merge/353758 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/w

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1746270-rolling-autosave-file-permissions into lp:widelands

2018-09-25 Thread Klaus Halfmann
Review: Approve review, compile, debug Tested this on OSX, but found the original Problem was on Windows. The code works as expected but I did not trigger the original Problem. e.g. when I make the complete save dir unreadable the save completly fails, well. OSX and *nixes behave differnt compar

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1746270-rolling-autosave-file-permissions into lp:widelands

2018-09-24 Thread Klaus Halfmann
Looks like Its time to switch on the debugger. Will compile this and think about all kind of nasty ways to break this :-) Could we inform the player if this finally failes? Code LGTM. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1746270-rolling-autosave-file-permissions/+merge/353

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1480927-building-texts into lp:widelands

2018-09-24 Thread Klaus Halfmann
Review: Approve One trasient failure on travis: Failed to connect to apt.llvm.org port 80: Connection timed out @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/bug-1480927-building-texts/+merge/353728 Your team Widelands Developers is subscribed to branch lp:~widel

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1480927-building-texts into lp:widelands

2018-09-23 Thread Klaus Halfmann
Review: Approve compile, review, testplay Now ok for me, will still do some testplaye, feel free to merge. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1480927-building-texts/+merge/353728 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-148092

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1480927-building-texts into lp:widelands

2018-09-22 Thread Klaus Halfmann
Review: Needs Fixing review, compile, testplay played this for a while found not noticable difference. Still we should fix the pass by reference for the enum, Ill do so next time I get access to this computer. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1480927-building-texts/+mer

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1480927-building-texts into lp:widelands

2018-09-22 Thread Klaus Halfmann
Checked that code, I do not _really_ know what you where doing here but its looks reasonable. One remark inline, I think we should change this. I will test this assuming that I notice almost no differecne except that no text is hidden. -- https://code.launchpad.net/~widelands-dev/widelands/bug

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

2018-09-07 Thread Klaus Halfmann
Review: Approve code review Code looks ok and will not afect normal builds in any way. Go for it. I cannot use this anyway as I use Macports. -- https://code.launchpad.net/~widelands-dev/widelands/macos_build_app_ICU/+merge/354490 Your team Widelands Developers is subscribed to branch lp:~widel

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

2018-08-26 Thread Klaus Halfmann
Review: Approve compile, review, test, Works for me, I cannot judge mutch abouthe the code. ypopezios: you are correct, but lets get rid of the bug first and do the improvement in some second step. Gun: will we need a resubmit before wer can merge this? @bonnybot merge -- https://code.launch

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

2018-08-24 Thread Klaus Halfmann
Review: Needs Fixing Checked out and compiled the branch, reproduced the bug on trunk. (SEGV on unknown address 0x00f0 ...) This version loads the map without any hazzles. * Bug: when creating a new Random Map (after loading one) the number of players is stuck at 2 and cannot be inc

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands

2018-08-22 Thread Klaus Halfmann
Now I am confused. That code should be semantically identical? As peformance optimizer I see the the extra null-assignement is a waste :-) Can you add a comment what compiler/enviroment causes this problem? I tested this in bzr8791[trunk] and it was ok for me. -- https://code.launchpad.net/~wid

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

2018-08-15 Thread Klaus Halfmann
Review: Approve dowload,short testrun Approve, checked both images, (debug build is 50M, my develop build is 67M but all libs are staically linked) I only tried to open a savegame. (which was from some other brach, and failed to load, well) travis currebtly has some erro as of a broken test /

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

2018-08-15 Thread Klaus Halfmann
@Toni We already have a description to build developer verions using macports: https://wl.widelands.org/wiki/Download/ And some dsicussion around this as well: https://wl.widelands.org/forum/topic/2627/ And the corresponding Portfile, which includes some packagnh, too: https://github.com/kencu

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

2018-08-14 Thread Klaus Halfmann
I rember that SirVer used brew as well for R19 and I used macports and we had some significat differences in the end, was much bigger, or such. SirVer, Gun: do we have that Ticket still in some Archive? Once we target R20 we should use all variants and use the smalles/fastest on. Lets see. -- h

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

2018-08-14 Thread Klaus Halfmann
Mmh, this does not contain the ASAN Flags as we have them in compile.sh, this would make sense for a debug build (yes, it _is_ slower :-) OTOH wee need this just for the release in then End, I would guess. I use macports so lets try this now wlbuild klaus$ ./build_app.sh clang debug ../maco

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

2018-08-14 Thread Klaus Halfmann
Toni, you re doing a great job! Just give me some time to catch up with your speed :-) You may contact me via PM or such, so we can align on our skills. Ill try to review this this week but will not promise anything ... -- https://code.launchpad.net/~widelands-dev/widelands/macos_build_app_comp

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition into lp:widelands

2018-08-12 Thread Klaus Halfmann
Mhh, travis had no sucess with any instalaltions, but did not actually compile anything on linux. Gun: can this go in anyway or shall we resubmit this? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition/+merge/349594 Your team Widelands Developers is

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

2018-08-11 Thread Klaus Halfmann
The proposal to merge lp:~widelands-dev/widelands/odd_locale_fix into lp:widelands has been updated. Commit message changed to: Fix undefined / unexpected behaviour in case LANG environment is empty. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/odd_locale_fix/+merg

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

2018-08-11 Thread Klaus Halfmann
Klaus Halfmann has proposed merging lp:~widelands-dev/widelands/odd_locale_fix into lp:widelands. Commit message: Fix udefined / unexpected bahviour in case LANG enviromane is empty. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition into lp:widelands

2018-07-31 Thread Klaus Halfmann
I still have no idea what is_warping_ actually means :-). It was my best idea to leave the original code there while fixing the bug. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition/+merge/349594 Your team Widelands Developers is subscribed to branch

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition into lp:widelands

2018-07-29 Thread Klaus Halfmann
Review: Resubmit I assume this bug was there since perhaps R18 but can only be found with ASAN. Now I need someone else for the review. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition/+merge/349594 Your team Widelands Developers is subscribed to bra

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition into lp:widelands

2018-07-29 Thread Klaus Halfmann
reproducing was (to) easy, just clik on the workarea button and right click immideatley after that. Problem is that ~BuildingWindow() calls hide_workarea(); calling configure_workarea_button() whhich tries to change the tooltip of the already deleted button. I can provide a quick fix which woul

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition into lp:widelands

2018-07-28 Thread Klaus Halfmann
Review: Needs Fixing I will try to merge trunk, which contains a workaroud for #1784200. Until then I wont be able to debug this any further :\ -- https://code.launchpad.net/~widelands-dev/widelands/bug-1619402-port-work-area-on-expedition/+merge/349594 Your team Widelands Developers is subscribe

<    1   2   3   4   5   6   7   >