From 46aa3f5a07a3dc7ce78f7272e0400a5a7f986821 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Wed, 7 Dec 2022 21:25:27 +0000 Subject: [PATCH] Refactor PluginSortingData to not depend on Plugin This is a step towards making it easier to test sorting without having to load real plugins. --- src/api/plugin.cpp | 41 ++++++++---- src/api/plugin.h | 19 ++++-- src/api/sorting/plugin_graph.cpp | 8 ++- src/api/sorting/plugin_sorting_data.cpp | 10 +-- src/api/sorting/plugin_sorting_data.h | 15 ++--- .../sorting/plugin_sorting_data_test.h | 63 +++++++++++-------- 6 files changed, 101 insertions(+), 55 deletions(-) diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp index f349c619..63c031b2 100644 --- a/src/api/plugin.cpp +++ b/src/api/plugin.cpp @@ -194,25 +194,40 @@ bool Plugin::DoFormIDsOverlap(const PluginInterface& plugin) const { return false; } -size_t Plugin::GetOverlapSize(const std::vector plugins) const { +size_t Plugin::GetOverlapSize( + const std::vector plugins) const { if (plugins.empty()) { return 0; } - std::vector<::Plugin*> esPlugins; - for (const auto& plugin : plugins) { - esPlugins.push_back(plugin->esPlugin.get()); - } + try { + std::vector<::Plugin*> esPlugins; + for (const auto& plugin : plugins) { + const auto otherPlugin = dynamic_cast(plugin); - size_t overlapSize = 0; - const auto ret = esp_plugin_records_overlap_size( - esPlugin.get(), esPlugins.data(), esPlugins.size(), &overlapSize); - if (ret != ESP_OK) { - throw FileAccessError("Error getting overlap size for \"" + name_ + - "\". esplugin error code: " + std::to_string(ret)); - } + esPlugins.push_back(otherPlugin->esPlugin.get()); + } - return overlapSize; + size_t overlapSize = 0; + const auto ret = esp_plugin_records_overlap_size( + esPlugin.get(), esPlugins.data(), esPlugins.size(), &overlapSize); + if (ret != ESP_OK) { + throw FileAccessError("Error getting overlap size for \"" + name_ + + "\". esplugin error code: " + std::to_string(ret)); + } + + return overlapSize; + } catch (std::bad_cast&) { + auto logger = getLogger(); + if (logger) { + logger->error( + "Tried to check how many FormIDs overlapped with a non-Plugin " + "implementation of PluginSortingInterface."); + } + throw std::invalid_argument( + "Tried to check how many FormIDs overlapped with a non-Plugin " + "implementation of PluginSortingInterface."); + } } size_t Plugin::NumOverrideFormIDs() const { return numOverrideRecords_; } diff --git a/src/api/plugin.h b/src/api/plugin.h index 49710679..174e9f4b 100644 --- a/src/api/plugin.h +++ b/src/api/plugin.h @@ -40,7 +40,17 @@ namespace loot { class GameCache; -class Plugin final : public PluginInterface { +// An interface containing member functions that are used when sorting plugins. +class PluginSortingInterface : public PluginInterface { +public: + virtual size_t NumOverrideFormIDs() const = 0; + virtual uint32_t GetRecordAndGroupCount() const = 0; + + virtual size_t GetOverlapSize( + const std::vector plugins) const = 0; +}; + +class Plugin final : public PluginSortingInterface { public: explicit Plugin(const GameType gameType, const GameCache& gameCache, @@ -62,11 +72,12 @@ public: bool IsEmpty() const override; bool LoadsArchive() const override; bool DoFormIDsOverlap(const PluginInterface& plugin) const override; - size_t GetOverlapSize(const std::vector plugins) const; + size_t GetOverlapSize( + const std::vector plugins) const override; // Load ordering functions. - size_t NumOverrideFormIDs() const; - uint32_t GetRecordAndGroupCount() const; + size_t NumOverrideFormIDs() const override; + uint32_t GetRecordAndGroupCount() const override; // Validity checks. static bool IsValid(const GameType gameType, diff --git a/src/api/sorting/plugin_graph.cpp b/src/api/sorting/plugin_graph.cpp index 287e4a23..5ee0c256 100644 --- a/src/api/sorting/plugin_graph.cpp +++ b/src/api/sorting/plugin_graph.cpp @@ -224,6 +224,12 @@ void PluginGraph::AddPluginVertices(Game& game, return lhs->GetName() < rhs->GetName(); }); + std::vector loadedPluginInterfaces; + std::transform(loadedPlugins.begin(), + loadedPlugins.end(), + std::back_inserter(loadedPluginInterfaces), + [](const Plugin* plugin) { return plugin; }); + for (const auto& plugin : loadedPlugins) { auto masterlistMetadata = game.GetDatabase() @@ -238,7 +244,7 @@ void PluginGraph::AddPluginVertices(Game& game, userMetadata, loadOrder, game.Type(), - loadedPlugins); + loadedPluginInterfaces); auto groupName = pluginSortingData.GetGroup(); const auto groupIt = groupPlugins.find(groupName); diff --git a/src/api/sorting/plugin_sorting_data.cpp b/src/api/sorting/plugin_sorting_data.cpp index 2331e42e..910ae277 100644 --- a/src/api/sorting/plugin_sorting_data.cpp +++ b/src/api/sorting/plugin_sorting_data.cpp @@ -31,10 +31,10 @@ #include "api/helpers/text.h" namespace loot { -std::vector GetPluginsSubset( - const std::vector& plugins, +std::vector GetPluginsSubset( + const std::vector& plugins, const std::vector& pluginNames) { - std::vector pluginsSubset; + std::vector pluginsSubset; for (const auto& pluginName : pluginNames) { auto pos = std::find_if(plugins.begin(), plugins.end(), [&](auto plugin) { @@ -50,12 +50,12 @@ std::vector GetPluginsSubset( } PluginSortingData::PluginSortingData( - const Plugin* plugin, + const PluginSortingInterface* plugin, const PluginMetadata& masterlistMetadata, const PluginMetadata& userMetadata, const std::vector& loadOrder, const GameType gameType, - const std::vector& loadedPlugins) : + const std::vector& loadedPlugins) : plugin_(plugin), masterlistLoadAfter_(masterlistMetadata.GetLoadAfterFiles()), userLoadAfter_(userMetadata.GetLoadAfterFiles()), diff --git a/src/api/sorting/plugin_sorting_data.h b/src/api/sorting/plugin_sorting_data.h index 7b4bfe77..86e0c66e 100644 --- a/src/api/sorting/plugin_sorting_data.h +++ b/src/api/sorting/plugin_sorting_data.h @@ -36,12 +36,13 @@ public: * PluginSortingData objects must not live longer than the Plugin objects * that they are constructed from. */ - explicit PluginSortingData(const Plugin* plugin, - const PluginMetadata& masterlistMetadata, - const PluginMetadata& userMetadata, - const std::vector& loadOrder, - const GameType gameType, - const std::vector& loadedPlugins); + explicit PluginSortingData( + const PluginSortingInterface* plugin, + const PluginMetadata& masterlistMetadata, + const PluginMetadata& userMetadata, + const std::vector& loadOrder, + const GameType gameType, + const std::vector& loadedPlugins); std::string GetName() const; bool IsMaster() const; @@ -63,7 +64,7 @@ public: const std::optional& GetLoadOrderIndex() const; private: - const Plugin* plugin_; + const PluginSortingInterface* plugin_; std::string group_; std::unordered_set afterGroupPlugins_; 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 5841ae01..836cbfec 100644 --- a/src/tests/api/internals/sorting/plugin_sorting_data_test.h +++ b/src/tests/api/internals/sorting/plugin_sorting_data_test.h @@ -64,6 +64,17 @@ protected: game.LoadPlugins(plugins, headersOnly); } + std::vector getLoadedPlugins() { + std::vector loadedPluginInterfaces; + const auto loadedPlugins = game_.GetCache().GetPlugins(); + std::transform(loadedPlugins.begin(), + loadedPlugins.end(), + std::back_inserter(loadedPluginInterfaces), + [](const Plugin *plugin) { return plugin; }); + + return loadedPluginInterfaces; + } + Game game_; const std::string blankEslEsp; }; @@ -83,42 +94,44 @@ TEST_P(PluginSortingDataTest, lightFlaggedEspFilesShouldNotBeTreatedAsMasters) { } ASSERT_NO_THROW(loadInstalledPlugins(game_, false)); + const auto loadedPlugins = getLoadedPlugins(); - auto esp = - PluginSortingData(dynamic_cast(game_.GetPlugin(blankEsp)), - PluginMetadata(), - PluginMetadata(), - getLoadOrder(), - game_.Type(), - game_.GetCache().GetPlugins()); + auto esp = PluginSortingData( + dynamic_cast(game_.GetPlugin(blankEsp)), + PluginMetadata(), + PluginMetadata(), + getLoadOrder(), + game_.Type(), + loadedPlugins); EXPECT_FALSE(esp.IsMaster()); - auto master = - PluginSortingData(dynamic_cast(game_.GetPlugin(blankEsm)), - PluginMetadata(), - PluginMetadata(), - getLoadOrder(), - game_.Type(), - game_.GetCache().GetPlugins()); + auto master = PluginSortingData( + dynamic_cast(game_.GetPlugin(blankEsm)), + PluginMetadata(), + PluginMetadata(), + getLoadOrder(), + game_.Type(), + loadedPlugins); EXPECT_TRUE(master.IsMaster()); if (GetParam() == GameType::fo4 || GetParam() == GameType::tes5se) { auto lightMaster = PluginSortingData( - dynamic_cast(game_.GetPlugin(blankEsl)), + dynamic_cast(game_.GetPlugin(blankEsl)), PluginMetadata(), PluginMetadata(), getLoadOrder(), game_.Type(), - game_.GetCache().GetPlugins()); + loadedPlugins); EXPECT_TRUE(lightMaster.IsMaster()); - auto lightPlugin = PluginSortingData( - dynamic_cast(game_.GetPlugin(blankEslEsp)), - PluginMetadata(), - PluginMetadata(), - getLoadOrder(), - game_.Type(), - game_.GetCache().GetPlugins()); + auto lightPlugin = + PluginSortingData(dynamic_cast( + game_.GetPlugin(blankEslEsp)), + PluginMetadata(), + PluginMetadata(), + getLoadOrder(), + game_.Type(), + loadedPlugins); EXPECT_FALSE(lightPlugin.IsMaster()); } } @@ -133,7 +146,7 @@ TEST_P(PluginSortingDataTest, PluginMetadata(), getLoadOrder(), game_.Type(), - game_.GetCache().GetPlugins()); + getLoadedPlugins()); EXPECT_EQ(4, plugin.NumOverrideFormIDs()); } @@ -145,9 +158,9 @@ TEST_P( } ASSERT_NO_THROW(loadInstalledPlugins(game_, false)); + auto loadedPlugins = getLoadedPlugins(); // Pretend that blankEsm isn't loaded. - auto loadedPlugins = game_.GetCache().GetPlugins(); for (auto it = loadedPlugins.begin(); it != loadedPlugins.end();) { if ((*it)->GetName() == blankEsm) { it = loadedPlugins.erase(it);