From 46bac43bc4711eb97d8d0188441453e8cecabe36 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 28 Jun 2024 19:57:19 +0100 Subject: [PATCH] Remove workaround for counting Morrowind override records It's not necessary now that esplugin can count override records for Morrowind plugins, and the handling of the case when not all masters are loaded is ineffective now that fully loading Morrowind plugins will fail if their masters are not also loaded. --- src/api/plugin.cpp | 33 ------- src/api/plugin.h | 5 - src/api/sorting/plugin_sorting_data.cpp | 25 +---- src/tests/api/internals/plugin_test.h | 91 ------------------- .../api/internals/sorting/plugin_graph_test.h | 5 - .../sorting/plugin_sorting_data_test.h | 30 ------ 6 files changed, 1 insertion(+), 188 deletions(-) diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp index 27f01707..028884ef 100644 --- a/src/api/plugin.cpp +++ b/src/api/plugin.cpp @@ -382,39 +382,6 @@ bool Plugin::DoRecordsOverlap(const PluginInterface& plugin) const { return false; } -size_t Plugin::GetOverlapSize( - const std::vector& plugins) const { - if (plugins.empty()) { - return 0; - } - - std::vector<::Plugin*> esPlugins; - for (const auto& plugin : plugins) { - const auto otherPlugin = dynamic_cast(plugin); - - if (otherPlugin == nullptr) { - const auto logger = getLogger(); - if (logger) { - logger->error( - "Tried to check how many records overlapped with a non-Plugin " - "implementation of PluginSortingInterface."); - } - throw std::invalid_argument( - "Tried to check how many records overlapped with a non-Plugin " - "implementation of PluginSortingInterface."); - } - - esPlugins.push_back(otherPlugin->esPlugin.get()); - } - - size_t overlapSize = 0; - const auto ret = esp_plugin_records_overlap_size( - esPlugin.get(), esPlugins.data(), esPlugins.size(), &overlapSize); - HandleEspluginError("get overlap size for \"" + name_ + "\"", ret); - - return overlapSize; -} - size_t Plugin::GetOverrideRecordCount() const { size_t overrideRecordCount; const auto ret = diff --git a/src/api/plugin.h b/src/api/plugin.h index 9b6c4137..1184ebb8 100644 --- a/src/api/plugin.h +++ b/src/api/plugin.h @@ -46,9 +46,6 @@ public: virtual size_t GetOverrideRecordCount() const = 0; virtual uint32_t GetRecordAndGroupCount() const = 0; - virtual size_t GetOverlapSize( - const std::vector& plugins) const = 0; - virtual size_t GetAssetCount() const = 0; virtual bool DoAssetsOverlap(const PluginSortingInterface& plugin) const = 0; }; @@ -81,8 +78,6 @@ public: bool IsEmpty() const override; bool LoadsArchive() const override; bool DoRecordsOverlap(const PluginInterface& plugin) const override; - size_t GetOverlapSize( - const std::vector& plugins) const override; // Load ordering functions. size_t GetOverrideRecordCount() const override; diff --git a/src/api/sorting/plugin_sorting_data.cpp b/src/api/sorting/plugin_sorting_data.cpp index 8b124af2..0a718751 100644 --- a/src/api/sorting/plugin_sorting_data.cpp +++ b/src/api/sorting/plugin_sorting_data.cpp @@ -75,30 +75,7 @@ PluginSortingData::PluginSortingData( } } - if (gameType == GameType::tes3) { - auto masterNames = plugin->GetMasters(); - if (masterNames.empty()) { - overrideRecordCount_ = 0; - } else { - auto masters = GetPluginsSubset(loadedPlugins, masterNames); - if (masters.size() == masterNames.size()) { - overrideRecordCount_ = plugin->GetOverlapSize(masters); - } else { - // Not all masters are loaded, fall back to using the plugin's - // total record count (Morrowind doesn't have groups). This is OK - // because plugins with missing masters can't be loaded by the game, - // so the correctness of their load order positions is less important - // (it may not matter at all, depending on the sophistication/usage of - // merge patches in Morrowind). It's better for LOOT to sort a load - // order with missing masters with potentially poorer results than - // for it to error out, as masters may be missing for a variety of - // development & testing reasons. - overrideRecordCount_ = plugin->GetRecordAndGroupCount(); - } - } - } else { - overrideRecordCount_ = plugin->GetOverrideRecordCount(); - } + overrideRecordCount_ = plugin->GetOverrideRecordCount(); } std::string PluginSortingData::GetName() const { return plugin_->GetName(); } diff --git a/src/tests/api/internals/plugin_test.h b/src/tests/api/internals/plugin_test.h index b42485ea..a42f3581 100644 --- a/src/tests/api/internals/plugin_test.h +++ b/src/tests/api/internals/plugin_test.h @@ -209,11 +209,6 @@ public: size_t GetOverrideRecordCount() const override { return 0; }; uint32_t GetRecordAndGroupCount() const override { return 0; }; - size_t GetOverlapSize( - const std::vector&) const override { - return 0; - }; - size_t GetAssetCount() const override { return 0; }; bool DoAssetsOverlap(const PluginSortingInterface&) const override { return true; @@ -672,92 +667,6 @@ TEST_P(PluginTest, EXPECT_TRUE(plugin2.DoRecordsOverlap(plugin1)); } -TEST_P(PluginTest, - getOverlapSizeShouldThrowIfGivenAVectorContainingANonPluginObject) { - Plugin plugin1( - game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsm, false); - OtherPluginType plugin2; - - EXPECT_THROW(plugin1.GetOverlapSize({&plugin2, &plugin2}), - std::invalid_argument); - EXPECT_EQ(0, plugin2.GetOverlapSize({&plugin1})); -} - -TEST_P(PluginTest, getOverlapSizeShouldCountEachRecordOnce) { - const auto plugin1Name = - GetParam() == GameType::starfield ? blankFullEsm : blankEsm; - - Plugin plugin1( - game_.GetType(), game_.GetCache(), game_.DataPath() / plugin1Name, false); - Plugin plugin2(game_.GetType(), - game_.GetCache(), - game_.DataPath() / (blankMasterDependentEsm + ".ghost"), - false); - - if (GetParam() == GameType::starfield) { - plugin1.ResolveRecordIds(nullptr); - - const auto pluginsMetadata = Plugin::GetPluginsMetadata({&plugin1}); - plugin2.ResolveRecordIds(pluginsMetadata.get()); - - EXPECT_EQ(1, plugin1.GetOverlapSize({&plugin2, &plugin2})); - } else { - EXPECT_EQ(4, plugin1.GetOverlapSize({&plugin2, &plugin2})); - } -} - -TEST_P(PluginTest, getOverlapSizeShouldCheckAgainstAllGivenPlugins) { - const auto plugin1Name = - GetParam() == GameType::starfield ? blankFullEsm : blankEsm; - - Plugin plugin1( - game_.GetType(), game_.GetCache(), game_.DataPath() / plugin1Name, false); - Plugin plugin2( - game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsp, false); - Plugin plugin3(game_.GetType(), - game_.GetCache(), - game_.DataPath() / (blankMasterDependentEsm + ".ghost"), - false); - - if (GetParam() == GameType::starfield) { - plugin1.ResolveRecordIds(nullptr); - plugin2.ResolveRecordIds(nullptr); - - const auto pluginsMetadata = Plugin::GetPluginsMetadata({&plugin1}); - plugin3.ResolveRecordIds(pluginsMetadata.get()); - - EXPECT_EQ(1, plugin1.GetOverlapSize({&plugin2, &plugin3})); - } else { - EXPECT_EQ(4, plugin1.GetOverlapSize({&plugin2, &plugin3})); - } -} - -TEST_P(PluginTest, - getOverlapSizeShouldReturnZeroForPluginsWithOnlyHeadersLoaded) { - Plugin plugin1( - game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsm, true); - Plugin plugin2(game_.GetType(), - game_.GetCache(), - game_.DataPath() / (blankMasterDependentEsm + ".ghost"), - true); - - EXPECT_EQ(0, plugin1.GetOverlapSize({&plugin2})); -} - -TEST_P(PluginTest, getOverlapSizeShouldReturnZeroForPluginsThatDoNotOverlap) { - Plugin plugin1( - game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsm, false); - Plugin plugin2( - game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsp, false); - - if (GetParam() == GameType::starfield) { - plugin1.ResolveRecordIds(nullptr); - plugin2.ResolveRecordIds(nullptr); - } - - EXPECT_EQ(0, plugin1.GetOverlapSize({&plugin2})); -} - TEST_P(PluginTest, getRecordAndGroupCountShouldReturnTheHeaderFieldValue) { Plugin plugin( game_.GetType(), game_.GetCache(), game_.DataPath() / blankEsm, true); diff --git a/src/tests/api/internals/sorting/plugin_graph_test.h b/src/tests/api/internals/sorting/plugin_graph_test.h index 8b846cb4..7044aa82 100644 --- a/src/tests/api/internals/sorting/plugin_graph_test.h +++ b/src/tests/api/internals/sorting/plugin_graph_test.h @@ -86,11 +86,6 @@ public: uint32_t GetRecordAndGroupCount() const override { return uint32_t(); } - size_t GetOverlapSize( - const std::vector&) const override { - return size_t(); - } - size_t GetAssetCount() const override { return assetCount_; }; bool DoAssetsOverlap(const PluginSortingInterface& plugin) const override { diff --git a/src/tests/api/internals/sorting/plugin_sorting_data_test.h b/src/tests/api/internals/sorting/plugin_sorting_data_test.h index 82fc8d48..5d288698 100644 --- a/src/tests/api/internals/sorting/plugin_sorting_data_test.h +++ b/src/tests/api/internals/sorting/plugin_sorting_data_test.h @@ -150,36 +150,6 @@ TEST_P(PluginSortingDataTest, getLoadedPlugins()); EXPECT_EQ(4, plugin.GetOverrideRecordCount()); } - -TEST_P( - PluginSortingDataTest, - constructorShouldUseTotalRecordCountAsOverrideRecordCountForTes3PluginWithAMasterThatIsNotLoaded) { - if (GetParam() != GameType::tes3) { - return; - } - - ASSERT_NO_THROW(loadInstalledPlugins(game_, false)); - auto loadedPlugins = getLoadedPlugins(); - - // Pretend that blankEsm isn't loaded. - for (auto it = loadedPlugins.begin(); it != loadedPlugins.end();) { - if ((*it)->GetName() == blankEsm) { - it = loadedPlugins.erase(it); - } else { - ++it; - } - } - - auto plugin = PluginSortingData( - dynamic_cast(game_.GetPlugin(blankMasterDependentEsm)), - PluginMetadata(), - PluginMetadata(), - getLoadOrder(), - game_.GetType(), - loadedPlugins); - - EXPECT_EQ(10, plugin.GetOverrideRecordCount()); -} } }