From 32f54eb55507220dc632f2422d1b69a995757961 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Mon, 15 Sep 2025 20:17:08 +0200 Subject: [PATCH 1/5] Turn menu.saveGame into a delayed action --- CMakeLists.txt | 2 +- apps/openmw/mwlua/luamanagerimp.cpp | 10 ++++++++++ apps/openmw/mwlua/luamanagerimp.hpp | 4 +++- apps/openmw/mwlua/menuscripts.cpp | 18 +++++++++++------- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 834668e92a..63ed9edfe1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,7 +82,7 @@ message(STATUS "Configuring OpenMW...") set(OPENMW_VERSION_MAJOR 0) set(OPENMW_VERSION_MINOR 50) set(OPENMW_VERSION_RELEASE 0) -set(OPENMW_LUA_API_REVISION 95) +set(OPENMW_LUA_API_REVISION 96) set(OPENMW_POSTPROCESSING_API_REVISION 3) set(OPENMW_VERSION_COMMITHASH "") diff --git a/apps/openmw/mwlua/luamanagerimp.cpp b/apps/openmw/mwlua/luamanagerimp.cpp index 0b760ccdf0..5236cd538a 100644 --- a/apps/openmw/mwlua/luamanagerimp.cpp +++ b/apps/openmw/mwlua/luamanagerimp.cpp @@ -316,6 +316,8 @@ namespace MWLua void LuaManager::applyDelayedActions() { + if (mApplyingDelayedActions) + return; mApplyingDelayedActions = true; for (DelayedAction& action : mActionQueue) action.apply(); @@ -324,6 +326,9 @@ namespace MWLua if (mTeleportPlayerAction) mTeleportPlayerAction->apply(); mTeleportPlayerAction.reset(); + for (DelayedAction& action : mSaveActionQueue) + action.apply(); + mSaveActionQueue.clear(); mApplyingDelayedActions = false; } @@ -824,6 +829,11 @@ namespace MWLua mTeleportPlayerAction = DelayedAction(&mLua, std::move(action), "TeleportPlayer"); } + void LuaManager::addSaveGameAction(std::function action) + { + mSaveActionQueue.emplace_back(&mLua, std::move(action), "SaveGame"); + } + void LuaManager::reportStats(unsigned int frameNumber, osg::Stats& stats) const { stats.setAttribute(frameNumber, "Lua UsedMemory", mLua.getTotalMemoryUsage()); diff --git a/apps/openmw/mwlua/luamanagerimp.hpp b/apps/openmw/mwlua/luamanagerimp.hpp index 42b18d236f..9386ce0499 100644 --- a/apps/openmw/mwlua/luamanagerimp.hpp +++ b/apps/openmw/mwlua/luamanagerimp.hpp @@ -125,8 +125,9 @@ 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. - void addAction(std::function action, std::string_view name = ""); + void addAction(std::function action, std::string_view name = {}); void addTeleportPlayerAction(std::function action); + void addSaveGameAction(std::function action); // Saving void write(ESM::ESMWriter& writer, Loading::Listener& progress) override; @@ -234,6 +235,7 @@ namespace MWLua }; std::vector mActionQueue; std::optional mTeleportPlayerAction; + std::vector mSaveActionQueue; std::vector> mUIMessages; std::vector> mInGameConsoleMessages; std::optional mDelayedUiModeChangedArg; diff --git a/apps/openmw/mwlua/menuscripts.cpp b/apps/openmw/mwlua/menuscripts.cpp index 6387a3dce1..81f20af955 100644 --- a/apps/openmw/mwlua/menuscripts.cpp +++ b/apps/openmw/mwlua/menuscripts.cpp @@ -8,6 +8,7 @@ #include "../mwstate/character.hpp" #include "context.hpp" +#include "luamanagerimp.hpp" namespace MWLua { @@ -72,13 +73,16 @@ namespace MWLua return sol::nullopt; }; - api["saveGame"] = [](std::string_view description, sol::optional slotName) { - MWBase::StateManager* manager = MWBase::Environment::get().getStateManager(); - const MWState::Character* character = manager->getCurrentCharacter(); - const MWState::Slot* slot = nullptr; - if (slotName) - slot = findSlot(character, *slotName); - manager->saveGame(description, slot); + api["saveGame"] = [context](std::string description, sol::optional slotName) { + context.mLuaManager->addSaveGameAction( + [description = std::move(description), slotName = std::move(slotName)]() { + MWBase::StateManager* manager = MWBase::Environment::get().getStateManager(); + const MWState::Character* character = manager->getCurrentCharacter(); + const MWState::Slot* slot = nullptr; + if (slotName) + slot = findSlot(character, *slotName); + manager->saveGame(description, slot); + }); }; auto getSaves = [](sol::state_view currentState, const MWState::Character& character) { From 6d5da282a53cebdd5716d6e55f47a6783aa33188 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Tue, 16 Sep 2025 22:02:23 +0200 Subject: [PATCH 2/5] Only allow saving in synchronizedUpdateUnsafe --- apps/openmw/mwlua/luamanagerimp.cpp | 25 +++++++++++++++---------- apps/openmw/mwlua/luamanagerimp.hpp | 5 +++-- apps/openmw/mwlua/menuscripts.cpp | 19 +++++++++---------- components/lua/scriptscontainer.cpp | 4 ++-- components/lua/scriptscontainer.hpp | 2 +- 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/apps/openmw/mwlua/luamanagerimp.cpp b/apps/openmw/mwlua/luamanagerimp.cpp index 5236cd538a..fde7a63dbb 100644 --- a/apps/openmw/mwlua/luamanagerimp.cpp +++ b/apps/openmw/mwlua/luamanagerimp.cpp @@ -42,6 +42,20 @@ namespace MWLua { + namespace + { + struct AllowSaving + { + bool& mAllowSaving; + AllowSaving(bool& allowSaving) + : mAllowSaving(allowSaving) + { + mAllowSaving = true; + } + + ~AllowSaving() { mAllowSaving = false; } + }; + } static LuaUtil::LuaStateSettings createLuaStateSettings() { @@ -264,6 +278,7 @@ namespace MWLua // can teleport the player to the starting location before the first frame is rendered. mGlobalScripts.newGameStarted(); } + AllowSaving savingGuard(mAllowSaving); // We apply input events in `synchronizedUpdate` rather than in `update` in order to reduce input latency. mProcessingInputEvents = true; @@ -316,8 +331,6 @@ namespace MWLua void LuaManager::applyDelayedActions() { - if (mApplyingDelayedActions) - return; mApplyingDelayedActions = true; for (DelayedAction& action : mActionQueue) action.apply(); @@ -326,9 +339,6 @@ namespace MWLua if (mTeleportPlayerAction) mTeleportPlayerAction->apply(); mTeleportPlayerAction.reset(); - for (DelayedAction& action : mSaveActionQueue) - action.apply(); - mSaveActionQueue.clear(); mApplyingDelayedActions = false; } @@ -829,11 +839,6 @@ namespace MWLua mTeleportPlayerAction = DelayedAction(&mLua, std::move(action), "TeleportPlayer"); } - void LuaManager::addSaveGameAction(std::function action) - { - mSaveActionQueue.emplace_back(&mLua, std::move(action), "SaveGame"); - } - void LuaManager::reportStats(unsigned int frameNumber, osg::Stats& stats) const { stats.setAttribute(frameNumber, "Lua UsedMemory", mLua.getTotalMemoryUsage()); diff --git a/apps/openmw/mwlua/luamanagerimp.hpp b/apps/openmw/mwlua/luamanagerimp.hpp index 9386ce0499..62d0de8f06 100644 --- a/apps/openmw/mwlua/luamanagerimp.hpp +++ b/apps/openmw/mwlua/luamanagerimp.hpp @@ -127,7 +127,6 @@ namespace MWLua // OSG Cull), so we need to queue it and apply from the main thread. void addAction(std::function action, std::string_view name = {}); void addTeleportPlayerAction(std::function action); - void addSaveGameAction(std::function action); // Saving void write(ESM::ESMWriter& writer, Loading::Listener& progress) override; @@ -175,6 +174,8 @@ namespace MWLua void sendLocalEvent( const MWWorld::Ptr& target, const std::string& name, const std::optional& data = std::nullopt); + bool savingAllowed() const { return mAllowSaving; } + private: void initConfiguration(); LocalScripts* createLocalScripts(const MWWorld::Ptr& ptr, @@ -188,6 +189,7 @@ namespace MWLua bool mApplyingDelayedActions = false; bool mNewGameStarted = false; bool mReloadAllScriptsRequested = false; + bool mAllowSaving = false; LuaUtil::ScriptsConfiguration mConfiguration; LuaUtil::LuaState mLua; LuaUi::ResourceManager mUiResourceManager; @@ -235,7 +237,6 @@ namespace MWLua }; std::vector mActionQueue; std::optional mTeleportPlayerAction; - std::vector mSaveActionQueue; std::vector> mUIMessages; std::vector> mInGameConsoleMessages; std::optional mDelayedUiModeChangedArg; diff --git a/apps/openmw/mwlua/menuscripts.cpp b/apps/openmw/mwlua/menuscripts.cpp index 81f20af955..528e742c17 100644 --- a/apps/openmw/mwlua/menuscripts.cpp +++ b/apps/openmw/mwlua/menuscripts.cpp @@ -73,16 +73,15 @@ namespace MWLua return sol::nullopt; }; - api["saveGame"] = [context](std::string description, sol::optional slotName) { - context.mLuaManager->addSaveGameAction( - [description = std::move(description), slotName = std::move(slotName)]() { - MWBase::StateManager* manager = MWBase::Environment::get().getStateManager(); - const MWState::Character* character = manager->getCurrentCharacter(); - const MWState::Slot* slot = nullptr; - if (slotName) - slot = findSlot(character, *slotName); - manager->saveGame(description, slot); - }); + api["saveGame"] = [context](std::string_view description, sol::optional slotName) { + if (!context.mLuaManager->savingAllowed()) + throw std::runtime_error("The game cannot be saved at the moment"); + MWBase::StateManager* manager = MWBase::Environment::get().getStateManager(); + const MWState::Character* character = manager->getCurrentCharacter(); + const MWState::Slot* slot = nullptr; + if (slotName) + slot = findSlot(character, *slotName); + manager->saveGame(description, slot); }; auto getSaves = [](sol::state_view currentState, const MWState::Character& character) { diff --git a/components/lua/scriptscontainer.cpp b/components/lua/scriptscontainer.cpp index f5b5605e37..36bdbca8b1 100644 --- a/components/lua/scriptscontainer.cpp +++ b/components/lua/scriptscontainer.cpp @@ -47,7 +47,7 @@ namespace LuaUtil } } - void ScriptsContainer::printError(int scriptId, std::string_view msg, const std::exception& e) + void ScriptsContainer::printError(int scriptId, std::string_view msg, const std::exception& e) const { Log(Debug::Error) << mNamePrefix << "[" << scriptPath(scriptId) << "] " << msg << ": " << e.what(); } @@ -408,7 +408,7 @@ namespace LuaUtil void ScriptsContainer::save(ESM::LuaScripts& data) { - if (UnloadedData* unloadedData = std::get_if(&mData)) + if (const UnloadedData* unloadedData = std::get_if(&mData)) { data.mScripts = unloadedData->mScripts; return; diff --git a/components/lua/scriptscontainer.hpp b/components/lua/scriptscontainer.hpp index 8eaaf2955f..275c300ac9 100644 --- a/components/lua/scriptscontainer.hpp +++ b/components/lua/scriptscontainer.hpp @@ -271,7 +271,7 @@ namespace LuaUtil // Returns script by id (throws an exception if doesn't exist) Script& getScript(int scriptId); - void printError(int scriptId, std::string_view msg, const std::exception& e); + void printError(int scriptId, std::string_view msg, const std::exception& e) const; const VFS::Path::Normalized& scriptPath(int scriptId) const { From 51a4f8749e74ddcda072fa51cac73567a66ea7ed Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Wed, 17 Sep 2025 21:11:57 +0200 Subject: [PATCH 3/5] Change method name and error message --- apps/openmw/mwlua/luamanagerimp.hpp | 2 +- apps/openmw/mwlua/menuscripts.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/openmw/mwlua/luamanagerimp.hpp b/apps/openmw/mwlua/luamanagerimp.hpp index 62d0de8f06..fee5260945 100644 --- a/apps/openmw/mwlua/luamanagerimp.hpp +++ b/apps/openmw/mwlua/luamanagerimp.hpp @@ -174,7 +174,7 @@ namespace MWLua void sendLocalEvent( const MWWorld::Ptr& target, const std::string& name, const std::optional& data = std::nullopt); - bool savingAllowed() const { return mAllowSaving; } + bool isSynchronizedUpdateRunning() const { return mAllowSaving; } private: void initConfiguration(); diff --git a/apps/openmw/mwlua/menuscripts.cpp b/apps/openmw/mwlua/menuscripts.cpp index 528e742c17..e71e2aeacb 100644 --- a/apps/openmw/mwlua/menuscripts.cpp +++ b/apps/openmw/mwlua/menuscripts.cpp @@ -74,8 +74,8 @@ namespace MWLua }; api["saveGame"] = [context](std::string_view description, sol::optional slotName) { - if (!context.mLuaManager->savingAllowed()) - throw std::runtime_error("The game cannot be saved at the moment"); + if (!context.mLuaManager->isSynchronizedUpdateRunning()) + throw std::runtime_error("The game cannot be saved outside events"); MWBase::StateManager* manager = MWBase::Environment::get().getStateManager(); const MWState::Character* character = manager->getCurrentCharacter(); const MWState::Slot* slot = nullptr; From f73f4ae415f986e75b41a5f275677efea376a245 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Thu, 18 Sep 2025 16:50:28 +0200 Subject: [PATCH 4/5] Reuse BoolScopeGuard --- apps/openmw/mwlua/luamanagerimp.cpp | 58 ++++++++++++++--------------- apps/openmw/mwlua/luamanagerimp.hpp | 4 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/apps/openmw/mwlua/luamanagerimp.cpp b/apps/openmw/mwlua/luamanagerimp.cpp index fde7a63dbb..c0146234f5 100644 --- a/apps/openmw/mwlua/luamanagerimp.cpp +++ b/apps/openmw/mwlua/luamanagerimp.cpp @@ -44,16 +44,16 @@ namespace MWLua { namespace { - struct AllowSaving + struct BoolScopeGuard { - bool& mAllowSaving; - AllowSaving(bool& allowSaving) - : mAllowSaving(allowSaving) + bool& mValue; + BoolScopeGuard(bool& value) + : mValue(value) { - mAllowSaving = true; + mValue = true; } - ~AllowSaving() { mAllowSaving = false; } + ~BoolScopeGuard() { mValue = false; } }; } @@ -278,32 +278,33 @@ namespace MWLua // can teleport the player to the starting location before the first frame is rendered. mGlobalScripts.newGameStarted(); } - AllowSaving savingGuard(mAllowSaving); + BoolScopeGuard updateGuard(mRunningSynchronizedUpdates); - // We apply input events in `synchronizedUpdate` rather than in `update` in order to reduce input latency. - mProcessingInputEvents = true; + MWBase::WindowManager* windowManager = MWBase::Environment::get().getWindowManager(); PlayerScripts* playerScripts = mPlayer.isEmpty() ? nullptr : dynamic_cast(mPlayer.getRefData().getLuaScripts()); - MWBase::WindowManager* windowManager = MWBase::Environment::get().getWindowManager(); - - for (const auto& event : mMenuInputEvents) - mMenuScripts.processInputEvent(event); - mMenuInputEvents.clear(); - if (playerScripts && !windowManager->containsMode(MWGui::GM_MainMenu)) + // We apply input events in `synchronizedUpdate` rather than in `update` in order to reduce input latency. { - for (const auto& event : mInputEvents) - playerScripts->processInputEvent(event); + BoolScopeGuard processingGuard(mProcessingInputEvents); + + for (const auto& event : mMenuInputEvents) + mMenuScripts.processInputEvent(event); + mMenuInputEvents.clear(); + if (playerScripts && !windowManager->containsMode(MWGui::GM_MainMenu)) + { + for (const auto& event : mInputEvents) + playerScripts->processInputEvent(event); + } + mInputEvents.clear(); + mLuaEvents.callMenuEventHandlers(); + double frameDuration = MWBase::Environment::get().getWorld()->getTimeManager()->isPaused() + ? 0.0 + : MWBase::Environment::get().getFrameDuration(); + mInputActions.update(frameDuration); + mMenuScripts.onFrame(frameDuration); + if (playerScripts) + playerScripts->onFrame(frameDuration); } - mInputEvents.clear(); - mLuaEvents.callMenuEventHandlers(); - double frameDuration = MWBase::Environment::get().getWorld()->getTimeManager()->isPaused() - ? 0.0 - : MWBase::Environment::get().getFrameDuration(); - mInputActions.update(frameDuration); - mMenuScripts.onFrame(frameDuration); - if (playerScripts) - playerScripts->onFrame(frameDuration); - mProcessingInputEvents = false; for (const auto& [message, mode] : mUIMessages) windowManager->messageBox(message, mode); @@ -331,7 +332,7 @@ namespace MWLua void LuaManager::applyDelayedActions() { - mApplyingDelayedActions = true; + BoolScopeGuard applyingGuard(mApplyingDelayedActions); for (DelayedAction& action : mActionQueue) action.apply(); mActionQueue.clear(); @@ -339,7 +340,6 @@ namespace MWLua if (mTeleportPlayerAction) mTeleportPlayerAction->apply(); mTeleportPlayerAction.reset(); - mApplyingDelayedActions = false; } void LuaManager::clear() diff --git a/apps/openmw/mwlua/luamanagerimp.hpp b/apps/openmw/mwlua/luamanagerimp.hpp index fee5260945..80c3163c80 100644 --- a/apps/openmw/mwlua/luamanagerimp.hpp +++ b/apps/openmw/mwlua/luamanagerimp.hpp @@ -174,7 +174,7 @@ namespace MWLua void sendLocalEvent( const MWWorld::Ptr& target, const std::string& name, const std::optional& data = std::nullopt); - bool isSynchronizedUpdateRunning() const { return mAllowSaving; } + bool isSynchronizedUpdateRunning() const { return mRunningSynchronizedUpdates; } private: void initConfiguration(); @@ -189,7 +189,7 @@ namespace MWLua bool mApplyingDelayedActions = false; bool mNewGameStarted = false; bool mReloadAllScriptsRequested = false; - bool mAllowSaving = false; + bool mRunningSynchronizedUpdates = false; LuaUtil::ScriptsConfiguration mConfiguration; LuaUtil::LuaState mLua; LuaUi::ResourceManager mUiResourceManager; From e4bc681c1b55c52808c528e75527c53fbbed026c Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sat, 20 Sep 2025 10:18:10 +0200 Subject: [PATCH 5/5] Tweak error message --- apps/openmw/mwlua/menuscripts.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/openmw/mwlua/menuscripts.cpp b/apps/openmw/mwlua/menuscripts.cpp index e71e2aeacb..c9134201d3 100644 --- a/apps/openmw/mwlua/menuscripts.cpp +++ b/apps/openmw/mwlua/menuscripts.cpp @@ -75,7 +75,7 @@ namespace MWLua api["saveGame"] = [context](std::string_view description, sol::optional slotName) { if (!context.mLuaManager->isSynchronizedUpdateRunning()) - throw std::runtime_error("The game cannot be saved outside events"); + throw std::runtime_error("menu.saveGame can only be used during engine or event handler processing"); MWBase::StateManager* manager = MWBase::Environment::get().getStateManager(); const MWState::Character* character = manager->getCurrentCharacter(); const MWState::Slot* slot = nullptr;