Merge branch 'lua_callback' into 'master'

Refactoring. Lua `Callback` is moved from apps/openmw/mwlua to components/lua.

See merge request OpenMW/openmw!1334

apps/openmw/mwlua/asyncbindings.cpp

@@ -52,13 +52,12 @@ namespace MWLua
         };
         api["callback"] = [](const AsyncPackageId& asyncId, sol::function fn)
         {
-            return Callback{std::move(fn), asyncId.mHiddenData};
+            return LuaUtil::Callback{std::move(fn), asyncId.mHiddenData};
         };
 
         auto initializer = [](sol::table hiddenData)
         {
-            LuaUtil::ScriptsContainer::ScriptId id = hiddenData[LuaUtil::ScriptsContainer::ScriptId::KEY];
+            LuaUtil::ScriptsContainer::ScriptId id = hiddenData[LuaUtil::ScriptsContainer::sScriptIdKey];
-            hiddenData[Callback::SCRIPT_NAME_KEY] = id.toString();
             return AsyncPackageId{id.mContainer, id.mIndex, hiddenData};
         };
         return sol::make_object(context.mLua->sol(), initializer);

apps/openmw/mwlua/luamanagerimp.cpp

@@ -78,16 +78,6 @@ namespace MWLua
         mInitialized = true;
     }
 
-    void Callback::operator()(sol::object arg) const
-    {
-        if (mHiddenData[LuaUtil::ScriptsContainer::ScriptId::KEY] != sol::nil)
-            LuaUtil::call(mFunc, std::move(arg));
-        else
-        {
-            Log(Debug::Debug) << "Ignored callback to removed script " << mHiddenData.get<std::string>(SCRIPT_NAME_KEY);
-        }
-    }
-
     void LuaManager::update(bool paused, float dt)
     {
         ObjectRegistry* objectRegistry = mWorldView.getObjectRegistry();

apps/openmw/mwlua/luamanagerimp.hpp

@@ -19,19 +19,6 @@
 namespace MWLua
 {
 
-    // Wrapper for a single-argument Lua function.
-    // Holds information about the script the function belongs to.
-    // Needed to prevent callback calls if the script was removed.
-    struct Callback
-    {
-        static constexpr std::string_view SCRIPT_NAME_KEY = "name";
-
-        sol::function mFunc;
-        sol::table mHiddenData;
-
-        void operator()(sol::object arg) const;
-    };
-
     class LuaManager : public MWBase::LuaManager
     {
     public:
@@ -82,13 +69,16 @@ namespace MWLua
         void reloadAllScripts() override;
 
         // Used to call Lua callbacks from C++
-        void queueCallback(Callback callback, sol::object arg) { mQueuedCallbacks.push_back({std::move(callback), std::move(arg)}); }
+        void queueCallback(LuaUtil::Callback callback, sol::object arg)
+        {
+            mQueuedCallbacks.push_back({std::move(callback), std::move(arg)});
+        }
 
         // Wraps Lua callback into an std::function.
         // NOTE: Resulted function is not thread safe. Can not be used while LuaManager::update() or
         //       any other Lua-related function is running.
         template <class Arg>
-        std::function<void(Arg)> wrapLuaCallback(const Callback& c)
+        std::function<void(Arg)> wrapLuaCallback(const LuaUtil::Callback& c)
         {
             return [this, c](Arg arg) { this->queueCallback(c, sol::make_object(c.mFunc.lua_state(), arg)); };
         }
@@ -131,7 +121,7 @@ namespace MWLua
 
         struct CallbackWithData
         {
-            Callback mCallback;
+            LuaUtil::Callback mCallback;
             sol::object mArg;
         };
         std::vector<CallbackWithData> mQueuedCallbacks;

components/lua/scriptscontainer.cpp

@@ -15,15 +15,6 @@ namespace LuaUtil
     static constexpr std::string_view HANDLER_LOAD = "onLoad";
     static constexpr std::string_view HANDLER_INTERFACE_OVERRIDE = "onInterfaceOverride";
 
-    std::string ScriptsContainer::ScriptId::toString() const
-    {
-        std::string res = mContainer->mNamePrefix;
-        res.push_back('[');
-        res.append(mPath);
-        res.push_back(']');
-        return res;
-    }
-
     ScriptsContainer::ScriptsContainer(LuaUtil::LuaState* lua, std::string_view namePrefix, ESM::LuaScriptCfg::Flags autoStartMode)
         : mNamePrefix(namePrefix), mLua(*lua), mAutoStartMode(autoStartMode)
     {
@@ -70,11 +61,19 @@ namespace LuaUtil
             return false;  // already present
 
         const std::string& path = scriptPath(scriptId);
+        std::string debugName = mNamePrefix;
+        debugName.push_back('[');
+        debugName.append(path);
+        debugName.push_back(']');
+
+        Script& script = mScripts[scriptId];
+        script.mHiddenData = mLua.newTable();
+        script.mHiddenData[sScriptIdKey] = ScriptId{this, scriptId};
+        script.mHiddenData[sScriptDebugNameKey] = debugName;
+        script.mPath = path;
+
         try
         {
-            Script& script = mScripts[scriptId];
-            script.mHiddenData = mLua.newTable();
-            script.mHiddenData[ScriptId::KEY] = ScriptId{this, scriptId, path};
             sol::object scriptOutput = mLua.runInNewSandbox(path, mNamePrefix, mAPI, script.mHiddenData);
             if (scriptOutput == sol::nil)
                 return true;
@@ -91,7 +90,7 @@ namespace LuaUtil
                 else if (sectionName == INTERFACE)
                     script.mInterface = value.as<sol::table>();
                 else
-                    Log(Debug::Error) << "Not supported section '" << sectionName << "' in " << mNamePrefix << "[" << path << "]";
+                    Log(Debug::Error) << "Not supported section '" << sectionName << "' in " << debugName;
             }
             if (engineHandlers != sol::nil)
             {
@@ -110,8 +109,7 @@ namespace LuaUtil
                     {
                         auto it = mEngineHandlers.find(handlerName);
                         if (it == mEngineHandlers.end())
-                            Log(Debug::Error) << "Not supported handler '" << handlerName
+                            Log(Debug::Error) << "Not supported handler '" << handlerName << "' in " << debugName;
-                                              << "' in " << mNamePrefix << "[" << path << "]";
                         else
                             insertHandler(it->second->mList, scriptId, fn);
                     }
@@ -131,7 +129,7 @@ namespace LuaUtil
 
             if (script.mInterfaceName.empty() == script.mInterface.has_value())
             {
-                Log(Debug::Error) << mNamePrefix << "[" << path << "]: 'interfaceName' should always be used together with 'interface'";
+                Log(Debug::Error) << debugName << ": 'interfaceName' should always be used together with 'interface'";
                 script.mInterfaceName.clear();
                 script.mInterface = sol::nil;
             }
@@ -145,8 +143,9 @@ namespace LuaUtil
         }
         catch (std::exception& e)
         {
+            mScripts[scriptId].mHiddenData[sScriptIdKey] = sol::nil;
             mScripts.erase(scriptId);
-            Log(Debug::Error) << "Can't start " << mNamePrefix << "[" << path << "]; " << e.what();
+            Log(Debug::Error) << "Can't start " << debugName << "; " << e.what();
             return false;
         }
     }
@@ -159,7 +158,7 @@ namespace LuaUtil
         Script& script = scriptIter->second;
         if (script.mInterface)
             removeInterface(scriptId, script);
-        script.mHiddenData[ScriptId::KEY] = sol::nil;
+        script.mHiddenData[sScriptIdKey] = sol::nil;
         mScripts.erase(scriptIter);
         for (auto& [_, handlers] : mEngineHandlers)
             removeHandler(handlers->mList, scriptId);
@@ -329,7 +328,9 @@ namespace LuaUtil
         for (auto& [scriptId, script] : mScripts)
         {
             ESM::LuaScript savedScript;
-            savedScript.mScriptPath = script.mHiddenData.get<ScriptId>(ScriptId::KEY).mPath;
+            // Note: We can not use `scriptPath(scriptId)` here because `save` can be called during
+            // evaluating "reloadlua" command when ScriptsConfiguration is already changed.
+            savedScript.mScriptPath = script.mPath;
             if (script.mOnSave)
             {
                 try
@@ -423,7 +424,7 @@ namespace LuaUtil
     ScriptsContainer::~ScriptsContainer()
     {
         for (auto& [_, script] : mScripts)
-            script.mHiddenData[ScriptId::KEY] = sol::nil;
+            script.mHiddenData[sScriptIdKey] = sol::nil;
     }
 
     // Note: shouldn't be called from destructor because mEngineHandlers has pointers on
@@ -431,7 +432,7 @@ namespace LuaUtil
     void ScriptsContainer::removeAllScripts()
     {
         for (auto& [_, script] : mScripts)
-            script.mHiddenData[ScriptId::KEY] = sol::nil;
+            script.mHiddenData[sScriptIdKey] = sol::nil;
         mScripts.clear();
         for (auto& [_, handlers] : mEngineHandlers)
             handlers->mList.clear();
@@ -529,4 +530,13 @@ namespace LuaUtil
         updateTimerQueue(mHoursTimersQueue, gameHours);
     }
 
+    void Callback::operator()(sol::object arg) const
+    {
+        if (mHiddenData[ScriptsContainer::sScriptIdKey] != sol::nil)
+            LuaUtil::call(mFunc, std::move(arg));
+        else
+            Log(Debug::Debug) << "Ignored callback to the removed script "
+                              << mHiddenData.get<std::string>(ScriptsContainer::sScriptDebugNameKey);
+    }
+
 }

components/lua/scriptscontainer.hpp

@@ -60,16 +60,18 @@ namespace LuaUtil
     class ScriptsContainer
     {
     public:
+        // ScriptId of each script is stored with this key in Script::mHiddenData.
+        // Removed from mHiddenData when the script if removed.
+        constexpr static std::string_view sScriptIdKey = "_id";
+
+        // Debug identifier of each script is stored with this key in Script::mHiddenData.
+        // Present in mHiddenData even after removal of the script from ScriptsContainer.
+        constexpr static std::string_view sScriptDebugNameKey = "_name";
+
         struct ScriptId
         {
-            // ScriptId is stored in hidden data (see getHiddenData) with this key.
-            constexpr static std::string_view KEY = "_id";
-
             ScriptsContainer* mContainer;
             int mIndex;  // index in LuaUtil::ScriptsConfiguration
-            std::string mPath;  // path to the script source in VFS
-
-            std::string toString() const;
         };
         using TimeUnit = ESM::LuaTimer::TimeUnit;
 
@@ -192,6 +194,7 @@ namespace LuaUtil
             sol::table mHiddenData;
             std::map<std::string, sol::function> mRegisteredCallbacks;
             std::map<int64_t, sol::function> mTemporaryCallbacks;
+            std::string mPath;
         };
         struct Timer
         {
@@ -239,6 +242,17 @@ namespace LuaUtil
         int64_t mTemporaryCallbackCounter = 0;
     };
 
+    // Wrapper for a single-argument Lua function.
+    // Holds information about the script the function belongs to.
+    // Needed to prevent callback calls if the script was removed.
+    struct Callback
+    {
+        sol::function mFunc;
+        sol::table mHiddenData;  // same object as Script::mHiddenData in ScriptsContainer
+
+        void operator()(sol::object arg) const;
+    };
+
 }
 
 #endif // COMPONENTS_LUA_SCRIPTSCONTAINER_H