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

2018-08-22 Thread Notabilis 
*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

2018-08-22 Thread Notabilis 
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 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

2018-08-22 Thread noreply 
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

2018-08-22 Thread GunChleoc 
> 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

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/~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

2018-08-21 Thread Notabilis 
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

2018-08-20 Thread bunnybot 
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

2018-08-20 Thread GunChleoc 
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