Merge lp:~widelands-dev/widelands/bug-1818227-replay-desync into lp:widelands

Proposed by GunChleoc
Status: Merged
Merged at revision: 9003
Proposed branch: lp:~widelands-dev/widelands/bug-1818227-replay-desync
Merge into: lp:widelands
Diff against target: 206 lines (+19/-51)
7 files modified
data/scripting/win_conditions/territorial_lord.lua (+3/-7)
data/scripting/win_conditions/territorial_time.lua (+3/-7)
data/scripting/win_conditions/wood_gnome.lua (+3/-8)
src/logic/game.cc (+4/-19)
src/logic/game.h (+0/-4)
src/logic/map.cc (+6/-4)
src/wui/interactive_gamebase.cc (+0/-2)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1818227-replay-desync
Reviewer Review Type Date Requested Status
Toni Förster tested Approve
Review via email: mp+363871@code.launchpad.net

Commit message

Move running of win condition scripts back to Game::init_newgame to fix desyncing replays.

To post a comment you must log in.
Revision history for this message
Toni Förster (stonerl) wrote :

Now we have the problem with 4 Minutes of writing scripting data for big maps like 'Magic Mountain', again.

Writing Scripting Data ... took 116673ms
.
.
.
Writing Scripting Data ... took 97252ms

review: Needs Fixing
Revision history for this message
kaputtnik (franku) wrote :

Please revert the changes made in r8993, otherwise i fear we will never get a new official release.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4550. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/500454380.
Appveyor build 4337. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1818227_replay_desync-4337.

9004. By GunChleoc

Remove extra init function from win conditions.

9005. By GunChleoc

Improved console logging.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I have removed the extra init function, this way the fields get added after the replay is saveloaded. This means we now get a black screen for big maps.

I can't get any log messages on the screen there, but I have improved the console logging a bit.

9006. By Toni Förster

tell the user that we initialize the game

Revision history for this message
Toni Förster (stonerl) wrote :

@GunChleoc I readded the "Initializing Game..." message. So we have a black screen but the user gets some feedback. I think this is a good compromise.

Revision history for this message
Toni Förster (stonerl) wrote :

If you do not plan any further changes, I would approve it like this.

We should open a bug targeted for build-21 and try to move the calculations to init screen again, to avoid the black screen entirely.

review: Approve (tested)
Revision history for this message
GunChleoc (gunchleoc) wrote :

Works for me - let's have it!

@bunnybot merge

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4555. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/500750597.
Appveyor build 4342. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1818227_replay_desync-4342.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/scripting/win_conditions/territorial_lord.lua'
2--- data/scripting/win_conditions/territorial_lord.lua 2019-02-24 19:41:36 +0000
3+++ data/scripting/win_conditions/territorial_lord.lua 2019-03-02 10:36:17 +0000
4@@ -23,22 +23,18 @@
5 "that area for at least 20 minutes."
6 )
7
8--- Table of fields that are worth conquering
9-local fields = {}
10-
11 return {
12 name = wc_name,
13 description = wc_desc,
14- init = function()
15- -- Get all valuable fields of the map
16- fields = wl.Game().map.conquerable_fields
17- end,
18 func = function()
19 local plrs = wl.Game().players
20
21 -- set the objective with the game type for all players
22 broadcast_objective("win_condition", wc_descname, wc_desc)
23
24+ -- Table of fields that are worth conquering
25+ local fields = wl.Game().map.conquerable_fields
26+
27 -- Configure how long the winner has to hold on to the territory
28 local time_to_keep_territory = 20 * 60 -- 20 minutes
29 -- time in secs, if == 0 -> victory
30
31=== modified file 'data/scripting/win_conditions/territorial_time.lua'
32--- data/scripting/win_conditions/territorial_time.lua 2019-02-24 19:41:36 +0000
33+++ data/scripting/win_conditions/territorial_time.lua 2019-03-02 10:36:17 +0000
34@@ -27,22 +27,18 @@
35 "after 4 hours, whichever comes first."
36 )
37
38--- Table of fields that are worth conquering
39-local fields = {}
40-
41 return {
42 name = wc_name,
43 description = wc_desc,
44- init = function()
45- -- Get all valuable fields of the map
46- fields = wl.Game().map.conquerable_fields
47- end,
48 func = function()
49 local plrs = wl.Game().players
50
51 -- set the objective with the game type for all players
52 broadcast_objective("win_condition", wc_descname, wc_desc)
53
54+ -- Table of fields that are worth conquering
55+ local fields = wl.Game().map.conquerable_fields
56+
57 -- variables to track the maximum 4 hours of gametime
58 local remaining_max_time = 4 * 60 * 60 -- 4 hours
59
60
61=== modified file 'data/scripting/win_conditions/wood_gnome.lua'
62--- data/scripting/win_conditions/wood_gnome.lua 2019-02-24 19:41:36 +0000
63+++ data/scripting/win_conditions/wood_gnome.lua 2019-03-02 10:36:17 +0000
64@@ -21,17 +21,9 @@
65 [[playing. The one with the most trees at that point will win the game.]])
66 local wc_trees_owned = _"Trees owned"
67
68-
69--- Table of terrestrial fields
70-local fields = {}
71-
72 return {
73 name = wc_name,
74 description = wc_desc,
75- init = function()
76- -- Get all terrestrial fields of the map
77- fields = wl.Game().map.terrestrial_fields
78- end,
79 func = function()
80 local plrs = wl.Game().players
81 local game = wl.Game()
82@@ -42,6 +34,9 @@
83 -- set the objective with the game type for all players
84 broadcast_objective("win_condition", wc_descname, wc_desc)
85
86+ -- Table of terrestrial fields
87+ local fields = wl.Game().map.terrestrial_fields
88+
89 -- The function to calculate the current points.
90 local _last_time_calculated = -100000
91 local _plrpoints = {}
92
93=== modified file 'src/logic/game.cc'
94--- src/logic/game.cc 2019-02-26 05:56:01 +0000
95+++ src/logic/game.cc 2019-03-02 10:36:17 +0000
96@@ -314,32 +314,17 @@
97
98 // Check for win_conditions
99 if (!settings.scenario) {
100- win_condition_script_ = settings.win_condition_script;
101- std::unique_ptr<LuaTable> table(lua().run_script(win_condition_script_));
102+ std::unique_ptr<LuaTable> table(lua().run_script(settings.win_condition_script));
103+ get_ibase()->log_message(_("Initializing game…"));
104 table->do_not_warn_about_unaccessed_keys();
105 win_condition_displayname_ = table->get_string("name");
106- // We run the actual win condition from InteractiveGameBase::start() to prevent a pure black
107- // screen while the game is being started - we can display a message there.
108+ std::unique_ptr<LuaCoroutine> cr = table->get_coroutine("func");
109+ enqueue_command(new CmdLuaCoroutine(get_gametime() + 100, std::move(cr)));
110 } else {
111 win_condition_displayname_ = "Scenario";
112 }
113 }
114
115-void Game::run_win_condition() {
116- if (!win_condition_script_.empty()) {
117- std::unique_ptr<LuaTable> table(lua().run_script(win_condition_script_));
118- table->do_not_warn_about_unaccessed_keys();
119- // Run separate initialization function if it is there.
120- if (table->has_key<std::string>("init")) {
121- std::unique_ptr<LuaCoroutine> cr = table->get_coroutine("init");
122- cr->resume();
123- }
124- std::unique_ptr<LuaCoroutine> cr = table->get_coroutine("func");
125- enqueue_command(new CmdLuaCoroutine(get_gametime() + 100, std::move(cr)));
126- win_condition_script_ = "";
127- }
128-}
129-
130 /**
131 * Initialize the savegame based on the given settings.
132 * At return the game is at the same state like a map loaded with Game::init()
133
134=== modified file 'src/logic/game.h'
135--- src/logic/game.h 2019-02-24 22:50:04 +0000
136+++ src/logic/game.h 2019-03-02 10:36:17 +0000
137@@ -177,9 +177,6 @@
138 void init_newgame(UI::ProgressWindow* loader_ui, const GameSettings&);
139 void init_savegame(UI::ProgressWindow* loader_ui, const GameSettings&);
140
141- /// Run the win condition that is defined by win_condition_script_
142- void run_win_condition();
143-
144 enum StartGameType { NewSPScenario, NewNonScenario, Loaded, NewMPScenario };
145
146 bool run(UI::ProgressWindow* loader_ui,
147@@ -407,7 +404,6 @@
148
149 /// For save games and statistics generation
150 std::string win_condition_displayname_;
151- std::string win_condition_script_;
152 bool replay_;
153 };
154
155
156=== modified file 'src/logic/map.cc'
157--- src/logic/map.cc 2019-02-26 05:56:01 +0000
158+++ src/logic/map.cc 2019-03-02 10:36:17 +0000
159@@ -262,7 +262,8 @@
160
161 std::set<FCoords> coords_to_check;
162
163- ScopedTimer timer("Calculating valuable fields took %ums");
164+ log("Collecting valuable fields ... ");
165+ ScopedTimer timer("took %ums");
166
167 // If we don't have the given coordinates yet, walk the map and collect conquerable fields,
168 // initialized with the given radius around the coordinates
169@@ -341,12 +342,13 @@
170 }
171 }
172
173- log("Found %" PRIuS " valuable fields\n", result.size());
174+ log("%" PRIuS " found ... ", result.size());
175 return result;
176 }
177
178 std::set<FCoords> Map::calculate_all_fields_excluding_caps(NodeCaps caps) const {
179- ScopedTimer timer("Calculating valuable fields took %ums");
180+ log("Collecting valuable fields ... ");
181+ ScopedTimer timer("took %ums");
182
183 std::set<FCoords> result;
184 for (MapIndex i = 0; i < max_index(); ++i) {
185@@ -356,7 +358,7 @@
186 }
187 }
188
189- log("Found %" PRIuS " valuable fields\n", result.size());
190+ log("%" PRIuS " found ... ", result.size());
191 return result;
192 }
193
194
195=== modified file 'src/wui/interactive_gamebase.cc'
196--- src/wui/interactive_gamebase.cc 2019-02-27 17:19:00 +0000
197+++ src/wui/interactive_gamebase.cc 2019-03-02 10:36:17 +0000
198@@ -156,8 +156,6 @@
199 }
200
201 void InteractiveGameBase::start() {
202- game().run_win_condition();
203-
204 // Multiplayer games don't save the view position, so we go to the starting position instead
205 if (is_multiplayer()) {
206 Widelands::PlayerNumber pln = player_number();

Subscribers

People subscribed via source and target branches

to status/vote changes: