Fix #9720: Delay start of GS/AI to after loading of savegame (#9745)

src/ai/ai.hpp

@@ -128,11 +128,6 @@ public:
 	 */
 	static void Save(CompanyID company);
 
-	/**
-	 * Load data for an AI from a savegame.
-	 */
-	static void Load(CompanyID company, int version);
-
 	/**
 	 * Get the number of days before the next AI should start.
 	 */

src/ai/ai_core.cpp

@@ -57,6 +57,8 @@
 	assert(c->ai_instance == nullptr);
 	c->ai_instance = new AIInstance();
 	c->ai_instance->Initialize(info);
+	c->ai_instance->LoadOnStack(config->GetToLoadData());
+	config->SetToLoadData(nullptr);
 
 	cur_company.Restore();
 
@@ -289,21 +291,6 @@
 	}
 }
 
-/* static */ void AI::Load(CompanyID company, int version)
-{
-	if (!_networking || _network_server) {
-		Company *c = Company::GetIfValid(company);
-		assert(c != nullptr && c->ai_instance != nullptr);
-
-		Backup<CompanyID> cur_company(_current_company, company, FILE_LINE);
-		c->ai_instance->Load(version);
-		cur_company.Restore();
-	} else {
-		/* Read, but ignore, the load data */
-		AIInstance::LoadEmpty();
-	}
-}
-
 /* static */ int AI::GetStartNextTime()
 {
 	/* Find the first company which doesn't exist yet */

src/game/game.hpp

@@ -87,11 +87,6 @@ public:
 	 */
 	static void Save();
 
-	/**
-	 * Load data for a GameScript from a savegame.
-	 */
-	static void Load(int version);
-
 	/** Wrapper function for GameScanner::GetConsoleList */
 	static std::string GetConsoleList(bool newest_only = false);
 	/** Wrapper function for GameScanner::GetConsoleLibraryList */

src/game/game_core.cpp

@@ -88,6 +88,8 @@
 	Game::info = info;
 	Game::instance = new GameInstance();
 	Game::instance->Initialize(info);
+	Game::instance->LoadOnStack(config->GetToLoadData());
+	config->SetToLoadData(nullptr);
 
 	cur_company.Restore();
 
@@ -214,18 +216,6 @@
 	}
 }
 
-/* static */ void Game::Load(int version)
-{
-	if (Game::instance != nullptr && (!_networking || _network_server)) {
-		Backup<CompanyID> cur_company(_current_company, OWNER_DEITY, FILE_LINE);
-		Game::instance->Load(version);
-		cur_company.Restore();
-	} else {
-		/* Read, but ignore, the load data */
-		GameInstance::LoadEmpty();
-	}
-}
-
 /* static */ std::string Game::GetConsoleList(bool newest_only)
 {
 	return Game::scanner_info->GetConsoleList(newest_only);

src/saveload/afterload.cpp

@@ -41,6 +41,7 @@
 #include "../road_cmd.h"
 #include "../ai/ai.hpp"
 #include "../ai/ai_gui.hpp"
+#include "../game/game.hpp"
 #include "../town.h"
 #include "../economy_base.h"
 #include "../animated_tile_func.h"
@@ -302,7 +303,6 @@ static void InitializeWindowsAndCaches()
 
 	CheckTrainsLengths();
 	ShowNewGRFError();
-	ShowAIDebugWindowIfAIError();
 
 	/* Rebuild the smallmap list of owners. */
 	BuildOwnerLegend();
@@ -537,6 +537,22 @@ static inline bool MayHaveBridgeAbove(TileIndex t)
 			IsTileType(t, MP_WATER) || IsTileType(t, MP_TUNNELBRIDGE) || IsTileType(t, MP_OBJECT);
 }
 
+/**
+ * Start the scripts.
+ */
+static void StartScripts()
+{
+	/* Start the GameScript. */
+	Game::StartNew();
+
+	/* Start the AIs. */
+	for (const Company *c : Company::Iterate()) {
+		if (Company::IsValidAiID(c->index)) AI::StartNew(c->index, false);
+	}
+
+	ShowAIDebugWindowIfAIError();
+}
+
 /**
  * Perform a (large) amount of savegame conversion *magic* in order to
  * load older savegames and to fill the caches for various purposes.
@@ -798,13 +814,6 @@ bool AfterLoadGame()
 	/* Update all vehicles */
 	AfterLoadVehicles(true);
 
-	/* Make sure there is an AI attached to an AI company */
-	{
-		for (const Company *c : Company::Iterate()) {
-			if (c->is_ai && c->ai_instance == nullptr) AI::StartNew(c->index);
-		}
-	}
-
 	/* make sure there is a town in the game */
 	if (_game_mode == GM_NORMAL && Town::GetNumItems() == 0) {
 		SetSaveLoadError(STR_ERROR_NO_TOWN_IN_SCENARIO);
@@ -3224,6 +3233,10 @@ bool AfterLoadGame()
 	ResetSignalHandlers();
 
 	AfterLoadLinkGraphs();
+
+	/* Start the scripts. This MUST happen after everything else. */
+	StartScripts();
+
 	return true;
 }
 

src/saveload/ai_sl.cpp

@@ -112,11 +112,8 @@ struct AIPLChunkHandler : ChunkHandler {
 
 			config->StringToSettings(_ai_saveload_settings);
 
-			/* Start the AI directly if it was active in the savegame */
-			if (Company::IsValidAiID(index)) {
-				AI::StartNew(index, false);
-				AI::Load(index, _ai_saveload_version);
-			}
+			/* Load the AI saved data */
+			if (Company::IsValidAiID(index)) config->SetToLoadData(AIInstance::Load(_ai_saveload_version));
 		}
 	}
 

src/saveload/game_sl.cpp

@@ -102,9 +102,8 @@ struct GSDTChunkHandler : ChunkHandler {
 
 		config->StringToSettings(_game_saveload_settings);
 
-		/* Start the GameScript directly if it was active in the savegame */
-		Game::StartNew();
-		Game::Load(_game_saveload_version);
+		/* Load the GameScript saved data */
+		config->SetToLoadData(GameInstance::Load(_game_saveload_version));
 
 		if (SlIterateArray() != -1) SlErrorCorrupt("Too many GameScript configs");
 	}

src/script/script_config.cpp

@@ -26,6 +26,7 @@ void ScriptConfig::Change(const char *name, int version, bool force_exact_match,
 	if (this->config_list != nullptr) delete this->config_list;
 	this->config_list = (info == nullptr) ? nullptr : new ScriptConfigItemList();
 	if (this->config_list != nullptr) this->PushExtraConfigList();
+	this->to_load_data.reset();
 
 	this->ClearConfigList();
 
@@ -49,6 +50,7 @@ ScriptConfig::ScriptConfig(const ScriptConfig *config)
 	this->version = config->version;
 	this->config_list = nullptr;
 	this->is_random = config->is_random;
+	this->to_load_data.reset();
 
 	for (const auto &item : config->settings) {
 		this->settings[stredup(item.first)] = item.second;
@@ -63,6 +65,7 @@ ScriptConfig::~ScriptConfig()
 	free(this->name);
 	this->ResetSettings();
 	if (this->config_list != nullptr) delete this->config_list;
+	this->to_load_data.reset();
 }
 
 ScriptInfo *ScriptConfig::GetInfo() const
@@ -238,3 +241,14 @@ const char *ScriptConfig::GetTextfile(TextfileType type, CompanyID slot) const
 
 	return ::GetTextfile(type, (slot == OWNER_DEITY) ? GAME_DIR : AI_DIR, this->GetInfo()->GetMainScript());
 }
+
+void ScriptConfig::SetToLoadData(ScriptInstance::ScriptData *data)
+{
+	this->to_load_data.reset(data);
+}
+
+ScriptInstance::ScriptData *ScriptConfig::GetToLoadData()
+{
+	return this->to_load_data.get();
+}
+

src/script/script_config.hpp

@@ -16,6 +16,7 @@
 #include "../core/string_compare_type.hpp"
 #include "../company_type.h"
 #include "../textfile_gui.h"
+#include "script_instance.hpp"
 
 /** Bitmask of flags for Script settings. */
 enum ScriptConfigFlags {
@@ -63,7 +64,8 @@ public:
 		version(-1),
 		info(nullptr),
 		config_list(nullptr),
-		is_random(false)
+		is_random(false),
+		to_load_data(nullptr)
 	{}
 
 	/**
@@ -185,13 +187,17 @@ public:
 	 */
 	const char *GetTextfile(TextfileType type, CompanyID slot) const;
 
+	void SetToLoadData(ScriptInstance::ScriptData *data);
+	ScriptInstance::ScriptData *GetToLoadData();
+
 protected:
-	const char *name;                  ///< Name of the Script
-	int version;                       ///< Version of the Script
-	class ScriptInfo *info;            ///< ScriptInfo object for related to this Script version
-	SettingValueList settings;         ///< List with all setting=>value pairs that are configure for this Script
-	ScriptConfigItemList *config_list; ///< List with all settings defined by this Script
-	bool is_random;                    ///< True if the AI in this slot was randomly chosen.
+	const char *name;                                         ///< Name of the Script
+	int version;                                              ///< Version of the Script
+	class ScriptInfo *info;                                   ///< ScriptInfo object for related to this Script version
+	SettingValueList settings;                                ///< List with all setting=>value pairs that are configure for this Script
+	ScriptConfigItemList *config_list;                        ///< List with all settings defined by this Script
+	bool is_random;                                           ///< True if the AI in this slot was randomly chosen.
+	std::unique_ptr<ScriptInstance::ScriptData> to_load_data; ///< Data to load after the Script start.
 
 	/**
 	 * In case you have mandatory non-Script-definable config entries in your

src/script/script_instance.cpp

@@ -343,17 +343,6 @@ void *ScriptInstance::GetLogPointer()
  *  - null:    No data.
  */
 
-/** The type of the data that follows in the savegame. */
-enum SQSaveLoadType {
-	SQSL_INT             = 0x00, ///< The following data is an integer.
-	SQSL_STRING          = 0x01, ///< The following data is an string.
-	SQSL_ARRAY           = 0x02, ///< The following data is an array.
-	SQSL_TABLE           = 0x03, ///< The following data is an table.
-	SQSL_BOOL            = 0x04, ///< The following data is a boolean.
-	SQSL_NULL            = 0x05, ///< A null variable.
-	SQSL_ARRAY_TABLE_END = 0xFF, ///< Marks the end of an array or table, no data follows.
-};
-
 static byte _script_sl_byte; ///< Used as source/target by the script saveload code to store/load a single byte.
 
 /** SaveLoad array that saves/loads exactly one byte. */
@@ -572,14 +561,14 @@ bool ScriptInstance::IsPaused()
 	return this->is_paused;
 }
 
-/* static */ bool ScriptInstance::LoadObjects(HSQUIRRELVM vm)
+/* static */ bool ScriptInstance::LoadObjects(ScriptData *data)
 {
 	SlObject(nullptr, _script_byte);
 	switch (_script_sl_byte) {
 		case SQSL_INT: {
 			int64 value;
 			SlCopy(&value, 1, IsSavegameVersionBefore(SLV_SCRIPT_INT64) ? SLE_FILE_I32 | SLE_VAR_I64 : SLE_INT64);
-			if (vm != nullptr) sq_pushinteger(vm, (SQInteger)value);
+			if (data != nullptr) data->push_back((SQInteger)value);
 			return true;
 		}
 
@@ -588,37 +577,79 @@ bool ScriptInstance::IsPaused()
 			static char buf[std::numeric_limits<decltype(_script_sl_byte)>::max()];
 			SlCopy(buf, _script_sl_byte, SLE_CHAR);
 			StrMakeValidInPlace(buf, buf + _script_sl_byte);
-			if (vm != nullptr) sq_pushstring(vm, buf, -1);
+			if (data != nullptr) data->push_back(std::string(buf));
+			return true;
+		}
+
+		case SQSL_ARRAY:
+		case SQSL_TABLE: {
+			if (data != nullptr) data->push_back((SQSaveLoadType)_script_sl_byte);
+			while (LoadObjects(data));
+			return true;
+		}
+
+		case SQSL_BOOL: {
+			SlObject(nullptr, _script_byte);
+			if (data != nullptr) data->push_back((SQBool)(_script_sl_byte != 0));
+			return true;
+		}
+
+		case SQSL_NULL: {
+			if (data != nullptr) data->push_back((SQSaveLoadType)_script_sl_byte);
 			return true;
 		}
 
+		case SQSL_ARRAY_TABLE_END: {
+			if (data != nullptr) data->push_back((SQSaveLoadType)_script_sl_byte);
+			return false;
+		}
+
+		default: SlErrorCorrupt("Invalid script data type");
+	}
+}
+
+/* static */ bool ScriptInstance::LoadObjects(HSQUIRRELVM vm, ScriptData *data)
+{
+	ScriptDataVariant value = data->front();
+	data->pop_front();
+
+	if (std::holds_alternative<SQInteger>(value)) {
+		sq_pushinteger(vm, std::get<SQInteger>(value));
+		return true;
+	}
+
+	if (std::holds_alternative<std::string>(value)) {
+		sq_pushstring(vm, std::get<std::string>(value).c_str(), -1);
+		return true;
+	}
+
+	if (std::holds_alternative<SQBool>(value)) {
+		sq_pushbool(vm, std::get<SQBool>(value));
+		return true;
+	}
+
+	switch (std::get<SQSaveLoadType>(value)) {
 		case SQSL_ARRAY: {
-			if (vm != nullptr) sq_newarray(vm, 0);
-			while (LoadObjects(vm)) {
-				if (vm != nullptr) sq_arrayappend(vm, -2);
+			sq_newarray(vm, 0);
+			while (LoadObjects(vm, data)) {
+				sq_arrayappend(vm, -2);
 				/* The value is popped from the stack by squirrel. */
 			}
 			return true;
 		}
 
 		case SQSL_TABLE: {
-			if (vm != nullptr) sq_newtable(vm);
-			while (LoadObjects(vm)) {
-				LoadObjects(vm);
-				if (vm != nullptr) sq_rawset(vm, -3);
+			sq_newtable(vm);
+			while (LoadObjects(vm, data)) {
+				LoadObjects(vm, data);
+				sq_rawset(vm, -3);
 				/* The key (-2) and value (-1) are popped from the stack by squirrel. */
 			}
 			return true;
 		}
 
-		case SQSL_BOOL: {
-			SlObject(nullptr, _script_byte);
-			if (vm != nullptr) sq_pushbool(vm, (SQBool)(_script_sl_byte != 0));
-			return true;
-		}
-
 		case SQSL_NULL: {
-			if (vm != nullptr) sq_pushnull(vm);
+			sq_pushnull(vm);
 			return true;
 		}
 
@@ -639,22 +670,35 @@ bool ScriptInstance::IsPaused()
 	LoadObjects(nullptr);
 }
 
-void ScriptInstance::Load(int version)
+/* static */ ScriptInstance::ScriptData *ScriptInstance::Load(int version)
 {
-	ScriptObject::ActiveInstance active(this);
-
-	if (this->engine == nullptr || version == -1) {
+	if (version == -1) {
 		LoadEmpty();
-		return;
+		return nullptr;
 	}
-	HSQUIRRELVM vm = this->engine->GetVM();
 
 	SlObject(nullptr, _script_byte);
 	/* Check if there was anything saved at all. */
-	if (_script_sl_byte == 0) return;
+	if (_script_sl_byte == 0) return nullptr;
+
+	ScriptData *data = new ScriptData();
+	data->push_back((SQInteger)version);
+	LoadObjects(data);
+	return data;
+}
+
+void ScriptInstance::LoadOnStack(ScriptData *data)
+{
+	ScriptObject::ActiveInstance active(this);
+
+	if (data == nullptr) return;
+
+	HSQUIRRELVM vm = this->engine->GetVM();
 
-	sq_pushinteger(vm, version);
-	LoadObjects(vm);
+	ScriptDataVariant version = data->front();
+	data->pop_front();
+	sq_pushinteger(vm, std::get<SQInteger>(version));
+	LoadObjects(vm, data);
 	this->is_save_data_on_stack = true;
 }
 

src/script/script_instance.hpp

@@ -10,6 +10,8 @@
 #ifndef SCRIPT_INSTANCE_HPP
 #define SCRIPT_INSTANCE_HPP
 
+#include <variant>
+#include <list>
 #include <squirrel.h>
 #include "script_suspend.hpp"
 
@@ -21,10 +23,25 @@ static const uint SQUIRREL_MAX_DEPTH = 25; ///< The maximum recursive depth for
 
 /** Runtime information about a script like a pointer to the squirrel vm and the current state. */
 class ScriptInstance {
+private:
+	/** The type of the data that follows in the savegame. */
+	enum SQSaveLoadType {
+		SQSL_INT             = 0x00, ///< The following data is an integer.
+		SQSL_STRING          = 0x01, ///< The following data is an string.
+		SQSL_ARRAY           = 0x02, ///< The following data is an array.
+		SQSL_TABLE           = 0x03, ///< The following data is an table.
+		SQSL_BOOL            = 0x04, ///< The following data is a boolean.
+		SQSL_NULL            = 0x05, ///< A null variable.
+		SQSL_ARRAY_TABLE_END = 0xFF, ///< Marks the end of an array or table, no data follows.
+	};
+
 public:
 	friend class ScriptObject;
 	friend class ScriptController;
 
+	typedef std::variant<SQInteger, std::string, SQBool, SQSaveLoadType> ScriptDataVariant;
+	typedef std::list<ScriptDataVariant> ScriptData;
+
 	/**
 	 * Create a new script.
 	 */
@@ -146,11 +163,18 @@ public:
 	static void SaveEmpty();
 
 	/**
-	 * Load data from a savegame and store it on the stack.
+	 * Load data from a savegame.
 	 * @param version The version of the script when saving, or -1 if this was
 	 *  not the original script saving the game.
+	 * @return a pointer to loaded data.
 	 */
-	void Load(int version);
+	static ScriptData *Load(int version);
+
+	/**
+	 * Store loaded data on the stack.
+	 * @param data The loaded data to store on the stack.
+	 */
+	void LoadOnStack(ScriptData *data);
 
 	/**
 	 * Load and discard data from a savegame.
@@ -289,7 +313,9 @@ private:
 	 * Load all objects from a savegame.
 	 * @return True if the loading was successful.
 	 */
-	static bool LoadObjects(HSQUIRRELVM vm);
+	static bool LoadObjects(ScriptData *data);
+
+	static bool LoadObjects(HSQUIRRELVM vm, ScriptData *data);
 };
 
 #endif /* SCRIPT_INSTANCE_HPP */