From 3e6828dbde0189e2c06a1cf5307e7a7b8f626eaf Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 28 Feb 2025 19:05:03 +0000 Subject: [PATCH] Simplify load order handling during sorting The old approach made sense once, when the load order was the current load order and not the order that the plugins are given in, but that was a long time ago. --- src/api/sorting/plugin_graph.cpp | 51 ++----------------- src/api/sorting/plugin_sort.cpp | 9 ++-- src/api/sorting/plugin_sorting_data.cpp | 37 ++++---------- src/api/sorting/plugin_sorting_data.h | 6 +-- .../api/internals/sorting/plugin_sort_test.h | 41 +++++++-------- 5 files changed, 40 insertions(+), 104 deletions(-) diff --git a/src/api/sorting/plugin_graph.cpp b/src/api/sorting/plugin_graph.cpp index 34688106..904ddbe4 100644 --- a/src/api/sorting/plugin_graph.cpp +++ b/src/api/sorting/plugin_graph.cpp @@ -593,48 +593,6 @@ bool FindPath(RawPluginGraph& graph, return false; } -int ComparePlugins(const PluginSortingData& plugin1, - const PluginSortingData& plugin2) { - if (plugin1.GetLoadOrderIndex().has_value() && - !plugin2.GetLoadOrderIndex().has_value()) { - return -1; - } - - if (!plugin1.GetLoadOrderIndex().has_value() && - plugin2.GetLoadOrderIndex().has_value()) { - return 1; - } - - if (plugin1.GetLoadOrderIndex().has_value() && - plugin2.GetLoadOrderIndex().has_value()) { - if (plugin1.GetLoadOrderIndex().value() < - plugin2.GetLoadOrderIndex().value()) { - return -1; - } else { - return 1; - } - } - - // Neither plugin has a load order position. Compare plugin basenames to - // get an ordering. - const auto name1 = plugin1.GetName(); - const auto name2 = plugin2.GetName(); - const auto basename1 = name1.substr(0, name1.length() - 4); - const auto basename2 = name2.substr(0, name2.length() - 4); - - const int result = CompareFilenames(basename1, basename2); - - if (result != 0) { - return result; - } else { - // Could be a .esp and .esm plugin with the same basename, - // compare their extensions. - const auto ext1 = name1.substr(name1.length() - 4); - const auto ext2 = name2.substr(name2.length() - 4); - return CompareFilenames(ext1, ext2); - } -} - bool PathsCache::IsPathCached(const vertex_t& fromVertex, const vertex_t& toVertex) const { const auto descendants = pathsCache_.find(fromVertex); @@ -695,13 +653,13 @@ std::pair PluginGraph::GetVertices() const { return boost::vertices(graph_); } -std::optional PluginGraph::GetVertexByName( - const std::string& name) { +std::optional PluginGraph::GetVertexByName(const std::string& name) { for (const auto& vertex : boost::make_iterator_range(GetVertices())) { const auto& vertexName = GetPlugin(vertex).GetName(); comparableFilenamesCache_.Insert(vertexName); const auto& comparableName = comparableFilenamesCache_.GetOrInsert(name); - const auto& comparableVertexName = comparableFilenamesCache_.Get(vertexName); + const auto& comparableVertexName = + comparableFilenamesCache_.Get(vertexName); if (CompareFilenames(comparableVertexName, comparableName) == 0) { return vertex; @@ -1241,7 +1199,8 @@ void PluginGraph::AddTieBreakEdges() { std::sort(vertices.begin(), vertices.end(), [this](const vertex_t& lhs, const vertex_t& rhs) { - return ComparePlugins(GetPlugin(lhs), GetPlugin(rhs)) < 0; + return GetPlugin(lhs).GetLoadOrderIndex() < + GetPlugin(rhs).GetLoadOrderIndex(); }); // Now iterate over the vertices in their sorted order. diff --git a/src/api/sorting/plugin_sort.cpp b/src/api/sorting/plugin_sort.cpp index 84ccd2cd..aa80a75a 100644 --- a/src/api/sorting/plugin_sort.cpp +++ b/src/api/sorting/plugin_sort.cpp @@ -38,11 +38,7 @@ std::vector GetPluginsSortingData( std::vector pluginsSortingData; pluginsSortingData.reserve(loadOrder.size()); - std::vector comparableLoadOrder; - for (const auto& plugin : loadOrder) { - comparableLoadOrder.push_back(ToComparableFilename(plugin->GetName())); - } - + size_t i = 0; for (const auto& plugin : loadOrder) { const auto pluginFilename = plugin->GetName(); @@ -53,9 +49,10 @@ std::vector GetPluginsSortingData( .value_or(PluginMetadata(pluginFilename)); const auto pluginSortingData = PluginSortingData( - plugin, masterlistMetadata, userMetadata, comparableLoadOrder); + plugin, masterlistMetadata, userMetadata, i); pluginsSortingData.push_back(pluginSortingData); + i += 1; } return pluginsSortingData; diff --git a/src/api/sorting/plugin_sorting_data.cpp b/src/api/sorting/plugin_sorting_data.cpp index ec5b5f0c..4a92724e 100644 --- a/src/api/sorting/plugin_sorting_data.cpp +++ b/src/api/sorting/plugin_sorting_data.cpp @@ -49,11 +49,10 @@ std::vector GetPluginsSubset( return pluginsSubset; } -PluginSortingData::PluginSortingData( - const PluginSortingInterface* plugin, - const PluginMetadata& masterlistMetadata, - const PluginMetadata& userMetadata, - const std::vector& loadOrder) : +PluginSortingData::PluginSortingData(const PluginSortingInterface* plugin, + const PluginMetadata& masterlistMetadata, + const PluginMetadata& userMetadata, + const size_t loadOrderIndex) : plugin_(plugin), name_(plugin == nullptr ? std::string() : plugin->GetName()), isMaster_(plugin != nullptr && plugin->IsMaster()), @@ -63,30 +62,17 @@ PluginSortingData::PluginSortingData( userLoadAfter_(userMetadata.GetLoadAfterFiles()), masterlistReq_(masterlistMetadata.GetRequirements()), userReq_(userMetadata.GetRequirements()), - groupIsUserMetadata_(userMetadata.GetGroup().has_value()) { - if (plugin == nullptr) { - return; - } - - const auto comparableName = ToComparableFilename(GetName()); - - for (size_t i = 0; i < loadOrder.size(); i++) { - if (CompareFilenames(comparableName, loadOrder.at(i)) == 0) { - loadOrderIndex_ = i; - break; - } - } - - overrideRecordCount_ = plugin->GetOverrideRecordCount(); -} + loadOrderIndex_(loadOrderIndex), + overrideRecordCount_(plugin == nullptr ? 0 + : plugin->GetOverrideRecordCount()), + groupIsUserMetadata_(userMetadata.GetGroup().has_value()) {} const std::string& PluginSortingData::GetName() const { return name_; } bool PluginSortingData::IsMaster() const { return isMaster_; } bool PluginSortingData::IsBlueprintMaster() const { - return isMaster_ && - plugin_->IsBlueprintPlugin(); + return isMaster_ && plugin_->IsBlueprintPlugin(); } std::vector PluginSortingData::GetMasters() const { @@ -138,7 +124,6 @@ const std::vector& PluginSortingData::GetMasterlistRequirements() const { const std::vector& PluginSortingData::GetUserRequirements() const { return userReq_; } -const std::optional& PluginSortingData::GetLoadOrderIndex() const { - return loadOrderIndex_; -} + +size_t PluginSortingData::GetLoadOrderIndex() const { return loadOrderIndex_; } } diff --git a/src/api/sorting/plugin_sorting_data.h b/src/api/sorting/plugin_sorting_data.h index 34e97439..d7c0afa8 100644 --- a/src/api/sorting/plugin_sorting_data.h +++ b/src/api/sorting/plugin_sorting_data.h @@ -44,7 +44,7 @@ public: explicit PluginSortingData(const PluginSortingInterface* plugin, const PluginMetadata& masterlistMetadata, const PluginMetadata& userMetadata, - const std::vector& loadOrder); + const size_t loadOrderIndex); const std::string& GetName() const; bool IsMaster() const; @@ -64,7 +64,7 @@ public: const std::vector& GetMasterlistRequirements() const; const std::vector& GetUserRequirements() const; - const std::optional& GetLoadOrderIndex() const; + size_t GetLoadOrderIndex() const; private: const PluginSortingInterface* plugin_{nullptr}; @@ -77,7 +77,7 @@ private: std::vector masterlistReq_; std::vector userReq_; - std::optional loadOrderIndex_; + size_t loadOrderIndex_{0}; size_t overrideRecordCount_{0}; bool groupIsUserMetadata_{0}; }; diff --git a/src/tests/api/internals/sorting/plugin_sort_test.h b/src/tests/api/internals/sorting/plugin_sort_test.h index 4e889996..7813751e 100644 --- a/src/tests/api/internals/sorting/plugin_sort_test.h +++ b/src/tests/api/internals/sorting/plugin_sort_test.h @@ -86,16 +86,11 @@ protected: PluginSortingData CreatePluginSortingData( const std::string& name, - const std::vector& loadOrder = {}) { + const size_t loadOrderIndex) { const auto plugin = GetPlugin(name); - std::vector comparableLoadOrder; - for (const auto& pluginName : loadOrder) { - comparableLoadOrder.push_back(ToComparableFilename(pluginName)); - } - return PluginSortingData( - plugin, PluginMetadata(), PluginMetadata(), comparableLoadOrder); + plugin, PluginMetadata(), PluginMetadata(), loadOrderIndex); } plugingraph::TestPlugin* GetPlugin(const std::string& name) { @@ -194,9 +189,9 @@ TEST_P(PluginSortTest, // Now sort the plugins. { std::vector pluginsSortingData{ - CreatePluginSortingData(p1->GetName(), loadOrder), - CreatePluginSortingData(p2->GetName(), loadOrder), - CreatePluginSortingData(p3->GetName(), loadOrder)}; + CreatePluginSortingData(p1->GetName(), 0), + CreatePluginSortingData(p2->GetName(), 1), + CreatePluginSortingData(p3->GetName(), 2)}; auto sorted = SortPlugins(std::move(pluginsSortingData), {Group()}, {}, {}); ASSERT_EQ(expectedSortedOrder, sorted); @@ -208,9 +203,9 @@ TEST_P(PluginSortTest, // order. { std::vector pluginsSortingData{ - CreatePluginSortingData(p1->GetName(), loadOrder), - CreatePluginSortingData(p2->GetName(), loadOrder), - CreatePluginSortingData(p3->GetName(), loadOrder)}; + CreatePluginSortingData(p1->GetName(), 1), + CreatePluginSortingData(p2->GetName(), 2), + CreatePluginSortingData(p3->GetName(), 0)}; auto sorted = SortPlugins(std::move(pluginsSortingData), {Group()}, {}, {}); ASSERT_EQ(expectedSortedOrder, sorted); @@ -602,8 +597,8 @@ TEST_P(PluginSortTest, esm->AddMaster(esp->GetName()); std::vector pluginsSortingData{ - CreatePluginSortingData(esm->GetName()), - CreatePluginSortingData(esp->GetName())}; + CreatePluginSortingData(esm->GetName(), 0), + CreatePluginSortingData(esp->GetName(), 1)}; try { SortPlugins(std::move(pluginsSortingData), {Group()}, {}, {}); @@ -755,8 +750,8 @@ TEST_P(PluginSortTest, esm->SetIsMaster(true); std::vector pluginsSortingData{ - CreatePluginSortingData(esm->GetName()), - CreatePluginSortingData(esp->GetName())}; + CreatePluginSortingData(esm->GetName(), 0), + CreatePluginSortingData(esp->GetName(), 1)}; try { SortPlugins(std::move(pluginsSortingData), {Group()}, {}, {esp->GetName()}); @@ -816,8 +811,8 @@ TEST_P( blueprint->SetIsBlueprintMaster(true); std::vector pluginsSortingData{ - CreatePluginSortingData(esp->GetName()), - CreatePluginSortingData(blueprint->GetName())}; + CreatePluginSortingData(esp->GetName(), 0), + CreatePluginSortingData(blueprint->GetName(), 1)}; const auto sorted = SortPlugins(std::move(pluginsSortingData), {Group()}, {}, {}); @@ -1106,8 +1101,8 @@ TEST_P( blueprint->SetIsBlueprintMaster(true); std::vector pluginsSortingData{ - CreatePluginSortingData(esm->GetName()), - CreatePluginSortingData(blueprint->GetName())}; + CreatePluginSortingData(esm->GetName(), 0), + CreatePluginSortingData(blueprint->GetName(), 0)}; const auto sorted = SortPlugins( std::move(pluginsSortingData), {Group()}, {}, {blueprint->GetName()}); @@ -1135,8 +1130,8 @@ TEST_P( blueprint->SetIsBlueprintMaster(true); std::vector pluginsSortingData{ - CreatePluginSortingData(esm->GetName()), - CreatePluginSortingData(blueprint->GetName())}; + CreatePluginSortingData(esm->GetName(), 0), + CreatePluginSortingData(blueprint->GetName(), 1)}; const auto sorted = SortPlugins( std::move(pluginsSortingData), {Group()}, {}, {blueprint->GetName()});