Provide the edge types in CyclicInteractionError's cycle path

Edges are given different types based on the source of the edge, e.g.
load after metadata, group metadata, masters.

CMakeLists.txt

@@ -195,6 +195,7 @@ set (LOOT_API_SRC "${CMAKE_BINARY_DIR}/generated/loot_version.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/metadata_list.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/masterlist.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/plugin.cpp"
+                  "${CMAKE_SOURCE_DIR}/src/api/sorting/cyclic_interaction_error.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/sorting/group_sort.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_sorter.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_sorting_data.cpp"

include/loot/exception/cyclic_interaction_error.h

@@ -26,8 +26,57 @@
 #define LOOT_EXCEPTION_CYCLIC_INTERACTION_ERROR
 
 #include <stdexcept>
+#include <string>
+#include <vector>
 
 namespace loot {
+/**
+ * @brief An enum representing the different possible types of interactions
+ *        between plugins or groups.
+ */
+enum struct EdgeType : unsigned int {
+  Hardcoded,
+  MasterFlag,
+  Master,
+  Requirement,
+  LoadAfter,
+  Group,
+  Overlap,
+  TieBreak,
+};
+
+/**
+ * @brief A class representing a plugin or group vertex in a cyclic interaction
+ *        path, and the type of the interaction with the next vertex in the
+ *        path.
+ */
+class Vertex {
+public:
+  /**
+   * @brief Construct a Vertex with the given name and out edge type.
+   * @param name The name of the plugin or group that this vertex represents.
+   * @param outEdgeType The type of the edge going out from this vertex.
+   */
+  Vertex(std::string name, EdgeType outEdgeType);
+
+  /**
+   * @brief Get the name of the plugin or group.
+   * @return The name of the plugin or group.
+  */
+  std::string GetName() const;
+
+  /**
+   * @brief Get the type of the edge going to the next vertex.
+   * @details Each edge goes from the vertex that loads earlier to the vertex
+   *          that loads later.
+   * @return The edge type.
+  */
+  EdgeType GetTypeOfEdgeToNextVertex() const;
+private:
+  std::string name_;
+  EdgeType outEdgeType_;
+};
+
 /**
  * @brief An exception class thrown if a cyclic interaction is detected when
  *        sorting a load order.
@@ -35,45 +84,22 @@ namespace loot {
 class CyclicInteractionError : public std::runtime_error {
 public:
   /**
-   * @brief Construct an exception detailing a plugin graph cycle.
-   * @param firstPlugin A plugin in the cycle.
-   * @param lastPlugin Another plugin in the cycle.
-   * @param backCycle A string describing the path from lastPlugin to
-   *                  firstPlugin.
+   * @brief Construct an exception detailing a plugin or group graph cycle.
+   * @param cycle A representation of the cyclic path.
    */
-  CyclicInteractionError(const std::string& firstPlugin,
-                         const std::string& lastPlugin,
-                         const std::string& backCycle) :
-      std::runtime_error("Cyclic interaction detected between plugins \"" +
-                         firstPlugin + "\" and \"" + lastPlugin +
-                         "\". Back cycle: " + backCycle),
-      firstPlugin_(firstPlugin),
-      lastPlugin_(lastPlugin),
-      backCycle_(backCycle) {}
+  CyclicInteractionError(std::vector<Vertex> cycle);
 
   /**
-   * Get the first plugin in the chosen forward path of the cycle.
-   * @return A plugin filename.
+   * @brief Get a representation of the cyclic path.
+   * @details Each Vertex is the name of a graph element (plugin or group) and
+   *          the type of the edge going to the next Vertex. The last Vertex
+   *          has an edge going to the first Vertex.
+   * @return A vector of Vertex elements representing the cyclic path.
    */
-  std::string getFirstPlugin() { return firstPlugin_; }
-
-  /**
-   * Get the first plugin in the chosen forward path of the cycle.
-   * @return A plugin filename.
-   */
-  std::string getLastPlugin() { return lastPlugin_; }
-
-  /**
-   * Get a description of the reverse path from the chosen last plugin to the
-   * chosen first plugin of the cycle.
-   * @return A string describing a path between two plugins in the plugin graph.
-   */
-  std::string getBackCycle() { return backCycle_; }
+  std::vector<Vertex> GetCycle();
 
 private:
-  const std::string firstPlugin_;
-  const std::string lastPlugin_;
-  const std::string backCycle_;
+  const std::vector<Vertex> cycle_;
 };
 }
 

src/api/sorting/cyclic_interaction_error.cpp

@@ -0,0 +1,79 @@
+/*  LOOT
+
+    A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and
+    Fallout: New Vegas.
+
+    Copyright (C) 2018    WrinklyNinja
+
+    This file is part of LOOT.
+
+    LOOT is free software: you can redistribute
+    it and/or modify it under the terms of the GNU General Public License
+    as published by the Free Software Foundation, either version 3 of
+    the License, or (at your option) any later version.
+
+    LOOT is distributed in the hope that it will
+    be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with LOOT.  If not, see
+    <https://www.gnu.org/licenses/>.
+    */
+#include "loot/exception/cyclic_interaction_error.h"
+
+namespace loot {
+Vertex::Vertex(std::string name, EdgeType outEdgeType) : name_(name), outEdgeType_(outEdgeType) {}
+
+std::string Vertex::GetName() const {
+  return name_;
+}
+
+EdgeType Vertex::GetTypeOfEdgeToNextVertex() const {
+  return outEdgeType_;
+}
+
+std::string describe(EdgeType edgeType) {
+  switch (edgeType) {
+  case EdgeType::Hardcoded:
+    return "Hardcoded";
+  case EdgeType::MasterFlag:
+    return "Master Flag";
+  case EdgeType::Master:
+    return "Master";
+  case EdgeType::Requirement:
+    return "Requirement";
+  case EdgeType::LoadAfter:
+    return "Load After";
+  case EdgeType::Group:
+    return "Group";
+  case EdgeType::Overlap:
+    return "Overlap";
+  case EdgeType::TieBreak:
+    return "Tie Break";
+  default:
+    return "Unknown";
+  }
+}
+
+
+// A.esp --[Master Flag]-> B.esp --Group->
+std::string describeCycle(const std::vector<Vertex>& cycle) {
+  std::string text;
+  for (const auto& vertex : cycle) {
+    text += vertex.GetName() + " --[" + describe(vertex.GetTypeOfEdgeToNextVertex()) + "]-> ";
+  }
+  if (!cycle.empty()) {
+    text += cycle[0].GetName();
+  }
+
+  return text;
+}
+
+CyclicInteractionError::CyclicInteractionError(std::vector<Vertex> cycle) :
+    std::runtime_error("Cyclic interaction detected: " + describeCycle(cycle)),
+    cycle_(cycle) {}
+
+std::vector<Vertex> CyclicInteractionError::GetCycle() { return cycle_; }
+}

src/api/sorting/group_sort.cpp

@@ -36,18 +36,22 @@ namespace loot {
 typedef boost::adjacency_list<boost::vecS,
                               boost::vecS,
                               boost::directedS,
-                              std::string> GroupGraph;
+                              std::string,
+                              EdgeType> GroupGraph;
 typedef boost::graph_traits<GroupGraph>::vertex_descriptor vertex_t;
 typedef boost::graph_traits<GroupGraph>::edge_descriptor edge_t;
 
-class GroupCycleDetector : public boost::dfs_visitor<> {
+class CycleDetector : public boost::dfs_visitor<> {
 public:
   void tree_edge(edge_t edge, const GroupGraph& graph) {
     auto source = boost::source(edge, graph);
-    auto name = graph[source];
+
+    auto vertex = Vertex(graph[source], graph[edge]);
 
     // Check if the plugin already exists in the recorded trail.
-    auto it = find(begin(trail), end(trail), name);
+    auto it = find_if(begin(trail), end(trail), [&](const Vertex& v) {
+      return v.GetName() == graph[source];
+    });
 
     if (it != end(trail)) {
       // Erase everything from this position onwards, as it doesn't
@@ -55,27 +59,27 @@ public:
       trail.erase(it, end(trail));
     }
 
-    trail.push_back(name);
+    trail.push_back(vertex);
   }
 
   void back_edge(edge_t edge, const GroupGraph& graph) {
     auto source = boost::source(edge, graph);
     auto target = boost::target(edge, graph);
 
-    trail.push_back(graph[source]);
-    std::string backCycle;
-    auto it = find(begin(trail), end(trail), graph[target]);
-    for (it; it != end(trail); ++it) {
-      backCycle += *it + ", ";
-    }
-    backCycle.erase(backCycle.length() - 2);
+    auto vertex = Vertex(graph[source], graph[edge]);
+    trail.push_back(vertex);
 
-    throw CyclicInteractionError(
-        graph[source], graph[target], backCycle);
+    auto it = find_if(begin(trail), end(trail), [&](const Vertex& v) {
+      return v.GetName() == graph[target];
+    });
+
+    if (it != trail.end()) {
+      throw CyclicInteractionError(std::vector<Vertex>(it, trail.end()));
+    }
   }
 
 private:
-  std::vector<std::string> trail;
+  std::vector<Vertex> trail;
 };
 
 class AfterGroupsVisitor : public boost::dfs_visitor<> {
@@ -128,7 +132,7 @@ std::unordered_map<std::string, std::unordered_set<std::string>> GetTransitiveAf
       }
 
       auto vertex = groupVertices[group.GetName()];
-      boost::add_edge(vertex, otherVertex->second, graph);
+      boost::add_edge(vertex, otherVertex->second, EdgeType::LoadAfter, graph);
     }
   }
 
@@ -136,7 +140,7 @@ std::unordered_map<std::string, std::unordered_set<std::string>> GetTransitiveAf
   if (logger) {
     logger->trace("Checking for cycles in the group graph");
   }
-  boost::depth_first_search(graph, visitor(GroupCycleDetector()));
+  boost::depth_first_search(graph, boost::visitor(CycleDetector()));
 
   std::unordered_map<std::string, std::unordered_set<std::string>> transitiveAfterGroups;
   for (const vertex_t& vertex : boost::make_iterator_range(boost::vertices(graph))) {

src/api/sorting/plugin_sorter.cpp

@@ -53,10 +53,13 @@ class CycleDetector : public boost::dfs_visitor<> {
 public:
   void tree_edge(edge_t edge, const PluginGraph& graph) {
     const vertex_t source = boost::source(edge, graph);
-    const string name = graph[source].GetName();
+
+    auto vertex = Vertex(graph[source].GetName(), graph[edge]);
 
     // Check if the plugin already exists in the recorded trail.
-    auto it = find(begin(trail), end(trail), name);
+    auto it = find_if(begin(trail), end(trail), [&](const Vertex& v) {
+      return v.GetName() == graph[source].GetName();
+    });
 
     if (it != end(trail)) {
       // Erase everything from this position onwards, as it doesn't
@@ -64,27 +67,27 @@ public:
       trail.erase(it, end(trail));
     }
 
-    trail.push_back(name);
+    trail.push_back(vertex);
   }
 
   void back_edge(edge_t edge, const PluginGraph& graph) {
     vertex_t source = boost::source(edge, graph);
     vertex_t target = boost::target(edge, graph);
 
-    trail.push_back(graph[source].GetName());
-    string backCycle;
-    auto it = find(begin(trail), end(trail), graph[target].GetName());
-    for (it; it != end(trail); ++it) {
-      backCycle += *it + ", ";
-    }
-    backCycle.erase(backCycle.length() - 2);
+    auto vertex = Vertex(graph[source].GetName(), graph[edge]);
+    trail.push_back(vertex);
 
-    throw CyclicInteractionError(
-        graph[source].GetName(), graph[target].GetName(), backCycle);
+    auto it = find_if(begin(trail), end(trail), [&](const Vertex& v) {
+      return v.GetName() == graph[target].GetName();
+    });
+
+    if (it != trail.end()) {
+      throw CyclicInteractionError(std::vector<Vertex>(it, trail.end()));
+    }
   }
 
 private:
-  list<string> trail;
+  vector<Vertex> trail;
 };
 
 std::vector<std::string> PluginSorter::Sort(Game& game) {
@@ -209,8 +212,9 @@ void PluginSorter::AddPluginVertices(Game& game) {
                      plugin->GetName());
     }
 
-    auto metadata =
-        game.GetDatabase()->GetPluginMetadata(plugin->GetName(), true, true).value_or(PluginMetadata(plugin->GetName()));
+    auto metadata = game.GetDatabase()
+                        ->GetPluginMetadata(plugin->GetName(), true, true)
+                        .value_or(PluginMetadata(plugin->GetName()));
 
     auto groupName = metadata.GetGroup().value_or(Group().GetName());
     auto groupIt = groupPlugins.find(groupName);
@@ -314,7 +318,8 @@ bool PluginSorter::EdgeCreatesCycle(const vertex_t& fromVertex,
       if (v == end || reverseVisited.count(v) > 0) {
         return true;
       }
-      for (auto adjacentV : boost::make_iterator_range(boost::adjacent_vertices(v, graph_))) {
+      for (auto adjacentV :
+           boost::make_iterator_range(boost::adjacent_vertices(v, graph_))) {
         if (forwardVisited.count(adjacentV) == 0) {
           forwardVisited.insert(adjacentV);
           forwardQueue.push(adjacentV);
@@ -327,7 +332,8 @@ bool PluginSorter::EdgeCreatesCycle(const vertex_t& fromVertex,
       if (v == start || forwardVisited.count(v) > 0) {
         return true;
       }
-      for (auto adjacentV : boost::make_iterator_range(boost::inv_adjacent_vertices(v, graph_))) {
+      for (auto adjacentV : boost::make_iterator_range(
+               boost::inv_adjacent_vertices(v, graph_))) {
         if (reverseVisited.count(adjacentV) == 0) {
           reverseVisited.insert(adjacentV);
           reverseQueue.push(adjacentV);
@@ -340,7 +346,8 @@ bool PluginSorter::EdgeCreatesCycle(const vertex_t& fromVertex,
 }
 
 void PluginSorter::AddEdge(const vertex_t& fromVertex,
-                           const vertex_t& toVertex) {
+                           const vertex_t& toVertex,
+                           EdgeType edgeType) {
   if (!boost::edge(fromVertex, toVertex, graph_).second) {
     if (logger_) {
       logger_->trace("Adding edge from \"{}\" to \"{}\".",
@@ -348,7 +355,7 @@ void PluginSorter::AddEdge(const vertex_t& fromVertex,
                      graph_[toVertex].GetName());
     }
 
-    boost::add_edge(fromVertex, toVertex, graph_);
+    boost::add_edge(fromVertex, toVertex, edgeType, graph_);
   }
 }
 
@@ -367,18 +374,18 @@ void PluginSorter::AddHardcodedPluginEdges(Game& game) {
 
     try {
       processedPluginPaths.insert(std::filesystem::canonical(pluginPath));
-    }
-    catch (std::filesystem::filesystem_error&) {
+    } catch (std::filesystem::filesystem_error&) {
       if (logger_) {
         logger_->trace(
-          "Skipping adding hardcoded plugin edges for \"{}\" as it is not "
-          "installed.",
-          plugin);
+            "Skipping adding hardcoded plugin edges for \"{}\" as it is not "
+            "installed.",
+            plugin);
       }
       continue;
     }
 
-    if (game.Type() == GameType::tes5 && loot::equivalent(plugin, "update.esm")) {
+    if (game.Type() == GameType::tes5 &&
+        loot::equivalent(plugin, "update.esm")) {
       if (logger_) {
         logger_->trace(
             "Skipping adding hardcoded plugin edges for Update.esm as it does "
@@ -412,8 +419,9 @@ void PluginSorter::AddHardcodedPluginEdges(Game& game) {
         continue;
       }
 
-      if (processedPluginPaths.count(std::filesystem::canonical(graphPluginPath)) == 0) {
-        AddEdge(pluginVertex, *vit);
+      if (processedPluginPaths.count(
+              std::filesystem::canonical(graphPluginPath)) == 0) {
+        AddEdge(pluginVertex, *vit, EdgeType::Hardcoded);
       }
     }
   }
@@ -442,7 +450,7 @@ void PluginSorter::AddSpecificEdges() {
         vertex = *vit2;
       }
 
-      AddEdge(parentVertex, vertex);
+      AddEdge(parentVertex, vertex, EdgeType::MasterFlag);
     }
 
     vertex_t parentVertex;
@@ -451,7 +459,7 @@ void PluginSorter::AddSpecificEdges() {
     }
     for (const auto& master : graph_[*vit].GetMasters()) {
       if (GetVertexByName(master, parentVertex))
-        AddEdge(parentVertex, *vit);
+        AddEdge(parentVertex, *vit, EdgeType::Master);
     }
 
     if (logger_) {
@@ -459,7 +467,7 @@ void PluginSorter::AddSpecificEdges() {
     }
     for (const auto& file : graph_[*vit].GetRequirements()) {
       if (GetVertexByName(file.GetName(), parentVertex))
-        AddEdge(parentVertex, *vit);
+        AddEdge(parentVertex, *vit, EdgeType::Requirement);
     }
 
     if (logger_) {
@@ -467,7 +475,7 @@ void PluginSorter::AddSpecificEdges() {
     }
     for (const auto& file : graph_[*vit].GetLoadAfterFiles()) {
       if (GetVertexByName(file.GetName(), parentVertex))
-        AddEdge(parentVertex, *vit);
+        AddEdge(parentVertex, *vit, EdgeType::LoadAfter);
     }
   }
 }
@@ -506,7 +514,7 @@ void ignorePlugin(const std::string& pluginName,
       pluginsToIgnore->second.insert(pluginName);
     } else {
       groupPluginsToIgnore.emplace(
-          group, std::unordered_set<std::string>({ pluginName }));
+          group, std::unordered_set<std::string>({pluginName}));
     }
   }
 }
@@ -640,7 +648,7 @@ void PluginSorter::AddGroupEdges() {
         shouldIgnoreGroupEdge(fromPlugin, toPlugin, groupPluginsToIgnore);
 
     if (!ignore) {
-      AddEdge(edgePair.first, edgePair.second);
+      AddEdge(edgePair.first, edgePair.second, EdgeType::Group);
     } else if (logger_) {
       logger_->trace(
           "Skipping edge from \"{}\" to \"{}\" as it would "
@@ -691,7 +699,7 @@ void PluginSorter::AddOverlapEdges() {
       }
 
       if (!EdgeCreatesCycle(fromVertex, toVertex))
-        AddEdge(fromVertex, toVertex);
+        AddEdge(fromVertex, toVertex, EdgeType::Overlap);
     }
   }
 }
@@ -764,7 +772,7 @@ void PluginSorter::AddTieBreakEdges() {
       }
 
       if (!EdgeCreatesCycle(fromVertex, toVertex))
-        AddEdge(fromVertex, toVertex);
+        AddEdge(fromVertex, toVertex, EdgeType::TieBreak);
     }
   }
 }

src/api/sorting/plugin_sorter.h

@@ -36,12 +36,14 @@
 #include "api/game/game.h"
 #include "api/plugin.h"
 #include "api/sorting/plugin_sorting_data.h"
+#include "loot/exception/cyclic_interaction_error.h"
 
 namespace loot {
 typedef boost::adjacency_list<boost::listS,
                               boost::listS,
                               boost::bidirectionalS,
-                              PluginSortingData>
+                              PluginSortingData,
+                              EdgeType>
     PluginGraph;
 typedef boost::graph_traits<PluginGraph>::vertex_descriptor vertex_t;
 typedef boost::associative_property_map<std::map<vertex_t, size_t>>
@@ -66,7 +68,9 @@ private:
   void AddOverlapEdges();
   void AddTieBreakEdges();
 
-  void AddEdge(const vertex_t& fromVertex, const vertex_t& toVertex);
+  void AddEdge(const vertex_t& fromVertex,
+               const vertex_t& toVertex,
+               EdgeType edgeType);
 
   PluginGraph graph_;
   std::map<vertex_t, size_t> indexMap_;

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

@@ -63,7 +63,29 @@ TEST(GetTransitiveAfterGroups, shouldThrowIfAfterGroupsAreCyclic) {
     Group("c", std::unordered_set<std::string>({ "b" }))
   });
 
-  EXPECT_THROW(GetTransitiveAfterGroups(groups), CyclicInteractionError);
+  try {
+    GetTransitiveAfterGroups(groups);
+    FAIL();
+  }
+  catch (CyclicInteractionError &e) {
+    ASSERT_EQ(3, e.GetCycle().size());
+    EXPECT_EQ(EdgeType::LoadAfter, e.GetCycle()[0].GetTypeOfEdgeToNextVertex());
+    EXPECT_EQ(EdgeType::LoadAfter, e.GetCycle()[1].GetTypeOfEdgeToNextVertex());
+    EXPECT_EQ(EdgeType::LoadAfter, e.GetCycle()[2].GetTypeOfEdgeToNextVertex());
+
+    // Vertices can be added in any order, so which group is first is undefined.
+    if (e.GetCycle()[0].GetName() == "a") {
+      EXPECT_EQ("c", e.GetCycle()[1].GetName());
+      EXPECT_EQ("b", e.GetCycle()[2].GetName());
+    } else if (e.GetCycle()[0].GetName() == "b") {
+      EXPECT_EQ("a", e.GetCycle()[1].GetName());
+      EXPECT_EQ("c", e.GetCycle()[2].GetName());
+    } else {
+      EXPECT_EQ("c", e.GetCycle()[0].GetName());
+      EXPECT_EQ("b", e.GetCycle()[1].GetName());
+      EXPECT_EQ("a", e.GetCycle()[2].GetName());
+    }
+  }
 }
 }
 }

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

@@ -373,8 +373,19 @@ TEST_P(
   plugin.SetGroup("group4");
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
-  EXPECT_THROW(ps.Sort(game_), CyclicInteractionError);
+  try {
+    PluginSorter ps;
+    ps.Sort(game_);
+    FAIL();
+  } catch (CyclicInteractionError &e) {
+    ASSERT_EQ(3, e.GetCycle().size());
+    EXPECT_EQ("Blank - Different Master Dependent.esm", e.GetCycle()[0].GetName());
+    EXPECT_EQ(EdgeType::Group, 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());
+  }
 }
 
 TEST_P(