Merge lp:~widelands-dev/widelands/bug-1811030-desync-ai into lp:widelands

Proposed by Notabilis
Status: Merged
Merged at revision: 8957
Proposed branch: lp:~widelands-dev/widelands/bug-1811030-desync-ai
Merge into: lp:widelands
Diff against target: 40 lines (+5/-5)
1 file modified
src/ai/defaultai_seafaring.cc (+5/-5)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1811030-desync-ai
Reviewer Review Type Date Requested Status
Klaus Halfmann compile test Approve
Review via email: mp+361689@code.launchpad.net

Commit message

Replacing logic_rand() with std::rand() in seafaring code of AI.
Should fix desyncs while network gaming.

Description of the change

Calls of logic_rand() have to be done on all participants of a network game. Since the AI code is only executed on the host, calling logic_rand() leads to different random numbers on the participants computers later on, resulting in desynchronized games.

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4391. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/478537625.
Appveyor build 4182. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1811030_desync_ai-4182.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

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, as I am compiling on OSX
using MacPorts I will verify OSX, too.

review: Approve (review)
Revision history for this message
Notabilis (notabilis27) wrote :

You should be able to test it with the savegame attached to the linked bug report. With trunk, the savegame desyncs immediately. With this branch you can continue playing. I tested it with two widelands instances running on my system, so no second computer/human should be required for testing.

If trying to reproduce the desync in a new game you have to wait until the AI starts expeditions, which might take some time.

Revision history for this message
kaputtnik (franku) wrote :

I am compiling this branch. Klaus, i will try to be in the lobby the next days.

Playing the map 'Crossing the Horizon' may speed up testing.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

Tested this now, Notabilis hint was 100% correct, I got a desync right at the start.

This can go in.

@bunnybot merge

review: Approve (compile test)
Revision history for this message
kaputtnik (franku) wrote :

Notabilis, is this bug maybe also fixed with this branch?

https://bugs.launchpad.net/widelands/+bug/1800366

Revision history for this message
bunnybot (widelandsofficial) wrote :

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 4391. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/478537625.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

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.

Revision history for this message
Notabilis (notabilis27) wrote :

I compiled the game revision of the other bug report with the changes of this branch applied and it indeed seems to fix the issues there.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/ai/defaultai_seafaring.cc'
2--- src/ai/defaultai_seafaring.cc 2018-09-04 15:48:47 +0000
3+++ src/ai/defaultai_seafaring.cc 2019-01-11 21:07:24 +0000
4@@ -455,7 +455,7 @@
5 }
6
7 Widelands::IslandExploreDirection DefaultAI::randomExploreDirection() {
8- return game().logic_rand() % 20 < 10 ? Widelands::IslandExploreDirection::kClockwise :
9+ return std::rand() % 20 < 10 ? Widelands::IslandExploreDirection::kClockwise :
10 Widelands::IslandExploreDirection::kCounterClockwise;
11 }
12
13@@ -486,7 +486,7 @@
14 spot_score);
15
16 // we make a decision based on the score value and random
17- if (game().logic_rand() % 8 < spot_score) {
18+ if (std::rand() % 8 < spot_score) {
19 // we build a port here
20 game().send_player_ship_construct_port(*so.ship, so.ship->exp_port_spaces().front());
21 so.last_command_time = gametime;
22@@ -579,15 +579,15 @@
23 assert(possible_directions.size() >= new_teritory_directions.size());
24
25 // If only open sea (no unexplored sea) is found, we don't always divert the ship
26- if (new_teritory_directions.empty() && game().logic_rand() % 100 < 80) {
27+ if (new_teritory_directions.empty() && std::rand() % 100 < 80) {
28 return false;
29 }
30
31 if (!possible_directions.empty() || !new_teritory_directions.empty()) {
32 const Direction direction =
33 !new_teritory_directions.empty() ?
34- new_teritory_directions.at(game().logic_rand() % new_teritory_directions.size()) :
35- possible_directions.at(game().logic_rand() % possible_directions.size());
36+ new_teritory_directions.at(std::rand() % new_teritory_directions.size()) :
37+ possible_directions.at(std::rand() % possible_directions.size());
38 game().send_player_ship_scouting_direction(*so.ship, static_cast<WalkingDir>(direction));
39
40 log("%d: %s: exploration - breaking for %s sea, dir=%u\n", pn,

Subscribers

People subscribed via source and target branches

to status/vote changes: