Refactor plugin sorting

This should make it easier to unit test building the plugin graph.

CMakeLists.txt

@@ -213,7 +213,8 @@ set (LIBLOOT_SRC "${CMAKE_BINARY_DIR}/generated/loot_version.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_sort.cpp"
+                  "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_graph.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_sorting_data.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/helpers/crc.cpp"
                   "${CMAKE_SOURCE_DIR}/src/api/helpers/git_helper.cpp"
@@ -267,7 +268,8 @@ set (LIBLOOT_HEADERS "${CMAKE_SOURCE_DIR}/include/loot/api.h"
                       "${CMAKE_SOURCE_DIR}/src/api/masterlist.h"
                       "${CMAKE_SOURCE_DIR}/src/api/plugin.h"
                       "${CMAKE_SOURCE_DIR}/src/api/sorting/group_sort.h"
-                      "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_sorter.h"
+                      "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_sort.h"
+                      "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_graph.h"
                       "${CMAKE_SOURCE_DIR}/src/api/sorting/plugin_sorting_data.h"
                       "${CMAKE_SOURCE_DIR}/src/api/helpers/git_helper.h"
                       "${CMAKE_SOURCE_DIR}/src/api/helpers/crc.h"
@@ -295,7 +297,8 @@ set (LOOT_TESTS_HEADERS "${CMAKE_SOURCE_DIR}/src/tests/api/internals/game/game_t
                         "${CMAKE_SOURCE_DIR}/src/tests/api/internals/metadata/tag_test.h"
                         "${CMAKE_SOURCE_DIR}/src/tests/api/internals/plugin_test.h"
                         "${CMAKE_SOURCE_DIR}/src/tests/api/internals/sorting/group_sort_test.h"
-                        "${CMAKE_SOURCE_DIR}/src/tests/api/internals/sorting/plugin_sorter_test.h"
+                        "${CMAKE_SOURCE_DIR}/src/tests/api/internals/sorting/plugin_sort_test.h"
+                        "${CMAKE_SOURCE_DIR}/src/tests/api/internals/sorting/plugin_graph_test.h"
                         "${CMAKE_SOURCE_DIR}/src/tests/api/internals/sorting/plugin_sorting_data_test.h"
                         "${CMAKE_SOURCE_DIR}/src/tests/api/internals/masterlist_test.h"
                         "${CMAKE_SOURCE_DIR}/src/tests/api/internals/metadata_list_test.h"

src/api/api_database.cpp

@@ -30,7 +30,7 @@
 #include "api/game/game.h"
 #include "api/metadata/condition_evaluator.h"
 #include "api/metadata/yaml/plugin_metadata.h"
-#include "api/sorting/plugin_sorter.h"
+#include "api/sorting/plugin_sort.h"
 #include "api/sorting/group_sort.h"
 #include "loot/metadata/group.h"
 #include "loot/exception/file_access_error.h"

src/api/game/game.cpp

@@ -26,13 +26,14 @@
 
 #include <algorithm>
 #include <cmath>
+#include <map>
 #include <thread>
 
 #include <boost/algorithm/string.hpp>
 
 #include "api/api_database.h"
 #include "api/helpers/logging.h"
-#include "api/sorting/plugin_sorter.h"
+#include "api/sorting/plugin_sort.h"
 #include "loot/exception/file_access_error.h"
 
 #ifdef _WIN32
@@ -220,8 +221,7 @@ std::vector<std::string> Game::SortPlugins(
   LoadPlugins(plugins, false);
 
   // Sort plugins into their load order.
-  PluginSorter sorter;
-  return sorter.Sort(*this);
+  return loot::SortPlugins(*this);
 }
 
 void Game::LoadCurrentLoadOrderState() {

src/api/sorting/cyclic_interaction_error.cpp

@@ -23,7 +23,7 @@
     */
 #include "loot/exception/cyclic_interaction_error.h"
 
-#include "api/sorting/plugin_sorter.h"
+#include "api/sorting/plugin_graph.h"
 
 namespace loot {
 // A.esp --[Master Flag]-> B.esp --Group->

src/api/sorting/group_sort.h

@@ -30,10 +30,7 @@
 #include <unordered_set>
 #include <vector>
 
-#include <boost/graph/adjacency_list.hpp>
-#include <boost/graph/depth_first_search.hpp>
-
-#include "loot/exception/cyclic_interaction_error.h"
+#include "loot/vertex.h"
 #include "loot/metadata/group.h"
 
 namespace loot {

src/api/sorting/plugin_graph.cpp

@@ -22,7 +22,7 @@
     <https://www.gnu.org/licenses/>.
     */
 
-#include "plugin_sorter.h"
+#include "plugin_graph.h"
 
 #include <cstdlib>
 #include <queue>
@@ -45,13 +45,13 @@ using std::string;
 using std::vector;
 
 namespace loot {
-typedef boost::graph_traits<PluginGraph>::vertex_iterator vertex_it;
-typedef boost::graph_traits<PluginGraph>::edge_descriptor edge_t;
-typedef boost::graph_traits<PluginGraph>::edge_iterator edge_it;
+typedef boost::graph_traits<RawPluginGraph>::vertex_iterator vertex_it;
+typedef boost::graph_traits<RawPluginGraph>::edge_descriptor edge_t;
+typedef boost::graph_traits<RawPluginGraph>::edge_iterator edge_it;
 
 class CycleDetector : public boost::dfs_visitor<> {
 public:
-  void tree_edge(edge_t edge, const PluginGraph& graph) {
+  void tree_edge(edge_t edge, const RawPluginGraph& graph) {
     auto source = boost::source(edge, graph);
 
     auto vertex = Vertex(graph[source].GetName(), graph[edge]);
@@ -70,7 +70,7 @@ public:
     trail.push_back(vertex);
   }
 
-  void back_edge(edge_t edge, const PluginGraph& graph) {
+  void back_edge(edge_t edge, const RawPluginGraph& graph) {
     auto source = boost::source(edge, graph);
     auto target = boost::target(edge, graph);
 
@@ -117,54 +117,34 @@ std::string describeEdgeType(EdgeType edgeType) {
   }
 }
 
-std::vector<std::string> PluginSorter::Sort(Game& game) {
-  logger_ = getLogger();
+size_t PluginGraph::CountVertices() const {
+  return boost::num_vertices(graph_);
+}
 
-  // Clear existing data.
-  graph_.clear();
-  indexMap_.clear();
-  pathsCache_.clear();
+std::vector<std::string> PluginGraph::TopologicalSort() const {
+  // Build an index map, which std::list-based VertexList graphs don't have.
+  std::map<vertex_t, size_t> indexMap;
+  auto vertexIndexMap = vertex_map_t(indexMap);
+  size_t i = 0;
+  BGL_FORALL_VERTICES(v, graph_, RawPluginGraph) { put(vertexIndexMap, v, i++); }
 
-  AddPluginVertices(game);
-
-  // If there aren't any vertices, exit early, because sorting assumes
-  // there is at least one plugin.
-  if (boost::num_vertices(graph_) == 0)
-    return vector<std::string>();
-
-  if (logger_) {
-    logger_->info("Current load order: ");
-    for (const auto& plugin : game.GetLoadOrder()) {
-      logger_->info("\t\t{}", plugin);
-    }
-  }
-
-  // Now add the interactions between plugins to the graph as edges.
-  AddSpecificEdges();
-  AddHardcodedPluginEdges(game);
-  AddGroupEdges();
-  AddOverlapEdges();
-  AddTieBreakEdges();
-
-  CheckForCycles();
-
-  // Now we can sort.
   list<vertex_t> sortedVertices;
-  if (logger_) {
-    logger_->trace("Performing topological sort on plugin graph...");
+  auto logger = getLogger();
+  if (logger) {
+    logger->trace("Performing topological sort on plugin graph...");
   }
   boost::topological_sort(graph_,
                           std::front_inserter(sortedVertices),
-                          boost::vertex_index_map(vertexIndexMap_));
+                          boost::vertex_index_map(vertexIndexMap));
 
   // Check that the sorted path is Hamiltonian (ie. unique).
-  if (logger_) {
-    logger_->trace("Checking uniqueness of calculated load order...");
+  if (logger) {
+    logger->trace("Checking uniqueness of calculated load order...");
   }
   for (auto it = sortedVertices.begin(); it != sortedVertices.end(); ++it) {
     if (next(it) != sortedVertices.end() &&
-        !boost::edge(*it, *next(it), graph_).second && logger_) {
-      logger_->error(
+        !boost::edge(*it, *next(it), graph_).second && logger) {
+      logger->error(
           "The calculated load order is not unique. No edge exists between {} "
           "and {}.",
           graph_[*it].GetName(),
@@ -173,21 +153,21 @@ std::vector<std::string> PluginSorter::Sort(Game& game) {
   }
 
   // Output a plugin list using the sorted vertices.
-  if (logger_) {
-    logger_->info("Calculated order: ");
+  if (logger) {
+    logger->info("Calculated order: ");
   }
   vector<std::string> plugins;
   for (const auto& vertex : sortedVertices) {
     plugins.push_back(graph_[vertex].GetName());
-    if (logger_) {
-      logger_->info("\t{}", plugins.back());
+    if (logger) {
+      logger->info("\t{}", plugins.back());
     }
   }
 
   return plugins;
 }
 
-void PluginSorter::AddPluginVertices(Game& game) {
+void PluginGraph::AddPluginVertices(Game& game) {
   // The resolution of tie-breaks in the plugin graph may be dependent
   // on the order in which vertices are iterated over, as an earlier tie
   // break resolution may cause a potential later tie break to instead
@@ -244,8 +224,6 @@ void PluginSorter::AddPluginVertices(Game& game) {
 
   // Map sets of transitive group dependencies to sets of transitive plugin
   // dependencies.
-  groups_ = game.GetDatabase()->GetGroups();
-
   auto groups = GetTransitiveAfterGroups(game.GetDatabase()->GetGroups(false),
                                          game.GetDatabase()->GetUserGroups());
   for (auto& group : groups) {
@@ -262,12 +240,13 @@ void PluginSorter::AddPluginVertices(Game& game) {
 
   // Add all transitive plugin dependencies for a group to the plugin's load
   // after metadata.
+  auto logger = getLogger();
   for (const auto& vertex :
        boost::make_iterator_range(boost::vertices(graph_))) {
     PluginSortingData& plugin = graph_[vertex];
 
-    if (logger_) {
-      logger_->trace(
+    if (logger) {
+      logger->trace(
           "Plugin \"{}\" belongs to group \"{}\", setting after group plugins",
           plugin.GetName(),
           plugin.GetGroup());
@@ -280,15 +259,9 @@ void PluginSorter::AddPluginVertices(Game& game) {
       plugin.SetAfterGroupPlugins(groupsIt->second);
     }
   }
-
-  // Prebuild an index map, which std::list-based VertexList graphs don't have.
-  vertexIndexMap_ = vertex_map_t(indexMap_);
-  size_t i = 0;
-  BGL_FORALL_VERTICES(v, graph_, PluginGraph)
-  put(vertexIndexMap_, v, i++);
 }
 
-std::optional<vertex_t> PluginSorter::GetVertexByName(
+std::optional<vertex_t> PluginGraph::GetVertexByName(
     const std::string& name) const {
   for (const auto& vertex :
        boost::make_iterator_range(boost::vertices(graph_))) {
@@ -300,15 +273,22 @@ std::optional<vertex_t> PluginSorter::GetVertexByName(
   return std::nullopt;
 }
 
-void PluginSorter::CheckForCycles() const {
-  if (logger_) {
-    logger_->trace("Checking plugin graph for cycles...");
+void PluginGraph::CheckForCycles() const {
+  auto logger = getLogger();
+  if (logger) {
+    logger->trace("Checking plugin graph for cycles...");
   }
+
+  std::map<vertex_t, size_t> indexMap;
+  auto vertexIndexMap = vertex_map_t(indexMap);
+  size_t i = 0;
+  BGL_FORALL_VERTICES(v, graph_, RawPluginGraph) { put(vertexIndexMap, v, i++); }
+
   boost::depth_first_search(
-      graph_, visitor(CycleDetector()).vertex_index_map(vertexIndexMap_));
+      graph_, visitor(CycleDetector()).vertex_index_map(vertexIndexMap));
 }
 
-bool PluginSorter::EdgeCreatesCycle(const vertex_t& fromVertex,
+bool PluginGraph::EdgeCreatesCycle(const vertex_t& fromVertex,
                                     const vertex_t& toVertex) {
   if (pathsCache_.count(GraphPath(toVertex, fromVertex)) != 0) {
     return true;
@@ -365,7 +345,7 @@ bool PluginSorter::EdgeCreatesCycle(const vertex_t& fromVertex,
   return false;
 }
 
-void PluginSorter::AddEdge(const vertex_t& fromVertex,
+void PluginGraph::AddEdge(const vertex_t& fromVertex,
                            const vertex_t& toVertex,
                            EdgeType edgeType) {
   auto graphPath = GraphPath(fromVertex, toVertex);
@@ -374,8 +354,9 @@ void PluginSorter::AddEdge(const vertex_t& fromVertex,
     return;
   }
 
-  if (logger_) {
-    logger_->trace("Adding {} edge from \"{}\" to \"{}\".",
+  auto logger = getLogger();
+  if (logger) {
+    logger->trace("Adding {} edge from \"{}\" to \"{}\".",
                    describeEdgeType(edgeType),
                    graph_[fromVertex].GetName(),
                    graph_[toVertex].GetName());
@@ -385,12 +366,13 @@ void PluginSorter::AddEdge(const vertex_t& fromVertex,
   pathsCache_.insert(graphPath);
 }
 
-void PluginSorter::AddHardcodedPluginEdges(Game& game) {
+void PluginGraph::AddHardcodedPluginEdges(Game& game) {
   using std::filesystem::u8path;
 
   auto implicitlyActivePlugins =
       game.GetLoadOrderHandler()->GetImplicitlyActivePlugins();
 
+  auto logger = getLogger();
   std::set<std::filesystem::path> processedPluginPaths;
   for (const auto& plugin : implicitlyActivePlugins) {
     auto pluginPath = game.DataPath() / u8path(plugin);
@@ -398,8 +380,8 @@ void PluginSorter::AddHardcodedPluginEdges(Game& game) {
     try {
       processedPluginPaths.insert(std::filesystem::canonical(pluginPath));
     } catch (std::filesystem::filesystem_error& e) {
-      if (logger_) {
-        logger_->trace(
+      if (logger) {
+        logger->trace(
             "Skipping adding hardcoded plugin edges for \"{}\" as its "
             "canonical path could not be determined: {}",
             plugin,
@@ -410,8 +392,8 @@ void PluginSorter::AddHardcodedPluginEdges(Game& game) {
 
     if (game.Type() == GameType::tes5 &&
         loot::equivalent(plugin, "update.esm")) {
-      if (logger_) {
-        logger_->trace(
+      if (logger) {
+        logger->trace(
             "Skipping adding hardcoded plugin edges for Update.esm as it does "
             "not have a hardcoded position for Skyrim.");
         continue;
@@ -421,8 +403,8 @@ void PluginSorter::AddHardcodedPluginEdges(Game& game) {
     auto pluginVertex = GetVertexByName(plugin);
 
     if (!pluginVertex.has_value()) {
-      if (logger_) {
-        logger_->trace(
+      if (logger) {
+        logger->trace(
             "Skipping adding hardcoded plugin edges for \"{}\" as it has not "
             "been loaded.",
             plugin);
@@ -451,7 +433,7 @@ void PluginSorter::AddHardcodedPluginEdges(Game& game) {
   }
 }
 
-void PluginSorter::AddSpecificEdges() {
+void PluginGraph::AddSpecificEdges() {
   // Add edges for all relationships that aren't overlaps.
   vertex_it vit, vitend;
   for (tie(vit, vitend) = boost::vertices(graph_); vit != vitend; ++vit) {
@@ -607,9 +589,11 @@ std::unordered_set<std::string> getGroupsInPaths(
   return groupsInPaths;
 }
 
-void PluginSorter::AddGroupEdges() {
+void PluginGraph::AddGroupEdges(const std::unordered_set<Group>& groups) {
   std::vector<std::pair<vertex_t, vertex_t>> acyclicEdgePairs;
   std::map<std::string, std::unordered_set<std::string>> groupPluginsToIgnore;
+
+  auto logger = getLogger();
   for (const vertex_t& vertex :
        boost::make_iterator_range(boost::vertices(graph_))) {
     for (const auto& pluginName : graph_[vertex].GetAfterGroupPlugins()) {
@@ -622,8 +606,8 @@ void PluginSorter::AddGroupEdges() {
         auto& fromPlugin = graph_[parentVertex.value()];
         auto& toPlugin = graph_[vertex];
 
-        if (logger_) {
-          logger_->trace(
+        if (logger) {
+          logger->trace(
               "Skipping group edge from \"{}\" to \"{}\" as it would "
               "create a cycle.",
               fromPlugin.GetName(),
@@ -657,7 +641,7 @@ void PluginSorter::AddGroupEdges() {
         }
 
         auto groupsInPaths = getGroupsInPaths(
-            groups_, fromPlugin.GetGroup(), toPlugin.GetGroup());
+            groups, fromPlugin.GetGroup(), toPlugin.GetGroup());
 
         ignorePlugin(pluginToIgnore, groupsInPaths, groupPluginsToIgnore);
 
@@ -676,8 +660,8 @@ void PluginSorter::AddGroupEdges() {
 
     if (!ignore) {
       AddEdge(edgePair.first, edgePair.second, EdgeType::group);
-    } else if (logger_) {
-      logger_->trace(
+    } else if (logger) {
+      logger->trace(
           "Skipping group edge from \"{}\" to \"{}\" as it would "
           "create a multi-group cycle.",
           fromPlugin.GetName(),
@@ -686,14 +670,15 @@ void PluginSorter::AddGroupEdges() {
   }
 }
 
-void PluginSorter::AddOverlapEdges() {
+void PluginGraph::AddOverlapEdges() {
+  auto logger = getLogger();
   vertex_it vit, vitend;
   for (tie(vit, vitend) = boost::vertices(graph_); vit != vitend; ++vit) {
     vertex_t vertex = *vit;
 
     if (graph_[vertex].NumOverrideFormIDs() == 0) {
-      if (logger_) {
-        logger_->trace(
+      if (logger) {
+        logger->trace(
             "Skipping vertex for \"{}\": the plugin contains no override "
             "records.",
             graph_[vertex].GetName());
@@ -771,7 +756,7 @@ int ComparePlugins(const PluginSortingData& plugin1,
   }
 }
 
-void PluginSorter::AddTieBreakEdges() {
+void PluginGraph::AddTieBreakEdges() {
   // In order for the sort to be performed stably, there must be only one
   // possible result. This can be enforced by adding edges between all vertices
   // that aren't already linked. Use existing load order to decide the direction

src/api/sorting/plugin_graph.h

@@ -45,8 +45,8 @@ typedef boost::adjacency_list<boost::listS,
                               boost::bidirectionalS,
                               PluginSortingData,
                               EdgeType>
-    PluginGraph;
-typedef boost::graph_traits<PluginGraph>::vertex_descriptor vertex_t;
+    RawPluginGraph;
+typedef boost::graph_traits<RawPluginGraph>::vertex_descriptor vertex_t;
 typedef boost::associative_property_map<std::map<vertex_t, size_t>>
     vertex_map_t;
 
@@ -78,32 +78,29 @@ struct hash<loot::GraphPath> {
 }
 
 namespace loot {
-class PluginSorter {
+class PluginGraph {
 public:
-  std::vector<std::string> Sort(Game& game);
-
-private:
-  std::optional<vertex_t> GetVertexByName(const std::string& name) const;
+  size_t CountVertices() const;
   void CheckForCycles() const;
-  bool EdgeCreatesCycle(const vertex_t& u, const vertex_t& v);
-
+  
   void AddPluginVertices(Game& game);
   void AddSpecificEdges();
   void AddHardcodedPluginEdges(Game& game);
-  void AddGroupEdges();
+  void AddGroupEdges(const std::unordered_set<Group>& groups);
   void AddOverlapEdges();
   void AddTieBreakEdges();
 
+  std::vector<std::string> TopologicalSort() const;
+
+private:
+  std::optional<vertex_t> GetVertexByName(const std::string& name) const;
+  bool EdgeCreatesCycle(const vertex_t& u, const vertex_t& v);
+
   void AddEdge(const vertex_t& fromVertex,
                const vertex_t& toVertex,
                EdgeType edgeType);
 
-  PluginGraph graph_;
-  std::map<vertex_t, size_t> indexMap_;
-  vertex_map_t vertexIndexMap_;
-  std::shared_ptr<spdlog::logger> logger_;
-  std::unordered_set<Group> groups_;
-
+  RawPluginGraph graph_;
   std::unordered_set<GraphPath> pathsCache_;
 };
 }

src/api/sorting/plugin_sort.cpp

@@ -0,0 +1,60 @@
+/*  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 "plugin_sort.h"
+
+#include "api/helpers/logging.h"
+#include "api/sorting/plugin_graph.h"
+
+namespace loot {
+std::vector<std::string> SortPlugins(Game& game) {
+  PluginGraph graph;
+
+  graph.AddPluginVertices(game);
+
+  // If there aren't any vertices, exit early, because sorting assumes
+  // there is at least one plugin.
+  if (graph.CountVertices() == 0)
+    return std::vector<std::string>();
+
+  auto logger = getLogger();
+  if (logger) {
+    logger->info("Current load order: ");
+    for (const auto& plugin : game.GetLoadOrder()) {
+      logger->info("\t\t{}", plugin);
+    }
+  }
+
+  // Now add the interactions between plugins to the graph as edges.
+  graph.AddSpecificEdges();
+  graph.AddHardcodedPluginEdges(game);
+  graph.AddGroupEdges(game.GetDatabase()->GetGroups());
+  graph.AddOverlapEdges();
+  graph.AddTieBreakEdges();
+
+  graph.CheckForCycles();
+
+  return graph.TopologicalSort();
+}
+}

src/api/sorting/plugin_sort.h

@@ -0,0 +1,37 @@
+/*  LOOT
+
+    A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and
+    Fallout: New Vegas.
+
+    Copyright (C) 2012-2016    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/>.
+    */
+
+#ifndef LOOT_API_SORTING_PLUGIN_SORT
+#define LOOT_API_SORTING_PLUGIN_SORT
+
+#include <string>
+#include <vector>
+
+#include "api/game/game.h"
+
+namespace loot {
+std::vector<std::string> SortPlugins(Game& game);
+}
+
+#endif

src/tests/api/internals/main.cpp

@@ -43,7 +43,8 @@
 #include "tests/api/internals/metadata_list_test.h"
 #include "tests/api/internals/plugin_test.h"
 #include "tests/api/internals/sorting/group_sort_test.h"
-#include "tests/api/internals/sorting/plugin_sorter_test.h"
+#include "tests/api/internals/sorting/plugin_graph_test.h"
+#include "tests/api/internals/sorting/plugin_sort_test.h"
 #include "tests/api/internals/sorting/plugin_sorting_data_test.h"
 
 TEST(ModuloOperator, shouldConformToTheCpp11Standard) {

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

@@ -0,0 +1,143 @@
+/*  LOOT
+
+A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and
+Fallout: New Vegas.
+
+Copyright (C) 2014-2016    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/>.
+*/
+
+#ifndef LOOT_TESTS_API_INTERNALS_SORTING_PLUGIN_SORTER_TEST
+#define LOOT_TESTS_API_INTERNALS_SORTING_PLUGIN_SORTER_TEST
+
+#include "api/sorting/plugin_graph.h"
+
+#include "loot/exception/cyclic_interaction_error.h"
+#include "loot/exception/undefined_group_error.h"
+#include "tests/common_game_test_fixture.h"
+
+namespace loot {
+namespace test {
+class PluginGraphTest : public CommonGameTestFixture {
+protected:
+  PluginGraphTest() :
+      game_(GetParam(), dataPath.parent_path(), localPath),
+      masterlistPath_(metadataFilesPath / "userlist.yaml"),
+      cccPath_(dataPath.parent_path() / getCCCFilename()),
+      blankEslEsp("Blank.esl.esp") {}
+
+  void loadInstalledPlugins(Game &game_, bool headersOnly) {
+    std::vector<std::string> plugins({
+        masterFile,
+        blankEsm,
+        blankDifferentEsm,
+        blankMasterDependentEsm,
+        blankDifferentMasterDependentEsm,
+        blankEsp,
+        blankDifferentEsp,
+        blankMasterDependentEsp,
+        blankDifferentMasterDependentEsp,
+        blankPluginDependentEsp,
+        blankDifferentPluginDependentEsp,
+    });
+
+    if (GetParam() == GameType::fo4 || GetParam() == GameType::tes5se) {
+      plugins.push_back(blankEsl);
+
+      if (std::filesystem::exists(dataPath / blankEslEsp)) {
+        plugins.push_back(blankEslEsp);
+      }
+    }
+
+    game_.IdentifyMainMasterFile(masterFile);
+    game_.LoadCurrentLoadOrderState();
+    game_.LoadPlugins(plugins, headersOnly);
+  }
+
+  void GenerateMasterlist() {
+    using std::endl;
+
+    std::ofstream masterlist(masterlistPath_);
+    masterlist << "groups:" << endl
+               << "  - name: earliest" << endl
+               << "  - name: earlier" << endl
+               << "    after:" << endl
+               << "      - earliest" << endl
+               << "  - name: default" << endl
+               << "    after:" << endl
+               << "      - earlier" << endl
+               << "  - name: group1" << endl
+               << "  - name: group2" << endl
+               << "    after:" << endl
+               << "      - group1" << endl
+               << "  - name: group3" << endl
+               << "    after:" << endl
+               << "      - group2" << endl
+               << "  - name: group4" << endl
+               << "    after:" << endl
+               << "      - default" << endl;
+
+    masterlist.close();
+  }
+
+  std::string getCCCFilename() {
+    if (GetParam() == GameType::fo4) {
+      return "Fallout4.ccc";
+    } else {
+      // Not every game has a .ccc file, but Skyrim SE does, so just assume
+      // that.
+      return "Skyrim.ccc";
+    }
+  }
+
+  void GenerateCCCFile() {
+    using std::endl;
+
+    if (GetParam() == GameType::fo4) {
+      std::ofstream ccc(cccPath_);
+      ccc << blankDifferentEsm << endl
+          << blankDifferentMasterDependentEsm << endl;
+
+      ccc.close();
+    }
+  }
+
+  Game game_;
+  const std::string blankEslEsp;
+  const std::filesystem::path masterlistPath_;
+  const std::filesystem::path cccPath_;
+};
+
+// Pass an empty first argument, as it's a prefix for the test instantation,
+// but we only have the one so no prefix is necessary.
+INSTANTIATE_TEST_CASE_P(,
+                        PluginGraphTest,
+                        ::testing::Values(GameType::tes4,
+                                          GameType::tes5,
+                                          GameType::tes5se));
+
+TEST_P(PluginGraphTest, topologicalSortWithNoLoadedPluginsShouldReturnAnEmptyList) {
+  PluginGraph graph;
+  std::vector<std::string> sorted = graph.TopologicalSort();
+
+  EXPECT_TRUE(sorted.empty());
+}
+}
+}
+
+#endif

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

@@ -22,10 +22,10 @@ along with LOOT.  If not, see
 <https://www.gnu.org/licenses/>.
 */
 
-#ifndef LOOT_TESTS_API_INTERNALS_SORTING_PLUGIN_SORTER_TEST
-#define LOOT_TESTS_API_INTERNALS_SORTING_PLUGIN_SORTER_TEST
+#ifndef LOOT_TESTS_API_INTERNALS_SORTING_PLUGIN_SORT_TEST
+#define LOOT_TESTS_API_INTERNALS_SORTING_PLUGIN_SORT_TEST
 
-#include "api/sorting/plugin_sorter.h"
+#include "api/sorting/plugin_sort.h"
 
 #include "loot/exception/cyclic_interaction_error.h"
 #include "loot/exception/undefined_group_error.h"
@@ -33,9 +33,9 @@ along with LOOT.  If not, see
 
 namespace loot {
 namespace test {
-class PluginSorterTest : public CommonGameTestFixture {
+class PluginSortTest : public CommonGameTestFixture {
 protected:
-  PluginSorterTest() :
+  PluginSortTest() :
       game_(GetParam(), dataPath.parent_path(), localPath),
       masterlistPath_(metadataFilesPath / "userlist.yaml"),
       cccPath_(dataPath.parent_path() / getCCCFilename()),
@@ -126,33 +126,31 @@ protected:
 // Pass an empty first argument, as it's a prefix for the test instantation,
 // but we only have the one so no prefix is necessary.
 INSTANTIATE_TEST_CASE_P(,
-                        PluginSorterTest,
+                        PluginSortTest,
                         ::testing::Values(GameType::tes3,
                                           GameType::tes4,
                                           GameType::fo4));
 
-TEST_P(PluginSorterTest, sortingWithNoLoadedPluginsShouldReturnAnEmptyList) {
-  PluginSorter sorter;
-  std::vector<std::string> sorted = sorter.Sort(game_);
+TEST_P(PluginSortTest, sortingWithNoLoadedPluginsShouldReturnAnEmptyList) {
+  std::vector<std::string> sorted = SortPlugins(game_);
 
   EXPECT_TRUE(sorted.empty());
 }
 
-TEST_P(PluginSorterTest,
+TEST_P(PluginSortTest,
        sortingShouldNotMakeUnnecessaryChangesToAnExistingLoadOrder) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder = getLoadOrder();
 
   // Check stability by running the sort 100 times.
   for (int i = 0; i < 100; i++) {
-    std::vector<std::string> sorted = ps.Sort(game_);
+    std::vector<std::string> sorted = SortPlugins(game_);
     ASSERT_EQ(expectedSortedOrder, sorted) << " for sort " << i;
   }
 }
 
-TEST_P(PluginSorterTest, sortingShouldResolveGroupsAsTransitiveLoadAfterSets) {
+TEST_P(PluginSortTest, sortingShouldResolveGroupsAsTransitiveLoadAfterSets) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
   GenerateMasterlist();
@@ -166,7 +164,6 @@ TEST_P(PluginSorterTest, sortingShouldResolveGroupsAsTransitiveLoadAfterSets) {
   plugin.SetGroup("group3");
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       masterFile,
       blankDifferentEsm,
@@ -185,22 +182,21 @@ TEST_P(PluginSorterTest, sortingShouldResolveGroupsAsTransitiveLoadAfterSets) {
     expectedSortedOrder.insert(expectedSortedOrder.begin() + 5, blankEsl);
   }
 
-  std::vector<std::string> sorted = ps.Sort(game_);
+  std::vector<std::string> sorted = SortPlugins(game_);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
-TEST_P(PluginSorterTest, sortingShouldThrowIfAPluginHasAGroupThatDoesNotExist) {
+TEST_P(PluginSortTest, sortingShouldThrowIfAPluginHasAGroupThatDoesNotExist) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
   PluginMetadata plugin(blankDifferentEsm);
   plugin.SetGroup("group1");
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
-  EXPECT_THROW(ps.Sort(game_), UndefinedGroupError);
+  EXPECT_THROW(SortPlugins(game_), UndefinedGroupError);
 }
 
-TEST_P(PluginSorterTest,
+TEST_P(PluginSortTest,
        sortingShouldIgnoreAGroupEdgeIfItWouldCauseACycleInIsolation) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
@@ -211,7 +207,6 @@ TEST_P(PluginSorterTest,
   plugin.SetGroup("group4");
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       masterFile,
       blankDifferentEsm,
@@ -230,12 +225,12 @@ TEST_P(PluginSorterTest,
     expectedSortedOrder.insert(expectedSortedOrder.begin() + 3, blankEsl);
   }
 
-  std::vector<std::string> sorted = ps.Sort(game_);
+  std::vector<std::string> sorted = SortPlugins(game_);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
 TEST_P(
-    PluginSorterTest,
+    PluginSortTest,
     sortingShouldIgnoreGroupEdgesInvolvedInABackCycleOfAGroupEdgeFromADefaultGroupPlugin) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
@@ -256,7 +251,6 @@ TEST_P(
   plugin.SetGroup("group2");
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       masterFile,
       blankEsm,
@@ -275,12 +269,12 @@ TEST_P(
     expectedSortedOrder.insert(expectedSortedOrder.begin() + 5, blankEsl);
   }
 
-  std::vector<std::string> sorted = ps.Sort(game_);
+  std::vector<std::string> sorted = SortPlugins(game_);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
 TEST_P(
-    PluginSorterTest,
+    PluginSortTest,
     sortingShouldIgnoreGroupEdgesInvolvedInABackCycleOfAGroupEdgeToADefaultGroupPlugin) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
@@ -295,7 +289,6 @@ TEST_P(
   plugin.SetGroup("earlier");
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       blankEsm,
       blankMasterDependentEsm,
@@ -316,12 +309,12 @@ TEST_P(
     expectedSortedOrder.insert(expectedSortedOrder.begin() + 3, masterFile);
   }
 
-  std::vector<std::string> sorted = ps.Sort(game_);
+  std::vector<std::string> sorted = SortPlugins(game_);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
 TEST_P(
-    PluginSorterTest,
+    PluginSortTest,
     sortingShouldThrowForAGroupEdgeThatCausesAMultiGroupCycleBetweenTwoNonDefaultGroups) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
@@ -341,8 +334,7 @@ TEST_P(
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
   try {
-    PluginSorter ps;
-    ps.Sort(game_);
+    SortPlugins(game_);
     FAIL();
   } catch (CyclicInteractionError &e) {
     ASSERT_EQ(3, e.GetCycle().size());
@@ -356,7 +348,7 @@ TEST_P(
   }
 }
 
-TEST_P(PluginSorterTest,
+TEST_P(PluginSortTest,
        sortingShouldNotIgnoreIntermediatePluginsInAMultiGroupCycleIfTheEarlierPluginIsNotAMasterAndTheLaterIs) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
@@ -371,7 +363,6 @@ TEST_P(PluginSorterTest,
   plugin.SetGroup("earlier");
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       blankDifferentEsm,
       blankEsm,
@@ -392,12 +383,12 @@ TEST_P(PluginSorterTest,
     expectedSortedOrder.insert(expectedSortedOrder.begin() + 1, masterFile);
   }
 
-  std::vector<std::string> sorted = ps.Sort(game_);
+  std::vector<std::string> sorted = SortPlugins(game_);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
 TEST_P(
-    PluginSorterTest,
+    PluginSortTest,
     sortingShouldNotIgnorePluginsInTheSameGroupAsTheTargetPluginOfAGroupEdgeThatCausesACycleInIsolation) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
 
@@ -412,7 +403,6 @@ TEST_P(
   plugin.SetGroup("group4");
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       masterFile,
       blankDifferentEsm,
@@ -431,11 +421,11 @@ TEST_P(
     expectedSortedOrder.insert(expectedSortedOrder.begin() + 2, blankEsl);
   }
 
-  std::vector<std::string> sorted = ps.Sort(game_);
+  std::vector<std::string> sorted = SortPlugins(game_);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
-TEST_P(PluginSorterTest,
+TEST_P(PluginSortTest,
        sortingShouldUseLoadAfterMetadataWhenDecidingRelativePluginPositions) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
   PluginMetadata plugin(blankEsp);
@@ -445,7 +435,6 @@ TEST_P(PluginSorterTest,
   });
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       masterFile,
       blankEsm,
@@ -464,11 +453,11 @@ TEST_P(PluginSorterTest,
     expectedSortedOrder.insert(expectedSortedOrder.begin() + 5, blankEsl);
   }
 
-  std::vector<std::string> sorted = ps.Sort(game_);
+  std::vector<std::string> sorted = SortPlugins(game_);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
-TEST_P(PluginSorterTest,
+TEST_P(PluginSortTest,
        sortingShouldUseRequirementMetadataWhenDecidingRelativePluginPositions) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
   PluginMetadata plugin(blankEsp);
@@ -478,7 +467,6 @@ TEST_P(PluginSorterTest,
   });
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       masterFile,
       blankEsm,
@@ -497,11 +485,11 @@ TEST_P(PluginSorterTest,
     expectedSortedOrder.insert(expectedSortedOrder.begin() + 5, blankEsl);
   }
 
-  std::vector<std::string> sorted = ps.Sort(game_);
+  std::vector<std::string> sorted = SortPlugins(game_);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
-TEST_P(PluginSorterTest,
+TEST_P(PluginSortTest,
        sortingShouldUseTheGameCCCFileToEnforceHardcodedLoadOrderPositions) {
   if (GetParam() != GameType::fo4) {
     return;
@@ -513,7 +501,6 @@ TEST_P(PluginSorterTest,
   Game newGame(GetParam(), dataPath.parent_path(), localPath);
   ASSERT_NO_THROW(loadInstalledPlugins(newGame, false));
 
-  PluginSorter ps;
   std::vector<std::string> expectedSortedOrder({
       masterFile,
       blankDifferentEsm,
@@ -529,18 +516,17 @@ TEST_P(PluginSorterTest,
       blankDifferentPluginDependentEsp,
   });
 
-  std::vector<std::string> sorted = ps.Sort(newGame);
+  std::vector<std::string> sorted = SortPlugins(newGame);
   EXPECT_EQ(expectedSortedOrder, sorted);
 }
 
-TEST_P(PluginSorterTest, sortingShouldThrowIfACyclicInteractionIsEncountered) {
+TEST_P(PluginSortTest, sortingShouldThrowIfACyclicInteractionIsEncountered) {
   ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
   PluginMetadata plugin(blankEsm);
   plugin.SetLoadAfterFiles({File(blankMasterDependentEsm)});
   game_.GetDatabase()->SetPluginUserMetadata(plugin);
 
-  PluginSorter ps;
-  EXPECT_THROW(ps.Sort(game_), CyclicInteractionError);
+  EXPECT_THROW(SortPlugins(game_), CyclicInteractionError);
 }
 }
 }