Remove LuaManager::Action

apps/openmw/mwlua/luamanagerimp.cpp

@@ -206,12 +206,12 @@ namespace MWLua
             windowManager->printToConsole(msg, "#" + color.toHex());
         mInGameConsoleMessages.clear();
 
-        for (std::unique_ptr<Action>& action : mActionQueue)
+        for (DelayedAction& action : mActionQueue)
-            action->safeApply();
+            action.apply();
         mActionQueue.clear();
 
         if (mTeleportPlayerAction)
-            mTeleportPlayerAction->safeApply();
+            mTeleportPlayerAction->apply();
         mTeleportPlayerAction.reset();
     }
 
@@ -465,22 +465,24 @@ namespace MWLua
                 "No Lua handlers for console\n", MWBase::WindowManager::sConsoleColor_Error);
     }
 
-    LuaManager::Action::Action(LuaUtil::LuaState* state)
+    LuaManager::DelayedAction::DelayedAction(LuaUtil::LuaState* state, std::function<void()> fn, std::string_view name)
+        : mFn(std::move(fn))
+        , mName(name)
     {
         static const bool luaDebug = Settings::Manager::getBool("lua debug", "Lua");
         if (luaDebug)
             mCallerTraceback = state->debugTraceback();
     }
 
-    void LuaManager::Action::safeApply() const
+    void LuaManager::DelayedAction::apply() const
     {
         try
         {
-            apply();
+            mFn();
         }
         catch (const std::exception& e)
         {
-            Log(Debug::Error) << "Error in " << this->toString() << ": " << e.what();
+            Log(Debug::Error) << "Error in DelayedAction " << mName << ": " << e.what();
 
             if (mCallerTraceback.empty())
                 Log(Debug::Error) << "Set 'lua_debug=true' in settings.cfg to enable action tracebacks";
@@ -489,35 +491,14 @@ namespace MWLua
         }
     }
 
-    namespace
-    {
-        class FunctionAction final : public LuaManager::Action
-        {
-        public:
-            FunctionAction(LuaUtil::LuaState* state, std::function<void()> fn, std::string_view name)
-                : Action(state)
-                , mFn(std::move(fn))
-                , mName(name)
-            {
-            }
-
-            void apply() const override { mFn(); }
-            std::string toString() const override { return "FunctionAction " + mName; }
-
-        private:
-            std::function<void()> mFn;
-            std::string mName;
-        };
-    }
-
     void LuaManager::addAction(std::function<void()> action, std::string_view name)
     {
-        mActionQueue.push_back(std::make_unique<FunctionAction>(&mLua, std::move(action), name));
+        mActionQueue.emplace_back(&mLua, std::move(action), name);
     }
 
     void LuaManager::addTeleportPlayerAction(std::function<void()> action)
     {
-        mTeleportPlayerAction = std::make_unique<FunctionAction>(&mLua, std::move(action), "TeleportPlayer");
+        mTeleportPlayerAction = DelayedAction(&mLua, std::move(action), "TeleportPlayer");
     }
 
     void LuaManager::reportStats(unsigned int frameNumber, osg::Stats& stats) const

apps/openmw/mwlua/luamanagerimp.hpp

@@ -86,24 +86,8 @@ namespace MWLua
         }
 
         // Some changes to the game world can not be done from the scripting thread (because it runs in parallel with
-        // OSG Cull), so we need to queue it and apply from the main thread. All such changes should be implemented as
+        // OSG Cull), so we need to queue it and apply from the main thread.
-        // classes inherited from MWLua::Action.
-        class Action
-        {
-        public:
-            Action(LuaUtil::LuaState* state);
-            virtual ~Action() {}
-
-            void safeApply() const;
-            virtual void apply() const = 0;
-            virtual std::string toString() const = 0;
-
-        private:
-            std::string mCallerTraceback;
-        };
-
         void addAction(std::function<void()> action, std::string_view name = "");
-        void addAction(std::unique_ptr<Action>&& action) { mActionQueue.push_back(std::move(action)); }
         void addTeleportPlayerAction(std::function<void()> action);
 
         // Saving
@@ -183,8 +167,19 @@ namespace MWLua
         std::vector<CallbackWithData> mQueuedCallbacks;
 
         // Queued actions that should be done in main thread. Processed by applyQueuedChanges().
-        std::vector<std::unique_ptr<Action>> mActionQueue;
+        class DelayedAction
-        std::unique_ptr<Action> mTeleportPlayerAction;
+        {
+        public:
+            DelayedAction(LuaUtil::LuaState* state, std::function<void()> fn, std::string_view name);
+            void apply() const;
+
+        private:
+            std::string mCallerTraceback;
+            std::function<void()> mFn;
+            std::string mName;
+        };
+        std::vector<DelayedAction> mActionQueue;
+        std::optional<DelayedAction> mTeleportPlayerAction;
         std::vector<std::string> mUIMessages;
         std::vector<std::pair<std::string, Misc::Color>> mInGameConsoleMessages;
 

apps/openmw/mwlua/postprocessingbindings.cpp

@@ -5,39 +5,6 @@
 
 #include "luamanagerimp.hpp"
 
-namespace
-{
-    template <class T>
-    class SetUniformShaderAction final : public MWLua::LuaManager::Action
-    {
-    public:
-        SetUniformShaderAction(
-            LuaUtil::LuaState* state, std::shared_ptr<fx::Technique> shader, const std::string& name, const T& value)
-            : MWLua::LuaManager::Action(state)
-            , mShader(std::move(shader))
-            , mName(name)
-            , mValue(value)
-        {
-        }
-
-        void apply() const override
-        {
-            MWBase::Environment::get().getWorld()->getPostProcessor()->setUniform(mShader, mName, mValue);
-        }
-
-        std::string toString() const override
-        {
-            return std::string("SetUniformShaderAction shader=") + (mShader ? mShader->getName() : "nil")
-                + std::string("uniform=") + (mShader ? mName : "nil");
-        }
-
-    private:
-        std::shared_ptr<fx::Technique> mShader;
-        std::string mName;
-        T mValue;
-    };
-}
-
 namespace MWLua
 {
     struct Shader;
@@ -84,7 +51,10 @@ namespace MWLua
     {
         return [context](const Shader& shader, const std::string& name, const T& value) {
             context.mLuaManager->addAction(
-                std::make_unique<SetUniformShaderAction<T>>(context.mLua, shader.mShader, name, value));
+                [&] {
+                    MWBase::Environment::get().getWorld()->getPostProcessor()->setUniform(shader.mShader, name, value);
+                },
+                "SetUniformShaderAction");
         };
     }
 
@@ -114,7 +84,10 @@ namespace MWLua
             }
 
             context.mLuaManager->addAction(
-                std::make_unique<SetUniformShaderAction<std::vector<T>>>(context.mLua, shader.mShader, name, values));
+                [&] {
+                    MWBase::Environment::get().getWorld()->getPostProcessor()->setUniform(shader.mShader, name, values);
+                },
+                "SetUniformShaderAction");
         };
     }
 

apps/openmw/mwlua/stats.cpp

@@ -55,29 +55,17 @@ namespace
 
 namespace MWLua
 {
-    namespace
+    static void addStatUpdateAction(MWLua::LuaManager* manager, const SelfObject& obj)
     {
-        class StatUpdateAction final : public LuaManager::Action
+        if (!obj.mStatsCache.empty())
-        {
+            return; // was already added before
-            ObjectId mId;
+        manager->addAction(
-
+            [obj = Object(obj)] {
-        public:
-            StatUpdateAction(LuaUtil::LuaState* state, ObjectId id)
-                : Action(state)
-                , mId(id)
-            {
-            }
-
-            void apply() const override
-            {
-                LObject obj(mId);
                 LocalScripts* scripts = obj.ptr().getRefData().getLuaScripts();
                 if (scripts)
                     scripts->applyStatsCache();
-            }
+            },
-
+            "StatUpdateAction");
-            std::string toString() const override { return "StatUpdateAction"; }
-        };
     }
 
     class LevelStat
@@ -99,8 +87,7 @@ namespace MWLua
         void setCurrent(const Context& context, const sol::object& value) const
         {
             SelfObject* obj = mObject.asSelfObject();
-            if (obj->mStatsCache.empty())
+            addStatUpdateAction(context.mLuaManager, *obj);
-                context.mLuaManager->addAction(std::make_unique<StatUpdateAction>(context.mLua, obj->id()));
             obj->mStatsCache[SelfObject::CachedStat{ &LevelStat::setValue, 0, "current" }] = value;
         }
 
@@ -158,8 +145,7 @@ namespace MWLua
         void cache(const Context& context, std::string_view prop, const sol::object& value) const
         {
             SelfObject* obj = mObject.asSelfObject();
-            if (obj->mStatsCache.empty())
+            addStatUpdateAction(context.mLuaManager, *obj);
-                context.mLuaManager->addAction(std::make_unique<StatUpdateAction>(context.mLua, obj->id()));
             obj->mStatsCache[SelfObject::CachedStat{ &DynamicStat::setValue, mIndex, prop }] = value;
         }
 
@@ -217,8 +203,7 @@ namespace MWLua
         void cache(const Context& context, std::string_view prop, const sol::object& value) const
         {
             SelfObject* obj = mObject.asSelfObject();
-            if (obj->mStatsCache.empty())
+            addStatUpdateAction(context.mLuaManager, *obj);
-                context.mLuaManager->addAction(std::make_unique<StatUpdateAction>(context.mLua, obj->id()));
             obj->mStatsCache[SelfObject::CachedStat{ &AttributeStat::setValue, mIndex, prop }] = value;
         }
 
@@ -302,8 +287,7 @@ namespace MWLua
         void cache(const Context& context, std::string_view prop, const sol::object& value) const
         {
             SelfObject* obj = mObject.asSelfObject();
-            if (obj->mStatsCache.empty())
+            addStatUpdateAction(context.mLuaManager, *obj);
-                context.mLuaManager->addAction(std::make_unique<StatUpdateAction>(context.mLua, obj->id()));
             obj->mStatsCache[SelfObject::CachedStat{ &SkillStat::setValue, mIndex, prop }] = value;
         }
 

apps/openmw/mwlua/types/actor.cpp

@@ -18,107 +18,84 @@
 
 namespace MWLua
 {
-    namespace
+    using EquipmentItem = std::variant<std::string, ObjectId>;
+    using Equipment = std::map<int, EquipmentItem>;
+
+    static void setEquipment(const MWWorld::Ptr& actor, const Equipment& equipment)
     {
-        class SetEquipmentAction final : public LuaManager::Action
+        MWWorld::InventoryStore& store = actor.getClass().getInventoryStore(actor);
-        {
+        std::array<bool, MWWorld::InventoryStore::Slots> usedSlots;
-        public:
+        std::fill(usedSlots.begin(), usedSlots.end(), false);
-            using Item = std::variant<std::string, ObjectId>; // recordId or ObjectId
-            using Equipment = std::map<int, Item>; // slot to item
 
-            SetEquipmentAction(LuaUtil::LuaState* state, ObjectId actor, Equipment equipment)
+        static constexpr int anySlot = -1;
-                : Action(state)
+        auto tryEquipToSlot = [&store, &usedSlots](int slot, const EquipmentItem& item) -> bool {
-                , mActor(actor)
+            auto old_it = slot != anySlot ? store.getSlot(slot) : store.end();
-                , mEquipment(std::move(equipment))
+            MWWorld::Ptr itemPtr;
+            if (std::holds_alternative<ObjectId>(item))
             {
-            }
+                itemPtr = MWBase::Environment::get().getWorldModel()->getPtr(std::get<ObjectId>(item));
-
+                if (old_it != store.end() && *old_it == itemPtr)
-            void apply() const override
+                    return true; // already equipped
-            {
+                if (itemPtr.isEmpty() || itemPtr.getRefData().getCount() == 0
-                MWWorld::Ptr actor = MWBase::Environment::get().getWorldModel()->getPtr(mActor);
+                    || itemPtr.getContainerStore() != static_cast<const MWWorld::ContainerStore*>(&store))
-                MWWorld::InventoryStore& store = actor.getClass().getInventoryStore(actor);
-                std::array<bool, MWWorld::InventoryStore::Slots> usedSlots;
-                std::fill(usedSlots.begin(), usedSlots.end(), false);
-
-                static constexpr int anySlot = -1;
-                auto tryEquipToSlot = [&store, &usedSlots](int slot, const Item& item) -> bool {
-                    auto old_it = slot != anySlot ? store.getSlot(slot) : store.end();
-                    MWWorld::Ptr itemPtr;
-                    if (std::holds_alternative<ObjectId>(item))
-                    {
-                        itemPtr = MWBase::Environment::get().getWorldModel()->getPtr(std::get<ObjectId>(item));
-                        if (old_it != store.end() && *old_it == itemPtr)
-                            return true; // already equipped
-                        if (itemPtr.isEmpty() || itemPtr.getRefData().getCount() == 0
-                            || itemPtr.getContainerStore() != static_cast<const MWWorld::ContainerStore*>(&store))
-                        {
-                            Log(Debug::Warning)
-                                << "Object" << std::get<ObjectId>(item).toString() << " is not in inventory";
-                            return false;
-                        }
-                    }
-                    else
-                    {
-                        const ESM::RefId& recordId = ESM::RefId::stringRefId(std::get<std::string>(item));
-                        if (old_it != store.end() && old_it->getCellRef().getRefId() == recordId)
-                            return true; // already equipped
-                        itemPtr = store.search(recordId);
-                        if (itemPtr.isEmpty() || itemPtr.getRefData().getCount() == 0)
-                        {
-                            Log(Debug::Warning) << "There is no object with recordId='" << recordId << "' in inventory";
-                            return false;
-                        }
-                    }
-
-                    auto [allowedSlots, _] = itemPtr.getClass().getEquipmentSlots(itemPtr);
-                    bool requestedSlotIsAllowed
-                        = std::find(allowedSlots.begin(), allowedSlots.end(), slot) != allowedSlots.end();
-                    if (!requestedSlotIsAllowed)
-                    {
-                        auto firstAllowed = std::find_if(
-                            allowedSlots.begin(), allowedSlots.end(), [&](int s) { return !usedSlots[s]; });
-                        if (firstAllowed == allowedSlots.end())
-                        {
-                            Log(Debug::Warning) << "No suitable slot for " << itemPtr.toString();
-                            return false;
-                        }
-                        slot = *firstAllowed;
-                    }
-
-                    // TODO: Refactor InventoryStore to accept Ptr and get rid of this linear search.
-                    MWWorld::ContainerStoreIterator it = std::find(store.begin(), store.end(), itemPtr);
-                    if (it == store.end()) // should never happen
-                        throw std::logic_error("Item not found in container");
-
-                    store.equip(slot, it);
-                    return requestedSlotIsAllowed; // return true if equipped to requested slot and false if slot was
-                                                   // changed
-                };
-
-                for (int slot = 0; slot < MWWorld::InventoryStore::Slots; ++slot)
                 {
-                    auto old_it = store.getSlot(slot);
+                    Log(Debug::Warning) << "Object" << std::get<ObjectId>(item).toString() << " is not in inventory";
-                    auto new_it = mEquipment.find(slot);
+                    return false;
-                    if (new_it == mEquipment.end())
+                }
-                    {
+            }
-                        if (old_it != store.end())
+            else
-                            store.unequipSlot(slot);
+            {
-                        continue;
+                const ESM::RefId& recordId = ESM::RefId::stringRefId(std::get<std::string>(item));
-                    }
+                if (old_it != store.end() && old_it->getCellRef().getRefId() == recordId)
-                    if (tryEquipToSlot(slot, new_it->second))
+                    return true; // already equipped
-                        usedSlots[slot] = true;
+                itemPtr = store.search(recordId);
+                if (itemPtr.isEmpty() || itemPtr.getRefData().getCount() == 0)
+                {
+                    Log(Debug::Warning) << "There is no object with recordId='" << recordId << "' in inventory";
+                    return false;
                 }
-                for (const auto& [slot, item] : mEquipment)
-                    if (slot >= MWWorld::InventoryStore::Slots)
-                        tryEquipToSlot(anySlot, item);
             }
 
-            std::string toString() const override { return "SetEquipmentAction"; }
+            auto [allowedSlots, _] = itemPtr.getClass().getEquipmentSlots(itemPtr);
+            bool requestedSlotIsAllowed
+                = std::find(allowedSlots.begin(), allowedSlots.end(), slot) != allowedSlots.end();
+            if (!requestedSlotIsAllowed)
+            {
+                auto firstAllowed
+                    = std::find_if(allowedSlots.begin(), allowedSlots.end(), [&](int s) { return !usedSlots[s]; });
+                if (firstAllowed == allowedSlots.end())
+                {
+                    Log(Debug::Warning) << "No suitable slot for " << itemPtr.toString();
+                    return false;
+                }
+                slot = *firstAllowed;
+            }
 
-        private:
+            // TODO: Refactor InventoryStore to accept Ptr and get rid of this linear search.
-            ObjectId mActor;
+            MWWorld::ContainerStoreIterator it = std::find(store.begin(), store.end(), itemPtr);
-            Equipment mEquipment;
+            if (it == store.end()) // should never happen
+                throw std::logic_error("Item not found in container");
+
+            store.equip(slot, it);
+            return requestedSlotIsAllowed; // return true if equipped to requested slot and false if slot was changed
         };
+
+        for (int slot = 0; slot < MWWorld::InventoryStore::Slots; ++slot)
+        {
+            auto old_it = store.getSlot(slot);
+            auto new_it = equipment.find(slot);
+            if (new_it == equipment.end())
+            {
+                if (old_it != store.end())
+                    store.unequipSlot(slot);
+                continue;
+            }
+            if (tryEquipToSlot(slot, new_it->second))
+                usedSlots[slot] = true;
+        }
+        for (const auto& [slot, item] : equipment)
+            if (slot >= MWWorld::InventoryStore::Slots)
+                tryEquipToSlot(anySlot, item);
     }
 
     void addActorBindings(sol::table actor, const Context& context)
@@ -263,13 +240,14 @@ namespace MWLua
             return store.isEquipped(item.ptr());
         };
         actor["setEquipment"] = [context](const SelfObject& obj, const sol::table& equipment) {
-            if (!obj.ptr().getClass().hasInventoryStore(obj.ptr()))
+            const MWWorld::Ptr& ptr = obj.ptr();
+            if (!ptr.getClass().hasInventoryStore(ptr))
             {
                 if (!equipment.empty())
                     throw std::runtime_error(obj.toString() + " has no equipment slots");
                 return;
             }
-            SetEquipmentAction::Equipment eqp;
+            Equipment eqp;
             for (auto& [key, value] : equipment)
             {
                 int slot = key.as<int>();
@@ -279,7 +257,7 @@ namespace MWLua
                     eqp[slot] = value.as<std::string>();
             }
             context.mLuaManager->addAction(
-                std::make_unique<SetEquipmentAction>(context.mLua, obj.id(), std::move(eqp)));
+                [obj = Object(ptr), eqp = std::move(eqp)] { setEquipment(obj.ptr(), eqp); }, "SetEquipmentAction");
         };
         actor["getPathfindingAgentBounds"] = [](sol::this_state lua, const LObject& o) {
             const DetourNavigator::AgentBounds agentBounds

apps/openmw/mwlua/uibindings.cpp

@@ -20,71 +20,20 @@ namespace MWLua
 {
     namespace
     {
-        class UiAction final : public LuaManager::Action
+        template <typename Fn>
+        void wrapAction(const std::shared_ptr<LuaUi::Element>& element, Fn&& fn)
         {
-        public:
+            try
-            enum Type
-            {
-                CREATE = 0,
-                UPDATE,
-                DESTROY,
-            };
-
-            UiAction(Type type, std::shared_ptr<LuaUi::Element> element, LuaUtil::LuaState* state)
-                : Action(state)
-                , mType{ type }
-                , mElement{ std::move(element) }
             {
+                fn();
             }
-
+            catch (...)
-            void apply() const override
             {
-                try
+                // prevent any actions on a potentially corrupted widget
-                {
+                element->mRoot = nullptr;
-                    switch (mType)
+                throw;
-                    {
-                        case CREATE:
-                            mElement->create();
-                            break;
-                        case UPDATE:
-                            mElement->update();
-                            break;
-                        case DESTROY:
-                            mElement->destroy();
-                            break;
-                    }
-                }
-                catch (std::exception&)
-                {
-                    // prevent any actions on a potentially corrupted widget
-                    mElement->mRoot = nullptr;
-                    throw;
-                }
             }
-
+        }
-            std::string toString() const override
-            {
-                std::string result;
-                switch (mType)
-                {
-                    case CREATE:
-                        result += "Create";
-                        break;
-                    case UPDATE:
-                        result += "Update";
-                        break;
-                    case DESTROY:
-                        result += "Destroy";
-                        break;
-                }
-                result += " UI";
-                return result;
-            }
-
-        private:
-            Type mType;
-            std::shared_ptr<LuaUi::Element> mElement;
-        };
 
         // Lua arrays index from 1
         inline size_t fromLuaIndex(size_t i)
@@ -102,17 +51,17 @@ namespace MWLua
         auto element = context.mLua->sol().new_usertype<LuaUi::Element>("Element");
         element["layout"] = sol::property([](LuaUi::Element& element) { return element.mLayout; },
             [](LuaUi::Element& element, const sol::table& layout) { element.mLayout = layout; });
-        element["update"] = [context](const std::shared_ptr<LuaUi::Element>& element) {
+        element["update"] = [luaManager = context.mLuaManager](const std::shared_ptr<LuaUi::Element>& element) {
             if (element->mDestroy || element->mUpdate)
                 return;
             element->mUpdate = true;
-            context.mLuaManager->addAction(std::make_unique<UiAction>(UiAction::UPDATE, element, context.mLua));
+            luaManager->addAction([element] { wrapAction(element, [&] { element->update(); }); }, "Update UI");
         };
-        element["destroy"] = [context](const std::shared_ptr<LuaUi::Element>& element) {
+        element["destroy"] = [luaManager = context.mLuaManager](const std::shared_ptr<LuaUi::Element>& element) {
             if (element->mDestroy)
                 return;
             element->mDestroy = true;
-            context.mLuaManager->addAction(std::make_unique<UiAction>(UiAction::DESTROY, element, context.mLua));
+            luaManager->addAction([element] { wrapAction(element, [&] { element->destroy(); }); }, "Destroy UI");
         };
 
         sol::table api = context.mLua->newTable();
@@ -144,9 +93,9 @@ namespace MWLua
             }
         };
         api["content"] = LuaUi::loadContentConstructor(context.mLua);
-        api["create"] = [context](const sol::table& layout) {
+        api["create"] = [luaManager = context.mLuaManager](const sol::table& layout) {
             auto element = LuaUi::Element::make(layout);
-            context.mLuaManager->addAction(std::make_unique<UiAction>(UiAction::CREATE, element, context.mLua));
+            luaManager->addAction([element] { wrapAction(element, [&] { element->create(); }); }, "Create UI");
             return element;
         };
         api["updateAll"] = [context]() {