Fix plugin hoisting for Starfield

This also changes the behaviour for the other asterisk-based games, as implicitly-active-plugins may now also be hoisted for them, but that might be correct (I've never tested that) and is unlikely to occur, as there's a limited set of implicitly active plugins.

src/load_order/asterisk_based.rs

@@ -22,7 +22,9 @@ use std::io::{BufWriter, Write};
 
 use unicase::{eq, UniCase};
 
-use super::mutable::{generic_insert_position, hoist_masters, read_plugin_names, MutableLoadOrder};
+use super::mutable::{
+    generic_insert_position, hoist_masters, read_plugin_names, HoistBehaviour, MutableLoadOrder,
+};
 use super::readable::{ReadableLoadOrder, ReadableLoadOrderBase};
 use super::strict_encode;
 use super::timestamp_based::save_load_order_using_timestamps;
@@ -115,10 +117,16 @@ impl WritableLoadOrder for AsteriskBasedLoadOrder {
         let filenames = self.find_plugins();
 
         self.load_unique_plugins(plugin_tuples, filenames);
-        hoist_masters(&mut self.plugins)?;
 
         self.add_implicitly_active_plugins()?;
 
+        let hoist_behaviour = match self.game_settings.id() {
+            GameId::Starfield => HoistBehaviour::All,
+            _ => HoistBehaviour::OnlyNonMasters,
+        };
+
+        hoist_masters(&mut self.plugins, hoist_behaviour)?;
+
         Ok(())
     }
 
@@ -640,23 +648,25 @@ mod tests {
         // has the ESM flag set that has another (normal) .esp as a master.
 
         let plugins_dir = &load_order.game_settings().plugins_directory();
-        copy_to_test_dir(
-            "Blank - Plugin Dependent.esp",
-            "Blank - Plugin Dependent.esp",
-            load_order.game_settings(),
-        );
-        set_master_flag(&plugins_dir.join("Blank - Plugin Dependent.esp"), true).unwrap();
-        std::fs::copy(&plugins_dir.join("Blank - Plugin Dependent.esp"), &plugins_dir.join("Blank - Plugin Dependent 2.esp")).unwrap();
+        let plugin_name_1 = "Blank - Plugin Dependent.esp";
+        let plugin_name_2 = "Blank - Plugin Dependent 2.esp";
+        copy_to_test_dir(plugin_name_1, plugin_name_1, load_order.game_settings());
+        set_master_flag(&plugins_dir.join(plugin_name_1), true).unwrap();
+        std::fs::copy(
+            &plugins_dir.join(plugin_name_1),
+            &plugins_dir.join(plugin_name_2),
+        )
+        .unwrap();
 
         let expected_filenames = vec![
             "Blank - Master Dependent.esp",
             "Blank - Different.esp",
             "Blàñk.esp",
             "Blank.esp",
             "Skyrim.esm",
-            "Blank - Plugin Dependent.esp",
+            plugin_name_1,
             "Blank.esm",
-            "Blank - Plugin Dependent 2.esp",
+            plugin_name_2,
         ];
         write_active_plugins_file(load_order.game_settings(), &expected_filenames);
 
@@ -665,12 +675,90 @@ mod tests {
         let expected_filenames = vec![
             "Skyrim.esm",
             "Blank.esp",
+            plugin_name_1,
+            "Blank.esm",
+            plugin_name_2,
+            "Blank - Master Dependent.esp",
+            "Blank - Different.esp",
+            "Blàñk.esp",
+        ];
+
+        assert_eq!(expected_filenames, load_order.plugin_names());
+    }
+
+    #[test]
+    fn load_should_not_hoist_masters_for_games_other_than_starfield() {
+        let tmp_dir = tempdir().unwrap();
+        let mut load_order = prepare(GameId::SkyrimSE, &tmp_dir.path());
+
+        // .esm plugins are loaded as ESMs, .esl plugins are loaded as ESMs and
+        // ESLs, ignoring their actual flags, so only worth testing a .esp that
+        // has the ESM flag set that has another (normal) .esp as a master.
+
+        let plugin_name = "Blank - Master Dependent.esm";
+        copy_to_test_dir(plugin_name, plugin_name, load_order.game_settings());
+
+        let expected_filenames = vec![
+            plugin_name,
+            "Blank - Master Dependent.esp",
+            "Blank - Different.esp",
+            "Blàñk.esp",
+            "Blank.esp",
+            "Skyrim.esm",
             "Blank - Plugin Dependent.esp",
             "Blank.esm",
             "Blank - Plugin Dependent 2.esp",
+        ];
+        write_active_plugins_file(load_order.game_settings(), &expected_filenames);
+
+        load_order.load().unwrap();
+
+        let expected_filenames = vec![
+            "Skyrim.esm",
+            plugin_name,
+            "Blank.esm",
             "Blank - Master Dependent.esp",
             "Blank - Different.esp",
             "Blàñk.esp",
+            "Blank.esp",
+        ];
+
+        assert_eq!(expected_filenames, load_order.plugin_names());
+    }
+
+    #[test]
+    fn load_should_hoist_masters_before_masters_for_starfield() {
+        let tmp_dir = tempdir().unwrap();
+        let mut load_order = prepare(GameId::Starfield, &tmp_dir.path());
+
+        // .esm plugins are loaded as ESMs, .esl plugins are loaded as ESMs and
+        // ESLs, ignoring their actual flags, so only worth testing a .esp that
+        // has the ESM flag set that has another (normal) .esp as a master.
+
+        let plugin_name = "Blank - Override.full.esm";
+        copy_to_test_dir(plugin_name, plugin_name, load_order.game_settings());
+
+        let expected_filenames = vec![
+            plugin_name,
+            "Starfield.esm",
+            "Blank.full.esm",
+            "Blank.medium.esm",
+            "Blank.small.esm",
+            "Blank.esp",
+            "Blank - Override.esp",
+        ];
+        write_active_plugins_file(load_order.game_settings(), &expected_filenames);
+
+        load_order.load().unwrap();
+
+        let expected_filenames = vec![
+            "Blank.full.esm",
+            plugin_name,
+            "Starfield.esm",
+            "Blank.medium.esm",
+            "Blank.small.esm",
+            "Blank.esp",
+            "Blank - Override.esp",
         ];
 
         assert_eq!(expected_filenames, load_order.plugin_names());

src/load_order/mutable.rs

@@ -201,10 +201,27 @@ pub fn plugin_line_mapper(line: &str) -> Option<String> {
     }
 }
 
+pub enum HoistBehaviour {
+    /// Hoisting only happens to non-master files that are masters of master files.
+    OnlyNonMasters,
+    /// Hoisting happens to master and non-master files that are masters of master files.
+    All,
+}
+
+impl HoistBehaviour {
+    fn only_non_masters(&self) -> bool {
+        match self {
+            HoistBehaviour::OnlyNonMasters => true,
+            _ => false,
+        }
+    }
+}
+
 /// If an ESM has an ESP as a master, the ESP will be loaded directly before the
 /// ESM instead of in its usual position. This function "hoists" such ESPs
 /// further up the load order.
-pub fn hoist_masters(plugins: &mut Vec<Plugin>) -> Result<(), Error> {
+/// Starfield extends this to also hoist ESMs before other ESMs that depend on them.
+pub fn hoist_masters(plugins: &mut Vec<Plugin>, behaviour: HoistBehaviour) -> Result<(), Error> {
     // Store plugins' current positions and where they need to move to.
     // Use a BTreeMap so that if a plugin needs to move for more than one ESM,
     // it will move for the earlier one and so also satisfy the later one, and
@@ -221,7 +238,7 @@ pub fn hoist_masters(plugins: &mut Vec<Plugin>) -> Result<(), Error> {
                 .iter()
                 .position(|p| p.name_matches(&master))
                 .unwrap_or(0);
-            if pos > index && !plugins[pos].is_master_file() {
+            if pos > index && !(behaviour.only_non_masters() && plugins[pos].is_master_file()) {
                 // Need to move the plugin to index, but can't do that while
                 // iterating, so store it for later.
                 from_to_map.entry(pos).or_insert(index);

src/load_order/textfile_based.rs

@@ -25,7 +25,7 @@ use unicase::{eq, UniCase};
 
 use super::mutable::{
     generic_insert_position, hoist_masters, load_active_plugins, plugin_line_mapper,
-    read_plugin_names, MutableLoadOrder,
+    read_plugin_names, HoistBehaviour, MutableLoadOrder,
 };
 use super::readable::{ReadableLoadOrder, ReadableLoadOrderBase};
 use super::strict_encode;
@@ -153,7 +153,7 @@ impl WritableLoadOrder for TextfileBasedLoadOrder {
             load_active_plugins(self, plugin_line_mapper)?;
         }
 
-        hoist_masters(&mut self.plugins)?;
+        hoist_masters(&mut self.plugins, HoistBehaviour::OnlyNonMasters)?;
 
         self.add_implicitly_active_plugins()?;
 

src/load_order/timestamp_based.rs

@@ -25,7 +25,7 @@ use rayon::prelude::*;
 use regex::Regex;
 
 use super::mutable::{
-    generic_insert_position, hoist_masters, load_active_plugins, MutableLoadOrder,
+    generic_insert_position, hoist_masters, load_active_plugins, HoistBehaviour, MutableLoadOrder,
 };
 use super::readable::{ReadableLoadOrder, ReadableLoadOrderBase};
 use super::strict_encode;
@@ -118,7 +118,7 @@ impl WritableLoadOrder for TimestampBasedLoadOrder {
 
         self.plugins = self.load_plugins_from_dir();
         self.plugins.par_sort_by(plugin_sorter);
-        hoist_masters(&mut self.plugins)?;
+        hoist_masters(&mut self.plugins, HoistBehaviour::OnlyNonMasters)?;
 
         let regex = Regex::new(r"(?i)GameFile[0-9]{1,3}=(.+\.es(?:m|p))")
             .expect("Hardcoded GameFile ini entry regex should be valid");