Added methods to MovableObject to run scripted functions for them on a given script and on all of their scripts (with the option to run on disabled or not) and moved this out of LuaMan. These methods can optionally take arguments to provide to the functions they run and, if there's more than one script on the MO, will spit out an error telling you want script and function screwed up.

Used these new methods through MovableObject
Renamed some supported script function names and made it so removing and disabling a script call the same function, but with a different value for their 2nd argument
Added details to LoadScript comments, cleaned up some junk

Author: garethyr

Entities/MovableObject.cpp

@@ -455,11 +455,7 @@ int MovableObject::Save(Writer &writer) const
 
 void MovableObject::Destroy(bool notInherited) {
     if (!m_ScriptObjectName.empty()) {
-        for (std::pair<std::string, bool> scriptEntry: m_LoadedScripts) {
-            if (scriptEntry.second == true) {
-                g_LuaMan.RunFunctionInPresetScript("Destroy", scriptEntry.first, m_ScriptPresetName, m_ScriptObjectName);
-            }
-        }
+        RunScriptedFunctionInAppropriateScripts("Destroy");
         g_LuaMan.RunScriptString(m_ScriptObjectName + " = nil;");
     }
 
@@ -499,7 +495,6 @@ int MovableObject::LoadScript(std::string const &scriptPath, bool loadAsEnabledS
     // If there's no ScriptPresetName this is the first script being loaded for this preset, or scripts have been reloaded.
     // Generate a ScriptPresetName, setup a table for the preset's functions, and clear the instance object name so it gets created in the first run of UpdateScripts
     if (m_ScriptPresetName.empty()) {
-        // TODO WAIT A MINUTE.. is this an original preset????!! .. does it matter? A: not really
         m_ScriptPresetName = GetClassName() + "s." + g_LuaMan.GetNewPresetID();
 
         if (g_LuaMan.RunScriptString(m_ScriptPresetName + " = {};") < 0) {
@@ -565,8 +560,8 @@ bool MovableObject::AddScript(std::string const &scriptPath) {
     switch (LoadScript(scriptPath)) {
         case 0:
             // If we have a ScriptObjectName that means Create has already been run for pre-existing scripts. Run it right away for this one.
-            if (!m_ScriptObjectName.empty() &&g_LuaMan.RunFunctionInPresetScript("Create", scriptPath, m_ScriptPresetName, m_ScriptObjectName) < 0) {
-                g_ConsoleMan.PrintString("ERROR: Failed to run Create function for newly added script with path " + scriptPath);
+            if (!m_ScriptObjectName.empty()) {
+                RunScriptedFunction(scriptPath, "Create");
                 return false;
             }
             return true;
@@ -589,9 +584,17 @@ bool MovableObject::AddScript(std::string const &scriptPath) {
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 bool MovableObject::RemoveScript(std::string const &scriptPath) {
+    if (m_LoadedScripts.empty() || m_ScriptPresetName.empty()) {
+        return false;
+    }
+
     auto scriptEntryIterator = FindScript(scriptPath);
     if (scriptEntryIterator != m_LoadedScripts.end()) {
         m_LoadedScripts.erase(scriptEntryIterator);
+        if (!m_ScriptObjectName.empty() && RunScriptedFunction(scriptPath, "OnScriptRemoveOrDisable", {}, {"true"}) < 0) {
+            g_ConsoleMan.PrintString("NOTE: The script has been removed despite this error.");
+            return false;
+        }
         return true;
     }
     return false;
@@ -600,14 +603,16 @@ bool MovableObject::RemoveScript(std::string const &scriptPath) {
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 bool MovableObject::EnableScript(std::string const &scriptPath) {
+    if (m_LoadedScripts.empty() || m_ScriptPresetName.empty()) {
+        return false;
+    }
+
     auto scriptEntryIterator = FindScript(scriptPath);
     if (scriptEntryIterator != m_LoadedScripts.end() && scriptEntryIterator->second == false) {
-        scriptEntryIterator->second = true;
-
-        if (g_LuaMan.RunFunctionInPresetScript("OnEnableScript", scriptPath, m_ScriptPresetName, m_ScriptObjectName) < 0) {
-            g_ConsoleMan.PrintString("ERROR: Failed to run OnEnableScript function for newly enabled script with path " + scriptPath);
+        if (!m_ScriptObjectName.empty() && RunScriptedFunction(scriptPath, "OnScriptEnable") < 0) {
             return false;
         }
+        scriptEntryIterator->second = true;
         return true;
     }
     return false;
@@ -616,21 +621,79 @@ bool MovableObject::EnableScript(std::string const &scriptPath) {
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 bool MovableObject::DisableScript(std::string const &scriptPath) {
+    if (m_LoadedScripts.empty() || m_ScriptPresetName.empty()) {
+        return false;
+    }
+
     auto scriptEntryIterator = FindScript(scriptPath);
     if (scriptEntryIterator != m_LoadedScripts.end() && scriptEntryIterator->second == true) {
-        scriptEntryIterator->second = false;
-
-        if (g_LuaMan.RunFunctionInPresetScript("OnDisableScript", scriptPath, m_ScriptPresetName, m_ScriptObjectName) < 0) {
-            g_ConsoleMan.PrintString("ERROR: Failed to run OnDisableScript function for newly disabled script with path " + scriptPath);
+        if (!m_ScriptObjectName.empty() && RunScriptedFunction(scriptPath, "OnScriptRemoveOrDisable", {}, {"false"}) < 0) {
             return false;
         }
+        scriptEntryIterator->second = false;
         return true;
     }
     return false;
 }
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
+int MovableObject::RunScriptedFunctionInAppropriateScripts(std::string const &functionName, bool runOnDisabledScripts, bool stopOnError, std::vector<Entity *> functionEntityArguments, std::vector<std::string> functionLiteralArguments) {
+    if (m_LoadedScripts.empty() || m_ScriptPresetName.empty() || m_ScriptObjectName.empty()) {
+        return -1;
+    }
+
+    int status = 0;
+    for (std::pair<std::string, bool> scriptEntry : m_LoadedScripts) {
+        if (runOnDisabledScripts || scriptEntry.second == true) {
+            status = RunScriptedFunction(scriptEntry.first, functionName, functionEntityArguments, functionLiteralArguments);
+            if (status < 0 && stopOnError) {
+                return status;
+            }
+        }
+    }
+    return status;
+}
+
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+int MovableObject::RunScriptedFunction(std::string const &scriptPath, std::string const &functionName, std::vector<Entity *> functionEntityArguments, std::vector<std::string> functionLiteralArguments) {
+    if (m_LoadedScripts.empty() || m_ScriptPresetName.empty() || m_ScriptObjectName.empty()) {
+        return -1;
+    }
+
+    std::string presetAndFunctionName = m_ScriptPresetName + "." + functionName;
+    std::string scriptString = "if " + presetAndFunctionName + " and " + m_ScriptObjectName + " and " + presetAndFunctionName + "[\"" + scriptPath + "\"] then ";
+    if (!functionEntityArguments.empty()) {
+        scriptString += "local entityArguments = LuaMan.TempEntities; ";
+    }
+    scriptString += presetAndFunctionName + "[\"" + scriptPath + "\"](" + m_ScriptObjectName;
+    if (!functionEntityArguments.empty()) {
+        g_LuaMan.SetTempEntityVector(functionEntityArguments);
+        for (const Entity *functionEntityArgument : functionEntityArguments) {
+            scriptString += ", To" + functionEntityArgument->GetClassName() + "(entityArguments())";
+        }
+    }
+    if (!functionLiteralArguments.empty()) {
+        for (const std::string functionLiteralArgument : functionLiteralArguments) {
+            scriptString += ", " + functionLiteralArgument;
+        }
+    }
+    scriptString += +"); end";
+
+    int status = g_LuaMan.RunScriptString(scriptString);
+    functionEntityArguments.clear();
+    functionLiteralArguments.clear();
+    if (status < 0 && m_LoadedScripts.size() > 1) {
+        g_ConsoleMan.PrintString("ERROR: An error occured while trying to run " + functionName + "function for script at path " + scriptPath);
+        return -2;
+    }
+
+    return 0;
+}
+
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
 /*
 //////////////////////////////////////////////////////////////////////////////////////////
 // Constructor:     MovableObject
@@ -902,21 +965,18 @@ int MovableObject::UpdateScripts() {
 
         // Give Lua access to this object, then use that access to set up the object's Lua representation
         g_MovableMan.SetScriptedEntity(this);
+
         if (g_LuaMan.RunScriptString(m_ScriptObjectName + " = To" + GetClassName() + "(MovableMan.ScriptedEntity);") < 0) {
             return -2;
         }
 
-        for (std::pair<std::string, bool> scriptEntry : m_LoadedScripts) {
-            if (g_LuaMan.RunFunctionInPresetScript("Create", scriptEntry.first, m_ScriptPresetName, m_ScriptObjectName) < 0) {
-                return -3;
-            }
+        if (RunScriptedFunctionInAppropriateScripts("Create", true, true) < 0) {
+            return -3;
         }
     }
 
-    for (std::pair<std::string, bool> scriptEntry : m_LoadedScripts) {
-        if (scriptEntry.second == true && g_LuaMan.RunFunctionInPresetScript("Update", scriptEntry.first, m_ScriptPresetName, m_ScriptObjectName) < 0) {
-            return -3;
-        }
+    if (RunScriptedFunctionInAppropriateScripts("Update", false, true) < 0) {
+        return -3;
     }
 
     return 0;
@@ -930,12 +990,7 @@ int MovableObject::OnPieMenu(Actor * pActor) {
     }
 	m_pPieMenuActor = pActor;
 
-    int status = 0;
-    for (std::pair<std::string, bool> scriptEntry : m_LoadedScripts) {
-        status = scriptEntry.second == true ? g_LuaMan.RunFunctionInPresetScript("OnPieMenu", scriptEntry.first, m_ScriptPresetName, m_ScriptObjectName) : status;
-        if (status < 0) { break; }
-    }
-    return status;
+    return RunScriptedFunctionInAppropriateScripts("OnPieMenu"); //TODO try passing actor here, see if it works. If it does, maybe quietly refactor this whole thing
 }
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

Entities/MovableObject.h

@@ -183,7 +183,7 @@ ENTITYALLOCATION(MovableObject)
     /// </summary>
     /// <param name="scriptPath">The path to the script to load.</param>
     /// <param name="loadAsEnabledScript">Whether or not the script should load as enabled. Defaults to true.</param>
-    /// <returns>An error return value signaling sucess or any particular failure. Anything below 0 is an error signal.</returns>
+    /// <returns>0 on success. -1 if scriptPath is empty. -2 if the script is already loaded. -3 if setup to load the script or modify the global lua state fails. -4 if the script fails to load.</returns>
     virtual int LoadScript(std::string const &scriptPath, bool loadAsEnabledScript = true);
 
     /// <summary>
@@ -241,6 +241,29 @@ ENTITYALLOCATION(MovableObject)
     /// <returns>Whether or not the script was succesfully disabled..</returns>
     virtual bool DisableScript(std::string const &scriptPath);
 
+    /// <summary>
+    /// Runs the given function in all scripts that have it, with the given arguments, with the ability to not run on disabled scripts and to cease running if there's an error.
+    /// The first argument to the function will always be 'self'. If either argument list is not emtpy, its entries will be passed into the Lua function in order, with entity arguments first.
+    /// </summary>
+    /// <param name="functionName">The name of the function to run.</param>
+    /// <param name="runOnDisabledScripts">Whether to run the function on disabled scripts. Defaults to false.</param>
+    /// <param name="stopOnError">Whether to stop if there's an error running any script, or simply print it to the console and continue. Defaults to false.</param>
+    /// <param name="functionEntityArguments">Optional vector of entity pointers that should be passed into the Lua function. Their internal Lua states will not be accessible. Defaults to empty.</param>
+    /// <param name="functionLiteralArguments">Optional vector of strings, that should be passed into the Lua function. Entries must be surrounded with escaped quotes (i.e.`\"`) they'll be passed in as-is, allowing them to act as booleans, etc.. Defaults to empty.</param>
+    /// <returns>An error return value signaling sucess or any particular failure. Anything below 0 is an error signal.</returns>
+    int RunScriptedFunctionInAppropriateScripts(std::string const &functionName, bool runOnDisabledScripts = false, bool stopOnError = false, std::vector<Entity *> functionEntityArguments = std::vector<Entity *>(), std::vector<std::string> functionLiteralArguments = std::vector<std::string>());
+
+    /// <summary>
+    /// Runs the given function for the given script, with the given arguments. The first argument to the function will always be 'self'.
+    /// If either argument list is not emtpy, its entries will be passed into the Lua function in order, with entity arguments first.
+    /// </summary>
+    /// <param name="scriptPath">The path to the script to run.</param>
+    /// <param name="functionName">The name of the function to run.</param>
+    /// <param name="functionEntityArguments">Optional vector of entity pointers that should be passed into the Lua function. Their internal Lua states will not be accessible. Defaults to empty.</param>
+    /// <param name="functionLiteralArguments">Optional vector of strings, that should be passed into the Lua function. Entries must be surrounded with escaped quotes (i.e.`\"`) they'll be passed in as-is, allowing them to act as booleans, etc.. Defaults to empty.</param>
+    /// <returns>An error return value signaling sucess or any particular failure. Anything below 0 is an error signal.</returns>
+    int RunScriptedFunction(std::string const &scriptPath, std::string const &functionName, std::vector<Entity *> functionEntityArguments = std::vector<Entity *>(), std::vector<std::string> functionLiteralArguments = std::vector<std::string>());
+
 //////////////////////////////////////////////////////////////////////////////////////////
 // Virtual method:  GetClass
 //////////////////////////////////////////////////////////////////////////////////////////
@@ -1751,7 +1774,7 @@ ENTITYALLOCATION(MovableObject)
     /// Gets a vector containing the script function names this class supports.
     /// </summary>
     /// <returns>A vector containing the script function names this class supports.</returns>
-    virtual const std::vector<std::string> GetSupportedScriptFunctionNames() { return std::vector<std::string> {"Create", "Destroy", "Update", "OnPieMenu", "OnEnableScript", "OnDisableScript"}; }
+    virtual const std::vector<std::string> GetSupportedScriptFunctionNames() { return std::vector<std::string> {"Create", "Destroy", "Update", "OnPieMenu", "OnScriptRemoveOrDisable", "OnScriptEnable"}; }
 
 //////////////////////////////////////////////////////////////////////////////////////////
 // Virtual method:  UpdateChildMOIDs

Managers/LuaMan.cpp

@@ -2419,19 +2419,6 @@ bool LuaMan::ExpressionIsTrue(string expression, bool consoleErrors)
     return result;
 }
 
-/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
-int LuaMan::RunFunctionInPresetScript(std::string const &functionName, std::string const &scriptPath, std::string const &presetLuaInstanceName, std::string const &objectLuaInstanceName) {
-    std::string presetAndFunctionName = presetLuaInstanceName + "." + functionName;
-    return RunScriptString(
-        "if " + presetAndFunctionName + " and " + objectLuaInstanceName + " then " +
-            presetAndFunctionName + "[\""+scriptPath+"\"](" + objectLuaInstanceName + "); " +
-        "end"
-    );
-}
-
-/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
 //////////////////////////////////////////////////////////////////////////////////////////
 // Method:          RunScriptString
 //////////////////////////////////////////////////////////////////////////////////////////

Managers/LuaMan.h

@@ -227,17 +227,6 @@ enum ServerResult
 
     bool ExpressionIsTrue(std::string expression, bool consoleErrors);
 
-    /// <summary>
-    /// Helper method to run the given function in all of the given preset instance's scripts on the given object instance. Note that this method does not do any safety checks.
-    /// </summary>
-    /// <param name="functionName">The name of the function to run.</param>
-    /// <param name="scriptPath">The path to the script whose function is being run. Used as an identifier in Lua.</param>
-    /// <param name="presetLuaInstanceName">The name of the lua variable that holds the representation of the preset instance whose function is being run.</param>
-    /// <param name="objectLuaInstanceName">The name of the lua variable that holds the representation of the object instance the function is being run on.</param>
-    /// <returns>Returns less than zero if any errors encountered when running this script. To get the actual error string, call GetLastError.</returns>
-    int LuaMan::RunFunctionInPresetScript(std::string const &functionName, std::string const &scriptPath, std::string const &presetLuaInstanceName, std::string const &objectLuaInstanceName);
-
-
 //////////////////////////////////////////////////////////////////////////////////////////
 // Method:          RunScriptString
 //////////////////////////////////////////////////////////////////////////////////////////