diff --git a/Cargo.lock b/Cargo.lock index c8a380cc..130c8515 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1320,6 +1320,7 @@ name = "eidos-plugins" version = "1.18.2" dependencies = [ "eidos-gamedef", + "eidos-ini", "encoding_rs", "esplugin", "libloadorder", diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index c406b291..094485b4 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -110,6 +110,8 @@ pub struct RealHooks<'a> { pub owner: String, /// Members installed under a name that was free, and what it was. pub renamed: Vec<(String, String)>, + /// Members whose previous folder was kept before it was replaced, and where. + pub kept: Vec<(String, PathBuf)>, pub payload_root: PathBuf, pub allow_runtime_mismatch: bool, } @@ -126,6 +128,26 @@ impl RealHooks<'_> { fn owns(&self, folder: &Path, m: &Mod) -> bool { owns_folder(folder, &self.owner_marker(m)) } + /// Move what the hooks noted during [`install::run`] into its report. + pub fn drain_notes(&mut self, report: &mut Report) { + for (member, folder) in std::mem::take(&mut self.renamed) { + report.renamed.push(Note { + subject: member, + detail: format!( + "installed as \"{folder}\", because a mod of yours already had that name" + ), + }); + } + for (member, backup) in std::mem::take(&mut self.kept) { + report.kept.push(Note { + subject: member, + detail: format!( + "its previous folder differed from what Eidos installed (a file changed, added or hidden, or it could not be checked), so it was kept as \"{}\". A backup is never loaded into the game: copy back what you still need", + backup.file_name().unwrap_or_default().to_string_lossy() + ), + }); + } + } /// Record the successful install without changing existing profile decisions. fn register( &mut self, @@ -242,6 +264,320 @@ fn reserve_folder( } } +/// Hand the folders another revision of this collection owns to the revision +/// `state` installs, before its first member. +/// +/// Called by [`crate::install::run`] once the recipe and the game runtime have +/// passed, never before: a revision the user cannot or will not install must +/// not take anything over, or the one they stay on can no longer verify it. +/// +/// The owner names the revision, so two revisions never mistake each other's +/// output - but each one then started from nothing. Updating to the author's +/// next revision collided every member with its own previous copy, installed +/// it again as "Name (2)", and left the old copies enabled, members the update +/// dropped included. +/// +/// A member keeps a folder when its key is unchanged or, for a Nexus member +/// whose file the author updated, when it is the only member from that mod page +/// on both sides; anything ambiguous installs fresh. The folder must still +/// carry the other revision's exact marker - a folder the user reinstalled by +/// hand has lost it - and must hold no saved installer answers, which are tied +/// to the old archive and owner, and no other profile may enable it. The +/// receipt moves with the folder, and an unchanged member's status with it, so +/// an unchanged member verifies in place and a changed one is replaced inside +/// the same folder, keeping its place in the mod list. +/// +/// Repeatable: a folder already carrying THIS revision's marker is taken again, +/// and `state` is saved with the claims before any marker moves, so a crash at +/// any point leaves folders either revision can take. Symmetric too: going back +/// to an older revision takes its folders back from the newer one - or, when +/// one of the rules above keeps a folder, drops its claim on it so the member +/// installs a fresh copy. +/// +/// Returns the folders another revision still owns that this one does not use +/// and the active profile still enables. They stay installed and enabled - +/// removing a mod is the user's call - so the report has to name them until +/// the user disables or removes them. A revision whose record cannot be read +/// is named too. +pub fn adopt_other_revisions( + inst: &Instance, + c: &Collection, + state: &mut InstallState, + save: &mut dyn FnMut(&InstallState) -> Result<(), String>, +) -> Result, String> { + use crate::state::{key_for, revision_dir, Status}; + let _lock = inst + .try_lock("adopting another collection revision") + .map_err(|e| e.to_string())?; + let mods = inst.mods_dir(); + let marker = |owner: &str, key: &str| { + serde_json::to_string(&[owner, key]).expect("strings serialize") + }; + let owner = format!("{}:{}:{}", state.game_domain, state.slug, state.revision); + let domain = &c.info.domain_name; + let keys: Vec = c.mods.iter().map(|m| key_for(m, domain)).collect(); + // mods/ is shared by every profile. A folder another profile enables is + // that profile's setup: taking it over would replace an updated member's + // files under it, so this revision installs its own copy as it used to. + // Folder -> the other profiles that enable it, which the note names. + let active = inst.active_profile(); + let mut elsewhere: std::collections::HashMap> = Default::default(); + for p in inst.profiles().into_iter().filter(|p| *p != active) { + for m in inst.profile(&p).modlist().into_iter().filter(|m| m.is_active()) { + elsewhere.entry(m.name).or_default().push(p.clone()); + } + } + // The leftover note asks the user to disable a folder, and it holds the + // install short of faithful, so it must go away once they have. + let enabled_here: std::collections::HashSet = inst + .modlist() + .into_iter() + .filter(|m| m.is_active()) + .map(|m| m.name) + .collect(); + let mut leftovers = Vec::new(); + for other in other_revisions(inst, state, &mut leftovers)? { + let other_owner = format!("{}:{}:{}", other.game_domain, other.slug, other.revision); + let mut folders = other.folders.clone(); + for (key, status) in &other.members { + if let Status::Installed(f) | Status::Approximate(f, _) = status { + folders.entry(key.clone()).or_insert_with(|| f.clone()); + } + } + // (this revision's key, the other revision's key) + let mut pairs: Vec<(String, String)> = keys + .iter() + .filter(|k| folders.contains_key(*k)) + .map(|k| (k.clone(), k.clone())) + .collect(); + // The INI Tweaks folder is no member, but it is this collection's output + // too: unpaired, it was installed again as "(2)" and the old one was + // reported as unused. Only when this revision ships fragments at all: + // an empty directory installs nothing, so the old ones stayed owned. + if folders.contains_key(INI_TWEAKS_KEY) + && ships_ini_fragments(&revision_dir(&inst.root, &state.slug, state.revision)) + { + pairs.push((INI_TWEAKS_KEY.into(), INI_TWEAKS_KEY.into())); + } + let dir = revision_dir(&inst.root, &other.slug, other.revision); + if let Some(text) = cached_manifest(&dir)? { + let old = crate::read(&text)?.collection; + let old_domain = if old.info.domain_name.trim().is_empty() { + &other.game_domain + } else { + &old.info.domain_name + }; + let mut pages: std::collections::BTreeMap<_, (Vec, Vec)> = + Default::default(); + for (m, key) in c.mods.iter().zip(&keys) { + if let (false, Some(page)) = (folders.contains_key(key), nexus_page(m, domain)) { + pages.entry(page).or_default().0.push(key.clone()); + } + } + for m in &old.mods { + let key = key_for(m, old_domain); + if let (true, false, Some(page)) = ( + folders.contains_key(&key), + keys.contains(&key), + nexus_page(m, old_domain), + ) { + pages.entry(page).or_default().1.push(key); + } + } + for (new, old) in pages.into_values() { + if let ([new], [old]) = (&new[..], &old[..]) { + pairs.push((new.clone(), old.clone())); + } + } + } + // Claimed in the state, and the state saved, BEFORE any marker moves: a + // crash in between leaves a folder both revisions' adoption can still + // take, never one that no state claims. + let mut moves = Vec::new(); + // Folders left alone for a reason the leftover note must give instead + // of "this revision does not use it". + let mut why_kept: std::collections::HashMap = Default::default(); + let mut dropped_claim = false; + for (key, other_key) in pairs { + let folder = &folders[&other_key]; + // A key this revision already records in the SAME folder is taken + // back too: that is going back to a revision after a newer one took + // its folders over, which otherwise left every member unverifiable. + // A member the user skipped here (`--no-optional`, run before this) + // is not taken: it would be owned, enabled and in no ordering pass, + // reported only as skipped. Left alone it is named below, as skipped. + if matches!(state.status(&key), Status::Skipped) { + why_kept.insert(folder.clone(), "you skipped this optional member in this revision"); + continue; + } + if state.folders.get(&key).is_some_and(|f| f != folder) + || state.folders.iter().any(|(k, f)| *k != key && f.eq_ignore_ascii_case(folder)) + || eidos_install::fix_directory_name(folder).as_deref() != Some(folder.as_str()) + { + continue; + } + let path = mods.join(folder); + let (from, to) = (marker(&other_owner, &other_key), marker(&owner, &key)); + if !(owns_folder(&path, &from) || owns_folder(&path, &to)) { + continue; + } + let answers = installer_answers::path(&path).exists(); + if answers || elsewhere.contains_key(folder) { + if answers { + why_kept.insert( + folder.clone(), + "it holds saved installer answers, so this revision installs its own copy", + ); + } + // Going back, this revision still records the folder the newer + // one took. Kept, that claim could never verify again - the + // folder carries the newer marker - so every member failed for + // good. Dropped, the member installs a fresh copy beside it. + if state.folders.get(&key) == Some(folder) && !owns_folder(&path, &to) { + state.folders.remove(&key); + state.set(&key, Status::Pending); + dropped_claim = true; + } + continue; + } + // An unchanged member keeps its status too. Left Pending, the run + // recovered it as Approximate ("interrupted status save") unless it + // had clone hashes, so nearly every update read as not faithful for + // good. Install still verifies it, and replaces it when that fails. + // An updated member stays Pending: it is replaced anyway. + if key == other_key && state.status(&key).is_open() { + let status = match other.status(&other_key) { + Status::Installed(_) => Status::Installed(folder.clone()), + Status::Approximate(_, why) => Status::Approximate(folder.clone(), why.clone()), + _ => Status::Pending, + }; + if status != Status::Pending { + state.set(&key, status); + } + } + // On every pass, not only the one that seeded the status: a run + // stopped part-way finds the rest already seeded, and install + // verifies them without ever registering them. + let register = matches!( + state.status(&key), + Status::Installed(f) | Status::Approximate(f, _) if f == folder + ); + state.folders.insert(key, folder.clone()); + moves.push((path, from, to, folder.clone(), register)); + } + if !moves.is_empty() || dropped_claim { + save(state)?; + } + for (path, from, to, folder, register) in moves { + crate::recipe::transfer_receipt(&path, &from, &to)?; + let mut meta = eidos_instance::ModMeta::read(&path.join("meta.ini")); + meta.set("eidosCollectionOwner", &to); + meta.write(&path.join("meta.ini")) + .map_err(|e| e.to_string())?; + // A verified member is never registered again, so this does it, as + // the recovery did: a profile that does not list the folder enables + // it, one that lists it keeps its own decision. + if register { + inst.register_installed_mod(&folder) + .map_err(|e| e.to_string())?; + } + } + for (key, folder) in &folders { + // A folder this revision records is one it uses, whoever marks it. + if eidos_install::fix_directory_name(folder).as_deref() == Some(folder.as_str()) + && !state.folders.values().any(|f| f.eq_ignore_ascii_case(folder)) + && enabled_here.contains(folder) + && owns_folder(&mods.join(folder), &marker(&other_owner, key)) + { + // mods/ is shared: removing a folder another profile enables + // removes it from that profile too, so that note never says so. + let detail = match elsewhere.get(folder) { + Some(profiles) => format!( + "revision {} of this collection installed it and another profile ('{}') still enables it, so it was not taken over; disable it in this profile, but do not remove it while that profile uses it", + other.revision, + profiles.join("', '") + ), + None => format!( + "revision {} of this collection installed it and {}; it is still enabled, so disable or remove it yourself", + other.revision, + why_kept.get(folder).copied().unwrap_or("this revision does not use it") + ), + }; + leftovers.push(Note { subject: folder.clone(), detail }); + } + } + } + Ok(leftovers) +} + +/// Every other revision of this collection with a state file here, newest first. +/// +/// A state file that cannot be read is skipped with a note in `notes`: it only +/// costs the takeover of that revision's folders, while the error +/// [`InstallState::load`] gives is about the CURRENT revision's record. +fn other_revisions( + inst: &Instance, + state: &InstallState, + notes: &mut Vec, +) -> Result, String> { + let dir = inst.root.join("collections"); + let entries = match std::fs::read_dir(&dir) { + Ok(entries) => entries, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(e) => return Err(e.to_string()), + }; + let prefix = format!("{}-", crate::state::safe_slug(&state.slug)); + let mut found = Vec::new(); + for entry in entries { + let name = entry.map_err(|e| e.to_string())?.file_name(); + let Some(revision) = name + .to_str() + .and_then(|n| n.strip_prefix(&prefix)) + .and_then(|n| n.strip_suffix(".state.json")) + .and_then(|n| n.parse::().ok()) + .filter(|r| *r != state.revision) + else { + continue; + }; + let other = match InstallState::load(&dir.join(&name)) { + Ok(Some(other)) => other, + Ok(None) => continue, + Err(_) => { + notes.push(Note { + subject: format!("revision {revision}"), + detail: format!( + "its record {} cannot be read, so its folders were not taken over and this revision installs its own copies", + dir.join(&name).display() + ), + }); + continue; + } + }; + // A sanitized slug can collide with another collection's. + if other + .validate_revision(&state.slug, revision, &state.game_domain) + .is_ok() + { + found.push(other); + } + } + found.sort_by(|a, b| b.revision.cmp(&a.revision)); + Ok(found) +} + +/// The Nexus mod page a member comes from, which survives a file update. +fn nexus_page(m: &Mod, collection_domain: &str) -> Option<(String, u64)> { + if m.source.kind != SourceType::Nexus { + return None; + } + let domain = if m.domain_name.trim().is_empty() { + collection_domain + } else { + &m.domain_name + }; + Some((domain.trim().to_ascii_lowercase(), m.source.mod_id?)) +} + impl Hooks for RealHooks<'_> { fn validate_recipe(&mut self, c: &Collection) -> Result<(), String> { for m in &c.mods { @@ -255,6 +591,14 @@ impl Hooks for RealHooks<'_> { fn allow_runtime_mismatch(&self) -> bool { self.allow_runtime_mismatch } + fn adopt( + &mut self, + c: &Collection, + state: &mut InstallState, + save: &mut dyn FnMut(&InstallState) -> Result<(), String>, + ) -> Result, String> { + adopt_other_revisions(self.inst, c, state, save) + } fn verify_installed(&mut self, m: &Mod, folder: &str) -> Result { if eidos_install::fix_directory_name(folder).as_deref() != Some(folder) { @@ -429,11 +773,10 @@ impl Hooks for RealHooks<'_> { { return Installed::Failed("Refusing to replace an unowned collection folder".into()); } - let fallback = self.game.prefix().and_then(|prefix| { - self.game - .plugin_spec() - .map(|spec| eidos_plugins::plugins_txt_dir(&prefix, &spec)) - }); + // Where the game keeps its activation state, as launch and the GUI + // read it: the install root for Morrowind and root-mode Oblivion, where + // AppData has nothing and every plugin would read as active. + let fallback = self.game.plugin_state_dir(); let ctx = eidos_install::fomod_context_for_instance( self.inst, &self.game.data_path, @@ -449,7 +792,16 @@ impl Hooks for RealHooks<'_> { Err(e) => return Installed::Failed(e), }; let marker = self.owner_marker(m); - let policy = eidos_install::OverwritePolicy::ReplaceOwned(marker.clone()); + // A member folder replaced by a new revision, or by a repair, can hold + // files its receipt never saw (an edited INI, files moved in by Sync to + // Mods). Keep that folder as `_backup`; an untouched one is + // replaced outright, so an update does not double the disk use. + let keep = crate::recipe::holds_unreceipted_files(&mods_dir.join(folder)).unwrap_or(true); + let policy = if keep { + eidos_install::OverwritePolicy::ReplaceOwnedWithBackup(marker.clone()) + } else { + eidos_install::OverwritePolicy::ReplaceOwned(marker.clone()) + }; let finish = |stage: &Path| { crate::recipe::finish(&mapped, &self.payload_root, stage, &marker) .map_err(eidos_install::InstallError::BadSelection) @@ -720,6 +1072,10 @@ impl Hooks for RealHooks<'_> { Ok(report) => { unmatched.extend(report.warnings); if report.pending_effects { + // The files are published, so a kept folder exists now. + if let Some(backup) = report.install.backup.clone() { + self.kept.push((m.name.clone(), backup)); + } return Installed::NeedsUser(format!("OMOD files were published; approved profile effects remain pending at {}. Resume to retry them", report.receipt_path.display())); } Ok(report.install) @@ -811,6 +1167,9 @@ impl Hooks for RealHooks<'_> { }; match result { Ok(rep) => { + if let Some(backup) = rep.backup.clone() { + self.kept.push((m.name.clone(), backup)); + } if let Err(e) = self.register(&rep.name, &m.name, &renamed_to) { return Installed::Failed(e); } @@ -994,7 +1353,13 @@ pub fn apply_plugin_states( }); return; }; - let local_dir = eidos_plugins::plugins_txt_dir(&prefix, &spec); + // Seed and fall back from where the game keeps its activation state, as + // launch and sort do: the install root for Morrowind and root-mode Oblivion. + // AppData held nothing there, so the profile was founded on an all-enabled + // default - and for Morrowind the write below then built the profile's + // Morrowind.ini from nothing, a `[Game Files]`-only stub every later launch + // deployed in place of the real INI. + let local_dir = eidos_plugins::plugin_state_dir(&prefix, &game.install_path, &spec); let _ = inst.ensure_profiles(); let prof = inst.active(); if prof.seed_plugin_state(&local_dir, &spec).is_err() { @@ -1044,8 +1409,11 @@ pub fn apply_plugin_states( }); return; } - // Shadow for tools that read the prefix; never fatal. - let _ = list.write_load_order(&local_dir, &spec); + // Shadow for tools that read the prefix; never fatal. Skipped for Timestamp + // games, as launch does: their state dir can be the game install itself. + if spec.mechanism != eidos_plugins::LoadOrderMechanism::Timestamp { + let _ = list.write_load_order(&local_dir, &spec); + } for name in missing { report.loot_notes.push(Note { subject: name, @@ -1176,6 +1544,28 @@ pub fn apply_plugin_rules( } } +/// The state key of the folder [`apply_ini_tweaks`] installs. +const INI_TWEAKS_KEY: &str = "aux:ini-tweaks"; + +/// The collection's "INI Tweaks" directory, in whatever case its author used. +fn ini_tweaks_source(dir: &Path) -> std::io::Result> { + Ok(std::fs::read_dir(dir)?.filter_map(Result::ok).map(|e| e.path()).find(|p| { + p.is_dir() + && p.file_name() + .is_some_and(|n| n.to_string_lossy().eq_ignore_ascii_case("ini tweaks")) + })) +} + +/// Whether `dir` ships INI fragments: the test [`apply_ini_tweaks`] makes +/// before it installs anything. +fn ships_ini_fragments(dir: &Path) -> bool { + ini_tweaks_source(dir) + .ok() + .flatten() + .and_then(|src| std::fs::read_dir(src).ok()) + .is_some_and(|mut files| files.any(|f| f.is_ok_and(|f| f.path().is_file()))) +} + /// Copy the collection's INI fragments in as a mod of their own. pub fn apply_ini_tweaks( inst: &Instance, @@ -1186,13 +1576,7 @@ pub fn apply_ini_tweaks( report: &mut Report, ) { let result = (|| -> Result<(), String> { - let entries = std::fs::read_dir(dir).map_err(|e| e.to_string())?; - let src = entries.filter_map(Result::ok).map(|e| e.path()).find(|p| { - p.is_dir() - && p.file_name() - .is_some_and(|n| n.to_string_lossy().eq_ignore_ascii_case("ini tweaks")) - }); - let Some(src) = src else { + let Some(src) = ini_tweaks_source(dir).map_err(|e| e.to_string())? else { return Ok(()); }; let root = std::fs::canonicalize(dir).map_err(|e| e.to_string())?; @@ -1212,7 +1596,7 @@ pub fn apply_ini_tweaks( if !files.iter().any(|f| f.path().is_file()) { return Ok(()); } - let key = "aux:ini-tweaks"; + let key = INI_TWEAKS_KEY; let owner = format!("{}:{}:{}", state.game_domain, state.slug, state.revision); let marker = serde_json::to_string(&[owner.as_str(), key]).expect("strings serialize"); let wanted = safe(&format!("{} - INI Tweaks", c.info.name)); @@ -1235,6 +1619,11 @@ pub fn apply_ini_tweaks( return Err("The INI output directory is not an owned directory".into()); } std::fs::create_dir_all(&dest).map_err(|e| e.to_string())?; + let shipped: Vec<_> = files + .iter() + .filter(|f| f.path().is_file()) + .map(|f| f.file_name()) + .collect(); let mut n = 0; for file in files { if !file.path().is_file() { @@ -1263,9 +1652,56 @@ pub fn apply_ini_tweaks( } n += 1; } + // A folder taken over from another revision still held its fragments, + // and the user's selection of them was merged in at every launch. Only + // the fragments a collection run installed itself are removed: one the + // user added here (by hand, or with MO2's Create Tweak) is theirs. A + // folder from before that record has no list, so a fragment this + // revision does not ship is only deselected there, and named. + let meta_path = inst.mods_dir().join(&name).join("meta.ini"); + let mut meta = eidos_instance::ModMeta::read(&meta_path); + let shipped: Vec = shipped.iter().map(|s| s.to_string_lossy().into_owned()).collect(); + let ours = meta.collection_ini_fragments(); + let mut removed = Vec::new(); + for old in ours.iter().flatten().filter(|old| !shipped.contains(old)) { + // The record is ours, but a name with a separator would leave dest. + if Path::new(old).file_name().is_none_or(|f| f != old.as_str()) { + continue; + } + let path = dest.join(old); + if std::fs::symlink_metadata(&path).is_ok_and(|m| m.file_type().is_file()) { + std::fs::remove_file(&path).map_err(|e| e.to_string())?; + removed.push(old.clone()); + } + } + let dropped: Vec = meta + .ini_tweaks() + .iter() + .filter(|t| if ours.is_some() { removed.contains(t) } else { !shipped.contains(t) }) + .cloned() + .collect(); + if !dropped.is_empty() { + let selected: Vec = + meta.ini_tweaks().iter().filter(|t| !dropped.contains(t)).cloned().collect(); + meta.set_ini_tweaks(&selected); + } + meta.set_collection_ini_fragments(&shipped); + meta.write(&meta_path).map_err(|e| e.to_string())?; inst.register_installed_mod(&name) .map_err(|e| e.to_string())?; - report.deferred.push(Note { subject: "INI tweaks".into(), detail: format!("{n} fragment(s) installed as '{name}'; select the wanted fragments in its INI Tweaks tab") }); + let mut detail = format!("{n} fragment(s) installed as '{name}'; select the wanted fragments in its INI Tweaks tab"); + if !removed.is_empty() { + detail += &format!(". Removed, as this revision no longer ships them: {}", removed.join(", ")); + } + let deselected: Vec<&str> = + dropped.iter().filter(|d| !removed.contains(d)).map(String::as_str).collect(); + if !deselected.is_empty() { + detail += &format!( + ". Deselected, as this revision does not ship them (select them again if you added them yourself): {}", + deselected.join(", ") + ); + } + report.deferred.push(Note { subject: "INI tweaks".into(), detail }); Ok(()) })(); if let Err(error) = result { @@ -1297,4 +1733,277 @@ mod cache_tests { assert!(cached_manifest(&dir).unwrap().is_none()); std::fs::remove_dir_all(root).unwrap(); } + + const GATE: &str = "skyrimspecialedition"; + fn mark(owner: &str, key: &str) -> String { + serde_json::to_string(&[owner, key]).unwrap() + } + fn key(file: u64) -> String { + format!("file:{GATE}:{file}") + } + fn owner(revision: u32) -> String { + format!("{GATE}:gate:{revision}") + } + fn gate_state(revision: u32) -> InstallState { + InstallState { slug: "gate".into(), revision, game_domain: GATE.into(), ..Default::default() } + } + /// Cache `mods` as the manifest of `revision` and return it parsed. + fn gate_manifest(root: &Path, revision: u32, mods: &[(&str, u64, u64)]) -> Collection { + let mods: Vec = mods + .iter() + .map(|(name, m, f)| format!(r#"{{"name":"{name}","source":{{"type":"nexus","modId":{m},"fileId":{f}}}}}"#)) + .collect(); + let text = format!(r#"{{"info":{{"name":"Gate","domainName":"{GATE}"}},"mods":[{}]}}"#, mods.join(",")); + let dir = crate::state::revision_dir(root, "gate", revision); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("collection.json"), &text).unwrap(); + std::fs::write(cache_marker(&dir), eidos_nexus::md5_file(&dir.join("collection.json")).unwrap()).unwrap(); + crate::read(&text).unwrap().collection + } + /// Revision 1 installed: one member revision 2 keeps, one it updates to a + /// new file of the same page, one it drops. Returns revision 2's recipe. + fn gate(tag: &str) -> (PathBuf, Instance, Collection) { + use crate::state::Status; + let root = std::env::temp_dir().join(format!("eidos-revision-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let inst = Instance::portable(root.clone()); + gate_manifest(&root, 1, &[("Kept", 1, 10), ("Updated", 2, 20), ("Dropped", 3, 30)]); + let mut old = gate_state(1); + for (name, file) in [("Kept", 10), ("Updated", 20), ("Dropped", 30)] { + let folder = inst.mods_dir().join(name); + std::fs::create_dir_all(&folder).unwrap(); + let mut meta = eidos_instance::ModMeta::default(); + meta.set("eidosCollectionOwner", &mark(&owner(1), &key(file))); + meta.write(&folder.join("meta.ini")).unwrap(); + old.folders.insert(key(file), name.into()); + old.set(&key(file), Status::Installed(name.into())); + } + let body = serde_json::json!({"schema":1,"owner":mark(&owner(1), &key(10)),"recipe":null,"files":{},"exclusions":{}}); + std::fs::write(inst.mods_dir().join("Kept/.eidos-collection-recipe.json"), body.to_string()).unwrap(); + old.save(&InstallState::path(&root, "gate", 1)).unwrap(); + std::fs::create_dir_all(inst.profile("Default").dir()).unwrap(); + std::fs::write(inst.profile("Default").dir().join("modlist.txt"), "+Kept\n+Updated\n+Dropped\n").unwrap(); + let new = gate_manifest(&root, 2, &[("Kept", 1, 10), ("Updated", 2, 21), ("Added", 4, 40)]); + (root, inst, new) + } + + #[test] + fn a_new_revision_takes_over_its_previous_folders_instead_of_duplicating_them() { + let (root, inst, new) = gate("adopt"); + let mut state = gate_state(2); + let mut saved = None; + let leftovers = adopt_other_revisions(&inst, &new, &mut state, &mut |s| { + // The claim is durable before the folder changes hands. + assert!(owns_folder(&inst.mods_dir().join("Kept"), &mark(&owner(1), &key(10)))); + saved = Some(s.folders.clone()); + Ok(()) + }) + .unwrap(); + assert_eq!(saved.as_ref(), Some(&state.folders)); + assert_eq!(state.folders.get(&key(10)).map(String::as_str), Some("Kept")); + assert_eq!(state.folders.get(&key(21)).map(String::as_str), Some("Updated")); + assert_eq!(state.folders.len(), 2); + assert!(owns_folder(&inst.mods_dir().join("Updated"), &mark(&owner(2), &key(21)))); + let receipt = inst.mods_dir().join("Kept/.eidos-collection-recipe.json"); + let moved: serde_json::Value = serde_json::from_slice(&std::fs::read(&receipt).unwrap()).unwrap(); + assert_eq!(moved["owner"], mark(&owner(2), &key(10))); + // The dropped member is named, not deleted. + assert_eq!(leftovers.iter().map(|n| n.subject.as_str()).collect::>(), ["Dropped"]); + assert!(inst.mods_dir().join("Dropped").is_dir()); + // The member reserves its old folder instead of "Kept (2)". + assert_eq!(reserve_folder(&inst, "Kept", &mark(&owner(2), &key(10)), state.folders.get(&key(10)).map(String::as_str)).unwrap(), "Kept"); + // Repeatable: a crash before the new state was saved adopts the same folders. + let mut again = gate_state(2); + assert_eq!(adopt_other_revisions(&inst, &new, &mut again, &mut |_| Ok(())).unwrap(), leftovers); + assert_eq!(again.folders, state.folders); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn an_unchanged_member_keeps_its_installed_status_and_an_updated_one_does_not() { + use crate::state::Status; + let (root, inst, new) = gate("status"); + std::fs::write(inst.profile("Default").dir().join("modlist.txt"), "+Updated\n+Dropped\n").unwrap(); + let mut state = gate_state(2); + adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + // Pending would be recovered as Approximate, an install that is not faithful. + assert_eq!(state.status(&key(10)), &Status::Installed("Kept".into())); + assert_eq!(state.status(&key(21)), &Status::Pending); + // Verified members are not registered again, so adoption enables an + // unlisted one. + assert!(inst.modlist().iter().any(|m| m.name == "Kept" && m.enabled)); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn the_ini_tweaks_folder_moves_to_a_revision_that_still_ships_ini_tweaks() { + let (root, inst, new) = gate("ini"); + let folder = inst.mods_dir().join("Gate - INI Tweaks"); + std::fs::create_dir_all(&folder).unwrap(); + let mut meta = eidos_instance::ModMeta::default(); + meta.set("eidosCollectionOwner", &mark(&owner(1), INI_TWEAKS_KEY)); + meta.write(&folder.join("meta.ini")).unwrap(); + let path = InstallState::path(&root, "gate", 1); + let mut old = InstallState::load(&path).unwrap().unwrap(); + old.folders.insert(INI_TWEAKS_KEY.into(), "Gate - INI Tweaks".into()); + old.save(&path).unwrap(); + std::fs::create_dir_all(folder.join("Ini Tweaks")).unwrap(); + std::fs::write(folder.join("Ini Tweaks/Dropped.ini"), "[Display]\n").unwrap(); + std::fs::write(folder.join("Ini Tweaks/Kept.ini"), "[Display]\n").unwrap(); + let mut meta = eidos_instance::ModMeta::read(&folder.join("meta.ini")); + meta.set_ini_tweaks(&["Dropped.ini".into(), "Kept.ini".into()]); + meta.write(&folder.join("meta.ini")).unwrap(); + let dir = crate::state::revision_dir(&root, "gate", 2); + std::fs::create_dir_all(dir.join("INI Tweaks")).unwrap(); + std::fs::write(inst.profile("Default").dir().join("modlist.txt"), "+Kept\n+Updated\n+Gate - INI Tweaks\n").unwrap(); + // An empty directory ships nothing, so it takes nothing over. + let mut state = gate_state(2); + let leftovers = adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + assert_eq!(state.folders.get(INI_TWEAKS_KEY), None); + assert!(leftovers.iter().any(|n| n.subject == "Gate - INI Tweaks"), "{leftovers:?}"); + std::fs::write(dir.join("INI Tweaks/Kept.ini"), "[Display]\n").unwrap(); + let mut state = gate_state(2); + let leftovers = adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + assert_eq!(state.folders.get(INI_TWEAKS_KEY).map(String::as_str), Some("Gate - INI Tweaks")); + assert!(owns_folder(&folder, &mark(&owner(2), INI_TWEAKS_KEY))); + assert!(leftovers.iter().all(|n| n.subject != "Gate - INI Tweaks"), "{leftovers:?}"); + // The fragment revision 2 dropped no longer applies at launch. This + // folder predates the record of what a run installed, so the unknown + // fragment is deselected and named, never deleted. + let mut report = Report::default(); + apply_ini_tweaks(&inst, &dir, &new, &mut state, &mut |_| Ok(()), &mut report); + assert!(report.failed.is_empty(), "{report:?}"); + assert!(folder.join("Ini Tweaks/Dropped.ini").is_file()); + assert!(folder.join("Ini Tweaks/Kept.ini").is_file()); + let meta = eidos_instance::ModMeta::read(&folder.join("meta.ini")); + assert_eq!(meta.ini_tweaks(), ["Kept.ini"]); + assert_eq!(meta.collection_ini_fragments(), Some(vec!["Kept.ini".to_string()])); + assert!(report.deferred.iter().any(|n| n.detail.contains("Deselected") && n.detail.contains("Dropped.ini")), "{report:?}"); + // Now the run's own fragments are known: one a later revision drops is + // removed, while a fragment the user added and selected stays. + std::fs::write(folder.join("Ini Tweaks/Mine.ini"), "[Display]\n").unwrap(); + let mut meta = eidos_instance::ModMeta::read(&folder.join("meta.ini")); + meta.set_ini_tweaks(&["Kept.ini".into(), "Mine.ini".into()]); + meta.write(&folder.join("meta.ini")).unwrap(); + std::fs::remove_file(dir.join("INI Tweaks/Kept.ini")).unwrap(); + std::fs::write(dir.join("INI Tweaks/New.ini"), "[Display]\n").unwrap(); + let mut report = Report::default(); + apply_ini_tweaks(&inst, &dir, &new, &mut state, &mut |_| Ok(()), &mut report); + assert!(report.failed.is_empty(), "{report:?}"); + assert!(!folder.join("Ini Tweaks/Kept.ini").exists()); + assert!(folder.join("Ini Tweaks/Mine.ini").is_file()); + assert!(folder.join("Ini Tweaks/New.ini").is_file()); + assert_eq!(eidos_instance::ModMeta::read(&folder.join("meta.ini")).ini_tweaks(), ["Mine.ini"]); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn a_member_skipped_in_the_new_revision_is_named_not_taken_over() { + use crate::state::Status; + let (root, inst, new) = gate("skipped"); + let mut state = gate_state(2); + state.set_by_user(&key(10), Status::Skipped); + let leftovers = adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + assert_eq!(state.folders.get(&key(10)), None); + assert!(owns_folder(&inst.mods_dir().join("Kept"), &mark(&owner(1), &key(10)))); + // This revision does use it; the user skipped it. + assert!(leftovers.iter().any(|n| n.subject == "Kept" && n.detail.contains("you skipped")), "{leftovers:?}"); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn a_folder_another_profile_enables_is_not_taken_over() { + let (root, inst, new) = gate("profiles"); + std::fs::create_dir_all(inst.profile("Stable").dir()).unwrap(); + std::fs::write(inst.profile("Stable").dir().join("modlist.txt"), "+Updated\n").unwrap(); + let mut state = gate_state(2); + let leftovers = adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + // "Stable" keeps revision 1's files; the update installs its own copy. + assert_eq!(state.folders.get(&key(21)), None); + // Removing it would remove it from "Stable" too, so the note never asks. + let note = leftovers.iter().find(|n| n.subject == "Updated").unwrap(); + assert!(note.detail.contains("'Stable'") && !note.detail.contains("or remove"), "{note:?}"); + assert!(owns_folder(&inst.mods_dir().join("Updated"), &mark(&owner(1), &key(20)))); + assert_eq!(state.folders.get(&key(10)).map(String::as_str), Some("Kept")); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn a_leftover_the_user_disabled_is_no_longer_reported() { + let (root, inst, new) = gate("disabled"); + std::fs::write(inst.profile("Default").dir().join("modlist.txt"), "+Kept\n+Updated\n-Dropped\n").unwrap(); + let mut state = gate_state(2); + let leftovers = adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + // Disabling is what the note asks for, so it must clear the note. + assert!(leftovers.is_empty(), "{leftovers:?}"); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn going_back_to_the_previous_revision_takes_its_folders_back() { + let (root, inst, new) = gate("rollback"); + let mut state = gate_state(2); + adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + state.save(&InstallState::path(&root, "gate", 2)).unwrap(); + let old_recipe = gate_manifest(&root, 1, &[("Kept", 1, 10), ("Updated", 2, 20), ("Dropped", 3, 30)]); + let mut old = InstallState::load(&InstallState::path(&root, "gate", 1)).unwrap().unwrap(); + let leftovers = adopt_other_revisions(&inst, &old_recipe, &mut old, &mut |_| Ok(())).unwrap(); + assert!(owns_folder(&inst.mods_dir().join("Kept"), &mark(&owner(1), &key(10)))); + assert!(owns_folder(&inst.mods_dir().join("Updated"), &mark(&owner(1), &key(20)))); + // Revision 1 uses both, so neither is reported as something it does not use. + assert!(leftovers.is_empty(), "{leftovers:?}"); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn going_back_drops_a_folder_it_cannot_take_back_so_the_member_installs_fresh() { + use crate::state::Status; + let (root, inst, new) = gate("rollback-kept"); + let mut state = gate_state(2); + adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + state.save(&InstallState::path(&root, "gate", 2)).unwrap(); + // A copy of the profile made before going back enables every folder. + std::fs::create_dir_all(inst.profile("Backup").dir()).unwrap(); + std::fs::write(inst.profile("Backup").dir().join("modlist.txt"), "+Kept\n+Updated\n").unwrap(); + let old_recipe = gate_manifest(&root, 1, &[("Kept", 1, 10), ("Updated", 2, 20), ("Dropped", 3, 30)]); + let mut old = InstallState::load(&InstallState::path(&root, "gate", 1)).unwrap().unwrap(); + let mut saved = None; + adopt_other_revisions(&inst, &old_recipe, &mut old, &mut |s| { + saved = Some(s.folders.clone()); + Ok(()) + }) + .unwrap(); + // Kept, the claim on a folder marked by revision 2 failed every run. + assert!(owns_folder(&inst.mods_dir().join("Kept"), &mark(&owner(2), &key(10)))); + assert_eq!(old.folders.get(&key(10)), None); + assert_eq!(old.status(&key(10)), &Status::Pending); + assert_eq!(saved.as_ref(), Some(&old.folders)); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn an_interrupted_adoption_still_registers_the_members_it_seeded() { + use crate::state::Status; + let (root, inst, new) = gate("resume"); + std::fs::write(inst.profile("Default").dir().join("modlist.txt"), "+Updated\n+Dropped\n").unwrap(); + // Seeded and saved, then stopped before the folder changed hands. + let mut state = gate_state(2); + state.set(&key(10), Status::Installed("Kept".into())); + state.folders.insert(key(10), "Kept".into()); + adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + assert!(inst.modlist().iter().any(|m| m.name == "Kept" && m.enabled)); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn an_unreadable_record_of_another_revision_does_not_stop_the_install() { + let (root, inst, new) = gate("unreadable"); + std::fs::write(InstallState::path(&root, "gate", 3), "{ truncated").unwrap(); + let mut state = gate_state(2); + let leftovers = adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + assert!(leftovers.iter().any(|n| n.subject == "revision 3"), "{leftovers:?}"); + // Revision 1 is still taken over. + assert_eq!(state.folders.get(&key(10)).map(String::as_str), Some("Kept")); + std::fs::remove_dir_all(root).unwrap(); + } } diff --git a/crates/eidos-collections/src/install.rs b/crates/eidos-collections/src/install.rs index 0418f1c5..a8e0c5f9 100644 --- a/crates/eidos-collections/src/install.rs +++ b/crates/eidos-collections/src/install.rs @@ -68,6 +68,17 @@ pub trait Hooks { fn allow_runtime_mismatch(&self) -> bool { false } + /// Take over what another revision of this collection installed, once the + /// recipe and the runtime have passed and before the first member. Returns + /// what the report has to name. + fn adopt( + &mut self, + _c: &Collection, + _state: &mut InstallState, + _save: &mut dyn FnMut(&InstallState) -> Result<(), String>, + ) -> Result, String> { + Ok(Vec::new()) + } /// Get this member's archive, or say why not. fn obtain(&mut self, m: &Mod) -> Obtained; /// Reserve an owned destination before the engine persists it and starts extraction. @@ -155,6 +166,19 @@ pub fn run( }), _ => {} } + // Only now: a revision stopped by either gate above must not have taken the + // folders of the revision the user stays on. + match hooks.adopt(c, state, save) { + Ok(notes) => report.deferred.extend(notes), + Err(error) => { + report.aborted = true; + report.failed.push(Note { + subject: "previous revision".into(), + detail: format!("its folders could not be taken over: {error}"), + }); + return report; + } + } let order = install_order(c); let total = order.len(); @@ -506,6 +530,42 @@ mod tests { ); } + #[test] + fn a_revision_stopped_at_the_runtime_gate_takes_nothing_over() { + struct Gated { + allow: bool, + adopted: usize, + } + impl Hooks for Gated { + fn runtime_check(&mut self, _: &Collection) -> crate::recipe::RuntimeCheck { + crate::recipe::RuntimeCheck::Mismatch { observed: "new".into(), expected: vec!["old".into()] } + } + fn allow_runtime_mismatch(&self) -> bool { + self.allow + } + fn adopt(&mut self, _: &Collection, _: &mut InstallState, _: &mut dyn FnMut(&InstallState) -> Result<(), String>) -> Result, String> { + self.adopted += 1; + Ok(vec![Note { subject: "Old".into(), detail: "unused".into() }]) + } + fn obtain(&mut self, _: &Mod) -> Obtained { + assert_eq!(self.adopted, 1, "adoption comes before the first member"); + Obtained::Ready("archive".into()) + } + fn install(&mut self, m: &Mod, _: &std::path::Path, _: &str) -> Installed { + Installed::Ok(m.name.clone()) + } + } + let c = collection(vec![member("A", 0, false, 1)]); + let mut hooks = Gated { allow: false, adopted: 0 }; + // The user may stay on the revision they have; it keeps its folders. + assert!(run(&c, &mut InstallState::default(), &mut hooks, &mut noop).aborted); + assert_eq!(hooks.adopted, 0); + hooks.allow = true; + let report = run(&c, &mut InstallState::default(), &mut hooks, &mut noop); + assert_eq!(hooks.adopted, 1); + assert!(report.deferred.iter().any(|n| n.subject == "Old")); + } + #[test] fn members_install_by_phase_then_by_the_order_the_collection_listed_them() { let c = collection(vec![ diff --git a/crates/eidos-collections/src/recipe.rs b/crates/eidos-collections/src/recipe.rs index a5846284..fb3052dc 100644 --- a/crates/eidos-collections/src/recipe.rs +++ b/crates/eidos-collections/src/recipe.rs @@ -707,3 +707,49 @@ pub fn verify_receipt(m: &Mod, payload: &Path, folder: &Path, owner: &str) -> Re && receipt.recipe == identity(m, payload)? && receipt.files == tree_digests(folder)?) } +/// Whether `folder` holds files its install receipt does not vouch for: a file +/// the user edited, added or hid, files moved in by Sync to Mods, or content +/// with no readable receipt at all. A fresh reservation holds nothing to lose. +pub fn holds_unreceipted_files(folder: &Path) -> Result { + let current = tree_digests(folder)?; + if current.is_empty() { + return Ok(false); + } + let path = folder.join(RECEIPT); + match fs::symlink_metadata(&path) { + Ok(meta) if meta.file_type().is_file() => {} + _ => return Ok(true), + } + Ok( + match serde_json::from_slice::(&bounded_read(&path, 64 * 1024 * 1024)?) { + Ok(receipt) => receipt.files != current, + Err(_) => true, + }, + ) +} +/// Hand a receipt to the next revision of the same collection. +/// +/// The owner is part of what a receipt proves, so without this every member a +/// new revision takes over would fail verification and be extracted again - +/// downloaded again too, when `downloads/` was cleaned. The recipe and file +/// digests are untouched and still checked against the new revision's member, +/// so a member the author changed is replaced as before. A receipt that is +/// missing, unreadable or names any other owner is left alone, and +/// verification rejects it. +pub fn transfer_receipt(folder: &Path, from: &str, to: &str) -> Result<(), String> { + let path = folder.join(RECEIPT); + if !fs::symlink_metadata(&path).is_ok_and(|m| m.file_type().is_file()) { + return Ok(()); + } + let Ok(mut receipt) = + serde_json::from_slice::(&bounded_read(&path, 64 * 1024 * 1024)?) + else { + return Ok(()); + }; + if receipt.owner != from { + return Ok(()); + } + receipt.owner = to.into(); + let bytes = serde_json::to_vec(&receipt).map_err(|e| e.to_string())?; + eidos_instance::write_atomic(&path, &bytes).map_err(|e| e.to_string()) +} diff --git a/crates/eidos-collections/src/report.rs b/crates/eidos-collections/src/report.rs index 0b0c53f4..2012c393 100644 --- a/crates/eidos-collections/src/report.rs +++ b/crates/eidos-collections/src/report.rs @@ -47,6 +47,9 @@ pub struct Report { /// Members installed under a different folder name because a mod that is /// not this collection's already had that one. pub renamed: Vec, + /// Members whose previous folder differed from its install receipt, so it + /// was kept beside the replacement instead of deleted. + pub kept: Vec, /// Unverified requirements and remaining collection-wide choices, including /// unknown runtime evidence, an approved runtime mismatch, and INI selection. pub deferred: Vec, @@ -129,6 +132,11 @@ impl Report { "Installed under a different name, because a mod of yours already had it:", &self.renamed, ); + section( + &mut out, + "Replaced, with the previous folder kept beside it:", + &self.kept, + ); section(&mut out, "Load order:", &self.loot_notes); section( &mut out, diff --git a/crates/eidos-collections/tests/downloads.rs b/crates/eidos-collections/tests/downloads.rs index 28bdab34..e3a12cfb 100644 --- a/crates/eidos-collections/tests/downloads.rs +++ b/crates/eidos-collections/tests/downloads.rs @@ -42,6 +42,7 @@ fn a_direct_member_preserves_unrelated_downloads_and_partials() { collection_domain: def.nexus_game.into(), owner: "test:1".into(), renamed: vec![], + kept: vec![], }; for (name, suffix) in [ ("Complete", ""), diff --git a/crates/eidos-collections/tests/installer_replay.rs b/crates/eidos-collections/tests/installer_replay.rs index 417b50ac..94cdfcad 100644 --- a/crates/eidos-collections/tests/installer_replay.rs +++ b/crates/eidos-collections/tests/installer_replay.rs @@ -88,6 +88,7 @@ print(json.dumps({'protocol':1,'request_id':r['request_id'],'outcome':o})) collection_domain: "oblivion".into(), owner: "fixture:1".into(), renamed: vec![], + kept: vec![], payload_root: root.clone(), allow_runtime_mismatch: false, }; diff --git a/crates/eidos-collections/tests/ownership.rs b/crates/eidos-collections/tests/ownership.rs index dacaddd0..e1a392d1 100644 --- a/crates/eidos-collections/tests/ownership.rs +++ b/crates/eidos-collections/tests/ownership.rs @@ -95,6 +95,7 @@ fn a_previously_unavailable_member_does_not_own_an_existing_personal_mod() { collection_domain: "skyrimspecialedition".into(), owner: "test:1".into(), renamed: vec![], + kept: vec![], }; let folder = hooks .reserve( @@ -254,6 +255,7 @@ fn a_stale_download_sidecar_does_not_hide_a_later_complete_archive() { collection_domain: "skyrimspecialedition".into(), owner: "test:1".into(), renamed: vec![], + kept: vec![], }; assert_eq!( hooks.obtain(&member), @@ -348,6 +350,7 @@ fn bundle_clone_patch_pipeline_resumes_and_failed_replacement_retains_owned_and_ collection_domain: c.info.domain_name.clone(), owner: "synthetic:1".into(), renamed: vec![], + kept: vec![], }; let mut state = InstallState::default(); let mut durable = state.clone(); @@ -459,6 +462,7 @@ fn omod_recipes_patch_before_publication_and_hashes_use_decoded_sources() { collection_domain: "oblivion".into(), owner: "omod:1".into(), renamed: vec![], + kept: vec![], }; for (name, hashes) in [ ("Plain", vec![]), @@ -521,3 +525,72 @@ fn omod_recipes_patch_before_publication_and_hashes_use_decoded_sources() { .to_string_lossy() .starts_with(".eidos-install"))); } + +#[test] +fn replacing_a_member_keeps_a_folder_that_holds_files_its_receipt_never_saw() { + let temp = Fixture::new(); + let inst = eidos_instance::Instance::portable(temp.0.join("instance")); + inst.create().unwrap(); + let def = eidos_games::catalog() + .iter() + .find(|g| g.id == "skyrimse") + .unwrap(); + let game = eidos_games::DetectedGame { + source: Default::default(), + def, + install_path: temp.0.join("game"), + data_path: temp.0.join("game/Data"), + compatdata: None, + steam_name: "test".into(), + }; + fs::create_dir_all(&game.data_path).unwrap(); + let member = Mod { + name: "Member".into(), + source: Source { + file_id: Some(1), + mod_id: Some(1), + ..Default::default() + }, + ..Default::default() + }; + let archive = archive(&temp.0); + let nexus = eidos_nexus::Nexus::with_bearer("synthetic-no-network"); + let mut say = |_: String| {}; + let mut hooks = RealHooks { + payload_root: std::path::PathBuf::new(), + allow_runtime_mismatch: false, + nexus: &nexus, + inst: &inst, + game: &game, + game_id: "skyrimse".into(), + say: &mut say, + collection_domain: "skyrimspecialedition".into(), + owner: "test:1".into(), + renamed: vec![], + kept: vec![], + }; + let folder = hooks.reserve(&member, None).unwrap(); + let installed = hooks.install(&member, &archive, &folder); + assert!(matches!(installed, Installed::Ok(_)), "{installed:?}"); + let backup = inst.mods_dir().join(format!("{folder}_backup")); + + // An untouched member is replaced outright: nothing is kept. + let installed = hooks.install(&member, &archive, &folder); + assert!(matches!(installed, Installed::Ok(_)), "{installed:?}"); + assert!(!backup.exists()); + assert!(hooks.kept.is_empty()); + + // A file the user moved in (Sync to Mods) is not in the receipt. + let mine = inst.mods_dir().join(&folder).join("scripts/mine.pex"); + fs::write(&mine, b"my edit").unwrap(); + let installed = hooks.install(&member, &archive, &folder); + assert!(matches!(installed, Installed::Ok(_)), "{installed:?}"); + assert!(!mine.exists()); + assert_eq!(fs::read(backup.join("scripts/mine.pex")).unwrap(), b"my edit"); + assert_eq!(hooks.kept, vec![(member.name.clone(), backup)]); + + let mut report = eidos_collections::report::Report::default(); + hooks.drain_notes(&mut report); + assert_eq!(report.kept.len(), 1); + assert!(report.render().contains("Member_backup"), "{}", report.render()); +} diff --git a/crates/eidos-collections/tests/plugin_states.rs b/crates/eidos-collections/tests/plugin_states.rs index 5c40df73..a2ff0ca8 100644 --- a/crates/eidos-collections/tests/plugin_states.rs +++ b/crates/eidos-collections/tests/plugin_states.rs @@ -110,3 +110,79 @@ fn external_store_plugin_activation_uses_actual_wine_prefix() { ); fs::remove_dir_all(root).unwrap(); } + +/// A root-mode game on a never-launched profile, with `A.esp` active and +/// `B.esp` deliberately off in the install-root state, and a collection that +/// switches `A.esp` off. +fn root_mode_fixture( + tag: &str, + id: &str, + data: &str, + files: &[(&str, &str)], +) -> (std::path::PathBuf, eidos_instance::Instance, eidos_games::DetectedGame) { + let root = std::env::temp_dir().join(format!("eidos-collection-{tag}-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + let inst = eidos_instance::Instance::portable(root.join("instance")); + inst.create().unwrap(); + let def = eidos_games::catalog().iter().find(|g| g.id == id).unwrap(); + let game = eidos_games::DetectedGame { + source: Default::default(), + def, + install_path: root.join("game"), + data_path: root.join("game").join(data), + compatdata: Some(root.join("compatdata")), + steam_name: "synthetic".into(), + }; + fs::create_dir_all(&game.data_path).unwrap(); + for (name, body) in files { + fs::write(game.install_path.join(name), body).unwrap(); + } + let collection = eidos_collections::Collection { + plugins: vec![eidos_collections::manifest::Plugin { + name: "A.esp".into(), + enabled: false, + }], + ..Default::default() + }; + let mut report = Default::default(); + for name in ["A.esp", "B.esp"] { + fs::write(game.data_path.join(name), []).unwrap(); + } + eidos_collections::driver::apply_plugin_states(&inst, &game, &collection, &mut report); + (root, inst, game) +} + +#[test] +fn morrowind_collection_seeds_the_install_ini_instead_of_writing_a_stub() { + let ini = "[General]\r\nSubtitles=1\r\n[Archives]\r\nArchive 0=Tribunal.bsa\r\n\ + [Game Files]\r\nGameFile0=A.esp\r\n"; + let (root, inst, game) = + root_mode_fixture("morrowind", "morrowind", "Data Files", &[("Morrowind.ini", ini)]); + let profile_ini = + fs::read_to_string(inst.active().plugins_state_dir().join("Morrowind.ini")).unwrap(); + // The real INI's other sections survive, and B.esp stays off as the user left it. + assert!(profile_ini.contains("Archive 0=Tribunal.bsa"), "{profile_ini}"); + assert!(profile_ini.contains("[General]"), "{profile_ini}"); + assert!(!profile_ini.contains("A.esp") && !profile_ini.contains("B.esp"), "{profile_ini}"); + // The install root is the state dir here; the prefix shadow must not touch it. + assert_eq!(fs::read_to_string(game.install_path.join("Morrowind.ini")).unwrap(), ini); + assert!(!game.install_path.join("loadorder.txt").exists()); + fs::remove_dir_all(root).unwrap(); +} + +#[test] +fn oblivion_root_mode_collection_seeds_the_install_root_plugins_txt() { + let ini = "[General]\r\nbUseMyGamesDirectory=0\r\n"; + let (root, inst, game) = root_mode_fixture( + "oblivion-root", + "oblivion", + "Data", + &[("Oblivion.ini", ini), ("plugins.txt", "A.esp\r\n")], + ); + let list = inst.plugin_list(&game.data_path, game.def.id, None).unwrap(); + // B.esp was off in the install-root plugins.txt; founding on the empty + // AppData dir turned it on. + assert!(list.plugins.iter().all(|p| !p.enabled), "{:?}", list.plugins); + assert_eq!(fs::read_to_string(game.install_path.join("plugins.txt")).unwrap(), "A.esp\r\n"); + fs::remove_dir_all(root).unwrap(); +} diff --git a/crates/eidos-collections/tests/recipes.rs b/crates/eidos-collections/tests/recipes.rs index e064321d..b5f1374a 100644 --- a/crates/eidos-collections/tests/recipes.rs +++ b/crates/eidos-collections/tests/recipes.rs @@ -573,6 +573,7 @@ fn actual_pe_runtime_evidence_gates_the_real_driver_before_member_mutation() { collection_domain: def.nexus_game.into(), owner: "test:1".into(), renamed: vec![], + kept: vec![], }; c.mods.push(Mod { name: "Must never be reserved".into(), diff --git a/crates/eidos-core/src/lib.rs b/crates/eidos-core/src/lib.rs index d4de8964..cf9f4908 100644 --- a/crates/eidos-core/src/lib.rs +++ b/crates/eidos-core/src/lib.rs @@ -364,6 +364,25 @@ impl LayerStack { Self::new_with_unindexed_subtree(layers, overwrite, None) } + /// A stack that never builds the lower index, for a caller asking it a + /// handful of shallow questions and then dropping it. + /// + /// The index costs a full recursive walk of every layer up front - a stat + /// and several allocations per file, ~300 ms on a 300-mod list - which only + /// pays for itself over the thousands of lookups a mount serves. The GUI's + /// health checks build a throwaway stack on every mod toggle to read one + /// root listing or one `SKSE/Plugins` directory; for them the walk IS the + /// cost. Every answer is the one `new` gives: no index is the same complete + /// fallback `EIDOS_NO_INDEX` forces. + pub fn new_unindexed(layers: Vec, overwrite: PathBuf) -> Self { + // Built over no layers, so the index walk has nothing to visit; the + // real layers go in after, with the index dropped so none answers. + let mut stack = Self::new(Vec::new(), overwrite); + stack.layers = layers; + stack.lower = None; + stack + } + /// Avoid indexing the descendants of a directory that a child mount will /// cover. This is an index boundary, not a visibility boundary: reads and /// listings there retain the normal live layer walk. Invalid or empty @@ -1023,21 +1042,50 @@ impl LayerStack { if real_dir { fs::create_dir_all(&dest)?; } else if src.is_file() { - fs::copy(&src, &dest)?; - // The point of a copy-up is that the result is WRITABLE, and - // `fs::copy` clones the source's mode - so a read-only lower - // file (a Steam depot restored 0444, a mod extracted from a - // Windows archive carrying the DOS read-only attribute) yields - // a read-only copy whose very next read-write open fails - // EACCES. Do it BEFORE clone_metadata: `lsetxattr` of a - // `user.*` attribute onto a 0444 file is refused even for the - // owner, so the xattrs would be silently dropped otherwise. - ensure_owner_writable(&dest); - // Re-apply the lower file's mtime/atime + user.* xattrs so the - // copied-up file looks unchanged to tools comparing mtimes - // (FileTime load order, xEdit, Wrye Bash) or reading DOS - // attributes - usvfs preserves these by writing through in place. - clone_metadata(&src, &dest); + // Staged under a temporary name and renamed into place only + // once complete. `fs::copy` straight onto `dest` left a + // truncated file behind whenever it failed part-way (ENOSPC + // on the Overwrite volume) or the process was killed mid-copy; + // the next call then saw `dest.exists()` and never copied + // again, so the cut-off file shadowed the intact lower one in + // this session and every later one. It also let a concurrent + // reader resolve the half-written file whenever the index was + // rebuilt from disk during the copy. The temp sits beside + // `dest` (same filesystem, so the rename is atomic) and ends + // in HIDDEN_SUFFIX, so readdir and resolve never serve it - a + // WHITEOUT_PREFIX name would instead hide a sibling. The + // short pid+counter name never collides and never pushes a + // long file name past NAME_MAX. A kill can still strand one, + // but only an invisible temp, never a wrong `dest`. + static COPY_UP_SEQ: AtomicU64 = AtomicU64::new(0); + let tmp = dest.with_file_name(format!( + ".eidos-copyup.{}.{}{HIDDEN_SUFFIX}", + std::process::id(), + COPY_UP_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); + let staged = fs::copy(&src, &tmp).and_then(|_| { + // The point of a copy-up is that the result is WRITABLE, and + // `fs::copy` clones the source's mode - so a read-only lower + // file (a Steam depot restored 0444, a mod extracted from a + // Windows archive carrying the DOS read-only attribute) yields + // a read-only copy whose very next read-write open fails + // EACCES. Do it BEFORE clone_metadata: `lsetxattr` of a + // `user.*` attribute onto a 0444 file is refused even for the + // owner, so the xattrs would be silently dropped otherwise. + ensure_owner_writable(&tmp); + // Re-apply the lower file's mtime/atime + user.* xattrs so the + // copied-up file looks unchanged to tools comparing mtimes + // (FileTime load order, xEdit, Wrye Bash) or reading DOS + // attributes - usvfs preserves these by writing through in place. + // Both land on the temp: the rename keeps the inode, so + // `dest` appears with its final mode, times and xattrs. + clone_metadata(&src, &tmp); + fs::rename(&tmp, &dest) + }); + if let Err(e) = staged { + let _ = fs::remove_file(&tmp); + return Err(e); + } } } } else if self.read_lower(vpath).is_some() { @@ -2024,6 +2072,28 @@ mod tests { assert_eq!(view.list_dir("").len(), 2); } + #[test] + fn unindexed_stack_skips_the_walk_and_answers_like_the_indexed_one() { + let t = TempTree::new(); + let (high, low, over) = (t.sub("high"), t.sub("low"), t.sub("over")); + put(&high, "Mod.esp", "high plugin"); + put(&high, "SKSE/Plugins/a.dll", "high dll"); + put(&low, "mod.esp", "low plugin"); + put(&low, "Gone.esp", "whited out"); + put(&low, "skse/plugins/b.dll", "low dll"); + let indexed = LayerStack::new(vec![high.clone(), low.clone()], over.clone()); + indexed.remove("Gone.esp").unwrap(); + let unindexed = LayerStack::new_unindexed(vec![high, low], over); + assert!(indexed.lower.is_some()); + assert!(unindexed.lower.is_none(), "no walk may run up front"); + for vpath in ["MOD.ESP", "Gone.esp", "skse/PLUGINS/b.dll", "missing.esp"] { + assert_eq!(unindexed.resolve_read(vpath), indexed.resolve_read(vpath)); + } + for dir in ["", "SKSE/Plugins"] { + assert_eq!(unindexed.list_dir(dir), indexed.list_dir(dir)); + } + } + #[test] fn directory_recreated_over_a_deleted_file_does_not_expose_deeper_layers() { let t = TempTree::new(); @@ -2285,6 +2355,24 @@ mod tests { ); } + #[test] + fn copy_up_that_fails_part_way_leaves_nothing_in_the_overwrite() { + let t = TempTree::new(); + let (game, over) = (t.sub("game"), t.sub("over")); + // A lower "file" whose read fails AFTER the copy has created its + // destination: `/proc/self/mem` is a regular file that opens fine and + // returns EIO at offset 0 (the zero page is never mapped) - the same + // shape as ENOSPC or a kill in the middle of a large copy. + std::os::unix::fs::symlink("/proc/self/mem", game.join("plugin.esp")).unwrap(); + let stack = LayerStack::new(vec![game.clone()], over.clone()); + + assert!(stack.open_for_write("plugin.esp").is_err()); + // No truncated copy at the final path to shadow the lower file for good, + // and no stranded temp either. + assert_eq!(fs::read_dir(&over).unwrap().count(), 0); + assert_eq!(stack.resolve_read("plugin.esp"), Some(game.join("plugin.esp"))); + } + #[test] fn copy_up_never_chmods_through_a_symlink_into_a_lower_layer() { use std::os::unix::fs::PermissionsExt; @@ -2766,11 +2854,11 @@ mod tests { let results: Vec<_> = workers.into_iter().map(|w| w.join().unwrap()).collect(); assert_eq!(results.iter().filter(|(_, r)| r.is_ok()).count(), 1); for (from, result) in results { - let folder = if result.is_ok() { - "dest" - } else { - assert_eq!(result.unwrap_err().raw_os_error(), Some(libc::ENOTEMPTY)); + let folder = if let Err(e) = result { + assert_eq!(e.raw_os_error(), Some(libc::ENOTEMPTY)); from + } else { + "dest" }; for index in 0..64 { assert_eq!(read(&overwrite.join(format!("{folder}/{index}.txt"))), from); diff --git a/crates/eidos-gamefeatures/src/archives.rs b/crates/eidos-gamefeatures/src/archives.rs index a1792c4d..95fa14b4 100644 --- a/crates/eidos-gamefeatures/src/archives.rs +++ b/crates/eidos-gamefeatures/src/archives.rs @@ -92,8 +92,10 @@ pub fn archive_plan( if line.starts_with([';', '#']) { continue; } - if let Some(s) = line.strip_prefix('[').and_then(|s| s.strip_suffix(']')) { - section = s.trim().to_ascii_lowercase(); + // The shared header rule, so a BOM'd first section is not read as + // keys of section "" (the engine sees past the BOM). + if let Some(s) = eidos_ini::section_header(line) { + section = s.to_ascii_lowercase(); continue; } if let Some((k, v)) = line.split_once('=') { @@ -318,6 +320,28 @@ mod tests { assert_eq!(plan.archives[1].plugin.as_deref(), Some("Patch.esp")); } + #[test] + fn a_bom_does_not_hide_the_first_ini_section() { + // A StarfieldCustom.ini saved as "UTF-8 with BOM": its first section is + // still [Archive] to the engine, so the archive it lists is registered. + let plan = archive_plan( + "starfield", + &names(&["MyMod - Main.ba2"]), + &[], + &names(&["\u{feff}[Archive]\r\nsResourceArchiveList2=MyMod - Main.ba2\r\n"]), + "en", + ); + assert_eq!( + plan.archives + .iter() + .map(|a| a.name.as_str()) + .collect::>(), + ["MyMod - Main.ba2"], + "{:?}", + plan.diagnostics + ); + } + #[test] fn disabled_archives_ini_overrides_language_and_order() { let files = names(&[ diff --git a/crates/eidos-gamefeatures/src/preflight.rs b/crates/eidos-gamefeatures/src/preflight.rs index ad25e775..99a71c7f 100644 --- a/crates/eidos-gamefeatures/src/preflight.rs +++ b/crates/eidos-gamefeatures/src/preflight.rs @@ -76,7 +76,11 @@ pub fn scan_skse( }; let mut data_layers = mods.to_vec(); data_layers.push(("Game".into(), game_data.to_path_buf())); - let data = eidos_core::LayerStack::new( + // Unindexed, both this stack and the root one below: each answers one + // listing and a couple of lookups, and the GUI's health checks run this on + // every mod toggle. An index would first walk every file of every mod (and, + // for the root, the whole game install) to answer those few questions. + let data = eidos_core::LayerStack::new_unindexed( data_layers.iter().map(|(_, p)| p.clone()).collect(), overwrite.to_path_buf(), ); @@ -103,7 +107,7 @@ pub fn scan_skse( .collect(); root_layers.push(("Game".into(), game_root.to_path_buf())); let root_overwrite = overwrite.join("Root"); - let root = eidos_core::LayerStack::new( + let root = eidos_core::LayerStack::new_unindexed( root_layers.iter().map(|(_, p)| p.clone()).collect(), root_overwrite.clone(), ); diff --git a/crates/eidos-gui/src/main.rs b/crates/eidos-gui/src/main.rs index 2295fedf..4b67761d 100644 --- a/crates/eidos-gui/src/main.rs +++ b/crates/eidos-gui/src/main.rs @@ -1222,6 +1222,9 @@ struct IniEditorState { /// buffer over a file that exists destroys it - and a permission error is /// exactly when that would happen. unreadable: bool, + /// A hash of the file's bytes as read, so a save can tell that another + /// process rewrote it since (see `IniEditorSave`). + on_disk: u64, } /// A mod-install name collision: `mods//` already exists, so the user picks @@ -1750,6 +1753,15 @@ pub(crate) struct DataRow { /// cloning ~5k Strings per redraw (which is what it did, and what made the /// "cache" allocate proportionally to its own payload). type CachedListing = (u64, std::rc::Rc>); +/// The same for one directory of the Data tab's merged view. +type CachedDataListing = (u64, std::rc::Rc>); +/// The Data tab's union and its layer labels, with the generation they were +/// built at (see [`view::data_stack`]). +type CachedDataStack = ( + u64, + std::rc::Rc, + std::rc::Rc, +); struct App { installer: Option, @@ -1763,8 +1775,17 @@ struct App { created: Option, error: Option, mods: Vec, + /// A content hash of `modlist.txt` as it was when `mods` was last read from + /// or written to it; `None` until then. Another Eidos process (the CLI, a + /// second window Steam or a collection link opened) can rewrite the file + /// while this window shows its old copy, and saving that copy would erase + /// what it wrote, so `save_mods` refuses when the file no longer matches. + modlist_seen: std::cell::Cell>, /// Cached ESP/ESM load order for the Plugins tab (recomputed on demand). plugins: Option, + /// The same guard for `plugins`: a hash of the profile's plugin state files + /// as `compute_plugins` read them or `write_plugin_state` last wrote them. + plugins_seen: std::cell::Cell>, /// Cached per-file conflict analysis for the Conflicts tab + mod-row flags. conflicts: Option, archive_epoch: std::cell::Cell, @@ -2226,7 +2247,7 @@ struct App { /// relative to `Data`, `""` for the root), each with the generation it was /// built at. The tree merges a level at a time, so only the directories the /// user actually opened are ever read. - data_listing: std::cell::RefCell)>>, + data_listing: std::cell::RefCell>, /// The profile chip row's data: every profile name and which one is active. /// /// Both were read from disk on EVERY frame of the main screen - a `read_dir` @@ -2248,7 +2269,7 @@ struct App { /// merge beside it: whiteouts, opaque directories, hidden names, case-folded /// dedup and NTFS collation all live in one place, and the tab can no longer /// disagree with the filesystem the game sees. - data_stack: std::cell::RefCell)>>, + data_stack: std::cell::RefCell>, /// Free-text filter over the Data tree. data_query: String, /// Show only paths more than one mod provides. @@ -4232,7 +4253,7 @@ mod tests { String::new(), ( app.view_generation.get(), - vec![DataRow { + std::rc::Rc::new(vec![DataRow { name: "SKSE".into(), source: "[skyrimse]".into(), is_dir: true, @@ -4240,7 +4261,7 @@ mod tests { size: None, mtime: None, conflicted: false, - }], + }]), ), ); app.listing_cache.borrow_mut().insert( @@ -6035,6 +6056,150 @@ mod tests { ); } + #[test] + fn a_plugins_tab_edit_on_a_fresh_morrowind_profile_keeps_the_real_ini() { + let root = temp_portable("morrowind"); + let inst = Instance::portable(root.join("instance")); + inst.create().unwrap(); + let mut app = app_for_game("morrowind"); + app.games[0].install_path = root.join("game"); + app.games[0].data_path = root.join("game/Data Files"); + let game = app.games[0].clone(); + fs::create_dir_all(&game.data_path).unwrap(); + for name in ["A.esp", "B.esp"] { + fs::write(game.data_path.join(name), []).unwrap(); + } + let ini = "[General]\r\nSubtitles=1\r\n[Game Files]\r\nGameFile0=A.esp\r\nGameFile1=B.esp\r\n"; + fs::write(game.install_path.join("Morrowind.ini"), ini).unwrap(); + app.created = Some(inst.clone()); + let spec = game.plugin_spec().unwrap(); + let mut list = inst + .plugin_list(&game.data_path, "morrowind", game.plugin_state_dir().as_deref()) + .unwrap(); + assert!(list.set_enabled("B.esp", false)); + write_plugin_state(&app, &list, &spec).unwrap(); + let written = + fs::read_to_string(inst.active().plugins_state_dir().join("Morrowind.ini")).unwrap(); + assert!(written.contains("Subtitles=1"), "{written}"); + assert!(written.contains("GameFile0=A.esp") && !written.contains("B.esp"), "{written}"); + assert_eq!(fs::read_to_string(game.install_path.join("Morrowind.ini")).unwrap(), ini); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn a_plugin_edit_in_a_stale_window_does_not_revert_another_process_sort() { + let root = temp_portable("morrowind"); + let inst = Instance::portable(root.join("instance")); + inst.create().unwrap(); + let mut app = app_for_game("morrowind"); + app.games[0].install_path = root.join("game"); + app.games[0].data_path = root.join("game/Data Files"); + let game = app.games[0].clone(); + fs::create_dir_all(&game.data_path).unwrap(); + for name in ["A.esp", "B.esp"] { + fs::write(game.data_path.join(name), []).unwrap(); + } + let ini = "[General]\r\n[Game Files]\r\nGameFile0=A.esp\r\nGameFile1=B.esp\r\n"; + fs::write(game.install_path.join("Morrowind.ini"), ini).unwrap(); + app.created = Some(inst.clone()); + let spec = game.plugin_spec().unwrap(); + app.plugins = compute_plugins(&app); + let list = app.plugins.clone().unwrap(); + write_plugin_state(&app, &list, &spec).unwrap(); + // `eidos sort` in a terminal rewrites the profile's order while the + // window keeps the list it read before. + let state = inst.active().plugins_state_dir().join("Morrowind.ini"); + let sorted = "[General]\r\n[Game Files]\r\nGameFile0=A.esp\r\nGameFile1=B.esp\r\n"; + assert_ne!(fs::read_to_string(&state).unwrap(), sorted, "the sort must change something"); + fs::write(&state, sorted).unwrap(); + let mut stale = list.clone(); + assert!(stale.set_enabled("B.esp", false)); + assert!(write_plugin_state(&app, &stale, &spec).is_err()); + assert_eq!(fs::read_to_string(&state).unwrap(), sorted); + // Re-read, the window's edits land again - twice, so its own write is + // not mistaken for another process's. + app.plugins = compute_plugins(&app); + let mut fresh = app.plugins.clone().unwrap(); + assert!(fresh.set_enabled("B.esp", false)); + write_plugin_state(&app, &fresh, &spec).unwrap(); + assert!(fresh.set_enabled("B.esp", true)); + write_plugin_state(&app, &fresh, &spec).unwrap(); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn saving_morrowind_ini_in_the_window_does_not_block_its_next_plugin_edit() { + // The INI editor writes the same Morrowind.ini the plugin state is hashed + // from; the window's own save is not another process's sort. + let root = temp_portable("morrowind"); + let inst = Instance::portable(root.join("instance")); + inst.create().unwrap(); + let mut app = app_for_game("morrowind"); + app.games[0].install_path = root.join("game"); + app.games[0].data_path = root.join("game/Data Files"); + let game = app.games[0].clone(); + fs::create_dir_all(&game.data_path).unwrap(); + for name in ["A.esp", "B.esp"] { + fs::write(game.data_path.join(name), []).unwrap(); + } + let ini = "[General]\r\n[Game Files]\r\nGameFile0=A.esp\r\nGameFile1=B.esp\r\n"; + fs::write(game.install_path.join("Morrowind.ini"), ini).unwrap(); + app.created = Some(inst.clone()); + app.tab = Tab::Plugins; + let spec = game.plugin_spec().unwrap(); + app.plugins = compute_plugins(&app); + let list = app.plugins.clone().unwrap(); + write_plugin_state(&app, &list, &spec).unwrap(); + + let _ = update_inner(&mut app, Message::ShowIniEditor); + { + let ed = app.ini_editor.as_mut().expect("the editor opened"); + assert_eq!(ed.current, "Morrowind.ini"); + let edited = ed.content.text().replace("[General]", "[General]\r\nSubtitles=1"); + ed.content = iced::widget::text_editor::Content::with_text(&edited); + ed.dirty = true; + } + let _ = update_inner(&mut app, Message::IniEditorSave); + assert!(!app.ini_editor.as_ref().unwrap().dirty, "status={:?}", app.status); + + let mut next = app.plugins.clone().unwrap(); + assert!(next.set_enabled("B.esp", false)); + write_plugin_state(&app, &next, &spec).unwrap(); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn the_ini_editor_opens_the_real_morrowind_ini_and_never_reverts_an_outside_write() { + // A fresh profile used to open an EMPTY buffer, and saving it founded + // the profile's Morrowind.ini on a stub. + let root = temp_portable("morrowind"); + let inst = Instance::portable(root.join("instance")); + inst.create().unwrap(); + let mut app = app_for_game("morrowind"); + app.games[0].install_path = root.join("game"); + app.games[0].data_path = root.join("game/Data Files"); + let game = app.games[0].clone(); + fs::create_dir_all(&game.data_path).unwrap(); + let ini = "[General]\r\n[Game Files]\r\nGameFile0=A.esp\r\n"; + fs::write(game.install_path.join("Morrowind.ini"), ini).unwrap(); + app.created = Some(inst.clone()); + + let _ = update_inner(&mut app, Message::ShowIniEditor); + let ed = app.ini_editor.as_mut().expect("the editor opened"); + assert!(!ed.missing, "status={:?}", app.status); + assert!(ed.original.contains("GameFile0=A.esp"), "{}", ed.original); + + // `eidos sort` rewrites it while the editor is open. + let path = inst.active().ini_path("Morrowind.ini"); + fs::write(&path, "[Game Files]\r\nGameFile0=B.esp\r\n").unwrap(); + ed.content = iced::widget::text_editor::Content::with_text("[General]\r\nSubtitles=1\r\n"); + ed.dirty = true; + let _ = update_inner(&mut app, Message::IniEditorSave); + assert!(app.ini_editor.as_ref().unwrap().dirty); + assert_eq!(fs::read_to_string(&path).unwrap(), "[Game Files]\r\nGameFile0=B.esp\r\n"); + fs::remove_dir_all(root).unwrap(); + } + /// An instance with two profiles and a save in the active one. fn saves_app() -> (App, PathBuf) { let root = temp_portable("skyrimse"); @@ -7363,6 +7528,115 @@ mod tests { (app, root) } + #[test] + fn an_edit_in_a_stale_window_keeps_what_another_process_wrote() { + let root = temp_portable("skyrimse"); + let inst = Instance::portable(root.clone()); + inst.create().unwrap(); + for n in ["Aaa", "Zzz"] { + fs::create_dir_all(root.join("mods").join(n)).unwrap(); + } + let mut app = app_for_game("skyrimse"); + app.created = Some(inst.clone()); + app.screen = Screen::Main; + reload_mods(&mut app); + // `eidos install` in a terminal while the window is open. + fs::create_dir_all(root.join("mods").join("SkyUI")).unwrap(); + inst.register_installed_mod("SkyUI").unwrap(); + // A tick in the window, which still shows the list from before. + let at = |app: &App| app.mods.iter().position(|m| m.name == "Aaa").unwrap(); + let i = at(&app); + app.mods[i].enabled = !app.mods[i].enabled; + mods_changed(&mut app); + let skyui = inst.modlist().into_iter().find(|m| m.name == "SkyUI"); + assert!(skyui.is_some_and(|m| m.enabled), "status={:?}", app.status); + assert!(app.mods.iter().any(|m| m.name == "SkyUI"), "the window reloaded"); + // Reloaded, the window's edits land again - twice, so its own save is + // not mistaken for another process's. + for on in [true, false] { + let i = at(&app); + app.mods[i].enabled = on; + mods_changed(&mut app); + let aaa = inst.modlist().into_iter().find(|m| m.name == "Aaa").unwrap(); + assert_eq!(aaa.enabled, on, "status={:?}", app.status); + } + // A removal edits modlist.txt itself before saving; that is the + // window's own write, not another process's. + let zzz = app.mods.iter().position(|m| m.name == "Zzz").unwrap(); + let _ = update_inner(&mut app, Message::ModRemove(zzz)); + let _ = update_inner(&mut app, Message::ModRemove(zzz)); + assert_eq!(app.status.as_deref(), Some("Removed 'Zzz'.")); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn a_rename_in_a_stale_window_is_refused_before_the_folder_moves() { + let root = temp_portable("skyrimse"); + let inst = Instance::portable(root.clone()); + inst.create().unwrap(); + for n in ["Aaa", "SkyUI"] { + fs::create_dir_all(root.join("mods").join(n)).unwrap(); + } + let mut app = app_for_game("skyrimse"); + app.created = Some(inst.clone()); + app.screen = Screen::Main; + reload_mods(&mut app); + for m in app.mods.iter_mut() { + m.enabled = true; + } + mods_changed(&mut app); + // `eidos install` in a terminal while the window is open. + fs::create_dir_all(root.join("mods").join("Foo")).unwrap(); + inst.register_installed_mod("Foo").unwrap(); + + let i = app.mods.iter().position(|m| m.name == "SkyUI").unwrap(); + let _ = update_inner(&mut app, Message::RenameStart(i)); + let _ = update_inner(&mut app, Message::RenameChanged("SkyUI 5.2".to_string())); + let _ = update_inner(&mut app, Message::RenameCommit); + + // Refused while the folder still had its old name, and said so: had it + // moved first, the reload would have listed "SkyUI 5.2" DISABLED. + assert!(root.join("mods").join("SkyUI").is_dir(), "status={:?}", app.status); + assert!(!root.join("mods").join("SkyUI 5.2").exists()); + assert!(app.status.as_deref().unwrap_or("").contains("another Eidos process")); + let listed = inst.modlist(); + assert!(listed.iter().any(|m| m.name == "SkyUI" && m.enabled)); + assert!(listed.iter().any(|m| m.name == "Foo" && m.enabled)); + assert!(app.mods.iter().any(|m| m.name == "Foo"), "the window reloaded"); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn batch_edits_in_a_stale_window_report_the_refusal_not_success() { + let root = temp_portable("skyrimse"); + let inst = Instance::portable(root.clone()); + inst.create().unwrap(); + for n in ["Aaa", "Bbb"] { + fs::create_dir_all(root.join("mods").join(n)).unwrap(); + } + let mut app = app_for_game("skyrimse"); + app.created = Some(inst.clone()); + app.screen = Screen::Main; + reload_mods(&mut app); + let stale = |app: &mut App, name: &str| { + fs::create_dir_all(root.join("mods").join(name)).unwrap(); + inst.register_installed_mod(name).unwrap(); + app.selected_mods = (0..app.mods.len()).collect(); + }; + + // "Enable selected" used to replace the refusal with "Enabled 2 mod(s)." + stale(&mut app, "Foo"); + let _ = update_inner(&mut app, Message::BatchToggleMods); + assert!(app.status.as_deref().unwrap_or("").contains("another Eidos process")); + + // And a batch Remove refuses BEFORE deleting, not after. + stale(&mut app, "Bar"); + let _ = update_inner(&mut app, Message::ConfirmBatchRemove); + assert!(app.status.as_deref().unwrap_or("").contains("another Eidos process")); + assert!(root.join("mods").join("Aaa").is_dir()); + fs::remove_dir_all(root).unwrap(); + } + #[test] fn renaming_a_mod_leaves_it_where_it_was_and_still_enabled() { // The defect, exactly as it was reported: rename a mod and it is @@ -7388,6 +7662,9 @@ mod tests { mods_changed(&mut app); let before = app.mods.iter().position(|m| m.name == "Middle").unwrap(); let total = app.mods.len(); + // A second profile sharing the pool must follow the rename too. + let inst = app.created.clone().unwrap(); + inst.profile("Other").create_from(&inst.active()).unwrap(); let _ = update_inner(&mut app, Message::RenameStart(before)); let _ = update_inner(&mut app, Message::RenameChanged("Renamed".to_string())); @@ -7421,6 +7698,11 @@ mod tests { .expect("in modlist.txt"); assert_eq!(reloaded, before); assert!(app.mods[reloaded].enabled); + let (other, trust) = inst.profile("Other").modlist_checked(); + assert!(trust.is_good(), "{trust:?}"); + let at = other.iter().position(|m| m.name == "Renamed").expect("listed in Other"); + assert_eq!(at, before); + assert!(other[at].enabled); let _ = fs::remove_dir_all(&root); } @@ -10235,6 +10517,24 @@ mod tests { let _ = fs::remove_dir_all(&root); } + #[test] + fn a_data_source_is_the_longest_root_found_by_lookup() { + // Attribution used to scan every root per entry, O(entries x mods). The + // lookup must keep its longest-prefix rule: a mod nested under another + // root belongs to the inner one, and a sibling that merely shares a + // string prefix (`mods/A` vs `mods/AB`) is not a match at all. + let mut sources = DataSources::new(); + sources.insert(PathBuf::from("/i/mods/A"), "A".to_string()); + sources.insert(PathBuf::from("/i/mods/A/inner"), "Inner".to_string()); + sources.insert(PathBuf::from("/g/Data"), "[skyrimse]".to_string()); + let src = |p: &str| data_source(&sources, Path::new(p)); + assert_eq!(src("/i/mods/A/inner/x.nif"), "Inner"); + assert_eq!(src("/i/mods/A/meshes/x.nif"), "A"); + assert_eq!(src("/i/mods/A"), "A"); + assert_eq!(src("/i/mods/AB/x.nif"), ""); + assert_eq!(src("/g/Data/Skyrim.esm"), "[skyrimse]"); + } + #[test] fn the_data_filter_reaches_into_folders_and_drops_empty_ones() { let (mut app, root) = data_app( diff --git a/crates/eidos-gui/src/modinfo.rs b/crates/eidos-gui/src/modinfo.rs index c3760204..e9d0336d 100644 --- a/crates/eidos-gui/src/modinfo.rs +++ b/crates/eidos-gui/src/modinfo.rs @@ -1757,8 +1757,9 @@ pub(crate) fn diagnostics(app: &App) -> Vec { // This check used to skip itself and print "load order not computed yet", // which reads as reassurance and is not: it says nothing was looked at, on // the one check most likely to predict a crash. If the cache is cold, - // compute the answer. `diagnostics` only runs when something changed, not - // per frame, so it can afford to. + // compute the answer. "Something changed" includes every checkbox click and + // every Ctrl+Arrow move, so `compute_plugins` must stay cheap: it reads the + // layers' root listings, never a full index of every mod's files. let computed; let plugins = match app.plugins.as_ref() { Some(list) => Some(list), @@ -1905,7 +1906,7 @@ pub(crate) fn diagnostics(app: &App) -> Vec { .mods .iter() .rev() - .filter(|m| m.is_active()) + .filter(|m| m.is_active() && !m.is_unmanaged()) .map(|m| (m.name.clone(), m.path.clone())) .collect::>(); for d in eidos_gamefeatures::preflight::scan_skse( @@ -2538,8 +2539,10 @@ pub(crate) fn compute_plugins(app: &App) -> Option { let spec = game.plugin_spec()?; let mut sources: Vec<(String, PathBuf)> = vec![(String::new(), game.data_path.clone())]; // app.mods is MO2 display order (lowest priority first) = the ascending order - // plugin discovery wants, so feed it through as-is. - let enabled = app.mods.iter().filter(|m| m.is_active()); + // plugin discovery wants, so feed it through as-is. Unmanaged rows (DLC and + // Creation Club) are not layers: their `path` is one `.esm` FILE inside Data, + // which is already the first source, and the mount drops them the same way. + let enabled = app.mods.iter().filter(|m| m.is_active() && !m.is_unmanaged()); sources.extend(enabled.map(|m| (m.name.clone(), m.path.clone()))); // The Overwrite layer is a plugin source too (a cleaned/generated .esp lands // there) - the launch path includes it, so the GUI must agree. @@ -2549,7 +2552,12 @@ pub(crate) fn compute_plugins(app: &App) -> Option { let mut list = PluginList::discover(&sources, &spec); if let Some(inst) = app.created.as_ref() { - let stack = eidos_core::LayerStack::new( + // Plugins only live at the root, so ONE merged root listing answers for + // all of them: a read of each layer's top directory, whiteouts and hidden + // names applied. This runs on every mod toggle (via `diagnostics`), so the + // stack is unindexed - an index walks every file of every enabled mod + // first, hundreds of milliseconds of frozen window on a large list. + let stack = eidos_core::LayerStack::new_unindexed( sources .iter() .rev() @@ -2558,13 +2566,23 @@ pub(crate) fn compute_plugins(app: &App) -> Option { .collect(), inst.overwrite_dir(), ); + let root: HashMap = stack + .list_dir("") + .into_iter() + .map(|(name, path)| (name.to_ascii_lowercase(), path)) + .collect(); list.plugins.retain(|plugin| { - stack - .resolve_read(&plugin.name) + root.get(&plugin.name.to_ascii_lowercase()) .is_some_and(|path| path.is_file()) }); } + // Recorded BEFORE the state is read, for the same reason as `reload_mods`: + // a write in between may refuse the next edit, never let a stale list over. + if let Some(inst) = app.created.as_ref() { + app.plugins_seen + .set(Some(plugin_state_fingerprint(&inst.active(), &spec))); + } // The load order is per-profile: read the active profile's own copy once it // has one, and otherwise the prefix's (which the profile adopts on first // launch). Same primitive as the launch path, so for PlainList games this also @@ -2668,6 +2686,19 @@ pub(crate) fn commit_plugin_order(app: &mut App, spec: &GameSpec) { } } +/// A hash of the profile's own plugin state - the files `compute_plugins` reads +/// and `write_plugin_state` rewrites. Not the prefix shadow: other tools write +/// that, and it is never read while the profile owns a state. +fn plugin_state_fingerprint(prof: &eidos_instance::Profile, spec: &GameSpec) -> u64 { + let dir = prof.plugins_state_dir(); + let read = |path: Option| path.and_then(|p| std::fs::read(p).ok()); + content_hash(&[ + read(eidos_plugins::newest_variant(&dir, spec.active_file())), + read(eidos_plugins::newest_variant(&dir, "loadorder.txt")), + read(Some(prof.locked_order_path())), + ]) +} + pub(crate) fn write_plugin_state( app: &App, list: &PluginList, @@ -2684,6 +2715,28 @@ pub(crate) fn write_plugin_state( .transpose()?; if let Some(inst) = app.created.as_ref() { let prof = inst.active(); + // `list` was read when the tab was opened, perhaps long ago. Since then + // `eidos sort`, `eidos collection` or a session another window started + // may have rewritten the profile's order, and writing `list` over it + // would revert that silently - then snapshot the revert, so not even the + // damage card could tell. Both callers re-read the list on Err. Checked + // before the seed below, which would itself change the files. + let unchanged = app + .plugins_seen + .get() + .is_none_or(|seen| seen == plugin_state_fingerprint(&prof, spec)); + if !unchanged { + return Err(std::io::Error::other( + "another Eidos process changed the load order; it has been reloaded - redo the change", + )); + } + // Adopt the game's state first, as launch and sort do, failing closed. + // Writing into an unseeded profile built Morrowind.ini from nothing: a + // `[Game Files]`-only stub that every later launch deployed in place of + // the real INI, since the seed never replaces a file the profile owns. + if let Some(dir) = selected_game(app).and_then(|g| g.plugin_state_dir()) { + prof.seed_plugin_state(&dir, spec)?; + } // A deliberate GUI edit is the user speaking: it must not trip the // "session damaged the active set" card, so the snapshot follows it - // EXCEPT while damage is currently flagged, where refreshing would @@ -2694,6 +2747,8 @@ pub(crate) fn write_plugin_state( if !damage_flagged { let _ = prof.snapshot_plugin_state(); } + app.plugins_seen + .set(Some(plugin_state_fingerprint(&prof, spec))); } if spec.mechanism != eidos_plugins::LoadOrderMechanism::Timestamp { if let Some(dir) = selected_game(app).and_then(|g| g.plugin_state_dir()) { @@ -4687,6 +4742,7 @@ pub(crate) fn after_install( load_downloads(app); } let mut where_to = String::new(); + let mut move_refused = None; // A drop aimed at a gap says WHERE, not just whether. Consumed here, after // `reload_mods`, because that is when the new row exists to be moved - and // this is MO2's own ordering too (install first, reposition after). @@ -4706,9 +4762,14 @@ pub(crate) fn after_install( let hidden = hidden_by_folds(app); let landed = move_block(&mut app.mods, &[at], dest); settle_folds_after_move(app, landed, 1, &hidden); - mods_changed(app); - app.selected_mod = Some(landed); - where_to = format!(" at priority {landed}"); + if mods_changed(app) { + app.selected_mod = Some(landed); + where_to = format!(" at priority {landed}"); + } else { + // The move was reverted; its reason rides along below instead + // of being replaced by "at priority N". + move_refused = app.status.take(); + } } } // A note the drop left for the installer to deliver - it could not say it @@ -4724,7 +4785,7 @@ pub(crate) fn after_install( } else { format!("Installed '{name}'{where_to}{note}.") }); - for warning in [warning, registration_warning].into_iter().flatten() { + for warning in [warning, registration_warning, move_refused].into_iter().flatten() { if let Some(status) = &mut app.status { status.push_str(&format!(" Warning: {warning}")); } diff --git a/crates/eidos-gui/src/state.rs b/crates/eidos-gui/src/state.rs index 35334020..3f81bbc5 100644 --- a/crates/eidos-gui/src/state.rs +++ b/crates/eidos-gui/src/state.rs @@ -279,7 +279,9 @@ pub(crate) fn new(launch_command: Vec) -> (App, Task) { created: None, error: None, mods: Vec::new(), + modlist_seen: std::cell::Cell::new(None), plugins: None, + plugins_seen: std::cell::Cell::new(None), conflicts: None, archive_epoch: std::cell::Cell::new(1), archive_completed_epoch: None, @@ -517,9 +519,11 @@ pub(crate) fn new(launch_command: Vec) -> (App, Task) { let _ = inst.ensure_profiles(); remember_open(&inst, id); app.selected = Some(i); - app.mods = modlist_with_unmanaged(&inst, app.games.get(i)); - app.categories = Some(inst.category_factory()); app.created = Some(inst); + // Through `reload_mods`, which also records what `modlist.txt` held: a + // window Steam opened is exactly the one most likely to sit next to + // another, and without the record its first save could not tell. + reload_mods(app); app.screen = Screen::Main; }; if let Some((i, inst)) = pinned { @@ -890,6 +894,9 @@ pub(crate) fn reload_mods(app: &mut App) { // across by name; anything that disappeared is dropped rather than silently // re-pointed at whatever took its place. let held = hold_mod_selection(app); + // Taken BEFORE the read: a write landing in between then makes the next + // save refuse needlessly, instead of letting a stale list through. + app.modlist_seen.set(Some(modlist_fingerprint(&inst))); app.mods = modlist_with_unmanaged(&inst, game.as_ref()); put_mod_selection(app, held); // Same moment the list is rebuilt: a category could have been added by an @@ -2284,6 +2291,47 @@ pub(crate) fn move_block(mods: &mut Vec, targets: &[usize], dest: usiz at } +/// A content hash, for telling whether a file changed since it was read. Not +/// mtime and size: two writes within one timestamp tick can leave both equal, +/// and flipping `+Mod` to `-Mod` does not change the size. +pub(crate) fn content_hash(value: &impl std::hash::Hash) -> u64 { + use std::hash::Hasher; + let mut h = std::hash::DefaultHasher::new(); + value.hash(&mut h); + h.finish() +} + +fn modlist_fingerprint(inst: &Instance) -> u64 { + content_hash(&inst.active().modlist_bytes()) +} + +/// Whether `modlist.txt` still holds what `app.mods` was read from (or nothing +/// was ever recorded to compare against). +fn modlist_unchanged(app: &App, inst: &Instance) -> bool { + app.modlist_seen + .get() + .is_none_or(|seen| seen == modlist_fingerprint(inst)) +} + +const MODLIST_STALE: &str = + "Not saved: another Eidos process changed the mod list. It has been reloaded - redo the change."; + +/// For a handler that changes `mods/` itself (renames a folder, creates one) +/// and then saves: the staleness check, made BEFORE the folder changes. Left +/// to `save_mods`, the refusal came after it, and the reload then found the +/// new folder listed nowhere and appended it DISABLED at the top priority +/// (a renamed mod, a mod built from the Overwrite stopped loading). Reloads and +/// says so when stale; `true` means refused, leave `mods/` alone. +pub(crate) fn refuse_stale_modlist(app: &mut App) -> bool { + if app.created.as_ref().is_none_or(|inst| modlist_unchanged(app, inst)) { + return false; + } + reload_mods(app); + mod_views_changed(app); + app.status = Some(MODLIST_STALE.to_string()); + true +} + /// Persist the mod list, surfacing a failure instead of losing it silently (a /// full disk or permission problem would otherwise revert the user's changes on /// the next restart with no warning). Returns the error text, if any. @@ -2296,9 +2344,46 @@ pub(crate) fn save_mods(app: &App) -> Option { Ok(l) => l, Err(e) => return Some(format!("Not saved: {e}.")), }; - inst.save_modlist(&app.mods) - .err() - .map(|e| format!("Could not save the mod list: {e}")) + // The lock serialises the writes, not this list's read, which may be minutes + // old: `eidos install` or a collection run since then rewrote the file, and + // writing `app.mods` wholesale would erase what they added (their mods come + // back DISABLED, the collection's order is gone). `mods_changed` reloads. + if !modlist_unchanged(app, inst) { + return Some(MODLIST_STALE.to_string()); + } + if let Err(e) = inst.save_modlist(&app.mods) { + return Some(format!("Could not save the mod list: {e}")); + } + app.modlist_seen.set(Some(modlist_fingerprint(inst))); + None +} + +/// [`Instance::forget_mods`] for mods the window has just deleted. It edits +/// `modlist.txt` itself, so the record of what the window last saw follows it - +/// otherwise the save right after would take the window's own edit for another +/// process's. Only when the list was current before: following an outside change +/// too would let that save erase it. +pub(crate) fn forget_removed_mods(app: &App, names: &[String]) -> Option> { + own_modlist_edit(app, |inst| inst.forget_mods(names)) +} + +/// [`Instance::rename_mod`] for a folder the window has just renamed, with the +/// same following of `modlist_seen` as [`forget_removed_mods`]. +pub(crate) fn rename_mod_lines(app: &App, old: &str, new: &str) -> Option> { + own_modlist_edit(app, |inst| inst.rename_mod(old, new)) +} + +fn own_modlist_edit( + app: &App, + edit: impl FnOnce(&Instance) -> std::io::Result<()>, +) -> Option> { + let inst = app.created.as_ref()?; + let current = modlist_unchanged(app, inst); + let result = edit(inst); + if current { + app.modlist_seen.set(Some(modlist_fingerprint(inst))); + } + Some(result) } /// Invalidate every memoised view listing. Cheap: the listings rebuild lazily on @@ -2624,16 +2709,26 @@ pub(crate) fn put_mod_selection(app: &mut App, held: HeldSelection) { } /// Persist the mod list and invalidate everything derived from it (plugin order, -/// conflict emblems, the per-mod metadata cache). -pub(crate) fn mods_changed(app: &mut App) { - if let Some(err) = save_mods(app) { - app.status = Some(err); - // The write was refused (another process owns the instance): the - // in-memory edit will never reach disk, and leaving it displayed shows - // the user a state that silently evaporates when they close the window. - // Disk is the truth; resync the view to it. +/// conflict emblems, the per-mod metadata cache). Returns whether the save +/// landed: a caller that reports success sets its status only then, or it +/// replaces the refusal this put there with news of an edit that was reverted. +pub(crate) fn mods_changed(app: &mut App) -> bool { + let refused = save_mods(app); + if let Some(err) = &refused { + app.status = Some(err.clone()); + // The write was refused (another process owns the instance, or rewrote + // the list since it was read): the in-memory edit will never reach disk, + // and leaving it displayed shows the user a state that silently + // evaporates when they close the window. Disk is the truth; resync the + // view to it. reload_mods(app); } + mod_views_changed(app); + refused.is_none() +} + +/// Everything derived from the mod list, recomputed after it changed. +fn mod_views_changed(app: &mut App) { // The merged view depends on which mods are enabled and in what order, not // just on their contents. bump_views(app); diff --git a/crates/eidos-gui/src/update.rs b/crates/eidos-gui/src/update.rs index dbf613cc..0bb93560 100644 --- a/crates/eidos-gui/src/update.rs +++ b/crates/eidos-gui/src/update.rs @@ -454,6 +454,7 @@ fn collection_on_worker( collection_domain: c.info.domain_name.clone(), owner: format!("{}:{}:{}", state.game_domain, state.slug, state.revision), renamed: Vec::new(), + kept: Vec::new(), }; // The bar counts members reached, which is the only honest measure here: a // collection that is half skipped still moves, and 7-Zip's own percentage @@ -469,14 +470,7 @@ fn collection_on_worker( |s: &eidos_collections::state::InstallState| s.save(&state_path).map_err(|e| e.to_string()); let mut report = eidos_collections::install::run(c, &mut state, &mut counting, &mut save); report.unknown_sections = read.unknown_sections.clone(); - for (member, folder) in std::mem::take(&mut hooks.renamed) { - report.renamed.push(eidos_collections::report::Note { - subject: member, - detail: format!( - "installed as \"{folder}\", because a mod of yours already had that name" - ), - }); - } + hooks.drain_notes(&mut report); if report.aborted { return Err(report.render()); } @@ -517,6 +511,14 @@ impl eidos_collections::install::Hooks for CountingHooks<'_> { fn allow_runtime_mismatch(&self) -> bool { self.inner.allow_runtime_mismatch() } + fn adopt( + &mut self, + c: &eidos_collections::Collection, + state: &mut eidos_collections::state::InstallState, + save: &mut dyn FnMut(&eidos_collections::state::InstallState) -> Result<(), String>, + ) -> Result, String> { + self.inner.adopt(c, state, save) + } fn obtain( &mut self, @@ -1932,9 +1934,10 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { // The card hosting the editor is dismissed by the commit, not by the // click that armed it. app.menu_mod = None; - mods_changed(app); // The toast quotes the same 1-based numbering the column shows. - app.status = Some(format!("Moved to priority {}.", at + 1)); + if mods_changed(app) { + app.status = Some(format!("Moved to priority {}.", at + 1)); + } } Message::SendToSeparatorStart(i) => { // Same as above: the chooser lives inside the menu card. @@ -2076,7 +2079,8 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { let Some(name) = app.overwrite_to_mod.take().map(|s| s.trim().to_string()) else { return Task::none(); }; - let Some(inst) = app.created.as_ref() else { + // Owned: the staleness check below needs `app` mutably. + let Some(inst) = app.created.clone() else { return Task::none(); }; let existing = inst.mods_dir().join(&name).exists(); @@ -2090,6 +2094,12 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { return Task::none(); } }; + // Checked before the move, as Rename does: a save refused after it + // reloads a list that does not name the new mod, which comes back + // DISABLED - generated output the game then silently stops loading. + if refuse_stale_modlist(app) { + return Task::none(); + } match inst.overwrite_into_mod(&name) { Ok(dest) => { // Highest priority (the end of the display order), which is where @@ -2103,12 +2113,14 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { }); } drop_files_cache(app, None); - mods_changed(app); + // Before mods_changed, which replaces it with the reason if + // the save is refused. app.status = Some(if existing { format!("Moved the Overwrite into '{name}'.") } else { format!("Created mod '{name}' from the Overwrite.") }); + mods_changed(app); } Err(e) => { app.status = Some(format!("Could not create the mod: {e}")); @@ -2458,6 +2470,13 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { return Task::none(); } }; + // And known current before the deletion, as Rename does: the + // save after it would refuse a list another process rewrote, + // and its "redo the change" would then cover a delete that + // already happened. + if refuse_stale_modlist(app) { + return Task::none(); + } if let Some(m) = app.mods.get(i).cloned() { match fs::remove_dir_all(&m.path) { Ok(()) => { @@ -2466,8 +2485,19 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { app.selected_mods.clear(); app.drag_state = None; drop_files_cache(app, Some(&m.name)); + // Forgotten in every profile, not only this one: a line + // left in another profile counts as a missing mod there, + // and enough of them wedge it (see ConfirmBatchRemove). + let forgot = + forget_removed_mods(app, std::slice::from_ref(&m.name)); + app.status = Some(match forgot { + Some(Err(e)) => format!( + "Removed '{}'. The mod list could not be updated: {e}.", + m.name + ), + _ => format!("Removed '{}'.", m.name), + }); mods_changed(app); - app.status = Some(format!("Removed '{}'.", m.name)); } Err(e) => app.status = Some(format!("Remove failed: {e}")), } @@ -2526,7 +2556,11 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { // folder moves a live union layer out from under a // running session, and the refused save afterwards // would leave modlist.txt naming a folder that no - // longer exists. + // longer exists. For the same reason the list must be + // known current BEFORE the rename: the lock is + // re-entrant, so the save can then only refuse a + // list another process rewrote, and that has to be + // caught while the folder still has its old name. let Some(inst) = app.created.as_ref() else { return Task::none(); }; @@ -2537,6 +2571,9 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { return Task::none(); } }; + if refuse_stale_modlist(app) { + return Task::none(); + } match fs::rename(&old.path, &dest) { Ok(()) => { if let Some(m) = app.mods.get_mut(i) { @@ -2558,8 +2595,20 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { app.collapsed.insert(typed.clone()); save_collapsed(app); } + // Renamed in every profile, not only this one: + // the pool is shared, and a profile still + // naming the old folder loses the mod's state + // and order (see Profile::rename_mod). + let renamed = rename_mod_lines(app, &old.name, &new_name); + // Before mods_changed, which replaces it with + // the reason if the save is refused. + app.status = Some(match renamed { + Some(Err(e)) => format!( + "Renamed to '{typed}'. The other profiles could not be updated: {e}." + ), + _ => format!("Renamed to '{typed}'."), + }); mods_changed(app); - app.status = Some(format!("Renamed to '{typed}'.")); } Err(e) => app.status = Some(format!("Rename failed: {e}")), } @@ -2570,6 +2619,23 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { } Message::AddSeparator(i) => { app.menu_mod = None; + // Under the lock and before the folder exists, as Rename does: a + // refused save after it reloads the list, and the new folder comes + // back unlisted at the end. Without the lock another process could + // still write between the check and the save. + let Some(inst) = app.created.as_ref() else { + return Task::none(); + }; + let _lock = match inst.try_lock("the Eidos window") { + Ok(l) => l, + Err(e) => { + app.status = Some(format!("Cannot add a separator now: {e}.")); + return Task::none(); + } + }; + if refuse_stale_modlist(app) { + return Task::none(); + } let mods_dir = app.created.as_ref().map(|inst| inst.mods_dir()); if let Some(mods_dir) = mods_dir { // A unique "Separator N" display name -> folder "_separator". @@ -2597,11 +2663,15 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { ); // Indices at/after the insertion point shifted. app.selected_mods.clear(); - mods_changed(app); - app.selected_mod = Some(idx); - // Open its rename editor so the user names it straight away. - app.rename = Some((idx, display)); - app.menu_mod = Some(idx); + // Only once the save landed: a refusal (a trust check, + // say) reloads the list, and the editor would then sit + // on whichever unrelated mod took this index. + if mods_changed(app) { + app.selected_mod = Some(idx); + // Open its rename editor so the user names it straight away. + app.rename = Some((idx, display)); + app.menu_mod = Some(idx); + } } Err(e) => app.status = Some(format!("Could not create separator: {e}")), } @@ -3431,8 +3501,12 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { app.status = Some(format!("{} has no INI files Eidos manages.", game.def.name)); return Task::none(); }; - let prof = inst.active(); - app.ini_editor = Some(load_ini_editor(&prof, files, first)); + let inst = inst.clone(); + if let Err(e) = seed_ini_from_game(app, &first) { + app.status = Some(e); + return Task::none(); + } + app.ini_editor = Some(load_ini_editor(&inst.active(), files, first)); } Message::CloseIniEditor => app.ini_editor = None, Message::IniEditorPick(name) => { @@ -3455,6 +3529,10 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { return Task::none(); } let files = ed.files.clone(); + if let Err(e) = seed_ini_from_game(app, &name) { + app.status = Some(e); + return Task::none(); + } app.ini_editor = Some(load_ini_editor(&inst.active(), files, name)); } Message::IniEditorAction(action) => { @@ -3485,6 +3563,27 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { )); return Task::none(); } + // Under the lock, and only over the file the buffer was read from. + // The buffer may be minutes old: since then `eidos sort` or + // `eidos collection` may have rewritten Morrowind.ini's + // [Game Files], or a session written its INIs back into the + // profile, and writing the buffer would revert that silently (the + // re-read below would then even record the revert as current). + let _lock = match inst.try_lock("the Eidos window") { + Ok(l) => l, + Err(e) => { + app.status = Some(format!("Not saved: {e}.")); + return Task::none(); + } + }; + if content_hash(&std::fs::read(&path).ok()) != ed.on_disk { + app.status = Some(format!( + "Not saved: {} changed on disk since the editor opened it. \ + Copy your edits, then close the editor and open it again.", + ed.current + )); + return Task::none(); + } // No trailing-newline surgery. iced 0.14's `Content::text()` // round-trips exactly, so the guard that used to sit here did not // prevent growth - it DELETED newlines the user had typed at the end @@ -3495,6 +3594,7 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { ed.original = ed.content.text(); ed.dirty = false; ed.missing = false; + ed.on_disk = content_hash(&std::fs::read(&path).ok()); app.archive_epoch .set(app.archive_epoch.get().wrapping_add(1)); app.status = Some(format!( @@ -3502,6 +3602,14 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { ed.current, inst.active().name )); + // Morrowind.ini (and plugins.txt) live in the profile's plugin + // state: the cached load order and the hash its next write is + // checked against are both stale now, and left so that write + // was refused as another process's change. Re-read both, which + // also picks up a `[Game Files]` line typed here. + if path.starts_with(inst.active().plugins_state_dir()) { + invalidate_plugins(app); + } } Err(e) => app.status = Some(format!("Could not save {}: {e}", ed.current)), } @@ -4870,6 +4978,21 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { Message::CreateEmptyMod => return update(app, Message::CreateEmptyModAt(app.mods.len())), Message::CreateEmptyModAt(at) => { app.menu_mod = None; + // Same as AddSeparator: checked under the lock, before the folder + // is created. + let Some(inst) = app.created.as_ref() else { + return Task::none(); + }; + let _lock = match inst.try_lock("the Eidos window") { + Ok(l) => l, + Err(e) => { + app.status = Some(format!("Cannot create a mod now: {e}.")); + return Task::none(); + } + }; + if refuse_stale_modlist(app) { + return Task::none(); + } if let Some(inst) = &app.created { // A unique "New Mod N" name, never colliding on disk. let mut n = 1usize; @@ -4897,12 +5020,14 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { save_collapsed(app); } } - mods_changed(app); - app.selected_mod = Some(idx); - app.selected_mods.clear(); - // Open its rename editor so the user names it straight away. - app.rename = Some((idx, name)); - app.menu_mod = Some(idx); + // Only once the save landed, as in AddSeparator. + if mods_changed(app) { + app.selected_mod = Some(idx); + app.selected_mods.clear(); + // Open its rename editor so the user names it straight away. + app.rename = Some((idx, name)); + app.menu_mod = Some(idx); + } } Err(e) => app.status = Some(format!("Could not create mod: {e}")), } @@ -5815,7 +5940,7 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { .collect(); let mut enabled = 0usize; for m in app.mods.iter_mut() { - if !m.enabled && wanted.contains(&m.name) { + if !m.enabled && !m.is_backup() && wanted.contains(&m.name) { m.enabled = true; enabled += 1; } @@ -5825,7 +5950,7 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { Some("Those mods are already enabled - the plugins still need turning on in the Plugins tab.".to_string()); return Task::none(); } - mods_changed(app); + let saved = mods_changed(app); // The plugin list changed shape, so the save's diff has to be redone // against it rather than left showing the old answer. But // mods_changed just INVALIDATED that list, and on the Saves tab @@ -5838,6 +5963,10 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { app.plugins = compute_plugins(app); } load_save_details(app); + // A refused save left its reason in the status; keep it there. + if !saved { + return Task::none(); + } let left = app.save_missing.len(); app.status = Some(if left == 0 { format!("Enabled {enabled} mod(s); this save's plugins are all available now.") @@ -6152,8 +6281,12 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { )); return Task::none(); } - mods_changed(app); + let saved = mods_changed(app); app.view_menu_open = false; + // A refused save left its reason in the status; keep it there. + if !saved { + return Task::none(); + } // A collapsed group hides rows exactly as a filter does, and the // status has to admit either. let total = app @@ -6176,7 +6309,11 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { // MO2-style batch enable/disable: if any selected real mod is enabled, // the whole selection is disabled; otherwise the whole selection is // enabled. Separators carry no toggle and are skipped. - let targets: Vec = real_selection(app); + // A backup is inert: ticking one would write `+X_backup` (see ToggleMod). + let targets: Vec = real_selection(app) + .into_iter() + .filter(|&i| app.mods.get(i).is_some_and(|m| !m.is_backup())) + .collect(); if targets.is_empty() { app.status = Some("Select one or more mods first.".to_string()); return Task::none(); @@ -6190,13 +6327,14 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { m.enabled = new_state; } } - mods_changed(app); app.menu_mod = None; - app.status = Some(format!( - "{} {} mod(s).", - if new_state { "Enabled" } else { "Disabled" }, - targets.len() - )); + if mods_changed(app) { + app.status = Some(format!( + "{} {} mod(s).", + if new_state { "Enabled" } else { "Disabled" }, + targets.len() + )); + } } Message::BatchRemoveMods => { let n = real_selection(app).len(); @@ -6225,10 +6363,14 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { return Task::none(); } }; + // Known current before the first deletion too, as in ModRemove. + if refuse_stale_modlist(app) { + return Task::none(); + } // Delete from the highest index down so the lower indices stay valid. let mut targets = real_selection(app); targets.sort_unstable(); - let mut removed = 0usize; + let mut gone: Vec = Vec::new(); let mut failed = 0usize; for &i in targets.iter().rev() { if let Some(m) = app.mods.get(i).cloned() { @@ -6236,7 +6378,7 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { Ok(()) => { app.mods.remove(i); drop_files_cache(app, Some(&m.name)); - removed += 1; + gone.push(m.name); } Err(_) => failed += 1, } @@ -6244,12 +6386,25 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { } app.selected_mods.clear(); app.selected_mod = None; - mods_changed(app); - app.status = Some(if failed == 0 { + let removed = gone.len(); + let mut status = if failed == 0 { format!("Removed {removed} mod(s).") } else { format!("Removed {removed} mod(s); {failed} could not be deleted.") - }); + }; + // Before the save: every profile's modlist.txt still lists the deleted + // folders, and a big enough batch of listed-but-missing mods is exactly + // what an unmounted drive looks like - the save, and every one after + // it, would be refused. Only the folders really deleted are forgotten: + // one that failed keeps its line, its slot and its enabled state. + let forgot = forget_removed_mods(app, &gone); + if let Some(Err(e)) = forgot { + status = format!("{status} The mod list could not be updated: {e}."); + } + // Set BEFORE mods_changed, which replaces it with the reason if the + // save is refused; set after, it hid that refusal behind "Removed". + app.status = Some(status); + mods_changed(app); } Message::BatchSendTop => { // Lift the whole selection (keeping its relative order) to the top. @@ -8312,6 +8467,9 @@ fn load_ini_editor( current: String, ) -> IniEditorState { let path = prof.ini_path(¤t); + // Hashed BEFORE the text is read: a write in between then refuses the + // save, never lets the older buffer over it. + let on_disk = content_hash(&std::fs::read(&path).ok()); // `read_text_lossy` returns None on ANY read failure, and a file that exists // but could not be read is not an empty one. Collapsing the two meant an // EACCES or an I/O error opened a blank editor whose always-enabled Save @@ -8327,11 +8485,42 @@ fn load_ini_editor( dirty: false, missing: !exists, unreadable, + on_disk, files, current, } } +/// Seed a file kept in the profile's plugin state (Morrowind.ini) from the +/// game's real state before the editor opens it, as `write_plugin_state` does. +/// Opened unseeded it showed an empty buffer, and saving that founded the +/// profile's Morrowind.ini on a stub that every later launch deployed in place +/// of the real INI, since the seed never replaces a file the profile owns. +/// Fails closed with the reason. +fn seed_ini_from_game(app: &mut App, file: &str) -> Result<(), String> { + let (Some(inst), Some(game)) = (app.created.as_ref(), selected_game(app)) else { + return Ok(()); + }; + let prof = inst.active(); + let path = prof.ini_path(file); + if path.is_file() || !path.starts_with(prof.plugins_state_dir()) { + return Ok(()); + } + let (Some(dir), Some(spec)) = (game.plugin_state_dir(), game.plugin_spec()) else { + return Ok(()); + }; + let seeded = inst + .try_lock("the Eidos window") + .and_then(|_lock| prof.seed_plugin_state(&dir, &spec)) + .map_err(|e| format!("Cannot open {file} now: {e}."))?; + if seeded > 0 { + // The profile's plugin state changed under the cached load order and + // the hash its next write is checked against. + invalidate_plugins(app); + } + Ok(()) +} + /// How much of a session log the pane reads. A launch log runs to megabytes and /// the interesting part is always the end, so only the tail is taken. const LOG_TAIL_BYTES: u64 = 512 * 1024; diff --git a/crates/eidos-gui/src/view.rs b/crates/eidos-gui/src/view.rs index 7aedea41..321c7e20 100644 --- a/crates/eidos-gui/src/view.rs +++ b/crates/eidos-gui/src/view.rs @@ -187,30 +187,37 @@ pub(crate) fn clear_dir_contents(dir: &Path) -> std::io::Result<()> { /// [`merged_listing`] memoised per directory against the view generation - it /// read every enabled mod's directory on each redraw of the Data tab. -pub(crate) fn cached_merged_listing(app: &App, dir: &str) -> Vec { +/// +/// An `Rc` for the same reason as [`cached_entries`]: a filter walks every +/// directory of the merged tree on every redraw, and a hit that deep-cloned +/// each listing copied ~100k rows per pointer move on a 347-mod instance. +pub(crate) fn cached_merged_listing(app: &App, dir: &str) -> std::rc::Rc> { let gen = app.view_generation.get(); if let Some((at, entries)) = app.data_listing.borrow().get(dir) { if *at == gen { - return entries.clone(); + return entries.clone(); // Rc bump, not a Vec copy } } - let entries = merged_listing(app, dir); + let entries = std::rc::Rc::new(merged_listing(app, dir)); app.data_listing .borrow_mut() .insert(dir.to_string(), (gen, entries.clone())); entries } -/// The union the Data tab reads, built once per view generation. +/// The union the Data tab reads, built once per view generation, with the label +/// of every layer root (see [`DataSources`]). /// /// The same `LayerStack` `eidos-launch` mounts: same layers, same order, same /// overwrite. Building it walks every enabled mod once, which is why it is /// cached against the view generation rather than rebuilt per directory. -pub(crate) fn data_stack(app: &App) -> Option> { +pub(crate) fn data_stack( + app: &App, +) -> Option<(std::rc::Rc, std::rc::Rc)> { let gen = app.view_generation.get(); - if let Some((at, stack)) = app.data_stack.borrow().as_ref() { + if let Some((at, stack, sources)) = app.data_stack.borrow().as_ref() { if *at == gen { - return Some(stack.clone()); + return Some((stack.clone(), sources.clone())); } } let inst = app.created.as_ref()?; @@ -220,8 +227,44 @@ pub(crate) fn data_stack(app: &App) -> Option; + +/// The label of the layer providing `real`: its LONGEST root prefix, so a mod +/// nested under another root is not attributed to the outer one. Walking +/// `ancestors()` from the deepest makes the first hit the longest match, and +/// `Path` hashes and compares by component exactly as `starts_with` matches. +pub(crate) fn data_source(sources: &DataSources, real: &Path) -> String { + real.ancestors() + .find_map(|a| sources.get(a)) + .cloned() + .unwrap_or_default() } /// The entries of ONE directory of the merged view (`dir` relative to `Data`, @@ -237,43 +280,16 @@ pub(crate) fn data_stack(app: &App) -> Option Vec { - let Some(stack) = data_stack(app) else { + let Some((stack, sources)) = data_stack(app) else { return Vec::new(); }; - // Where each real path came from, so a winner can be named. Built once per - // call rather than per row: a deep tree asks this thousands of times. - let mut sources: Vec<(PathBuf, String)> = Vec::new(); - if let Some(inst) = app.created.as_ref() { - sources.push((inst.overwrite_dir(), "[Overwrite]".to_string())); - } - // Unmanaged rows are EXCLUDED. Their `path` is a single plugin file inside - // the game's own Data directory, and a longest-prefix match against that - // would attribute every vanilla file to whichever DLC row sorted first - and - // put a Hide button on the pristine game install. - for m in app - .mods - .iter() - .filter(|m| m.is_active() && !m.is_unmanaged()) - { - sources.push((m.path.clone(), m.name.clone())); - } - if let Some(g) = selected_game(app) { - sources.push((g.data_path.clone(), format!("[{}]", g.def.id))); - } let conflicts = app.conflicts.as_ref(); let mut out: Vec = stack .list_dir_typed(dir) .into_iter() .map(|(name, real, ftype)| { - // Longest match wins: a mod nested under another root would - // otherwise be attributed to whichever prefix came first. - let source = sources - .iter() - .filter(|(root, _)| real.starts_with(root)) - .max_by_key(|(root, _)| root.as_os_str().len()) - .map(|(_, label)| label.clone()) - .unwrap_or_default(); + let source = data_source(&sources, &real); let md = fs::symlink_metadata(&real).ok(); let is_dir = ftype.map(|t| t.is_dir()).unwrap_or_else(|| real.is_dir()); // The conflict map is keyed by lowercased relative path and is @@ -435,7 +451,7 @@ pub(crate) fn data_tree_rows(app: &App, limit: usize) -> Vec { // dropped if nothing under it survived. let query = app.data_query.trim().to_lowercase(); let filtering = !query.is_empty() || app.data_conflicts_only; - for row in cached_merged_listing(app, dir) { + for row in cached_merged_listing(app, dir).iter() { // Checked against the KEPT count, and re-checked after each subtree. // Filtering removes rows again, so `out.len()` alone stopped being a // bound the moment a filter was typed: the walk then stat'd the @@ -443,21 +459,26 @@ pub(crate) fn data_tree_rows(app: &App, limit: usize) -> Vec { if out.len() >= limit { return; } + let keeps = !filtering + || (row.name.to_lowercase().contains(&query) + && (!app.data_conflicts_only || row.conflicted)); + // A file that does not match has nothing under it to earn it a row, + // so it is passed over before its path is built or it is cloned: a + // selective filter crosses nearly the whole tree on every redraw. + if !keeps && !row.is_dir { + continue; + } let rel = if dir.is_empty() { row.name.clone() } else { format!("{dir}/{}", row.name) }; let expanded = row.is_dir && (app.data_expanded.contains(&rel) || filtering); - let keeps = !filtering - || (row.name.to_lowercase().contains(&query) - && (!app.data_conflicts_only || row.conflicted)); let at = out.len(); - let is_dir = row.is_dir; out.push(TreeRow { depth, rel: rel.clone(), - row, + row: row.clone(), }); if expanded { walk(app, &rel, depth + 1, limit, out); @@ -468,7 +489,7 @@ pub(crate) fn data_tree_rows(app: &App, limit: usize) -> Vec { // and shorten the list below the budget, suppressing the "showing // the first N" notice that explains why. let budget_spent = out.len() >= limit; - if filtering && !keeps && !budget_spent && (!is_dir || out.len() == at + 1) { + if !keeps && !budget_spent && out.len() == at + 1 { out.remove(at); } } diff --git a/crates/eidos-ini/src/lib.rs b/crates/eidos-ini/src/lib.rs index 0e57b281..8b89814d 100644 --- a/crates/eidos-ini/src/lib.rs +++ b/crates/eidos-ini/src/lib.rs @@ -20,8 +20,15 @@ pub fn newline_style(text: &str) -> &'static str { /// If `line` is a section header `[name]`, returns the trimmed `name` (no /// brackets). INI section names are matched case-insensitively by callers. +/// +/// A leading UTF-8 BOM (U+FEFF) is skipped: `str::trim` keeps it (it is not +/// White_Space), and the INI readers decode the file as-is, so a file saved as +/// "UTF-8 with BOM" hid its FIRST section. set_key then appended a duplicate +/// `[General]` at the end, which Wine never reads (its profile parser consumes +/// the BOM and stops at the first matching section). Only the match skips it: +/// set_key and delete_key copy lines verbatim, so the BOM survives a rewrite. pub fn section_header(line: &str) -> Option<&str> { - let t = line.trim(); + let t = line.trim_start_matches('\u{feff}').trim(); t.strip_prefix('[') .and_then(|s| s.strip_suffix(']')) .map(str::trim) @@ -253,6 +260,20 @@ mod tests { ); } + #[test] + fn a_bom_does_not_hide_the_first_section() { + // Notepad's "UTF-8 with BOM": the BOM sits right before the first + // header. Without skipping it, get_key saw no [General] and set_key + // appended a second one the engine never reads. + let text = "\u{feff}[General]\r\nbUseMyGamesDirectory=1\r\n[Display]\r\nx=1\r\n"; + assert_eq!(get_key(text, "General", "bUseMyGamesDirectory"), Some("1")); + let out = set_key(text, "General", "bUseMyGamesDirectory", "0"); + assert_eq!( + out, + "\u{feff}[General]\r\nbUseMyGamesDirectory=0\r\n[Display]\r\nx=1\r\n" + ); + } + #[test] fn set_key_creates_updates_and_preserves() { // Create in an empty document. diff --git a/crates/eidos-instance/src/lib.rs b/crates/eidos-instance/src/lib.rs index 686fceb4..b812d616 100644 --- a/crates/eidos-instance/src/lib.rs +++ b/crates/eidos-instance/src/lib.rs @@ -1058,6 +1058,35 @@ impl Instance { self.active().save_modlist(mods) } + /// Forget mods the caller has just deleted from `mods/`, in EVERY profile - + /// see [`Profile::forget_mods`]. The pool is shared, so a line left behind in + /// a profile that is not active wedges that profile the day it is switched to. + /// Every profile is tried; the last failure, if any, is returned. + pub fn forget_mods(&self, names: &[String]) -> std::io::Result<()> { + let _lock = self.try_lock("forgetting removed mods")?; + let mut result = Ok(()); + for name in self.profiles() { + if let Err(e) = self.profile(&name).forget_mods(names) { + result = Err(e); + } + } + result + } + + /// Follow a folder the caller has just renamed in `mods/`, in EVERY profile - + /// see [`Profile::rename_mod`]. Same lock and same error rule as + /// [`Instance::forget_mods`]. + pub fn rename_mod(&self, old: &str, new: &str) -> std::io::Result<()> { + let _lock = self.try_lock("renaming a mod")?; + let mut result = Ok(()); + for name in self.profiles() { + if let Err(e) = self.profile(&name).rename_mod(old, new) { + result = Err(e); + } + } + result + } + /// Enabled mods of the active profile, highest priority first. pub fn load_order(&self) -> Vec { self.active().load_order() @@ -2804,6 +2833,57 @@ mod whole_audit_regressions { assert!(result.is_err()); } + #[test] + fn removing_a_big_batch_of_mods_does_not_wedge_any_profile() { + // 11 of 20 deleted in one go is past the "looks like an unmounted drive" + // threshold; the line each one left behind used to refuse every save. + let fixture = Fixture::new(); + let i = &fixture.0; + let mods: Vec = (0..20) + .map(|n| i.create_empty_mod(&format!("Mod{n:02}")).unwrap()) + .collect(); + i.save_modlist(&mods).unwrap(); + i.profile("Other").create_from(&i.active()).unwrap(); + let gone: Vec = mods[..11].iter().map(|m| m.name.clone()).collect(); + for m in &mods[..11] { + fs::remove_dir_all(&m.path).unwrap(); + } + + i.forget_mods(&gone).unwrap(); + + let (list, trust) = i.modlist_checked(); + assert!(trust.is_good(), "{trust:?}"); + let names = |l: &[ModEntry]| l.iter().map(|m| (m.name.clone(), m.enabled)).collect::>(); + assert_eq!(names(&list), names(&mods[11..])); + i.save_modlist(&list).unwrap(); + assert!(i.profile("Other").modlist_checked().1.is_good()); + } + + #[test] + fn renaming_mods_keeps_every_profile_s_order_and_state() { + // 11 of 20 renamed: each old name used to stay behind in the other + // profile as a LOST mod, past the "unmounted drive" threshold. + let fixture = Fixture::new(); + let i = &fixture.0; + let mut mods: Vec = (0..20) + .map(|n| i.create_empty_mod(&format!("Mod{n:02}")).unwrap()) + .collect(); + mods[3].enabled = false; + i.save_modlist(&mods).unwrap(); + i.profile("Other").create_from(&i.active()).unwrap(); + for m in &mut mods[..11] { + let new = format!("Short{}", &m.name[3..]); + fs::rename(&m.path, i.mods_dir().join(&new)).unwrap(); + i.rename_mod(&m.name, &new).unwrap(); + m.name = new; + } + + let (list, trust) = i.profile("Other").modlist_checked(); + assert!(trust.is_good(), "{trust:?}"); + let names = |l: &[ModEntry]| l.iter().map(|m| (m.name.clone(), m.enabled)).collect::>(); + assert_eq!(names(&list), names(&mods)); + } + #[test] fn mo2_import_refuses_a_busy_instance_before_writing() { let fixture = Fixture::new(); diff --git a/crates/eidos-instance/src/meta.rs b/crates/eidos-instance/src/meta.rs index a9ea382f..bfb0fa2c 100644 --- a/crates/eidos-instance/src/meta.rs +++ b/crates/eidos-instance/src/meta.rs @@ -270,6 +270,21 @@ impl ModMeta { self.raw("eidosCollectionOwner") } + /// The INI fragments a collection itself installed into this folder, so a + /// later revision removes only those and never one the user added. `None` + /// for a folder written before this record existed: provenance unknown. + pub fn collection_ini_fragments(&self) -> Option> { + serde_json::from_str(self.raw("eidosCollectionIniFragments")?).ok() + } + + pub fn set_collection_ini_fragments(&mut self, names: &[String]) { + // JSON keeps arbitrary file names on one INI line, like eidosInstallWarning. + self.set( + "eidosCollectionIniFragments", + &serde_json::to_string(names).expect("strings serialize"), + ); + } + /// A `[General]` string value, unquoted and with empty treated as absent. fn string(&self, key: &str) -> Option { self.raw(key).map(unquote).filter(|s| !s.is_empty()) diff --git a/crates/eidos-instance/src/profile/modlist.rs b/crates/eidos-instance/src/profile/modlist.rs index be9bb1de..bd303383 100644 --- a/crates/eidos-instance/src/profile/modlist.rs +++ b/crates/eidos-instance/src/profile/modlist.rs @@ -121,6 +121,14 @@ impl Profile { own } + /// The raw bytes of the file [`Profile::modlist`] reads (`None` when there is + /// none). A caller holding a list it read earlier compares these to tell + /// whether another process has rewritten the file since: the instance lock + /// serialises the writes, not a read made minutes before the write. + pub fn modlist_bytes(&self) -> Option> { + fs::read(self.modlist_source()).ok() + } + pub fn create(&self) -> io::Result<()> { fs::create_dir_all(self.dir()) } @@ -270,9 +278,14 @@ impl Profile { // highest priority and leaves it DISABLED - it has no idea where in the // conflict order it belongs, and enabling it silently could overwrite half // the load order's files on the next launch. + // `out` is in file order, highest priority FIRST, so "highest priority" is + // the front: pushing these after the listed rows put them at the very + // bottom once enabled, under every other mod (and a mod captured from + // Overwrite, which only enables this entry in place, lost every conflict). + let mut fresh = Vec::new(); for name in present { if seen.insert(name.clone()) { - out.push(ModEntry { + fresh.push(ModEntry { path: mods_dir.join(&name), name, enabled: false, @@ -280,6 +293,7 @@ impl Profile { }); } } + out.splice(0..0, fresh); let trust = if list_lost { ListTrust::Suspect( "modlist.txt exists but could not be read (truncated, or permissions) - \ @@ -343,13 +357,108 @@ impl Profile { if target.exists() { let _ = fs::copy(&target, target.with_extension("txt.bak")); } + if s.is_empty() { + // No rows is "no order yet", which only an ABSENT file says: an empty + // one reads as a truncated list as soon as a folder exists (see + // `modlist_checked`), so the next install or save is refused. The + // window saves right after removing the last mods, and writing "" here + // put back the file `forget_mods` had just deleted. Once this profile's + // own file is gone, `modlist_source` may name the legacy flat file it + // shadowed; that one goes too, or it would come back as the list. + let remove = |path: PathBuf| match fs::remove_file(path) { + Err(e) if e.kind() != io::ErrorKind::NotFound => Err(e), + _ => Ok(()), + }; + remove(target)?; + return remove(self.modlist_source()); + } // Through the shared writer, whose temp name is unique per process: the - // window and `eidos install` both write this file and neither serialises - // against the other (flock is advisory and the CLI does not take it), so - // a fixed temp name let two of them splice the curated order. + // window, `eidos install` and `eidos collection` all write this file. + // Every one of them takes the instance lock first, so the writes are + // serialised; the unique temp name is defence in depth against a fixed + // name letting two writers splice the curated order. What the lock does + // NOT cover is a caller whose `mods` was read before another process + // wrote: this rewrites the whole file from that list, so such a caller + // must compare `modlist_bytes` with what it read (the window does). crate::write_atomic(&target, s.as_bytes()) } + /// Drop the lines of mods whose folders the caller has just deleted, leaving + /// every other line - order, enabled state, `*` rows, comments - as it was. + /// + /// Deleting a folder without this leaves its line behind, and the trust check + /// counts a listed mod with no folder as LOST: remove a dozen at once and the + /// next save looks exactly like an unmounted drive, so it is refused, and so is + /// every save after it, because nothing can rewrite the file any more. This + /// edit cannot be fooled by that drive - it reads no scan and drops only the + /// names it is handed - so it needs no trust check of its own. + pub fn forget_mods(&self, names: &[String]) -> io::Result<()> { + let src = self.modlist_source(); + let text = match fs::read_to_string(&src) { + Ok(text) => text, + Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(e) => return Err(e), + }; + let mut kept = String::new(); + let mut dropped = false; + for line in text.lines() { + let t = line.trim(); + // Same reading as `modlist_checked`: `+`, `-` or a bare name is a mod. + let name = t.strip_prefix(['+', '-']).unwrap_or(t).trim(); + if !t.starts_with(['*', '#']) && names.iter().any(|n| n == name) { + dropped = true; + continue; + } + kept.push_str(line); + kept.push('\n'); + } + if !dropped { + return Ok(()); + } + if kept.trim().is_empty() { + // An empty file next to surviving folders reads as a TRUNCATED list + // (see `modlist_checked`); an absent one is the honest "no order yet". + return fs::remove_file(&src); + } + fs::create_dir_all(self.dir())?; + crate::write_atomic(&self.modlist_path(), kept.as_bytes()) + } + + /// [`Profile::forget_mods`]'s twin for a folder the caller has just renamed: + /// the line keeps its `+`/`-` prefix and its place, only the name changes. + /// Left alone, the old name counts as a LOST mod (enough of them and the + /// list is judged an unmounted drive), and the new folder, listed nowhere, + /// comes back DISABLED at the top priority. MO2 does the same edit + /// (Profile::renameModInAllProfiles). + pub fn rename_mod(&self, old: &str, new: &str) -> io::Result<()> { + let src = self.modlist_source(); + let text = match fs::read_to_string(&src) { + Ok(text) => text, + Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(e) => return Err(e), + }; + let mut out = String::new(); + let mut renamed = false; + for line in text.lines() { + let t = line.trim(); + let name = t.strip_prefix(['+', '-']).unwrap_or(t).trim(); + if !t.starts_with(['*', '#']) && name == old { + // `name` is a suffix of `t`, so what precedes it is the prefix. + out.push_str(&t[..t.len() - name.len()]); + out.push_str(new); + renamed = true; + } else { + out.push_str(line); + } + out.push('\n'); + } + if !renamed { + return Ok(()); + } + fs::create_dir_all(self.dir())?; + crate::write_atomic(&self.modlist_path(), out.as_bytes()) + } + /// Register new content without changing a previously listed mod's priority or activation. pub(crate) fn register_installed_mod(&self, name: &str) -> io::Result<()> { if !crate::tools::is_mod_folder_name(name) || !self.mods_dir().join(name).is_dir() { @@ -387,13 +496,15 @@ impl Profile { /// Why writing `modlist.txt` right now would destroy the curated order rather /// than record an edit, or `None` when it is safe. /// - /// The check is deliberately absolute rather than proportional, because the - /// disaster is absolute: the mod pool is unreachable, so the in-memory list is - /// missing EVERYTHING and any save flattens the order to nothing. A user who - /// really did delete every mod hits this too and has to say so by removing - /// `modlist.txt` themselves - an annoyance, weighed against permanently losing - /// the one thing on disk that cannot be re-derived: which of forty overlapping - /// mods wins each file conflict, and which are installed but deliberately off. + /// The check is [`ListTrust::judge`]: everything missing, or a large share of + /// the list missing at once. The disaster it stops is the unreachable mod pool, + /// where the in-memory list is missing EVERYTHING and any save flattens the + /// order to nothing. A user who really did delete that many mods by hand hits + /// this too and has to say so by removing `modlist.txt` themselves - an + /// annoyance, weighed against permanently losing the one thing on disk that + /// cannot be re-derived: which of forty overlapping mods wins each file + /// conflict, and which are installed but deliberately off. Eidos's own Remove + /// does not hit it: it calls [`Profile::forget_mods`] for what it deleted. /// /// MO2 has no equivalent. `Profile::refreshModStatus` rewrites the file inside /// the same refresh that dropped the entries, and the guard that looks like diff --git a/crates/eidos-instance/src/profile/tests.rs b/crates/eidos-instance/src/profile/tests.rs index 0c91f8bd..5a5823a6 100644 --- a/crates/eidos-instance/src/profile/tests.rs +++ b/crates/eidos-instance/src/profile/tests.rs @@ -505,13 +505,15 @@ fn a_folder_nobody_listed_appears_disabled() { // (MO2 parity): nothing knows where in the conflict order it belongs, and // silently enabling it could overwrite half the load order's files on the // next launch. A mod installed THROUGH Eidos never takes this path - the - // installer writes its own modlist entry. + // installer writes its own modlist entry. It lands at the HIGHEST priority, + // the last display row, like MO2 and like a fresh install: once enabled it is + // meant to win, not to sit under every other mod. let read: Vec<_> = p .modlist() .iter() .map(|m| (m.name.clone(), m.enabled)) .collect(); - assert_eq!(read, vec![("New".into(), false), ("A".into(), true)]); + assert_eq!(read, vec![("A".into(), true), ("New".into(), false)]); let _ = fs::remove_dir_all(&root); } @@ -755,6 +757,25 @@ fn display_order_file_order_and_load_order_stay_consistent() { let _ = fs::remove_dir_all(&root); } +#[test] +fn saving_an_empty_list_leaves_no_file_to_block_the_next_install() { + // The window saves right after the last mod is removed. An empty modlist.txt + // next to the next installed folder reads as a truncated list and refuses it. + let root = inst_with_mods(&["Gone"]); + let p = prof(&root, "Default"); + p.save_modlist(&p.modlist()).unwrap(); + fs::remove_dir_all(root.join("mods/Gone")).unwrap(); + p.forget_mods(&["Gone".to_string()]).unwrap(); + + p.save_modlist(&[]).unwrap(); + + assert!(!p.modlist_path().exists()); + fs::create_dir_all(root.join("mods/New")).unwrap(); + p.register_installed_mod("New").unwrap(); + assert!(p.modlist_checked().1.is_good()); + let _ = fs::remove_dir_all(&root); +} + #[test] fn separator_round_trips_keeps_position_and_is_excluded_from_load_order() { // A separator is a real `*_separator` folder; it must round-trip in place, diff --git a/crates/eidos-plugins/Cargo.toml b/crates/eidos-plugins/Cargo.toml index 67976752..2ca43000 100644 --- a/crates/eidos-plugins/Cargo.toml +++ b/crates/eidos-plugins/Cargo.toml @@ -10,6 +10,7 @@ edition.workspace = true esplugin = "6" encoding_rs = "0.8" eidos-gamedef = { path = "../eidos-gamedef" } +eidos-ini = { path = "../eidos-ini" } [dev-dependencies] libloadorder = "=18.8.2" diff --git a/crates/eidos-plugins/src/timestamp.rs b/crates/eidos-plugins/src/timestamp.rs index 97026652..e1b8925c 100644 --- a/crates/eidos-plugins/src/timestamp.rs +++ b/crates/eidos-plugins/src/timestamp.rs @@ -18,34 +18,25 @@ impl GameSpec { } } /// Location to READ the game's existing activation state. Writers use profile storage. +/// The flag is read through `eidos_ini::get_key`, the reader the root-mode writer +/// pairs with: first value wins like the engine's (Wine's) parser, and a BOM does +/// not hide `[General]`. A private parser kept the LAST duplicate and missed a +/// BOM'd header, so Eidos could read plugins.txt from where the game does not. pub fn plugin_state_dir(prefix: &Path, game_root: &Path, spec: &GameSpec) -> PathBuf { if spec.esplugin_id == esplugin::GameId::Morrowind || spec.esplugin_id == esplugin::GameId::Oblivion && newest_variant(game_root, "Oblivion.ini") .and_then(|p| read_decoded(&p)) - .is_some_and(|s| ini_value(&s, "General", "bUseMyGamesDirectory") == Some("0")) + .is_some_and(|s| { + eidos_ini::get_key(&s, "General", "bUseMyGamesDirectory").map(str::trim) + == Some("0") + }) { game_root.to_path_buf() } else { plugins_txt_dir(prefix, spec) } } -fn ini_value<'a>(text: &'a str, section: &str, key: &str) -> Option<&'a str> { - let mut inside = false; - let mut value = None; - for line in text.lines().map(str::trim) { - if let Some(s) = line.strip_prefix('[').and_then(|s| s.strip_suffix(']')) { - inside = s.eq_ignore_ascii_case(section); - } else if inside { - if let Some((k, v)) = line.split_once('=') { - if k.trim().eq_ignore_ascii_case(key) { - value = Some(v.trim()); - } - } - } - } - value -} pub fn morrowind_active(text: &str) -> Vec { let mut inside = false; let mut entries = BTreeMap::new(); diff --git a/crates/eidos-plugins/tests/engine_order.rs b/crates/eidos-plugins/tests/engine_order.rs index 1aeaffb0..7cb2f51c 100644 --- a/crates/eidos-plugins/tests/engine_order.rs +++ b/crates/eidos-plugins/tests/engine_order.rs @@ -228,6 +228,25 @@ fn oblivion_state_location_obeys_the_install_root_flag() { ); } #[test] +fn oblivion_install_root_flag_is_read_like_the_engine() { + // The engine (Wine's profile parser) keeps the FIRST duplicate and sees past + // a UTF-8 BOM; the state must be read from where the game reads it. + let t = Temp::new(); + let prefix = t.0.join("prefix"); + let spec = GameSpec::for_id("oblivion").unwrap(); + for ini in [ + "[General]\nbUseMyGamesDirectory=0\nbUseMyGamesDirectory=1\n", + "\u{feff}[General]\r\nbUseMyGamesDirectory=0\r\n", + ] { + fs::write(t.0.join("Oblivion.ini"), ini).unwrap(); + assert_eq!( + eidos_plugins::plugin_state_dir(&prefix, &t.0, &spec), + t.0, + "{ini:?}" + ); + } +} +#[test] fn timestamp_order_matches_libloadorder_without_changing_sources() { for (id, oracle_id) in [ ("morrowind", loadorder::GameId::Morrowind), diff --git a/crates/eidos-transfer/src/plan.rs b/crates/eidos-transfer/src/plan.rs index 741f9855..e901d3c9 100644 --- a/crates/eidos-transfer/src/plan.rs +++ b/crates/eidos-transfer/src/plan.rs @@ -218,9 +218,11 @@ pub fn plan(inst: &Instance, opt: &Options) -> Plan { // contributing the moment this was pushed, so without this line // a mod folder whose only child cannot be read produces no // entries at all - and the whole ancestor chain vanishes from - // the archive while `left` names only the deepest one. 7-Zip - // stores a directory it cannot read and warns, which is the - // honest outcome: the shape survives, the loss is reported. + // the archive while `left` names only the deepest one. The pack + // stages every directory entry from an empty scratch folder, so + // 7-Zip never opens this one and never warns: the shape survives, + // and the loss is reported by the pack's own warning built from + // `left` (`left_out_warning`). if !rel.is_empty() { out.entries.push(rel); } diff --git a/crates/eidos-transfer/src/transfer.rs b/crates/eidos-transfer/src/transfer.rs index 975fcbef..a8a27787 100644 --- a/crates/eidos-transfer/src/transfer.rs +++ b/crates/eidos-transfer/src/transfer.rs @@ -8,7 +8,7 @@ use std::path::{Path, PathBuf}; use eidos_instance::Instance; use crate::manifest::BackupManifest; -use crate::plan::{scrub_meta, Outside, Plan}; +use crate::plan::{scrub_meta, Left, Outside, Plan, Why}; use crate::relocate::{relocate, Relocated}; use crate::{ existing_ancestor, free_bytes, human_bytes, Options, TransferError, MANIFEST_NAME, @@ -62,10 +62,59 @@ pub struct PackReport { pub source_bytes: u64, pub entries: usize, pub manifest: BackupManifest, - /// Things worth saying that are not failures. + /// What did not make it into the archive although the user would expect it + /// to. The file is real and worth keeping, but a non-empty list means the + /// backup is NOT complete, and both front ends say so (`eidos pack` exits 1). pub warnings: Vec, } +/// The one warning naming what the walk skipped for a reason the rules did not +/// anticipate: a symbolic link, a name 7-Zip's list file cannot carry, a folder +/// that could not be read. The deliberate exclusions (the prefix, logs, caches) +/// are not losses and stay out of it. +/// +/// Without this, those items lived only in the manifest and the CLI's pre-pack +/// listing, and the report came back clean: a `mods/` symlinked to a second +/// drive was dropped whole while the window said "Packed" and `eidos pack ... +/// && rm -rf ` went ahead. An unreadable folder is worse - the pack stages +/// it from an empty scratch folder, so 7-Zip never sees it and never warns. +fn left_out_warning(left: &[Left]) -> Option { + const SHOWN: usize = 20; + let lost: Vec<&Left> = left + .iter() + .filter(|l| { + matches!( + l.why, + Why::Symlink | Why::NotUtf8 | Why::LineBreak | Why::Unreadable(_) + ) + }) + .collect(); + if lost.is_empty() { + return None; + } + let mut out = format!( + "{} item(s) could not be packed and are NOT in the backup:", + lost.len() + ); + for l in lost.iter().take(SHOWN) { + out.push_str(&format!("\n {} - {}", l.path, l.why)); + // A `downloads/` linked to another drive is usually deliberate, and the + // walk never follows a link (7-Zip would copy whatever it points at). + // Leaving it out on request is not a loss, so say how: otherwise a + // `pack && ...` script fails on every run with no way out given. + if l.path == "downloads" && l.why == Why::Symlink { + out.push_str( + " (to back up without it, pass --no-downloads or untick \ + \"Include downloads/\" in the window)", + ); + } + } + if lost.len() > SHOWN { + out.push_str(&format!("\n ... and {} more", lost.len() - SHOWN)); + } + Some(out) +} + impl PackReport { /// The archive as a percentage of the source. /// @@ -347,7 +396,7 @@ impl Transfer { fs::create_dir_all(&stage).map_err(io("could not stage the manifest"))?; let manifest = BackupManifest::describe(inst, plan, &self.opt); - let mut warnings = Vec::new(); + let mut warnings: Vec = left_out_warning(&plan.left).into_iter().collect(); fs::write(stage.join(MANIFEST_NAME), manifest.render()) .map_err(io("could not write the manifest"))?; let mut staged = vec![MANIFEST_NAME.to_string()]; @@ -795,6 +844,32 @@ mod tests { } } + #[test] + fn only_unplanned_losses_make_the_backup_incomplete() { + let left = |path: &str, why: Why| Left { + path: path.into(), + why, + }; + // A stock instance: every exclusion is deliberate, nothing is lost. + assert_eq!( + left_out_warning(&[left("logs", Why::Local), left("loot", Why::Refetchable)]), + None + ); + let w = left_out_warning(&[ + left("logs", Why::Local), + left("mods", Why::Symlink), + left("mods/A/locked", Why::Unreadable("Permission denied".into())), + ]) + .expect("a symlinked mods/ is a loss"); + assert!(w.starts_with("2 item(s)"), "{w}"); + assert!(w.contains("mods - a symbolic link"), "{w}"); + assert!(w.contains("mods/A/locked - unreadable"), "{w}"); + assert!(!w.contains("logs"), "{w}"); + assert!(!w.contains("--no-downloads"), "{w}"); + let w = left_out_warning(&[left("downloads", Why::Symlink)]).unwrap(); + assert!(w.contains("downloads - a symbolic link (to back up without it, pass --no-downloads"), "{w}"); + } + #[test] fn a_scratch_folder_removes_itself() { let path = { diff --git a/crates/eidos-transfer/tests/round_trip.rs b/crates/eidos-transfer/tests/round_trip.rs index af727b16..4f1b7354 100644 --- a/crates/eidos-transfer/tests/round_trip.rs +++ b/crates/eidos-transfer/tests/round_trip.rs @@ -146,7 +146,14 @@ fn an_instance_survives_being_packed_and_put_back_somewhere_else() { .pack(&inst, &p, &archive, &mut |pc| seen.push(pc)) .expect("pack"); assert!(archive.is_file()); - assert!(report.warnings.is_empty(), "{:?}", report.warnings); + // The one loss in this instance is the symlink, and it makes the backup + // incomplete rather than quietly smaller. + assert_eq!(report.warnings.len(), 1, "{:?}", report.warnings); + assert!( + report.warnings[0].contains("mods/danger-link - a symbolic link"), + "{:?}", + report.warnings + ); assert_eq!( seen.last().copied(), Some(100), @@ -330,9 +337,10 @@ fn a_file_that_vanishes_under_the_pack_costs_that_file_and_not_the_backup() { let t = Transfer::new(opt).unwrap(); let report = t.pack(&inst, &p, &archive, &mut |_| {}).expect("pack"); assert!(archive.is_file(), "the archive is real and worth keeping"); - assert_eq!(report.warnings.len(), 1, "{:?}", report.warnings); + // Two: the fixture's symlink, and the vanished file. + assert_eq!(report.warnings.len(), 2, "{:?}", report.warnings); assert!( - report.warnings[0].contains("missing some files"), + report.warnings[1].contains("missing some files"), "{:?}", report.warnings ); diff --git a/crates/eidos/src/collection.rs b/crates/eidos/src/collection.rs index 4917c49c..0d1ee7da 100644 --- a/crates/eidos/src/collection.rs +++ b/crates/eidos/src/collection.rs @@ -197,16 +197,12 @@ pub(crate) fn cmd_collection(args: &[String]) { collection_domain: c.info.domain_name.clone(), owner: format!("{}:{}:{}", state.game_domain, state.slug, state.revision), renamed: Vec::new(), + kept: Vec::new(), }; let mut save = |s: &InstallState| s.save(&state_path).map_err(|e| e.to_string()); let mut report = eidos_collections::install::run(c, &mut state, &mut hooks, &mut save); report.unknown_sections = read.unknown_sections.clone(); - for (member, folder) in std::mem::take(&mut hooks.renamed) { - report.renamed.push(eidos_collections::report::Note { - subject: member, - detail: format!("installed as \"{folder}\", because a mod of yours already had that name"), - }); - } + hooks.drain_notes(&mut report); if report.aborted { eidos_log::warn!("{}", report.render()); diff --git a/crates/eidos/src/install.rs b/crates/eidos/src/install.rs index 43f2ad67..1bd4fa55 100644 --- a/crates/eidos/src/install.rs +++ b/crates/eidos/src/install.rs @@ -244,10 +244,9 @@ pub(crate) fn cmd_install(args: &[String]) { .name .clone() .unwrap_or_else(|| eidos_install::mod_name_for(&options.archive)); - let fallback = game.prefix().and_then(|p| { - game.plugin_spec() - .map(|s| eidos_plugins::plugins_txt_dir(&p, &s)) - }); + // Same fallback as the GUI: the install root for Morrowind and root-mode + // Oblivion, where AppData has no state and every plugin would read active. + let fallback = game.plugin_state_dir(); let fomod = eidos_install::fomod_context_for_instance( &inst, &game.data_path, diff --git a/crates/eidos/src/prepare.rs b/crates/eidos/src/prepare.rs index 19d5a47e..ffab3136 100644 --- a/crates/eidos/src/prepare.rs +++ b/crates/eidos/src/prepare.rs @@ -481,6 +481,12 @@ pub(crate) fn sync_saves_for_cloud( .unwrap_or_default(); let mut new_entries: Vec = Vec::new(); + // Every profile of the instance binds over this same prefix dir, so a copy + // a SIBLING profile pushed is provenance-known too: it still lives in that + // profile's own saves. Checking only our manifest made two profiles + // alternating on a fixed name (quicksave.fos, autosave.ess) import each + // other's copy as a fresh orphan-* group on every sync, forever. + let mut siblings = None; // Rescue the whole batch before replacing any file, so a failed co-save // rescue also leaves its prefix save untouched. let mut copies = Vec::new(); @@ -493,7 +499,12 @@ pub(crate) fn sync_saves_for_cloud( Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} Err(e) => return Err(e), Ok(d) if src_mtime > d => { - if !manifest.contains(&manifest_key(&dst)?) { + let key = manifest_key(&dst)?; + if !manifest.contains(&key) + && !siblings + .get_or_insert_with(|| instance_sync_keys(prof_saves)) + .contains(&key) + { preserve_diverged_save(&dst, d, prof_saves)?; } } @@ -518,6 +529,78 @@ pub(crate) fn sync_saves_for_cloud( Ok(n) } +/// Every content key any profile of this instance has pushed to the prefix: +/// `prof_saves` is `/profiles//saves`, and all those profiles +/// bind over the same prefix Saves dir. +fn instance_sync_keys(prof_saves: &std::path::Path) -> std::collections::HashSet { + let Some(profiles) = prof_saves.parent().and_then(|p| p.parent()) else { + return Default::default(); + }; + std::fs::read_dir(profiles) + .into_iter() + .flatten() + .flatten() + .filter_map(|e| std::fs::read_to_string(e.path().join("saves/.cloud-sync-manifest")).ok()) + .flat_map(|t| t.lines().map(String::from).collect::>()) + .collect() +} + +/// Prefix saves the profile bind will hide for good: written after the profile +/// was seeded (the `.seeded` marker's mtime), absent from the profile, and never +/// pushed there by any profile of this instance. A Steam Cloud download from +/// another device or a launch without Eidos lands here, and nothing pulls it in: +/// seeding is one-time so saves the user deleted stay deleted. Older names are +/// that deleted set, so they are not reported. +fn hidden_prefix_saves(prof_saves: &std::path::Path, prefix_saves: &std::path::Path) -> Vec { + let Ok(seeded) = std::fs::metadata(prof_saves.join(".seeded")).and_then(|m| m.modified()) + else { + return Vec::new(); // not seeded yet: the next seed adopts everything + }; + let pushed: std::collections::HashSet = instance_sync_keys(prof_saves) + .into_iter() + .filter_map(|k| k.split_once('\t').map(|(name, _)| name.to_string())) + .collect(); + std::fs::read_dir(prefix_saves) + .into_iter() + .flatten() + .flatten() + .filter_map(|e| { + let name = e.file_name().into_string().ok()?; + let newer = e + .metadata() + .and_then(|m| m.modified()) + .is_ok_and(|t| t > seeded); + (eidos_instance::is_save_listing(&name) + && newer + && !pushed.contains(&name) + && !prof_saves.join(&name).exists()) + .then_some(name) + }) + .collect() +} + +/// Whether the hidden set differs from the one the last launch warned about, +/// recording the new set. The warning is about files the user has to move by +/// hand, and it repeated on every launch until they did, burying itself in the +/// log; now it comes once per change. An empty set clears the record, so the +/// same names turning up again later are reported again. The record sits beside +/// `.seeded` on the profile side: the cloud sync only copies save data out. +fn hidden_saves_changed(prof_saves: &std::path::Path, hidden: &[String]) -> bool { + let record = prof_saves.join(".hidden-saves-warned"); + if hidden.is_empty() { + let _ = std::fs::remove_file(record); + return false; + } + let mut names = hidden.to_vec(); + names.sort(); + let body = names.join("\n"); + if std::fs::read_to_string(&record).is_ok_and(|known| known == body) { + return false; + } + let _ = std::fs::write(record, body); + true +} + /// Content provenance: matching sizes and second-resolution mtimes cannot prove /// the prefix still holds our previous copy. A stdlib hash change merely causes /// a conservative rescue on the first sync after an upgrade. @@ -594,6 +677,12 @@ pub(crate) fn preserve_diverged_save( let _ = std::fs::remove_file(&orphan); return Err(error); } + // Keep each file's own date, like the seed and the sync copy: a + // rescue stamped "now" sorted another profile's or a stale save + // above the playthrough the user made last session. + if let Ok(mtime) = std::fs::metadata(&path).and_then(|m| m.modified()) { + let _ = file.set_modified(mtime); + } } Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists @@ -633,6 +722,19 @@ pub(crate) fn prepare_saves( prof.name ); } + let hidden = hidden_prefix_saves(&prof.saves_dir(), &source); + if hidden_saves_changed(&prof.saves_dir(), &hidden) { + eidos_log::warn!( + "eidos play: {} save(s) in {} are not in profile '{}' (another device, a launch \ + without Eidos or another instance wrote them) and stay hidden while its saves \ + are bound; copy them into {} to play them: {}", + hidden.len(), + source.display(), + prof.name, + prof.saves_dir().display(), + hidden.join(", ") + ); + } Ok(Some((prof.saves_dir(), source))) } @@ -1145,6 +1247,105 @@ mod rescue_tests { })); fs::remove_dir_all(root).unwrap(); } + fn stamped(path: &std::path::Path, bytes: &[u8], secs: u64) { + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, bytes).unwrap(); + fs::File::options() + .write(true) + .open(path) + .unwrap() + .set_modified(UNIX_EPOCH + Duration::from_secs(secs)) + .unwrap(); + } + + fn orphans(dir: &std::path::Path) -> Vec { + fs::read_dir(dir) + .unwrap() + .flatten() + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| n.starts_with("orphan-")) + .collect() + } + + #[test] + fn sibling_profiles_alternating_on_a_fixed_name_mint_no_orphans() { + let root = + std::env::temp_dir().join(format!("eidos-rescue-siblings-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + let a = root.join("profiles/Main/saves"); + let b = root.join("profiles/Hardcore/saves"); + let prefix = root.join("prefix"); + let t = 1_700_000_000; + for (dir, tag, secs) in [ + (&a, "main 1", t), + (&b, "hardcore 1", t + 10), + (&a, "main 2", t + 20), + ] { + stamped(&dir.join("quicksave.fos"), tag.as_bytes(), secs); + stamped(&dir.join("quicksave.nvse"), tag.as_bytes(), secs); + sync_saves_for_cloud(dir, &prefix).unwrap(); + assert_eq!(fs::read(prefix.join("quicksave.fos")).unwrap(), tag.as_bytes()); + } + // Each overwritten prefix copy still lives in the profile that pushed it. + assert!(orphans(&a).is_empty(), "{:?}", orphans(&a)); + assert!(orphans(&b).is_empty(), "{:?}", orphans(&b)); + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn rescued_orphans_keep_their_own_mtime() { + let root = std::env::temp_dir().join(format!("eidos-rescue-mtime-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + let profile = root.join("profile"); + let prefix = root.join("prefix"); + let t = 1_700_000_000; + stamped(&prefix.join("quicksave.ess"), b"old session", t); + stamped(&prefix.join("quicksave.skse"), b"old cosave", t + 1); + fs::create_dir_all(&profile).unwrap(); + fs::write(profile.join("quicksave.ess"), b"new session").unwrap(); + sync_saves_for_cloud(&profile, &prefix).unwrap(); + for (ext, secs) in [("ess", t), ("skse", t + 1)] { + let orphan = profile.join(format!("orphan-{t}-quicksave.{ext}")); + assert_eq!( + fs::metadata(orphan).unwrap().modified().unwrap(), + UNIX_EPOCH + Duration::from_secs(secs), + "a rescue stamped now sorts above the live playthrough" + ); + } + fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn prefix_saves_made_after_seeding_outside_the_instance_are_reported() { + let root = std::env::temp_dir().join(format!("eidos-hidden-saves-{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + let saves = root.join("profiles/Main/saves"); + let prefix = root.join("prefix"); + let t = 1_700_000_000; + stamped(&saves.join(".seeded"), b"done", t); + stamped(&saves.join("Kept.ess"), b"kept", t + 10); + // Seeded then deleted by the user: older than the marker, stays deleted. + stamped(&prefix.join("Deleted.ess"), b"deleted", t - 10); + stamped(&prefix.join("Kept.ess"), b"kept", t + 10); + // A sibling profile's own push, not a stranger's. + let pushed = root.join("profiles/Other/saves"); + stamped(&prefix.join("Other.ess"), b"other", t + 10); + fs::create_dir_all(&pushed).unwrap(); + fs::write(pushed.join(".cloud-sync-manifest"), "Other.ess\t0000000000000000\n").unwrap(); + // Made on a Steam Deck and downloaded by Steam Cloud. + stamped(&prefix.join("Deck.ess"), b"deck", t + 10); + stamped(&prefix.join("Deck.skse"), b"deck cosave", t + 10); + let hidden = hidden_prefix_saves(&saves, &prefix); + assert_eq!(hidden, vec!["Deck.ess".to_string()]); + // Warned once per change, not on every launch. + assert!(hidden_saves_changed(&saves, &hidden)); + assert!(!hidden_saves_changed(&saves, &hidden)); + assert!(hidden_saves_changed(&saves, &["Deck.ess".into(), "Deck2.ess".into()])); + assert!(!hidden_saves_changed(&saves, &[])); + assert!(hidden_saves_changed(&saves, &hidden)); + fs::remove_dir_all(root).unwrap(); + } + #[test] fn failed_rescue_keeps_both_prefix_files_unchanged() { use std::os::unix::fs::PermissionsExt; diff --git a/docs/guide/usage.md b/docs/guide/usage.md index 89921c0b..c8da2cf4 100644 --- a/docs/guide/usage.md +++ b/docs/guide/usage.md @@ -81,6 +81,49 @@ namespace, then runs the command through that view. Writes (saves, regenerated configs) land in the instance's `overwrite/` layer; the game install and every mod source stay byte-for-byte pristine. +### The mod list + +As in MO2, each profile has its own `modlist.txt` (which mods are enabled, in +what priority), while the mod folders live in the instance's shared `mods/`. A +folder you drop into `mods/` by hand is not listed yet: it shows up **disabled +at the highest priority** - the bottom of the window's list, where MO2 puts it +too - so once you enable it, it wins its conflicts. Older versions put it at +the top, where every other mod overrode it. + +Because the folders are shared, removing or renaming a mod changes every +profile: its line is dropped from, or renamed in, every profile's +`modlist.txt`, and each profile keeps its own state and order for the rest. A +profile left with no mods has no `modlist.txt` at all rather than an empty one, +which read as a damaged file and blocked the next install. + +The window works from its own copy of the mod list and load order. If another +Eidos process writes either one while it is open - `eidos install` or +`eidos sort` in a terminal, a collection, a session Steam started, a second +window - the window's next edit is refused instead of overwriting that work. It +reloads from disk and says *Not saved: another Eidos process changed the mod +list. It has been reloaded - redo the change.* (or the same for the load +order); only that one click is lost, so make it again. Rename, New mod, Add +separator, Remove and turning the Overwrite into a mod check before touching +`mods/`, so a refused edit changes nothing on disk. The INI editor likewise +refuses to save over a file that changed since it opened it. + +### Saves belong to the profile + +Each profile keeps its own saves in `profiles//saves/`, and for the length +of a run that folder is mounted over the game's save folder in the Proton +prefix. The first launch of a profile copies the saves already in the prefix +into it, once, so a save you later delete from the profile stays deleted. After +each run the profile's newest saves are copied back into the prefix for Steam +Cloud; a prefix save Eidos did not put there is first kept in the profile as +`orphan-