Merge branch 'cleanup_dialogue_api' into 'master'

Cleanup dialogue API

See merge request OpenMW/openmw!4088

apps/openmw/mwlua/dialoguebindings.cpp

@@ -1,10 +1,11 @@
 #include "dialoguebindings.hpp"
+
+#include "context.hpp"
+
 #include "apps/openmw/mwbase/environment.hpp"
 #include "apps/openmw/mwworld/esmstore.hpp"
 #include "apps/openmw/mwworld/store.hpp"
-#include "context.hpp"
+
-#include "object.hpp"
-#include <algorithm>
 #include <components/esm3/loaddial.hpp>
 #include <components/lua/luastate.hpp>
 #include <components/misc/resourcehelpers.hpp>
@@ -12,122 +13,44 @@
 
 namespace
 {
-    template <ESM::Dialogue::Type filter>
+    std::vector<const ESM::Dialogue*> makeIndex(const MWWorld::Store<ESM::Dialogue>& store, ESM::Dialogue::Type type)
-    class FilteredDialogueStore
     {
-        const MWWorld::Store<ESM::Dialogue>& mDialogueStore;
+        std::vector<const ESM::Dialogue*> result;
-
+        for (const ESM::Dialogue& v : store)
-        const ESM::Dialogue* foundDialogueFilteredOut(const ESM::Dialogue* possibleResult) const
+            if (v.mType == type)
-        {
+                result.push_back(&v);
-            if (possibleResult && possibleResult->mType == filter)
-            {
-                return possibleResult;
-            }
-            return nullptr;
-        }
-
-    public:
-        FilteredDialogueStore()
-            : mDialogueStore{ MWBase::Environment::get().getESMStore()->get<ESM::Dialogue>() }
-        {
-        }
-
-        class FilteredDialogueIterator
-        {
-            using DecoratedIterator = MWWorld::Store<ESM::Dialogue>::iterator;
-            DecoratedIterator mIter;
-            DecoratedIterator mEndIter;
-
-        public:
-            using iterator_category = DecoratedIterator::iterator_category;
-            using value_type = DecoratedIterator::value_type;
-            using difference_type = DecoratedIterator::difference_type;
-            using pointer = DecoratedIterator::pointer;
-            using reference = DecoratedIterator::reference;
-
-            FilteredDialogueIterator(const DecoratedIterator& pointingIterator, const DecoratedIterator& end)
-                : mIter{ pointingIterator }
-                , mEndIter{ end }
-            {
-            }
-
-            FilteredDialogueIterator& operator++()
-            {
-                if (mIter == mEndIter)
-                {
-                    return *this;
-                }
-
-                do
-                {
-                    ++mIter;
-                } while (mIter != mEndIter && mIter->mType != filter);
-                return *this;
-            }
-
-            FilteredDialogueIterator operator++(int)
-            {
-                FilteredDialogueIterator iter = *this;
-                ++(*this);
-                return iter;
-            }
-
-            FilteredDialogueIterator& operator+=(difference_type advance)
-            {
-                while (advance > 0 && mIter != mEndIter)
-                {
-                    ++(*this);
-                    --advance;
-                }
-                return *this;
-            }
-
-            bool operator==(const FilteredDialogueIterator& x) const { return mIter == x.mIter; }
-
-            bool operator!=(const FilteredDialogueIterator& x) const { return !(*this == x); }
-
-            const value_type& operator*() const { return *mIter; }
-
-            const value_type* operator->() const { return &(*mIter); }
-        };
-
-        using iterator = FilteredDialogueIterator;
-
-        const ESM::Dialogue* search(const ESM::RefId& id) const
-        {
-            return foundDialogueFilteredOut(mDialogueStore.search(id));
-        }
-
-        const ESM::Dialogue* at(size_t index) const
-        {
-            auto result = begin();
-            result += index;
-
-            if (result == end())
-            {
-                return nullptr;
-            }
-
-            return &(*result);
-        }
-
-        size_t getSize() const
-        {
-            return std::count_if(
-                mDialogueStore.begin(), mDialogueStore.end(), [](const auto& d) { return d.mType == filter; });
-        }
-
-        iterator begin() const
-        {
-            iterator result{ mDialogueStore.begin(), mDialogueStore.end() };
-            while (result != end() && result->mType != filter)
-            {
-                ++result;
-            }
         return result;
     }
 
-        iterator end() const { return iterator{ mDialogueStore.end(), mDialogueStore.end() }; }
+    template <ESM::Dialogue::Type type>
+    class FilteredDialogueStore
+    {
+        const MWWorld::Store<ESM::Dialogue>& mDialogueStore;
+        std::vector<const ESM::Dialogue*> mIndex;
+
+    public:
+        explicit FilteredDialogueStore(const MWWorld::Store<ESM::Dialogue>& store)
+            : mDialogueStore(store)
+            , mIndex{ makeIndex(store, type) }
+        {
+        }
+
+        const ESM::Dialogue* search(const ESM::RefId& id) const
+        {
+            const ESM::Dialogue* dialogue = mDialogueStore.search(id);
+            if (dialogue != nullptr && dialogue->mType == type)
+                return dialogue;
+            return nullptr;
+        }
+
+        const ESM::Dialogue* at(std::size_t index) const
+        {
+            if (index >= mIndex.size())
+                return nullptr;
+            return mIndex[index];
+        }
+
+        std::size_t getSize() const { return mIndex.size(); }
     };
 
     template <ESM::Dialogue::Type filter>
@@ -156,7 +79,7 @@ namespace
         storeBindingsClass[sol::meta_function::ipairs] = lua["ipairsForArray"].template get<sol::function>();
         storeBindingsClass[sol::meta_function::pairs] = lua["ipairsForArray"].template get<sol::function>();
 
-        table["records"] = StoreT{};
+        table["records"] = StoreT{ MWBase::Environment::get().getESMStore()->get<ESM::Dialogue>() };
     }
 
     struct DialogueInfos

apps/openmw/mwlua/dialoguebindings.hpp

@@ -6,7 +6,8 @@
 namespace MWLua
 {
     struct Context;
-    sol::table initCoreDialogueBindings(const Context&);
+
+    sol::table initCoreDialogueBindings(const Context& context);
 }
 
 #endif // MWLUA_DIALOGUEBINDINGS_H

scripts/data/morrowind_tests/builtin.omwscripts

@@ -1 +0,0 @@
-# It is an empty file that overrides builtin.omwscripts and disables builtin scripts

scripts/data/morrowind_tests/global.lua

@@ -0,0 +1,34 @@
+local testing = require('testing_util')
+local util = require('openmw.util')
+local world = require('openmw.world')
+local core = require('openmw.core')
+local types = require('openmw.types')
+
+if not core.contentFiles.has('Morrowind.esm') then
+    error('This test requires Morrowind.esm')
+end
+
+function makeTests(modules)
+    local tests = {}
+
+    for _, moduleName in ipairs(modules) do
+        local module = require(moduleName)
+        for _, v in ipairs(module) do
+            table.insert(tests, {string.format('[%s] %s', moduleName, v[1]), v[2]})
+        end
+    end
+
+    return tests
+end
+
+local testModules = {
+    'global_issues',
+    'global_dialogues',
+}
+
+return {
+    engineHandlers = {
+        onUpdate = testing.testRunner(makeTests(testModules)),
+    },
+    eventHandlers = testing.eventHandlers,
+}

scripts/data/morrowind_tests/global_dialogues.lua

@@ -0,0 +1,47 @@
+local testing = require('testing_util')
+local core = require('openmw.core')
+
+function iterateOverRecords(records)
+    local firstRecordId = nil
+    local lastRecordId = nil
+    local count = 0
+    for _, v in ipairs(records) do
+        firstRecordId = firstRecordId or v.id
+        lastRecordId = v.id
+        count = count + 1
+    end
+    return firstRecordId, lastRecordId, count
+end
+
+return {
+    {'Should support iteration over journal dialogues', function()
+        local firstRecordId, lastRecordId, count = iterateOverRecords(core.dialogue.journal.records)
+        testing.expectEqual(firstRecordId, '11111 test journal')
+        testing.expectEqual(lastRecordId, 'va_vamprich')
+        testing.expectEqual(count, 632)
+    end},
+    {'Should support iteration over topic dialogues', function()
+        local firstRecordId, lastRecordId, count = iterateOverRecords(core.dialogue.topic.records)
+        testing.expectEqual(firstRecordId, '1000-drake pledge')
+        testing.expectEqual(lastRecordId, 'zenithar')
+        testing.expectEqual(count, 1698)
+    end},
+    {'Should support iteration over greeting dialogues', function()
+        local firstRecordId, lastRecordId, count = iterateOverRecords(core.dialogue.greeting.records)
+        testing.expectEqual(firstRecordId, 'greeting 0')
+        testing.expectEqual(lastRecordId, 'greeting 9')
+        testing.expectEqual(count, 10)
+    end},
+    {'Should support iteration over persuasion dialogues', function()
+        local firstRecordId, lastRecordId, count = iterateOverRecords(core.dialogue.persuasion.records)
+        testing.expectEqual(firstRecordId, 'admire fail')
+        testing.expectEqual(lastRecordId, 'taunt success')
+        testing.expectEqual(count, 10)
+    end},
+    {'Should support iteration over voice dialogues', function()
+        local firstRecordId, lastRecordId, count = iterateOverRecords(core.dialogue.voice.records)
+        testing.expectEqual(firstRecordId, 'alarm')
+        testing.expectEqual(lastRecordId, 'thief')
+        testing.expectEqual(count, 8)
+    end},
+}

scripts/data/morrowind_tests/global_issues.lua

@@ -4,11 +4,7 @@ local world = require('openmw.world')
 local core = require('openmw.core')
 local types = require('openmw.types')
 
-if not core.contentFiles.has('Morrowind.esm') then
+return {
-    error('This test requires Morrowind.esm')
-end
-
-local tests = {
     {'Player should be able to walk up stairs in Ebonheart docks (#4247)', function()
         world.players[1]:teleport('', util.vector3(19867, -102180, -79), util.transform.rotateZ(math.rad(91)))
         coroutine.yield()
@@ -42,10 +38,3 @@ local tests = {
         testing.expectThat(types.Container.inventory(barrel):find('ring_keley'), isFargothRing)
     end},
 }
-
-return {
-    engineHandlers = {
-        onUpdate = testing.testRunner(tests),
-    },
-    eventHandlers = testing.eventHandlers,
-}

scripts/data/morrowind_tests/morrowind_tests.omwscripts

@@ -1,2 +1,2 @@
-GLOBAL: test.lua
+GLOBAL: global.lua
 PLAYER: player.lua

scripts/data/morrowind_tests/openmw.cfg

@@ -7,5 +7,5 @@ data-local=test_workdir
 data=../integration_tests/testing_util
 data=.
 content=Morrowind.esm
-content=test.omwscripts
+content=morrowind_tests.omwscripts
 fallback-archive=Morrowind.bsa

scripts/data/morrowind_tests/player.lua

@@ -78,7 +78,7 @@ testing.registerLocalTest('Guard in Imperial Prison Ship should find path (#7241
 
 return {
     engineHandlers = {
-        onUpdate = testing.updateLocal,
+        onFrame = testing.updateLocal,
     },
     eventHandlers = testing.eventHandlers
 }