diff --git a/apps/essimporter/importer.cpp b/apps/essimporter/importer.cpp index 3799e6981a..c610224b33 100644 --- a/apps/essimporter/importer.cpp +++ b/apps/essimporter/importer.cpp @@ -347,7 +347,7 @@ namespace ESSImport ESM::ESMWriter writer; - writer.setFormat(ESM::SavedGame::sCurrentFormat); + writer.setFormatVersion(ESM::CurrentSaveGameFormatVersion); std::ofstream stream(mOutFile, std::ios::out | std::ios::binary); // all unused diff --git a/apps/opencs/model/doc/savingstages.cpp b/apps/opencs/model/doc/savingstages.cpp index 441e508c43..c247e90aa3 100644 --- a/apps/opencs/model/doc/savingstages.cpp +++ b/apps/opencs/model/doc/savingstages.cpp @@ -91,7 +91,7 @@ void CSMDoc::WriteHeaderStage::perform(int stage, Messages& messages) // ESM::Header::CurrentFormat is `1` but since new records are not yet used in opencs // we use the format `0` for compatibility with old versions. - mState.getWriter().setFormat(0); + mState.getWriter().setFormatVersion(ESM::DefaultFormatVersion); } else { diff --git a/apps/opencs/model/world/columnimp.hpp b/apps/opencs/model/world/columnimp.hpp index baaf249231..1b8a43cfd7 100644 --- a/apps/opencs/model/world/columnimp.hpp +++ b/apps/opencs/model/world/columnimp.hpp @@ -2232,7 +2232,7 @@ namespace CSMWorld { } - QVariant get(const Record& record) const override { return record.get().mFormat; } + QVariant get(const Record& record) const override { return record.get().mFormatVersion; } bool isEditable() const override { return false; } }; diff --git a/apps/opencs/model/world/metadata.cpp b/apps/opencs/model/world/metadata.cpp index d747515c86..c1c6325e3d 100644 --- a/apps/opencs/model/world/metadata.cpp +++ b/apps/opencs/model/world/metadata.cpp @@ -8,21 +8,21 @@ void CSMWorld::MetaData::blank() { // ESM::Header::CurrentFormat is `1` but since new records are not yet used in opencs // we use the format `0` for compatibility with old versions. - mFormat = 0; + mFormatVersion = ESM::DefaultFormatVersion; mAuthor.clear(); mDescription.clear(); } void CSMWorld::MetaData::load(ESM::ESMReader& esm) { - mFormat = esm.getHeader().mFormat; + mFormatVersion = esm.getHeader().mFormatVersion; mAuthor = esm.getHeader().mData.author; mDescription = esm.getHeader().mData.desc; } void CSMWorld::MetaData::save(ESM::ESMWriter& esm) const { - esm.setFormat(mFormat); + esm.setFormatVersion(mFormatVersion); esm.setAuthor(mAuthor); esm.setDescription(mDescription); } diff --git a/apps/opencs/model/world/metadata.hpp b/apps/opencs/model/world/metadata.hpp index 13721253d5..5d892508af 100644 --- a/apps/opencs/model/world/metadata.hpp +++ b/apps/opencs/model/world/metadata.hpp @@ -2,6 +2,8 @@ #define CSM_WOLRD_METADATA_H #include +#include + #include namespace ESM @@ -16,7 +18,7 @@ namespace CSMWorld { ESM::RefId mId; - int mFormat; + ESM::FormatVersion mFormatVersion; std::string mAuthor; std::string mDescription; diff --git a/apps/openmw/mwstate/statemanagerimp.cpp b/apps/openmw/mwstate/statemanagerimp.cpp index 680bc5b10c..b4da73c9da 100644 --- a/apps/openmw/mwstate/statemanagerimp.cpp +++ b/apps/openmw/mwstate/statemanagerimp.cpp @@ -250,7 +250,7 @@ void MWState::StateManager::saveGame(const std::string& description, const Slot* for (const std::string& contentFile : MWBase::Environment::get().getWorld()->getContentFiles()) writer.addMaster(contentFile, 0); // not using the size information anyway -> use value of 0 - writer.setFormat(ESM::SavedGame::sCurrentFormat); + writer.setFormatVersion(ESM::CurrentSaveGameFormatVersion); // all unused writer.setVersion(0); @@ -400,7 +400,7 @@ void MWState::StateManager::loadGame(const Character* character, const std::file ESM::ESMReader reader; reader.open(filepath); - if (reader.getFormat() > ESM::SavedGame::sCurrentFormat) + if (reader.getFormatVersion() > ESM::CurrentSaveGameFormatVersion) throw std::runtime_error( "This save file was created using a newer version of OpenMW and is thus not supported. Please upgrade " "to the newest OpenMW version to load this file."); diff --git a/apps/openmw/mwworld/esmloader.cpp b/apps/openmw/mwworld/esmloader.cpp index e0280c91b5..0d55847a19 100644 --- a/apps/openmw/mwworld/esmloader.cpp +++ b/apps/openmw/mwworld/esmloader.cpp @@ -55,7 +55,7 @@ namespace MWWorld if (!mMasterFileFormat.has_value() && (Misc::StringUtils::ciEndsWith(reader->getName().u8string(), u8".esm") || Misc::StringUtils::ciEndsWith(reader->getName().u8string(), u8".omwgame"))) - mMasterFileFormat = reader->getFormat(); + mMasterFileFormat = reader->getFormatVersion(); break; } case ESM::Format::Tes4: diff --git a/apps/openmw/mwworld/player.cpp b/apps/openmw/mwworld/player.cpp index 3a6417cde0..3fb99333d6 100644 --- a/apps/openmw/mwworld/player.cpp +++ b/apps/openmw/mwworld/player.cpp @@ -327,10 +327,10 @@ namespace MWWorld // this is the one object we can not silently drop. throw std::runtime_error("invalid player state record (object state)"); } - if (reader.getFormat() < 17) + if (reader.getFormatVersion() <= ESM::MaxClearModifiersFormatVersion) convertMagicEffects( player.mObject.mCreatureStats, player.mObject.mInventory, &player.mObject.mNpcStats); - else if (reader.getFormat() < 20) + else if (reader.getFormatVersion() <= ESM::MaxOldCreatureStatsFormatVersion) convertStats(player.mObject.mCreatureStats); if (!player.mObject.mEnabled) @@ -353,7 +353,7 @@ namespace MWWorld saveStats(); setWerewolfStats(); } - else if (reader.getFormat() < 19) + else if (reader.getFormatVersion() <= ESM::MaxOldSkillsAndAttributesFormatVersion) { setWerewolfStats(); if (player.mSetWerewolfAcrobatics) diff --git a/apps/openmw/mwworld/weather.cpp b/apps/openmw/mwworld/weather.cpp index b7ddd6e924..49159839e1 100644 --- a/apps/openmw/mwworld/weather.cpp +++ b/apps/openmw/mwworld/weather.cpp @@ -921,8 +921,7 @@ namespace MWWorld { if (ESM::REC_WTHR == type) { - static const int oldestCompatibleSaveFormat = 2; - if (reader.getFormat() < oldestCompatibleSaveFormat) + if (reader.getFormatVersion() <= ESM::MaxOldWeatherFormatVersion) { // Weather state isn't really all that important, so to preserve older save games, we'll just discard // the older weather records, rather than fail to handle the record. diff --git a/apps/openmw_test_suite/lua/test_configuration.cpp b/apps/openmw_test_suite/lua/test_configuration.cpp index a166fcf821..49fc93a3d8 100644 --- a/apps/openmw_test_suite/lua/test_configuration.cpp +++ b/apps/openmw_test_suite/lua/test_configuration.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -152,7 +153,7 @@ namespace writer.setAuthor(""); writer.setDescription(""); writer.setRecordCount(1); - writer.setFormat(ESM::Header::CurrentFormat); + writer.setFormatVersion(ESM::CurrentContentFormatVersion); writer.setVersion(); writer.addMaster("morrowind.esm", 0); diff --git a/apps/openmw_test_suite/mwworld/test_store.cpp b/apps/openmw_test_suite/mwworld/test_store.cpp index cca5a5cd35..b183f672f3 100644 --- a/apps/openmw_test_suite/mwworld/test_store.cpp +++ b/apps/openmw_test_suite/mwworld/test_store.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -16,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -238,20 +240,21 @@ TEST_F(ContentFileTest, autocalc_test) */ /// Base class for tests of ESMStore that do not rely on external content files +template struct StoreTest : public ::testing::Test { -protected: - MWWorld::ESMStore mEsmStore; }; +TYPED_TEST_SUITE_P(StoreTest); + /// Create an ESM file in-memory containing the specified record. /// @param deleted Write record with deleted flag? template -std::unique_ptr getEsmFile(T record, bool deleted) +std::unique_ptr getEsmFile(T record, bool deleted, ESM::FormatVersion formatVersion) { ESM::ESMWriter writer; auto stream = std::make_unique(); - writer.setFormat(0); + writer.setFormatVersion(formatVersion); writer.save(*stream); writer.startRecord(T::sRecordId); record.save(writer, deleted); @@ -260,41 +263,79 @@ std::unique_ptr getEsmFile(T record, bool deleted) return stream; } +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::CurrentSaveGameFormatVersion, + }; + + template > + struct HasBlankFunction : std::false_type + { + }; + + template + struct HasBlankFunction().blank())>> : std::true_type + { + }; + + template + constexpr bool hasBlankFunction = HasBlankFunction::value; +} + /// Tests deletion of records. -TEST_F(StoreTest, delete_test) +TYPED_TEST_P(StoreTest, delete_test) { - const ESM::RefId recordId = ESM::RefId::stringRefId("foobar"); + using RecordType = TypeParam; - typedef ESM::Apparatus RecordType; + for (const ESM::FormatVersion formatVersion : formats) + { + SCOPED_TRACE("FormatVersion: " + std::to_string(formatVersion)); + const ESM::RefId recordId = ESM::RefId::stringRefId("foobar"); - RecordType record; - record.blank(); - record.mId = recordId; + RecordType record; + if constexpr (hasBlankFunction) + record.blank(); + record.mId = recordId; - ESM::ESMReader reader; - ESM::Dialogue* dialogue = nullptr; + ESM::ESMReader reader; + ESM::Dialogue* dialogue = nullptr; + MWWorld::ESMStore esmStore; - // master file inserts a record - reader.open(getEsmFile(record, false), "filename"); - mEsmStore.load(reader, &dummyListener, dialogue); - mEsmStore.setUp(); + // master file inserts a record + reader.open(getEsmFile(record, false, formatVersion), "filename"); + esmStore.load(reader, &dummyListener, dialogue); + esmStore.setUp(); - ASSERT_TRUE(mEsmStore.get().getSize() == 1); + EXPECT_EQ(esmStore.get().getSize(), 1); - // now a plugin deletes it - reader.open(getEsmFile(record, true), "filename"); - mEsmStore.load(reader, &dummyListener, dialogue); - mEsmStore.setUp(); + // now a plugin deletes it + reader.open(getEsmFile(record, true, formatVersion), "filename"); + esmStore.load(reader, &dummyListener, dialogue); + esmStore.setUp(); - ASSERT_TRUE(mEsmStore.get().getSize() == 0); + EXPECT_EQ(esmStore.get().getSize(), 0); - // now another plugin inserts it again - // expected behaviour is the record to reappear rather than staying deleted - reader.open(getEsmFile(record, false), "filename"); - mEsmStore.load(reader, &dummyListener, dialogue); - mEsmStore.setUp(); + // now another plugin inserts it again + // expected behaviour is the record to reappear rather than staying deleted + reader.open(getEsmFile(record, false, formatVersion), "filename"); + esmStore.load(reader, &dummyListener, dialogue); + esmStore.setUp(); - ASSERT_TRUE(mEsmStore.get().getSize() == 1); + EXPECT_EQ(esmStore.get().getSize(), 1); + } } template @@ -327,42 +368,204 @@ static void testAllRecNameIntUnique(const MWWorld::ESMStore::StoreTuple& stores) std::apply([&stores](auto&&... x) { (testRecNameIntCount(x, stores), ...); }, stores); } -TEST_F(StoreTest, eachRecordTypeShouldHaveUniqueRecordId) +TEST(StoreTest, eachRecordTypeShouldHaveUniqueRecordId) { testAllRecNameIntUnique(MWWorld::ESMStore::StoreTuple()); } /// Tests overwriting of records. -TEST_F(StoreTest, overwrite_test) +TYPED_TEST_P(StoreTest, overwrite_test) +{ + using RecordType = TypeParam; + + for (const ESM::FormatVersion formatVersion : formats) + { + SCOPED_TRACE("FormatVersion: " + std::to_string(formatVersion)); + + const ESM::RefId recordId = ESM::RefId::stringRefId("foobar"); + const ESM::RefId recordIdUpper = ESM::RefId::stringRefId("Foobar"); + + RecordType record; + if constexpr (hasBlankFunction) + record.blank(); + record.mId = recordId; + + ESM::ESMReader reader; + ESM::Dialogue* dialogue = nullptr; + MWWorld::ESMStore esmStore; + + // master file inserts a record + reader.open(getEsmFile(record, false, formatVersion), "filename"); + esmStore.load(reader, &dummyListener, dialogue); + esmStore.setUp(); + + // now a plugin overwrites it with changed data + record.mId = recordIdUpper; // change id to uppercase, to test case smashing while we're at it + record.mModel = "the_new_model"; + reader.open(getEsmFile(record, false, formatVersion), "filename"); + esmStore.load(reader, &dummyListener, dialogue); + esmStore.setUp(); + + // verify that changes were actually applied + const RecordType* overwrittenRec = esmStore.get().search(recordId); + + ASSERT_NE(overwrittenRec, nullptr); + + EXPECT_EQ(overwrittenRec->mModel, "the_new_model"); + } +} + +namespace { - const ESM::RefId recordId = ESM::RefId::stringRefId("foobar"); - const ESM::RefId recordIdUpper = ESM::RefId::stringRefId("Foobar"); + template + struct StoreSaveLoadTest : public ::testing::Test + { + }; + + 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) + { + using RecordType = TypeParam; + + const int index = 3; + ESM::RefId refId; + if constexpr (hasIndex && !std::is_same_v) + refId = ESM::RefId::stringRefId(RecordType::indexToId(index)); + else + refId = ESM::RefId::stringRefId("foobar"); + + for (const ESM::FormatVersion formatVersion : formats) + { + SCOPED_TRACE("FormatVersion: " + std::to_string(formatVersion)); + + RecordType record; + + if constexpr (hasBlankFunction) + record.blank(); + + record.mId = refId; + + if constexpr (hasIndex) + record.mIndex = index; + + if constexpr (std::is_same_v) + record.mValue = ESM::Variant(42); + + ESM::ESMReader reader; + ESM::Dialogue* dialogue = nullptr; + MWWorld::ESMStore esmStore; + + reader.open(getEsmFile(record, false, formatVersion), "filename"); + esmStore.load(reader, &dummyListener, dialogue); + esmStore.setUp(); + + const RecordType* result = nullptr; + if constexpr (std::is_same_v) + result = esmStore.get().search(index, 0); + else if constexpr (hasIndex) + result = esmStore.get().search(index); + else + result = esmStore.get().search(refId); + + ASSERT_NE(result, nullptr); + EXPECT_EQ(result->mId, refId); + } + } + + static_assert(hasIndex); + + template > + struct HasSaveFunction : std::false_type + { + }; - typedef ESM::Apparatus RecordType; + template + struct HasSaveFunction().save(std::declval(), bool()))>> + : std::true_type + { + }; + + template + struct ConcatTypes; + + template + struct ConcatTypes> + { + using Type = std::tuple; + }; + + template