Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands
*with a debug build, sorry. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands
That bug is really fun but the streamreader isn't to blame. ;-) The problem is that the float_32() call not only returns the value but also modifies the internal state of the stream. As a consequence, the order of assignment to x and y matters (or more precisely, the order in which the respective float_32() calls are made). It seems as if calling it twice as constructor parameters results in the *second* call being done first, resulting in x and y being swapped after the assignment. Doing only one call as parameter and assigning the other one later on is fine. Or just assign both later on as in this branch. Took me quite some time of trial&error until I realized that... I am using gcc version 8.2.0 with a release build. The order of the evaluation is undefined in the standard, so it might indeed be compiler dependent. The null-assignment is required since the default constructor is deleted. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands
> That code should be semantically identical? That's what I thought too when I broke it. I really don't want to dig into the streamreader right now though - there is some subtle unexpected side-effect going on there. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands
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/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands
Review: Approve Code is looking good and working as intended. That definitely is one nasty bug... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands
Continuous integration builds have changed state: Travis build 3819. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/418129123. Appveyor build 3618. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1784122_singleplayer_viewport-3618. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands. Commit message: Fix saveloading of interactive player's viewport Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1784122 in widelands: "When new singleplayer game is started, the viewport is on (0, 0)" https://bugs.launchpad.net/widelands/+bug/1784122 For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1784122-singleplayer-viewport/+merge/353391 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1784122-singleplayer-viewport into lp:widelands. === modified file 'src/game_io/game_interactive_player_packet.cc' --- src/game_io/game_interactive_player_packet.cc 2018-07-08 15:16:16 + +++ src/game_io/game_interactive_player_packet.cc 2018-08-20 07:44:00 + @@ -59,7 +59,11 @@ if (player_number > max) throw GameDataError("The game has no players!"); } - Vector2f center_map_pixel(fr.float_32(), fr.float_32()); + + Vector2f center_map_pixel = Vector2f::zero(); + center_map_pixel.x = fr.float_32(); + center_map_pixel.y = fr.float_32(); + uint32_t const display_flags = fr.unsigned_32(); if (InteractiveBase* const ibase = game.get_ibase()) { ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp