From 86cdff1f217cadb236af04a454767f61b8d972b6 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 13 Jan 2023 20:15:19 +0000 Subject: [PATCH] Rework how groups are obtained when adding group edges Pass the groups graph in and derived everything from it. This duplicates a little work between the two plugin graphs (the group vertex map and group predecessors map could be shared), but the group graph is generally small enough that it doesn't seem to be significant. --- src/api/sorting/plugin_graph.cpp | 216 +++++++++--------- src/api/sorting/plugin_graph.h | 5 +- src/api/sorting/plugin_sort.cpp | 31 +-- .../api/internals/sorting/plugin_graph_test.h | 176 +++++++------- 4 files changed, 195 insertions(+), 233 deletions(-) diff --git a/src/api/sorting/plugin_graph.cpp b/src/api/sorting/plugin_graph.cpp index d190f7db..f76e0d1f 100644 --- a/src/api/sorting/plugin_graph.cpp +++ b/src/api/sorting/plugin_graph.cpp @@ -41,6 +41,8 @@ namespace loot { typedef boost::graph_traits::edge_descriptor edge_t; typedef boost::graph_traits::edge_iterator edge_it; +typedef boost::graph_traits::vertex_descriptor GroupGraphVertex; + class CycleDetector : public boost::dfs_visitor<> { public: void tree_edge(edge_t edge, const RawPluginGraph& graph) { @@ -80,20 +82,14 @@ private: std::vector trail; }; -struct GroupPlugin { - vertex_t vertex{0}; - bool groupIsUserMetadata{false}; -}; - struct PredecessorGroupPlugin { vertex_t vertex{0}; - bool pathInvolvesUserMetadata{false}; + bool groupPathInvolvesUserMetadata{false}; }; std::unordered_map> GetPredecessorGroupsPlugins( - const std::unordered_map>& - groupsPlugins, + const std::unordered_map>& groupsPlugins, const std::unordered_map>& predecessorGroupsMap) { std::unordered_map> @@ -110,9 +106,7 @@ GetPredecessorGroupsPlugins( // metadata have a path involving user metadata. for (const auto& groupPlugin : pluginsIt->second) { predecessorGroupPlugins.push_back(PredecessorGroupPlugin{ - groupPlugin.vertex, - predecessorGroup.pathInvolvesUserMetadata || - groupPlugin.groupIsUserMetadata}); + groupPlugin, predecessorGroup.pathInvolvesUserMetadata}); } } } @@ -123,27 +117,20 @@ GetPredecessorGroupsPlugins( return predecessorGroupsPlugins; } -std::unordered_map> -GetPredecessorGroupsPlugins( - const PluginGraph& graph, - const std::unordered_map>& - predecessorGroupsMap) { - // Each element of the vector is a pair of a plugin name and if it's in the - // group due to user metadata. - std::unordered_map> groupsPlugins; +std::unordered_map> GetGroupsPlugins( + const PluginGraph& graph) { + std::unordered_map> groupsPlugins; for (const vertex_t& vertex : boost::make_iterator_range(graph.GetVertices())) { - const auto& plugin = graph.GetPlugin(vertex); - const auto groupName = plugin.GetGroup(); - const auto groupPlugin = GroupPlugin{vertex, plugin.IsGroupUserMetadata()}; + const auto groupName = graph.GetPlugin(vertex).GetGroup(); const auto groupIt = groupsPlugins.find(groupName); if (groupIt == groupsPlugins.end()) { - groupsPlugins.emplace(groupName, std::vector({groupPlugin})); + groupsPlugins.emplace(groupName, std::vector({vertex})); } else { - groupIt->second.push_back(groupPlugin); + groupIt->second.push_back(vertex); } } @@ -155,35 +142,22 @@ GetPredecessorGroupsPlugins( for (auto& groupPlugins : groupsPlugins) { std::sort(groupPlugins.second.begin(), groupPlugins.second.end(), - [&](const GroupPlugin& lhs, const GroupPlugin& rhs) { - return graph.GetPlugin(lhs.vertex).GetName() < - graph.GetPlugin(rhs.vertex).GetName(); + [&](const vertex_t& lhs, const vertex_t& rhs) { + return graph.GetPlugin(lhs).GetName() < + graph.GetPlugin(rhs).GetName(); }); } - // Map sets of transitive group dependencies to sets of transitive plugin - // dependencies. - return GetPredecessorGroupsPlugins(groupsPlugins, predecessorGroupsMap); + return groupsPlugins; } std::vector GetPredecessorGroupPlugins( - const PluginSortingData& plugin, + const std::string& groupName, const std::unordered_map>& predecessorGroupsPlugins) { - const auto groupsIt = predecessorGroupsPlugins.find(plugin.GetGroup()); + const auto groupsIt = predecessorGroupsPlugins.find(groupName); if (groupsIt == predecessorGroupsPlugins.end()) { - throw UndefinedGroupError(plugin.GetGroup()); - } - - if (plugin.IsGroupUserMetadata()) { - // If the current plugin is a member of its group due to user metadata, - // then all predecessor plugins are such due to user metadata. - auto predecessorGroupPlugins = groupsIt->second; - for (auto& predecessorGroupPlugin : predecessorGroupPlugins) { - predecessorGroupPlugin.pathInvolvesUserMetadata = true; - } - - return predecessorGroupPlugins; + throw UndefinedGroupError(groupName); } return groupsIt->second; @@ -229,76 +203,88 @@ void IgnorePluginGroupEdges( } } -// Look for paths to targetGroupName from group. Don't pass visitedGroups by -// reference as each after group should be able to record paths independently. -std::unordered_set FindGroupsInAllPaths( - const Group& group, - const std::string& targetGroupName, - const std::unordered_map& groups, - std::unordered_set visitedGroups) { - // If the current group is the target group, return the set of groups in the - // path leading to it. - if (group.GetName() == targetGroupName) { - return visitedGroups; - } +class IntermediateGroupsFinder : public boost::dfs_visitor<> { +public: + typedef boost::graph_traits::edge_descriptor GroupGraphEdge; + typedef boost::graph_traits::vertex_descriptor GroupGraphVertex; - if (group.GetAfterGroups().empty()) { - return std::unordered_set(); - } + explicit IntermediateGroupsFinder( + const GroupGraphVertex& targetGroup, + std::unordered_set& groupNamesInPath) : + targetGroup_(targetGroup), groupNamesInPath_(&groupNamesInPath) {} - visitedGroups.insert(group.GetName()); - - // Recurse on each after group. We want to find all paths, so merge - // all return values. - std::unordered_set mergedVisitedGroups; - for (const auto& afterGroupName : group.GetAfterGroups()) { - const auto groupIt = groups.find(afterGroupName); - if (groupIt == groups.end()) { - throw std::runtime_error("Cannot find group \"" + afterGroupName + - "\" during sorting."); + void tree_edge(GroupGraphEdge edge, const GroupGraph& graph) { + // If this target vertex is the visitor's target group, add all the names of + // the groups in the current path stack to the names set. Also add the names + // if the target vertex name appears in the set of names in the path, as + // that indicates that the current path merges with an already-walked path + // so also ends up at the target vertex. + const auto target = boost::target(edge, graph); + if (target == targetGroup_ || + groupNamesInPath_->count(graph[target]) != 0) { + for (const auto& vertex : pathStack_) { + if (vertex != startVertex_) { + groupNamesInPath_->insert(graph[vertex]); + } + } } - - const auto recursedVisitedGroups = FindGroupsInAllPaths( - groupIt->second, targetGroupName, groups, visitedGroups); - - mergedVisitedGroups.insert(recursedVisitedGroups.begin(), - recursedVisitedGroups.end()); } - // Return mergedVisitedGroups if it is empty, to indicate the current group's - // after groups had no path to the target group. - if (mergedVisitedGroups.empty()) { - return mergedVisitedGroups; + void forward_or_cross_edge(GroupGraphEdge edge, const GroupGraph& graph) { + tree_edge(edge, graph); } - // If any after groups had paths to the target group, mergedVisitedGroups - // will be non-empty. To ensure that it contains full paths, merge it - // with visitedGroups and return that merged set. - visitedGroups.insert(mergedVisitedGroups.begin(), mergedVisitedGroups.end()); + void start_vertex(GroupGraphVertex vertex, const GroupGraph&) { + startVertex_ = vertex; + } - return visitedGroups; + void discover_vertex(GroupGraphVertex vertex, const GroupGraph&) { + pathStack_.push_back(vertex); + } + + void finish_vertex(GroupGraphVertex, const GroupGraph&) { + pathStack_.pop_back(); + } + +private: + GroupGraphVertex targetGroup_{0}; + std::unordered_set* groupNamesInPath_{nullptr}; + + GroupGraphVertex startVertex_{0}; + std::vector pathStack_; +}; + +std::unordered_map GetGroupVertexMap( + const GroupGraph& graph) { + std::unordered_map map; + for (const vertex_t& vertex : + boost::make_iterator_range(boost::vertices(graph))) { + map.emplace(graph[vertex], vertex); + } + + return map; } std::unordered_set FindGroupsInAllPaths( - const std::unordered_map& groups, - const std::string& firstGroupName, - const std::string& lastGroupName) { - // Groups are linked in reverse order, i.e. firstGroup can be found from - // lastGroup, but not the other way around. - const auto groupIt = groups.find(lastGroupName); - if (groupIt == groups.end()) { - throw std::runtime_error("Cannot find group \"" + lastGroupName + - "\" during sorting."); - } + const GroupGraph& groupGraph, + const GroupGraphVertex& fromGroup, + const GroupGraphVertex& toGroup) { + std::vector colorVec( + boost::num_vertices(groupGraph)); + const auto colorMap = boost::make_iterator_property_map( + colorVec.begin(), + boost::get(boost::vertex_index, groupGraph), + colorVec.at(0)); - auto groupsInPaths = FindGroupsInAllPaths(groupIt->second, - firstGroupName, - groups, - std::unordered_set()); + std::unordered_set foundGroups; - groupsInPaths.erase(lastGroupName); + // The starting vertex is the last group in the path, since the group graph's + // edges point from the group that loads after to the group it loads after. + IntermediateGroupsFinder visitor(fromGroup, foundGroups); - return groupsInPaths; + boost::depth_first_visit(groupGraph, toGroup, visitor, colorMap); + + return foundGroups; } std::string describeEdgeType(EdgeType edgeType) { @@ -897,17 +883,18 @@ void PluginGraph::AddHardcodedPluginEdges( } } -void PluginGraph::AddGroupEdges( - const std::unordered_map& groups, - const std::unordered_map>& - predecessorGroupsMap) { +void PluginGraph::AddGroupEdges(const GroupGraph& groupGraph) { const auto logger = getLogger(); if (logger) { logger->trace("Adding edges based on plugin group memberships..."); } + // First build a map from groups to the plugins in those groups. + const auto groupsPlugins = GetGroupsPlugins(*this); + const auto groupVertexMap = GetGroupVertexMap(groupGraph); + const auto predecessorGroupsMap = GetPredecessorGroups(groupGraph); const auto predecessorGroupsPlugins = - GetPredecessorGroupsPlugins(*this, predecessorGroupsMap); + GetPredecessorGroupsPlugins(groupsPlugins, predecessorGroupsMap); // Tuple fields are from, to, and edge type. std::vector> acyclicEdges; @@ -916,17 +903,16 @@ void PluginGraph::AddGroupEdges( for (const vertex_t& vertex : boost::make_iterator_range(GetVertices())) { const auto& toPlugin = GetPlugin(vertex); - const auto predecessorGroupPlugins = - GetPredecessorGroupPlugins(toPlugin, predecessorGroupsPlugins); + const auto predecessorGroupPlugins = GetPredecessorGroupPlugins( + toPlugin.GetGroup(), predecessorGroupsPlugins); for (const auto& plugin : predecessorGroupPlugins) { // After group plugin names are taken from other PluginSortingData names, // so exact string comparisons can be used. const auto parentVertex = plugin.vertex; + const auto& fromPlugin = GetPlugin(parentVertex); if (PathExists(vertex, parentVertex)) { - const auto& fromPlugin = GetPlugin(parentVertex); - if (logger) { logger->debug( "Skipping group edge from \"{}\" to \"{}\" as it would " @@ -961,8 +947,11 @@ void PluginGraph::AddGroupEdges( continue; } - const auto groupsInPaths = FindGroupsInAllPaths( - groups, fromPlugin.GetGroup(), toPlugin.GetGroup()); + const auto fromGroup = groupVertexMap.at(fromPlugin.GetGroup()); + const auto toGroup = groupVertexMap.at(toPlugin.GetGroup()); + + const auto groupsInPaths = + FindGroupsInAllPaths(groupGraph, fromGroup, toGroup); IgnorePluginGroupEdges( pluginToIgnore, groupsInPaths, groupPluginsToIgnore); @@ -970,7 +959,10 @@ void PluginGraph::AddGroupEdges( continue; } - const auto edgeType = plugin.pathInvolvesUserMetadata + const auto edgeInvolvesUserMetadata = + plugin.groupPathInvolvesUserMetadata || + fromPlugin.IsGroupUserMetadata() || toPlugin.IsGroupUserMetadata(); + const auto edgeType = edgeInvolvesUserMetadata ? EdgeType::userGroup : EdgeType::masterlistGroup; diff --git a/src/api/sorting/plugin_graph.h b/src/api/sorting/plugin_graph.h index 5e561767..70bf23d9 100644 --- a/src/api/sorting/plugin_graph.h +++ b/src/api/sorting/plugin_graph.h @@ -104,10 +104,7 @@ public: void AddSpecificEdges(); void AddHardcodedPluginEdges( const std::vector& hardcodedPlugins); - void AddGroupEdges( - const std::unordered_map& groups, - const std::unordered_map>& - predecessorGroupsMap); + void AddGroupEdges(const GroupGraph& groupGraph); void AddOverlapEdges(); void AddTieBreakEdges(); diff --git a/src/api/sorting/plugin_sort.cpp b/src/api/sorting/plugin_sort.cpp index 73d5ffe7..10d23bee 100644 --- a/src/api/sorting/plugin_sort.cpp +++ b/src/api/sorting/plugin_sort.cpp @@ -77,19 +77,6 @@ std::vector GetPluginsSortingData( return pluginsSortingData; } -std::unordered_map GetGroupsMap( - const std::vector masterlistGroups, - const std::vector userGroups) { - const auto mergedGroups = MergeGroups(masterlistGroups, userGroups); - - std::unordered_map groupsMap; - for (const auto& group : mergedGroups) { - groupsMap.emplace(group.GetName(), group); - } - - return groupsMap; -} - bool IsInRange(const std::vector::const_iterator& begin, const std::vector::const_iterator& end, const std::string& name) { @@ -265,9 +252,7 @@ std::vector SortPlugins( const std::vector::const_iterator& begin, const std::vector::const_iterator& end, const std::vector& hardcodedPlugins, - const std::unordered_map& groupsMap, - const std::unordered_map>& - predecessorGroupsMap) { + const GroupGraph& groupGraph) { PluginGraph graph; for (auto it = begin; it != end; ++it) { @@ -278,7 +263,7 @@ std::vector SortPlugins( graph.AddSpecificEdges(); graph.AddHardcodedPluginEdges(hardcodedPlugins); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Check for cycles now because from this point on edges are only added if // they don't cause cycles, and adding tie-break edges is by far the slowest @@ -332,10 +317,7 @@ std::vector SortPlugins( return lhs.GetName() < rhs.GetName(); }); - // Create some shared data structures. - const auto groupsMap = GetGroupsMap(masterlistGroups, userGroups); const auto groupGraph = BuildGroupGraph(masterlistGroups, userGroups); - const auto predecessorGroupsMap = GetPredecessorGroups(groupGraph); // Some parts of sorting are O(N^2) for N plugins, and master flags cause // O(M*N) edges to be added for M masters and N non-masters, which can be @@ -369,20 +351,17 @@ std::vector SortPlugins( auto newMastersLoadOrder = SortPlugins(pluginsSortingData.begin(), firstBlueprintPluginIt, earlyLoadingPlugins, - groupsMap, - predecessorGroupsMap); + groupGraph); const auto newBlueprintMastersLoadOrder = SortPlugins(firstBlueprintPluginIt, firstNonMasterIt, earlyLoadingPlugins, - groupsMap, - predecessorGroupsMap); + groupGraph); const auto newNonMastersLoadOrder = SortPlugins(firstNonMasterIt, pluginsSortingData.end(), earlyLoadingPlugins, - groupsMap, - predecessorGroupsMap); + groupGraph); newMastersLoadOrder.insert(newMastersLoadOrder.end(), newNonMastersLoadOrder.begin(), diff --git a/src/tests/api/internals/sorting/plugin_graph_test.h b/src/tests/api/internals/sorting/plugin_graph_test.h index b56f5062..11b9d3c2 100644 --- a/src/tests/api/internals/sorting/plugin_graph_test.h +++ b/src/tests/api/internals/sorting/plugin_graph_test.h @@ -141,22 +141,7 @@ protected: Group("F", {"E"})}; std::vector userlistGroups{Group("C", {"B"})}; - groupsMap = GetGroupsMap(masterlistGroups, userlistGroups); - const auto groupGraph = BuildGroupGraph(masterlistGroups, userlistGroups); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); - } - - static std::unordered_map GetGroupsMap( - const std::vector masterlistGroups, - const std::vector userGroups) { - const auto mergedGroups = MergeGroups(masterlistGroups, userGroups); - - std::unordered_map groupsMap; - for (const auto& group : mergedGroups) { - groupsMap.emplace(group.GetName(), group); - } - - return groupsMap; + groupGraph = BuildGroupGraph(masterlistGroups, userlistGroups); } PluginSortingData CreatePluginSortingData(const std::string& name) { @@ -194,9 +179,7 @@ protected: return plugins.insert_or_assign(name, plugin).first->second.get(); } - std::unordered_map groupsMap; - std::unordered_map> - predecessorGroupsMap; + GroupGraph groupGraph; private: std::map> plugins; @@ -374,7 +357,7 @@ TEST_F( const auto a = graph.AddVertex(CreatePluginSortingData("A.esp", "A", true)); const auto b = graph.AddVertex(CreatePluginSortingData("B.esp", "B")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Cause a cycle to see the edge types. graph.AddEdge(b, a, EdgeType::master); @@ -399,7 +382,7 @@ TEST_F( const auto a = graph.AddVertex(CreatePluginSortingData("A.esp", "A")); const auto b = graph.AddVertex(CreatePluginSortingData("B.esp", "B", true)); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Cause a cycle to see the edge types. graph.AddEdge(b, a, EdgeType::master); @@ -423,7 +406,7 @@ TEST_F(PluginGraphTest, const auto b = graph.AddVertex(CreatePluginSortingData("B.esp", "B")); const auto d = graph.AddVertex(CreatePluginSortingData("D.esp")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Cause a cycle to see the edge types. graph.AddEdge(d, b, EdgeType::master); @@ -447,7 +430,7 @@ TEST_F(PluginGraphTest, const auto a = graph.AddVertex(CreatePluginSortingData("A.esp", "A")); const auto c = graph.AddVertex(CreatePluginSortingData("C.esp", "C")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Cause a cycle to see the edge types. graph.AddEdge(c, a, EdgeType::master); @@ -471,7 +454,7 @@ TEST_F(PluginGraphTest, const auto a = graph.AddVertex(CreatePluginSortingData("A.esp", "A")); const auto d = graph.AddVertex(CreatePluginSortingData("D.esp")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Cause a cycle to see the edge types. graph.AddEdge(d, a, EdgeType::master); @@ -495,7 +478,7 @@ TEST_F(PluginGraphTest, const auto a = graph.AddVertex(CreatePluginSortingData("A.esp", "A")); const auto b = graph.AddVertex(CreatePluginSortingData("B.esp", "B")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Cause a cycle to see the edge types. graph.AddEdge(b, a, EdgeType::master); @@ -527,7 +510,7 @@ TEST_F( graph.AddEdge(b1, a1, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A2.esp -> B1.esp -> A1.esp -> B2.esp -> C1.esp // -> C2.esp @@ -553,7 +536,7 @@ TEST_F(PluginGraphTest, addGroupEdgesShouldAddEdgesAcrossEmptyGroups) { const auto a = graph.AddVertex(CreatePluginSortingData("A.esp", "A")); const auto c = graph.AddVertex(CreatePluginSortingData("C.esp", "C")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> C.esp EXPECT_TRUE(graph.EdgeExists(a, c)); @@ -569,7 +552,7 @@ TEST_F(PluginGraphTest, const auto d = graph.AddVertex(CreatePluginSortingData("D.esp")); const auto e = graph.AddVertex(CreatePluginSortingData("E.esp", "E")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> D.esp -> E.esp // ----------> @@ -588,7 +571,7 @@ TEST_F(PluginGraphTest, addGroupEdgesShouldSkipAnEdgeThatWouldCauseACycle) { graph.AddEdge(c, a, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be C.esp -> A.esp EXPECT_TRUE(graph.EdgeExists(c, a)); @@ -607,7 +590,7 @@ TEST_F( graph.AddEdge(c, a, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be C.esp -> A.esp -> B.esp EXPECT_TRUE(graph.EdgeExists(c, a)); @@ -640,7 +623,7 @@ TEST_F( graph.AddEdge(c1, a, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be C1.esp -> A.esp -> C2.esp EXPECT_TRUE(graph.EdgeExists(c1, a)); @@ -666,7 +649,7 @@ TEST_F( graph.AddEdge(c, d2, EdgeType::master); graph.AddEdge(c, d3, EdgeType::masterFlag); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be: C.esp -> D2.esp -> B.esp -> D3.esp // -> D1.esp -> @@ -702,7 +685,7 @@ TEST_F( graph.AddEdge(b1, a1, EdgeType::master); graph.AddEdge(c1, b2, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A2.esp -> B1.esp -> A1.esp -> C1.esp -> B2.esp -> C2.esp EXPECT_TRUE(graph.EdgeExists(b1, a1)); @@ -734,7 +717,7 @@ TEST_F( graph.AddEdge(b1, a1, EdgeType::master); graph.AddEdge(c1, b1, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A2.esp -> C1.esp -> B1.esp -> A1.esp -> B2.esp -> C2.esp EXPECT_TRUE(graph.EdgeExists(b1, a1)); @@ -766,7 +749,7 @@ TEST_F( graph.AddEdge(c, b1, EdgeType::master); graph.AddEdge(c, b2, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> C1.esp -> B1.esp // -> B2.esp @@ -800,7 +783,7 @@ TEST_F( graph.AddEdge(d1, c1, EdgeType::master); graph.AddEdge(d2, c1, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be: // A2.esp -> D1.esp -> C1.esp -> B1.esp -> A1.esp -> B2.esp -> C2.esp @@ -837,7 +820,7 @@ TEST_F( graph.AddEdge(d, b, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be D.esp -> B.esp -> C.esp EXPECT_TRUE(graph.EdgeExists(b, c)); @@ -857,7 +840,7 @@ TEST_F( graph.AddEdge(f, d, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be E.esp -> F.esp -> D.esp EXPECT_TRUE(graph.EdgeExists(e, f)); @@ -878,7 +861,7 @@ TEST_F( graph.AddEdge(e, d, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be E.esp -> D.esp -> F.esp EXPECT_TRUE(graph.EdgeExists(e, d)); @@ -900,7 +883,7 @@ TEST_F( graph.AddEdge(f, d2, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be D1.esp -> E.esp -> F.esp -> D2.esp EXPECT_TRUE(graph.EdgeExists(e, f)); @@ -925,7 +908,7 @@ TEST_F( graph.AddEdge(d, b, EdgeType::master); graph.AddEdge(f, d, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // No ideal result, expected is F.esp -> D.esp -> B.esp -> C.esp -> E.esp EXPECT_TRUE(graph.EdgeExists(f, d)); @@ -954,7 +937,7 @@ TEST_F( graph.AddEdge(d2, b, EdgeType::master); graph.AddEdge(f, d1, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be D2.esp -> B.esp -> C.esp -> E.esp -> F.esp -> D1.esp EXPECT_TRUE(graph.EdgeExists(d2, b)); @@ -985,7 +968,7 @@ TEST_F( graph.AddEdge(d4, c, EdgeType::master); graph.AddEdge(f, d1, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be: // D2.esp -> B.esp -> D4.esp -> C.esp -> D3.esp -> E.esp -> F.esp -> D1.esp @@ -1013,9 +996,7 @@ TEST_F(PluginGraphTest, Group("D", {"A"}), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1024,7 +1005,7 @@ TEST_F(PluginGraphTest, const auto c = graph.AddVertex(CreatePluginSortingData("C.esp", "C")); const auto d = graph.AddVertex(CreatePluginSortingData("D.esp", "D")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> B.esp -> C.esp // -> D.esp @@ -1048,9 +1029,7 @@ TEST_F(PluginGraphTest, Group("E", {"C", "D"}), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1060,7 +1039,7 @@ TEST_F(PluginGraphTest, const auto d = graph.AddVertex(CreatePluginSortingData("D.esp", "D")); const auto e = graph.AddVertex(CreatePluginSortingData("E.esp", "E")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> B.esp -> C.esp -> E.esp // -> D.esp ----------> @@ -1086,9 +1065,7 @@ TEST_F( Group("D", {"B", "C"}), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1097,7 +1074,7 @@ TEST_F( const auto c = graph.AddVertex(CreatePluginSortingData("C.esp", "C")); const auto d = graph.AddVertex(CreatePluginSortingData("D.esp", "D")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> B.esp -> D.esp // -> C.esp -> @@ -1121,9 +1098,7 @@ TEST_F( Group("E", {"D"}), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1135,7 +1110,7 @@ TEST_F( graph.AddEdge(d, c, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> B.esp -> D.esp -> C.esp -> E.esp EXPECT_TRUE(graph.EdgeExists(d, c)); @@ -1164,9 +1139,7 @@ TEST_F(PluginGraphTest, Group("G", {"E", "F"}), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); const auto a = graph.AddVertex(CreatePluginSortingData("A.esp", "A")); const auto b = graph.AddVertex(CreatePluginSortingData("B.esp", "B")); @@ -1176,7 +1149,7 @@ TEST_F(PluginGraphTest, const auto f = graph.AddVertex(CreatePluginSortingData("F.esp", "F")); const auto g = graph.AddVertex(CreatePluginSortingData("G.esp", "G")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be: // A.esp -> B.esp -> D.esp -> E.esp -> G.esp @@ -1198,13 +1171,46 @@ TEST_F(PluginGraphTest, EXPECT_NO_THROW(graph.CheckForCycles()); } +TEST_F( + PluginGraphTest, + addGroupEdgesShouldFindAllGroupsInAllPathsBetweenTwoGroupsWhenIgnoringAPlugin) { + PluginGraph graph; + + std::vector masterlistGroups{Group("A"), + Group("B", {"A"}), + Group("C", {"B"}), + Group("D", {"C"}), + Group("default", {"B", "D"})}; + + groupGraph = BuildGroupGraph(masterlistGroups, {}); + + const auto a = graph.AddVertex(CreatePluginSortingData("A.esp", "A")); + const auto b = graph.AddVertex(CreatePluginSortingData("B.esp", "B")); + const auto c = graph.AddVertex(CreatePluginSortingData("C.esp", "C")); + const auto d = graph.AddVertex(CreatePluginSortingData("D.esp", "D")); + const auto e = graph.AddVertex(CreatePluginSortingData("E.esp")); + + graph.AddEdge(e, a, EdgeType::master); + + graph.AddGroupEdges(groupGraph); + + EXPECT_TRUE(graph.EdgeExists(a, b)); + EXPECT_TRUE(graph.EdgeExists(b, c)); + EXPECT_TRUE(graph.EdgeExists(c, d)); + + EXPECT_FALSE(graph.EdgeExists(a, e)); + EXPECT_FALSE(graph.EdgeExists(b, e)); + EXPECT_FALSE(graph.EdgeExists(c, e)); + EXPECT_FALSE(graph.EdgeExists(d, e)); + + EXPECT_NO_THROW(graph.CheckForCycles()); +} + TEST_F(PluginGraphTest, addGroupEdgesShouldHandleIsolatedGroups) { std::vector masterlistGroups{ Group("A"), Group("B", {"A"}), Group("C"), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1212,7 +1218,7 @@ TEST_F(PluginGraphTest, addGroupEdgesShouldHandleIsolatedGroups) { const auto b = graph.AddVertex(CreatePluginSortingData("B.esp", "B")); const auto c = graph.AddVertex(CreatePluginSortingData("C.esp", "C")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> B.esp // C.esp @@ -1229,9 +1235,7 @@ TEST_F(PluginGraphTest, addGroupEdgesShouldHandleDisconnectedGroupGraphs) { std::vector masterlistGroups{ Group("A"), Group("B", {"A"}), Group("C"), Group("D", {"C"}), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1240,7 +1244,7 @@ TEST_F(PluginGraphTest, addGroupEdgesShouldHandleDisconnectedGroupGraphs) { const auto c = graph.AddVertex(CreatePluginSortingData("C.esp", "C")); const auto d = graph.AddVertex(CreatePluginSortingData("D.esp", "D")); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> B.esp // C.esp -> D.esp @@ -1266,9 +1270,7 @@ TEST_F(PluginGraphTest, Group("D", {"C"}), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1279,7 +1281,7 @@ TEST_F(PluginGraphTest, graph.AddEdge(c, b, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> C.esp -> D.esp // B.esp ----------> @@ -1301,9 +1303,7 @@ TEST_F( {Group("C", {"B"}), Group("B"), Group("default", {"C"})}}; for (const auto& masterlistGroups : masterlistsGroups) { - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1313,7 +1313,7 @@ TEST_F( graph.AddEdge(d, b, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be D.esp -> B.esp -> C.esp EXPECT_TRUE(graph.EdgeExists(b, c)); @@ -1341,9 +1341,7 @@ TEST_F( Group("D", {"C"}), Group()}}; for (const auto& masterlistGroups : orders) { - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1354,7 +1352,7 @@ TEST_F( graph.AddEdge(d, a, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be B.esp -> D.esp -> A.esp -> C.esp // B.esp -------------------> @@ -1390,9 +1388,7 @@ TEST_F(PluginGraphTest, Group("E", {"D"}), Group()}}; for (const auto& masterlistGroups : orders) { - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); PluginGraph graph; @@ -1404,7 +1400,7 @@ TEST_F(PluginGraphTest, graph.AddEdge(e, c, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A.esp -> B.esp -> D.esp -> E.esp -> C.esp EXPECT_TRUE(graph.EdgeExists(a, b)); @@ -1432,9 +1428,7 @@ TEST_F(PluginGraphTest, addGroupEdgesShouldNotDependOnPluginGraphVertexOrder) { std::vector masterlistGroups{ Group("A"), Group("B", {"A"}), Group("C", {"B"}), Group()}; - groupsMap = GetGroupsMap(masterlistGroups, {}); - const auto groupGraph = BuildGroupGraph(masterlistGroups, {}); - predecessorGroupsMap = GetPredecessorGroups(groupGraph); + groupGraph = BuildGroupGraph(masterlistGroups, {}); const auto a1Data = CreatePluginSortingData("A1.esp", "A"); const auto a2Data = CreatePluginSortingData("A2.esp", "A"); @@ -1473,7 +1467,7 @@ TEST_F(PluginGraphTest, addGroupEdgesShouldNotDependOnPluginGraphVertexOrder) { graph.AddEdge(c, a1, EdgeType::master); - graph.AddGroupEdges(groupsMap, predecessorGroupsMap); + graph.AddGroupEdges(groupGraph); // Should be A2.esp -> C.esp -> A1.esp -> B.esp // A2.esp -------------------->