From d5954aba68873b3b955c34d1481a226e21bfb17c Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 26 Mar 2023 13:23:19 +0200 Subject: [PATCH 1/7] Add suffix to the format version name --- components/esm3/formatversion.hpp | 2 +- components/esm3/savedgame.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/esm3/formatversion.hpp b/components/esm3/formatversion.hpp index 2824ced3ec..04fcb7e8a6 100644 --- a/components/esm3/formatversion.hpp +++ b/components/esm3/formatversion.hpp @@ -21,7 +21,7 @@ namespace ESM inline constexpr FormatVersion MaxOldCreatureStatsFormatVersion = 19; inline constexpr FormatVersion MaxLimitedSizeStringsFormatVersion = 22; inline constexpr FormatVersion MaxStringRefIdFormatVersion = 23; - inline constexpr FormatVersion MaxSavedGameCellNameAsRefId = 24; + inline constexpr FormatVersion MaxSavedGameCellNameAsRefIdFormatVersion = 24; inline constexpr FormatVersion CurrentSaveGameFormatVersion = 25; } diff --git a/components/esm3/savedgame.cpp b/components/esm3/savedgame.cpp index 1cf0ea033c..6652179ea9 100644 --- a/components/esm3/savedgame.cpp +++ b/components/esm3/savedgame.cpp @@ -13,7 +13,7 @@ namespace ESM mPlayerClassId = esm.getHNORefId("PLCL"); mPlayerClassName = esm.getHNOString("PLCN"); - if (esm.getFormatVersion() <= ESM::MaxSavedGameCellNameAsRefId) + if (esm.getFormatVersion() <= ESM::MaxSavedGameCellNameAsRefIdFormatVersion) mPlayerCellName = esm.getHNRefId("PLCE").toString(); else mPlayerCellName = esm.getHNString("PLCE"); From e1f580e7a0c30dcbf67abf4e57051e1fe24e2e58 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 26 Mar 2023 13:44:10 +0200 Subject: [PATCH 2/7] Use static constexpr string_view for hardcoded ids --- apps/opencs/model/doc/document.cpp | 35 +++++++++++++----------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/apps/opencs/model/doc/document.cpp b/apps/opencs/model/doc/document.cpp index f46fb7be97..634e6728f7 100644 --- a/apps/opencs/model/doc/document.cpp +++ b/apps/opencs/model/doc/document.cpp @@ -107,17 +107,16 @@ void CSMDoc::Document::addOptionalGmsts() void CSMDoc::Document::addOptionalGlobals() { - static const char* sGlobals[] = { + static constexpr std::string_view globals[] = { "DaysPassed", "PCWerewolf", "PCYear", - nullptr, }; - for (int i = 0; sGlobals[i]; ++i) + for (std::size_t i = 0; i < std::size(globals); ++i) { ESM::Global global; - global.mId = ESM::RefId::stringRefId(sGlobals[i]); + global.mId = ESM::RefId::stringRefId(globals[i]); global.blank(); global.mValue.setType(ESM::VT_Long); @@ -176,7 +175,7 @@ void CSMDoc::Document::addOptionalMagicEffect(const ESM::MagicEffect& magicEffec void CSMDoc::Document::createBase() { - static const char* sGlobals[] = { + static constexpr std::string_view globals[] = { "Day", "DaysPassed", "GameHour", @@ -185,13 +184,12 @@ void CSMDoc::Document::createBase() "PCVampire", "PCWerewolf", "PCYear", - nullptr, }; - for (int i = 0; sGlobals[i]; ++i) + for (std::size_t i = 0; i < std::size(globals); ++i) { ESM::Global record; - record.mId = ESM::RefId::stringRefId(sGlobals[i]); + record.mId = ESM::RefId::stringRefId(globals[i]); record.mRecordFlags = 0; record.mValue.setType(i == 2 ? ESM::VT_Float : ESM::VT_Long); @@ -213,7 +211,7 @@ void CSMDoc::Document::createBase() getData().getSkills().add(record); } - static const char* sVoice[] = { + static constexpr std::string_view voices[] = { "Intruder", "Attack", "Hello", @@ -222,20 +220,19 @@ void CSMDoc::Document::createBase() "Idle", "Flee", "Hit", - nullptr, }; - for (int i = 0; sVoice[i]; ++i) + for (const std::string_view voice : voices) { ESM::Dialogue record; - record.mId = ESM::RefId::stringRefId(sVoice[i]); + record.mId = ESM::RefId::stringRefId(voice); record.mType = ESM::Dialogue::Voice; record.blank(); getData().getTopics().add(record); } - static const char* sGreetings[] = { + static constexpr std::string_view greetings[] = { "Greeting 0", "Greeting 1", "Greeting 2", @@ -246,20 +243,19 @@ void CSMDoc::Document::createBase() "Greeting 7", "Greeting 8", "Greeting 9", - nullptr, }; - for (int i = 0; sGreetings[i]; ++i) + for (const std::string_view greeting : greetings) { ESM::Dialogue record; - record.mId = ESM::RefId::stringRefId(sGreetings[i]); + record.mId = ESM::RefId::stringRefId(greeting); record.mType = ESM::Dialogue::Greeting; record.blank(); getData().getTopics().add(record); } - static const char* sPersuasion[] = { + static constexpr std::string_view persuasions[] = { "Intimidate Success", "Intimidate Fail", "Service Refusal", @@ -270,13 +266,12 @@ void CSMDoc::Document::createBase() "Admire Fail", "Taunt Fail", "Bribe Fail", - nullptr, }; - for (int i = 0; sPersuasion[i]; ++i) + for (const std::string_view persuasion : persuasions) { ESM::Dialogue record; - record.mId = ESM::RefId::stringRefId(sPersuasion[i]); + record.mId = ESM::RefId::stringRefId(persuasion); record.mType = ESM::Dialogue::Persuasion; record.blank(); From 90ed24f4c9b8bc9e005310c4fdcc7b55f180bc30 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 26 Mar 2023 15:19:24 +0200 Subject: [PATCH 3/7] Split type traits for ESM4, ESM3 and unite common --- apps/esmtool/tes4.cpp | 5 +-- apps/openmw_test_suite/mwworld/test_store.cpp | 28 +++++---------- components/esm/typetraits.hpp | 35 +++++++++++++++++++ components/esm3/typetraits.hpp | 22 ++++++++++++ components/esm4/typetraits.hpp | 26 -------------- 5 files changed, 68 insertions(+), 48 deletions(-) create mode 100644 components/esm/typetraits.hpp create mode 100644 components/esm3/typetraits.hpp diff --git a/apps/esmtool/tes4.cpp b/apps/esmtool/tes4.cpp index 6b13aa705c..25054b614a 100644 --- a/apps/esmtool/tes4.cpp +++ b/apps/esmtool/tes4.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -93,7 +94,7 @@ namespace EsmTool std::cout << "\n Record: " << ESM::NAME(reader.hdr().record.typeId).toStringView(); if constexpr (ESM4::hasFormId) std::cout << "\n FormId: " << value.mFormId; - if constexpr (ESM4::hasId) + if constexpr (ESM::hasId) std::cout << "\n Id: " << value.mId; if constexpr (ESM4::hasFlags) std::cout << "\n Record flags: " << recordFlags(value.mFlags); @@ -103,7 +104,7 @@ namespace EsmTool std::cout << "\n Parent: " << value.mParent; if constexpr (ESM4::hasEditorId) std::cout << "\n EditorId: " << value.mEditorId; - if constexpr (ESM4::hasModel) + if constexpr (ESM::hasModel) std::cout << "\n Model: " << value.mModel; if constexpr (ESM4::hasNif) std::cout << "\n Nif:" << WriteArray("\n - ", value.mNif); diff --git a/apps/openmw_test_suite/mwworld/test_store.cpp b/apps/openmw_test_suite/mwworld/test_store.cpp index a275c09bc3..268bc8bc0b 100644 --- a/apps/openmw_test_suite/mwworld/test_store.cpp +++ b/apps/openmw_test_suite/mwworld/test_store.cpp @@ -11,8 +11,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -20,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -428,19 +429,6 @@ namespace { }; - template > - struct HasIndex : std::false_type - { - }; - - template - struct HasIndex> : std::true_type - { - }; - - template - constexpr bool hasIndex = HasIndex::value; - TYPED_TEST_SUITE_P(StoreSaveLoadTest); TYPED_TEST_P(StoreSaveLoadTest, shouldNotChangeRefId) @@ -449,7 +437,7 @@ namespace const int index = 3; decltype(RecordType::mId) refId; - if constexpr (hasIndex && !std::is_same_v) + if constexpr (ESM::hasIndex && !std::is_same_v) refId = RecordType::indexToRefId(index); else refId = ESM::StringRefId("foobar"); @@ -465,7 +453,7 @@ namespace record.mId = refId; - if constexpr (hasIndex) + if constexpr (ESM::hasIndex) record.mIndex = index; if constexpr (std::is_same_v) @@ -482,7 +470,7 @@ namespace const RecordType* result = nullptr; if constexpr (std::is_same_v) result = esmStore.get().search(index, 0); - else if constexpr (hasIndex) + else if constexpr (ESM::hasIndex) result = esmStore.get().search(index); else result = esmStore.get().search(refId); @@ -492,7 +480,7 @@ namespace } } - static_assert(hasIndex); + static_assert(ESM::hasIndex); template > struct HasSaveFunction : std::false_type @@ -558,9 +546,9 @@ namespace }; using RecordTypes = typename ToRecordTypes::Type; - using RecordTypesWithId = typename FilterTypes::Type; + using RecordTypesWithId = typename FilterTypes::Type; using RecordTypesWithSave = typename FilterTypes::Type; - using RecordTypesWithModel = typename FilterTypes::Type; + using RecordTypesWithModel = typename FilterTypes::Type; REGISTER_TYPED_TEST_SUITE_P(StoreSaveLoadTest, shouldNotChangeRefId); diff --git a/components/esm/typetraits.hpp b/components/esm/typetraits.hpp new file mode 100644 index 0000000000..89f9e48695 --- /dev/null +++ b/components/esm/typetraits.hpp @@ -0,0 +1,35 @@ +#ifndef OPENMW_COMPONENTS_ESM_TYPETRAITS +#define OPENMW_COMPONENTS_ESM_TYPETRAITS + +#include + +namespace ESM +{ + template > + struct HasId : std::false_type + { + }; + + template + struct HasId> : std::true_type + { + }; + + template + inline constexpr bool hasId = HasId::value; + + template > + struct HasModel : std::false_type + { + }; + + template + struct HasModel> : std::true_type + { + }; + + template + inline constexpr bool hasModel = HasModel::value; +} + +#endif // OPENMW_COMPONENTS_ESM_TYPETRAITS diff --git a/components/esm3/typetraits.hpp b/components/esm3/typetraits.hpp new file mode 100644 index 0000000000..04529f3088 --- /dev/null +++ b/components/esm3/typetraits.hpp @@ -0,0 +1,22 @@ +#ifndef OPENMW_COMPONENTS_ESM3_TYPETRAITS +#define OPENMW_COMPONENTS_ESM3_TYPETRAITS + +#include + +namespace ESM +{ + template > + struct HasIndex : std::false_type + { + }; + + template + struct HasIndex> : std::true_type + { + }; + + template + inline constexpr bool hasIndex = HasIndex::value; +} + +#endif // OPENMW_COMPONENTS_ESM3_TYPETRAITS diff --git a/components/esm4/typetraits.hpp b/components/esm4/typetraits.hpp index 95e6f607e6..1d39a23018 100644 --- a/components/esm4/typetraits.hpp +++ b/components/esm4/typetraits.hpp @@ -18,19 +18,6 @@ namespace ESM4 template inline constexpr bool hasFormId = HasFormId::value; - template > - struct HasId : std::false_type - { - }; - - template - struct HasId> : std::true_type - { - }; - - template - inline constexpr bool hasId = HasId::value; - template > struct HasParentFormId : std::false_type { @@ -83,19 +70,6 @@ namespace ESM4 template inline constexpr bool hasEditorId = HasEditorId::value; - template > - struct HasModel : std::false_type - { - }; - - template - struct HasModel> : std::true_type - { - }; - - template - inline constexpr bool hasModel = HasModel::value; - template > struct HasNif : std::false_type { From d6c8c54dc547b1af33414efc48d49d7baad4648a Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 26 Mar 2023 15:59:45 +0200 Subject: [PATCH 4/7] Generate test cases for all ESM3 format versions since MaxStringRefIdFormatVersion --- apps/openmw_test_suite/esm3/testsaveload.cpp | 17 +++++--- apps/openmw_test_suite/mwworld/test_store.cpp | 43 +++++++++++-------- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/apps/openmw_test_suite/esm3/testsaveload.cpp b/apps/openmw_test_suite/esm3/testsaveload.cpp index ab08c72e4d..09b5b11ed4 100644 --- a/apps/openmw_test_suite/esm3/testsaveload.cpp +++ b/apps/openmw_test_suite/esm3/testsaveload.cpp @@ -71,11 +71,16 @@ namespace ESM { using namespace ::testing; - constexpr std::array formats = { - MaxLimitedSizeStringsFormatVersion, - MaxStringRefIdFormatVersion, - CurrentSaveGameFormatVersion, - }; + std::vector getFormats() + { + std::vector result({ + MaxLimitedSizeStringsFormatVersion, + MaxStringRefIdFormatVersion, + }); + for (ESM::FormatVersion v = result.back() + 1; v <= ESM::CurrentSaveGameFormatVersion; ++v) + result.push_back(v); + return result; + } constexpr std::uint32_t fakeRecordId = fourCC("FAKE"); @@ -327,6 +332,6 @@ namespace ESM EXPECT_EQ(result.mKeys, record.mKeys); } - INSTANTIATE_TEST_SUITE_P(FormatVersions, Esm3SaveLoadRecordTest, ValuesIn(formats)); + INSTANTIATE_TEST_SUITE_P(FormatVersions, Esm3SaveLoadRecordTest, ValuesIn(getFormats())); } } diff --git a/apps/openmw_test_suite/mwworld/test_store.cpp b/apps/openmw_test_suite/mwworld/test_store.cpp index 268bc8bc0b..8872f4e50b 100644 --- a/apps/openmw_test_suite/mwworld/test_store.cpp +++ b/apps/openmw_test_suite/mwworld/test_store.cpp @@ -269,22 +269,27 @@ std::unique_ptr getEsmFile(T record, bool deleted, ESM::FormatVers namespace { - constexpr std::array formats = { - ESM::DefaultFormatVersion, - ESM::CurrentContentFormatVersion, - ESM::MaxOldWeatherFormatVersion, - ESM::MaxOldDeathAnimationFormatVersion, - ESM::MaxOldForOfWarFormatVersion, - ESM::MaxWerewolfDeprecatedDataFormatVersion, - ESM::MaxOldTimeLeftFormatVersion, - ESM::MaxIntFallbackFormatVersion, - ESM::MaxClearModifiersFormatVersion, - ESM::MaxOldAiPackageFormatVersion, - ESM::MaxOldSkillsAndAttributesFormatVersion, - ESM::MaxOldCreatureStatsFormatVersion, - ESM::MaxStringRefIdFormatVersion, - ESM::CurrentSaveGameFormatVersion, - }; + std::vector getFormats() + { + std::vector result({ + ESM::DefaultFormatVersion, + ESM::CurrentContentFormatVersion, + ESM::MaxOldWeatherFormatVersion, + ESM::MaxOldDeathAnimationFormatVersion, + ESM::MaxOldForOfWarFormatVersion, + ESM::MaxWerewolfDeprecatedDataFormatVersion, + ESM::MaxOldTimeLeftFormatVersion, + ESM::MaxIntFallbackFormatVersion, + ESM::MaxClearModifiersFormatVersion, + ESM::MaxOldAiPackageFormatVersion, + ESM::MaxOldSkillsAndAttributesFormatVersion, + ESM::MaxOldCreatureStatsFormatVersion, + ESM::MaxStringRefIdFormatVersion, + }); + for (ESM::FormatVersion v = result.back() + 1; v <= ESM::CurrentSaveGameFormatVersion; ++v) + result.push_back(v); + return result; + } template > struct HasBlankFunction : std::false_type @@ -305,7 +310,7 @@ TYPED_TEST_P(StoreTest, delete_test) { using RecordType = TypeParam; - for (const ESM::FormatVersion formatVersion : formats) + for (const ESM::FormatVersion formatVersion : getFormats()) { SCOPED_TRACE("FormatVersion: " + std::to_string(formatVersion)); const ESM::RefId recordId = ESM::RefId::stringRefId("foobar"); @@ -383,7 +388,7 @@ TYPED_TEST_P(StoreTest, overwrite_test) { using RecordType = TypeParam; - for (const ESM::FormatVersion formatVersion : formats) + for (const ESM::FormatVersion formatVersion : getFormats()) { SCOPED_TRACE("FormatVersion: " + std::to_string(formatVersion)); @@ -442,7 +447,7 @@ namespace else refId = ESM::StringRefId("foobar"); - for (const ESM::FormatVersion formatVersion : formats) + for (const ESM::FormatVersion formatVersion : getFormats()) { SCOPED_TRACE("FormatVersion: " + std::to_string(formatVersion)); From 4716583f3e985e8d9becf1bacc5d3fc071843a3a Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 26 Mar 2023 16:02:24 +0200 Subject: [PATCH 5/7] Set ESM::Dialogue::mType on blank and skip load --- components/esm3/loaddial.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/esm3/loaddial.cpp b/components/esm3/loaddial.cpp index 7681b42db7..265a8dcdf9 100644 --- a/components/esm3/loaddial.cpp +++ b/components/esm3/loaddial.cpp @@ -37,6 +37,7 @@ namespace ESM else { esm.skip(size); + mType = Unknown; } break; } @@ -67,6 +68,7 @@ namespace ESM void Dialogue::blank() { + mType = Unknown; mInfo.clear(); } From 06f42ba69cdf67a768cce30b5e0b82881fb80ab0 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 26 Mar 2023 16:04:15 +0200 Subject: [PATCH 6/7] Use fixed size enum type for ESM::Dialogue::mType --- apps/opencs/model/world/columnimp.hpp | 3 ++- components/esm3/loaddial.hpp | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/opencs/model/world/columnimp.hpp b/apps/opencs/model/world/columnimp.hpp index a24a661cda..91db8d0c41 100644 --- a/apps/opencs/model/world/columnimp.hpp +++ b/apps/opencs/model/world/columnimp.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -1342,7 +1343,7 @@ namespace CSMWorld { ESXRecordT record2 = record.get(); - record2.mType = data.toInt(); + record2.mType = static_cast(data.toInt()); record.setModified(record2); } diff --git a/components/esm3/loaddial.hpp b/components/esm3/loaddial.hpp index 6c3ec16e71..9fb57aa390 100644 --- a/components/esm3/loaddial.hpp +++ b/components/esm3/loaddial.hpp @@ -30,7 +30,7 @@ namespace ESM /// Return a string descriptor for this record type. Currently used for debugging / error logs only. static std::string_view getRecordType() { return "Dialogue"; } - enum Type + enum Type : std::int8_t { Topic = 0, Voice = 1, @@ -41,7 +41,7 @@ namespace ESM }; RefId mId; - signed char mType; + Type mType; InfoContainer mInfo; InfoOrder mInfoOrder; From 452d1e7e49fa256ec3f001cd783c3389900fd84f Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 26 Mar 2023 15:21:09 +0200 Subject: [PATCH 7/7] Store original string id for Dialogue records --- apps/esmtool/record.cpp | 1 + apps/opencs/model/doc/document.cpp | 3 ++ apps/opencs/model/world/collection.hpp | 11 ++++ .../model/world/testinfocollection.cpp | 51 +++++++++---------- apps/openmw/mwdialogue/dialoguemanagerimp.cpp | 9 ++-- apps/openmw_test_suite/esm3/testsaveload.cpp | 13 +++++ apps/openmw_test_suite/mwworld/test_store.cpp | 8 ++- components/esm3/formatversion.hpp | 3 +- components/esm3/loaddial.cpp | 39 +++++++++++++- components/esm3/loaddial.hpp | 1 + components/esm3/typetraits.hpp | 13 +++++ 11 files changed, 115 insertions(+), 37 deletions(-) diff --git a/apps/esmtool/record.cpp b/apps/esmtool/record.cpp index 97789ebaab..a908a2a45c 100644 --- a/apps/esmtool/record.cpp +++ b/apps/esmtool/record.cpp @@ -741,6 +741,7 @@ namespace EsmTool template <> void Record::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 diff --git a/apps/opencs/model/doc/document.cpp b/apps/opencs/model/doc/document.cpp index 634e6728f7..b4b7290f49 100644 --- a/apps/opencs/model/doc/document.cpp +++ b/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(); diff --git a/apps/opencs/model/world/collection.hpp b/apps/opencs/model/world/collection.hpp index e8847253d2..d1b20bdedd 100644 --- a/apps/opencs/model/world/collection.hpp +++ b/apps/opencs/model/world/collection.hpp @@ -14,6 +14,7 @@ #include +#include #include #include "collectionbase.hpp" @@ -278,6 +279,11 @@ namespace CSMWorld } } + if constexpr (std::is_same_v) + { + 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) + { + record.mStringId = record.mId.getRefIdString(); + } + auto record2 = std::make_unique>(); record2->mState = Record::State_ModifiedOnly; record2->mModified = record; diff --git a/apps/opencs_tests/model/world/testinfocollection.cpp b/apps/opencs_tests/model/world/testinfocollection.cpp index 4aa277a22f..77e727d9aa 100644 --- a/apps/opencs_tests/model/world/testinfocollection.cpp +++ b/apps/opencs_tests/model/world/testinfocollection.cpp @@ -41,12 +41,13 @@ namespace CSMWorld }; DialogueData generateDialogueWithInfos( - std::size_t infoCount, const ESM::RefId& dialogueId = ESM::RefId::stringRefId("dialogue")) + std::size_t infoCount, std::string_view dialogueId = "dialogue") { DialogueData 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( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue2")), base, collection, infoOrder); - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder); - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue2"), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue0"), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue1"), base, collection, infoOrder); collection.sort(infoOrder); @@ -558,10 +563,8 @@ namespace CSMWorld InfoOrderByTopic infoOrder; InfoCollection collection; - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder); - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue0"), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue1"), base, collection, infoOrder); collection.sort(infoOrder); @@ -574,10 +577,8 @@ namespace CSMWorld InfoOrderByTopic infoOrder; InfoCollection collection; - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder); - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue0"), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue1"), base, collection, infoOrder); EXPECT_FALSE(collection.reorderRows(5, {})); } @@ -588,10 +589,8 @@ namespace CSMWorld InfoOrderByTopic infoOrder; InfoCollection collection; - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder); - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue0"), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "dialogue1"), base, collection, infoOrder); EXPECT_FALSE(collection.reorderRows(0, { 0, 1, 2 })); } @@ -602,10 +601,8 @@ namespace CSMWorld InfoOrderByTopic infoOrder; InfoCollection collection; - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(3, ESM::RefId::stringRefId("dialogue0")), base, collection, infoOrder); - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(3, ESM::RefId::stringRefId("dialogue1")), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(3, "dialogue0"), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(3, "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( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("d0")), base, collection, infoOrder); - saveAndLoadDialogueWithInfos( - generateDialogueWithInfos(2, ESM::RefId::stringRefId("d1")), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "d0"), base, collection, infoOrder); + saveAndLoadDialogueWithInfos(generateDialogueWithInfos(2, "d1"), base, collection, infoOrder); collection.sort(infoOrder); diff --git a/apps/openmw/mwdialogue/dialoguemanagerimp.cpp b/apps/openmw/mwdialogue/dialoguemanagerimp.cpp index c82c6e2f5e..c0e506ebf2 100644 --- a/apps/openmw/mwdialogue/dialoguemanagerimp.cpp +++ b/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& gmsts = MWBase::Environment::get().getWorld()->getStore().get(); - 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( - title.getRefIdString(), Interpreter::fixDefinesDialog(info->mResponse, interpreterContext)); + callback->addResponse(title, Interpreter::fixDefinesDialog(info->mResponse, interpreterContext)); if (dialogue.mType == ESM::Dialogue::Topic) { diff --git a/apps/openmw_test_suite/esm3/testsaveload.cpp b/apps/openmw_test_suite/esm3/testsaveload.cpp index 09b5b11ed4..0729b0ec26 100644 --- a/apps/openmw_test_suite/esm3/testsaveload.cpp +++ b/apps/openmw_test_suite/esm3/testsaveload.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -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())); } } diff --git a/apps/openmw_test_suite/mwworld/test_store.cpp b/apps/openmw_test_suite/mwworld/test_store.cpp index 8872f4e50b..90294aadaa 100644 --- a/apps/openmw_test_suite/mwworld/test_store.cpp +++ b/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 && !std::is_same_v) 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) + record.mStringId = stringId; + if constexpr (ESM::hasIndex) record.mIndex = index; @@ -486,6 +490,7 @@ namespace } static_assert(ESM::hasIndex); + static_assert(ESM::hasStringId); template > 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) { diff --git a/components/esm3/formatversion.hpp b/components/esm3/formatversion.hpp index 04fcb7e8a6..144b4c23af 100644 --- a/components/esm3/formatversion.hpp +++ b/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 diff --git a/components/esm3/loaddial.cpp b/components/esm3/loaddial.cpp index 265a8dcdf9..788e140573 100644 --- a/components/esm3/loaddial.cpp +++ b/components/esm3/loaddial.cpp @@ -3,6 +3,8 @@ #include "esmreader.hpp" #include "esmwriter.hpp" +#include + 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); } } diff --git a/components/esm3/loaddial.hpp b/components/esm3/loaddial.hpp index 9fb57aa390..1bd736202d 100644 --- a/components/esm3/loaddial.hpp +++ b/components/esm3/loaddial.hpp @@ -41,6 +41,7 @@ namespace ESM }; RefId mId; + std::string mStringId; Type mType; InfoContainer mInfo; InfoOrder mInfoOrder; diff --git a/components/esm3/typetraits.hpp b/components/esm3/typetraits.hpp index 04529f3088..f5447f9508 100644 --- a/components/esm3/typetraits.hpp +++ b/components/esm3/typetraits.hpp @@ -17,6 +17,19 @@ namespace ESM template inline constexpr bool hasIndex = HasIndex::value; + + template > + struct HasStringId : std::false_type + { + }; + + template + struct HasStringId> : std::true_type + { + }; + + template + inline constexpr bool hasStringId = HasStringId::value; } #endif // OPENMW_COMPONENTS_ESM3_TYPETRAITS