Fix priority inheritance during sorting

During sorting, plugins inherit the largest priority value of
the plugins it loads after, if that value is larger than the
plugin's own priority value. However, if a plugin loads after a
plugin that itself inherits a priority, and the former is evaluated
first, it would inherit the wrong priority value, leading to
an incorrect sorting order.

This commit fixes the issue by propagating inherited priority
values after the initial evaluation loop, using a depth-first
search starting from each plugin with a positive priority.
Fixes #486.

src/backend/plugin_sorter.cpp

@@ -118,6 +118,8 @@ namespace loot {
         BOOST_LOG_TRIVIAL(debug) << "Adding non-overlap edges.";
         AddSpecificEdges();
 
+        PropagatePriorities();
+
         BOOST_LOG_TRIVIAL(debug) << "Adding priority edges.";
         AddPriorityEdges();
 
@@ -249,12 +251,66 @@ namespace loot {
         return false;
     }
 
+    void PluginSorter::PropagatePriorities() {
+        /* If a plugin has a priority value > 0, that value should be
+           inherited by all plugins that have edges coming from that
+           plugin, ie. those that load after it, unless the plugin being
+           compared itself has a larger value. */
+
+        // Find all vertices with priorities > 0.
+        std::vector<vertex_t> positivePriorityVertices;
+        vertex_it vit, vitend;
+        boost::tie(vit, vitend) = boost::vertices(graph);
+        std::copy_if(vit,
+                     vitend,
+                     std::back_inserter(positivePriorityVertices),
+                     [&](const vertex_t& vertex) {
+            return graph[vertex].Priority() > 0;
+        });
+
+        // To reduce the number of priorities that will need setting,
+        // sort the vertices in order of decreasing priority.
+        std::sort(begin(positivePriorityVertices),
+                  end(positivePriorityVertices),
+                  [&](const vertex_t& lhs, const vertex_t& rhs) {
+            return graph[lhs].Priority() > graph[rhs].Priority();
+        });
+
+        // Create a color map.
+        std::vector<boost::default_color_type> colorVec(num_vertices(graph));
+        boost::iterator_property_map<boost::default_color_type*, vertex_map_t> colorMap(&colorVec.front(), vertexIndexMap);
+
+        // Now loop over the vertices. For each one, do a depth-first
+        // search, setting priorities until an equal or larger value is
+        // encountered.
+        for (const vertex_t& vertex : positivePriorityVertices) {
+            BOOST_LOG_TRIVIAL(trace) << "Doing DFS for " << graph[vertex].Name() << " which has priority " << graph[vertex].Priority();
+            boost::dfs_visitor<> visitor;
+            boost::depth_first_visit(graph,
+                                     vertex,
+                                     visitor,
+                                     colorMap,
+                                     [&vertex](const vertex_t& currentVertex, const PluginGraph& graph) {
+                if (graph[currentVertex].Priority() < graph[vertex].Priority()) {
+                    BOOST_LOG_TRIVIAL(trace) << "Overriding priority for " << graph[currentVertex].Name() << " from " << graph[currentVertex].Priority() << " to " << graph[vertex].Priority();
+                    // const_cast is necessary because depth_first_search
+                    // takes a const graph.
+                    const_cast<PluginGraph&>(graph)[currentVertex].Priority(graph[vertex].Priority());
+
+                    return false;
+                }
+
+                return currentVertex != vertex
+                    && graph[currentVertex].Priority() >= graph[vertex].Priority();
+            });
+        }
+    }
+
     void PluginSorter::AddSpecificEdges() {
         //Add edges for all relationships that aren't overlaps or priority differences.
         loot::vertex_it vit, vitend;
         for (boost::tie(vit, vitend) = boost::vertices(graph); vit != vitend; ++vit) {
             vertex_t parentVertex;
-            int parentPriority = graph[*vit].Priority();
 
             BOOST_LOG_TRIVIAL(trace) << "Adding specific edges to vertex for \"" << graph[*vit].Name() << "\".";
 
@@ -294,11 +350,6 @@ namespace loot {
                     BOOST_LOG_TRIVIAL(trace) << "Adding edge from \"" << graph[parentVertex].Name() << "\" to \"" << graph[*vit].Name() << "\".";
 
                     boost::add_edge(parentVertex, *vit, graph);
-
-                    int priority = graph[parentVertex].Priority();
-                    if (priority > parentPriority) {
-                        parentPriority = priority;
-                    }
                 }
             }
             BOOST_LOG_TRIVIAL(trace) << "Adding in-edges for requirements.";
@@ -309,11 +360,6 @@ namespace loot {
                     BOOST_LOG_TRIVIAL(trace) << "Adding edge from \"" << graph[parentVertex].Name() << "\" to \"" << graph[*vit].Name() << "\".";
 
                     boost::add_edge(parentVertex, *vit, graph);
-
-                    int priority = graph[parentVertex].Priority();
-                    if (priority > parentPriority) {
-                        parentPriority = priority;
-                    }
                 }
             }
 
@@ -325,26 +371,13 @@ namespace loot {
                     BOOST_LOG_TRIVIAL(trace) << "Adding edge from \"" << graph[parentVertex].Name() << "\" to \"" << graph[*vit].Name() << "\".";
 
                     boost::add_edge(parentVertex, *vit, graph);
-
-                    int priority = graph[parentVertex].Priority();
-                    if (priority > parentPriority) {
-                        parentPriority = priority;
-                    }
                 }
             }
-
-            //parentPriority is now the highest priority value of any plugin that the current plugin needs to load after.
-            //Set the current plugin's priority to parentPlugin.
-            if (parentPriority > 0 && graph[*vit].Priority() < parentPriority) {
-                BOOST_LOG_TRIVIAL(trace) << "Overriding priority for " << graph[*vit].Name() << " from " << graph[*vit].Priority() << " to " << parentPriority;
-                graph[*vit].Priority(parentPriority);
-            }
         }
     }
 
     void PluginSorter::AddPriorityEdges() {
         loot::vertex_it vit, vitend;
-
         for (boost::tie(vit, vitend) = boost::vertices(graph); vit != vitend; ++vit) {
             BOOST_LOG_TRIVIAL(trace) << "Adding priority difference edges to vertex for \"" << graph[*vit].Name() << "\".";
             //Priority differences should only be taken account between plugins that conflict.

src/backend/plugin_sorter.h

@@ -56,6 +56,8 @@ namespace loot {
 
         int plugincmp(const std::string& plugin1, const std::string& plugin2) const;
 
+        void PropagatePriorities();
+
         void BuildPluginGraph(Game& game, const unsigned int language);
         void AddSpecificEdges();
         void AddPriorityEdges();

src/tests/backend/test_plugin_sorter.h

@@ -56,6 +56,17 @@ protected:
         });
     }
 
+    inline static std::list<std::string> GetActualSortedOrder(const std::list<loot::Plugin>& sortedPlugins) {
+        std::list<std::string> output;
+        std::transform(begin(sortedPlugins),
+                       end(sortedPlugins),
+                       std::back_inserter(output),
+                       [](const loot::Plugin& plugin) {
+            return plugin.Name();
+        });
+        return output;
+    }
+
     loot::Game game;
     std::function<void(const std::string&)> callback;
 };
@@ -116,6 +127,57 @@ TEST_F(PluginSorter, Sort_WithPriority) {
     EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder)));
 }
 
+TEST_F(PluginSorter, sortingWithPrioritiesShouldInheritRecursivelyRegardlessOfEvaluationOrder) {
+    ASSERT_NO_THROW(game.LoadPlugins(false));
+
+    // Set Blank.esp's priority.
+    loot::PluginMetadata plugin("Blank.esp");
+    plugin.Priority(2);
+    game.GetUserlist().AddPlugin(plugin);
+
+    // Load Blank - Master Dependent.esp after Blank.esp so that it
+    // inherits Blank.esp's priority.
+    plugin = loot::PluginMetadata("Blank - Master Dependent.esp");
+    plugin.LoadAfter({
+        loot::File("Blank.esp"),
+    });
+    game.GetUserlist().AddPlugin(plugin);
+
+    // Load Blank - Different.esp after Blank - Master Dependent.esp, so
+    // that it inherits its inherited priority.
+    plugin = loot::PluginMetadata("Blank - Different.esp");
+    plugin.LoadAfter({
+        loot::File("Blank - Master Dependent.esp"),
+    });
+    game.GetUserlist().AddPlugin(plugin);
+
+    // Set Blank - Different Master Dependent.esp to have a higher priority
+    // than 0 but lower than Blank.esp. Need to also make it a global priority
+    // because it doesn't otherwise conflict with the other plugins.
+    plugin = loot::PluginMetadata("Blank - Different Master Dependent.esp");
+    plugin.Priority(1);
+    plugin.SetPriorityGlobal(true);
+    game.GetUserlist().AddPlugin(plugin);
+
+    loot::PluginSorter ps;
+    std::list<std::string> expectedSortedOrder({
+        "Skyrim.esm",
+        "Blank.esm",
+        "Blank - Different.esm",
+        "Blank - Master Dependent.esm",
+        "Blank - Different Master Dependent.esm",
+        "Blank - Different Master Dependent.esp",
+        "Blank.esp",
+        "Blank - Master Dependent.esp",
+        "Blank - Different.esp",
+        "Blank - Plugin Dependent.esp",
+        "Blank - Different Plugin Dependent.esp",
+    });
+
+    std::list<std::string> actualSortedOrder = GetActualSortedOrder(ps.Sort(game, loot::Language::english, callback));
+    EXPECT_EQ(expectedSortedOrder, actualSortedOrder);
+}
+
 TEST_F(PluginSorter, Sort_WithLoadAfter) {
     ASSERT_NO_THROW(game.LoadPlugins(false));
     loot::PluginMetadata plugin("Blank.esp");