Fixes and refactoring

apps/openmw/engine.cpp

@@ -833,18 +833,17 @@ void OMW::Engine::prepareEngine (Settings::Manager & settings)
 class OMW::Engine::LuaWorker
 {
 public:
-    explicit LuaWorker(Engine* engine) :
+    explicit LuaWorker(Engine* engine) : mEngine(engine)
-        mEngine(engine),
-        mSeparateThread(Settings::Manager::getInt("lua num threads", "Lua") > 0)
     {
-        if (mSeparateThread)
+        if (Settings::Manager::getInt("lua num threads", "Lua") > 0)
             mThread = std::thread([this]{ threadBody(); });
     };
 
     void allowUpdate(double dt)
     {
         mDt = dt;
-        if (!mSeparateThread)
+        mIsGuiMode = mEngine->mEnvironment.getWindowManager()->isGuiMode();
+        if (!mThread)
             return;
         {
             std::lock_guard<std::mutex> lk(mMutex);
@@ -855,7 +854,7 @@ public:
 
     void finishUpdate()
     {
-        if (mSeparateThread)
+        if (mThread)
         {
             std::unique_lock<std::mutex> lk(mMutex);
             mCV.wait(lk, [&]{ return !mUpdateRequest; });
@@ -867,8 +866,8 @@ public:
 
     void join()
     {
-        if (mSeparateThread)
+        if (mThread)
-            mThread.join();
+            mThread->join();
     }
 
 private:
@@ -879,7 +878,7 @@ private:
         const unsigned int frameNumber = viewer->getFrameStamp()->getFrameNumber();
         ScopedProfile<UserStatsType::Lua> profile(frameStart, frameNumber, *osg::Timer::instance(), *viewer->getViewerStats());
 
-        mEngine->mLuaManager->update(mEngine->mEnvironment.getWindowManager()->isGuiMode(), mDt);
+        mEngine->mLuaManager->update(mIsGuiMode, mDt);
     }
 
     void threadBody()
@@ -898,12 +897,12 @@ private:
     }
 
     Engine* mEngine;
-    const bool mSeparateThread;
     std::mutex mMutex;
     std::condition_variable mCV;
     bool mUpdateRequest;
     double mDt = 0;
-    std::thread mThread;
+    bool mIsGuiMode = false;
+    std::optional<std::thread> mThread;
 };
 
 // Initialise and enter main loop.

apps/openmw/mwbase/luamanager.hpp

@@ -17,7 +17,7 @@ namespace ESM
 {
     class ESMReader;
     class ESMWriter;
-    class LuaScripts;
+    struct LuaScripts;
 }
 
 namespace MWBase
@@ -40,15 +40,16 @@ namespace MWBase
         // virtual void objectOnHit(const MWWorld::Ptr &ptr, float damage, bool ishealth, const MWWorld::Ptr &object,
         //                          const MWWorld::Ptr &attacker, const osg::Vec3f &hitPosition, bool successful) = 0;
 
-        struct ActorControls {
+        struct ActorControls
-            bool disableAI;
+        {
-            bool controlledFromLua;
+            bool mDisableAI;
+            bool mControlledFromLua;
 
-            bool jump;
+            bool mJump;
-            bool run;
+            bool mRun;
-            float movement;
+            float mMovement;
-            float sideMovement;
+            float mSideMovement;
-            float turn;
+            float mTurn;
         };
 
         virtual ActorControls* getActorControls(const MWWorld::Ptr&) const = 0;

apps/openmw/mwlua/actions.cpp

@@ -53,7 +53,7 @@ namespace MWLua
         std::fill(usedSlots.begin(), usedSlots.end(), false);
 
         constexpr int anySlot = -1;
-        auto tryEquipToSlot = [&](int slot, const Item& item) -> bool
+        auto tryEquipToSlot = [&actor, &store, &usedSlots, &worldView, anySlot](int slot, const Item& item) -> bool
         {
             auto old_it = slot != anySlot ? store.getSlot(slot) : store.end();
             MWWorld::Ptr itemPtr;
@@ -83,23 +83,16 @@ namespace MWLua
             }
 
             auto [allowedSlots, _] = itemPtr.getClass().getEquipmentSlots(itemPtr);
-            bool requestedSlotIsAllowed = false;
+            bool requestedSlotIsAllowed = std::find(allowedSlots.begin(), allowedSlots.end(), slot) != allowedSlots.end();
-            for (int allowedSlot : allowedSlots)
-                requestedSlotIsAllowed = requestedSlotIsAllowed || allowedSlot == slot;
             if (!requestedSlotIsAllowed)
             {
-                slot = anySlot;
+                auto firstAllowed = std::find_if(allowedSlots.begin(), allowedSlots.end(), [&](int s) { return !usedSlots[s]; });
-                for (int allowedSlot : allowedSlots)
+                if (firstAllowed == allowedSlots.end())
-                    if (!usedSlots[allowedSlot])
-                    {
-                        slot = allowedSlot;
-                        break;
-                    }
-                if (slot == anySlot)
                 {
                     Log(Debug::Warning) << "No suitable slot for " << ptrToString(itemPtr);
                     return false;
                 }
+                slot = *firstAllowed;
             }
 
             // TODO: Refactor InventoryStore to accept Ptr and get rid of this linear search.
@@ -124,7 +117,7 @@ namespace MWLua
             if (tryEquipToSlot(slot, new_it->second))
                 usedSlots[slot] = true;
         }
-        for (auto [slot, item] : mEquipment)
+        for (const auto& [slot, item] : mEquipment)
             if (slot >= MWWorld::InventoryStore::Slots)
                 tryEquipToSlot(anySlot, item);
     }

apps/openmw/mwlua/asyncbindings.cpp

@@ -17,6 +17,7 @@ namespace MWLua
 
     sol::function getAsyncPackageInitializer(const Context& context)
     {
+        using TimeUnit = LuaUtil::ScriptsContainer::TimeUnit;
         sol::usertype<AsyncPackageId> api = context.mLua->sol().new_usertype<AsyncPackageId>("AsyncPackage");
         api["registerTimerCallback"] = [](const AsyncPackageId& asyncId, std::string_view name, sol::function callback)
         {
@@ -27,21 +28,25 @@ namespace MWLua
                                                               const TimerCallback& callback, sol::object callbackArg)
         {
             callback.mAsyncId.mContainer->setupSerializableTimer(
-                false, world->getGameTimeInSeconds() + delay, callback.mAsyncId.mScript, callback.mName, std::move(callbackArg));
+                TimeUnit::SECONDS, world->getGameTimeInSeconds() + delay,
+                callback.mAsyncId.mScript, callback.mName, std::move(callbackArg));
         };
         api["newTimerInHours"] = [world=context.mWorldView](const AsyncPackageId&, double delay,
                                                             const TimerCallback& callback, sol::object callbackArg)
         {
             callback.mAsyncId.mContainer->setupSerializableTimer(
-                true, world->getGameTimeInHours() + delay, callback.mAsyncId.mScript, callback.mName, std::move(callbackArg));
+                TimeUnit::HOURS, world->getGameTimeInHours() + delay,
+                callback.mAsyncId.mScript, callback.mName, std::move(callbackArg));
         };
         api["newUnsavableTimerInSeconds"] = [world=context.mWorldView](const AsyncPackageId& asyncId, double delay, sol::function callback)
         {
-            asyncId.mContainer->setupUnsavableTimer(false, world->getGameTimeInSeconds() + delay, asyncId.mScript, std::move(callback));
+            asyncId.mContainer->setupUnsavableTimer(
+                TimeUnit::SECONDS, world->getGameTimeInSeconds() + delay, asyncId.mScript, std::move(callback));
         };
         api["newUnsavableTimerInHours"] = [world=context.mWorldView](const AsyncPackageId& asyncId, double delay, sol::function callback)
         {
-            asyncId.mContainer->setupUnsavableTimer(true, world->getGameTimeInHours() + delay, asyncId.mScript, std::move(callback));
+            asyncId.mContainer->setupUnsavableTimer(
+                TimeUnit::HOURS, world->getGameTimeInHours() + delay, asyncId.mScript, std::move(callback));
         };
 
         auto initializer = [](sol::table hiddenData)

apps/openmw/mwlua/localscripts.cpp

@@ -22,11 +22,11 @@ namespace MWLua
     {
         using ActorControls = MWBase::LuaManager::ActorControls;
         sol::usertype<ActorControls> controls = context.mLua->sol().new_usertype<ActorControls>("ActorControls");
-        controls["movement"] = &ActorControls::movement;
+        controls["movement"] = &ActorControls::mMovement;
-        controls["sideMovement"] = &ActorControls::sideMovement;
+        controls["sideMovement"] = &ActorControls::mSideMovement;
-        controls["turn"] = &ActorControls::turn;
+        controls["turn"] = &ActorControls::mTurn;
-        controls["run"] = &ActorControls::run;
+        controls["run"] = &ActorControls::mRun;
-        controls["jump"] = &ActorControls::jump;
+        controls["jump"] = &ActorControls::mJump;
 
         sol::usertype<SelfObject> selfAPI =
             context.mLua->sol().new_usertype<SelfObject>("SelfObject", sol::base_classes, sol::bases<LObject>());
@@ -34,8 +34,8 @@ namespace MWLua
         selfAPI["object"] = sol::readonly_property([](SelfObject& self) -> LObject { return LObject(self); });
         selfAPI["controls"] = sol::readonly_property([](SelfObject& self) { return &self.mControls; });
         selfAPI["isActive"] = [](SelfObject& self) { return &self.mIsActive; };
-        selfAPI["setDirectControl"] = [](SelfObject& self, bool v) { self.mControls.controlledFromLua = v; };
+        selfAPI["setDirectControl"] = [](SelfObject& self, bool v) { self.mControls.mControlledFromLua = v; };
-        selfAPI["enableAI"] = [](SelfObject& self, bool v) { self.mControls.disableAI = !v; };
+        selfAPI["enableAI"] = [](SelfObject& self, bool v) { self.mControls.mDisableAI = !v; };
         selfAPI["setEquipment"] = [manager=context.mLuaManager](const SelfObject& obj, sol::table equipment)
         {
             if (!obj.ptr().getClass().hasInventoryStore(obj.ptr()))
@@ -79,16 +79,11 @@ namespace MWLua
         };
     }
 
-    std::unique_ptr<LocalScripts> LocalScripts::create(LuaUtil::LuaState* lua, const LObject& obj)
-    {
-        return std::unique_ptr<LocalScripts>(new LocalScripts(lua, obj));
-    }
-
     LocalScripts::LocalScripts(LuaUtil::LuaState* lua, const LObject& obj)
         : LuaUtil::ScriptsContainer(lua, "L" + idToString(obj.id())), mData(obj)
     {
-        mData.mControls.controlledFromLua = false;
+        mData.mControls.mControlledFromLua = false;
-        mData.mControls.disableAI = false;
+        mData.mControls.mDisableAI = false;
         this->addPackage("openmw.self", sol::make_object(lua->sol(), &mData));
         registerEngineHandlers({&mOnActiveHandlers, &mOnInactiveHandlers, &mOnConsumeHandlers});
     }

apps/openmw/mwlua/localscripts.hpp

@@ -19,8 +19,8 @@ namespace MWLua
     class LocalScripts : public LuaUtil::ScriptsContainer
     {
     public:
-        static std::unique_ptr<LocalScripts> create(LuaUtil::LuaState* lua, const LObject& obj);
         static void initializeSelfPackage(const Context&);
+        LocalScripts(LuaUtil::LuaState* lua, const LObject& obj);
 
         MWBase::LuaManager::ActorControls* getActorControls() { return &mData.mControls; }
 
@@ -42,8 +42,8 @@ namespace MWLua
         void receiveEngineEvent(const EngineEvent&, ObjectRegistry*);
 
     protected:
-        LocalScripts(LuaUtil::LuaState* lua, const LObject& obj);
         SelfObject mData;
+
     private:
         EngineHandlerList mOnActiveHandlers{"onActive"};
         EngineHandlerList mOnInactiveHandlers{"onInactive"};

apps/openmw/mwlua/luabindings.cpp

@@ -76,7 +76,7 @@ namespace MWLua
             if (cell)
                 return GCell{cell};
             else
-                return {};
+                return sol::nullopt;
         };
         api["getExteriorCell"] = [worldView=context.mWorldView](int x, int y) -> sol::optional<GCell>
         {
@@ -84,7 +84,7 @@ namespace MWLua
             if (cell)
                 return GCell{cell};
             else
-                return {};
+                return sol::nullopt;
         };
         api["activeActors"] = GObjectList{worldView->getActorsInScene()};
         api["selectObjects"] = [context](const Queries::Query& query)
@@ -156,7 +156,9 @@ namespace MWLua
         for (const Queries::Field* field : group.mFields)
         {
             sol::table subgroup = res;
-            for (int i = 0; i < static_cast<int>(field->path().size()) - 1; ++i)
+            if (field->path().empty())
+                throw std::logic_error("Empty path in Queries::Field");
+            for (size_t i = 0; i < field->path().size() - 1; ++i)
             {
                 const std::string& name = field->path()[i];
                 if (subgroup[name] == sol::nil)

apps/openmw/mwlua/luamanagerimp.cpp

@@ -7,6 +7,7 @@
 #include <components/esm/luascripts.hpp>
 
 #include <components/lua/utilpackage.hpp>
+#include <components/lua/omwscriptsparser.hpp>
 
 #include "../mwbase/windowmanager.hpp"
 
@@ -19,9 +20,10 @@
 namespace MWLua
 {
 
-    LuaManager::LuaManager(const VFS::Manager* vfs, const std::vector<std::string>& globalScriptLists) : mLua(vfs)
+    LuaManager::LuaManager(const VFS::Manager* vfs, const std::vector<std::string>& scriptLists) : mLua(vfs)
     {
         Log(Debug::Info) << "Lua version: " << LuaUtil::getLuaVersion();
+        mGlobalScriptList = LuaUtil::parseOMWScriptsFiles(vfs, scriptLists);
 
         mGlobalSerializer = createUserdataSerializer(false, mWorldView.getObjectRegistry());
         mLocalSerializer = createUserdataSerializer(true, mWorldView.getObjectRegistry());
@@ -58,37 +60,6 @@ namespace MWLua
         mCameraPackage = initCameraPackage(localContext);
         mUserInterfacePackage = initUserInterfacePackage(localContext);
         mNearbyPackage = initNearbyPackage(localContext);
-
-        auto endsWith = [](std::string_view s, std::string_view suffix)
-        {
-            return s.size() >= suffix.size() && std::equal(suffix.rbegin(), suffix.rend(), s.rbegin());
-        };
-        for (const std::string& scriptListFile : globalScriptLists)
-        {
-            if (!endsWith(scriptListFile, ".omwscripts"))
-            {
-                Log(Debug::Error) << "Script list should have suffix '.omwscripts', got: '" << scriptListFile << "'";
-                continue;
-            }
-            std::string content(std::istreambuf_iterator<char>(*vfs->get(scriptListFile)), {});
-            std::string_view view(content);
-            while (!view.empty())
-            {
-                size_t pos = 0;
-                while (pos < view.size() && view[pos] != '\n')
-                    pos++;
-                std::string_view line = view.substr(0, pos);
-                view = view.substr(pos + 1);
-                if (line.empty() || line[0] == '#')
-                    continue;
-                if (line.back() == '\r')
-                    line = line.substr(0, pos - 1);
-                if (endsWith(line, ".lua"))
-                    mGlobalScriptList.push_back(std::string(line));
-                else
-                    Log(Debug::Error) << "Lua script should have suffix '.lua', got: '" << line.substr(0, 300) << "'";
-            }
-        }
     }
 
     void LuaManager::init()
@@ -306,18 +277,18 @@ namespace MWLua
 
     LocalScripts* LuaManager::createLocalScripts(const MWWorld::Ptr& ptr)
     {
-        std::unique_ptr<LocalScripts> scripts;
+        std::shared_ptr<LocalScripts> scripts;
         // When loading a game, it can be called before LuaManager::setPlayer,
         // so we can't just check ptr == mPlayer here.
         if (*ptr.getCellRef().getRefIdPtr() == "player")
         {
             mPlayerScripts = new PlayerScripts(&mLua, LObject(getId(ptr), mWorldView.getObjectRegistry()));
-            scripts = std::unique_ptr<LocalScripts>(mPlayerScripts);
+            scripts = std::shared_ptr<LocalScripts>(mPlayerScripts);
             scripts->addPackage("openmw.ui", mUserInterfacePackage);
             scripts->addPackage("openmw.camera", mCameraPackage);
         }
         else
-            scripts = LocalScripts::create(&mLua, LObject(getId(ptr), mWorldView.getObjectRegistry()));
+            scripts = std::make_shared<LocalScripts>(&mLua, LObject(getId(ptr), mWorldView.getObjectRegistry()));
         scripts->addPackage("openmw.nearby", mNearbyPackage);
         scripts->setSerializer(mLocalSerializer.get());
 

apps/openmw/mwlua/luamanagerimp.hpp

@@ -23,7 +23,6 @@ namespace MWLua
     {
     public:
         LuaManager(const VFS::Manager* vfs, const std::vector<std::string>& globalScriptLists);
-        ~LuaManager() {}
 
         // Called by engine.cpp when environment is fully initialized.
         void init();

apps/openmw/mwlua/objectbindings.cpp

@@ -104,7 +104,7 @@ namespace MWLua
             if (ptr.isInCell())
                 return Cell<ObjectT>{ptr.getCell()};
             else
-                return {};
+                return sol::nullopt;
         });
         objectT["position"] = sol::readonly_property([](const ObjectT& o) -> osg::Vec3f
         {
@@ -123,17 +123,17 @@ namespace MWLua
             context.mLocalEventQueue->push_back({dest.id(), std::move(eventName), LuaUtil::serialize(eventData, context.mSerializer)});
         };
 
-        objectT["canMove"] = [context](const ObjectT& o)
+        objectT["canMove"] = [](const ObjectT& o)
         {
             const MWWorld::Class& cls = o.ptr().getClass();
             return cls.getMaxSpeed(o.ptr()) > 0;
         };
-        objectT["getRunSpeed"] = [context](const ObjectT& o)
+        objectT["getRunSpeed"] = [](const ObjectT& o)
         {
             const MWWorld::Class& cls = o.ptr().getClass();
             return cls.getRunSpeed(o.ptr());
         };
-        objectT["getWalkSpeed"] = [context](const ObjectT& o)
+        objectT["getWalkSpeed"] = [](const ObjectT& o)
         {
             const MWWorld::Class& cls = o.ptr().getClass();
             return cls.getWalkSpeed(o.ptr());
@@ -182,12 +182,12 @@ namespace MWLua
         {
             const MWWorld::CellRef& cellRef = ptr(o).getCellRef();
             if (!cellRef.getTeleport())
-                return {};
+                return sol::nullopt;
             MWWorld::CellStore* cell = worldView->findCell(cellRef.getDestCell(), cellRef.getDoorDest().asVec3());
             if (cell)
                 return Cell<ObjectT>{cell};
             else
-                return {};
+                return sol::nullopt;
         });
     }
 

apps/openmw/mwlua/query.cpp

@@ -84,9 +84,12 @@ namespace MWLua
         }
         if (ptr.getRefData().getCount() == 0)
             return false;
-        // It is stupid, but "prisonmarker" has class "Door" despite that it is only an invisible marker. So we ignore all markers.
+
+        // It is important to exclude all markers before checking what class it is.
+        // For example "prisonmarker" has class "Door" despite that it is only an invisible marker.
         if (isMarker(ptr))
             return false;
+
         const MWWorld::Class& cls = ptr.getClass();
         if (cls.isActivator() != (query.mQueryType == ObjectQueryTypes::ACTIVATORS))
             return false;

apps/openmw/mwmechanics/actors.cpp

@@ -2065,7 +2065,7 @@ namespace MWMechanics
                         if (iter->first != player)
                         {
                             CreatureStats &stats = iter->first.getClass().getCreatureStats(iter->first);
-                            if (isConscious(iter->first) && !(luaControls && luaControls->disableAI))
+                            if (isConscious(iter->first) && !(luaControls && luaControls->mDisableAI))
                             {
                                 stats.getAiSequence().execute(iter->first, *ctrl, duration);
                                 updateGreetingState(iter->first, *iter->second, timerUpdateHello > 0);
@@ -2074,7 +2074,7 @@ namespace MWMechanics
                             }
                         }
                     }
-                    else if (aiActive && iter->first != player && isConscious(iter->first) && !(luaControls && luaControls->disableAI))
+                    else if (aiActive && iter->first != player && isConscious(iter->first) && !(luaControls && luaControls->mDisableAI))
                     {
                         CreatureStats &stats = iter->first.getClass().getCreatureStats(iter->first);
                         stats.getAiSequence().execute(iter->first, *ctrl, duration, /*outOfRange*/true);
@@ -2101,21 +2101,21 @@ namespace MWMechanics
                         float rotationZ = mov.mRotation[2];
                         bool jump = mov.mPosition[2] == 1;
                         bool runFlag = stats.getMovementFlag(MWMechanics::CreatureStats::Flag_Run);
-                        if (luaControls->controlledFromLua)
+                        if (luaControls->mControlledFromLua)
                         {
-                            mov.mPosition[0] = luaControls->sideMovement;
+                            mov.mPosition[0] = luaControls->mSideMovement;
-                            mov.mPosition[1] = luaControls->movement;
+                            mov.mPosition[1] = luaControls->mMovement;
-                            mov.mPosition[2] = luaControls->jump ? 1 : 0;
+                            mov.mPosition[2] = luaControls->mJump ? 1 : 0;
                             mov.mRotation[1] = 0;
-                            mov.mRotation[2] = luaControls->turn;
+                            mov.mRotation[2] = luaControls->mTurn;
-                            mov.mSpeedFactor = osg::Vec2(luaControls->movement, luaControls->sideMovement).length();
+                            mov.mSpeedFactor = osg::Vec2(luaControls->mMovement, luaControls->mSideMovement).length();
-                            stats.setMovementFlag(MWMechanics::CreatureStats::Flag_Run, luaControls->run);
+                            stats.setMovementFlag(MWMechanics::CreatureStats::Flag_Run, luaControls->mRun);
                         }
-                        luaControls->sideMovement = movement.x();
+                        luaControls->mSideMovement = movement.x();
-                        luaControls->movement = movement.y();
+                        luaControls->mMovement = movement.y();
-                        luaControls->turn = rotationZ;
+                        luaControls->mTurn = rotationZ;
-                        luaControls->jump = jump;
+                        luaControls->mJump = jump;
-                        luaControls->run = runFlag;
+                        luaControls->mRun = runFlag;
                     }
                 }
             }

apps/openmw/mwworld/cellref.cpp

@@ -22,8 +22,9 @@ namespace MWWorld
             lastAssignedRefNum.mIndex++;
             if (lastAssignedRefNum.mIndex == 0)  // mIndex overflow, so mContentFile should be changed
             {
-                lastAssignedRefNum.mContentFile--;
+                if (lastAssignedRefNum.mContentFile > std::numeric_limits<int32_t>::min())
-                if (lastAssignedRefNum.mContentFile > 0)
+                    lastAssignedRefNum.mContentFile--;
+                else
                     Log(Debug::Error) << "RefNum counter overflow in CellRef::getOrAssignRefNum";
             }
             mCellRef.mRefNum = lastAssignedRefNum;

apps/openmw/mwworld/refdata.cpp

@@ -23,7 +23,7 @@ enum RefDataFlags
 namespace MWWorld
 {
 
-    void RefData::setLuaScripts(std::unique_ptr<MWLua::LocalScripts>&& scripts)
+    void RefData::setLuaScripts(std::shared_ptr<MWLua::LocalScripts>&& scripts)
     {
         mChanged = true;
         mLuaScripts = std::move(scripts);

apps/openmw/mwworld/refdata.hpp

@@ -103,7 +103,7 @@ namespace MWWorld
             void setLocals (const ESM::Script& script);
 
             MWLua::LocalScripts* getLuaScripts() { return mLuaScripts.get(); }
-            void setLuaScripts(std::unique_ptr<MWLua::LocalScripts>&&);
+            void setLuaScripts(std::shared_ptr<MWLua::LocalScripts>&&);
 
             void setCount (int count);
             ///< Set object count (an object pile is a simple object with a count >1).

apps/openmw_test_suite/CMakeLists.txt

@@ -20,6 +20,7 @@ if (GTEST_FOUND AND GMOCK_FOUND)
         lua/test_utilpackage.cpp
         lua/test_serialization.cpp
         lua/test_querypackage.cpp
+        lua/test_omwscriptsparser.cpp
 
         misc/test_stringops.cpp
         misc/test_endianness.cpp

apps/openmw_test_suite/lua/test_omwscriptsparser.cpp

@@ -0,0 +1,59 @@
+#include "gmock/gmock.h"
+#include <gtest/gtest.h>
+
+#include <components/lua/omwscriptsparser.hpp>
+
+#include "testing_util.hpp"
+
+namespace
+{
+    using namespace testing;
+
+    TestFile file1(
+        "#comment.lua\n"
+        "\n"
+        "script1.lua\n"
+        "some mod/Some Script.lua"
+    );
+    TestFile file2(
+        "#comment.lua\r\n"
+        "\r\n"
+        "script2.lua\r\n"
+        "some other mod/Some Script.lua\r"
+    );
+    TestFile emptyFile("");
+    TestFile invalidFile("Invalid file");
+
+    struct OMWScriptsParserTest : Test
+    {
+        std::unique_ptr<VFS::Manager> mVFS = createTestVFS({
+            {"file1.omwscripts", &file1},
+            {"file2.omwscripts", &file2},
+            {"empty.omwscripts", &emptyFile},
+            {"invalid.lua", &file1},
+            {"invalid.omwscripts", &invalidFile},
+        });
+    };
+
+    TEST_F(OMWScriptsParserTest, Basic)
+    {
+        internal::CaptureStdout();
+        std::vector<std::string> res = LuaUtil::parseOMWScriptsFiles(
+            mVFS.get(), {"file2.omwscripts", "empty.omwscripts", "file1.omwscripts"});
+        EXPECT_EQ(internal::GetCapturedStdout(), "");
+        EXPECT_THAT(res, ElementsAre("script2.lua", "some other mod/Some Script.lua",
+                                     "script1.lua", "some mod/Some Script.lua"));
+    }
+
+    TEST_F(OMWScriptsParserTest, InvalidFiles)
+    {
+        internal::CaptureStdout();
+        std::vector<std::string> res = LuaUtil::parseOMWScriptsFiles(
+            mVFS.get(), {"invalid.lua", "invalid.omwscripts"});
+        EXPECT_EQ(internal::GetCapturedStdout(),
+                  "Script list should have suffix '.omwscripts', got: 'invalid.lua'\n"
+                  "Lua script should have suffix '.lua', got: 'Invalid file'\n");
+        EXPECT_THAT(res, ElementsAre());
+    }
+
+}

apps/openmw_test_suite/lua/test_scriptscontainer.cpp

@@ -304,6 +304,7 @@ return {
 
     TEST_F(LuaScriptsContainerTest, Timers)
     {
+        using TimeUnit = LuaUtil::ScriptsContainer::TimeUnit;
         LuaUtil::ScriptsContainer scripts(&mLua, "Test");
         EXPECT_TRUE(scripts.addNewScript("test1.lua"));
         EXPECT_TRUE(scripts.addNewScript("test2.lua"));
@@ -321,18 +322,18 @@ return {
 
         scripts.processTimers(1, 2);
 
-        scripts.setupSerializableTimer(false, 10, "test1.lua", "B", sol::make_object(mLua.sol(), 3));
+        scripts.setupSerializableTimer(TimeUnit::SECONDS, 10, "test1.lua", "B", sol::make_object(mLua.sol(), 3));
-        scripts.setupSerializableTimer(true, 10, "test2.lua", "B", sol::make_object(mLua.sol(), 4));
+        scripts.setupSerializableTimer(TimeUnit::HOURS, 10, "test2.lua", "B", sol::make_object(mLua.sol(), 4));
-        scripts.setupSerializableTimer(false, 5, "test1.lua", "A", sol::make_object(mLua.sol(), 1));
+        scripts.setupSerializableTimer(TimeUnit::SECONDS, 5, "test1.lua", "A", sol::make_object(mLua.sol(), 1));
-        scripts.setupSerializableTimer(true, 5, "test2.lua", "A", sol::make_object(mLua.sol(), 2));
+        scripts.setupSerializableTimer(TimeUnit::HOURS, 5, "test2.lua", "A", sol::make_object(mLua.sol(), 2));
-        scripts.setupSerializableTimer(false, 15, "test1.lua", "A", sol::make_object(mLua.sol(), 10));
+        scripts.setupSerializableTimer(TimeUnit::SECONDS, 15, "test1.lua", "A", sol::make_object(mLua.sol(), 10));
-        scripts.setupSerializableTimer(false, 15, "test1.lua", "B", sol::make_object(mLua.sol(), 20));
+        scripts.setupSerializableTimer(TimeUnit::SECONDS, 15, "test1.lua", "B", sol::make_object(mLua.sol(), 20));
 
-        scripts.setupUnsavableTimer(false, 10, "test2.lua", fn2);
+        scripts.setupUnsavableTimer(TimeUnit::SECONDS, 10, "test2.lua", fn2);
-        scripts.setupUnsavableTimer(true, 10, "test1.lua", fn2);
+        scripts.setupUnsavableTimer(TimeUnit::HOURS, 10, "test1.lua", fn2);
-        scripts.setupUnsavableTimer(false, 5, "test2.lua", fn1);
+        scripts.setupUnsavableTimer(TimeUnit::SECONDS, 5, "test2.lua", fn1);
-        scripts.setupUnsavableTimer(true, 5, "test1.lua", fn1);
+        scripts.setupUnsavableTimer(TimeUnit::HOURS, 5, "test1.lua", fn1);
-        scripts.setupUnsavableTimer(false, 15, "test2.lua", fn1);
+        scripts.setupUnsavableTimer(TimeUnit::SECONDS, 15, "test2.lua", fn1);
 
         EXPECT_EQ(counter1, 0);
         EXPECT_EQ(counter3, 0);

components/CMakeLists.txt

@@ -29,7 +29,7 @@ endif (GIT_CHECKOUT)
 # source files
 
 add_component_dir (lua
-    luastate scriptscontainer utilpackage serialization
+    luastate scriptscontainer utilpackage serialization omwscriptsparser
     )
 
 add_component_dir (settings

components/esm/luascripts.cpp

@@ -48,7 +48,7 @@ void ESM::LuaScripts::load(ESMReader& esm)
         {
             esm.getSubHeader();
             LuaTimer timer;
-            esm.getT(timer.mHours);
+            esm.getT(timer.mUnit);
             esm.getT(timer.mTime);
             timer.mCallbackName = esm.getHNString("LUAC");
             timer.mCallbackArgument = loadLuaBinaryData(esm);
@@ -68,7 +68,7 @@ void ESM::LuaScripts::save(ESMWriter& esm) const
         for (const LuaTimer& timer : script.mTimers)
         {
             esm.startSubRecord("LUAT");
-            esm.writeT(timer.mHours);
+            esm.writeT(timer.mUnit);
             esm.writeT(timer.mTime);
             esm.endRecord("LUAT");
             esm.writeHNString("LUAC", timer.mCallbackName);

components/esm/luascripts.hpp

@@ -14,8 +14,14 @@ namespace ESM
 
     struct LuaTimer
     {
+        enum class TimeUnit : bool
+        {
+            SECONDS = 0,
+            HOURS = 1,
+        };
+
+        TimeUnit mUnit;
         double mTime;
-        bool mHours;  // false - game seconds, true - game hours
         std::string mCallbackName;
         std::string mCallbackArgument;  // Serialized Lua table. It is a binary data. Can contain '\0'.
     };

components/lua/luastate.cpp

@@ -12,9 +12,7 @@ namespace LuaUtil
     static std::string packageNameToPath(std::string_view packageName)
     {
         std::string res(packageName);
-        for (size_t i = 0; i < res.size(); ++i)
+        std::replace(res.begin(), res.end(), '.', '/');
-            if (res[i] == '.')
-                res[i] = '/';
         res.append(".lua");
         return res;
     }
@@ -28,7 +26,7 @@ namespace LuaUtil
     {
         mLua.open_libraries(sol::lib::base, sol::lib::coroutine, sol::lib::math, sol::lib::string, sol::lib::table);
 
-        mLua["math"]["randomseed"](static_cast<unsigned>(time(NULL)));
+        mLua["math"]["randomseed"](static_cast<unsigned>(std::time(nullptr)));
         mLua["math"]["randomseed"] = sol::nil;
 
         mLua["writeToLog"] = [](std::string_view s) { Log(Debug::Level::Info) << s; };

components/lua/omwscriptsparser.cpp

@@ -0,0 +1,44 @@
+#include "omwscriptsparser.hpp"
+
+#include <algorithm>
+
+#include <components/debug/debuglog.hpp>
+
+std::vector<std::string> LuaUtil::parseOMWScriptsFiles(const VFS::Manager* vfs, const std::vector<std::string>& scriptLists)
+{
+    auto endsWith = [](std::string_view s, std::string_view suffix)
+    {
+        return s.size() >= suffix.size() && std::equal(suffix.rbegin(), suffix.rend(), s.rbegin());
+    };
+    std::vector<std::string> res;
+    for (const std::string& scriptListFile : scriptLists)
+    {
+        if (!endsWith(scriptListFile, ".omwscripts"))
+        {
+            Log(Debug::Error) << "Script list should have suffix '.omwscripts', got: '" << scriptListFile << "'";
+            continue;
+        }
+        std::string content(std::istreambuf_iterator<char>(*vfs->get(scriptListFile)), {});
+        std::string_view view(content);
+        while (!view.empty())
+        {
+            size_t pos = 0;
+            while (pos < view.size() && view[pos] != '\n')
+                pos++;
+            std::string_view line = view.substr(0, pos);
+            view = view.substr(std::min(pos + 1, view.size()));
+            if (!line.empty() && line.back() == '\r')
+                line = line.substr(0, pos - 1);
+            // Lines starting with '#' are comments.
+            // TODO: Maybe make the parser more robust. It is a bit inconsistent that 'path/#to/file.lua'
+            //       is a valid path, but '#path/to/file.lua' is considered as a comment and ignored.
+            if (line.empty() || line[0] == '#')
+                continue;
+            if (endsWith(line, ".lua"))
+                res.push_back(std::string(line));
+            else
+                Log(Debug::Error) << "Lua script should have suffix '.lua', got: '" << line.substr(0, 300) << "'";
+        }
+    }
+    return res;
+}

components/lua/omwscriptsparser.hpp

@@ -0,0 +1,14 @@
+#ifndef COMPONENTS_LUA_OMWSCRIPTSPARSER_H
+#define COMPONENTS_LUA_OMWSCRIPTSPARSER_H
+
+#include <components/vfs/manager.hpp>
+
+namespace LuaUtil
+{
+
+    // Parses list of `*.omwscripts` files.
+    std::vector<std::string> parseOMWScriptsFiles(const VFS::Manager* vfs, const std::vector<std::string>& scriptLists);
+
+}
+
+#endif // COMPONENTS_LUA_OMWSCRIPTSPARSER_H

components/lua/scriptscontainer.cpp

@@ -78,10 +78,10 @@ namespace LuaUtil
 
     bool ScriptsContainer::removeScript(const std::string& path)
     {
-        auto it = mScripts.find(path);
+        auto scriptIter = mScripts.find(path);
-        if (it == mScripts.end())
+        if (scriptIter == mScripts.end())
             return false;  // no such script
-        sol::object& script = it->second.mInterface;
+        sol::object& script = scriptIter->second.mInterface;
         if (getFieldOrNil(script, INTERFACE_NAME) != sol::nil)
         {
             std::string_view interfaceName = getFieldOrNil(script, INTERFACE_NAME).as<std::string_view>();
@@ -110,10 +110,10 @@ namespace LuaUtil
             for (auto& [key, value] : sol::table(engineHandlers))
             {
                 std::string_view handlerName = key.as<std::string_view>();
-                auto it = mEngineHandlers.find(handlerName);
+                auto handlerIter = mEngineHandlers.find(handlerName);
-                if (it == mEngineHandlers.end())
+                if (handlerIter == mEngineHandlers.end())
                     continue;
-                std::vector<sol::protected_function>& list = it->second->mList;
+                std::vector<sol::protected_function>& list = handlerIter->second->mList;
                 list.erase(std::find(list.begin(), list.end(), value.as<sol::protected_function>()));
             }
         }
@@ -126,7 +126,7 @@ namespace LuaUtil
                 list.erase(std::find(list.begin(), list.end(), value.as<sol::protected_function>()));
             }
         }
-        mScripts.erase(it);
+        mScripts.erase(scriptIter);
         mScriptOrder.erase(std::find(mScriptOrder.begin(), mScriptOrder.end(), path));
         return true;
     }
@@ -201,13 +201,13 @@ namespace LuaUtil
     void ScriptsContainer::save(ESM::LuaScripts& data)
     {
         std::map<std::string, std::vector<ESM::LuaTimer>> timers;
-        auto saveTimerFn = [&](const Timer& timer, bool inHours)
+        auto saveTimerFn = [&](const Timer& timer, TimeUnit timeUnit)
         {
             if (!timer.mSerializable)
                 return;
             ESM::LuaTimer savedTimer;
             savedTimer.mTime = timer.mTime;
-            savedTimer.mHours = inHours;
+            savedTimer.mUnit = timeUnit;
             savedTimer.mCallbackName = std::get<std::string>(timer.mCallback);
             savedTimer.mCallbackArgument = timer.mSerializedArg;
             if (timers.count(timer.mScript) == 0)
@@ -215,9 +215,9 @@ namespace LuaUtil
             timers[timer.mScript].push_back(std::move(savedTimer));
         };
         for (const Timer& timer : mSecondsTimersQueue)
-            saveTimerFn(timer, false);
+            saveTimerFn(timer, TimeUnit::SECONDS);
         for (const Timer& timer : mHoursTimersQueue)
-            saveTimerFn(timer, true);
+            saveTimerFn(timer, TimeUnit::HOURS);
         data.mScripts.clear();
         for (const std::string& path : mScriptOrder)
         {
@@ -291,7 +291,7 @@ namespace LuaUtil
                     // updates refnums, so timer.mSerializedArg may be not equal to savedTimer.mCallbackArgument.
                     timer.mSerializedArg = serialize(timer.mArg, mSerializer);
 
-                    if (savedTimer.mHours)
+                    if (savedTimer.mUnit == TimeUnit::HOURS)
                         mHoursTimersQueue.push_back(std::move(timer));
                     else
                         mSecondsTimersQueue.push_back(std::move(timer));
@@ -352,7 +352,7 @@ namespace LuaUtil
         std::push_heap(timerQueue.begin(), timerQueue.end());
     }
 
-    void ScriptsContainer::setupSerializableTimer(bool inHours, double time, const std::string& scriptPath,
+    void ScriptsContainer::setupSerializableTimer(TimeUnit timeUnit, double time, const std::string& scriptPath,
                                                   std::string_view callbackName, sol::object callbackArg)
     {
         Timer t;
@@ -362,10 +362,10 @@ namespace LuaUtil
         t.mTime = time;
         t.mArg = callbackArg;
         t.mSerializedArg = serialize(t.mArg, mSerializer);
-        insertTimer(inHours ? mHoursTimersQueue : mSecondsTimersQueue, std::move(t));
+        insertTimer(timeUnit == TimeUnit::HOURS ? mHoursTimersQueue : mSecondsTimersQueue, std::move(t));
     }
 
-    void ScriptsContainer::setupUnsavableTimer(bool inHours, double time, const std::string& scriptPath, sol::function callback)
+    void ScriptsContainer::setupUnsavableTimer(TimeUnit timeUnit, double time, const std::string& scriptPath, sol::function callback)
     {
         Timer t;
         t.mScript = scriptPath;
@@ -376,7 +376,7 @@ namespace LuaUtil
         getHiddenData(scriptPath)[TEMPORARY_TIMER_CALLBACKS][mTemporaryCallbackCounter] = std::move(callback);
         mTemporaryCallbackCounter++;
 
-        insertTimer(inHours ? mHoursTimersQueue : mSecondsTimersQueue, std::move(t));
+        insertTimer(timeUnit == TimeUnit::HOURS ? mHoursTimersQueue : mSecondsTimersQueue, std::move(t));
     }
 
     void ScriptsContainer::callTimer(const Timer& t)

components/lua/scriptscontainer.hpp

@@ -67,6 +67,7 @@ namespace LuaUtil
             ScriptsContainer* mContainer;
             std::string mPath;
         };
+        using TimeUnit = ESM::LuaTimer::TimeUnit;
 
         // `namePrefix` is a common prefix for all scripts in the container. Used in logs for error messages and `print` output.
         ScriptsContainer(LuaUtil::LuaState* lua, std::string_view namePrefix);
@@ -122,17 +123,17 @@ namespace LuaUtil
         void registerTimerCallback(const std::string& scriptPath, std::string_view callbackName, sol::function callback);
 
         // Sets up a timer, that can be automatically saved and loaded.
-        //   inHours - false if time unit is game seconds and true if time unit if game hours.
+        //   timeUnit - game seconds (TimeUnit::Seconds) or game hours (TimeUnit::Hours).
         //   time - the absolute game time (in seconds or in hours) when the timer should be executed.
         //   scriptPath - script path in VFS is used as script id. The script with the given path should already present in the container.
         //   callbackName - callback (should be registered in advance) for this timer.
         //   callbackArg - parameter for the callback (should be serializable).
-        void setupSerializableTimer(bool inHours, double time, const std::string& scriptPath,
+        void setupSerializableTimer(TimeUnit timeUnit, double time, const std::string& scriptPath,
                                     std::string_view callbackName, sol::object callbackArg);
 
         // Creates a timer. `callback` is an arbitrary Lua function. This type of timers is called "unsavable"
         // because it can not be stored in saves. I.e. loading a saved game will not fully restore the state.
-        void setupUnsavableTimer(bool inHours, double time, const std::string& scriptPath, sol::function callback);
+        void setupUnsavableTimer(TimeUnit timeUnit, double time, const std::string& scriptPath, sol::function callback);
 
     protected:
         struct EngineHandlerList

components/lua/serialization.cpp

@@ -10,7 +10,7 @@ namespace LuaUtil
 
     constexpr unsigned char FORMAT_VERSION = 0;
 
-    enum class SerializedType
+    enum class SerializedType : char
     {
         NUMBER =       0x0,
         LONG_STRING =  0x1,
@@ -20,8 +20,10 @@ namespace LuaUtil
 
         VEC2 =         0x10,
         VEC3 =         0x11,
+
+        // All values should be lesser than 0x20 (SHORT_STRING_FLAG).
     };
-    constexpr unsigned char SHORT_STRING_FLAG = 0x20;    // 0x001SSSSS. SSSSS = string length
+    constexpr unsigned char SHORT_STRING_FLAG = 0x20;    // 0b001SSSSS. SSSSS = string length
     constexpr unsigned char CUSTOM_FULL_FLAG = 0x40;     // 0b01TTTTTT + 32bit dataSize
     constexpr unsigned char CUSTOM_COMPACT_FLAG = 0x80;  // 0b1SSSSTTT. SSSS = dataSize, TTT = (typeName size - 1)
 
@@ -224,8 +226,8 @@ namespace LuaUtil
                 sol::stack::push<osg::Vec3f>(lua.lua_state(), osg::Vec3f(x, y, z));
                 return;
             }
-            default: throw std::runtime_error("Unknown type: " + std::to_string(type));
         }
+        throw std::runtime_error("Unknown type: " + std::to_string(type));
     }
 
     BinaryData serialize(const sol::object& obj, const UserdataSerializer* customSerializer)