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.
2026-07-27 14:16:01 -07:00 · 2025-01-17 23:13:13 +00:00
parent 885c6fd2be
commit 86cdff1f21
4 changed files with 195 additions and 233 deletions
@@ -41,6 +41,8 @@ namespace loot {
 typedef boost::graph_traits<RawPluginGraph>::edge_descriptor edge_t;
 typedef boost::graph_traits<RawPluginGraph>::edge_iterator edge_it;

+typedef boost::graph_traits<GroupGraph>::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<Vertex> 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<std::string, std::vector<PredecessorGroupPlugin>>
 GetPredecessorGroupsPlugins(
-    const std::unordered_map<std::string, std::vector<GroupPlugin>>&
-        groupsPlugins,
+    const std::unordered_map<std::string, std::vector<vertex_t>>& groupsPlugins,
    const std::unordered_map<std::string, std::vector<PredecessorGroup>>&
        predecessorGroupsMap) {
  std::unordered_map<std::string, std::vector<PredecessorGroupPlugin>>
@@ -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<std::string, std::vector<PredecessorGroupPlugin>>
-GetPredecessorGroupsPlugins(
-    const PluginGraph& graph,
-    const std::unordered_map<std::string, std::vector<PredecessorGroup>>&
-        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<std::string, std::vector<GroupPlugin>> groupsPlugins;
+std::unordered_map<std::string, std::vector<vertex_t>> GetGroupsPlugins(
+    const PluginGraph& graph) {
+  std::unordered_map<std::string, std::vector<vertex_t>> 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>({groupPlugin}));
+      groupsPlugins.emplace(groupName, std::vector<vertex_t>({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<PredecessorGroupPlugin> GetPredecessorGroupPlugins(
-    const PluginSortingData& plugin,
+    const std::string& groupName,
    const std::unordered_map<std::string, std::vector<PredecessorGroupPlugin>>&
        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<std::string> FindGroupsInAllPaths(
-    const Group& group,
-    const std::string& targetGroupName,
-    const std::unordered_map<std::string, Group>& groups,
-    std::unordered_set<std::string> 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<GroupGraph>::edge_descriptor GroupGraphEdge;
+  typedef boost::graph_traits<GroupGraph>::vertex_descriptor GroupGraphVertex;

-  if (group.GetAfterGroups().empty()) {
-    return std::unordered_set<std::string>();
-  }
+  explicit IntermediateGroupsFinder(
+      const GroupGraphVertex& targetGroup,
+      std::unordered_set<std::string>& 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<std::string> 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<std::string>* groupNamesInPath_{nullptr};
+
+  GroupGraphVertex startVertex_{0};
+  std::vector<vertex_t> pathStack_;
+};
+
+std::unordered_map<std::string, GroupGraphVertex> GetGroupVertexMap(
+    const GroupGraph& graph) {
+  std::unordered_map<std::string, GroupGraphVertex> map;
+  for (const vertex_t& vertex :
+       boost::make_iterator_range(boost::vertices(graph))) {
+    map.emplace(graph[vertex], vertex);
+  }
+
+  return map;
 }

 std::unordered_set<std::string> FindGroupsInAllPaths(
-    const std::unordered_map<std::string, Group>& 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<boost::default_color_type> 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::string>());
+  std::unordered_set<std::string> 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<std::string, Group>& groups,
-    const std::unordered_map<std::string, std::vector<PredecessorGroup>>&
-        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<std::tuple<vertex_t, vertex_t, EdgeType>> 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;

@@ -104,10 +104,7 @@ public:
  void AddSpecificEdges();
  void AddHardcodedPluginEdges(
      const std::vector<std::string>& hardcodedPlugins);
-  void AddGroupEdges(
-      const std::unordered_map<std::string, Group>& groups,
-      const std::unordered_map<std::string, std::vector<PredecessorGroup>>&
-          predecessorGroupsMap);
+  void AddGroupEdges(const GroupGraph& groupGraph);
  void AddOverlapEdges();
  void AddTieBreakEdges();

@@ -77,19 +77,6 @@ std::vector<PluginSortingData> GetPluginsSortingData(
  return pluginsSortingData;
 }

-std::unordered_map<std::string, Group> GetGroupsMap(
-    const std::vector<Group> masterlistGroups,
-    const std::vector<Group> userGroups) {
-  const auto mergedGroups = MergeGroups(masterlistGroups, userGroups);
-
-  std::unordered_map<std::string, Group> groupsMap;
-  for (const auto& group : mergedGroups) {
-    groupsMap.emplace(group.GetName(), group);
-  }
-
-  return groupsMap;
-}
-
 bool IsInRange(const std::vector<PluginSortingData>::const_iterator& begin,
               const std::vector<PluginSortingData>::const_iterator& end,
               const std::string& name) {
@@ -265,9 +252,7 @@ std::vector<std::string> SortPlugins(
    const std::vector<PluginSortingData>::const_iterator& begin,
    const std::vector<PluginSortingData>::const_iterator& end,
    const std::vector<std::string>& hardcodedPlugins,
-    const std::unordered_map<std::string, Group>& groupsMap,
-    const std::unordered_map<std::string, std::vector<PredecessorGroup>>&
-        predecessorGroupsMap) {
+    const GroupGraph& groupGraph) {
  PluginGraph graph;

  for (auto it = begin; it != end; ++it) {
@@ -278,7 +263,7 @@ std::vector<std::string> 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<std::string> 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<std::string> 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(),
@@ -141,22 +141,7 @@ protected:
                                        Group("F", {"E"})};
    std::vector<Group> userlistGroups{Group("C", {"B"})};

-    groupsMap = GetGroupsMap(masterlistGroups, userlistGroups);
-    const auto groupGraph = BuildGroupGraph(masterlistGroups, userlistGroups);
-    predecessorGroupsMap = GetPredecessorGroups(groupGraph);
-  }
-
-  static std::unordered_map<std::string, Group> GetGroupsMap(
-      const std::vector<Group> masterlistGroups,
-      const std::vector<Group> userGroups) {
-    const auto mergedGroups = MergeGroups(masterlistGroups, userGroups);
-
-    std::unordered_map<std::string, Group> 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<std::string, Group> groupsMap;
-  std::unordered_map<std::string, std::vector<PredecessorGroup>>
-      predecessorGroupsMap;
+  GroupGraph groupGraph;

 private:
  std::map<std::string, std::shared_ptr<plugingraph::TestPlugin>> 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<Group> 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<Group> 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<Group> 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<Group> 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 -------------------->