Changed const ordering in MovableObject script methods, changed OnPieMenu method comment and moved OnPieMenu nearer the back of the supported script function names in MovableObject since it makes more sense

Moved scriptString construction logic into LuaMan so it can be used by things that aren't MovableObjects (i.e. GAScripted and GlobalScript). Also changed string in RunScript to be const &

Author: garethyr

Entities/MovableObject.cpp

@@ -485,7 +485,7 @@ int MovableObject::SetupScriptObjectNameAndRunLuaCreateFunctions() {
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-int MovableObject::LoadScript(std::string const &scriptPath, bool loadAsEnabledScript) {
+int MovableObject::LoadScript(const std::string &scriptPath, bool loadAsEnabledScript) {
     // Return an error if the script path is empty or already there
     if (scriptPath.empty()) {
         return -1;
@@ -574,7 +574,7 @@ int MovableObject::ReloadScripts() {
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-bool MovableObject::AddScript(std::string const &scriptPath) {
+bool MovableObject::AddScript(const std::string &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.
@@ -601,7 +601,7 @@ bool MovableObject::AddScript(std::string const &scriptPath) {
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-bool MovableObject::RemoveScript(std::string const &scriptPath) {
+bool MovableObject::RemoveScript(const std::string &scriptPath) {
     if (m_LoadedScripts.empty() || m_ScriptPresetName.empty()) {
         return false;
     }
@@ -620,7 +620,7 @@ bool MovableObject::RemoveScript(std::string const &scriptPath) {
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-bool MovableObject::EnableScript(std::string const &scriptPath) {
+bool MovableObject::EnableScript(const std::string &scriptPath) {
     if (m_LoadedScripts.empty() || m_ScriptPresetName.empty()) {
         return false;
     }
@@ -638,7 +638,7 @@ bool MovableObject::EnableScript(std::string const &scriptPath) {
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-bool MovableObject::DisableScript(std::string const &scriptPath) {
+bool MovableObject::DisableScript(const std::string &scriptPath) {
     if (m_LoadedScripts.empty() || m_ScriptPresetName.empty()) {
         return false;
     }
@@ -656,35 +656,20 @@ bool MovableObject::DisableScript(std::string const &scriptPath) {
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-int MovableObject::RunScriptedFunction(std::string const &scriptPath, std::string const &functionName, std::vector<Entity *> functionEntityArguments, std::vector<std::string> functionLiteralArguments) {
+int MovableObject::RunScriptedFunction(const std::string &scriptPath, const std::string &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);
+    std::string fullFunctionName = presetAndFunctionName + "[\"" + scriptPath + "\"]";
+    
+    int status = g_LuaMan.RunScriptedFunction(fullFunctionName, m_ScriptObjectName, {presetAndFunctionName, m_ScriptObjectName, fullFunctionName}, functionEntityArguments, functionLiteralArguments);
     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);
+        g_ConsoleMan.PrintString("ERROR: An error occured while trying to run the " + functionName + " function for script at path " + scriptPath);
         return -2;
     }
 
@@ -693,7 +678,7 @@ int MovableObject::RunScriptedFunction(std::string const &scriptPath, std::strin
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-int MovableObject::RunScriptedFunctionInAppropriateScripts(std::string const &functionName, bool runOnDisabledScripts, bool stopOnError, std::vector<Entity *> functionEntityArguments, std::vector<std::string> functionLiteralArguments) {
+int MovableObject::RunScriptedFunctionInAppropriateScripts(const std::string &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;
     }

Entities/MovableObject.h

@@ -184,7 +184,7 @@ ENTITYALLOCATION(MovableObject)
     /// <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>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);
+    virtual int LoadScript(const std::string &scriptPath, bool loadAsEnabledScript = true);
 
     /// <summary>
     /// Reloads the all of the scripts on this object. This will also update the original preset in PresetMan with the updated scripts so future objects spawned will use the new scripts.
@@ -204,42 +204,42 @@ ENTITYALLOCATION(MovableObject)
     /// </summary>
     /// <param name="scriptPath">The path to the script to check.</param>
     /// <returns>Whether or not the script is on this MO.</returns>
-    virtual bool const HasScript(std::string const &scriptPath) { return FindScript(scriptPath) != m_LoadedScripts.end(); }
+    virtual bool const HasScript(const std::string &scriptPath) { return FindScript(scriptPath) != m_LoadedScripts.end(); }
 
     /// <summary>
     /// Adds the script at the given path as one of the scripts on this MO.
     /// </summary>
     /// <param name="scriptPath">The path to the script to add.</param>
     /// <returns>Whether or not the script was successfully added.</returns>
-    virtual bool AddScript(std::string const &scriptPath);
+    virtual bool AddScript(const std::string &scriptPath);
 
     /// <summary>
     /// Removes the script at the given path so it will no longer be one of the scripts on this MO.
     /// </summary>
     /// <param name="scriptPath">The path to the script to remove.</param>
     /// <returns>Whether or not the script was successfully removed.</returns>
-    virtual bool RemoveScript(std::string const &scriptPath);
+    virtual bool RemoveScript(const std::string &scriptPath);
 
     /// <summary>
     /// Checks if the script at the given path is one of the enabled scripts on this MO.
     /// </summary>
     /// <param name="scriptPath">The path to the script to check.</param>
     /// <returns>Whether or not the script is enabled on this MO.</returns>
-    virtual bool const ScriptEnabled(std::string const &scriptPath) { auto scriptIterator = FindScript(scriptPath); return scriptIterator != m_LoadedScripts.end() && scriptIterator->second == true; }
+    virtual bool const ScriptEnabled(const std::string &scriptPath) { auto scriptIterator = FindScript(scriptPath); return scriptIterator != m_LoadedScripts.end() && scriptIterator->second == true; }
 
     /// <summary>
     /// Enable the script at the given path on this MO.
     /// </summary>
     /// <param name="scriptPath">The path to the script to enable.</param>
     /// <returns>Whether or not the script was succesfully enabled.</returns>
-    virtual bool EnableScript(std::string const &scriptPath);
+    virtual bool EnableScript(const std::string &scriptPath);
 
     /// <summary>
     /// Disables the script at the given path for this MO.
     /// </summary>
     /// <param name="scriptPath">The path to the script to disable.</param>
     /// <returns>Whether or not the script was succesfully disabled..</returns>
-    virtual bool DisableScript(std::string const &scriptPath);
+    virtual bool DisableScript(const std::string &scriptPath);
 
     /// <summary>
     /// Runs the given function for the given script, with the given arguments. The first argument to the function will always be 'self'.
@@ -250,7 +250,7 @@ ENTITYALLOCATION(MovableObject)
     /// <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>());
+    int RunScriptedFunction(const std::string &scriptPath, const std::string &functionName, std::vector<Entity *> functionEntityArguments = std::vector<Entity *>(), std::vector<std::string> functionLiteralArguments = std::vector<std::string>());
 
     /// <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.
@@ -262,7 +262,7 @@ ENTITYALLOCATION(MovableObject)
     /// <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>());
+    int RunScriptedFunctionInAppropriateScripts(const std::string &functionName, bool runOnDisabledScripts = false, bool stopOnError = false, std::vector<Entity *> functionEntityArguments = std::vector<Entity *>(), std::vector<std::string> functionLiteralArguments = std::vector<std::string>());
 
 //////////////////////////////////////////////////////////////////////////////////////////
 // Virtual method:  GetClass
@@ -1532,15 +1532,11 @@ ENTITYALLOCATION(MovableObject)
 
     virtual int UpdateScripts();
 
-
-//////////////////////////////////////////////////////////////////////////////////////////
-// Virtual method:  OnPieMenu
-//////////////////////////////////////////////////////////////////////////////////////////
-// Description:     Executes the Lua-defined OnPieMenu event handler.
-// Arguments:       Actor which triggered the pie menu event
-// Return value:    An error return value signaling sucess or any particular failure.
-//                  Anything below 0 is an error signal.
-
+    /// <summary>
+    /// Executes the Lua-defined OnPieMenu event handler for this MO.
+    /// </summary>
+    /// <param name="pActor">Actor which triggered the pie menu event.</param>
+    /// <returns>An error return value signaling sucess or any particular failure. Anything below 0 is an error signal.</returns>
 	virtual int OnPieMenu(Actor *pActor);
 
 
@@ -1774,7 +1770,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", "OnScriptRemoveOrDisable", "OnScriptEnable"}; }
+    virtual const std::vector<std::string> GetSupportedScriptFunctionNames() { return std::vector<std::string> {"Create", "Destroy", "Update", "OnScriptRemoveOrDisable", "OnScriptEnable", "OnPieMenu"}; }
 
     /// <summary>
     /// Does necessary work to setup a script object name for this object, allowing it to be accessed in Lua, then runs all of the MO's scripts' Create functions in Lua.

Managers/LuaMan.cpp

@@ -2421,13 +2421,51 @@ bool LuaMan::ExpressionIsTrue(string expression, bool consoleErrors)
     return result;
 }
 
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+int LuaMan::RunScriptedFunction(const std::string &functionName, const std::string &selfObjectName, std::vector<std::string> variablesToSafetyCheck, std::vector<Entity *> functionEntityArguments, std::vector<std::string> functionLiteralArguments) {
+    std::string scriptString = "";
+    if (!variablesToSafetyCheck.empty()) {
+        scriptString += "if ";
+        for (const std::string &variableToSafetyCheck : variablesToSafetyCheck) {
+            if (&variableToSafetyCheck != &variablesToSafetyCheck[0]) {
+                scriptString += " and ";
+            }
+            scriptString += variableToSafetyCheck;
+        }
+        scriptString += " then ";
+    }
+    if (!functionEntityArguments.empty()) {
+        scriptString += "local entityArguments = LuaMan.TempEntities; ";
+    }
+    scriptString += functionName + "(" + selfObjectName;
+    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 += ");";
+
+    if (!variablesToSafetyCheck.empty()) { scriptString += " end"; };
+    
+    return RunScriptString(scriptString);
+}
+
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
 //////////////////////////////////////////////////////////////////////////////////////////
 // Method:          RunScriptString
 //////////////////////////////////////////////////////////////////////////////////////////
 // Description:     Takes a string containing a script snippet and runs it on the master
 //                  state.
 
-int LuaMan::RunScriptString(string scriptString, bool consoleErrors)
+int LuaMan::RunScriptString(const std::string &scriptString, bool consoleErrors)
 {
     if (scriptString.empty())
         return -1;

Managers/LuaMan.h

@@ -227,6 +227,18 @@ enum ServerResult
 
     bool ExpressionIsTrue(std::string expression, bool consoleErrors);
 
+    /// <summary>
+    /// Runs the given Lua function with optional safety checks and arguments. The first argument to the function will always be the self object.
+    /// If either argument list has entries, they will be passed into the function in order, with entity arguments first.
+    /// </summary>
+    /// <param name="functionName">The name that gives access to the function in the global Lua namespace.</param>
+    /// <param name="selfObjectName">The name that gives access to the self object in the global Lua namespace.</param>
+    /// <param name="variablesToSafetyCheck">Optional vector of strings that should be safety checked in order before running the Lua function. Defaults to empty.</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(const std::string &functionName, const std::string &selfObjectName, std::vector<std::string> variablesToSafetyCheck = std::vector<std::string>(), std::vector<Entity *> functionEntityArguments = std::vector<Entity *>(), std::vector<std::string> functionLiteralArguments = std::vector<std::string>());
+
 //////////////////////////////////////////////////////////////////////////////////////////
 // Method:          RunScriptString
 //////////////////////////////////////////////////////////////////////////////////////////
@@ -237,7 +249,7 @@ enum ServerResult
 // Return value:    Returns less than zero if any errors encountered when running this script.
 //                  To get the actual error string, call GetLastError.
 
-    int RunScriptString(std::string scriptString, bool consoleErrors = true);
+    int RunScriptString(const std::string &scriptString, bool consoleErrors = true);
 
 
 //////////////////////////////////////////////////////////////////////////////////////////