From 5546f5ec937887990dbe68e991ea7fde2368f88b Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 24 Jun 2024 18:14:18 +0100 Subject: [PATCH] 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 | 110 +++++++++++++++++++++++++++--- src/load_order/mutable.rs | 21 +++++- src/load_order/textfile_based.rs | 4 +- src/load_order/timestamp_based.rs | 4 +- 4 files changed, 122 insertions(+), 17 deletions(-) diff --git a/src/load_order/asterisk_based.rs b/src/load_order/asterisk_based.rs index 6d26606..23197ea 100644 --- a/src/load_order/asterisk_based.rs +++ b/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,13 +648,15 @@ 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", @@ -654,9 +664,9 @@ mod tests { "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()); diff --git a/src/load_order/mutable.rs b/src/load_order/mutable.rs index f7e355d..0d06978 100644 --- a/src/load_order/mutable.rs +++ b/src/load_order/mutable.rs @@ -201,10 +201,27 @@ pub fn plugin_line_mapper(line: &str) -> Option { } } +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) -> Result<(), Error> { +/// Starfield extends this to also hoist ESMs before other ESMs that depend on them. +pub fn hoist_masters(plugins: &mut Vec, 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) -> 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); diff --git a/src/load_order/textfile_based.rs b/src/load_order/textfile_based.rs index 22983e5..86033b0 100644 --- a/src/load_order/textfile_based.rs +++ b/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()?; diff --git a/src/load_order/timestamp_based.rs b/src/load_order/timestamp_based.rs index 44c7b2a..6c523c3 100644 --- a/src/load_order/timestamp_based.rs +++ b/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");