Use common info ordering implementation for engine and editor

apps/openmw/mwworld/esmstore.cpp

@@ -318,7 +318,7 @@ namespace MWWorld
                 {
                     if (dialogue)
                     {
-                        dialogue->readInfo(esm, esm.getIndex() != 0);
+                        dialogue->readInfo(esm);
                     }
                     else
                     {

apps/openmw/mwworld/store.cpp

@@ -1070,7 +1070,7 @@ namespace MWWorld
         // DialInfos marked as deleted are kept during the loading phase, so that the linked list
         // structure is kept intact for inserting further INFOs. Delete them now that loading is done.
         for (auto& [_, dial] : mStatic)
-            dial.clearDeletedInfos();
+            dial.setUp();
 
         mShared.clear();
         mShared.reserve(mStatic.size());

apps/openmw_test_suite/mwworld/test_store.cpp

@@ -671,7 +671,7 @@ namespace
         EXPECT_THAT(dialogue->mInfo, ElementsAre(HasIdEqualTo("info0"), HasIdEqualTo("info1"), HasIdEqualTo("info2")));
     }
 
-    TEST(MWWorldStoreTest, shouldLoadDialogueWithInfosAsIsWhenReversed)
+    TEST(MWWorldStoreTest, shouldLoadDialogueWithInfosAndOrderWhenReversed)
     {
         DialogueData data = generateDialogueWithInfos(3);
 
@@ -683,7 +683,7 @@ namespace
 
         const ESM::Dialogue* dialogue = esmStore.get<ESM::Dialogue>().search(ESM::RefId::stringRefId("dialogue"));
         ASSERT_NE(dialogue, nullptr);
-        EXPECT_THAT(dialogue->mInfo, ElementsAre(HasIdEqualTo("info2"), HasIdEqualTo("info1"), HasIdEqualTo("info0")));
+        EXPECT_THAT(dialogue->mInfo, ElementsAre(HasIdEqualTo("info0"), HasIdEqualTo("info1"), HasIdEqualTo("info2")));
     }
 
     TEST(MWWorldStoreTest, shouldLoadDialogueWithInfosInsertingNewRecordBasedOnPrev)

components/esm3/infoorder.cpp

@@ -1,87 +0,0 @@
-#include "infoorder.hpp"
-
-namespace ESM
-{
-    const std::list<OrderedInfo>* InfoOrder::findInfosByTopic(const ESM::RefId& refId) const
-    {
-        const auto it = mOrderByTopic.find(refId);
-        if (it == mOrderByTopic.end())
-            return nullptr;
-        return &it->second;
-    }
-
-    void InfoOrder::insertInfo(const OrderedInfo& info)
-    {
-        auto it = mInfoPositions.find(info.mId);
-
-        if (it != mInfoPositions.end() && it->second->mPrev == info.mPrev)
-        {
-            it->second->mNext = info.mNext;
-            return;
-        }
-
-        auto& infos = mOrderByTopic[info.mTopicId];
-
-        if (it == mInfoPositions.end())
-            it = mInfoPositions.emplace(info.mId, infos.end()).first;
-
-        std::list<OrderedInfo>::iterator& position = it->second;
-
-        const auto insertOrSplice = [&](std::list<OrderedInfo>::const_iterator before) {
-            if (position == infos.end())
-                position = infos.insert(before, info);
-            else
-                infos.splice(before, infos, position);
-        };
-
-        if (info.mPrev.empty())
-        {
-            insertOrSplice(infos.begin());
-            return;
-        }
-
-        const auto prevIt = mInfoPositions.find(info.mPrev);
-        if (prevIt != mInfoPositions.end())
-        {
-            insertOrSplice(std::next(prevIt->second));
-            return;
-        }
-
-        const auto nextIt = mInfoPositions.find(info.mNext);
-        if (nextIt != mInfoPositions.end())
-        {
-            insertOrSplice(nextIt->second);
-            return;
-        }
-
-        insertOrSplice(infos.end());
-    }
-
-    void InfoOrder::removeInfo(const ESM::RefId& infoRefId)
-    {
-        const auto it = mInfoPositions.find(infoRefId);
-
-        if (it == mInfoPositions.end())
-            return;
-
-        const auto topicIt = mOrderByTopic.find(it->second->mTopicId);
-
-        if (topicIt != mOrderByTopic.end())
-            topicIt->second.erase(it->second);
-
-        mInfoPositions.erase(it);
-    }
-
-    void InfoOrder::removeTopic(const ESM::RefId& topicRefId)
-    {
-        const auto it = mOrderByTopic.find(topicRefId);
-
-        if (it == mOrderByTopic.end())
-            return;
-
-        for (const OrderedInfo& info : it->second)
-            mInfoPositions.erase(info.mId);
-
-        mOrderByTopic.erase(it);
-    }
-}

components/esm3/loaddial.cpp

@@ -70,62 +70,17 @@ namespace ESM
         mInfo.clear();
     }
 
-    void Dialogue::readInfo(ESMReader& esm, bool merge)
+    void Dialogue::readInfo(ESMReader& esm)
     {
         DialInfo info;
         bool isDeleted = false;
         info.load(esm, isDeleted);
-
+        mInfoOrder.insertInfo(std::move(info), isDeleted);
-        if (!merge || mInfo.empty())
-        {
-            mLookup[info.mId] = std::make_pair(mInfo.insert(mInfo.end(), info), isDeleted);
-            return;
     }
 
-        LookupMap::iterator lookup = mLookup.find(info.mId);
+    void Dialogue::setUp()
-
-        if (lookup != mLookup.end())
     {
-            auto it = lookup->second.first;
+        mInfoOrder.removeDeleted();
-            if (it->mPrev == info.mPrev)
+        mInfoOrder.extractOrderedInfo(mInfo);
-            {
-                *it = info;
-                lookup->second.second = isDeleted;
-                return;
-            }
-            // Since the new version of this record has a different prev linked list connection, we need to re-insert
-            // the record
-            mInfo.erase(it);
-            mLookup.erase(lookup);
-        }
-
-        if (!info.mPrev.empty())
-        {
-            lookup = mLookup.find(info.mPrev);
-            if (lookup != mLookup.end())
-            {
-                auto it = lookup->second.first;
-
-                mLookup[info.mId] = std::make_pair(mInfo.insert(++it, info), isDeleted);
-            }
-            else
-                mLookup[info.mId] = std::make_pair(mInfo.insert(mInfo.end(), info), isDeleted);
-        }
-        else
-            mLookup[info.mId] = std::make_pair(mInfo.insert(mInfo.begin(), info), isDeleted);
-    }
-
-    void Dialogue::clearDeletedInfos()
-    {
-        LookupMap::const_iterator current = mLookup.begin();
-        LookupMap::const_iterator end = mLookup.end();
-        for (; current != end; ++current)
-        {
-            if (current->second.second)
-            {
-                mInfo.erase(current->second.first);
-            }
-        }
-        mLookup.clear();
     }
 }

components/esm3/loaddial.hpp

@@ -7,6 +7,7 @@
 
 #include "components/esm/defs.hpp"
 #include "components/esm/refid.hpp"
+#include "components/esm3/infoorder.hpp"
 
 #include "loadinfo.hpp"
 
@@ -23,6 +24,8 @@ namespace ESM
 
     struct Dialogue
     {
+        using InfoContainer = std::list<DialInfo>;
+
         constexpr static RecNameInts sRecordId = REC_DIAL;
         /// Return a string descriptor for this record type. Currently used for debugging / error logs only.
         static std::string_view getRecordType() { return "Dialogue"; }
@@ -39,17 +42,12 @@ namespace ESM
 
         RefId mId;
         signed char mType;
-
+        InfoContainer mInfo;
-        typedef std::list<DialInfo> InfoContainer;
+        InfoOrder<DialInfo> mInfoOrder;
 
         // Parameters: Info ID, (Info iterator, Deleted flag)
         typedef std::map<ESM::RefId, std::pair<InfoContainer::iterator, bool>> LookupMap;
 
-        InfoContainer mInfo;
-
-        // This is only used during the loading phase to speed up DialInfo merging.
-        LookupMap mLookup;
-
         void load(ESMReader& esm, bool& isDeleted);
         ///< Loads all sub-records of Dialogue record
         void loadId(ESMReader& esm);
@@ -60,11 +58,10 @@ namespace ESM
         void save(ESMWriter& esm, bool isDeleted = false) const;
 
         /// Remove all INFOs that are deleted
-        void clearDeletedInfos();
+        void setUp();
 
         /// Read the next info record
-        /// @param merge Merge with existing list, or just push each record to the end of the list?
+        void readInfo(ESMReader& esm);
-        void readInfo(ESMReader& esm, bool merge);
 
         void blank();
         ///< Set record to default state (does not touch the ID and does not change the type).