Merge pull request #182 from cortex-command-community/ordered-scripts

Scripts Maintaining Insertion Order

Author: Causeless

CHANGELOG.md

@@ -187,6 +187,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 - Fixed several issues with the way pie menus and aiming interacts between players, such as opening the pie menu always resetting the M&KB player's aim and pie selection, as well as another issue where the pie menu would fail to appear entirely for some players.
 
+- Fixed issue where scripts applied to `MovableObject`s could become disordered in certain circumstances.
+
 </details>
 
 <details><summary><b>Removed</b></summary>

Source/Entities/MovableObject.cpp

@@ -79,6 +79,7 @@ void MovableObject::Clear() {
 	m_HUDVisible = true;
 	m_IsTraveling = false;
 	m_AllLoadedScripts.clear();
+	m_EnabledScripts.clear();
 	m_FunctionsAndScripts.clear();
 	m_StringValueMap.clear();
 	m_NumberValueMap.clear();
@@ -225,8 +226,8 @@ int MovableObject::Create(const MovableObject& reference) {
 	m_PostEffectEnabled = reference.m_PostEffectEnabled;
 
 	m_ForceIntoMasterLuaState = reference.m_ForceIntoMasterLuaState;
-	for (auto& [scriptPath, scriptEnabled]: reference.m_AllLoadedScripts) {
-		LoadScript(scriptPath, scriptEnabled);
+	for (const auto& scriptPath: reference.m_AllLoadedScripts) {
+		LoadScript(scriptPath, reference.m_EnabledScripts.at(scriptPath));
 	}
 
 	if (reference.m_pScreenEffect) {
@@ -442,7 +443,7 @@ int MovableObject::Save(Writer& writer) const {
 	writer << m_CanBeSquished;
 	writer.NewProperty("HUDVisible");
 	writer << m_HUDVisible;
-	for (const auto& [scriptPath, scriptEnabled]: m_AllLoadedScripts) {
+	for (const auto& [scriptPath, scriptEnabled]: m_EnabledScripts) {
 		if (!scriptPath.empty()) {
 			writer.NewProperty("ScriptPath");
 			writer << scriptPath;
@@ -545,7 +546,8 @@ int MovableObject::LoadScript(const std::string& scriptPath, bool loadAsEnabledS
 		}
 	}
 
-	m_AllLoadedScripts.try_emplace(scriptPath, loadAsEnabledScript);
+	m_EnabledScripts.try_emplace(scriptPath, loadAsEnabledScript);
+	m_AllLoadedScripts.push_back(scriptPath);
 
 	std::unordered_map<std::string, LuabindObjectWrapper*> scriptFileFunctions;
 	if (usedState.RunScriptFileAndRetrieveFunctions(scriptPath, GetSupportedScriptFunctionNames(), scriptFileFunctions) < 0) {
@@ -580,14 +582,16 @@ int MovableObject::ReloadScripts() {
 		movableObjectPreset->ReloadScripts();
 	}
 
-	std::unordered_map<std::string, bool> loadedScriptsCopy = m_AllLoadedScripts;
+	std::unordered_map<std::string, bool> enabledScriptsCopy = m_EnabledScripts;
+	std::vector<std::string> loadedScriptsCopy = m_AllLoadedScripts;
+	m_EnabledScripts.clear();
 	m_AllLoadedScripts.clear();
 	m_FunctionsAndScripts.clear();
-	for (const auto& [scriptPath, scriptEnabled]: loadedScriptsCopy) {
-		status = LoadScript(scriptPath, scriptEnabled);
+	for (const auto& scriptPath: loadedScriptsCopy) {
+		status = LoadScript(scriptPath, enabledScriptsCopy.at(scriptPath));
 		// If the script fails to load because of an error in its Lua, we need to manually add the script path so it's not lost forever.
 		if (status == -4) {
-			m_AllLoadedScripts.try_emplace(scriptPath, scriptEnabled);
+			m_EnabledScripts.try_emplace(scriptPath, enabledScriptsCopy.at(scriptPath));
 		} else if (status < 0) {
 			break;
 		}
@@ -618,7 +622,7 @@ bool MovableObject::EnableOrDisableScript(const std::string& scriptPath, bool en
 		return false;
 	}
 
-	if (auto scriptEntryIterator = m_AllLoadedScripts.find(scriptPath); scriptEntryIterator != m_AllLoadedScripts.end() && scriptEntryIterator->second == !enableScript) {
+	if (auto scriptEntryIterator = m_EnabledScripts.find(scriptPath); scriptEntryIterator != m_EnabledScripts.end() && scriptEntryIterator->second == !enableScript) {
 		if (ObjectScriptsInitialized() && RunFunctionOfScript(scriptPath, enableScript ? "OnScriptEnable" : "OnScriptDisable") < 0) {
 			return false;
 		}
@@ -640,7 +644,7 @@ bool MovableObject::EnableOrDisableScript(const std::string& scriptPath, bool en
 }
 
 void MovableObject::EnableOrDisableAllScripts(bool enableScripts) {
-	for (const auto& [scriptPath, scriptIsEnabled]: m_AllLoadedScripts) {
+	for (const auto& [scriptPath, scriptIsEnabled]: m_EnabledScripts) {
 		if (enableScripts != scriptIsEnabled) {
 			EnableOrDisableScript(scriptPath, enableScripts);
 		}
@@ -688,9 +692,7 @@ int MovableObject::RunFunctionOfScript(const std::string& scriptPath, const std:
 	for (const LuaFunction& luaFunction: m_FunctionsAndScripts.at(functionName)) {
 		const LuabindObjectWrapper* luabindObjectWrapper = luaFunction.m_LuaFunction.get();
 		if (scriptPath == luabindObjectWrapper->GetFilePath() && usedState.RunScriptFunctionObject(luabindObjectWrapper, "_ScriptedObjects", std::to_string(m_UniqueID), functionEntityArguments, functionLiteralArguments) < 0) {
-			if (m_AllLoadedScripts.size() > 1) {
-				g_ConsoleMan.PrintString("ERROR: An error occured while trying to run the " + functionName + " function for script at path " + scriptPath);
-			}
+			g_ConsoleMan.PrintString("ERROR: An error occured while trying to run the " + functionName + " function for script at path " + scriptPath);
 			return -2;
 		}
 	}

Source/Entities/MovableObject.h

@@ -112,14 +112,14 @@ namespace RTE {
 		/// Checks if the script at the given path is one of the scripts on this MO.
 		/// @param scriptPath The path to the script to check.
 		/// @return Whether or not the script is on this MO.
-		bool HasScript(const std::string& scriptPath) const { return m_AllLoadedScripts.find(scriptPath) != m_AllLoadedScripts.end(); }
+		bool HasScript(const std::string& scriptPath) const { return std::find(m_AllLoadedScripts.begin(), m_AllLoadedScripts.end(), scriptPath) != m_AllLoadedScripts.end(); }
 
 		/// Checks if the script at the given path is one of the enabled scripts on this MO.
 		/// @param scriptPath The path to the script to check.
 		/// @return Whether or not the script is enabled on this MO.
 		bool ScriptEnabled(const std::string& scriptPath) const {
-			auto scriptPathIterator = m_AllLoadedScripts.find(scriptPath);
-			return scriptPathIterator != m_AllLoadedScripts.end() && scriptPathIterator->second == true;
+			auto scriptPathIterator = m_EnabledScripts.find(scriptPath);
+			return scriptPathIterator != m_EnabledScripts.end() && scriptPathIterator->second == true;
 		}
 
 		/// Enables or dsiableds the script at the given path on this MO.
@@ -1235,7 +1235,8 @@ namespace RTE {
 		};
 
 		std::string m_ScriptObjectName; //!< The name of this object for script usage.
-		std::unordered_map<std::string, bool> m_AllLoadedScripts; //!< A map of script paths to the enabled state of the given script.
+		std::vector<std::string> m_AllLoadedScripts; //!< A vector of script for scripts applied to this object, in order of insertion.
+		std::unordered_map<std::string, bool> m_EnabledScripts; //!< A map of script paths to the enabled state of the given script.
 		std::unordered_map<std::string, std::vector<LuaFunction>> m_FunctionsAndScripts; //!< A map of function names to vectors of Lua functions. Used to maintain script execution order and avoid extraneous Lua calls.
 
 		volatile bool m_RequestedSyncedUpdate; //!< For optimisation purposes, scripts explicitly request a synced update if they want one.