diff --git a/src/backend/game.cpp b/src/backend/game.cpp index f3db9b83..a71d75dd 100644 --- a/src/backend/game.cpp +++ b/src/backend/game.cpp @@ -817,8 +817,15 @@ namespace loot { graph[v].CheckInstallValidity(*this); } + // Get the existing load order. + list loadorder; + GetLoadOrder(loadorder); + BOOST_LOG_TRIVIAL(info) << "Fetched existing load order: "; + for (const auto &plugin : loadorder) + BOOST_LOG_TRIVIAL(info) << plugin; + // Now add edges and sort. progressCallback(lc::translate("Adding edges to plugin graph and performing topological sort...")); - return loot::Sort(graph); + return loot::Sort(graph, loadorder); } } diff --git a/src/backend/graph.cpp b/src/backend/graph.cpp index c13b89e6..ad33e44b 100644 --- a/src/backend/graph.cpp +++ b/src/backend/graph.cpp @@ -290,13 +290,10 @@ namespace loot { parentVertex = *vit2; vertex = *vit; } - else if (graph[*vit].Name() < graph[*vit2].Name()) { //There needs to be an edge between the two, but direction cannot be decided using overlap size. Just use names. - parentVertex = *vit; - vertex = *vit2; - } else { - parentVertex = *vit2; - vertex = *vit; + // There's no way to determine the order between the two, so just leave them to be treated like + // any two unlinked, non-conflicting plugins in the next pass. + continue; } //BOOST_LOG_TRIVIAL(trace) << "Checking edge validity between \"" << graph[*vit].Name() << "\" and \"" << graph[*vit2].Name() << "\"."; @@ -310,7 +307,50 @@ namespace loot { } } - std::list Sort(PluginGraph& graph) { + size_t LoadOrderPos(const list& loadorder, const std::string& plugin) { + auto it = find(loadorder.begin(), loadorder.end(), plugin); + + if (it != loadorder.end()) + return distance(loadorder.begin(), it); + else + throw error(error::sorting_error, "No existing load order position for " + plugin); + } + + void AddTieBreakEdges(PluginGraph& graph, const list& loadorder, const vertex_map_t& v_index_map) { + // 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 of these edges. + loot::vertex_it vit, vitend; + for (boost::tie(vit, vitend) = boost::vertices(graph); vit != vitend; ++vit) { + BOOST_LOG_TRIVIAL(trace) << "Adding tie-break edges to vertex for \"" << graph[*vit].Name() << "\"."; + + loot::vertex_it vit2, vitend2; + for (boost::tie(vit2, vitend2) = boost::vertices(graph); vit2 != vitend2; ++vit2) { + if (vit == vit2 || boost::edge(*vit, *vit2, graph).second || boost::edge(*vit2, *vit, graph).second) + //Vertices are the same or are already linked. + continue; + + vertex_t vertex, parentVertex; + if (LoadOrderPos(loadorder, graph[*vit].Name()) < LoadOrderPos(loadorder, graph[*vit2].Name())) { + parentVertex = *vit; + vertex = *vit2; + } + else { + parentVertex = *vit2; + vertex = *vit; + } + + //BOOST_LOG_TRIVIAL(trace) << "Checking edge validity between \"" << graph[*vit].Name() << "\" and \"" << graph[*vit2].Name() << "\"."; + if (!EdgeCreatesCycle(parentVertex, vertex, graph, v_index_map)) { //No edge going the other way, OK to add this edge. + BOOST_LOG_TRIVIAL(trace) << "Adding edge from \"" << graph[parentVertex].Name() << "\" to \"" << graph[vertex].Name() << "\"."; + + boost::add_edge(parentVertex, vertex, graph); + } + } + } + } + + std::list Sort(PluginGraph& graph, const list& loadorder) { // Prebuild an index map, which std::list-based VertexList graphs don't have. map index_map; vertex_map_t v_index_map(index_map); @@ -329,6 +369,9 @@ namespace loot { BOOST_LOG_TRIVIAL(debug) << "Adding overlap edges."; AddOverlapEdges(graph, v_index_map); + BOOST_LOG_TRIVIAL(debug) << "Adding tie-break edges."; + AddTieBreakEdges(graph, loadorder, v_index_map); + BOOST_LOG_TRIVIAL(info) << "Checking to see if the graph is cyclic."; CheckForCycles(graph, v_index_map); diff --git a/src/backend/graph.h b/src/backend/graph.h index 4d7f97da..5f85b848 100644 --- a/src/backend/graph.h +++ b/src/backend/graph.h @@ -20,7 +20,7 @@ You should have received a copy of the GNU General Public License along with LOOT. If not, see . -*/ + */ #ifndef __LOOT_GRAPH__ #define __LOOT_GRAPH__ @@ -33,11 +33,10 @@ #include namespace loot { - typedef boost::adjacency_list PluginGraph; typedef boost::graph_traits::vertex_descriptor vertex_t; - std::list Sort(PluginGraph& graph); + std::list Sort(PluginGraph& graph, const std::list& loadorder); } #endif diff --git a/src/tests/api/api.h b/src/tests/api/api.h index e77b07f9..47342ff7 100644 --- a/src/tests/api/api.h +++ b/src/tests/api/api.h @@ -360,17 +360,17 @@ TEST_F(OblivionAPIOperationsTest, SortPlugins) { // Expected order was obtained from running the API function once. std::list expectedOrder = { + "Oblivion.esm", "Blank.esm", "Blank - Different.esm", - "Blank - Different Master Dependent.esm", "Blank - Master Dependent.esm", - "Oblivion.esm", + "Blank - Different Master Dependent.esm", "Blank.esp", "Blank - Different.esp", - "Blank - Different Master Dependent.esp", - "Blank - Different Plugin Dependent.esp", "Blank - Master Dependent.esp", + "Blank - Different Master Dependent.esp", "Blank - Plugin Dependent.esp", + "Blank - Different Plugin Dependent.esp", }; std::list actualOrder; for (size_t i = 0; i < numPlugins; ++i) { @@ -391,20 +391,17 @@ TEST_F(SkyrimAPIOperationsTest, SortPlugins) { // Expected order was obtained from running the API function once. std::list expectedOrder = { + "Skyrim.esm", "Blank.esm", "Blank - Different.esm", - "Blank - Different Master Dependent.esm", "Blank - Master Dependent.esm", - "Skyrim.esm", + "Blank - Different Master Dependent.esm", "Blank.esp", "Blank - Different.esp", - "Blank - Different Master Dependent.esp", - "Blank - Different Plugin Dependent.esp", "Blank - Master Dependent.esp", - "Blank - Plugin Dependent.esp", "Blank - Different Master Dependent.esp", - "Blank - Different.esp", - "Blank - Different Plugin Dependent.esp" + "Blank - Plugin Dependent.esp", + "Blank - Different Plugin Dependent.esp", }; std::list actualOrder; for (size_t i = 0; i < numPlugins; ++i) {