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

[GunChleoc]

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

[Toni Förster (stonerl)]

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

[kaputtnik (franku)]

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

[bunnybot (widelandsofficial)]

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.

[GunChleoc (gunchleoc)]

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.

[Toni Förster (stonerl)]

@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.

[Toni Förster (stonerl)]

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)

[GunChleoc (gunchleoc)]

Works for me - let's have it!

@bunnybot merge

[bunnybot (widelandsofficial)]

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.

=== modified file 'data/scripting/win_conditions/territorial_lord.lua'
--- data/scripting/win_conditions/territorial_lord.lua	2019-02-24 19:41:36 +0000
+++ data/scripting/win_conditions/territorial_lord.lua	2019-03-02 10:36:17 +0000
@@ -23,22 +23,18 @@
    "that area for at least 20 minutes."
 )
 
--- Table of fields that are worth conquering
-local fields = {}
-
 return {
    name = wc_name,
    description = wc_desc,
-   init = function()
-      -- Get all valuable fields of the map
-      fields = wl.Game().map.conquerable_fields
-   end,
    func = function()
       local plrs = wl.Game().players
 
       -- set the objective with the game type for all players
       broadcast_objective("win_condition", wc_descname, wc_desc)
 
+      -- Table of fields that are worth conquering
+      local fields = wl.Game().map.conquerable_fields
+
       -- Configure how long the winner has to hold on to the territory
       local time_to_keep_territory = 20 * 60 -- 20 minutes
       -- time in secs, if == 0 -> victory

=== modified file 'data/scripting/win_conditions/territorial_time.lua'
--- data/scripting/win_conditions/territorial_time.lua	2019-02-24 19:41:36 +0000
+++ data/scripting/win_conditions/territorial_time.lua	2019-03-02 10:36:17 +0000
@@ -27,22 +27,18 @@
    "after 4 hours, whichever comes first."
 )
 
--- Table of fields that are worth conquering
-local fields = {}
-
 return {
    name = wc_name,
    description = wc_desc,
-   init = function()
-      -- Get all valuable fields of the map
-      fields = wl.Game().map.conquerable_fields
-   end,
    func = function()
       local plrs = wl.Game().players
 
       -- set the objective with the game type for all players
       broadcast_objective("win_condition", wc_descname, wc_desc)
 
+      -- Table of fields that are worth conquering
+      local fields = wl.Game().map.conquerable_fields
+
       -- variables to track the maximum 4 hours of gametime
       local remaining_max_time = 4 * 60 * 60 -- 4 hours
 

=== modified file 'data/scripting/win_conditions/wood_gnome.lua'
--- data/scripting/win_conditions/wood_gnome.lua	2019-02-24 19:41:36 +0000
+++ data/scripting/win_conditions/wood_gnome.lua	2019-03-02 10:36:17 +0000
@@ -21,17 +21,9 @@
 [[playing. The one with the most trees at that point will win the game.]])
 local wc_trees_owned = _"Trees owned"
 
-
--- Table of terrestrial fields
-local fields = {}
-
 return {
    name = wc_name,
    description = wc_desc,
-   init = function()
-      -- Get all terrestrial fields of the map
-      fields = wl.Game().map.terrestrial_fields
-   end,
    func = function()
    local plrs = wl.Game().players
    local game = wl.Game()
@@ -42,6 +34,9 @@
    -- set the objective with the game type for all players
    broadcast_objective("win_condition", wc_descname, wc_desc)
 
+   -- Table of terrestrial fields
+   local fields = wl.Game().map.terrestrial_fields
+
    -- The function to calculate the current points.
    local _last_time_calculated = -100000
    local _plrpoints = {}

=== modified file 'src/logic/game.cc'
--- src/logic/game.cc	2019-02-26 05:56:01 +0000
+++ src/logic/game.cc	2019-03-02 10:36:17 +0000
@@ -314,32 +314,17 @@
 
 	// Check for win_conditions
 	if (!settings.scenario) {
-		win_condition_script_ = settings.win_condition_script;
-		std::unique_ptr<LuaTable> table(lua().run_script(win_condition_script_));
+		std::unique_ptr<LuaTable> table(lua().run_script(settings.win_condition_script));
+		get_ibase()->log_message(_("Initializing game…"));
 		table->do_not_warn_about_unaccessed_keys();
 		win_condition_displayname_ = table->get_string("name");
-		// We run the actual win condition from InteractiveGameBase::start() to prevent a pure black
-		// screen while the game is being started - we can display a message there.
+		std::unique_ptr<LuaCoroutine> cr = table->get_coroutine("func");
+		enqueue_command(new CmdLuaCoroutine(get_gametime() + 100, std::move(cr)));
 	} else {
 		win_condition_displayname_ = "Scenario";
 	}
 }
 
-void Game::run_win_condition() {
-	if (!win_condition_script_.empty()) {
-		std::unique_ptr<LuaTable> table(lua().run_script(win_condition_script_));
-		table->do_not_warn_about_unaccessed_keys();
-		// Run separate initialization function if it is there.
-		if (table->has_key<std::string>("init")) {
-			std::unique_ptr<LuaCoroutine> cr = table->get_coroutine("init");
-			cr->resume();
-		}
-		std::unique_ptr<LuaCoroutine> cr = table->get_coroutine("func");
-		enqueue_command(new CmdLuaCoroutine(get_gametime() + 100, std::move(cr)));
-		win_condition_script_ = "";
-	}
-}
-
 /**
  * Initialize the savegame based on the given settings.
  * At return the game is at the same state like a map loaded with Game::init()

=== modified file 'src/logic/game.h'
--- src/logic/game.h	2019-02-24 22:50:04 +0000
+++ src/logic/game.h	2019-03-02 10:36:17 +0000
@@ -177,9 +177,6 @@
 	void init_newgame(UI::ProgressWindow* loader_ui, const GameSettings&);
 	void init_savegame(UI::ProgressWindow* loader_ui, const GameSettings&);
 
-	/// Run the win condition that is defined by win_condition_script_
-	void run_win_condition();
-
 	enum StartGameType { NewSPScenario, NewNonScenario, Loaded, NewMPScenario };
 
 	bool run(UI::ProgressWindow* loader_ui,
@@ -407,7 +404,6 @@
 
 	/// For save games and statistics generation
 	std::string win_condition_displayname_;
-	std::string win_condition_script_;
 	bool replay_;
 };
 

=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2019-02-26 05:56:01 +0000
+++ src/logic/map.cc	2019-03-02 10:36:17 +0000
@@ -262,7 +262,8 @@
 
 	std::set<FCoords> coords_to_check;
 
-	ScopedTimer timer("Calculating valuable fields took %ums");
+	log("Collecting valuable fields ... ");
+	ScopedTimer timer("took %ums");
 
 	// If we don't have the given coordinates yet, walk the map and collect conquerable fields,
 	// initialized with the given radius around the coordinates
@@ -341,12 +342,13 @@
 		}
 	}
 
-	log("Found %" PRIuS " valuable fields\n", result.size());
+	log("%" PRIuS " found ... ", result.size());
 	return result;
 }
 
 std::set<FCoords> Map::calculate_all_fields_excluding_caps(NodeCaps caps) const {
-	ScopedTimer timer("Calculating valuable fields took %ums");
+	log("Collecting valuable fields ... ");
+	ScopedTimer timer("took %ums");
 
 	std::set<FCoords> result;
 	for (MapIndex i = 0; i < max_index(); ++i) {
@@ -356,7 +358,7 @@
 		}
 	}
 
-	log("Found %" PRIuS " valuable fields\n", result.size());
+	log("%" PRIuS " found ... ", result.size());
 	return result;
 }
 

=== modified file 'src/wui/interactive_gamebase.cc'
--- src/wui/interactive_gamebase.cc	2019-02-27 17:19:00 +0000
+++ src/wui/interactive_gamebase.cc	2019-03-02 10:36:17 +0000
@@ -156,8 +156,6 @@
 }
 
 void InteractiveGameBase::start() {
-	game().run_win_condition();
-
 	// Multiplayer games don't save the view position, so we go to the starting position instead
 	if (is_multiplayer()) {
 		Widelands::PlayerNumber pln = player_number();