Merge branch 'lua_storage' into 'master'

[Lua] Update openmw.storage

See merge request OpenMW/openmw!1795

apps/openmw/mwlua/luamanagerimp.cpp

@@ -277,8 +277,8 @@ namespace MWLua
             mPlayer.getRefData().setLuaScripts(nullptr);
             mPlayer = MWWorld::Ptr();
         }
-        mGlobalStorage.clearTemporary();
+        mGlobalStorage.clearTemporaryAndRemoveCallbacks();
-        mPlayerStorage.clearTemporary();
+        mPlayerStorage.clearTemporaryAndRemoveCallbacks();
     }
 
     void LuaManager::setupPlayer(const MWWorld::Ptr& ptr)

apps/openmw_test_suite/lua/test_storage.cpp

@@ -2,6 +2,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <components/lua/scriptscontainer.hpp>
 #include <components/lua/storage.hpp>
 
 namespace
@@ -19,15 +20,27 @@ namespace
         sol::state mLua;
         LuaUtil::LuaStorage::initLuaBindings(mLua);
         LuaUtil::LuaStorage storage(mLua);
+
+        std::vector<std::string> callbackCalls;
+        LuaUtil::Callback callback{
+            sol::make_object(mLua, [&](const std::string& section, const sol::optional<std::string>& key)
+            {
+                if (key)
+                    callbackCalls.push_back(section + "_" + *key);
+                else
+                    callbackCalls.push_back(section + "_*");
+            }),
+            sol::table(mLua, sol::create)
+        };
+        callback.mHiddenData[LuaUtil::ScriptsContainer::sScriptIdKey] = "fakeId";
+
         mLua["mutable"] = storage.getMutableSection("test");
         mLua["ro"] = storage.getReadOnlySection("test");
+        mLua["ro"]["subscribe"](mLua["ro"], callback);
 
         mLua.safe_script("mutable:set('x', 5)");
         EXPECT_EQ(get<int>(mLua, "mutable:get('x')"), 5);
         EXPECT_EQ(get<int>(mLua, "ro:get('x')"), 5);
-        EXPECT_FALSE(get<bool>(mLua, "mutable:wasChanged()"));
-        EXPECT_TRUE(get<bool>(mLua, "ro:wasChanged()"));
-        EXPECT_FALSE(get<bool>(mLua, "ro:wasChanged()"));
 
         EXPECT_THROW(mLua.safe_script("ro:set('y', 3)"), std::exception);
 
@@ -42,9 +55,8 @@ namespace
         mLua.safe_script("mutable:reset({x=4, y=7})");
         EXPECT_EQ(get<int>(mLua, "ro:get('x')"), 4);
         EXPECT_EQ(get<int>(mLua, "ro:get('y')"), 7);
-        EXPECT_FALSE(get<bool>(mLua, "mutable:wasChanged()"));
+
-        EXPECT_TRUE(get<bool>(mLua, "ro:wasChanged()"));
+        EXPECT_THAT(callbackCalls, ::testing::ElementsAre("test_x", "test_*", "test_*"));
-        EXPECT_FALSE(get<bool>(mLua, "ro:wasChanged()"));
     }
 
     TEST(LuaUtilStorageTest, Table)
@@ -81,7 +93,7 @@ namespace
         EXPECT_EQ(get<int>(mLua, "permanent:get('x')"), 1);
         EXPECT_EQ(get<int>(mLua, "temporary:get('y')"), 2);
 
-        storage.clearTemporary();
+        storage.clearTemporaryAndRemoveCallbacks();
         mLua["permanent"] = storage.getMutableSection("permanent");
         mLua["temporary"] = storage.getMutableSection("temporary");
         EXPECT_EQ(get<int>(mLua, "permanent:get('x')"), 1);

components/lua/scriptscontainer.hpp

@@ -249,16 +249,28 @@ namespace LuaUtil
         sol::function mFunc;
         sol::table mHiddenData;  // same object as Script::mHiddenData in ScriptsContainer
 
+        bool isValid() const { return mHiddenData[ScriptsContainer::sScriptIdKey] != sol::nil; }
+
         template <typename... Args>
         sol::object operator()(Args&&... args) const
         {
-            if (mHiddenData[ScriptsContainer::sScriptIdKey] != sol::nil)
+            if (isValid())
                 return LuaUtil::call(mFunc, std::forward<Args>(args)...);
             else
                 Log(Debug::Debug) << "Ignored callback to the removed script "
                                   << mHiddenData.get<std::string>(ScriptsContainer::sScriptDebugNameKey);
             return sol::nil;
         }
+
+        template <typename... Args>
+        void tryCall(Args&&... args) const
+        {
+            try { (*this)(std::forward<Args>(args)...); }
+            catch (std::exception& e)
+            {
+                Log(Debug::Error) << "Error in callback: " << e.what();
+            }
+        }
     };
 
 }

components/lua/storage.cpp

@@ -8,9 +8,7 @@
 namespace sol
 {
     template <>
-    struct is_automagical<LuaUtil::LuaStorage::SectionMutableView> : std::false_type {};
+    struct is_automagical<LuaUtil::LuaStorage::SectionView> : std::false_type {};
-    template <>
-    struct is_automagical<LuaUtil::LuaStorage::SectionReadOnlyView> : std::false_type {};
 }
 
 namespace LuaUtil
@@ -38,19 +36,49 @@ namespace LuaUtil
             return sEmpty;
     }
 
-    void LuaStorage::Section::set(std::string_view key, const sol::object& value)
+    void LuaStorage::Section::runCallbacks(sol::optional<std::string_view> changedKey)
     {
-        mValues[std::string(key)] = Value(value);
+        mStorage->mRunningCallbacks = true;
-        mChangeCounter++;
+        mCallbacks.erase(std::remove_if(mCallbacks.begin(), mCallbacks.end(), [&](const Callback& callback)
-        if (mStorage->mListener)
+        {
-            (*mStorage->mListener)(mSectionName, key, value);
+            bool valid = callback.isValid();
+            if (valid)
+                callback.tryCall(mSectionName, changedKey);
+            return !valid;
+        }), mCallbacks.end());
+        mStorage->mRunningCallbacks = false;
     }
 
-    bool LuaStorage::Section::wasChanged(int64_t& lastCheck)
+    void LuaStorage::Section::set(std::string_view key, const sol::object& value)
     {
-        bool res = lastCheck < mChangeCounter;
+        if (mStorage->mRunningCallbacks)
-        lastCheck = mChangeCounter;
+            throw std::runtime_error("Not allowed to change storage in storage handlers because it can lead to an infinite recursion");
-        return res;
+        if (value != sol::nil)
+            mValues[std::string(key)] = Value(value);
+        else
+        {
+            auto it = mValues.find(key);
+            if (it != mValues.end())
+                mValues.erase(it);
+        }
+        if (mStorage->mListener)
+            mStorage->mListener->valueChanged(mSectionName, key, value);
+        runCallbacks(key);
+    }
+
+    void LuaStorage::Section::setAll(const sol::optional<sol::table>& values)
+    {
+        if (mStorage->mRunningCallbacks)
+            throw std::runtime_error("Not allowed to change storage in storage handlers because it can lead to an infinite recursion");
+        mValues.clear();
+        if (values)
+        {
+            for (const auto& [k, v] : *values)
+                mValues[k.as<std::string>()] = Value(v);
+        }
+        if (mStorage->mListener)
+            mStorage->mListener->sectionReplaced(mSectionName, values);
+        runCallbacks(sol::nullopt);
     }
 
     sol::table LuaStorage::Section::asTable()
@@ -64,62 +92,53 @@ namespace LuaUtil
     void LuaStorage::initLuaBindings(lua_State* L)
     {
         sol::state_view lua(L);
-        sol::usertype<SectionReadOnlyView> roView = lua.new_usertype<SectionReadOnlyView>("ReadOnlySection");
+        sol::usertype<SectionView> sview = lua.new_usertype<SectionView>("Section");
-        sol::usertype<SectionMutableView> mutableView = lua.new_usertype<SectionMutableView>("MutableSection");
+        sview["get"] = [](sol::this_state s, const SectionView& section, std::string_view key)
-        roView["get"] = [](sol::this_state s, SectionReadOnlyView& section, std::string_view key)
         {
             return section.mSection->get(key).getReadOnly(s);
         };
-        roView["getCopy"] = [](sol::this_state s, SectionReadOnlyView& section, std::string_view key)
+        sview["getCopy"] = [](sol::this_state s, const SectionView& section, std::string_view key)
         {
             return section.mSection->get(key).getCopy(s);
         };
-        roView["wasChanged"] = [](SectionReadOnlyView& section) { return section.mSection->wasChanged(section.mLastCheck); };
+        sview["asTable"] = [](const SectionView& section) { return section.mSection->asTable(); };
-        roView["asTable"] = [](SectionReadOnlyView& section) { return section.mSection->asTable(); };
+        sview["subscribe"] = [](const SectionView& section, const Callback& callback)
-        mutableView["get"] = [](sol::this_state s, SectionMutableView& section, std::string_view key)
         {
-            return section.mSection->get(key).getReadOnly(s);
+            std::vector<Callback>& callbacks = section.mSection->mCallbacks;
-        };
+            if (!callbacks.empty() && callbacks.size() == callbacks.capacity())
-        mutableView["getCopy"] = [](sol::this_state s, SectionMutableView& section, std::string_view key)
-        {
-            return section.mSection->get(key).getCopy(s);
-        };
-        mutableView["wasChanged"] = [](SectionMutableView& section) { return section.mSection->wasChanged(section.mLastCheck); };
-        mutableView["asTable"] = [](SectionMutableView& section) { return section.mSection->asTable(); };
-        mutableView["reset"] = [](SectionMutableView& section, sol::optional<sol::table> newValues)
-        {
-            section.mSection->mValues.clear();
-            if (newValues)
             {
-                for (const auto& [k, v] : *newValues)
+                callbacks.erase(std::remove_if(callbacks.begin(), callbacks.end(),
-                {
+                                               [&](const Callback& c) { return !c.isValid(); }),
-                    try
+                                callbacks.end());
-                    {
-                        section.mSection->set(k.as<std::string_view>(), v);
-                    }
-                    catch (std::exception& e)
-                    {
-                        Log(Debug::Error) << "LuaUtil::LuaStorage::Section::reset(table): " << e.what();
-                    }
-                }
             }
-            section.mSection->mChangeCounter++;
+            callbacks.push_back(callback);
-            section.mLastCheck = section.mSection->mChangeCounter;
         };
-        mutableView["removeOnExit"] = [](SectionMutableView& section) { section.mSection->mPermanent = false; };
+        sview["reset"] = [](const SectionView& section, const sol::optional<sol::table>& newValues)
-        mutableView["set"] = [](SectionMutableView& section, std::string_view key, const sol::object& value)
         {
-            if (section.mLastCheck == section.mSection->mChangeCounter)
+            if (section.mReadOnly)
-                section.mLastCheck++;
+                throw std::runtime_error("Access to storage is read only");
+            section.mSection->setAll(newValues);
+        };
+        sview["removeOnExit"] = [](const SectionView& section)
+        {
+            if (section.mReadOnly)
+                throw std::runtime_error("Access to storage is read only");
+            section.mSection->mPermanent = false;
+        };
+        sview["set"] = [](const SectionView& section, std::string_view key, const sol::object& value)
+        {
+            if (section.mReadOnly)
+                throw std::runtime_error("Access to storage is read only");
             section.mSection->set(key, value);
         };
     }
 
-    void LuaStorage::clearTemporary()
+    void LuaStorage::clearTemporaryAndRemoveCallbacks()
     {
         auto it = mData.begin();
         while (it != mData.end())
         {
+            it->second->mCallbacks.clear();
             if (!it->second->mPermanent)
             {
                 it->second->mValues.clear();
@@ -157,7 +176,7 @@ namespace LuaUtil
         sol::table data(mLua, sol::create);
         for (const auto& [sectionName, section] : mData)
         {
-            if (section->mPermanent)
+            if (section->mPermanent && !section->mValues.empty())
                 data[sectionName] = section->asTable();
         }
         std::string serializedData = serialize(data);
@@ -178,23 +197,17 @@ namespace LuaUtil
         return newIt->second;
     }
 
-    sol::object LuaStorage::getReadOnlySection(std::string_view sectionName)
+    sol::object LuaStorage::getSection(std::string_view sectionName, bool readOnly)
     {
         const std::shared_ptr<Section>& section = getSection(sectionName);
-        return sol::make_object<SectionReadOnlyView>(mLua, SectionReadOnlyView{section, section->mChangeCounter});
+        return sol::make_object<SectionView>(mLua, SectionView{section, readOnly});
     }
 
-    sol::object LuaStorage::getMutableSection(std::string_view sectionName)
+    sol::table LuaStorage::getAllSections(bool readOnly)
-    {
-        const std::shared_ptr<Section>& section = getSection(sectionName);
-        return sol::make_object<SectionMutableView>(mLua, SectionMutableView{section, section->mChangeCounter});
-    }
-
-    sol::table LuaStorage::getAllSections()
     {
         sol::table res(mLua, sol::create);
         for (const auto& [sectionName, _] : mData)
-            res[sectionName] = getMutableSection(sectionName);
+            res[sectionName] = getSection(sectionName, readOnly);
         return res;
     }
 

components/lua/storage.hpp

@@ -4,6 +4,7 @@
 #include <map>
 #include <sol/sol.hpp>
 
+#include "scriptscontainer.hpp"
 #include "serialization.hpp"
 
 namespace LuaUtil
@@ -16,18 +17,28 @@ namespace LuaUtil
 
         explicit LuaStorage(lua_State* lua) : mLua(lua) {}
 
-        void clearTemporary();
+        void clearTemporaryAndRemoveCallbacks();
         void load(const std::string& path);
         void save(const std::string& path) const;
 
-        sol::object getReadOnlySection(std::string_view sectionName);
+        sol::object getSection(std::string_view sectionName, bool readOnly);
-        sol::object getMutableSection(std::string_view sectionName);
+        sol::object getMutableSection(std::string_view sectionName) { return getSection(sectionName, false); }
-        sol::table getAllSections();
+        sol::object getReadOnlySection(std::string_view sectionName) { return getSection(sectionName, true); }
+        sol::table getAllSections(bool readOnly = false);
 
-        void set(std::string_view section, std::string_view key, const sol::object& value) { getSection(section)->set(key, value); }
+        void setSingleValue(std::string_view section, std::string_view key, const sol::object& value)
+            { getSection(section)->set(key, value); }
 
-        using ListenerFn = std::function<void(std::string_view, std::string_view, const sol::object&)>;
+        void setSectionValues(std::string_view section, const sol::optional<sol::table>& values)
-        void setListener(ListenerFn fn) { mListener = std::move(fn); }
+            { getSection(section)->setAll(values); }
+
+        class Listener
+        {
+        public:
+            virtual void valueChanged(std::string_view section, std::string_view key, const sol::object& value) const = 0;
+            virtual void sectionReplaced(std::string_view section, const sol::optional<sol::table>& values) const = 0;
+        };
+        void setListener(const Listener* listener) { mListener = listener; }
 
     private:
         class Value
@@ -48,32 +59,29 @@ namespace LuaUtil
             explicit Section(LuaStorage* storage, std::string name) : mStorage(storage), mSectionName(std::move(name)) {}
             const Value& get(std::string_view key) const;
             void set(std::string_view key, const sol::object& value);
-            bool wasChanged(int64_t& lastCheck);
+            void setAll(const sol::optional<sol::table>& values);
             sol::table asTable();
+            void runCallbacks(sol::optional<std::string_view> changedKey);
 
             LuaStorage* mStorage;
             std::string mSectionName;
             std::map<std::string, Value, std::less<>> mValues;
+            std::vector<Callback> mCallbacks;
             bool mPermanent = true;
-            int64_t mChangeCounter = 0;
             static Value sEmpty;
         };
-        struct SectionMutableView
+        struct SectionView
         {
-            std::shared_ptr<Section> mSection = nullptr;
+            std::shared_ptr<Section> mSection;
-            int64_t mLastCheck = 0;
+            bool mReadOnly;
-        };
-        struct SectionReadOnlyView
-        {
-            std::shared_ptr<Section> mSection = nullptr;
-            int64_t mLastCheck = 0;
         };
 
         const std::shared_ptr<Section>& getSection(std::string_view sectionName);
 
         lua_State* mLua;
         std::map<std::string_view, std::shared_ptr<Section>> mData;
-        std::optional<ListenerFn> mListener;
+        const Listener* mListener = nullptr;
+        bool mRunningCallbacks = false;
     };
 
 }

files/lua_api/openmw/storage.lua

@@ -6,14 +6,14 @@
 -- local myModData = storage.globalSection('MyModExample')
 -- myModData:set("someVariable", 1.0)
 -- myModData:set("anotherVariable", { exampleStr='abc', exampleBool=true })
--- local function update()
+-- local async = require('openmw.async')
---     if myModCfg:checkChanged() then
+-- myModData:subscribe(async:callback(function(section, key)
---         print('Data was changes by another script:')
+--     if key then
---         print('MyModExample.someVariable =', myModData:get('someVariable'))
+--         print('Value is changed:', key, '=', myModData:get(key))
---         print('MyModExample.anotherVariable.exampleStr =',
+--     else
---               myModData:get('anotherVariable').exampleStr)
+--         print('All values are changed')
 --     end
--- end
+-- end))
 
 ---
 -- Get a section of the global storage; can be used by any script, but only global scripts can change values.
@@ -58,10 +58,12 @@
 -- @param #string key
 
 ---
--- Return `True` if any value in this section was changed by another script since the last `wasChanged`.
+-- Subscribe to changes in this section.
--- @function [parent=#StorageSection] wasChanged
+-- First argument of the callback is the name of the section (so one callback can be used for different sections).
+-- The second argument is the changed key (or `nil` if `reset` was used and all values were changed at the same time)
+-- @function [parent=#StorageSection] subscribe
 -- @param self
--- @return #boolean
+-- @param openmw.async#Callback callback
 
 ---
 -- Copy all values and return them as a table.
@@ -71,14 +73,14 @@
 
 ---
 -- Remove all existing values and assign values from given (the arg is optional) table.
--- Note: `section:reset()` removes all values, but not the section itself. Use `section:removeOnExit()` to remove the section completely.
+-- This function can not be used for a global storage section from a local script.
+-- Note: `section:reset()` removes the section.
 -- @function [parent=#StorageSection] reset
 -- @param self
 -- @param #table values (optional) New values
 
 ---
 -- Make the whole section temporary: will be removed on exit or when load a save.
--- No section can be removed immediately because other scripts may use it at the moment.
 -- Temporary sections have the same interface to get/set values, the only difference is they will not
 -- be saved to the permanent storage on exit.
 -- This function can not be used for a global storage section from a local script.