From 702eb19271fd8f9bb011f8caef887cd44a84e746 Mon Sep 17 00:00:00 2001 From: Petr Mikheev Date: Fri, 23 Apr 2021 02:49:12 +0200 Subject: [PATCH] Fixes and refactoring --- apps/openmw/engine.cpp | 21 ++++--- apps/openmw/mwbase/luamanager.hpp | 19 +++--- apps/openmw/mwlua/actions.cpp | 19 ++---- apps/openmw/mwlua/asyncbindings.cpp | 13 ++-- apps/openmw/mwlua/localscripts.cpp | 23 +++----- apps/openmw/mwlua/localscripts.hpp | 4 +- apps/openmw/mwlua/luabindings.cpp | 8 ++- apps/openmw/mwlua/luamanagerimp.cpp | 41 ++----------- apps/openmw/mwlua/luamanagerimp.hpp | 1 - apps/openmw/mwlua/objectbindings.cpp | 12 ++-- apps/openmw/mwlua/query.cpp | 5 +- apps/openmw/mwmechanics/actors.cpp | 28 ++++----- apps/openmw/mwworld/cellref.cpp | 5 +- apps/openmw/mwworld/refdata.cpp | 2 +- apps/openmw/mwworld/refdata.hpp | 2 +- apps/openmw_test_suite/CMakeLists.txt | 1 + .../lua/test_omwscriptsparser.cpp | 59 +++++++++++++++++++ .../lua/test_scriptscontainer.cpp | 25 ++++---- components/CMakeLists.txt | 2 +- components/esm/luascripts.cpp | 4 +- components/esm/luascripts.hpp | 8 ++- components/lua/luastate.cpp | 6 +- components/lua/omwscriptsparser.cpp | 44 ++++++++++++++ components/lua/omwscriptsparser.hpp | 14 +++++ components/lua/scriptscontainer.cpp | 32 +++++----- components/lua/scriptscontainer.hpp | 7 ++- components/lua/serialization.cpp | 8 ++- 27 files changed, 254 insertions(+), 159 deletions(-) create mode 100644 apps/openmw_test_suite/lua/test_omwscriptsparser.cpp create mode 100644 components/lua/omwscriptsparser.cpp create mode 100644 components/lua/omwscriptsparser.hpp diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp index 71d169c0bc..1bd79cdd02 100644 --- a/apps/openmw/engine.cpp +++ b/apps/openmw/engine.cpp @@ -833,18 +833,17 @@ void OMW::Engine::prepareEngine (Settings::Manager & settings) class OMW::Engine::LuaWorker { public: - explicit LuaWorker(Engine* engine) : - mEngine(engine), - mSeparateThread(Settings::Manager::getInt("lua num threads", "Lua") > 0) + explicit LuaWorker(Engine* engine) : mEngine(engine) { - 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 lk(mMutex); @@ -855,7 +854,7 @@ public: void finishUpdate() { - if (mSeparateThread) + if (mThread) { std::unique_lock lk(mMutex); mCV.wait(lk, [&]{ return !mUpdateRequest; }); @@ -867,8 +866,8 @@ public: void join() { - if (mSeparateThread) - mThread.join(); + if (mThread) + mThread->join(); } private: @@ -879,7 +878,7 @@ private: const unsigned int frameNumber = viewer->getFrameStamp()->getFrameNumber(); ScopedProfile 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 mThread; }; // Initialise and enter main loop. diff --git a/apps/openmw/mwbase/luamanager.hpp b/apps/openmw/mwbase/luamanager.hpp index 24c1780984..4e437246c4 100644 --- a/apps/openmw/mwbase/luamanager.hpp +++ b/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 { - bool disableAI; - bool controlledFromLua; + struct ActorControls + { + bool mDisableAI; + bool mControlledFromLua; - bool jump; - bool run; - float movement; - float sideMovement; - float turn; + bool mJump; + bool mRun; + float mMovement; + float mSideMovement; + float mTurn; }; virtual ActorControls* getActorControls(const MWWorld::Ptr&) const = 0; diff --git a/apps/openmw/mwlua/actions.cpp b/apps/openmw/mwlua/actions.cpp index 00707b3d31..1664501f4f 100644 --- a/apps/openmw/mwlua/actions.cpp +++ b/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; - for (int allowedSlot : allowedSlots) - requestedSlotIsAllowed = requestedSlotIsAllowed || allowedSlot == slot; + bool requestedSlotIsAllowed = std::find(allowedSlots.begin(), allowedSlots.end(), slot) != allowedSlots.end(); if (!requestedSlotIsAllowed) { - slot = anySlot; - for (int allowedSlot : allowedSlots) - if (!usedSlots[allowedSlot]) - { - slot = allowedSlot; - break; - } - if (slot == anySlot) + 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 " << 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); } diff --git a/apps/openmw/mwlua/asyncbindings.cpp b/apps/openmw/mwlua/asyncbindings.cpp index 83c1dbd81b..fee6788b89 100644 --- a/apps/openmw/mwlua/asyncbindings.cpp +++ b/apps/openmw/mwlua/asyncbindings.cpp @@ -17,6 +17,7 @@ namespace MWLua sol::function getAsyncPackageInitializer(const Context& context) { + using TimeUnit = LuaUtil::ScriptsContainer::TimeUnit; sol::usertype api = context.mLua->sol().new_usertype("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) diff --git a/apps/openmw/mwlua/localscripts.cpp b/apps/openmw/mwlua/localscripts.cpp index e4c50334d4..d9bb5ff26e 100644 --- a/apps/openmw/mwlua/localscripts.cpp +++ b/apps/openmw/mwlua/localscripts.cpp @@ -22,11 +22,11 @@ namespace MWLua { using ActorControls = MWBase::LuaManager::ActorControls; sol::usertype controls = context.mLua->sol().new_usertype("ActorControls"); - controls["movement"] = &ActorControls::movement; - controls["sideMovement"] = &ActorControls::sideMovement; - controls["turn"] = &ActorControls::turn; - controls["run"] = &ActorControls::run; - controls["jump"] = &ActorControls::jump; + controls["movement"] = &ActorControls::mMovement; + controls["sideMovement"] = &ActorControls::mSideMovement; + controls["turn"] = &ActorControls::mTurn; + controls["run"] = &ActorControls::mRun; + controls["jump"] = &ActorControls::mJump; sol::usertype selfAPI = context.mLua->sol().new_usertype("SelfObject", sol::base_classes, sol::bases()); @@ -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["enableAI"] = [](SelfObject& self, bool v) { self.mControls.disableAI = !v; }; + selfAPI["setDirectControl"] = [](SelfObject& self, bool v) { self.mControls.mControlledFromLua = 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::create(LuaUtil::LuaState* lua, const LObject& obj) - { - return std::unique_ptr(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.disableAI = false; + mData.mControls.mControlledFromLua = false; + mData.mControls.mDisableAI = false; this->addPackage("openmw.self", sol::make_object(lua->sol(), &mData)); registerEngineHandlers({&mOnActiveHandlers, &mOnInactiveHandlers, &mOnConsumeHandlers}); } diff --git a/apps/openmw/mwlua/localscripts.hpp b/apps/openmw/mwlua/localscripts.hpp index 49612be441..80d04b7a40 100644 --- a/apps/openmw/mwlua/localscripts.hpp +++ b/apps/openmw/mwlua/localscripts.hpp @@ -19,8 +19,8 @@ namespace MWLua class LocalScripts : public LuaUtil::ScriptsContainer { public: - static std::unique_ptr 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"}; diff --git a/apps/openmw/mwlua/luabindings.cpp b/apps/openmw/mwlua/luabindings.cpp index 5ed5378c0a..f48f78fe38 100644 --- a/apps/openmw/mwlua/luabindings.cpp +++ b/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 { @@ -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(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) diff --git a/apps/openmw/mwlua/luamanagerimp.cpp b/apps/openmw/mwlua/luamanagerimp.cpp index bd07c191f9..3c67be5418 100644 --- a/apps/openmw/mwlua/luamanagerimp.cpp +++ b/apps/openmw/mwlua/luamanagerimp.cpp @@ -7,6 +7,7 @@ #include #include +#include #include "../mwbase/windowmanager.hpp" @@ -19,9 +20,10 @@ namespace MWLua { - LuaManager::LuaManager(const VFS::Manager* vfs, const std::vector& globalScriptLists) : mLua(vfs) + LuaManager::LuaManager(const VFS::Manager* vfs, const std::vector& 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(*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 scripts; + std::shared_ptr 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(mPlayerScripts); + scripts = std::shared_ptr(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(&mLua, LObject(getId(ptr), mWorldView.getObjectRegistry())); scripts->addPackage("openmw.nearby", mNearbyPackage); scripts->setSerializer(mLocalSerializer.get()); diff --git a/apps/openmw/mwlua/luamanagerimp.hpp b/apps/openmw/mwlua/luamanagerimp.hpp index 5cfadebc40..be7174a35a 100644 --- a/apps/openmw/mwlua/luamanagerimp.hpp +++ b/apps/openmw/mwlua/luamanagerimp.hpp @@ -23,7 +23,6 @@ namespace MWLua { public: LuaManager(const VFS::Manager* vfs, const std::vector& globalScriptLists); - ~LuaManager() {} // Called by engine.cpp when environment is fully initialized. void init(); diff --git a/apps/openmw/mwlua/objectbindings.cpp b/apps/openmw/mwlua/objectbindings.cpp index 64ac84e4f0..d4ae041549 100644 --- a/apps/openmw/mwlua/objectbindings.cpp +++ b/apps/openmw/mwlua/objectbindings.cpp @@ -104,7 +104,7 @@ namespace MWLua if (ptr.isInCell()) return Cell{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{cell}; else - return {}; + return sol::nullopt; }); } diff --git a/apps/openmw/mwlua/query.cpp b/apps/openmw/mwlua/query.cpp index f42c7ae2d4..c357e56884 100644 --- a/apps/openmw/mwlua/query.cpp +++ b/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; diff --git a/apps/openmw/mwmechanics/actors.cpp b/apps/openmw/mwmechanics/actors.cpp index 97d95251a4..844b2b6987 100644 --- a/apps/openmw/mwmechanics/actors.cpp +++ b/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[1] = luaControls->movement; - mov.mPosition[2] = luaControls->jump ? 1 : 0; + mov.mPosition[0] = luaControls->mSideMovement; + mov.mPosition[1] = luaControls->mMovement; + mov.mPosition[2] = luaControls->mJump ? 1 : 0; mov.mRotation[1] = 0; - mov.mRotation[2] = luaControls->turn; - mov.mSpeedFactor = osg::Vec2(luaControls->movement, luaControls->sideMovement).length(); - stats.setMovementFlag(MWMechanics::CreatureStats::Flag_Run, luaControls->run); + mov.mRotation[2] = luaControls->mTurn; + mov.mSpeedFactor = osg::Vec2(luaControls->mMovement, luaControls->mSideMovement).length(); + stats.setMovementFlag(MWMechanics::CreatureStats::Flag_Run, luaControls->mRun); } - luaControls->sideMovement = movement.x(); - luaControls->movement = movement.y(); - luaControls->turn = rotationZ; - luaControls->jump = jump; - luaControls->run = runFlag; + luaControls->mSideMovement = movement.x(); + luaControls->mMovement = movement.y(); + luaControls->mTurn = rotationZ; + luaControls->mJump = jump; + luaControls->mRun = runFlag; } } } diff --git a/apps/openmw/mwworld/cellref.cpp b/apps/openmw/mwworld/cellref.cpp index 88c38736e8..5e91ddcc67 100644 --- a/apps/openmw/mwworld/cellref.cpp +++ b/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 > 0) + if (lastAssignedRefNum.mContentFile > std::numeric_limits::min()) + lastAssignedRefNum.mContentFile--; + else Log(Debug::Error) << "RefNum counter overflow in CellRef::getOrAssignRefNum"; } mCellRef.mRefNum = lastAssignedRefNum; diff --git a/apps/openmw/mwworld/refdata.cpp b/apps/openmw/mwworld/refdata.cpp index b506671086..7398aef77e 100644 --- a/apps/openmw/mwworld/refdata.cpp +++ b/apps/openmw/mwworld/refdata.cpp @@ -23,7 +23,7 @@ enum RefDataFlags namespace MWWorld { - void RefData::setLuaScripts(std::unique_ptr&& scripts) + void RefData::setLuaScripts(std::shared_ptr&& scripts) { mChanged = true; mLuaScripts = std::move(scripts); diff --git a/apps/openmw/mwworld/refdata.hpp b/apps/openmw/mwworld/refdata.hpp index 3ae6a03293..d90224b9bb 100644 --- a/apps/openmw/mwworld/refdata.hpp +++ b/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&&); + void setLuaScripts(std::shared_ptr&&); void setCount (int count); ///< Set object count (an object pile is a simple object with a count >1). diff --git a/apps/openmw_test_suite/CMakeLists.txt b/apps/openmw_test_suite/CMakeLists.txt index bd8b032378..7800efac3d 100644 --- a/apps/openmw_test_suite/CMakeLists.txt +++ b/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 diff --git a/apps/openmw_test_suite/lua/test_omwscriptsparser.cpp b/apps/openmw_test_suite/lua/test_omwscriptsparser.cpp new file mode 100644 index 0000000000..b1526ef9b6 --- /dev/null +++ b/apps/openmw_test_suite/lua/test_omwscriptsparser.cpp @@ -0,0 +1,59 @@ +#include "gmock/gmock.h" +#include + +#include + +#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 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 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 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()); + } + +} diff --git a/apps/openmw_test_suite/lua/test_scriptscontainer.cpp b/apps/openmw_test_suite/lua/test_scriptscontainer.cpp index 072ad334e8..8f05138782 100644 --- a/apps/openmw_test_suite/lua/test_scriptscontainer.cpp +++ b/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(true, 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(true, 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(false, 15, "test1.lua", "B", sol::make_object(mLua.sol(), 20)); - - scripts.setupUnsavableTimer(false, 10, "test2.lua", fn2); - scripts.setupUnsavableTimer(true, 10, "test1.lua", fn2); - scripts.setupUnsavableTimer(false, 5, "test2.lua", fn1); - scripts.setupUnsavableTimer(true, 5, "test1.lua", fn1); - scripts.setupUnsavableTimer(false, 15, "test2.lua", fn1); + scripts.setupSerializableTimer(TimeUnit::SECONDS, 10, "test1.lua", "B", sol::make_object(mLua.sol(), 3)); + scripts.setupSerializableTimer(TimeUnit::HOURS, 10, "test2.lua", "B", sol::make_object(mLua.sol(), 4)); + scripts.setupSerializableTimer(TimeUnit::SECONDS, 5, "test1.lua", "A", sol::make_object(mLua.sol(), 1)); + scripts.setupSerializableTimer(TimeUnit::HOURS, 5, "test2.lua", "A", sol::make_object(mLua.sol(), 2)); + scripts.setupSerializableTimer(TimeUnit::SECONDS, 15, "test1.lua", "A", sol::make_object(mLua.sol(), 10)); + scripts.setupSerializableTimer(TimeUnit::SECONDS, 15, "test1.lua", "B", sol::make_object(mLua.sol(), 20)); + + scripts.setupUnsavableTimer(TimeUnit::SECONDS, 10, "test2.lua", fn2); + scripts.setupUnsavableTimer(TimeUnit::HOURS, 10, "test1.lua", fn2); + scripts.setupUnsavableTimer(TimeUnit::SECONDS, 5, "test2.lua", fn1); + scripts.setupUnsavableTimer(TimeUnit::HOURS, 5, "test1.lua", fn1); + scripts.setupUnsavableTimer(TimeUnit::SECONDS, 15, "test2.lua", fn1); EXPECT_EQ(counter1, 0); EXPECT_EQ(counter3, 0); diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt index 7629c9798f..9d72320ca9 100644 --- a/components/CMakeLists.txt +++ b/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 diff --git a/components/esm/luascripts.cpp b/components/esm/luascripts.cpp index 41d25ee8be..1dd45ab2b1 100644 --- a/components/esm/luascripts.cpp +++ b/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); diff --git a/components/esm/luascripts.hpp b/components/esm/luascripts.hpp index ef3553e2fb..f268f41536 100644 --- a/components/esm/luascripts.hpp +++ b/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'. }; diff --git a/components/lua/luastate.cpp b/components/lua/luastate.cpp index 4020c815d0..25fa3aead1 100644 --- a/components/lua/luastate.cpp +++ b/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) - if (res[i] == '.') - res[i] = '/'; + std::replace(res.begin(), res.end(), '.', '/'); 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(time(NULL))); + mLua["math"]["randomseed"](static_cast(std::time(nullptr))); mLua["math"]["randomseed"] = sol::nil; mLua["writeToLog"] = [](std::string_view s) { Log(Debug::Level::Info) << s; }; diff --git a/components/lua/omwscriptsparser.cpp b/components/lua/omwscriptsparser.cpp new file mode 100644 index 0000000000..bc73e013db --- /dev/null +++ b/components/lua/omwscriptsparser.cpp @@ -0,0 +1,44 @@ +#include "omwscriptsparser.hpp" + +#include + +#include + +std::vector LuaUtil::parseOMWScriptsFiles(const VFS::Manager* vfs, const std::vector& 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 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(*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; +} diff --git a/components/lua/omwscriptsparser.hpp b/components/lua/omwscriptsparser.hpp new file mode 100644 index 0000000000..1da9f123b2 --- /dev/null +++ b/components/lua/omwscriptsparser.hpp @@ -0,0 +1,14 @@ +#ifndef COMPONENTS_LUA_OMWSCRIPTSPARSER_H +#define COMPONENTS_LUA_OMWSCRIPTSPARSER_H + +#include + +namespace LuaUtil +{ + + // Parses list of `*.omwscripts` files. + std::vector parseOMWScriptsFiles(const VFS::Manager* vfs, const std::vector& scriptLists); + +} + +#endif // COMPONENTS_LUA_OMWSCRIPTSPARSER_H diff --git a/components/lua/scriptscontainer.cpp b/components/lua/scriptscontainer.cpp index cb2ab0a97e..a53b1d0404 100644 --- a/components/lua/scriptscontainer.cpp +++ b/components/lua/scriptscontainer.cpp @@ -78,10 +78,10 @@ namespace LuaUtil bool ScriptsContainer::removeScript(const std::string& path) { - auto it = mScripts.find(path); - if (it == mScripts.end()) + auto scriptIter = mScripts.find(path); + 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(); @@ -110,10 +110,10 @@ namespace LuaUtil for (auto& [key, value] : sol::table(engineHandlers)) { std::string_view handlerName = key.as(); - auto it = mEngineHandlers.find(handlerName); - if (it == mEngineHandlers.end()) + auto handlerIter = mEngineHandlers.find(handlerName); + if (handlerIter == mEngineHandlers.end()) continue; - std::vector& list = it->second->mList; + std::vector& list = handlerIter->second->mList; list.erase(std::find(list.begin(), list.end(), value.as())); } } @@ -126,7 +126,7 @@ namespace LuaUtil list.erase(std::find(list.begin(), list.end(), value.as())); } } - 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> 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(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) diff --git a/components/lua/scriptscontainer.hpp b/components/lua/scriptscontainer.hpp index da36109851..7b2b2a7aa9 100644 --- a/components/lua/scriptscontainer.hpp +++ b/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 diff --git a/components/lua/serialization.cpp b/components/lua/serialization.cpp index acbe3c23c7..53b6fe3b92 100644 --- a/components/lua/serialization.cpp +++ b/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(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)