From b468a81506657e85c09553cbb895691a11222582 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:25:29 +0200 Subject: [PATCH 01/30] fix(core): make copy-up atomic open_for_write copied a lower file straight onto its final Overwrite path, so a copy that failed part-way (ENOSPC) or was killed left a truncated file there. Later calls saw it existed and never copied again, so it shadowed the intact lower file for good, and an index rebuild during a copy could serve the half-written file to readers. The copy now goes to a hidden sibling temp (.mohidden), gets its mode and metadata, then is renamed into place; the temp is removed on error. Also satisfies clippy unnecessary_unwrap in a test. --- crates/eidos-core/src/lib.rs | 85 ++++++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/crates/eidos-core/src/lib.rs b/crates/eidos-core/src/lib.rs index d4de896..fb7e9b2 100644 --- a/crates/eidos-core/src/lib.rs +++ b/crates/eidos-core/src/lib.rs @@ -1023,21 +1023,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() { @@ -2285,6 +2314,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 +2813,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); From 0b332873efa98f4db89b556dc7a1bebe4f1a2423 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:29:40 +0200 Subject: [PATCH 02/30] fix(instance): forget removed mods so a batch remove cannot wedge the list Removing mods deleted their folders but left their lines in modlist.txt, so the trust check counted them as lost: a batch past the "unmounted drive" threshold made every later save, install registration and launch refuse, and other profiles listing those mods were wedged the same way. Remove now drops exactly the deleted names from every profile's modlist.txt before saving, and no longer hides a refused save behind "Removed N mod(s)". --- crates/eidos-gui/src/update.rs | 36 ++++++++++--- crates/eidos-instance/src/lib.rs | 41 ++++++++++++++ crates/eidos-instance/src/profile/modlist.rs | 57 +++++++++++++++++--- 3 files changed, 121 insertions(+), 13 deletions(-) diff --git a/crates/eidos-gui/src/update.rs b/crates/eidos-gui/src/update.rs index dbf613c..0d60569 100644 --- a/crates/eidos-gui/src/update.rs +++ b/crates/eidos-gui/src/update.rs @@ -2466,8 +2466,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 = app.created.as_ref() + .map(|inst| inst.forget_mods(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}")), } @@ -6228,7 +6239,7 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { // 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 +6247,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 +6255,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 = app.created.as_ref().map(|inst| inst.forget_mods(&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. diff --git a/crates/eidos-instance/src/lib.rs b/crates/eidos-instance/src/lib.rs index 686fceb..77b0bc5 100644 --- a/crates/eidos-instance/src/lib.rs +++ b/crates/eidos-instance/src/lib.rs @@ -1058,6 +1058,21 @@ 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 + } + /// Enabled mods of the active profile, highest priority first. pub fn load_order(&self) -> Vec { self.active().load_order() @@ -2804,6 +2819,32 @@ 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 mo2_import_refuses_a_busy_instance_before_writing() { let fixture = Fixture::new(); diff --git a/crates/eidos-instance/src/profile/modlist.rs b/crates/eidos-instance/src/profile/modlist.rs index be9bb1d..8682b5b 100644 --- a/crates/eidos-instance/src/profile/modlist.rs +++ b/crates/eidos-instance/src/profile/modlist.rs @@ -350,6 +350,47 @@ impl Profile { 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()) + } + /// 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 +428,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 From d32e9e840dba06c2777cc233707db175ff720f0b Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:30:37 +0200 Subject: [PATCH 03/30] fix(instance): put unlisted mod folders at the highest priority Reconciliation pushed folders missing from modlist.txt after every listed row in highest-first file order, so after the display reverse they became the LOWEST priority: once enabled, every other mod overrode them, and a mod created by capture_overwrite_into_mod (which only enables that entry in place) lost every conflict. Splice them in at the front instead, as the comment and MO2 intend; they stay disabled. --- crates/eidos-instance/src/profile/modlist.rs | 8 +++++++- crates/eidos-instance/src/profile/tests.rs | 6 ++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/eidos-instance/src/profile/modlist.rs b/crates/eidos-instance/src/profile/modlist.rs index 8682b5b..a6f2077 100644 --- a/crates/eidos-instance/src/profile/modlist.rs +++ b/crates/eidos-instance/src/profile/modlist.rs @@ -270,9 +270,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 +285,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) - \ diff --git a/crates/eidos-instance/src/profile/tests.rs b/crates/eidos-instance/src/profile/tests.rs index 0c91f8b..bef9233 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); } From e001f174ff464ac42ac1302381a13b202d747efa Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:31:54 +0200 Subject: [PATCH 04/30] fix(transfer): report skipped symlinks and unreadable folders as an incomplete pack The walk records symlinks, unreadable folders and names 7-Zip cannot take in plan.left, but the pack report never carried them, so a mods/ or downloads/ symlinked to another drive was dropped whole while the GUI said "Packed" and `eidos pack` exited 0. Transfer::pack now adds one warning listing those losses (capped at 20), which both front ends already treat as NOT complete. Deliberate exclusions (prefix, logs, caches) stay out of it. --- crates/eidos-transfer/src/plan.rs | 8 ++- crates/eidos-transfer/src/transfer.rs | 68 ++++++++++++++++++++++- crates/eidos-transfer/tests/round_trip.rs | 14 ++++- docs/guide/usage.md | 8 ++- 4 files changed, 86 insertions(+), 12 deletions(-) diff --git a/crates/eidos-transfer/src/plan.rs b/crates/eidos-transfer/src/plan.rs index 741f985..e901d3c 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 975fcbe..e979042 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,49 @@ 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)); + } + 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 +386,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 +834,29 @@ 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}"); + } + #[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 af727b1..4f1b735 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/docs/guide/usage.md b/docs/guide/usage.md index 89921c0..8e6e65e 100644 --- a/docs/guide/usage.md +++ b/docs/guide/usage.md @@ -184,8 +184,10 @@ eidos unpack ~/backup.eidos /mnt/games/EidosSkyrim # on the other machine and how much room it needs, without writing anything. `eidos unpack --info` does the same for a file you already have. -If 7-Zip could not read something, the archive is still written and named - it -is worth having - but `eidos pack` exits **1** and says what is missing. An +If 7-Zip could not read something, or the walk had to skip something you would +expect in a backup (a symbolic link, a folder it could not read, a name 7-Zip +cannot be given safely), the archive is still written and named - it is worth +having - but `eidos pack` exits **1** and says what is missing. An incomplete backup is not a success, and this is a command people put in front of `&&`. @@ -217,7 +219,7 @@ complete and is not. | `loot/`, except `userlist.yaml` | A masterlist cache Eidos re-fetches on demand. Your own LOOT rules are not a cache - nobody re-fetches those - so that one file rides along. | | `.base/`, `.base-root/` | Empty mountpoints where the game's own files are stashed during a session. | | Half-written files | A paused download (`*.unfinished`), an atomic write in flight (`*.eidos-tmp*`). | -| Symbolic links | 7-Zip would follow one and copy whatever it points at, which for an absolute link means pulling a foreign tree into your backup. They are reported instead. | +| Symbolic links | 7-Zip would follow one and copy whatever it points at, which for an absolute link means pulling a foreign tree into your backup. They are reported instead, and the backup counts as incomplete: a `mods/` or `downloads/` linked to another drive is NOT packed. | Every one of these goes into `eidos-backup.ini` at the root of the archive, with its reason - so the answer to "what is not in here" is `cat`, not a guess. The From 72c048231f16b2598beb49aebc6a42d3e85ab4a5 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:34:05 +0200 Subject: [PATCH 05/30] fix(ini): stop a UTF-8 BOM from hiding an INI's first section section_header used str::trim, which keeps U+FEFF, so a game INI saved as "UTF-8 with BOM" lost its first section: set_key appended a duplicate [General]/[Archive] at the end that Wine never reads (root mode, OMOD EditINI, INI tweaks, BSA invalidation). section_header now skips a leading BOM. plugin_state_dir reads bUseMyGamesDirectory through eidos_ini::get_key (first value wins, like the engine) and archive_plan uses section_header. --- Cargo.lock | 1 + crates/eidos-gamefeatures/src/archives.rs | 28 ++++++++++++++++++++-- crates/eidos-ini/src/lib.rs | 23 +++++++++++++++++- crates/eidos-plugins/Cargo.toml | 1 + crates/eidos-plugins/src/timestamp.rs | 25 +++++++------------ crates/eidos-plugins/tests/engine_order.rs | 19 +++++++++++++++ 6 files changed, 77 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c8a380c..130c851 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-gamefeatures/src/archives.rs b/crates/eidos-gamefeatures/src/archives.rs index a1792c4..95fa14b 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-ini/src/lib.rs b/crates/eidos-ini/src/lib.rs index 0e57b28..8b89814 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-plugins/Cargo.toml b/crates/eidos-plugins/Cargo.toml index 6797675..2ca4300 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 9702665..e1b8925 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 1aeaffb..7cb2f51 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), From 6a9eba02376e5b457b4c284d79c05c431b8f9715 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:38:23 +0200 Subject: [PATCH 06/30] fix(plugins): seed the profile from the game's real state before writing it The collection driver seeded and read plugin state from AppData, and the GUI Plugins tab wrote without seeding at all. For Morrowind and root-mode Oblivion the state lives in the install root, so a fresh profile was founded on an all-enabled default, and a Morrowind write built a [Game Files]-only stub INI that every later launch deployed. Both now seed from plugin_state_dir like launch and sort; the collection shadow write skips Timestamp games so it never lands in the install, and CLI/collection FOMOD contexts use the same fallback. --- crates/eidos-collections/src/driver.rs | 24 ++++-- .../eidos-collections/tests/plugin_states.rs | 76 +++++++++++++++++++ crates/eidos-gui/src/main.rs | 30 ++++++++ crates/eidos-gui/src/modinfo.rs | 7 ++ crates/eidos/src/install.rs | 7 +- 5 files changed, 132 insertions(+), 12 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index c406b29..9173387 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -429,11 +429,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, @@ -994,7 +993,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 +1049,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, diff --git a/crates/eidos-collections/tests/plugin_states.rs b/crates/eidos-collections/tests/plugin_states.rs index 5c40df7..a2ff0ca 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-gui/src/main.rs b/crates/eidos-gui/src/main.rs index 2295fed..2e4b5aa 100644 --- a/crates/eidos-gui/src/main.rs +++ b/crates/eidos-gui/src/main.rs @@ -6035,6 +6035,36 @@ 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(); + } + /// An instance with two profiles and a save in the active one. fn saves_app() -> (App, PathBuf) { let root = temp_portable("skyrimse"); diff --git a/crates/eidos-gui/src/modinfo.rs b/crates/eidos-gui/src/modinfo.rs index c376020..b5650ba 100644 --- a/crates/eidos-gui/src/modinfo.rs +++ b/crates/eidos-gui/src/modinfo.rs @@ -2684,6 +2684,13 @@ pub(crate) fn write_plugin_state( .transpose()?; if let Some(inst) = app.created.as_ref() { let prof = inst.active(); + // 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 diff --git a/crates/eidos/src/install.rs b/crates/eidos/src/install.rs index 43f2ad6..1bd4fa5 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, From b629533de6ab9e8a4f3f853fcd71ac078a4d4fc6 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:46:03 +0200 Subject: [PATCH 07/30] fix(gui): refuse to save a mod list or load order another process changed The window wrote its cached mod list and plugin order wholesale, read minutes earlier. A CLI install, collection or sort in between was erased by the next click (installed mods came back disabled, the order reverted), and the plugin snapshot was refreshed to the revert. The window now keeps a content hash of modlist.txt and of the profile's plugin state, checks it under the lock before writing, and reloads on mismatch instead. --- crates/eidos-gui/src/main.rs | 91 ++++++++++++++++++++ crates/eidos-gui/src/modinfo.rs | 36 ++++++++ crates/eidos-gui/src/state.rs | 75 ++++++++++++++-- crates/eidos-gui/src/update.rs | 6 +- crates/eidos-instance/src/profile/modlist.rs | 18 +++- 5 files changed, 211 insertions(+), 15 deletions(-) diff --git a/crates/eidos-gui/src/main.rs b/crates/eidos-gui/src/main.rs index 2e4b5aa..7cbfe00 100644 --- a/crates/eidos-gui/src/main.rs +++ b/crates/eidos-gui/src/main.rs @@ -1763,8 +1763,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, @@ -6065,6 +6074,47 @@ mod tests { 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(); + } + /// An instance with two profiles and a save in the active one. fn saves_app() -> (App, PathBuf) { let root = temp_portable("skyrimse"); @@ -7393,6 +7443,47 @@ 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 renaming_a_mod_leaves_it_where_it_was_and_still_enabled() { // The defect, exactly as it was reported: rename a mod and it is diff --git a/crates/eidos-gui/src/modinfo.rs b/crates/eidos-gui/src/modinfo.rs index b5650ba..3510efd 100644 --- a/crates/eidos-gui/src/modinfo.rs +++ b/crates/eidos-gui/src/modinfo.rs @@ -2565,6 +2565,12 @@ pub(crate) fn compute_plugins(app: &App) -> Option { }); } + // 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 +2674,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 +2703,21 @@ 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 @@ -2701,6 +2735,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()) { diff --git a/crates/eidos-gui/src/state.rs b/crates/eidos-gui/src/state.rs index 3533402..3fb03d7 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,28 @@ 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)) +} + /// 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 +2325,36 @@ 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( + "Not saved: another Eidos process changed the mod list. It has been reloaded - redo the change." + .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> { + let inst = app.created.as_ref()?; + let current = modlist_unchanged(app, inst); + let forgot = inst.forget_mods(names); + if current { + app.modlist_seen.set(Some(modlist_fingerprint(inst))); + } + Some(forgot) } /// Invalidate every memoised view listing. Cheap: the listings rebuild lazily on @@ -2628,10 +2684,11 @@ pub(crate) fn put_mod_selection(app: &mut App, held: HeldSelection) { 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. + // 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); } // The merged view depends on which mods are enabled and in what order, not diff --git a/crates/eidos-gui/src/update.rs b/crates/eidos-gui/src/update.rs index 0d60569..13906f8 100644 --- a/crates/eidos-gui/src/update.rs +++ b/crates/eidos-gui/src/update.rs @@ -2469,8 +2469,8 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { // 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 = app.created.as_ref() - .map(|inst| inst.forget_mods(std::slice::from_ref(&m.name))); + 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}.", @@ -6266,7 +6266,7 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { // 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 = app.created.as_ref().map(|inst| inst.forget_mods(&gone)); + let forgot = forget_removed_mods(app, &gone); if let Some(Err(e)) = forgot { status = format!("{status} The mod list could not be updated: {e}."); } diff --git a/crates/eidos-instance/src/profile/modlist.rs b/crates/eidos-instance/src/profile/modlist.rs index a6f2077..bc80281 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()) } @@ -350,9 +358,13 @@ impl Profile { let _ = fs::copy(&target, target.with_extension("txt.bak")); } // 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()) } From 06934767cf4cf2e81c8ccd454df8fab2ba1904e9 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:49:18 +0200 Subject: [PATCH 08/30] fix(saves): stop profiles sharing a prefix from rescuing each other's saves The cloud sync checked only the current profile's manifest, so two profiles alternating on a fixed save name imported each other's copy as a new orphan group on every sync. Copies any sibling profile pushed now count as known. Rescued orphans keep their source mtime instead of sorting above the live save, and launch warns about prefix saves made after seeding (another device, a launch without Eidos) that the profile bind would otherwise hide silently. --- crates/eidos/src/prepare.rs | 174 +++++++++++++++++++++++++++++++++++- 1 file changed, 173 insertions(+), 1 deletion(-) diff --git a/crates/eidos/src/prepare.rs b/crates/eidos/src/prepare.rs index 19d5a47..6bd5674 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,56 @@ 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() +} + /// 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 +655,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 +700,19 @@ pub(crate) fn prepare_saves( prof.name ); } + let hidden = hidden_prefix_saves(&prof.saves_dir(), &source); + if !hidden.is_empty() { + 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 +1225,98 @@ 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); + assert_eq!(hidden_prefix_saves(&saves, &prefix), vec!["Deck.ess".to_string()]); + fs::remove_dir_all(root).unwrap(); + } + #[test] fn failed_rescue_keeps_both_prefix_files_unchanged() { use std::os::unix::fs::PermissionsExt; From 323dbe2204fa04bccdd23ea1c42bee40d4ce1efd Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 21:56:03 +0200 Subject: [PATCH 09/30] fix(collections): let a new revision take over the previous revision's folders The ownership marker and the state file are per revision, so installing revision N+1 of an installed collection started from nothing: every member collided with its own previous copy, was extracted again as "Name (2)", and the old copies, including members the update dropped, stayed enabled. Before reserving anything, both the CLI and the window now hand each folder still carrying another revision's exact marker to the matching member (same key, or the only member of the same Nexus page when its file was updated), moving its receipt with it, and report folders no revision member uses. --- crates/eidos-collections/src/driver.rs | 234 +++++++++++++++++++++++++ crates/eidos-collections/src/recipe.rs | 26 +++ crates/eidos-gui/src/update.rs | 4 + crates/eidos/src/collection.rs | 11 ++ docs/guide/usage.md | 6 + 5 files changed, 281 insertions(+) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index 9173387..0964552 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -242,6 +242,180 @@ fn reserve_folder( } } +/// Hand the folders another revision of this collection owns to the revision +/// `state` installs, before it starts. +/// +/// 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. The receipt moves with the folder, 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, +/// so a crash before the new state is saved loses nothing. +/// +/// Returns the folders another revision still owns that this one does not use. +/// They stay installed and enabled - removing a mod is the user's call - so the +/// report has to name them. +pub fn adopt_other_revisions( + inst: &Instance, + c: &Collection, + state: &mut InstallState, +) -> 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(); + let mut leftovers = Vec::new(); + for other in other_revisions(inst, state)? { + 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(); + 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())); + } + } + } + for (key, other_key) in pairs { + let folder = &folders[&other_key]; + if state.folders.contains_key(&key) + || state.folders.values().any(|f| 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)) + || installer_answers::path(&path).exists() + { + continue; + } + 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())?; + state.folders.insert(key, folder.clone()); + } + for (key, folder) in &folders { + if eidos_install::fix_directory_name(folder).as_deref() == Some(folder.as_str()) + && owns_folder(&mods.join(folder), &marker(&other_owner, key)) + { + leftovers.push(Note { + subject: folder.clone(), + detail: format!( + "revision {} of this collection installed it and this revision does not use it; it is still enabled, so disable or remove it yourself", + other.revision + ), + }); + } + } + } + Ok(leftovers) +} + +/// Every other revision of this collection with a state file here, newest first. +fn other_revisions(inst: &Instance, state: &InstallState) -> 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 Some(other) = InstallState::load(&dir.join(&name))? else { + 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 { @@ -1305,4 +1479,64 @@ mod cache_tests { assert!(cached_manifest(&dir).unwrap().is_none()); std::fs::remove_dir_all(root).unwrap(); } + + #[test] + fn a_new_revision_takes_over_its_previous_folders_instead_of_duplicating_them() { + use crate::state::Status; + let root = std::env::temp_dir().join(format!("eidos-revision-adopt-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let inst = Instance::portable(root.clone()); + let domain = "skyrimspecialedition"; + let (old_owner, new_owner) = (format!("{domain}:gate:1"), format!("{domain}:gate:2")); + let mark = |owner: &str, key: &str| serde_json::to_string(&[owner, key]).unwrap(); + let key = |file: u64| format!("file:{domain}:{file}"); + let manifest = |mods: &[(&str, u64, u64)]| { + let mods: Vec = mods + .iter() + .map(|(name, m, f)| format!(r#"{{"name":"{name}","source":{{"type":"nexus","modId":{m},"fileId":{f}}}}}"#)) + .collect(); + format!(r#"{{"info":{{"name":"Gate","domainName":"{domain}"}},"mods":[{}]}}"#, mods.join(",")) + }; + // Revision 1, installed: one member stays, one gets a new file of the + // same page, one is dropped by revision 2. + let old_dir = crate::state::revision_dir(&root, "gate", 1); + std::fs::create_dir_all(&old_dir).unwrap(); + std::fs::write(old_dir.join("collection.json"), manifest(&[("Kept", 1, 10), ("Updated", 2, 20), ("Dropped", 3, 30)])).unwrap(); + std::fs::write(cache_marker(&old_dir), eidos_nexus::md5_file(&old_dir.join("collection.json")).unwrap()).unwrap(); + let mut old = InstallState { slug: "gate".into(), revision: 1, game_domain: domain.into(), ..Default::default() }; + 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(&old_owner, &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 receipt = inst.mods_dir().join("Kept/.eidos-collection-recipe.json"); + let body = serde_json::json!({"schema":1,"owner":mark(&old_owner, &key(10)),"recipe":null,"files":{},"exclusions":{}}); + std::fs::write(&receipt, body.to_string()).unwrap(); + old.save(&InstallState::path(&root, "gate", 1)).unwrap(); + + let new = crate::read(&manifest(&[("Kept", 1, 10), ("Updated", 2, 21), ("Added", 4, 40)])).unwrap().collection; + let fresh = || InstallState { slug: "gate".into(), revision: 2, game_domain: domain.into(), ..Default::default() }; + let mut state = fresh(); + let leftovers = adopt_other_revisions(&inst, &new, &mut state).unwrap(); + 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(&new_owner, &key(21)))); + let moved: serde_json::Value = serde_json::from_slice(&std::fs::read(&receipt).unwrap()).unwrap(); + assert_eq!(moved["owner"], mark(&new_owner, &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(&new_owner, &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 = fresh(); + assert_eq!(adopt_other_revisions(&inst, &new, &mut again).unwrap(), leftovers); + assert_eq!(again.folders, state.folders); + std::fs::remove_dir_all(root).unwrap(); + } } diff --git a/crates/eidos-collections/src/recipe.rs b/crates/eidos-collections/src/recipe.rs index a584628..ea26c5a 100644 --- a/crates/eidos-collections/src/recipe.rs +++ b/crates/eidos-collections/src/recipe.rs @@ -707,3 +707,29 @@ pub fn verify_receipt(m: &Mod, payload: &Path, folder: &Path, owner: &str) -> Re && receipt.recipe == identity(m, payload)? && receipt.files == tree_digests(folder)?) } +/// 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-gui/src/update.rs b/crates/eidos-gui/src/update.rs index 13906f8..1b5177b 100644 --- a/crates/eidos-gui/src/update.rs +++ b/crates/eidos-gui/src/update.rs @@ -439,6 +439,9 @@ fn collection_on_worker( }; state.validate_revision(&rev.slug, rev.revision_number, &rev.game_domain)?; + // Before anything is reserved: members another revision of this collection + // already installed keep their folders instead of colliding with them. + let leftovers = eidos_collections::driver::adopt_other_revisions(inst, c, &mut state)?; let total = c.mods.len().max(1); let mut say = |_line: String| {}; @@ -477,6 +480,7 @@ fn collection_on_worker( ), }); } + report.deferred.extend(leftovers); if report.aborted { return Err(report.render()); } diff --git a/crates/eidos/src/collection.rs b/crates/eidos/src/collection.rs index 4917c49..23fc82c 100644 --- a/crates/eidos/src/collection.rs +++ b/crates/eidos/src/collection.rs @@ -185,6 +185,16 @@ pub(crate) fn cmd_collection(args: &[String]) { } }; + // Before anything is reserved: members another revision of this collection + // already installed keep their folders instead of colliding with them. + let leftovers = match driver::adopt_other_revisions(&inst, c, &mut state) { + Ok(l) => l, + Err(e) => { + eidos_log::warn!("eidos collection: {e}"); + exit(1); + } + }; + let mut say = |line: String| println!(" {line}"); let mut hooks = driver::RealHooks { nexus: &nexus, @@ -207,6 +217,7 @@ pub(crate) fn cmd_collection(args: &[String]) { detail: format!("installed as \"{folder}\", because a mod of yours already had that name"), }); } + report.deferred.extend(leftovers); if report.aborted { eidos_log::warn!("{}", report.render()); diff --git a/docs/guide/usage.md b/docs/guide/usage.md index 8e6e65e..9078e81 100644 --- a/docs/guide/usage.md +++ b/docs/guide/usage.md @@ -138,6 +138,12 @@ renamed into place, so an interruption leaves the old record or the new one and never half of either; if it ever cannot be read, Eidos stops and says so rather than quietly starting the whole collection over. Run the same command again. +Installing a newer revision of a collection you already have takes over the +previous revision's folders: an unchanged member is verified in place, a +member whose file the author updated is replaced inside its old folder, and a +member the new revision dropped is left installed and named in the report for +you to disable or remove. + ### What it will not pretend **A free Nexus account cannot fetch the member mods.** Nexus does not mint From 2410c586e1a942dde5e29a1b2d64ee60907df9d8 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:00:10 +0200 Subject: [PATCH 10/30] fix(gui): stop every mod toggle from walking every mod's files twice Each toggle or reorder reruns diagnostics on the UI thread, which built two throwaway indexed LayerStacks (plugin visibility and the SKSE scan), each a full recursive walk of every enabled mod. Unmanaged DLC rows (FILE paths) were also passed as layers, so the walk failed after finishing and every plugin fell back to a per-layer scan. Add LayerStack::new_unindexed for one-shot stacks, answer plugin visibility from one merged root listing, and drop unmanaged rows. --- crates/eidos-core/src/lib.rs | 41 ++++++++++++++++++++++ crates/eidos-gamefeatures/src/preflight.rs | 8 +++-- crates/eidos-gui/src/modinfo.rs | 28 ++++++++++----- 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/crates/eidos-core/src/lib.rs b/crates/eidos-core/src/lib.rs index fb7e9b2..cf9f490 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 @@ -2053,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(); diff --git a/crates/eidos-gamefeatures/src/preflight.rs b/crates/eidos-gamefeatures/src/preflight.rs index ad25e77..99a71c7 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/modinfo.rs b/crates/eidos-gui/src/modinfo.rs index 3510efd..ca5d911 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,9 +2566,13 @@ 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()) }); } From 134bc64fa04282390ea16540c4111cc6faff162e Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:03:44 +0200 Subject: [PATCH 11/30] fix(gui): stop the Data tab filter freezing on large mod lists A filter walks the whole merged tree, and each entry's provider was found by a starts_with scan over every enabled mod: O(entries x mods), about 4.5 s of frozen UI at 347 mods, repeated after every mod toggle. Every redraw then deep-cloned each cached directory listing again. The layer roots now sit in a map built once per view generation beside the LayerStack, looked up by walking the path's ancestors (same longest-prefix rule), and listings are shared through an Rc; non-matching files are skipped before they are cloned. --- crates/eidos-gui/src/main.rs | 35 +++++++++-- crates/eidos-gui/src/view.rs | 111 +++++++++++++++++++++-------------- 2 files changed, 97 insertions(+), 49 deletions(-) diff --git a/crates/eidos-gui/src/main.rs b/crates/eidos-gui/src/main.rs index 7cbfe00..a1e359c 100644 --- a/crates/eidos-gui/src/main.rs +++ b/crates/eidos-gui/src/main.rs @@ -1750,6 +1750,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, @@ -2235,7 +2244,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` @@ -2257,7 +2266,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. @@ -4241,7 +4250,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, @@ -4249,7 +4258,7 @@ mod tests { size: None, mtime: None, conflicted: false, - }], + }]), ), ); app.listing_cache.borrow_mut().insert( @@ -10356,6 +10365,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/view.rs b/crates/eidos-gui/src/view.rs index 7aedea4..321c7e2 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); } } From 7a174519b742090e68047a8fc0292a62f8a384a1 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:07:04 +0200 Subject: [PATCH 12/30] fix(instance): delete modlist.txt instead of saving it empty Removing the last mods deleted modlist.txt, but the window saves right after, and save_modlist wrote an empty file back. An empty list next to the next installed folder reads as truncated, so that install, every later save and the launch were refused until the file was deleted by hand. save_modlist now removes the file (and the legacy flat one it shadowed) when there are no rows, so "no order yet" is always an absent file. --- crates/eidos-instance/src/profile/modlist.rs | 15 +++++++++++++++ crates/eidos-instance/src/profile/tests.rs | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/crates/eidos-instance/src/profile/modlist.rs b/crates/eidos-instance/src/profile/modlist.rs index bc80281..0cf6dc6 100644 --- a/crates/eidos-instance/src/profile/modlist.rs +++ b/crates/eidos-instance/src/profile/modlist.rs @@ -357,6 +357,21 @@ 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, `eidos install` and `eidos collection` all write this file. // Every one of them takes the instance lock first, so the writes are diff --git a/crates/eidos-instance/src/profile/tests.rs b/crates/eidos-instance/src/profile/tests.rs index bef9233..5a5823a 100644 --- a/crates/eidos-instance/src/profile/tests.rs +++ b/crates/eidos-instance/src/profile/tests.rs @@ -757,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, From b4056481442c0b5476270df913b55a0d1f881433 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:11:37 +0200 Subject: [PATCH 13/30] fix(gui): check the mod list is current before renaming or creating a mod folder Rename, Overwrite-into-mod, Add separator and New mod change mods/ first and save after. When another process had rewritten modlist.txt, the new stale check refused that save only after the folder moved, and the reload appended the renamed or new mod DISABLED at the top while the status claimed success. The check now runs before the folder changes, and the success status is set before the save so a refusal is no longer hidden. --- crates/eidos-gui/src/main.rs | 37 ++++++++++++++++++++++++++++++++++ crates/eidos-gui/src/state.rs | 29 ++++++++++++++++++++++---- crates/eidos-gui/src/update.rs | 36 +++++++++++++++++++++++++++++---- 3 files changed, 94 insertions(+), 8 deletions(-) diff --git a/crates/eidos-gui/src/main.rs b/crates/eidos-gui/src/main.rs index a1e359c..b75b507 100644 --- a/crates/eidos-gui/src/main.rs +++ b/crates/eidos-gui/src/main.rs @@ -7493,6 +7493,43 @@ mod tests { 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 renaming_a_mod_leaves_it_where_it_was_and_still_enabled() { // The defect, exactly as it was reported: rename a mod and it is diff --git a/crates/eidos-gui/src/state.rs b/crates/eidos-gui/src/state.rs index 3fb03d7..15fb6a2 100644 --- a/crates/eidos-gui/src/state.rs +++ b/crates/eidos-gui/src/state.rs @@ -2313,6 +2313,25 @@ fn modlist_unchanged(app: &App, inst: &Instance) -> bool { .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. @@ -2330,10 +2349,7 @@ pub(crate) fn save_mods(app: &App) -> Option { // 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( - "Not saved: another Eidos process changed the mod list. It has been reloaded - redo the change." - .to_string(), - ); + 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}")); @@ -2691,6 +2707,11 @@ pub(crate) fn mods_changed(app: &mut App) { // view to it. reload_mods(app); } + mod_views_changed(app); +} + +/// 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 1b5177b..ed4b709 100644 --- a/crates/eidos-gui/src/update.rs +++ b/crates/eidos-gui/src/update.rs @@ -2080,7 +2080,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(); @@ -2094,6 +2095,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 @@ -2107,12 +2114,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}")); @@ -2541,7 +2550,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(); }; @@ -2552,6 +2565,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) { @@ -2573,8 +2589,10 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { app.collapsed.insert(typed.clone()); save_collapsed(app); } - mods_changed(app); + // Before mods_changed, which replaces it with + // the reason if the save is refused. app.status = Some(format!("Renamed to '{typed}'.")); + mods_changed(app); } Err(e) => app.status = Some(format!("Rename failed: {e}")), } @@ -2585,6 +2603,12 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { } Message::AddSeparator(i) => { app.menu_mod = None; + // Before the folder exists, as Rename does: a refused save after it + // reloads the list, and the rename editor opened below would then + // sit on whichever unrelated mod took this index. + 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". @@ -4885,6 +4909,10 @@ 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 before the folder is created. + 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; From 384513a71ffcb047ced2a463b2ed173114a718e4 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:12:05 +0200 Subject: [PATCH 14/30] fix(gui): re-read the load order after the INI editor saves Morrowind.ini For Morrowind the plugin state hash covers the profile's Morrowind.ini, which the window's own INI editor writes. The save left the cached load order and its hash stale, so the next plugin click or LOOT sort was refused as "another Eidos process". Saving a file inside the profile's plugin state directory now invalidates the plugin list, which re-reads both. --- crates/eidos-gui/src/main.rs | 41 ++++++++++++++++++++++++++++++++++ crates/eidos-gui/src/update.rs | 8 +++++++ 2 files changed, 49 insertions(+) diff --git a/crates/eidos-gui/src/main.rs b/crates/eidos-gui/src/main.rs index b75b507..2356912 100644 --- a/crates/eidos-gui/src/main.rs +++ b/crates/eidos-gui/src/main.rs @@ -6124,6 +6124,47 @@ mod tests { 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(); + } + /// An instance with two profiles and a save in the active one. fn saves_app() -> (App, PathBuf) { let root = temp_portable("skyrimse"); diff --git a/crates/eidos-gui/src/update.rs b/crates/eidos-gui/src/update.rs index ed4b709..eeb1d2d 100644 --- a/crates/eidos-gui/src/update.rs +++ b/crates/eidos-gui/src/update.rs @@ -3541,6 +3541,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)), } From f5585ffb89ba3775dddaa5bac637f1c1ff494a92 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:16:12 +0200 Subject: [PATCH 15/30] fix(collections): let an older revision take its folders back After a newer revision took over a collection's folders, re-running the older revision skipped every key it already recorded, so each member failed verification and every folder was listed as one it does not use. A key recorded in the same folder the other revision holds is now transferred back, and a folder this revision records is never reported as unused. --- crates/eidos-collections/src/driver.rs | 107 +++++++++++++++++-------- 1 file changed, 72 insertions(+), 35 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index 0964552..cad2c5e 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -261,7 +261,8 @@ fn reserve_folder( /// same folder, keeping its place in the mod list. /// /// Repeatable: a folder already carrying THIS revision's marker is taken again, -/// so a crash before the new state is saved loses nothing. +/// so a crash before the new state is saved loses nothing. Symmetric too: going +/// back to an older revision takes its folders back from the newer one. /// /// Returns the folders another revision still owns that this one does not use. /// They stay installed and enabled - removing a mod is the user's call - so the @@ -330,8 +331,11 @@ pub fn adopt_other_revisions( } for (key, other_key) in pairs { let folder = &folders[&other_key]; - if state.folders.contains_key(&key) - || state.folders.values().any(|f| f.eq_ignore_ascii_case(folder)) + // 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. + 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; @@ -351,7 +355,9 @@ pub fn adopt_other_revisions( state.folders.insert(key, folder.clone()); } 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)) && owns_folder(&mods.join(folder), &marker(&other_owner, key)) { leftovers.push(Note { @@ -1480,63 +1486,94 @@ mod cache_tests { std::fs::remove_dir_all(root).unwrap(); } - #[test] - fn a_new_revision_takes_over_its_previous_folders_instead_of_duplicating_them() { + 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-adopt-{}", std::process::id())); + 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()); - let domain = "skyrimspecialedition"; - let (old_owner, new_owner) = (format!("{domain}:gate:1"), format!("{domain}:gate:2")); - let mark = |owner: &str, key: &str| serde_json::to_string(&[owner, key]).unwrap(); - let key = |file: u64| format!("file:{domain}:{file}"); - let manifest = |mods: &[(&str, u64, u64)]| { - let mods: Vec = mods - .iter() - .map(|(name, m, f)| format!(r#"{{"name":"{name}","source":{{"type":"nexus","modId":{m},"fileId":{f}}}}}"#)) - .collect(); - format!(r#"{{"info":{{"name":"Gate","domainName":"{domain}"}},"mods":[{}]}}"#, mods.join(",")) - }; - // Revision 1, installed: one member stays, one gets a new file of the - // same page, one is dropped by revision 2. - let old_dir = crate::state::revision_dir(&root, "gate", 1); - std::fs::create_dir_all(&old_dir).unwrap(); - std::fs::write(old_dir.join("collection.json"), manifest(&[("Kept", 1, 10), ("Updated", 2, 20), ("Dropped", 3, 30)])).unwrap(); - std::fs::write(cache_marker(&old_dir), eidos_nexus::md5_file(&old_dir.join("collection.json")).unwrap()).unwrap(); - let mut old = InstallState { slug: "gate".into(), revision: 1, game_domain: domain.into(), ..Default::default() }; + 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(&old_owner, &key(file))); + 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 receipt = inst.mods_dir().join("Kept/.eidos-collection-recipe.json"); - let body = serde_json::json!({"schema":1,"owner":mark(&old_owner, &key(10)),"recipe":null,"files":{},"exclusions":{}}); - std::fs::write(&receipt, body.to_string()).unwrap(); + 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(); + let new = gate_manifest(&root, 2, &[("Kept", 1, 10), ("Updated", 2, 21), ("Added", 4, 40)]); + (root, inst, new) + } - let new = crate::read(&manifest(&[("Kept", 1, 10), ("Updated", 2, 21), ("Added", 4, 40)])).unwrap().collection; - let fresh = || InstallState { slug: "gate".into(), revision: 2, game_domain: domain.into(), ..Default::default() }; - let mut state = fresh(); + #[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 leftovers = adopt_other_revisions(&inst, &new, &mut state).unwrap(); 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(&new_owner, &key(21)))); + 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(&new_owner, &key(10))); + 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(&new_owner, &key(10)), state.folders.get(&key(10)).map(String::as_str)).unwrap(), "Kept"); + 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 = fresh(); + let mut again = gate_state(2); assert_eq!(adopt_other_revisions(&inst, &new, &mut again).unwrap(), leftovers); assert_eq!(again.folders, state.folders); 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).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).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(); + } } From 310191fecfd41c470f067793947860ba96d15561 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:17:18 +0200 Subject: [PATCH 16/30] fix(collections): carry an unchanged member's status to the next revision Adoption seeded only the folder, so an unchanged member started Pending and was recovered as Approximate ("interrupted status save") unless it had clone hashes: nearly every revision update read as not faithful, on every run. A key-matched member now takes the other revision's Installed or Approximate status with its folder, still verified before it is trusted, and is registered in the active profile as the recovery path did. --- crates/eidos-collections/src/driver.rs | 41 ++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index cad2c5e..a0832e4 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -256,9 +256,10 @@ fn reserve_folder( /// 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. The receipt moves with the folder, so an -/// unchanged member verifies in place and a changed one is replaced inside the -/// same folder, keeping its place in the mod list. +/// to the old archive and owner. 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, /// so a crash before the new state is saved loses nothing. Symmetric too: going @@ -352,6 +353,26 @@ pub fn adopt_other_revisions( meta.set("eidosCollectionOwner", &to); meta.write(&path.join("meta.ini")) .map_err(|e| e.to_string())?; + // 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. 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 key == other_key && state.status(&key).is_open() { + let seeded = match other.status(&other_key) { + Status::Installed(_) => Status::Installed(folder.clone()), + Status::Approximate(_, why) => Status::Approximate(folder.clone(), why.clone()), + _ => Status::Pending, + }; + if seeded != Status::Pending { + inst.register_installed_mod(folder) + .map_err(|e| e.to_string())?; + state.set(&key, seeded); + } + } state.folders.insert(key, folder.clone()); } for (key, folder) in &folders { @@ -1561,6 +1582,20 @@ mod cache_tests { 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"); + let mut state = gate_state(2); + adopt_other_revisions(&inst, &new, &mut state).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 it. + assert!(inst.modlist().iter().any(|m| m.name == "Kept" && m.enabled)); + 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"); From 129e3d79e9b0f77a8e63eda22bdeab7ec83c4636 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:17:51 +0200 Subject: [PATCH 17/30] fix(collections): carry the INI Tweaks folder over to the next revision Adoption only paired member keys, so the " - INI Tweaks" folder was never handed over: the new revision installed it again as "(2)" and the old one was reported as unused. When the new revision still ships INI Tweaks, its aux:ini-tweaks key is now paired like a member's, so apply_ini_tweaks reuses the same folder. --- crates/eidos-collections/src/driver.rs | 52 ++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index a0832e4..79171ae 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -299,6 +299,15 @@ pub fn adopt_other_revisions( .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 INI Tweaks at all. + if folders.contains_key(INI_TWEAKS_KEY) + && ini_tweaks_source(&revision_dir(&inst.root, &state.slug, state.revision)) + .is_ok_and(|src| src.is_some()) + { + 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; @@ -1385,6 +1394,18 @@ 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")) + })) +} + /// Copy the collection's INI fragments in as a mod of their own. pub fn apply_ini_tweaks( inst: &Instance, @@ -1395,13 +1416,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())?; @@ -1421,7 +1436,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)); @@ -1596,6 +1611,27 @@ mod cache_tests { 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(crate::state::revision_dir(&root, "gate", 2).join("INI Tweaks")).unwrap(); + let mut state = gate_state(2); + let leftovers = adopt_other_revisions(&inst, &new, &mut state).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:?}"); + 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"); From 576912b7834ac151015db317319eae4b52a61aea Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:19:40 +0200 Subject: [PATCH 18/30] fix(collections): take over a previous revision only once the install can go ahead Both callers adopted the previous revision's folders before install::run, so a revision stopped by the recipe check or a game runtime mismatch had already rewritten every marker without saving any state: the revision the user stayed on could no longer verify its members. Adoption is now a Hooks step run after both gates, and it saves the claimed folders before any marker moves. --- crates/eidos-collections/src/driver.rs | 82 ++++++++++++++++++------- crates/eidos-collections/src/install.rs | 60 ++++++++++++++++++ crates/eidos-gui/src/update.rs | 12 ++-- crates/eidos/src/collection.rs | 11 ---- 4 files changed, 127 insertions(+), 38 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index 79171ae..48472d8 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -243,7 +243,11 @@ fn reserve_folder( } /// Hand the folders another revision of this collection owns to the revision -/// `state` installs, before it starts. +/// `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 @@ -262,8 +266,9 @@ fn reserve_folder( /// the mod list. /// /// Repeatable: a folder already carrying THIS revision's marker is taken again, -/// so a crash before the new state is saved loses nothing. Symmetric too: going -/// back to an older revision takes its folders back from the newer one. +/// 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. /// /// Returns the folders another revision still owns that this one does not use. /// They stay installed and enabled - removing a mod is the user's call - so the @@ -272,6 +277,7 @@ 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 @@ -339,6 +345,10 @@ pub fn adopt_other_revisions( } } } + // 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(); for (key, other_key) in pairs { let folder = &folders[&other_key]; // A key this revision already records in the SAME folder is taken @@ -357,32 +367,42 @@ pub fn adopt_other_revisions( { continue; } - 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())?; // 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. 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. + // An updated member stays Pending: it is replaced anyway. + let mut seeded = false; if key == other_key && state.status(&key).is_open() { - let seeded = match other.status(&other_key) { + 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 seeded != Status::Pending { - inst.register_installed_mod(folder) - .map_err(|e| e.to_string())?; - state.set(&key, seeded); + seeded = status != Status::Pending; + if seeded { + state.set(&key, status); } } state.folders.insert(key, folder.clone()); + moves.push((path, from, to, folder.clone(), seeded)); + } + if !moves.is_empty() { + save(state)?; + } + for (path, from, to, folder, seeded) 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 seeded { + 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. @@ -465,6 +485,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) { @@ -1577,7 +1605,15 @@ mod cache_tests { 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 leftovers = adopt_other_revisions(&inst, &new, &mut state).unwrap(); + 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); @@ -1592,7 +1628,7 @@ mod cache_tests { 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).unwrap(), leftovers); + 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(); } @@ -1602,7 +1638,7 @@ mod cache_tests { use crate::state::Status; let (root, inst, new) = gate("status"); let mut state = gate_state(2); - adopt_other_revisions(&inst, &new, &mut state).unwrap(); + 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); @@ -1625,7 +1661,7 @@ mod cache_tests { old.save(&path).unwrap(); std::fs::create_dir_all(crate::state::revision_dir(&root, "gate", 2).join("INI Tweaks")).unwrap(); let mut state = gate_state(2); - let leftovers = adopt_other_revisions(&inst, &new, &mut state).unwrap(); + 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:?}"); @@ -1636,11 +1672,11 @@ mod cache_tests { 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).unwrap(); + 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).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. diff --git a/crates/eidos-collections/src/install.rs b/crates/eidos-collections/src/install.rs index 0418f1c..a8e0c5f 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-gui/src/update.rs b/crates/eidos-gui/src/update.rs index eeb1d2d..b973ffe 100644 --- a/crates/eidos-gui/src/update.rs +++ b/crates/eidos-gui/src/update.rs @@ -439,9 +439,6 @@ fn collection_on_worker( }; state.validate_revision(&rev.slug, rev.revision_number, &rev.game_domain)?; - // Before anything is reserved: members another revision of this collection - // already installed keep their folders instead of colliding with them. - let leftovers = eidos_collections::driver::adopt_other_revisions(inst, c, &mut state)?; let total = c.mods.len().max(1); let mut say = |_line: String| {}; @@ -480,7 +477,6 @@ fn collection_on_worker( ), }); } - report.deferred.extend(leftovers); if report.aborted { return Err(report.render()); } @@ -521,6 +517,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, diff --git a/crates/eidos/src/collection.rs b/crates/eidos/src/collection.rs index 23fc82c..4917c49 100644 --- a/crates/eidos/src/collection.rs +++ b/crates/eidos/src/collection.rs @@ -185,16 +185,6 @@ pub(crate) fn cmd_collection(args: &[String]) { } }; - // Before anything is reserved: members another revision of this collection - // already installed keep their folders instead of colliding with them. - let leftovers = match driver::adopt_other_revisions(&inst, c, &mut state) { - Ok(l) => l, - Err(e) => { - eidos_log::warn!("eidos collection: {e}"); - exit(1); - } - }; - let mut say = |line: String| println!(" {line}"); let mut hooks = driver::RealHooks { nexus: &nexus, @@ -217,7 +207,6 @@ pub(crate) fn cmd_collection(args: &[String]) { detail: format!("installed as \"{folder}\", because a mod of yours already had that name"), }); } - report.deferred.extend(leftovers); if report.aborted { eidos_log::warn!("{}", report.render()); From dd032d2fca4130208570e084def00f4c85778283 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:20:03 +0200 Subject: [PATCH 19/30] fix(collections): do not take over a member the user skipped in the new revision --no-optional marks optional members Skipped before the install starts, and adoption then handed them their previous folder anyway: owned by the new revision, still enabled, left out of the ordering pass and of the leftover list, and reported only as skipped. A Skipped member is no longer adopted, so its old folder keeps its marker and is named as unused. --- crates/eidos-collections/src/driver.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index 48472d8..b6f5e21 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -354,7 +354,11 @@ pub fn adopt_other_revisions( // 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 as unused below. if state.folders.get(&key).is_some_and(|f| f != folder) + || matches!(state.status(&key), Status::Skipped) || 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()) { @@ -1668,6 +1672,19 @@ mod cache_tests { 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)))); + assert!(leftovers.iter().any(|n| n.subject == "Kept"), "{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"); From 064dc9bf775064e35a9b3d1392c66e24e8a5deb9 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:20:51 +0200 Subject: [PATCH 20/30] fix(collections): leave a folder another profile enables to that profile mods/ is shared by every profile, so adopting a folder another profile enables replaced an updated member's files under that profile, which then loaded the new revision's files with its old order and settings. Adoption now skips any folder a profile other than the active one enables, the new revision installs its own copy as before, and the leftover note says why. --- crates/eidos-collections/src/driver.rs | 45 ++++++++++++++++++++++---- docs/guide/usage.md | 4 ++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index b6f5e21..bb06f54 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -260,10 +260,10 @@ fn reserve_folder( /// 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. 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. +/// 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 @@ -290,6 +290,18 @@ pub fn adopt_other_revisions( 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. + let active = inst.active_profile(); + let elsewhere: std::collections::HashSet = inst + .profiles() + .into_iter() + .filter(|p| *p != active) + .flat_map(|p| inst.profile(&p).modlist()) + .filter(|m| m.is_active()) + .map(|m| m.name) + .collect(); let mut leftovers = Vec::new(); for other in other_revisions(inst, state)? { let other_owner = format!("{}:{}:{}", other.game_domain, other.slug, other.revision); @@ -359,6 +371,7 @@ pub fn adopt_other_revisions( // reported only as skipped. Left alone it is named as unused below. if state.folders.get(&key).is_some_and(|f| f != folder) || matches!(state.status(&key), Status::Skipped) + || elsewhere.contains(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()) { @@ -417,8 +430,13 @@ pub fn adopt_other_revisions( leftovers.push(Note { subject: folder.clone(), detail: format!( - "revision {} of this collection installed it and this revision does not use it; it is still enabled, so disable or remove it yourself", - other.revision + "revision {} of this collection installed it and this revision does not use it{}; it is still enabled, so disable or remove it yourself", + other.revision, + if elsewhere.contains(folder) { + " (another profile enables it, so it was not taken over)" + } else { + "" + } ), }); } @@ -1685,6 +1703,21 @@ mod cache_tests { 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("Default").dir()).unwrap(); + 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); + 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); + 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 going_back_to_the_previous_revision_takes_its_folders_back() { let (root, inst, new) = gate("rollback"); diff --git a/docs/guide/usage.md b/docs/guide/usage.md index 9078e81..91a990a 100644 --- a/docs/guide/usage.md +++ b/docs/guide/usage.md @@ -142,7 +142,9 @@ Installing a newer revision of a collection you already have takes over the previous revision's folders: an unchanged member is verified in place, a member whose file the author updated is replaced inside its old folder, and a member the new revision dropped is left installed and named in the report for -you to disable or remove. +you to disable or remove. A folder another profile enables is never taken +over, because the mods folder is shared and that profile would change under +you: the new revision installs its own copy beside it instead. ### What it will not pretend From 853197cea8e7720bb4c850f14f573160c67d8091 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:21:25 +0200 Subject: [PATCH 21/30] fix(collections): stop naming a leftover folder once the user disabled it The leftover note tells the user to disable or remove a folder the new revision does not use, and it keeps the install short of faithful, but it only checked the old marker: disabling the mod changed nothing, so the note and the window's error came back on every run. A leftover is now named only while the active profile still enables it. --- crates/eidos-collections/src/driver.rs | 34 ++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index bb06f54..264c83e 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -270,9 +270,10 @@ fn reserve_folder( /// any point leaves folders either revision can take. Symmetric too: going back /// to an older revision takes its folders back from the newer one. /// -/// Returns the folders another revision still owns that this one does not use. -/// They stay installed and enabled - removing a mod is the user's call - so the -/// report has to name them. +/// 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. pub fn adopt_other_revisions( inst: &Instance, c: &Collection, @@ -302,6 +303,14 @@ pub fn adopt_other_revisions( .filter(|m| m.is_active()) .map(|m| m.name) .collect(); + // 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)? { let other_owner = format!("{}:{}:{}", other.game_domain, other.slug, other.revision); @@ -425,6 +434,7 @@ pub fn adopt_other_revisions( // 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)) { leftovers.push(Note { @@ -1619,6 +1629,8 @@ mod cache_tests { 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) } @@ -1659,12 +1671,14 @@ mod cache_tests { 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 it. + // 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(); } @@ -1706,7 +1720,6 @@ mod cache_tests { #[test] fn a_folder_another_profile_enables_is_not_taken_over() { let (root, inst, new) = gate("profiles"); - std::fs::create_dir_all(inst.profile("Default").dir()).unwrap(); 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); @@ -1718,6 +1731,17 @@ mod cache_tests { 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"); From 98fce7abd2048faad52f4d0afd8264ba74460241 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:39:30 +0200 Subject: [PATCH 22/30] fix(collections): fall back to a fresh copy when a revision cannot take a folder back Going back to an older revision kept its claim on a folder the newer one had taken over even when the take-back was refused (another profile enables it, or it holds installer answers). The folder carries the newer marker, so the member failed verification on every run and never installed again. The claim is now dropped and the member reset to Pending, so it installs a fresh copy beside the kept folder. The leftover note no longer tells the user to remove a folder another profile enables (removing it strips it from that profile too); it names that profile and asks only to disable it here. A member skipped with --no-optional, or kept for its installer answers, is described as such instead of as one this revision does not use. Adoption now registers every member whose status is Installed or Approximate in its folder on each pass, so a run stopped part-way no longer leaves seeded members verified but disabled in the profile. An unreadable record of another revision is skipped with a note instead of aborting the install with advice meant for the current revision. --- crates/eidos-collections/src/driver.rs | 186 +++++++++++++++++++------ docs/guide/usage.md | 5 +- 2 files changed, 151 insertions(+), 40 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index 264c83e..6d0173d 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -268,12 +268,15 @@ fn reserve_folder( /// 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. +/// 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. +/// 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, @@ -294,15 +297,14 @@ pub fn adopt_other_revisions( // 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 elsewhere: std::collections::HashSet = inst - .profiles() - .into_iter() - .filter(|p| *p != active) - .flat_map(|p| inst.profile(&p).modlist()) - .filter(|m| m.is_active()) - .map(|m| m.name) - .collect(); + 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 @@ -312,7 +314,7 @@ pub fn adopt_other_revisions( .map(|m| m.name) .collect(); let mut leftovers = Vec::new(); - for other in other_revisions(inst, state)? { + 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 { @@ -370,6 +372,10 @@ pub fn adopt_other_revisions( // 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 @@ -377,10 +383,12 @@ pub fn adopt_other_revisions( // 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 as unused below. + // 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) - || matches!(state.status(&key), Status::Skipped) - || elsewhere.contains(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()) { @@ -388,9 +396,26 @@ pub fn adopt_other_revisions( } 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)) - || installer_answers::path(&path).exists() - { + 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 @@ -398,25 +423,30 @@ pub fn adopt_other_revisions( // 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. - let mut seeded = false; 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, }; - seeded = status != Status::Pending; - if seeded { + 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(), seeded)); + moves.push((path, from, to, folder.clone(), register)); } - if !moves.is_empty() { + if !moves.is_empty() || dropped_claim { save(state)?; } - for (path, from, to, folder, seeded) in moves { + 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); @@ -425,7 +455,7 @@ pub fn adopt_other_revisions( // 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 seeded { + if register { inst.register_installed_mod(&folder) .map_err(|e| e.to_string())?; } @@ -437,18 +467,21 @@ pub fn adopt_other_revisions( && enabled_here.contains(folder) && owns_folder(&mods.join(folder), &marker(&other_owner, key)) { - leftovers.push(Note { - subject: folder.clone(), - detail: format!( - "revision {} of this collection installed it and this revision does not use it{}; it is still enabled, so disable or remove it yourself", + // 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, - if elsewhere.contains(folder) { - " (another profile enables it, so it was not taken over)" - } else { - "" - } + 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 }); } } } @@ -456,7 +489,15 @@ pub fn adopt_other_revisions( } /// Every other revision of this collection with a state file here, newest first. -fn other_revisions(inst: &Instance, state: &InstallState) -> Result, String> { +/// +/// 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, @@ -476,8 +517,19 @@ fn other_revisions(inst: &Instance, state: &InstallState) -> Result 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 @@ -1713,7 +1765,8 @@ mod cache_tests { 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)))); - assert!(leftovers.iter().any(|n| n.subject == "Kept"), "{leftovers:?}"); + // 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(); } @@ -1723,9 +1776,12 @@ mod cache_tests { 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); - adopt_other_revisions(&inst, &new, &mut state, &mut |_| Ok(())).unwrap(); + 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(); @@ -1757,4 +1813,56 @@ mod cache_tests { 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/docs/guide/usage.md b/docs/guide/usage.md index 91a990a..d085705 100644 --- a/docs/guide/usage.md +++ b/docs/guide/usage.md @@ -144,7 +144,10 @@ member whose file the author updated is replaced inside its old folder, and a member the new revision dropped is left installed and named in the report for you to disable or remove. A folder another profile enables is never taken over, because the mods folder is shared and that profile would change under -you: the new revision installs its own copy beside it instead. +you: the new revision installs its own copy beside it instead, and the report +asks you to disable the old one in this profile, never to remove it. Going back +to an older revision takes its folders back the same way, or installs fresh +copies of the ones it cannot take. ### What it will not pretend From 7e036e0c96c6078b4b4e0579cadaa398eb33ae1a Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:39:39 +0200 Subject: [PATCH 23/30] fix(collections): make an adopted INI Tweaks folder mirror the new revision A revision that took over the previous revision's INI Tweaks folder only copied its own fragments in, so fragments the author dropped stayed on disk and stayed selected in meta.ini, and were merged into the deployed INIs at every launch. Fragment files the revision does not ship are now removed from the owned folder and dropped from the selection. The folder is also no longer paired when the new revision's INI Tweaks directory holds no fragment files: nothing was installed then, and the old fragments stayed owned by the new revision with no note. --- crates/eidos-collections/src/driver.rs | 65 ++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/crates/eidos-collections/src/driver.rs b/crates/eidos-collections/src/driver.rs index 6d0173d..ad7a1e7 100644 --- a/crates/eidos-collections/src/driver.rs +++ b/crates/eidos-collections/src/driver.rs @@ -330,10 +330,10 @@ pub fn adopt_other_revisions( .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 INI Tweaks at all. + // 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) - && ini_tweaks_source(&revision_dir(&inst.root, &state.slug, state.revision)) - .is_ok_and(|src| src.is_some()) + && ships_ini_fragments(&revision_dir(&inst.root, &state.slug, state.revision)) { pairs.push((INI_TWEAKS_KEY.into(), INI_TWEAKS_KEY.into())); } @@ -1518,6 +1518,16 @@ fn ini_tweaks_source(dir: &Path) -> std::io::Result> { })) } +/// 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, @@ -1571,6 +1581,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() { @@ -1599,6 +1614,27 @@ 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. The + // folder is this collection's output, so it mirrors this revision. + for entry in std::fs::read_dir(&dest).map_err(|e| e.to_string())? { + let entry = entry.map_err(|e| e.to_string())?; + if entry.file_type().is_ok_and(|t| t.is_file()) && !shipped.contains(&entry.file_name()) { + std::fs::remove_file(entry.path()).map_err(|e| e.to_string())?; + } + } + let meta_path = inst.mods_dir().join(&name).join("meta.ini"); + let mut meta = eidos_instance::ModMeta::read(&meta_path); + let selected: Vec = meta + .ini_tweaks() + .iter() + .filter(|t| shipped.iter().any(|s| *s == t.as_str())) + .cloned() + .collect(); + if selected.len() != meta.ini_tweaks().len() { + meta.set_ini_tweaks(&selected); + 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") }); @@ -1747,12 +1783,33 @@ mod cache_tests { 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(crate::state::revision_dir(&root, "gate", 2).join("INI Tweaks")).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. + 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").exists()); + assert!(folder.join("Ini Tweaks/Kept.ini").is_file()); + assert_eq!(eidos_instance::ModMeta::read(&folder.join("meta.ini")).ini_tweaks(), ["Kept.ini"]); std::fs::remove_dir_all(root).unwrap(); } From cfe2d984dfbac1efce6d0b15df9a340bff5e3756 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:46:38 +0200 Subject: [PATCH 24/30] fix(gui): keep mod-list refusals visible and other profiles in step Renaming a mod renamed the shared folder but only the active profile's modlist.txt, so every other profile lost the mod's state and order and enough renames got it judged an unmounted drive. Instance::rename_mod now renames the line in every profile under the lock, as forget_mods does. Enable/disable selected, Enable all, Send to priority, Fix save mods and an aimed install set their success text after mods_changed and hid its refusal. mods_changed now says whether the save landed and they only report success then. Add separator and New mod check staleness under the lock and open the rename editor only after a saved list; Remove and batch Remove check staleness before deleting, so the refusal never covers a delete that already happened. The INI editor opened an unseeded Morrowind.ini empty and saving it founded a stub that every launch deployed. It now seeds from the game's real state first, and a save refuses to write over a file another process changed since it was opened. --- crates/eidos-gui/src/main.rs | 74 ++++++++ crates/eidos-gui/src/modinfo.rs | 14 +- crates/eidos-gui/src/state.rs | 29 ++- crates/eidos-gui/src/update.rs | 179 ++++++++++++++++--- crates/eidos-instance/src/lib.rs | 39 ++++ crates/eidos-instance/src/profile/modlist.rs | 35 ++++ 6 files changed, 332 insertions(+), 38 deletions(-) diff --git a/crates/eidos-gui/src/main.rs b/crates/eidos-gui/src/main.rs index 2356912..4b67761 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 @@ -6165,6 +6168,38 @@ mod tests { 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"); @@ -7571,6 +7606,37 @@ mod tests { 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 @@ -7596,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())); @@ -7629,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); } diff --git a/crates/eidos-gui/src/modinfo.rs b/crates/eidos-gui/src/modinfo.rs index ca5d911..e9d0336 100644 --- a/crates/eidos-gui/src/modinfo.rs +++ b/crates/eidos-gui/src/modinfo.rs @@ -4742,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). @@ -4761,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 @@ -4779,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 15fb6a2..3f81bbc 100644 --- a/crates/eidos-gui/src/state.rs +++ b/crates/eidos-gui/src/state.rs @@ -2364,13 +2364,26 @@ pub(crate) fn save_mods(app: &App) -> Option { /// 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 forgot = inst.forget_mods(names); + let result = edit(inst); if current { app.modlist_seen.set(Some(modlist_fingerprint(inst))); } - Some(forgot) + Some(result) } /// Invalidate every memoised view listing. Cheap: the listings rebuild lazily on @@ -2696,10 +2709,13 @@ 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); +/// 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 @@ -2708,6 +2724,7 @@ pub(crate) fn mods_changed(app: &mut App) { reload_mods(app); } mod_views_changed(app); + refused.is_none() } /// Everything derived from the mod list, recomputed after it changed. diff --git a/crates/eidos-gui/src/update.rs b/crates/eidos-gui/src/update.rs index b973ffe..46db3f3 100644 --- a/crates/eidos-gui/src/update.rs +++ b/crates/eidos-gui/src/update.rs @@ -1940,9 +1940,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. @@ -2475,6 +2476,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(()) => { @@ -2593,9 +2601,19 @@ 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(format!("Renamed to '{typed}'.")); + 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); } Err(e) => app.status = Some(format!("Rename failed: {e}")), @@ -2607,9 +2625,20 @@ pub(crate) fn update_inner(app: &mut App, message: Message) -> Task { } Message::AddSeparator(i) => { app.menu_mod = None; - // Before the folder exists, as Rename does: a refused save after it - // reloads the list, and the rename editor opened below would then - // sit on whichever unrelated mod took this index. + // 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(); } @@ -2640,11 +2669,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}")), } @@ -3474,8 +3507,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) => { @@ -3498,6 +3535,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) => { @@ -3528,6 +3569,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 @@ -3538,6 +3600,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!( @@ -4921,7 +4984,18 @@ 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 before the folder is created. + // 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(); } @@ -4952,12 +5026,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}")), } @@ -5880,7 +5956,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 @@ -5893,6 +5969,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.") @@ -6207,8 +6287,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 @@ -6245,13 +6329,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(); @@ -6280,6 +6365,10 @@ 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(); @@ -8380,6 +8469,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 @@ -8395,11 +8487,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-instance/src/lib.rs b/crates/eidos-instance/src/lib.rs index 77b0bc5..b812d61 100644 --- a/crates/eidos-instance/src/lib.rs +++ b/crates/eidos-instance/src/lib.rs @@ -1073,6 +1073,20 @@ impl Instance { 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() @@ -2845,6 +2859,31 @@ mod whole_audit_regressions { 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/profile/modlist.rs b/crates/eidos-instance/src/profile/modlist.rs index 0cf6dc6..bd30338 100644 --- a/crates/eidos-instance/src/profile/modlist.rs +++ b/crates/eidos-instance/src/profile/modlist.rs @@ -424,6 +424,41 @@ impl Profile { 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() { From ff89389e6b43340152e968ac0c4e0017da70ae1d Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:49:28 +0200 Subject: [PATCH 25/30] fix(saves): warn about hidden prefix saves once per change The warning naming prefix saves the profile bind hides repeated on every launch until the files were moved, burying itself in the log. The set it last reported is now recorded beside .seeded in the profile's saves, and the warning only comes again when that set changes; an empty set clears the record so the same names turning up later are reported again. --- crates/eidos/src/prepare.rs | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/eidos/src/prepare.rs b/crates/eidos/src/prepare.rs index 6bd5674..ffab313 100644 --- a/crates/eidos/src/prepare.rs +++ b/crates/eidos/src/prepare.rs @@ -579,6 +579,28 @@ fn hidden_prefix_saves(prof_saves: &std::path::Path, prefix_saves: &std::path::P .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. @@ -701,7 +723,7 @@ pub(crate) fn prepare_saves( ); } let hidden = hidden_prefix_saves(&prof.saves_dir(), &source); - if !hidden.is_empty() { + 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 \ @@ -1313,7 +1335,14 @@ mod rescue_tests { // 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); - assert_eq!(hidden_prefix_saves(&saves, &prefix), vec!["Deck.ess".to_string()]); + 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(); } From 0c3c141c044c4d1d91018e65babf4d24b4232435 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:49:28 +0200 Subject: [PATCH 26/30] fix(transfer): say how to pack without a linked downloads folder A downloads/ symlinked to another drive is often deliberate, yet every pack reported it as missing and exited 1 with no way out given, so a `pack && ...` script failed on every run. The warning now points at --no-downloads (or the window's Include downloads/ box), which makes it a deliberate exclusion that does not count as a loss. --- crates/eidos-transfer/src/transfer.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/eidos-transfer/src/transfer.rs b/crates/eidos-transfer/src/transfer.rs index e979042..a8a2778 100644 --- a/crates/eidos-transfer/src/transfer.rs +++ b/crates/eidos-transfer/src/transfer.rs @@ -98,6 +98,16 @@ fn left_out_warning(left: &[Left]) -> Option { ); 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)); @@ -855,6 +865,9 @@ mod tests { 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] From 06638f1387a44d4a3e873d341ca1cfbf2f2190c9 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Wed, 7 Oct 2026 22:49:28 +0200 Subject: [PATCH 27/30] docs(guide): document the mod list, profile saves and collection changes The guide did not say that unlisted folders now appear disabled at the highest priority, that Remove and Rename apply to every profile, that an emptied list leaves no modlist.txt, or that the window refuses and reloads an edit when another Eidos process changed the list. It also said nothing about per-profile saves and the hidden-saves warning, how collection leftovers and --no-optional skips are reported, what a failed takeover says, or how to pack without a linked downloads/. --- docs/guide/usage.md | 52 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/docs/guide/usage.md b/docs/guide/usage.md index d085705..c8da2cf 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-