Distinguish group edges by data source

If a group edge exists due to the presence of some user metadata,
distinguish that from when the edge exists only due to masterlist
metadata.

The logic for detecting when a path between two groups involves user
metadata gives results that are stable but that do not consistently
paths that only consist of masterlist metadata. If there are multiple
paths between two groups and some involve user metadata, the same path
will be picked every time, but the path picked depends on the structure
of the groups graph.

In practice this shouldn't be much of a problem because the paths are
only exposed when there's a cycle, and in that case all paths between
the two groups need to be removed/broken anyway.

include/loot/enum/edge_type.h

@@ -41,7 +41,8 @@ enum struct EdgeType : unsigned int {
   userRequirement,
   masterlistLoadAfter,
   userLoadAfter,
-  group,
+  masterlistGroup,
+  userGroup,
   overlap,
   tieBreak,
 };

src/api/sorting/group_sort.cpp

@@ -44,21 +44,44 @@ typedef boost::graph_traits<GroupGraph>::vertex_descriptor vertex_t;
 typedef boost::graph_traits<GroupGraph>::edge_descriptor edge_t;
 typedef boost::associative_property_map<std::map<edge_t, int>> edge_map_t;
 
-class AfterGroupsVisitor : public boost::dfs_visitor<> {
+// This visitor is responsible for recording a vertex's successor vertices,
+// and whether they are reachable without user metadata or not. However, a
+// vertex is only recorded the first time it's discovered, vertices and
+// edges are iterated over in their insertion order, and masterlist metadata
+// is inserted first, so it depends on the structure and sources of the data
+// which path is encountered first.
+// E.g. for vertices A, B and C, if C -> B and C -> A are masterlist edges
+// added in that order and B -> A is a userlist edge then C -> B -> A will
+// be visited first and A will then have been finished so C -> A won't be
+// recorded, so it'll look the relationship between C and A involves user
+// metadata when it isn't necessary.
+class PredecessorGroupsVisitor : public boost::dfs_visitor<> {
 public:
-  AfterGroupsVisitor(std::unordered_set<std::string>& visitedGroups) :
+  PredecessorGroupsVisitor(std::vector<PredecessorGroup>& visitedGroups) :
       visitedGroups_(visitedGroups) {}
-  void tree_edge(edge_t edge, const GroupGraph& graph) {
-    auto target = boost::target(edge, graph);
-    visitedGroups_.insert(graph[target]);
+
+  void discover_vertex(vertex_t vertex, const GroupGraph& graph) {
+    pathStack_.push_back(vertex);
+
+    if (pathStack_.size() > 1) {
+      bool pathInvolvesUserMetadata = false;
+      for (size_t i = 0; i < pathStack_.size() - 1; i += 1) {
+        const auto edge = boost::edge(pathStack_[i], pathStack_[i + 1], graph);
+
+        pathInvolvesUserMetadata |=
+            graph[edge.first] == EdgeType::userLoadAfter;
+      }
+
+      visitedGroups_.push_back(
+          PredecessorGroup{graph[vertex], pathInvolvesUserMetadata});
+    }
   }
 
-  std::unordered_set<std::string> get_visited_groups() const {
-    return visitedGroups_;
-  }
+  void finish_vertex(vertex_t, const GroupGraph&) { pathStack_.pop_back(); }
 
 private:
-  std::unordered_set<std::string>& visitedGroups_;
+  std::vector<PredecessorGroup>& visitedGroups_;
+  std::vector<vertex_t> pathStack_;
 };
 
 class CycleDetector : public boost::dfs_visitor<> {
@@ -102,19 +125,20 @@ private:
   std::vector<Vertex> trail;
 };
 
-std::string joinVector(const std::vector<std::string>& set) {
+std::string joinVector(const std::vector<std::string>& container) {
   std::string output;
-  for (const auto& element : set) {
+  for (const auto& element : container) {
     output += element + ", ";
   }
 
   return output.substr(0, output.length() - 2);
 }
 
-std::string joinUnorderedSet(const std::unordered_set<std::string>& set) {
+std::string joinVector(const std::vector<PredecessorGroup>& container) {
   std::string output;
-  for (const auto& element : set) {
-    output += element + ", ";
+  for (const auto& element : container) {
+    const auto via = element.pathInvolvesUserMetadata ? "user" : "masterlist";
+    output += element.name + " (via " + via + " metadata), ";
   }
 
   return output.substr(0, output.length() - 2);
@@ -180,9 +204,9 @@ GroupGraph BuildGraph(const std::vector<Group>& masterlistGroups,
   return graph;
 }
 
-std::unordered_map<std::string, std::unordered_set<std::string>>
-GetTransitiveAfterGroups(const std::vector<Group>& masterlistGroups,
-                         const std::vector<Group>& userGroups) {
+std::unordered_map<std::string, std::vector<PredecessorGroup>>
+GetPredecessorGroups(const std::vector<Group>& masterlistGroups,
+                     const std::vector<Group>& userGroups) {
   GroupGraph graph = BuildGraph(masterlistGroups, userGroups);
 
   auto logger = getLogger();
@@ -196,12 +220,12 @@ GetTransitiveAfterGroups(const std::vector<Group>& masterlistGroups,
   }
   boost::depth_first_search(graph, boost::visitor(CycleDetector()));
 
-  std::unordered_map<std::string, std::unordered_set<std::string>>
+  std::unordered_map<std::string, std::vector<PredecessorGroup>>
       transitiveAfterGroups;
   for (const vertex_t& vertex :
        boost::make_iterator_range(boost::vertices(graph))) {
-    std::unordered_set<std::string> visitedGroups;
-    const AfterGroupsVisitor afterGroupsVisitor(visitedGroups);
+    std::vector<PredecessorGroup> visitedGroups;
+    const PredecessorGroupsVisitor afterGroupsVisitor(visitedGroups);
 
     // Create a color map.
     std::vector<boost::default_color_type> colorVec(boost::num_vertices(graph));
@@ -216,7 +240,7 @@ GetTransitiveAfterGroups(const std::vector<Group>& masterlistGroups,
     if (logger) {
       logger->debug("Group \"{}\" transitively loads after groups \"{}\"",
                     graph[vertex],
-                    joinUnorderedSet(visitedGroups));
+                    joinVector(visitedGroups));
     }
   }
 

src/api/sorting/group_sort.h

@@ -27,17 +27,21 @@
 
 #include <string>
 #include <unordered_map>
-#include <unordered_set>
 #include <vector>
 
 #include "loot/metadata/group.h"
 #include "loot/vertex.h"
 
 namespace loot {
+struct PredecessorGroup {
+  std::string name;
+  bool pathInvolvesUserMetadata{false};
+};
+
 // Map entries are a group name and names of transitive load after groups.
-std::unordered_map<std::string, std::unordered_set<std::string>>
-GetTransitiveAfterGroups(const std::vector<Group>& masterlistGroups,
-                         const std::vector<Group>& userGroups);
+std::unordered_map<std::string, std::vector<PredecessorGroup>>
+GetPredecessorGroups(const std::vector<Group>& masterlistGroups,
+                     const std::vector<Group>& userGroups);
 
 std::vector<Vertex> GetGroupsPath(const std::vector<Group>& masterlistGroups,
                                   const std::vector<Group>& userGroups,

src/api/sorting/plugin_graph.cpp

@@ -211,8 +211,10 @@ std::string describeEdgeType(EdgeType edgeType) {
       return "Masterlist Load After";
     case EdgeType::userLoadAfter:
       return "User Load After";
-    case EdgeType::group:
-      return "Group";
+    case EdgeType::masterlistGroup:
+      return "Masterlist Group";
+    case EdgeType::userGroup:
+      return "User Group";
     case EdgeType::overlap:
       return "Overlap";
     case EdgeType::tieBreak:
@@ -715,16 +717,17 @@ void PluginGraph::AddGroupEdges(
     logger->trace("Adding edges based on plugin group memberships...");
   }
 
-  std::vector<std::pair<vertex_t, vertex_t>> acyclicEdgePairs;
+  // Tuple fields are from, to, and edge type.
+  std::vector<std::tuple<vertex_t, vertex_t, EdgeType>> acyclicEdges;
   std::map<std::string, std::unordered_set<std::string>> groupPluginsToIgnore;
 
   for (const vertex_t& vertex : boost::make_iterator_range(GetVertices())) {
     const auto& toPlugin = GetPlugin(vertex);
 
-    for (const auto& pluginName : toPlugin.GetAfterGroupPlugins()) {
+    for (const auto& plugin : toPlugin.GetPredecessorGroupPlugins()) {
       // After group plugin names are taken from other PluginSortingData names,
       // so exact string comparisons can be used.
-      const auto parentVertex = GetVertexByExactName(pluginName);
+      const auto parentVertex = GetVertexByExactName(plugin.name);
       if (!parentVertex.has_value()) {
         continue;
       }
@@ -775,22 +778,31 @@ void PluginGraph::AddGroupEdges(
         continue;
       }
 
-      acyclicEdgePairs.push_back(std::make_pair(parentVertex.value(), vertex));
+      const auto edgeType = plugin.pathInvolvesUserMetadata
+                                ? EdgeType::userGroup
+                                : EdgeType::masterlistGroup;
+
+      acyclicEdges.push_back(
+          std::make_tuple(parentVertex.value(), vertex, edgeType));
     }
   }
 
-  for (const auto& edgePair : acyclicEdgePairs) {
-    const auto& fromPlugin = GetPlugin(edgePair.first);
-    const auto& toPlugin = GetPlugin(edgePair.second);
+  for (const auto& edge : acyclicEdges) {
+    const auto fromVertex = std::get<0>(edge);
+    const auto toVertex = std::get<1>(edge);
+    const auto edgeType = std::get<2>(edge);
+    const auto& fromPlugin = GetPlugin(fromVertex);
+    const auto& toPlugin = GetPlugin(toVertex);
     const bool ignore =
         ShouldIgnoreGroupEdge(fromPlugin, toPlugin, groupPluginsToIgnore);
 
     if (!ignore) {
-      AddEdge(edgePair.first, edgePair.second, EdgeType::group);
+      AddEdge(fromVertex, toVertex, edgeType);
     } else if (logger) {
       logger->debug(
-          "Skipping group edge from \"{}\" to \"{}\" as it would "
+          "Skipping {} edge from \"{}\" to \"{}\" as it would "
           "create a multi-group cycle.",
+          describeEdgeType(edgeType),
           fromPlugin.GetName(),
           toPlugin.GetName());
     }

src/api/sorting/plugin_sort.cpp

@@ -32,6 +32,39 @@
 #include "loot/exception/undefined_group_error.h"
 
 namespace loot {
+std::unordered_map<std::string, std::vector<PredecessorGroupPlugin>>
+GetPredecessorGroupPlugins(
+    const std::unordered_map<std::string,
+                             std::vector<std::pair<std::string, bool>>>&
+        groupPlugins,
+    const std::unordered_map<std::string, std::vector<PredecessorGroup>>&
+        predecessorGroupsMap) {
+  std::unordered_map<std::string, std::vector<PredecessorGroupPlugin>>
+      predecessorGroupsPlugins;
+  for (const auto& group : predecessorGroupsMap) {
+    std::vector<PredecessorGroupPlugin> predecessorGroupPlugins;
+
+    for (const auto& predecessorGroup : group.second) {
+      const auto pluginsIt = groupPlugins.find(predecessorGroup.name);
+      if (pluginsIt != groupPlugins.end()) {
+        // If the path from the predecessor group to this one involves user
+        // metadata, the plugins' paths all involve user metadata, otherwise
+        // only those plugins that belong to the predecessor group due to user
+        // metadata have a path involving user metadata.
+        for (const auto& groupPlugin : pluginsIt->second) {
+          predecessorGroupPlugins.push_back(PredecessorGroupPlugin{
+              groupPlugin.first,
+              predecessorGroup.pathInvolvesUserMetadata || groupPlugin.second});
+        }
+      }
+    }
+
+    predecessorGroupsPlugins.insert({group.first, predecessorGroupPlugins});
+  }
+
+  return predecessorGroupsPlugins;
+}
+
 int ComparePlugins(const PluginSortingData& plugin1,
                    const PluginSortingData& plugin2) {
   if (plugin1.GetLoadOrderIndex().has_value() &&
@@ -131,33 +164,33 @@ std::vector<PluginSortingData> GetPluginsSortingData(
     pluginsSortingData.push_back(pluginSortingData);
   }
 
-  std::unordered_map<std::string, std::vector<std::string>> groupPlugins;
+  // 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<std::pair<std::string, bool>>>
+      groupPlugins;
   for (const auto& plugin : pluginsSortingData) {
     const auto groupName = plugin.GetGroup();
+    const auto groupPlugin =
+        std::make_pair(plugin.GetName(), plugin.IsGroupUserMetadata());
     const auto groupIt = groupPlugins.find(groupName);
+
     if (groupIt == groupPlugins.end()) {
-      groupPlugins.emplace(groupName,
-                           std::vector<std::string>({plugin.GetName()}));
+      groupPlugins.emplace(
+          groupName, std::vector<std::pair<std::string, bool>>({groupPlugin}));
     } else {
-      groupIt->second.push_back(plugin.GetName());
+      groupIt->second.push_back(groupPlugin);
     }
   }
 
   // Map sets of transitive group dependencies to sets of transitive plugin
   // dependencies.
-  auto groups = GetTransitiveAfterGroups(game.GetDatabase().GetGroups(false),
-                                         game.GetDatabase().GetUserGroups());
-  for (auto& group : groups) {
-    std::unordered_set<std::string> transitivePlugins;
-    for (const auto& afterGroup : group.second) {
-      const auto pluginsIt = groupPlugins.find(afterGroup);
-      if (pluginsIt != groupPlugins.end()) {
-        transitivePlugins.insert(pluginsIt->second.begin(),
-                                 pluginsIt->second.end());
-      }
-    }
-    group.second = transitivePlugins;
-  }
+  const auto predecessorGroupsMap = GetPredecessorGroups(
+      game.GetDatabase().GetGroups(false), game.GetDatabase().GetUserGroups());
+
+  // Replace the transitive after group names with the names of the plugins in
+  // those groups.
+  const auto predecessorGroupsPlugins =
+      GetPredecessorGroupPlugins(groupPlugins, predecessorGroupsMap);
 
   // Add all transitive plugin dependencies for a group to the plugin's load
   // after metadata.
@@ -170,11 +203,22 @@ std::vector<PluginSortingData> GetPluginsSortingData(
           plugin.GetGroup());
     }
 
-    const auto groupsIt = groups.find(plugin.GetGroup());
-    if (groupsIt == groups.end()) {
+    const auto groupsIt = predecessorGroupsPlugins.find(plugin.GetGroup());
+    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;
+      }
+
+      plugin.SetPredecessorGroupPlugins(predecessorGroupPlugins);
     } else {
-      plugin.SetAfterGroupPlugins(groupsIt->second);
+      plugin.SetPredecessorGroupPlugins(groupsIt->second);
     }
   }
 

src/api/sorting/plugin_sorting_data.cpp

@@ -57,18 +57,13 @@ PluginSortingData::PluginSortingData(
     const GameType gameType,
     const std::vector<const PluginInterface*>& loadedPlugins) :
     plugin_(plugin),
+    group_(userMetadata.GetGroup().value_or(
+        masterlistMetadata.GetGroup().value_or(Group::DEFAULT_NAME))),
     masterlistLoadAfter_(masterlistMetadata.GetLoadAfterFiles()),
     userLoadAfter_(userMetadata.GetLoadAfterFiles()),
     masterlistReq_(masterlistMetadata.GetRequirements()),
-    userReq_(userMetadata.GetRequirements()) {
-  if (userMetadata.GetGroup()) {
-    group_ = userMetadata.GetGroup().value();
-  } else if (masterlistMetadata.GetGroup()) {
-    group_ = masterlistMetadata.GetGroup().value();
-  } else {
-    group_ = Group().GetName();
-  }
-
+    userReq_(userMetadata.GetRequirements()),
+    groupIsUserMetadata_(userMetadata.GetGroup().has_value()) {
   if (plugin == nullptr) {
     return;
   }
@@ -148,14 +143,18 @@ bool PluginSortingData::DoAssetsOverlap(const PluginSortingData& plugin) const {
 
 std::string PluginSortingData::GetGroup() const { return group_; }
 
-std::unordered_set<std::string> PluginSortingData::GetAfterGroupPlugins()
-    const {
-  return afterGroupPlugins_;
+bool PluginSortingData::IsGroupUserMetadata() const {
+  return groupIsUserMetadata_;
 }
 
-void PluginSortingData::SetAfterGroupPlugins(
-    std::unordered_set<std::string> plugins) {
-  afterGroupPlugins_ = plugins;
+std::vector<PredecessorGroupPlugin>
+PluginSortingData::GetPredecessorGroupPlugins() const {
+  return predecessorGroupPlugins_;
+}
+
+void PluginSortingData::SetPredecessorGroupPlugins(
+    std::vector<PredecessorGroupPlugin> plugins) {
+  predecessorGroupPlugins_ = plugins;
 }
 
 const std::vector<File>& PluginSortingData::GetMasterlistLoadAfterFiles()

src/api/sorting/plugin_sorting_data.h

@@ -31,6 +31,11 @@
 #include "loot/metadata/plugin_metadata.h"
 
 namespace loot {
+struct PredecessorGroupPlugin {
+  std::string name;
+  bool pathInvolvesUserMetadata{false};
+};
+
 class PluginSortingData {
 public:
   explicit PluginSortingData() = default;
@@ -59,9 +64,10 @@ public:
   bool DoAssetsOverlap(const PluginSortingData& plugin) const;
 
   std::string GetGroup() const;
+  bool IsGroupUserMetadata() const;
 
-  std::unordered_set<std::string> GetAfterGroupPlugins() const;
-  void SetAfterGroupPlugins(std::unordered_set<std::string> plugins);
+  std::vector<PredecessorGroupPlugin> GetPredecessorGroupPlugins() const;
+  void SetPredecessorGroupPlugins(std::vector<PredecessorGroupPlugin> plugins);
 
   const std::vector<File>& GetMasterlistLoadAfterFiles() const;
   const std::vector<File>& GetUserLoadAfterFiles() const;
@@ -73,7 +79,7 @@ public:
 private:
   const PluginSortingInterface* plugin_{nullptr};
   std::string group_;
-  std::unordered_set<std::string> afterGroupPlugins_;
+  std::vector<PredecessorGroupPlugin> predecessorGroupPlugins_;
 
   std::vector<File> masterlistLoadAfter_;
   std::vector<File> userLoadAfter_;
@@ -82,6 +88,7 @@ private:
 
   std::optional<size_t> loadOrderIndex_;
   size_t overrideRecordCount_{0};
+  bool groupIsUserMetadata_{0};
 };
 }
 

src/tests/api/internals/sorting/group_sort_test.h

@@ -32,34 +32,101 @@ along with LOOT.  If not, see
 #include "loot/exception/undefined_group_error.h"
 
 namespace loot {
+bool operator==(const PredecessorGroup& lhs, const PredecessorGroup& rhs) {
+  return lhs.pathInvolvesUserMetadata == rhs.pathInvolvesUserMetadata &&
+         lhs.name == rhs.name;
+}
+
 namespace test {
-TEST(GetTransitiveAfterGroups, shouldMapGroupsToTheirTransitiveAfterGroups) {
+TEST(GetPredecessorGroups, shouldMapGroupsToTheirPredecessorGroups) {
   std::vector<Group> groups({Group("a"), Group("b", {"a"}), Group("c", {"b"})});
 
-  auto mapped = GetTransitiveAfterGroups(groups, {});
+  auto predecessors = GetPredecessorGroups(groups, {});
 
-  EXPECT_TRUE(mapped["a"].empty());
-  EXPECT_EQ(std::unordered_set<std::string>({"a"}), mapped["b"]);
-  EXPECT_EQ(std::unordered_set<std::string>({"a", "b"}), mapped["c"]);
+  EXPECT_TRUE(predecessors["a"].empty());
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"a"}}), predecessors["b"]);
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"b"}, {"a"}}), predecessors["c"]);
 }
 
-TEST(GetTransitiveAfterGroups, shouldThrowIfAnAfterGroupDoesNotExist) {
+TEST(GetPredecessorGroups,
+     shouldRecordIfADirectSuccessorIsDefinedInUserMetadata) {
+  std::vector<Group> masterlistGroups({Group("a")});
+  std::vector<Group> userlistGroups({Group("b", {"a"})});
+
+  auto predecessors = GetPredecessorGroups(masterlistGroups, userlistGroups);
+
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"a", true}}), predecessors["b"]);
+}
+
+TEST(GetPredecessorGroups,
+     shouldRecordIfADirectPredecessorIsLinkedDueToUserMetadata) {
+  std::vector<Group> masterlistGroups({Group("a"), Group("b")});
+  std::vector<Group> userlistGroups({Group("b", {"a"})});
+
+  auto predecessors = GetPredecessorGroups(masterlistGroups, userlistGroups);
+
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"a", true}}), predecessors["b"]);
+}
+
+TEST(GetPredecessorGroups,
+     shouldRecordIfAnIndirectSuccessorIsDefinedInUserMetadata) {
+  std::vector<Group> masterlistGroups({Group("a"), Group("b", {"a"})});
+  std::vector<Group> userlistGroups({Group("c", {"b"})});
+
+  auto predecessors = GetPredecessorGroups(masterlistGroups, userlistGroups);
+
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"a"}}), predecessors["b"]);
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"b", true}, {"a", true}}),
+            predecessors["c"]);
+}
+
+TEST(GetPredecessorGroups,
+     shouldRecordIfAnIndirectPredecessorIsLinkedDueToUserMetadata) {
+  std::vector<Group> masterlistGroups(
+      {Group("a"), Group("b"), Group("c", {"b"})});
+  std::vector<Group> userlistGroups({Group("b", {"a"})});
+
+  auto predecessors = GetPredecessorGroups(masterlistGroups, userlistGroups);
+
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"a", true}}), predecessors["b"]);
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"b"}, {"a", true}}),
+            predecessors["c"]);
+}
+
+TEST(GetPredecessorGroups,
+     shouldNotLeakUserMetadataInvolvementToSeparatePaths) {
+  // This arrangement of groups ensures that a masterlist-sourced edge is
+  // followed after a userlist-sourced edge along a different path, to check
+  // encountering a userlist-sourced edge along one path does not poison
+  // discovery of other paths.
+  std::vector<Group> masterlistGroups(
+      {Group("a"), Group("b"), Group("c"), Group("d", {"b", "c"})});
+  std::vector<Group> userlistGroups({Group("b", {"a"})});
+
+  auto predecessors = GetPredecessorGroups(masterlistGroups, userlistGroups);
+
+  EXPECT_EQ(std::vector<PredecessorGroup>({{"b"}, {"a", true}, {"c"}}),
+            predecessors["d"]);
+}
+
+TEST(GetPredecessorGroups, shouldThrowIfAnAfterGroupDoesNotExist) {
   std::vector<Group> groups({Group("b", {"a"})});
 
-  EXPECT_THROW(GetTransitiveAfterGroups(groups, {}), UndefinedGroupError);
+  EXPECT_THROW(GetPredecessorGroups(groups, {}), UndefinedGroupError);
 }
 
-TEST(GetTransitiveAfterGroups, shouldThrowIfAfterGroupsAreCyclic) {
+TEST(GetPredecessorGroups, shouldThrowIfAfterGroupsAreCyclic) {
   std::vector<Group> groups({Group("a"), Group("b", {"a"})});
   std::vector<Group> userGroups({Group("a", {"c"}), Group("c", {"b"})});
 
   try {
-    GetTransitiveAfterGroups(groups, userGroups);
+    GetPredecessorGroups(groups, userGroups);
     FAIL();
-  } catch (CyclicInteractionError &e) {
+  } catch (CyclicInteractionError& e) {
     ASSERT_EQ(3, e.GetCycle().size());
 
-    // Vertices can be added in any order, so which group is first is undefined.
+    // Vertices can be added in any order, so which group is first is
+    // undefined.
     if (e.GetCycle()[0].GetName() == "a") {
       EXPECT_EQ(EdgeType::userLoadAfter,
                 e.GetCycle()[0].GetTypeOfEdgeToNextVertex());

src/tests/api/internals/sorting/plugin_sort_test.h

@@ -341,20 +341,20 @@ TEST_P(
       EXPECT_EQ("Blank.esm", e.GetCycle()[0].GetName());
       EXPECT_EQ(EdgeType::master, e.GetCycle()[0].GetTypeOfEdgeToNextVertex());
       EXPECT_EQ("Blank - Master Dependent.esm", e.GetCycle()[1].GetName());
-      EXPECT_EQ(EdgeType::group, e.GetCycle()[1].GetTypeOfEdgeToNextVertex());
+      EXPECT_EQ(EdgeType::userGroup, e.GetCycle()[1].GetTypeOfEdgeToNextVertex());
       EXPECT_EQ("Blank - Different.esm", e.GetCycle()[2].GetName());
       EXPECT_EQ(EdgeType::master, e.GetCycle()[2].GetTypeOfEdgeToNextVertex());
       EXPECT_EQ("Blank - Different Master Dependent.esm",
                 e.GetCycle()[3].GetName());
-      EXPECT_EQ(EdgeType::group, e.GetCycle()[3].GetTypeOfEdgeToNextVertex());
+      EXPECT_EQ(EdgeType::userGroup, e.GetCycle()[3].GetTypeOfEdgeToNextVertex());
     } else {
       ASSERT_EQ(3, e.GetCycle().size());
       EXPECT_EQ(masterFile, e.GetCycle()[0].GetName());
-      EXPECT_EQ(EdgeType::group, e.GetCycle()[0].GetTypeOfEdgeToNextVertex());
+      EXPECT_EQ(EdgeType::userGroup, e.GetCycle()[0].GetTypeOfEdgeToNextVertex());
       EXPECT_EQ("Blank.esm", e.GetCycle()[1].GetName());
       EXPECT_EQ(EdgeType::master, e.GetCycle()[1].GetTypeOfEdgeToNextVertex());
       EXPECT_EQ("Blank - Master Dependent.esm", e.GetCycle()[2].GetName());
-      EXPECT_EQ(EdgeType::group, e.GetCycle()[2].GetTypeOfEdgeToNextVertex());
+      EXPECT_EQ(EdgeType::userGroup, e.GetCycle()[2].GetTypeOfEdgeToNextVertex());
     }
   }
 }