From fb7ac05e1e582526978c26fc4e1306f2342727a6 Mon Sep 17 00:00:00 2001
From: Oliver Hamlet <oliver.hamlet@gmail.com>
Date: Tue, 16 Apr 2019 21:16:13 +0100
Subject: [PATCH] Don't ignore intermediate plugins in a plugin-> master group
 cycle

Intermediate masters may be able to respect the later plugin's group
without causing a cycle, and intermediate non-masters would have their
own cycles detected independently.
---
 src/api/sorting/plugin_sorter.cpp             | 10 ++++
 .../internals/sorting/plugin_sorter_test.h    | 49 +++++++++++++++++--
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/src/api/sorting/plugin_sorter.cpp b/src/api/sorting/plugin_sorter.cpp
index d3b92901..2ad6908c 100644
--- a/src/api/sorting/plugin_sorter.cpp
+++ b/src/api/sorting/plugin_sorter.cpp
@@ -551,6 +551,16 @@ void PluginSorter::AddGroupEdges() {
                 toPlugin.GetName());
           }
 
+          // If the earlier plugin is not a master and the later plugin is,
+          // don't ignore the plugin with the default group for all
+          // intermediate plugins, as some of those plugins may be masters
+          // that wouldn't be involved in the cycle, and any of those
+          // plugins that are not masters would have their own cycles
+          // detected anyway.
+          if (!fromPlugin.IsMaster() && toPlugin.IsMaster()) {
+            continue;
+          }
+
           // The default group is a special case, as it's given to plugins
           // with no metadata. If a plugin in the default group causes
           // a cycle due to its group, ignore that plugin's group for all
diff --git a/src/tests/api/internals/sorting/plugin_sorter_test.h b/src/tests/api/internals/sorting/plugin_sorter_test.h
index 56c0a639..b1002ad3 100644
--- a/src/tests/api/internals/sorting/plugin_sorter_test.h
+++ b/src/tests/api/internals/sorting/plugin_sorter_test.h
@@ -164,11 +164,10 @@ TEST_P(PluginSorterTest,
         PluginMetadata());
     EXPECT_TRUE(lightMaster.IsMaster());
 
-    auto lightMasterEsp =
-        PluginSortingData(*dynamic_cast<const Plugin *>(
-                              game_.GetPlugin(blankEslEsp).get()),
-                          PluginMetadata(),
-                          PluginMetadata());
+    auto lightMasterEsp = PluginSortingData(
+        *dynamic_cast<const Plugin *>(game_.GetPlugin(blankEslEsp).get()),
+        PluginMetadata(),
+        PluginMetadata());
     EXPECT_FALSE(lightMasterEsp.IsMaster());
   }
 }
@@ -394,6 +393,46 @@ TEST_P(
   }
 }
 
+TEST_P(PluginSorterTest,
+       sortingShouldNotIgnoreIntermediatePluginsInAMultiGroupCycleIfTheEarlierPluginIsNotAMasterAndTheLaterIs) {
+  ASSERT_NO_THROW(loadInstalledPlugins(game_, false));
+
+  GenerateMasterlist();
+  game_.GetDatabase()->LoadLists(masterlistPath_);
+
+  PluginMetadata plugin(blankMasterDependentEsp);
+  plugin.SetGroup("earliest");
+  game_.GetDatabase()->SetPluginUserMetadata(plugin);
+
+  plugin = PluginMetadata(blankDifferentEsm);
+  plugin.SetGroup("earlier");
+  game_.GetDatabase()->SetPluginUserMetadata(plugin);
+
+  PluginSorter ps;
+  std::vector<std::string> expectedSortedOrder({
+      blankDifferentEsm,
+      blankEsm,
+      blankMasterDependentEsm,
+      blankDifferentMasterDependentEsm,
+      blankMasterDependentEsp,
+      blankEsp,
+      blankDifferentEsp,
+      blankDifferentMasterDependentEsp,
+      blankPluginDependentEsp,
+      blankDifferentPluginDependentEsp,
+  });
+
+  if (GetParam() == GameType::fo4 || GetParam() == GameType::tes5se) {
+    expectedSortedOrder.insert(expectedSortedOrder.begin(), masterFile);
+    expectedSortedOrder.insert(expectedSortedOrder.begin() + 5, blankEsl);
+  } else {
+    expectedSortedOrder.insert(expectedSortedOrder.begin() + 1, masterFile);
+  }
+
+  std::vector<std::string> sorted = ps.Sort(game_);
+  EXPECT_EQ(expectedSortedOrder, sorted);
+}
+
 TEST_P(
     PluginSorterTest,
     sortingShouldNotIgnorePluginsInTheSameGroupAsTheTargetPluginOfAGroupEdgeThatCausesACycleInIsolation) {