From 1a6eba1c3b4373f6bc6ff97107571cdc959b73f2 Mon Sep 17 00:00:00 2001
From: Oliver Hamlet <oliver@ortham.io>
Date: Sat, 10 Dec 2022 12:24:20 +0000
Subject: [PATCH] Split PluginGraph::TopologicalSort() into smaller functions

To help with testing.
---
 src/api/sorting/plugin_graph.cpp              | 49 ++++++++++---------
 src/api/sorting/plugin_graph.h                |  9 +++-
 src/api/sorting/plugin_sort.cpp               | 21 +++++++-
 .../api/internals/sorting/plugin_graph_test.h |  2 +-
 4 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/src/api/sorting/plugin_graph.cpp b/src/api/sorting/plugin_graph.cpp
index 9d66c289..3b1e5cc0 100644
--- a/src/api/sorting/plugin_graph.cpp
+++ b/src/api/sorting/plugin_graph.cpp
@@ -334,7 +334,7 @@ void PluginGraph::CheckForCycles() const {
       graph_, visitor(CycleDetector()).vertex_index_map(vertexIndexMap));
 }
 
-std::vector<std::string> PluginGraph::TopologicalSort() const {
+std::vector<vertex_t> PluginGraph::TopologicalSort() const {
   // Build an index map, which std::list-based VertexList graphs don't have.
   std::map<vertex_t, size_t> indexMap;
   const auto vertexIndexMap = vertex_map_t(indexMap);
@@ -343,43 +343,44 @@ std::vector<std::string> PluginGraph::TopologicalSort() const {
     put(vertexIndexMap, v, i++);
   }
 
-  std::list<vertex_t> sortedVertices;
+  std::vector<vertex_t> sortedVertices;
   auto logger = getLogger();
   if (logger) {
     logger->trace("Performing topological sort on plugin graph...");
   }
   boost::topological_sort(graph_,
-                          std::front_inserter(sortedVertices),
+                          std::back_inserter(sortedVertices),
                           boost::vertex_index_map(vertexIndexMap));
 
-  // Check that the sorted path is Hamiltonian (ie. unique).
+  std::reverse(sortedVertices.begin(), sortedVertices.end());
+
+  return sortedVertices;
+}
+
+std::optional<std::pair<vertex_t, vertex_t>> PluginGraph::IsHamiltonianPath(
+    const std::vector<vertex_t>& path) const {
+  auto logger = getLogger();
   if (logger) {
-    logger->trace("Checking uniqueness of calculated load order...");
+    logger->trace("Checking uniqueness of path through plugin graph...");
   }
-  for (auto it = sortedVertices.begin(); it != sortedVertices.end(); ++it) {
-    if (next(it) != sortedVertices.end() &&
-        !boost::edge(*it, *next(it), graph_).second && logger) {
-      logger->error(
-          "The calculated load order is not unique. No edge exists between {} "
-          "and {}.",
-          GetPlugin(*it).GetName(),
-          GetPlugin(*next(it)).GetName());
+
+  for (auto it = path.begin(); it != path.end(); ++it) {
+    if (next(it) != path.end() && !boost::edge(*it, *next(it), graph_).second) {
+      return std::make_pair(*it, *next(it));
     }
   }
 
-  // Output a plugin list using the sorted vertices.
-  if (logger) {
-    logger->info("Calculated order: ");
-  }
-  vector<std::string> plugins;
-  for (const auto& vertex : sortedVertices) {
-    plugins.push_back(GetPlugin(vertex).GetName());
-    if (logger) {
-      logger->info("\t{}", plugins.back());
-    }
+  return std::nullopt;
+}
+
+std::vector<std::string> PluginGraph::ToPluginNames(
+    const std::vector<vertex_t>& path) const {
+  std::vector<std::string> names;
+  for (const auto& vertex : path) {
+    names.push_back(GetPlugin(vertex).GetName());
   }
 
-  return plugins;
+  return names;
 }
 
 bool PluginGraph::EdgeExists(const vertex_t& fromVertex,
diff --git a/src/api/sorting/plugin_graph.h b/src/api/sorting/plugin_graph.h
index 33ccca55..cd41f72f 100644
--- a/src/api/sorting/plugin_graph.h
+++ b/src/api/sorting/plugin_graph.h
@@ -69,7 +69,14 @@ public:
   const PluginSortingData& GetPlugin(const vertex_t& vertex) const;
 
   void CheckForCycles() const;
-  std::vector<std::string> TopologicalSort() const;
+  std::vector<vertex_t> TopologicalSort() const;
+
+  // If the path is not Hamiltonian, returns the first pair of vertices
+  // in the path that do not have an edge between them.
+  std::optional<std::pair<vertex_t, vertex_t>> IsHamiltonianPath(
+      const std::vector<vertex_t>& path) const;
+  std::vector<std::string> ToPluginNames(
+      const std::vector<vertex_t>& path) const;
 
   bool EdgeExists(const vertex_t& fromVertex, const vertex_t& toVertex);
   bool PathExists(const vertex_t& fromVertex, const vertex_t& toVertex);
diff --git a/src/api/sorting/plugin_sort.cpp b/src/api/sorting/plugin_sort.cpp
index a38ab8a7..8948de48 100644
--- a/src/api/sorting/plugin_sort.cpp
+++ b/src/api/sorting/plugin_sort.cpp
@@ -71,6 +71,25 @@ std::vector<std::string> SortPlugins(
   // The check doesn't take a significant amount of time.
   graph.CheckForCycles();
 
-  return graph.TopologicalSort();
+  const auto path = graph.TopologicalSort();
+
+  const auto result = graph.IsHamiltonianPath(path);
+  if (result.has_value() && logger) {
+    logger->error("The path is not unique. No edge exists between {} and {}.",
+                  graph.GetPlugin(result.value().first).GetName(),
+                  graph.GetPlugin(result.value().second).GetName());
+  }
+
+  // Output a plugin list using the sorted vertices.
+  const auto newLoadOrder = graph.ToPluginNames(path);
+
+  if (logger) {
+    logger->info("Calculated order: ");
+    for (const auto& name : newLoadOrder) {
+      logger->info("\t{}", name);
+    }
+  }
+
+  return newLoadOrder;
 }
 }
diff --git a/src/tests/api/internals/sorting/plugin_graph_test.h b/src/tests/api/internals/sorting/plugin_graph_test.h
index 4c561ffc..08dc7846 100644
--- a/src/tests/api/internals/sorting/plugin_graph_test.h
+++ b/src/tests/api/internals/sorting/plugin_graph_test.h
@@ -33,7 +33,7 @@ namespace loot {
 namespace test {
 TEST(PluginGraph, topologicalSortWithNoLoadedPluginsShouldReturnAnEmptyList) {
   PluginGraph graph;
-  std::vector<std::string> sorted = graph.TopologicalSort();
+  std::vector<vertex_t> sorted = graph.TopologicalSort();
 
   EXPECT_TRUE(sorted.empty());
 }