Store original string id for Dialogue records

apps/esmtool/record.cpp

@@ -741,6 +741,7 @@ namespace EsmTool
     template <>
     void Record<ESM::Dialogue>::print()
     {
+        std::cout << "  StringId: " << mData.mStringId << std::endl;
         std::cout << "  Type: " << dialogTypeLabel(mData.mType) << " (" << (int)mData.mType << ")" << std::endl;
         std::cout << "  Deleted: " << mIsDeleted << std::endl;
         // Sadly, there are no DialInfos, because the loader dumps as it

apps/opencs/model/doc/document.cpp

@@ -226,6 +226,7 @@ void CSMDoc::Document::createBase()
     {
         ESM::Dialogue record;
         record.mId = ESM::RefId::stringRefId(voice);
+        record.mStringId = voice;
         record.mType = ESM::Dialogue::Voice;
         record.blank();
 
@@ -249,6 +250,7 @@ void CSMDoc::Document::createBase()
     {
         ESM::Dialogue record;
         record.mId = ESM::RefId::stringRefId(greeting);
+        record.mStringId = greeting;
         record.mType = ESM::Dialogue::Greeting;
         record.blank();
 
@@ -272,6 +274,7 @@ void CSMDoc::Document::createBase()
     {
         ESM::Dialogue record;
         record.mId = ESM::RefId::stringRefId(persuasion);
+        record.mStringId = persuasion;
         record.mType = ESM::Dialogue::Persuasion;
         record.blank();
 

apps/opencs/model/world/collection.hpp

@@ -14,6 +14,7 @@
 
 #include <QVariant>
 
+#include <components/esm3/loaddial.hpp>
 #include <components/misc/strings/lower.hpp>
 
 #include "collectionbase.hpp"
@@ -278,6 +279,11 @@ namespace CSMWorld
             }
         }
 
+        if constexpr (std::is_same_v<ESXRecordT, ESM::Dialogue>)
+        {
+            copy->mModified.mStringId = copy->mModified.mId.getRefIdString();
+        }
+
         const int index = getAppendIndex(destination, type);
         insertRecord(std::move(copy), getAppendIndex(destination, type));
 
@@ -489,6 +495,11 @@ namespace CSMWorld
         setRecordId(id, record);
         record.blank();
 
+        if constexpr (std::is_same_v<ESXRecordT, ESM::Dialogue>)
+        {
+            record.mStringId = record.mId.getRefIdString();
+        }
+
         auto record2 = std::make_unique<Record<ESXRecordT>>();
         record2->mState = Record<ESXRecordT>::State_ModifiedOnly;
         record2->mModified = record;

apps/opencs_tests/model/world/testinfocollection.cpp

@@ -41,12 +41,13 @@ namespace CSMWorld
         };
 
         DialogueData<ESM::DialInfo> generateDialogueWithInfos(
-            std::size_t infoCount, const ESM::RefId& dialogueId = ESM::RefId::stringRefId("dialogue"))
+            std::size_t infoCount, std::string_view dialogueId = "dialogue")
         {
             DialogueData<ESM::DialInfo> result;
 
             result.mDialogue.blank();
-            result.mDialogue.mId = dialogueId;
+            result.mDialogue.mId = ESM::RefId::stringRefId(dialogueId);
+            result.mDialogue.mStringId = dialogueId;
 
             for (std::size_t i = 0; i < infoCount; ++i)
             {
@@ -133,6 +134,7 @@ namespace CSMWorld
             ESM::Dialogue dialogue;
             dialogue.blank();
             dialogue.mId = ESM::RefId::stringRefId("dialogue");
+            dialogue.mStringId = "Dialogue";
 
             ESM::DialInfo info;
             info.blank();
@@ -157,6 +159,7 @@ namespace CSMWorld
             ESM::Dialogue dialogue;
             dialogue.blank();
             dialogue.mId = ESM::RefId::stringRefId("dialogue");
+            dialogue.mStringId = "Dialogue";
 
             ESM::DialInfo info;
             info.blank();
@@ -181,6 +184,7 @@ namespace CSMWorld
             ESM::Dialogue dialogue;
             dialogue.blank();
             dialogue.mId = ESM::RefId::stringRefId("dialogue");
+            dialogue.mStringId = "Dialogue";
 
             ESM::DialInfo info;
             info.blank();
@@ -207,6 +211,7 @@ namespace CSMWorld
             ESM::Dialogue dialogue;
             dialogue.blank();
             dialogue.mId = ESM::RefId::stringRefId("dialogue");
+            dialogue.mStringId = "Dialogue";
 
             ESM::DialInfo info;
             info.blank();
@@ -233,6 +238,7 @@ namespace CSMWorld
             ESM::Dialogue dialogue;
             dialogue.blank();
             dialogue.mId = ESM::RefId::stringRefId("dialogue");
+            dialogue.mStringId = "Dialogue";
 
             DialInfoData info;
             info.mValue.blank();
@@ -252,6 +258,7 @@ namespace CSMWorld
             ESM::Dialogue dialogue;
             dialogue.blank();
             dialogue.mId = ESM::RefId::stringRefId("dialogue");
+            dialogue.mStringId = "Dialogue";
 
             DialInfoData info;
             info.mValue.blank();
@@ -275,6 +282,7 @@ namespace CSMWorld
             ESM::Dialogue dialogue;
             dialogue.blank();
             dialogue.mId = ESM::RefId::stringRefId("dialogue");
+            dialogue.mStringId = "Dialogue";
 
             DialInfoData info;
             info.mValue.blank();
@@ -535,12 +543,9 @@ namespace CSMWorld
             InfoOrderByTopic infoOrder;
             InfoCollection collection;
 
-            saveAndLoadDialogueWithInfos(
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue2"), base, collection, infoOrder);
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue2")), base, collection, infoOrder);
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue0"), base, collection, infoOrder);
-            saveAndLoadDialogueWithInfos(
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue1"), base, collection, infoOrder);
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder);
-            saveAndLoadDialogueWithInfos(
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder);
 
             collection.sort(infoOrder);
 
@@ -558,10 +563,8 @@ namespace CSMWorld
             InfoOrderByTopic infoOrder;
             InfoCollection collection;
 
-            saveAndLoadDialogueWithInfos(
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue0"), base, collection, infoOrder);
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder);
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue1"), base, collection, infoOrder);
-            saveAndLoadDialogueWithInfos(
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder);
 
             collection.sort(infoOrder);
 
@@ -574,10 +577,8 @@ namespace CSMWorld
             InfoOrderByTopic infoOrder;
             InfoCollection collection;
 
-            saveAndLoadDialogueWithInfos(
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue0"), base, collection, infoOrder);
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder);
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue1"), base, collection, infoOrder);
-            saveAndLoadDialogueWithInfos(
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder);
 
             EXPECT_FALSE(collection.reorderRows(5, {}));
         }
@@ -588,10 +589,8 @@ namespace CSMWorld
             InfoOrderByTopic infoOrder;
             InfoCollection collection;
 
-            saveAndLoadDialogueWithInfos(
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue0"), base, collection, infoOrder);
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder);
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue1"), base, collection, infoOrder);
-            saveAndLoadDialogueWithInfos(
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder);
 
             EXPECT_FALSE(collection.reorderRows(0, { 0, 1, 2 }));
         }
@@ -602,10 +601,8 @@ namespace CSMWorld
             InfoOrderByTopic infoOrder;
             InfoCollection collection;
 
-            saveAndLoadDialogueWithInfos(
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(3, "dialogue0"), base, collection, infoOrder);
-                generateDialogueWithInfos(3, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder);
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(3, "dialogue1"), base, collection, infoOrder);
-            saveAndLoadDialogueWithInfos(
-                generateDialogueWithInfos(3, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder);
 
             EXPECT_EQ(collection.searchId(ESM::RefId::stringRefId("dialogue0#info0")), 0);
             EXPECT_EQ(collection.searchId(ESM::RefId::stringRefId("dialogue0#info1")), 1);
@@ -629,10 +626,8 @@ namespace CSMWorld
             InfoOrderByTopic infoOrder;
             InfoCollection collection;
 
-            saveAndLoadDialogueWithInfos(
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "d0"), base, collection, infoOrder);
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("d0")), base, collection, infoOrder);
+            saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "d1"), base, collection, infoOrder);
-            saveAndLoadDialogueWithInfos(
-                generateDialogueWithInfos(2, ESM::RefId::stringRefId("d1")), base, collection, infoOrder);
 
             collection.sort(infoOrder);
 

apps/openmw/mwdialogue/dialoguemanagerimp.cpp

@@ -297,7 +297,7 @@ namespace MWDialogue
 
         if (info)
         {
-            ESM::RefId title;
+            std::string title;
             if (dialogue.mType == ESM::Dialogue::Persuasion)
             {
                 // Determine GMST from dialogue topic. GMSTs are:
@@ -310,14 +310,13 @@ namespace MWDialogue
                 const MWWorld::Store<ESM::GameSetting>& gmsts
                     = MWBase::Environment::get().getWorld()->getStore().get<ESM::GameSetting>();
 
-                title = ESM::RefId::stringRefId(gmsts.find(modifiedTopic)->mValue.getString());
+                title = gmsts.find(modifiedTopic)->mValue.getString();
             }
             else
-                title = topic;
+                title = dialogue.mStringId;
 
             MWScript::InterpreterContext interpreterContext(&mActor.getRefData().getLocals(), mActor);
-            callback->addResponse(
+            callback->addResponse(title, Interpreter::fixDefinesDialog(info->mResponse, interpreterContext));
-                title.getRefIdString(), Interpreter::fixDefinesDialog(info->mResponse, interpreterContext));
 
             if (dialogue.mType == ESM::Dialogue::Topic)
             {

apps/openmw_test_suite/esm3/testsaveload.cpp

@@ -2,6 +2,7 @@
 #include <components/esm3/esmreader.hpp>
 #include <components/esm3/esmwriter.hpp>
 #include <components/esm3/loadcont.hpp>
+#include <components/esm3/loaddial.hpp>
 #include <components/esm3/loadregn.hpp>
 #include <components/esm3/loadscpt.hpp>
 #include <components/esm3/player.hpp>
@@ -332,6 +333,18 @@ namespace ESM
             EXPECT_EQ(result.mKeys, record.mKeys);
         }
 
+        TEST_P(Esm3SaveLoadRecordTest, dialogueShouldNotChange)
+        {
+            Dialogue record;
+            record.blank();
+            record.mStringId = generateRandomString(32);
+            record.mId = ESM::RefId::stringRefId(record.mStringId);
+            Dialogue result;
+            saveAndLoadRecord(record, GetParam(), result);
+            EXPECT_EQ(result.mId, record.mId);
+            EXPECT_EQ(result.mStringId, record.mStringId);
+        }
+
         INSTANTIATE_TEST_SUITE_P(FormatVersions, Esm3SaveLoadRecordTest, ValuesIn(getFormats()));
     }
 }

apps/openmw_test_suite/mwworld/test_store.cpp

@@ -441,11 +441,12 @@ namespace
         using RecordType = TypeParam;
 
         const int index = 3;
+        const std::string stringId = "foobar";
         decltype(RecordType::mId) refId;
         if constexpr (ESM::hasIndex<RecordType> && !std::is_same_v<RecordType, ESM::LandTexture>)
             refId = RecordType::indexToRefId(index);
         else
-            refId = ESM::StringRefId("foobar");
+            refId = ESM::StringRefId(stringId);
 
         for (const ESM::FormatVersion formatVersion : getFormats())
         {
@@ -458,6 +459,9 @@ namespace
 
             record.mId = refId;
 
+            if constexpr (ESM::hasStringId<RecordType>)
+                record.mStringId = stringId;
+
             if constexpr (ESM::hasIndex<RecordType>)
                 record.mIndex = index;
 
@@ -486,6 +490,7 @@ namespace
     }
 
     static_assert(ESM::hasIndex<ESM::MagicEffect>);
+    static_assert(ESM::hasStringId<ESM::Dialogue>);
 
     template <class T, class = std::void_t<>>
     struct HasSaveFunction : std::false_type
@@ -593,6 +598,7 @@ namespace
 
         result.mDialogue.blank();
         result.mDialogue.mId = ESM::RefId::stringRefId("dialogue");
+        result.mDialogue.mStringId = "Dialogue";
 
         for (std::size_t i = 0; i < infoCount; ++i)
         {

components/esm3/formatversion.hpp

@@ -22,7 +22,8 @@ namespace ESM
     inline constexpr FormatVersion MaxLimitedSizeStringsFormatVersion = 22;
     inline constexpr FormatVersion MaxStringRefIdFormatVersion = 23;
     inline constexpr FormatVersion MaxSavedGameCellNameAsRefIdFormatVersion = 24;
-    inline constexpr FormatVersion CurrentSaveGameFormatVersion = 25;
+    inline constexpr FormatVersion MaxNameIsRefIdOnlyFormatVersion = 25;
+    inline constexpr FormatVersion CurrentSaveGameFormatVersion = 26;
 }
 
 #endif

components/esm3/loaddial.cpp

@@ -3,6 +3,8 @@
 #include "esmreader.hpp"
 #include "esmwriter.hpp"
 
+#include <stdexcept>
+
 namespace ESM
 {
 
@@ -14,7 +16,20 @@ namespace ESM
 
     void Dialogue::loadId(ESMReader& esm)
     {
-        mId = esm.getHNRefId("NAME");
+        if (esm.getFormatVersion() <= MaxStringRefIdFormatVersion)
+        {
+            mStringId = esm.getHNString("NAME");
+            mId = ESM::RefId::stringRefId(mStringId);
+            return;
+        }
+
+        if (esm.getFormatVersion() <= MaxNameIsRefIdOnlyFormatVersion)
+        {
+            mId = esm.getHNRefId("NAME");
+            return;
+        }
+
+        mId = esm.getHNRefId("ID__");
     }
 
     void Dialogue::loadData(ESMReader& esm, bool& isDeleted)
@@ -46,22 +61,42 @@ namespace ESM
                     mType = Unknown;
                     isDeleted = true;
                     break;
+                case SREC_NAME:
+                    mStringId = esm.getHString();
+                    break;
                 default:
                     esm.fail("Unknown subrecord");
                     break;
             }
         }
+
+        if (!isDeleted && MaxStringRefIdFormatVersion < esm.getFormatVersion()
+            && esm.getFormatVersion() <= MaxNameIsRefIdOnlyFormatVersion)
+            mStringId = mId.toString();
     }
 
     void Dialogue::save(ESMWriter& esm, bool isDeleted) const
     {
-        esm.writeHNCRefId("NAME", mId);
+        if (esm.getFormatVersion() <= MaxStringRefIdFormatVersion)
+        {
+            if (mId != mStringId)
+                throw std::runtime_error("Trying to save Dialogue record with name \"" + mStringId
+                    + "\" not maching id " + mId.toDebugString());
+            esm.writeHNString("NAME", mStringId);
+        }
+        else if (esm.getFormatVersion() <= MaxNameIsRefIdOnlyFormatVersion)
+            esm.writeHNRefId("NAME", mId);
+        else
+            esm.writeHNRefId("ID__", mId);
+
         if (isDeleted)
         {
             esm.writeHNString("DELE", "", 3);
         }
         else
         {
+            if (esm.getFormatVersion() > MaxNameIsRefIdOnlyFormatVersion)
+                esm.writeHNString("NAME", mStringId);
             esm.writeHNT("DATA", mType);
         }
     }

components/esm3/loaddial.hpp

@@ -41,6 +41,7 @@ namespace ESM
         };
 
         RefId mId;
+        std::string mStringId;
         Type mType;
         InfoContainer mInfo;
         InfoOrder<DialInfo> mInfoOrder;

components/esm3/typetraits.hpp

@@ -17,6 +17,19 @@ namespace ESM
 
     template <class T>
     inline constexpr bool hasIndex = HasIndex<T>::value;
+
+    template <class T, class = std::void_t<>>
+    struct HasStringId : std::false_type
+    {
+    };
+
+    template <class T>
+    struct HasStringId<T, std::void_t<decltype(T::mStringId)>> : std::true_type
+    {
+    };
+
+    template <class T>
+    inline constexpr bool hasStringId = HasStringId<T>::value;
 }
 
 #endif // OPENMW_COMPONENTS_ESM3_TYPETRAITS