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 -------------------->