Thinking ordered scripts, previously scripts were recorded with an unordered map from script name to enabled state, however this doesn't guarantee maintained order, causing script failure. An ordered map wouldn't work because it requires a comparison function, when the order needs to be determined by insertion order. It seems like it may as well be handled with a vector of script names, while maintaining the unordered map of script name to enabled state. However, each script name is recorded twice there (this is also what I have implemented, it works). Probably the best solution would be to just keep a vector of script name and a vector of corresponding enabled states, however the efficiency of searching goes from log of size to linear with position, which seems bad, but nothing should ever have enough scripts on it to be a problem, is what I imagine. Feels like I'm missing the optimal solution.

Author: Bit-Crust

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 map of script paths to the enabled state of the given script.
+		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.