From 11c5eeee3f9857b57eb08d2f2e02332b8998506d Mon Sep 17 00:00:00 2001 From: forkless Date: Tue, 2 Jun 2026 23:49:35 +0200 Subject: [PATCH] doc comments, governance, decisions, polish --- Cargo.lock | 12 +++--- DECISIONS.md | 112 ++++++++++++++++++++++++++++++++++++++++++++++++ GOVERNANCE.md | 12 +++++- KNOWN_ISSUES.md | 7 ++- src/guard.rs | 31 ++++++++++---- src/main.rs | 48 +++++++++++++-------- src/tui.rs | 28 ++++++++---- 7 files changed, 207 insertions(+), 43 deletions(-) create mode 100644 DECISIONS.md diff --git a/Cargo.lock b/Cargo.lock index 7217b91..45999c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -40,9 +40,9 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "bitflags" -version = "2.11.1" +version = "2.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" +checksum = "84d7ced0ae9557296835c32bf1b1e02b44c746701f898460fb000d7eaa84f00a" [[package]] name = "bumpalo" @@ -446,9 +446,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.30" +version = "0.4.31" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "616ec5685824bcc94416c6d4a7a446eea774a31efd7062c8480ba6fd06d7a6e5" +checksum = "113b30b4cd05f7c06868fdb2854f66a7b9fece9a48425351cd532e810d74024f" [[package]] name = "lru" @@ -895,9 +895,9 @@ checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" [[package]] name = "unicode-segmentation" -version = "1.13.2" +version = "1.13.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9629274872b2bfaf8d66f5f15725007f635594914870f65218920345aa11aa8c" +checksum = "c6f5d3c3b1bf09027a88a6bc961fc00497d651009560b5463668dc81b0fa87a8" [[package]] name = "unicode-truncate" diff --git a/DECISIONS.md b/DECISIONS.md new file mode 100644 index 0000000..6bfb792 --- /dev/null +++ b/DECISIONS.md @@ -0,0 +1,112 @@ +# Design Decisions + +This file captures the rationale behind significant architecture and format +choices, so the reasoning is preserved for future maintainers (including +yourself six months from now). + +--- + +## Sentinel File vs config.ini (v0.3.2) + +### Problem +`config.ini` persisted the save folder path to disk, including the user's +filesystem-username. This is a privacy concern — paths are visible next to +the binary. + +### Decision +Remove `config.ini` entirely. The save folder is session-only — set it each +time via **Set save folder**. The disclaimer acceptance is tracked via a +0-byte sentinel file (`NotAlterra_LICENSE_ACCEPTED`) alongside the binary. + +### Rationale + +**Privacy** — no paths written to disk. The save folder exists only in +memory while the tool runs. + +**Simplicity** — no config parsing, no INI format to maintain, no migration +code for renamed keys. + +**Sentinel, not config** — a 0-byte file communicates exactly one boolean +(disclaimer accepted). It cannot grow into a configuration file over time. +The format intentionally prevents scope creep. + +**What was removed:** +- `AppConfig` struct (save_path, ini_path, save_scan, disclaimer_accepted) +- `load_config()` / `save_config()` with INI parsing +- Cached `ini_path` — now derived from save folder at runtime +- Four integration tests for config round-trips + +--- + +## Manual Path Entry vs Auto-Discovery (v0.3.0) + +### Problem +Auto-discovery scanned user profiles and system directories for Subnautica 2 +save folders. This is a privacy concern — it traverses `/home/*` (Linux) and +`C:\Users\*` (Windows). + +### Decision +Replace full auto-discovery with manual path entry via **Set save folder**. +Keep a lightweight `quick_discover()` that checks only the current user's +default install paths at startup. + +### Rationale + +**Privacy** — no scanning of other users' profiles or system drives. + +**Current-user convenience** — `quick_discover()` checks 1 path on Windows, +3 paths on Linux, all within the current user's own directories. Returns +the first match silently, no UI. If nothing is found, the user enters their +path manually. + +**Discovery module retained** — `validate_custom_path()` and +`derive_ini_path()` still live in `discovery.rs` for the manual entry flow. +The aggressive scan functions (`discover_save_folders()`, `scan_other_users()`, +`walk_for_subnautica()`) are removed. + +--- + +## tar.gz Backup Format (v0.4.0) + +### Problem +Directory-tree backups (`NotAlterra_Backups/notalterra_copy_/`) +are messy, uncompressed, and have no integrity guarantees. + +### Decision +One `tar.gz` archive per backup event, stored in `backups/saves/`. + +### Rationale + +**No vendor lock-in** — standard `tar -xzf` recovers data without the tool. +If NotAlterra stops working, the user's backups are still accessible with +standard system utilities. + +**Single file per event** — reduces clutter. One backup = one file, not +a directory tree with 15+ loose save files. + +**Compression** — save files compress well (~75MB → ~20MB). Reduces disk +usage without user effort. + +**Pure Rust implementation** — `tar` + `flate2` crates, 200M+ downloads +combined. No system dependencies, no external tools. + +**Per-entry restore** — extracting a single save file from the archive +does not require decompressing the entire archive. + +### Safeguards + +- **Atomic write**: backup written to `.tmp` file, then atomically renamed. + Power loss during backup discards a temp file, not a real backup. +- **Integrity check after creation**: archive is read back and validated + before reporting success. +- **SHA256 manifest**: a `MANIFEST` file inside each archive records the + hash of every contained save file. On restore, each extracted file is + verified against its expected hash — silent bit-rot detected before bad + data reaches the save folder. +- **Fuzz target**: round-trip fuzzing (create archive from diverse inputs → + restore → compare) catches logic bugs. + +### Migration +Existing `NotAlterra_Backups/` directory-tree backups are detected and +transparently imported on first run after upgrade. No manual migration +required. diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 74d6b0e..a9086e9 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -66,7 +66,8 @@ not published. The maintainer tests the draft binaries before publishing. Before signing a release tag, the maintainer verifies: -- [ ] `cargo test --workspace` — all tests pass +- [ ] Impact analysis completed — all call sites for new/changed functions identified and updated +- [ ] `cargo test --workspace` — all tests pass (including new integration tests for features shipped in this release) - [ ] `python3 tests/_check.py` — 100% doc coverage - [ ] CHANGELOG.md has an entry for the new version - [ ] `git status` — no uncommitted changes @@ -84,7 +85,14 @@ Planned changes for upcoming releases, ordered by priority. | Target | Item | |--------|------| -| v0.4.0 | TBD — see GitHub issues for planned features | +| v0.4.0 | Auto-remove stale `config.ini` from prior versions on first launch | +| v0.4.0 | Add `--help` flag | +| v0.4.0 | Restructure file layout: `backups/saves/` (tar.gz), `backups/config/` (.ini), `logs/transaction.log` | +| v0.4.0 | tar.gz backup format — one archive per backup event (all slots, not per-slot), pure Rust; standard `tar -xzf` recovers data without the tool (no vendor lock-in); safeguards: atomic write (`.tmp` → rename), integrity check after creation, per-entry restore without full decompress, per-file SHA256 manifest verified on restore | +| v0.4.0 | Migration path: detect and import old `NotAlterra_Backups/` directory-tree backups into new format | +| v0.4.0 | Add `backups/`, `logs/`, `NotAlterra_LICENSE_ACCEPTED` to `.gitignore` | +| v0.4.0 | Fuzz target for backup round-trip (create diverse save sets → archive → restore → verify integrity) | +| v0.4.0 | Unit + integration tests for every new feature (backup round-trip, migration, --help, config cleanup) | Items may shift between releases depending on feedback and urgency. diff --git a/KNOWN_ISSUES.md b/KNOWN_ISSUES.md index bbdd81b..b5ff209 100644 --- a/KNOWN_ISSUES.md +++ b/KNOWN_ISSUES.md @@ -1,3 +1,8 @@ # Known Issues -No known issues at this time. +## Stale config.ini from prior versions + +Users upgrading from v0.3.0 or earlier will have a `config.ini` file next to +the binary that no longer serves any function. It can be safely deleted. + +**Planned**: Auto-remove stale `config.ini` on first launch after upgrade. diff --git a/src/guard.rs b/src/guard.rs index 952fd23..5ff60d7 100644 --- a/src/guard.rs +++ b/src/guard.rs @@ -13,8 +13,11 @@ use std::path::{Path, PathBuf}; // ── process detection ────────────────────────────────────────────────────── /// Return `true` if Subnautica 2 appears to be running. -/// Check if Subnautica 2 is currently running (Windows). -/// Process detection is disabled — returns `false` to avoid AV false positives. +/// +/// Always returns `false` — process detection (`tasklist` / `pgrep`) is +/// intentionally disabled to avoid Windows Defender false positives +/// (Trojan:Win32/Wacatac.C!ml). Users are reminded to close the game +/// manually before using the tool. #[cfg(target_os = "windows")] pub fn game_running() -> bool { // Process detection disabled — avoid AV false-positives. @@ -34,7 +37,9 @@ fn _game_running_windows() -> bool { } /// Check if Subnautica 2 is currently running (Linux). -/// Process detection is disabled — always returns `false` to avoid AV false positives. +/// +/// Always returns `false` — process detection via `pgrep` is disabled to +/// avoid false positives. Users are reminded to close the game manually. #[cfg(not(target_os = "windows"))] pub fn game_running() -> bool { false @@ -61,7 +66,8 @@ fn _game_running_linux() -> bool { // ── transaction logging ──────────────────────────────────────────────────── -/// Path to transaction.log next to the binary. +/// Path to `transaction.log` alongside the binary. All timestamped actions +/// are appended here for audit trail purposes. pub fn log_path() -> PathBuf { exe_dir().join("transaction.log") } @@ -77,7 +83,11 @@ fn exe_dir() -> PathBuf { /// Maximum lines before rotation. const MAX_LOG_LINES: usize = 10_000; -/// Append a timestamped log entry. Auto-rotates if the log exceeds 10k lines. +/// Append a timestamped log entry to `transaction.log`. +/// +/// Format: `YYYY-MM-DD HH:MM:SS | ACTION | detail | result` +/// Auto-rotates if the log exceeds 10,000 lines — the oldest lines are +/// discarded, keeping only the most recent 10,000. pub fn log_action( action: &str, detail: &str, @@ -105,9 +115,11 @@ pub fn log_action( f.write_all(line.as_bytes())?; Ok(()) } -/// Truncate a filesystem path to start at `Subnautica2/` or `Subnautica2\` -/// for privacy-safe logging. Returns the original path if no truncation -/// is possible. +/// Truncate a filesystem path to start at `Subnautica2/` or `Subnautica2\`, +/// stripping the user-specific prefix for privacy-safe logging. +/// +/// Returns the original path unchanged if `Subnautica2` is not found in the +/// input (e.g. custom paths entered via `Set save folder`). pub fn sanitize_path(p: &str) -> String { let needle = "Subnautica2"; let sep = if p.contains('\\') { "\\" } else { "/" }; @@ -118,7 +130,8 @@ pub fn sanitize_path(p: &str) -> String { } } -/// Check whether a path looks like a network/UNC path (for warning purposes). +/// Check whether a path looks like a network/UNC path — for warning purposes. +/// Matches paths starting with `\\` (Windows) or `//` (Linux). pub fn is_network_path(p: &str) -> bool { p.starts_with("\\\\") || p.starts_with("//") } diff --git a/src/main.rs b/src/main.rs index 015e922..266d696 100644 --- a/src/main.rs +++ b/src/main.rs @@ -111,7 +111,9 @@ impl App { } } -/// Internal helper — see module-level documentation for context. +/// Return the directory containing the running executable. Used to locate +/// the sentinel file, backups directory, and `transaction.log` alongside +/// the binary. fn exe_dir() -> PathBuf { std::env::current_exe() .ok() @@ -119,7 +121,9 @@ fn exe_dir() -> PathBuf { .unwrap_or_else(|| PathBuf::from(".")) } -/// Internal helper — see module-level documentation for context. +/// Refresh the dashboard counters (live saves, backups, ini backup status) +/// shown in the header bar. Called after changing the save folder or after +/// any backup/restore operation. fn refresh_stats(tui_state: &mut tui::AppState, save_folder: Option<&Path>) { tui_state.save_path = save_folder.map(|p| p.display().to_string()); let backup_root = exe_dir().join("NotAlterra_Backups"); @@ -666,7 +670,7 @@ fn action_restore_backup(terminal: &mut Terminal, app: &mut App) // ── .ini submenu ─────────────────────────────────────────────────────────── -/// Internal helper — see module-level documentation for context. +/// Display the .ini management submenu with Backup, Restore, and Delete options. fn run_ini_submenu(terminal: &mut Terminal, app: &mut App) -> Result<()> { let ini_path = get_ini_path(terminal, app)?; let backup_root = app.backup_root(); @@ -719,7 +723,7 @@ fn run_ini_submenu(terminal: &mut Terminal, app: &mut App) -> Res Ok(()) } -/// Internal helper — see module-level documentation for context. +/// Back up all `.ini` files from the Config\Windows folder into a timestamped archive. fn ini_backup_action( terminal: &mut Terminal, app: &mut App, @@ -747,7 +751,8 @@ fn ini_backup_action( Ok(()) } -/// Internal helper — see module-level documentation for context. +/// Restore `.ini` files from a selected backup into the Config\Windows folder. +/// Creates a pre-restore safety copy of the current `.ini` files first. fn ini_restore_action( terminal: &mut Terminal, app: &mut App, @@ -811,7 +816,8 @@ fn ini_restore_action( Ok(()) } -/// Internal helper — see module-level documentation for context. +/// Delete all `.ini` files from the Config\Windows folder. +/// Refuses to proceed unless at least one `.ini` backup exists in the backup root. fn ini_delete_action( terminal: &mut Terminal, app: &mut App, @@ -956,7 +962,9 @@ fn action_inspect_saves(terminal: &mut Terminal, app: &mut App) - // ── helpers ──────────────────────────────────────────────────────────────── -/// Internal helper — see module-level documentation for context. +/// Poll for a single key event with a timeout in milliseconds. +/// Returns `None` if no key is pressed within the timeout. Filters out +/// `KeyEventKind::Release` events to avoid double-firing on held keys. fn poll_key(timeout_ms: u64) -> Result> { if event::poll(std::time::Duration::from_millis(timeout_ms))? { if let Event::Key(key) = event::read()? { @@ -999,10 +1007,9 @@ fn slot_number(slot: &str) -> String { .unwrap_or_else(|| slot.to_string()) } -/// Internal helper — see module-level documentation for context. -/// Internal helper — see module-level documentation for context. -/// Display an informational dialog with a single OK button. -/// Display a dialog with styled content lines (colors, bold). +/// Display a dialog with styled content lines (colors, bold). Accepts +/// `Line` slices — use for metadata displays, help text, or any content +/// that needs inline formatting. Press Enter or Space to dismiss. fn ok_dialog_styled(terminal: &mut Terminal, app: &App, title: &str, lines: &[Line]) -> Result<()> { loop { terminal.draw(|f| tui::draw_ok_dialog_styled(f, &app.tui_state, title, lines))?; @@ -1014,8 +1021,9 @@ fn ok_dialog_styled(terminal: &mut Terminal, app: &App, title: &s } } -/// Internal helper — see module-level documentation for context. -/// Display an informational dialog with a single OK button. +/// Display a plain-text informational dialog with a single OK button. +/// `msg` supports newlines for multi-line messages. Press Enter or +/// Space to dismiss. For styled content, use `ok_dialog_styled`. fn ok_dialog(terminal: &mut Terminal, app: &App, title: &str, msg: &str) -> Result<()> { loop { terminal.draw(|f| tui::draw_ok_dialog(f, &app.tui_state, title, msg))?; @@ -1027,13 +1035,15 @@ fn ok_dialog(terminal: &mut Terminal, app: &App, title: &str, msg } } -/// Internal helper — see module-level documentation for context. +/// Check whether at least one backup directory exists in the backup root. +/// Used to gate destructive operations behind a backup requirement. fn has_existing_backup(app: &App) -> bool { let root = app.backup_root(); root.exists() && std::fs::read_dir(&root).is_ok_and(|mut d| d.any(|e| e.is_ok_and(|e| e.path().is_dir()))) } -/// Internal helper — see module-level documentation for context. +/// Gate: warn the user if no full backup exists yet. Returns `false` if +/// no backup is found (caller should abort the destructive operation). fn require_backup(terminal: &mut Terminal, app: &mut App) -> Result { if has_existing_backup(app) { return Ok(true); @@ -1043,7 +1053,9 @@ fn require_backup(terminal: &mut Terminal, app: &mut App) -> Resu Ok(false) } -/// Internal helper — see module-level documentation for context. +/// Derive the canonical `.sav` target filename from a `.bak` filename. +/// E.g. `savegame_0_9.bak` → `savegame_0.sav`. Falls back to replacing +/// `.bak` with `.sav` if the slot pattern is not recognized. fn derive_target_sav(bak_name: &str) -> String { crate::gvas::derive_slot_from_filename(bak_name) .map(|s| format!("{s}.sav")) @@ -1150,7 +1162,9 @@ fn fs_meta(path: &Path) -> Result { std::fs::metadata(path).map_err(|_| ()) } -/// Internal helper — see module-level documentation for context. +/// Format a playtime value in seconds to a human-readable string +/// (`HHh MMm` or `MMm`, zero-padded to 2 digits). Returns `—` for +/// `None` or sub-minute values. fn format_playtime(seconds: Option) -> String { match seconds { Some(s) if s >= 3600.0 => { diff --git a/src/tui.rs b/src/tui.rs index 5017b69..96c4ac7 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -209,8 +209,9 @@ pub fn draw_confirm_popup( draw_whale_separator(f, bar, app); } -/// Render a simple informational dialog with a message. -/// Shows title, body text, and an OK button. +/// Render an informational dialog with a plain-text message and OK button. +/// Auto-sizes to fit content. Title is displayed in cyan, message in gray, +/// whale separator at the bottom. Press Enter or Space to dismiss. pub fn draw_ok_dialog(f: &mut Frame, app: &AppState, title: &str, message: &str) { let content_w = message.lines().map(|l| l.len()).max().unwrap_or(20).max(title.len()) as u16 + 10; let popup_w = content_w.max(50).min(f.area().width.saturating_sub(4)); @@ -231,7 +232,9 @@ pub fn draw_ok_dialog(f: &mut Frame, app: &AppState, title: &str, message: &str) draw_whale_separator(f, bar, app); } -/// Render a dialog with styled content lines.\n/// Supports inline formatting (colors, bold) via [`Line`]. +/// Render a dialog with styled content lines. Supports inline formatting +/// (colors, bold) via [`Line`] slices. Use for metadata displays, help +/// text, or any content that needs per-span styling. pub fn draw_ok_dialog_styled(f: &mut Frame, app: &AppState, title: &str, lines: &[Line]) { let content_w = lines.iter().map(|l| l.width() as u16).max().unwrap_or(20).max(title.len() as u16) + 10; let popup_w = content_w.max(50).min(f.area().width.saturating_sub(4)); @@ -288,7 +291,9 @@ pub fn draw_sub_menu( draw_status_bar(f, chunks[3], app); } -/// Draw a simple text screen with a "press any key" prompt. +/// Draw a full-screen text display with a "press any key" prompt at the +/// bottom. Used for status messages during long operations (scanning, +/// backing up) and for displaying scan results. pub fn draw_text_screen( f: &mut Frame, app: &AppState, @@ -586,8 +591,10 @@ fn draw_status_bar(f: &mut Frame, area: Rect, app: &AppState) { draw_whale_separator(f, area, app); } -/// Separator line with a whale patrolling right-to-left. -/// Disappears for ~10 s after reaching the left edge. +/// Draw the bottom status bar with an animated whale patrolling right-to-left. +/// The whale moves one position every 180ms. After reaching the left edge, +/// it disappears for ~5.4s (30 cooldown ticks) before reappearing on the +/// right. Two variants alternate every 400ms. pub fn draw_whale_separator(f: &mut Frame, area: Rect, app: &AppState) { if area.width < 4 { return; } let elapsed = app.whale_start.elapsed().as_millis() as u64; @@ -801,8 +808,13 @@ pub fn draw_input_dialog( draw_whale_separator(f, bar, _app); } -/// Truncate a path to show the tail (most specific directories). -/// e.g. `C:\Users\...\Subnautica2\Saved\SaveGames` → `…\Subnautica2\Saved\SaveGames` +/// Truncate a filesystem path to show only the most specific directories. +/// Walks forward to the first path separator after the truncation point to +/// avoid splitting mid-component. +/// +/// Examples: +/// - `C:\Users\user\AppData\...\SaveGames` → `…\AppData\...\SaveGames` +/// - A short path that fits `max_width` is returned unchanged. fn truncate_path_tail(path: &str, max_width: usize) -> String { if path.len() <= max_width { return path.to_string();