Merge branch 'lua_action_errors' into 'master'

Add error checking in MWLua::Action

See merge request OpenMW/openmw!1401

apps/openmw/mwlua/actions.cpp

@@ -3,6 +3,7 @@
 #include <cstring>
 
 #include <components/debug/debuglog.hpp>
+#include <components/lua/luastate.hpp>
 
 #include "../mwworld/cellstore.hpp"
 #include "../mwworld/class.hpp"
@@ -12,14 +13,34 @@
 namespace MWLua
 {
 
+#ifdef NDEBUG
+    Action::Action(LuaUtil::LuaState* state) {}
+#else
+    Action::Action(LuaUtil::LuaState* state) : mCallerTraceback(state->debugTraceback()) {}
+#endif
+
+    void Action::safeApply(WorldView& w) const
+    {
+        try
+        {
+            apply(w);
+        }
+        catch (const std::exception& e)
+        {
+            Log(Debug::Error) << "Error in " << this->toString() << ": " << e.what();
+#ifdef NDEBUG
+            Log(Debug::Error) << "Traceback is available only in debug builds";
+#else
+            Log(Debug::Error) << "Caller " << mCallerTraceback;
+#endif
+        }
+    }
+
     void TeleportAction::apply(WorldView& worldView) const
     {
         MWWorld::CellStore* cell = worldView.findCell(mCell, mPos);
         if (!cell)
-        {
+            throw std::runtime_error(std::string("cell not found: '") + mCell + "'");
-            Log(Debug::Error) << "LuaManager::applyTeleport -> cell not found: '" << mCell << "'";
-            return;
-        }
 
         MWBase::World* world = MWBase::Environment::get().getWorld();
         MWWorld::Ptr obj = worldView.getObjectRegistry()->getPtr(mObject, false);

apps/openmw/mwlua/actions.hpp

@@ -6,6 +6,11 @@
 #include "object.hpp"
 #include "worldview.hpp"
 
+namespace LuaUtil
+{
+    class LuaState;
+}
+
 namespace MWLua
 {
 
@@ -16,17 +21,27 @@ namespace MWLua
     class Action
     {
     public:
+        Action(LuaUtil::LuaState* state);
         virtual ~Action() {}
+
+        void safeApply(WorldView&) const;
         virtual void apply(WorldView&) const = 0;
+        virtual std::string toString() const = 0;
+
+    private:
+#ifndef NDEBUG
+        std::string mCallerTraceback;
+#endif
     };
 
     class TeleportAction final : public Action
     {
     public:
-        TeleportAction(ObjectId object, std::string cell, const osg::Vec3f& pos, const osg::Vec3f& rot)
+        TeleportAction(LuaUtil::LuaState* state, ObjectId object, std::string cell, const osg::Vec3f& pos, const osg::Vec3f& rot)
-            : mObject(object), mCell(std::move(cell)), mPos(pos), mRot(rot) {}
+            : Action(state), mObject(object), mCell(std::move(cell)), mPos(pos), mRot(rot) {}
 
         void apply(WorldView&) const override;
+        std::string toString() const override { return "TeleportAction"; }
 
     private:
         ObjectId mObject;
@@ -41,9 +56,11 @@ namespace MWLua
         using Item = std::variant<std::string, ObjectId>;  // recordId or ObjectId
         using Equipment = std::map<int, Item>;  // slot to item
 
-        SetEquipmentAction(ObjectId actor, Equipment equipment) : mActor(actor), mEquipment(std::move(equipment)) {}
+        SetEquipmentAction(LuaUtil::LuaState* state, ObjectId actor, Equipment equipment)
+            : Action(state), mActor(actor), mEquipment(std::move(equipment)) {}
 
         void apply(WorldView&) const override;
+        std::string toString() const override { return "SetEquipmentAction"; }
 
     private:
         ObjectId mActor;

apps/openmw/mwlua/localscripts.cpp

@@ -39,7 +39,7 @@ namespace MWLua
         selfAPI["controls"] = sol::readonly_property([](SelfObject& self) { return &self.mControls; });
         selfAPI["isActive"] = [](SelfObject& self) { return &self.mIsActive; };
         selfAPI["enableAI"] = [](SelfObject& self, bool v) { self.mControls.mDisableAI = !v; };
-        selfAPI["setEquipment"] = [manager=context.mLuaManager](const SelfObject& obj, sol::table equipment)
+        selfAPI["setEquipment"] = [context](const SelfObject& obj, sol::table equipment)
         {
             if (!obj.ptr().getClass().hasInventoryStore(obj.ptr()))
             {
@@ -56,7 +56,7 @@ namespace MWLua
                 else
                     eqp[slot] = value.as<std::string>();
             }
-            manager->addAction(std::make_unique<SetEquipmentAction>(obj.id(), std::move(eqp)));
+            context.mLuaManager->addAction(std::make_unique<SetEquipmentAction>(context.mLua, obj.id(), std::move(eqp)));
         };
         selfAPI["getCombatTarget"] = [worldView=context.mWorldView](SelfObject& self) -> sol::optional<LObject>
         {

apps/openmw/mwlua/luabindings.cpp

@@ -28,8 +28,7 @@ namespace MWLua
         api["API_REVISION"] = 9;
         api["quit"] = [lua]()
         {
-            std::string traceback = lua->sol()["debug"]["traceback"]().get<std::string>();
+            Log(Debug::Warning) << "Quit requested by a Lua script.\n" << lua->debugTraceback();
-            Log(Debug::Warning) << "Quit requested by a Lua script.\n" << traceback;
             MWBase::Environment::get().getStateManager()->requestQuit();
         };
         api["sendGlobalEvent"] = [context](std::string eventName, const sol::object& eventData)

apps/openmw/mwlua/luamanagerimp.cpp

@@ -188,11 +188,11 @@ namespace MWLua
         mUIMessages.clear();
 
         for (std::unique_ptr<Action>& action : mActionQueue)
-            action->apply(mWorldView);
+            action->safeApply(mWorldView);
         mActionQueue.clear();
         
         if (mTeleportPlayerAction)
-            mTeleportPlayerAction->apply(mWorldView);
+            mTeleportPlayerAction->safeApply(mWorldView);
         mTeleportPlayerAction.reset();
     }
 

apps/openmw/mwlua/objectbindings.cpp

@@ -179,16 +179,16 @@ namespace MWLua
                 localScripts->removeScript(*scriptId);
             };
 
-            objectT["teleport"] = [luaManager=context.mLuaManager](const GObject& object, std::string_view cell,
+            objectT["teleport"] = [context](const GObject& object, std::string_view cell,
-                                                                   const osg::Vec3f& pos, const sol::optional<osg::Vec3f>& optRot)
+                                            const osg::Vec3f& pos, const sol::optional<osg::Vec3f>& optRot)
             {
                 MWWorld::Ptr ptr = object.ptr();
                 osg::Vec3f rot = optRot ? *optRot : ptr.getRefData().getPosition().asRotationVec3();
-                auto action = std::make_unique<TeleportAction>(object.id(), std::string(cell), pos, rot);
+                auto action = std::make_unique<TeleportAction>(context.mLua, object.id(), std::string(cell), pos, rot);
                 if (ptr == MWBase::Environment::get().getWorld()->getPlayerPtr())
-                    luaManager->addTeleportPlayerAction(std::move(action));
+                    context.mLuaManager->addTeleportPlayerAction(std::move(action));
                 else
-                    luaManager->addAction(std::move(action));
+                    context.mLuaManager->addAction(std::move(action));
             };
         }
         else
@@ -352,7 +352,7 @@ namespace MWLua
 
         if constexpr (std::is_same_v<ObjectT, GObject>)
         {  // Only for global scripts
-            objectT["setEquipment"] = [manager=context.mLuaManager](const GObject& obj, sol::table equipment)
+            objectT["setEquipment"] = [context](const GObject& obj, sol::table equipment)
             {
                 if (!obj.ptr().getClass().hasInventoryStore(obj.ptr()))
                 {
@@ -360,7 +360,8 @@ namespace MWLua
                         throw std::runtime_error(ptrToString(obj.ptr()) + " has no equipment slots");
                     return;
                 }
-                manager->addAction(std::make_unique<SetEquipmentAction>(obj.id(), parseEquipmentTable(equipment)));
+                context.mLuaManager->addAction(std::make_unique<SetEquipmentAction>(
+                    context.mLua, obj.id(), parseEquipmentTable(equipment)));
             };
 
             // TODO

components/lua/luastate.hpp

@@ -35,6 +35,11 @@ namespace LuaUtil
         // Returns underlying sol::state.
         sol::state& sol() { return mLua; }
 
+        // Can be used by a C++ function that is called from Lua to get the Lua traceback.
+        // Makes no sense if called not from Lua code.
+        // Note: It is a slow function, should be used for debug purposes only.
+        std::string debugTraceback() { return mLua["debug"]["traceback"]().get<std::string>(); }
+
         // A shortcut to create a new Lua table.
         sol::table newTable() { return sol::table(mLua, sol::create); }