From 2a70673a8d335bf284fc9f7297777d25c3eb99d9 Mon Sep 17 00:00:00 2001 From: Geoffrey Vancoetsem <10533139+geeooff@users.noreply.github.com> Date: Sat, 19 Sep 2026 02:03:30 +0200 Subject: [PATCH 1/5] Reload the configuration as it changes and show a fault in the icon The watcher used to read config.toml once, and a file it could not use made it exit with a code and no icon. Now `serve` loads the file itself and a supervisor on the worker thread runs one engine per usable configuration: a thread parked on the folder's change notification (`config::watch`, 250 ms settle, bytes compared) signals a reload, the engine stops through the handover path with the session left open in the marker, and the next engine resumes it -- a game in progress is not disturbed, and no new engine state was needed. A file that cannot be used freezes the program instead: `LoadError` carries a one-line summary (`line 3: unknown field ...`, `the file is missing`, the validation's sentence), a `FaultSink` beside the session sink hands it to the tray, and the tray draws the reserved error state with the summary as the menu's first line. `log_level` follows live through a reload layer; `log_dir` waits for the next start and says so. `StopSignal` gains a `Reload` reason and a child signal: the engine runs on a child of the process-wide stop, whose own event carries the reload and is reset between engines while the parent is never reset, so a Quit during a reload cannot be lost. Exit codes 3 and 4 are now the other commands'. Six new tests: the summaries, the watch on a real folder, the reload scenario in the engine, the fault on every tray surface. Co-Authored-By: Claude Opus 5 --- Cargo.toml | 3 + src/cli.rs | 8 +- src/config.rs | 301 +++++++++++++++++++++++++++++++--- src/detect/presence_writer.rs | 7 +- src/engine.rs | 36 ++-- src/engine/tests.rs | 62 ++++++- src/exit.rs | 35 ++-- src/logging.rs | 73 ++++++++- src/marker.rs | 1 + src/sensor.rs | 30 ++++ src/service.rs | 214 +++++++++++++++++++++--- src/tray.rs | 185 +++++++++++++++++---- src/win.rs | 182 +++++++++++++++++++- 13 files changed, 1020 insertions(+), 117 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cf2d902..3bc85f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,9 @@ windows = { version = "0.62", features = [ # fetched. Microsoft's libraries, no HTTP or hashing crate. "Win32_Networking_WinHttp", "Win32_Security_Cryptography", + # FindFirstChangeNotificationW: the configuration folder, watched for the + # live reload. + "Win32_Storage_FileSystem", # WNDCLASSEXW names HBRUSH, HICON and HCURSOR, so the window class needs Gdi # even though this program never draws anything. "Win32_Graphics_Gdi", diff --git a/src/cli.rs b/src/cli.rs index 40d9a38..b671e52 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -179,6 +179,12 @@ pub fn run(cli: Cli, console: bool) -> Result<()> { } let path = resolve_config_path(cli.config)?; + // The watcher loads the file itself: one it cannot use is shown in the + // icon and waited on, not a reason to exit. The commands below need a + // usable one and say so with the exit code. + if matches!(cli.command, None | Some(Command::Run { .. })) { + return service::serve(&path, cli.log_level.as_deref(), console); + } let config = Config::load(&path)?; let level = cli .log_level @@ -204,7 +210,7 @@ pub fn run(cli: Cli, console: bool) -> Result<()> { actions::run_all(actions, &actions::ActionContext::new(label, None)); Ok(()) } - _ => service::serve(config, &path, &level, console), + _ => unreachable!("every other command returned above"), } } diff --git a/src/config.rs b/src/config.rs index 7df16cf..0790f8b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,12 +1,22 @@ //! Configuration model, loaded from a TOML file. +//! +//! The watcher reads the file at start and again whenever it changes: a +//! thread waits on the folder's change notification, and a change to the +//! file's bytes stops the running engine for one built on the new file. +//! A file that cannot be used does not stop the program -- the icon says +//! what is wrong, in the words of [`LoadError::summary`], and nothing is +//! watched until it is fixed. Decided in `docs/design/09-robustness.md`. use std::collections::BTreeMap; use std::path::{Path, PathBuf}; +use std::sync::Arc; use std::time::Duration; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; +use crate::win::{FolderEvent, FolderWatch, StopSignal}; + pub const CONFIG_FILE_NAME: &str = "config.toml"; pub const APP_DIR_NAME: &str = "GameModeExecutor"; @@ -149,41 +159,117 @@ impl Action { } } -/// The configuration file is missing. Carried as error context so the program -/// can exit with a code that says which of the two failures happened. +/// Why a configuration file cannot be used. +/// +/// `Display` and the source chain are the whole story, for a console; the +/// exit code tells the two failures apart for a script. [`summary`] is the +/// one line the menu has room for. +/// +/// [`summary`]: LoadError::summary #[derive(Debug)] -pub struct Missing(pub PathBuf); +pub enum LoadError { + /// The file cannot be read -- most often it is not there. + Missing { + path: PathBuf, + source: std::io::Error, + }, + /// The file does not parse. `line` is where, when the parser says. + /// Boxed: the parser's error carries the whole input for its caret. + Syntax { + path: PathBuf, + line: Option, + source: Box, + }, + /// The file parses but asks for something the program cannot do. + Invalid { path: PathBuf, reason: String }, +} -impl std::fmt::Display for Missing { +impl std::fmt::Display for LoadError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "cannot read config file `{}`", self.0.display()) + match self { + Self::Missing { path, .. } => write!(f, "cannot read config file `{}`", path.display()), + Self::Syntax { path, .. } => { + write!(f, "config file `{}` is not usable", path.display()) + } + Self::Invalid { path, reason } => { + write!( + f, + "config file `{}` is not usable: {reason}", + path.display() + ) + } + } } } -impl std::error::Error for Missing {} - -/// The configuration file is present but unusable, whether it failed to parse -/// or failed validation. -#[derive(Debug)] -pub struct Invalid(pub PathBuf); +impl std::error::Error for LoadError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Missing { source, .. } => Some(source), + Self::Syntax { source, .. } => Some(source.as_ref()), + Self::Invalid { .. } => None, + } + } +} -impl std::fmt::Display for Invalid { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "config file `{}` is not usable", self.0.display()) +impl LoadError { + /// What is wrong, in one line and without the path, for a menu entry + /// next to *Edit configuration* and for the log: `line 3: unknown field + /// `log_levl`, expected one of ...`, `the file is missing`, + /// `detection.poll_interval must be greater than zero`. + pub fn summary(&self) -> String { + match self { + Self::Missing { source, .. } if source.kind() == std::io::ErrorKind::NotFound => { + "the file is missing".to_owned() + } + Self::Missing { source, .. } => format!("cannot read the file: {source}"), + Self::Syntax { line, source, .. } => { + let message = source + .message() + .split_whitespace() + .collect::>() + .join(" "); + match line { + Some(line) => format!("line {line}: {message}"), + None => message, + } + } + Self::Invalid { reason, .. } => reason.clone(), + } } } -impl std::error::Error for Invalid {} +/// Told when the configuration becomes unusable, with why, and when it is +/// usable again, with `None`. The tray draws it; the supervisor in +/// `service` decides it. +pub type FaultSink = Arc) + Send + Sync>; impl Config { - pub fn load(path: &Path) -> Result { - let text = std::fs::read_to_string(path) - .map_err(|error| anyhow::Error::new(error).context(Missing(path.to_path_buf())))?; - let config: Self = toml::from_str(&text) - .map_err(|error| anyhow::Error::new(error).context(Invalid(path.to_path_buf())))?; - config - .validate() - .map_err(|error| error.context(Invalid(path.to_path_buf())))?; + pub fn load(path: &Path) -> Result { + Self::parse(&Self::read(path)?, path) + } + + /// The file's text, or why it cannot be read. + pub fn read(path: &Path) -> Result { + std::fs::read_to_string(path).map_err(|source| LoadError::Missing { + path: path.to_path_buf(), + source, + }) + } + + /// `text` as read from `path`, parsed and validated. + pub fn parse(text: &str, path: &Path) -> Result { + let config: Self = toml::from_str(text).map_err(|source| LoadError::Syntax { + path: path.to_path_buf(), + line: source + .span() + .map(|span| text[..span.start.min(text.len())].matches('\n').count() + 1), + source: Box::new(source), + })?; + config.validate().map_err(|error| LoadError::Invalid { + path: path.to_path_buf(), + reason: format!("{error:#}"), + })?; Ok(config) } @@ -247,6 +333,71 @@ pub fn local_dir() -> Option { std::env::var_os("LOCALAPPDATA").map(|local| PathBuf::from(local).join(APP_DIR_NAME)) } +/// How long after the last change notification the file is read again. +/// Editors write in several steps -- a temporary file, a rename, a +/// truncate and a write -- and each step is a notification; reading in +/// the middle would see half a file. +pub const RELOAD_SETTLE: Duration = Duration::from_millis(250); + +/// Watch `path` and signal `reload` -- with [`StopSignal::signal_reload`] +/// -- each time the file's bytes change, until `stop` is set. +/// +/// A thread parked on the folder's change notification and the stop event, +/// so it costs nothing while nothing happens. What it compares is the +/// bytes, not the parse: a file written back unchanged is not a reload, and +/// a file that no longer parses is one, since the engine must stop. `last` +/// is the text the caller loaded, so the first change seen is a change to +/// what is running. Returns the thread, so the caller can wait for it to +/// have gone. +pub fn watch( + path: PathBuf, + last: Option, + stop: Arc, + reload: Arc, +) -> Result> { + let dir = path + .parent() + .filter(|dir| !dir.as_os_str().is_empty()) + .map_or_else(|| PathBuf::from("."), Path::to_path_buf); + let folder = FolderWatch::open(&dir)?; + tracing::debug!( + target: crate::logging::target::WATCHER, + folder = %dir.display(), + "Watching the configuration's folder for changes" + ); + Ok(std::thread::spawn(move || { + let mut last = last; + let mut settling = false; + loop { + let timeout = settling.then_some(RELOAD_SETTLE); + match folder.wait(&stop, timeout) { + FolderEvent::Stopped => return, + // Anything in the folder; whether it was the file is + // settled by reading it once the editor has finished. + FolderEvent::Changed => settling = true, + FolderEvent::TimedOut => { + settling = false; + let now = std::fs::read_to_string(&path).ok(); + if now == last { + tracing::debug!( + target: crate::logging::target::WATCHER, + "The configuration's folder changed, the file did not" + ); + continue; + } + last = now; + tracing::debug!( + target: crate::logging::target::WATCHER, + path = %path.display(), + "The configuration file changed, reloading" + ); + reload.signal_reload(); + } + } + } + })) +} + /// The configuration `init` writes: `config.example.toml` at the root of the /// repository, compiled in, so the file a user starts from and the one the /// repository documents are the same bytes. @@ -425,4 +576,108 @@ poll_intervall = \"2s\" .is_err() ); } + + // -------------------------------------------------- what the menu says -- + + /// The menu has one line and the person reading it has the file open + /// beside it: the line number and the parser's words, nothing else. + #[test] + fn a_syntax_error_is_summarised_with_its_line() { + let path = Path::new("config.toml"); + let text = "[general] +log_level = \"info\" +log_levl = 1 +"; + let error = Config::parse(text, path).unwrap_err(); + assert!( + matches!(error, LoadError::Syntax { line: Some(3), .. }), + "{error:?}" + ); + let summary = error.summary(); + assert!( + summary.starts_with("line 3: unknown field `log_levl`"), + "{summary}" + ); + assert!(!summary.contains('\n'), "one line: {summary:?}"); + // The console still gets the parser's own account, caret and all. + assert!(format!("{:#}", anyhow::Error::new(error)).contains("not usable")); + } + + #[test] + fn a_missing_file_and_a_bad_value_are_summarised_in_their_own_words() { + let path = Path::new(r"C:\nowhere\GameModeExecutor\config.toml"); + let missing = Config::load(path).unwrap_err(); + assert!(matches!(missing, LoadError::Missing { .. }), "{missing:?}"); + assert_eq!(missing.summary(), "the file is missing"); + + let invalid = Config::parse( + "[detection] +poll_interval = \"0s\" +", + path, + ) + .unwrap_err(); + assert!(matches!(invalid, LoadError::Invalid { .. }), "{invalid:?}"); + assert_eq!( + invalid.summary(), + "detection.poll_interval must be greater than zero" + ); + } + + // ------------------------------------------------------- the reload -- + + fn scratch() -> PathBuf { + static COUNTER: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let n = COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!( + "gamemode-executor-config-{}-{n}", + std::process::id() + )); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + /// The watch signals a reload when the file's bytes change, and not + /// when the folder is touched or the same bytes are written back; the + /// process-wide stop ends it. + #[test] + fn the_watch_signals_a_change_to_the_file_and_nothing_else() { + let dir = scratch(); + let path = dir.join(CONFIG_FILE_NAME); + std::fs::write(&path, "[general]\n").unwrap(); + let stop = Arc::new(StopSignal::new().unwrap()); + let reload = Arc::new(StopSignal::child_of(&stop).unwrap()); + let thread = watch( + path.clone(), + Some("[general]\n".to_owned()), + Arc::clone(&stop), + Arc::clone(&reload), + ) + .unwrap(); + + // Another file, and the same bytes again: the folder changed, the + // configuration did not. + std::fs::write(dir.join("other.txt"), "x").unwrap(); + std::fs::write(&path, "[general]\n").unwrap(); + assert!( + !reload.wait_timeout(RELOAD_SETTLE * 4), + "no reload for a folder change that left the file as it was" + ); + + std::fs::write(&path, "[general]\nlog_level = \"debug\"\n").unwrap(); + assert!( + reload.wait_timeout(Duration::from_secs(5)), + "a change to the bytes is a reload" + ); + assert_eq!(reload.reason(), crate::win::StopReason::Reload); + assert!(reload.take_reload()); + + // The same again, now that the watch remembers the new bytes. + std::fs::write(&path, "[general]\nlog_level = \"debug\"\n").unwrap(); + assert!(!reload.wait_timeout(RELOAD_SETTLE * 4)); + + stop.signal(); + thread.join().unwrap(); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/src/detect/presence_writer.rs b/src/detect/presence_writer.rs index f451286..a88da4e 100644 --- a/src/detect/presence_writer.rs +++ b/src/detect/presence_writer.rs @@ -113,9 +113,10 @@ pub fn wait_for_exit_until( Some(timeout) => timeout.as_millis().min(u128::from(INFINITE - 1)) as u32, None => INFINITE, }; - let handles = [process, stop.handle()]; - // SAFETY: both handles are valid for the whole wait -- `process` was just - // opened and is closed only afterwards, and the stop event lives as long + let mut handles = vec![process]; + handles.extend(stop.handles()); + // SAFETY: every handle is valid for the whole wait -- `process` was just + // opened and is closed only afterwards, and the stop events live as long // as `stop`. let result = unsafe { WaitForMultipleObjects(&handles, false, millis) }; // SAFETY: closes the handle opened above, exactly once. diff --git a/src/engine.rs b/src/engine.rs index 1a833e1..f5119c0 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -134,7 +134,8 @@ impl Engine { /// /// The other case, decided 2026-09-18: the writer is still running, so the /// game never ended -- the last watcher handed the session over for an - /// update, or crashed under it. Then nothing runs, neither stop nor start, + /// update, the last engine of this process stopped for a reload, or a + /// watcher crashed under it. Then nothing runs, neither stop nor start, /// and the session is taken up where it was. Looking for the writer /// *before* recovering is what keeps a game still on from getting the /// idle and then the gaming configuration seconds apart. Returns the @@ -153,17 +154,21 @@ impl Engine { Some(game) => tracing::info!( target: target::GAME, since = pending.since.as_deref(), - "The last watcher left a session open with {game} still running, so it resumes where it was" + "A session was left open with {game} still running, so it resumes where it was" ), None => tracing::info!( target: target::GAME, since = pending.since.as_deref(), - "The last watcher left a session open with a game still running, so it resumes where it was" + "A session was left open with a game still running, so it resumes where it was" ), } self.report(&Session::Playing(signal.clone())); return Some((pid, signal)); } + // The session is over. Said before the commands, as `fire_stop` does, + // and said at all because the icon may still show the session the + // engine before this one -- stopped for a reload -- left open. + self.report(&Session::Idle); match &pending.game { Some(game) => tracing::info!( target: target::GAME, @@ -270,13 +275,24 @@ impl Engine { // A handover: the watcher that follows resumes this session, // so nothing runs and the marker stays open. The commands // would only have swapped the configuration twice in the - // middle of a game. - if stop.reason() == StopReason::Handover { - tracing::info!( - target: target::WATCHER, - "Stopping for an update; the game session is handed to the next watcher" - ); - return Ok(()); + // middle of a game. A reload is the same handover, to the + // engine the supervisor builds next on the changed file. + match stop.reason() { + StopReason::Handover => { + tracing::info!( + target: target::WATCHER, + "Stopping for an update; the game session is handed to the next watcher" + ); + return Ok(()); + } + StopReason::Reload => { + tracing::debug!( + target: target::WATCHER, + "Stopping for a reload; the game session is kept open for the next engine" + ); + return Ok(()); + } + StopReason::Restore => {} } if self.config.general.stop_actions_on_exit { // Stays at info: without it the reader sees a session end diff --git a/src/engine/tests.rs b/src/engine/tests.rs index 32b0444..b4d5f0d 100644 --- a/src/engine/tests.rs +++ b/src/engine/tests.rs @@ -39,6 +39,9 @@ struct Scripted { /// A stop reported by `wait_for_writer_exit` is a handover, as /// `stop --handover` from an update makes it. stops_by_handover: bool, + /// A stop reported by `wait_for_writer_exit` is a reload, as a change + /// to the configuration file makes it. + stops_by_reload: bool, stop: Arc, } @@ -54,6 +57,7 @@ impl Scripted { counters_unreadable: false, session_ends_with_writer: false, stops_by_handover: false, + stops_by_reload: false, stop: Arc::clone(stop), } } @@ -63,6 +67,11 @@ impl Scripted { self } + fn stops_by_reload(mut self) -> Self { + self.stops_by_reload = true; + self + } + fn list_unreadable(mut self) -> Self { self.list_unreadable = true; self @@ -134,6 +143,9 @@ impl Sensor for Scripted { if self.stops_by_handover && outcome == WaitOutcome::Stopped { self.stop.signal_handover(); } + if self.stops_by_reload && outcome == WaitOutcome::Stopped { + self.stop.signal_reload(); + } Ok(outcome) } @@ -650,6 +662,44 @@ fn a_handover_mid_game_runs_nothing_and_leaves_the_session_open() { ); } +#[test] +fn a_reload_mid_game_is_a_handover_to_the_next_engine() { + // The configuration changed while a game is on. The engine stops as for + // an update -- nothing runs, the session stays open -- and the signal + // says so, so the supervisor knows to build the next engine rather than + // return; taking the reload clears it for that engine's run. + let stop = Arc::new(StopSignal::new().unwrap()); + let sensor = Scripted::new(&stop) + .writer(&[Some(7)]) + .waits(&[WaitOutcome::Stopped]) + .stops_by_reload() + .candidates(&[&[game(10, "game.exe")]]); + let dir = scratch(); + let ran = dir.join("stop-ran"); + let mut config = quick_config(); + config.on_game_stop = stop_event(vec![touch(&ran)]); + let (sink, log) = recorder(); + + let mut engine = Engine::new(config, sensor) + .reporting_to(sink) + .remembering(Marker::in_dir(&dir)); + engine.run(&stop).unwrap(); + + assert!(!ran.exists(), "the stop commands did not run"); + assert!( + Marker::in_dir(&dir).pending().is_some(), + "the session stays open" + ); + assert_eq!( + seen(&log), + vec![Session::Playing(Some(game(10, "game.exe")))], + "the session was never reported as ended" + ); + assert_eq!(stop.reason(), StopReason::Reload); + assert!(stop.take_reload(), "the reload is the supervisor's to take"); + assert!(!stop.is_set(), "and the next engine waits afresh"); +} + #[test] fn a_session_handed_over_is_resumed_without_running_anything() { // The next watcher starts with the marker open and the writer alive: it @@ -718,7 +768,11 @@ fn a_session_handed_over_whose_game_ended_meanwhile_is_closed_at_start() { assert!(stopped.exists(), "the stop commands ran at start"); assert!(Marker::in_dir(&dir).pending().is_none()); - assert!(seen(&log).is_empty(), "recovery is not a session"); + assert_eq!( + seen(&log), + vec![Session::Idle], + "the session left open is reported closed, and that is all" + ); } // ------------------------------------------------------------- recovery -- @@ -746,7 +800,11 @@ fn a_marker_left_behind_runs_the_stop_commands_before_watching() { Marker::in_dir(&dir).pending().is_none(), "and the marker is gone" ); - assert!(seen(&log).is_empty(), "recovery is not a session"); + assert_eq!( + seen(&log), + vec![Session::Idle], + "the session left open is reported closed; recovery is not a session" + ); } #[test] diff --git a/src/exit.rs b/src/exit.rs index 65b70ca..d9f725b 100644 --- a/src/exit.rs +++ b/src/exit.rs @@ -14,17 +14,16 @@ pub const CONFIG_MISSING: u8 = 3; pub const CONFIG_INVALID: u8 = 4; pub const ALREADY_RUNNING: u8 = 5; -/// Pick the code that describes a failure, by looking for the markers the -/// relevant errors carry as context. +/// Pick the code that describes a failure, by looking for the errors that +/// have one of their own anywhere in the chain. pub fn code_for(error: &anyhow::Error) -> u8 { - if error.downcast_ref::().is_some() { - CONFIG_MISSING - } else if error.downcast_ref::().is_some() { - CONFIG_INVALID - } else if error.downcast_ref::().is_some() { - ALREADY_RUNNING - } else { - FAILURE + match error.downcast_ref::() { + Some(config::LoadError::Missing { .. }) => CONFIG_MISSING, + Some(config::LoadError::Syntax { .. } | config::LoadError::Invalid { .. }) => { + CONFIG_INVALID + } + None if error.downcast_ref::().is_some() => ALREADY_RUNNING, + None => FAILURE, } } @@ -35,11 +34,19 @@ mod tests { #[test] fn a_missing_file_is_told_apart_from_an_unusable_one() { - let missing = anyhow::Error::new(std::io::Error::other("no such file")) - .context(config::Missing(PathBuf::from("a.toml"))); - let invalid = - anyhow::anyhow!("bad value").context(config::Invalid(PathBuf::from("a.toml"))); + let path = PathBuf::from("a.toml"); + let missing = anyhow::Error::new(config::LoadError::Missing { + path: path.clone(), + source: std::io::Error::other("no such file"), + }) + .context("while starting"); + let syntax = anyhow::Error::new(config::Config::parse("[general", &path).unwrap_err()); + let invalid = anyhow::Error::new(config::LoadError::Invalid { + path, + reason: "bad value".to_owned(), + }); assert_eq!(code_for(&missing), CONFIG_MISSING); + assert_eq!(code_for(&syntax), CONFIG_INVALID); assert_eq!(code_for(&invalid), CONFIG_INVALID); } diff --git a/src/logging.rs b/src/logging.rs index 6b0e6b5..d12d370 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -15,6 +15,8 @@ use std::fmt::Write as _; use std::io::IsTerminal; use std::path::Path; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, OnceLock}; use anyhow::{Context, Result}; use tracing::field::{Field, Visit}; @@ -23,9 +25,9 @@ use tracing_subscriber::fmt::format::Writer; use tracing_subscriber::fmt::time::FormatTime; use tracing_subscriber::fmt::{FmtContext, FormatEvent, FormatFields}; use tracing_subscriber::layer::SubscriberExt; -use tracing_subscriber::registry::LookupSpan; +use tracing_subscriber::registry::{LookupSpan, Registry}; use tracing_subscriber::util::SubscriberInitExt; -use tracing_subscriber::{EnvFilter, fmt}; +use tracing_subscriber::{EnvFilter, fmt, reload}; use windows::Win32::System::SystemInformation::GetLocalTime; /// The categories a line can belong to. @@ -111,10 +113,17 @@ fn level_colour(level: &Level) -> &'static str { struct Line { /// Print the structured fields. They are the technical annex to a line, so /// they appear only when the reader asked for that level of detail. - verbose: bool, + /// Shared with [`set_level`], which moves it with the level. + verbose: Arc, ansi: bool, } +impl Line { + fn verbose(&self) -> bool { + self.verbose.load(Ordering::Relaxed) + } +} + impl FormatEvent for Line where S: Subscriber + for<'a> LookupSpan<'a>, @@ -157,7 +166,7 @@ where let mut collected = Collected::default(); event.record(&mut collected); write!(writer, " {}", collected.message)?; - if self.verbose && !collected.fields.is_empty() { + if self.verbose() && !collected.fields.is_empty() { if self.ansi { write!(writer, " {DIM}{}{RESET}", collected.fields)?; } else { @@ -266,6 +275,46 @@ fn verbose_for(level: &str) -> bool { asked.contains("debug") || asked.contains("trace") } +/// The two things a change of `log_level` moves after `init`: the filter, +/// through its reload handle, and whether the fields are printed. +struct Live { + filter: reload::Handle, + verbose: Arc, + /// `RUST_LOG` was set at start, and keeps winning. + from_env: bool, +} + +static LIVE: OnceLock = OnceLock::new(); + +/// Change the level after `init`, when the configuration's `log_level` +/// changed under a running watcher. `RUST_LOG`, when set, still wins, as it +/// did at start. +pub fn set_level(level: &str) { + let Some(live) = LIVE.get() else { + return; + }; + if live.from_env { + tracing::debug!( + target: target::WATCHER, + level, + "log_level changed, but RUST_LOG is set and keeps deciding" + ); + return; + } + match live.filter.reload(EnvFilter::new(directives(level))) { + Ok(()) => { + live.verbose.store(verbose_for(level), Ordering::Relaxed); + tracing::debug!(target: target::WATCHER, level, "Log level changed"); + } + Err(error) => tracing::warn!( + target: target::WATCHER, + level, + error = %error, + "The log level could not be changed; the previous one stays" + ), + } +} + /// Initialise logging. `RUST_LOG` overrides `level` when set. /// /// `console` says whether this process has a console at all, which is a @@ -278,9 +327,11 @@ fn verbose_for(level: &str) -> bool { /// visible, and whether anything is written to it. The result was a visible /// window that stayed blank forever. pub fn init(level: &str, log_dir: Option<&Path>, console: bool) -> Result<()> { - let verbose = verbose_for(level); + let verbose = Arc::new(AtomicBool::new(verbose_for(level))); + let from_env = std::env::var_os("RUST_LOG").is_some(); let filter = EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(directives(level))); + let (filter, handle) = reload::Layer::new(filter); let file_layer = match log_dir { Some(dir) => { @@ -311,7 +362,7 @@ pub fn init(level: &str, log_dir: Option<&Path>, console: bool) -> Result<()> { Some( fmt::layer() .event_format(Line { - verbose, + verbose: Arc::clone(&verbose), ansi: false, }) .with_writer(std::sync::Mutex::new(file)), @@ -325,7 +376,7 @@ pub fn init(level: &str, log_dir: Option<&Path>, console: bool) -> Result<()> { // or a pipe -- where they are noise rather than colour. let console_layer = console.then(|| { fmt::layer().event_format(Line { - verbose, + verbose: Arc::clone(&verbose), ansi: std::io::stdout().is_terminal(), }) }); @@ -335,6 +386,12 @@ pub fn init(level: &str, log_dir: Option<&Path>, console: bool) -> Result<()> { .with(console_layer) .with(file_layer) .init(); + // Set once; a second `init` would have failed just above. + let _ = LIVE.set(Live { + filter: handle, + verbose, + from_env, + }); Ok(()) } @@ -368,7 +425,7 @@ mod tests { let buffer = Buffer::default(); let layer = fmt::layer() .event_format(Line { - verbose, + verbose: Arc::new(AtomicBool::new(verbose)), ansi: false, }) .with_writer(buffer.clone()); diff --git a/src/marker.rs b/src/marker.rs index 6c34ba8..29b4e1c 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -29,6 +29,7 @@ use std::path::{Path, PathBuf}; /// comment lines inside are for whoever opens it anyway. pub const FILE_NAME: &str = "pending-stop-actions"; +#[derive(Clone)] pub struct Marker { path: PathBuf, } diff --git a/src/sensor.rs b/src/sensor.rs index 0aa9b13..2dc1079 100644 --- a/src/sensor.rs +++ b/src/sensor.rs @@ -46,6 +46,36 @@ pub trait Sensor { fn rendering_load(&self, sample: Duration) -> Result>; } +/// A borrowed sensor answers as the sensor does: the supervisor keeps one +/// `Windows` for the life of the process and builds an engine on it for +/// each configuration. +impl Sensor for &S { + fn writer_pid(&self) -> Option { + (**self).writer_pid() + } + + fn wait_for_writer_exit( + &self, + pid: u32, + stop: &StopSignal, + timeout: Option, + ) -> Result { + (**self).wait_for_writer_exit(pid, stop, timeout) + } + + fn candidates(&self) -> Result> { + (**self).candidates() + } + + fn is_running(&self, pid: u32) -> bool { + (**self).is_running(pid) + } + + fn rendering_load(&self, sample: Duration) -> Result> { + (**self).rendering_load(sample) + } +} + /// The real machine. pub struct Windows { /// Resolved from the registry once at startup, never hard-coded. diff --git a/src/service.rs b/src/service.rs index b09f1c8..bcb85b7 100644 --- a/src/service.rs +++ b/src/service.rs @@ -20,13 +20,24 @@ //! are never pulled from under a running watcher. With `StopReason::Handover` //! the session window gets `WM_HANDOVER` instead, and a game session that //! is open stays open for the watcher that follows. +//! +//! **The configuration is the supervisor's, not the engine's.** The worker +//! thread runs a supervisor: one engine per usable configuration, built on +//! the file as it is and stopped -- through the handover path, session kept +//! open -- when the file changes, so that the next engine reads the new one +//! and resumes what the last left. A file that cannot be used freezes the +//! program rather than stopping it: the icon shows the fault, nothing is +//! watched, and the next usable file starts an engine that settles the +//! open session the way a start does. Decided 2026-09-16, built +//! 2026-09-19; `docs/design/09-robustness.md` says why it is strict. +use std::path::Path; use std::sync::Arc; use std::time::{Duration, Instant}; use anyhow::{Context, Result}; -use crate::config::{self, Config}; +use crate::config::{self, Config, FaultSink, LoadError}; use crate::win::{SessionWindow, SingleInstance, StopReason, StopSignal}; use crate::{engine, logging, sensor, tray, update, win}; @@ -46,25 +57,40 @@ pub const INSTANCE: &str = "GameModeExecutor"; /// watcher that is wedged. const STOP_PATIENCE: Duration = Duration::from_secs(30); +/// The level the log opens at when the configuration cannot say. +const DEFAULT_LEVEL: &str = "info"; + /// Run the watcher until it is stopped. /// +/// The configuration is loaded here rather than by the caller, because a +/// file that cannot be used is not a reason to exit: the watcher starts, +/// shows the fault and waits for the file to change. `level` is the +/// command line's `--log-level`, which wins over the file's, now and at +/// every reload. +/// /// `console` says whether this process has a console: it gates both the log's /// console layer and the Ctrl-C handler, neither of which means anything /// without one. -pub fn serve( - config: Config, - config_path: &std::path::Path, - level: &str, - console: bool, -) -> Result<()> { +pub fn serve(config_path: &Path, level: Option<&str>, console: bool) -> Result<()> { + let (text, loaded) = load(config_path); // The watcher always keeps a log file. A windowless instance has nowhere // else to write, and a console one is usually left running unattended. - let log_dir = config - .general - .log_dir - .clone() + // A file that cannot be read cannot say where: the default, then. + let log_dir = loaded + .as_ref() + .ok() + .and_then(|config| config.general.log_dir.clone()) .or_else(|| config::local_dir().map(|dir| dir.join("logs"))); - logging::init(level, log_dir.as_deref(), console)?; + let opened_at = level + .map(str::to_owned) + .or_else(|| { + loaded + .as_ref() + .ok() + .map(|config| config.general.log_level.clone()) + }) + .unwrap_or_else(|| DEFAULT_LEVEL.to_owned()); + logging::init(&opened_at, log_dir.as_deref(), console)?; // Installed as early as the log exists, so a panic anywhere after this // leaves a FATAL line behind rather than a process that simply vanished. logging::install_panic_hook(); @@ -132,20 +158,47 @@ pub fn serve( ); // The engine reports session changes to the tray, which is how the icon, - // the tooltip and the menu stay in step with each other and with reality. + // the tooltip and the menu stay in step with each other and with reality; + // the supervisor reports the configuration's faults the same way. let sink = tray::session_sink(window_id); - let worker_stop = Arc::clone(&stop); + let faults = tray::fault_sink(window_id); // State, so it lives with the local profile and not with the log, which // the user may have sent elsewhere and is entitled to empty. let marker = crate::marker::Marker::in_local_dir(); + // One engine run stops on this; the process stops on `stop`, which it + // answers to as well. + let run_stop = Arc::new(StopSignal::child_of(&stop)?); + // Watching the folder is a convenience over restarting; a folder that + // cannot be watched says so and the file is read at the next start. + let watcher = match config::watch( + config_path.to_path_buf(), + text, + Arc::clone(&stop), + Arc::clone(&run_stop), + ) { + Ok(watcher) => Some(watcher), + Err(error) => { + tracing::warn!( + target: logging::target::WATCHER, + error = %format!("{error:#}"), + "The configuration's folder cannot be watched, so a change to the file \ + takes effect at the next start" + ); + None + } + }; + let mut supervised = Supervised { + path: config_path.to_path_buf(), + level: level.map(str::to_owned), + applied_level: opened_at, + log_dir: log_dir.clone(), + sink, + faults, + marker, + }; let worker = std::thread::spawn(move || { - let outcome = sensor::Windows::new() - .map(|sensor| engine::Engine::new(config, sensor).reporting_to(sink)) - .map(|engine| match marker { - Some(marker) => engine.remembering(marker), - None => engine, - }) - .and_then(|mut engine| engine.run(&worker_stop)); + let outcome = + sensor::Windows::new().and_then(|sensor| supervised.run(&sensor, loaded, &run_stop)); // Order matters: release WM_ENDSESSION first, then wake the loop. finished.signal(); win::wake_message_loop(window_id); @@ -164,12 +217,129 @@ pub fn serve( let outcome = worker .join() .map_err(|_| anyhow::anyhow!("the watcher thread panicked"))?; + if let Some(watcher) = watcher { + // Parked on the stop event too, so this is immediate. + let _ = watcher.join(); + } outcome?; tracing::info!(target: logging::target::WATCHER, "Stopped"); Ok(()) } +/// The file's text, when it can be read, and what it parses to. +/// +/// The text goes to the folder watch, so that the first change it reports +/// is a change to what is running and not to what it read itself a moment +/// later. +fn load(path: &Path) -> (Option, Result) { + match Config::read(path) { + Ok(text) => { + let parsed = Config::parse(&text, path); + (Some(text), parsed) + } + Err(error) => (None, Err(error)), + } +} + +/// What the supervisor keeps across engines. +struct Supervised { + path: std::path::PathBuf, + /// The command line's level, which wins over the file's. + level: Option, + /// The level the log is at, so a reload that keeps it is silent. + applied_level: String, + /// Where the log was opened; a file that moves it is told to wait. + log_dir: Option, + sink: engine::SessionSink, + faults: FaultSink, + marker: Option, +} + +impl Supervised { + /// One engine per usable configuration, until the process stops. + /// + /// `stop` is the run's signal: set by the folder watch for a reload, by + /// the process-wide stop for anything else. An engine that returns on + /// a reload has left an open session in the marker, the way a handover + /// does, and the next engine's recovery settles it: resumed when the + /// game is still on, its stop commands run when it is not. A file that + /// cannot be used is shown and waited on; nothing runs meanwhile, not + /// even the stop commands of a session that ends, which is what + /// "disabled outright" means and why the marker is the right memory. + fn run( + &mut self, + sensor: &sensor::Windows, + mut loaded: Result, + stop: &StopSignal, + ) -> Result<()> { + let mut first = true; + loop { + match loaded { + Ok(config) => { + (self.faults)(None); + // The log first, so the line below is written the way + // the new file asks. + self.follow(&config); + if !first { + tracing::info!( + target: logging::target::WATCHER, + path = %self.path.display(), + "Configuration reloaded" + ); + } + let mut engine = + engine::Engine::new(config, sensor).reporting_to(Arc::clone(&self.sink)); + if let Some(marker) = &self.marker { + engine = engine.remembering(marker.clone()); + } + engine.run(stop)?; + } + Err(fault) => { + (self.faults)(Some(&fault)); + tracing::error!( + target: logging::target::WATCHER, + path = %self.path.display(), + "The configuration cannot be used, so nothing is watched until it is \ + fixed: {}", + fault.summary() + ); + // Frozen: the folder watch or the process-wide stop + // ends this, nothing else. + stop.wait(); + } + } + if !stop.take_reload() { + return Ok(()); + } + first = false; + loaded = load(&self.path).1; + } + } + + /// Apply what a configuration says about the log itself: the level + /// follows live unless the command line fixed it; the folder cannot + /// move under an open file and waits for the next start. + fn follow(&mut self, config: &Config) { + if self.level.is_none() && config.general.log_level != self.applied_level { + logging::set_level(&config.general.log_level); + self.applied_level = config.general.log_level.clone(); + } + let wanted = config + .general + .log_dir + .clone() + .or_else(|| config::local_dir().map(|dir| dir.join("logs"))); + if wanted != self.log_dir { + tracing::warn!( + target: logging::target::WATCHER, + wanted = wanted.as_deref().map(|dir| dir.display().to_string()), + "log_dir changed; the log moves there at the next start" + ); + } + } +} + /// What `stop` found. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Stopped { @@ -209,6 +379,8 @@ pub fn stop(reason: StopReason) -> Result { waited = ?asked.elapsed(), "Watcher stopped, as asked; a game session that was open waits for the next one" ), + // Refused by `close_session_window` above, before any wait. + StopReason::Reload => unreachable!("a reload is never asked of another process"), } return Ok(Stopped::Stopped); } diff --git a/src/tray.rs b/src/tray.rs index 0847af7..9873ed1 100644 --- a/src/tray.rs +++ b/src/tray.rs @@ -43,7 +43,8 @@ use crate::win::StopSignal; /// Our callback message. `WM_APP + 1` is the watcher-finished message in `win`. const WM_TRAY: u32 = WM_APP + 2; -/// Posted by the watcher thread when the session changed. +/// Posted by the watcher thread when the session changed, or the +/// configuration became unusable or usable again. const WM_SESSION: u32 = WM_APP + 3; /// Posted by the updater's worker when an outcome left a notice to show. @@ -72,11 +73,31 @@ enum Session { static SESSION: std::sync::Mutex = std::sync::Mutex::new(Session::Idle); -fn session() -> Session { - SESSION - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .clone() +/// Why the configuration cannot be used, in one line, or `None` while it +/// can. The second axis the icon is drawn from, written by the supervisor +/// in `service` through [`fault_sink`]. Nothing is watched while it is +/// `Some`, so it takes precedence over the session on every surface. +static FAULT: std::sync::Mutex> = std::sync::Mutex::new(None); + +/// The two facts every surface is drawn from, read together so the icon, +/// the tooltip and the menu cannot disagree about either. +#[derive(Clone, Debug, PartialEq, Eq)] +struct Facts { + session: Session, + fault: Option, +} + +fn facts() -> Facts { + Facts { + session: SESSION + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone(), + fault: FAULT + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone(), + } } /// Hand this to the engine so it reports session changes here. @@ -114,6 +135,33 @@ pub fn session_sink(window: isize) -> crate::engine::SessionSink { }) } +/// Hand this to the supervisor so it reports the configuration's faults +/// here: the summary is stored and the window's thread redraws from it. +pub fn fault_sink(window: isize) -> crate::config::FaultSink { + Arc::new(move |fault: Option<&crate::config::LoadError>| { + let next = fault.map(crate::config::LoadError::summary); + { + let mut held = FAULT + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if *held == next { + return; + } + *held = next; + } + // SAFETY: posting carries no pointers, and a window that is gone makes + // the call fail, which is ignored. + unsafe { + let _ = PostMessageW( + Some(HWND(window as *mut std::ffi::c_void)), + WM_SESSION, + WPARAM(0), + LPARAM(0), + ); + } + }) +} + /// Hand this to the updater so it wakes the window's thread when an /// outcome left a notice; the thread reads the notice itself. pub fn update_sink(window: isize) -> Arc { @@ -260,11 +308,16 @@ mod dark { /// What the icon should be showing, derived rather than stored. fn current_state() -> State { - state_for(&session()) + state_for(&facts()) } -fn state_for(session: &Session) -> State { - match session { +/// A fault is the error state whatever the session: nothing is watched +/// while the configuration is unusable, so a green icon would be a lie. +fn state_for(facts: &Facts) -> State { + if facts.fault.is_some() { + return State::Error; + } + match facts.session { Session::Idle => State::Idle, Session::Playing(_) => State::Active, } @@ -273,24 +326,33 @@ fn state_for(session: &Session) -> State { /// Windows truncates `szTip` at 128 units including the terminator, and a /// game's name is not always short. fn tooltip() -> String { - truncate(&tooltip_for(&session()), 127) + truncate(&tooltip_for(&facts()), 127) } -fn tooltip_for(session: &Session) -> String { - match session { +fn tooltip_for(facts: &Facts) -> String { + if facts.fault.is_some() { + return "GameModeExecutor - configuration error".to_owned(); + } + match &facts.session { Session::Idle => "GameModeExecutor - no game detected".to_owned(), Session::Playing(Some(name)) => format!("GameModeExecutor - playing {name}"), Session::Playing(None) => format!("GameModeExecutor - {UNNAMED}"), } } -/// The disabled first line of the menu: the same fact, room for more words. +/// The disabled first line of the menu: the same fact, room for more words +/// -- and, for a fault, the words that say what to fix, next to the *Edit +/// configuration* entry that opens the file. Cut where a menu would run +/// off the screen; the log has the whole line. fn menu_header() -> String { - menu_header_for(&session()) + truncate(&menu_header_for(&facts()), 160) } -fn menu_header_for(session: &Session) -> String { - match session { +fn menu_header_for(facts: &Facts) -> String { + if let Some(fault) = &facts.fault { + return format!("Configuration error: {fault}"); + } + match &facts.session { Session::Idle => "No game detected".to_owned(), Session::Playing(Some(name)) => format!("Playing {name}"), Session::Playing(None) => UNNAMED.to_owned(), @@ -369,9 +431,10 @@ pub enum State { Idle, /// A game is detected. Active, - /// Reserved: nothing sets this yet, and the engine has no notion of a - /// standing error. The artwork exists so the meaning is already spoken for - /// and nobody reaches for the slash to mean something else. + /// The configuration cannot be used and nothing is watched until it is + /// fixed; the menu's first line says what is wrong. The one standing + /// error the program has, since 2026-09-19; the artwork was reserved + /// for it so nobody reached for the slash to mean something else. Error, } @@ -554,7 +617,8 @@ impl Tray { } WM_SETTINGCHANGE if setting_is(lparam, "ImmersiveColorSet") => Plan::Reload, WM_DPICHANGED => Plan::Reload, - // The engine says a game started, was renamed, or ended. + // The engine says a game started, was renamed, or ended; or the + // supervisor says the configuration broke or was fixed. WM_SESSION => Plan::Reload, WM_UPDATE => Plan::Notify, _ => Plan::Ignore, @@ -1066,22 +1130,44 @@ mod tests { /// checked against one another rather than one at a time. #[test] fn the_three_surfaces_agree() { - let playing = Session::Playing(Some("bf6.exe".to_owned())); + let playing = sound(Session::Playing(Some("bf6.exe".to_owned()))); assert_eq!(state_for(&playing), State::Active); assert!(tooltip_for(&playing).contains("bf6.exe")); assert!(menu_header_for(&playing).contains("bf6.exe")); - let idle = Session::Idle; + let idle = sound(Session::Idle); assert_eq!(state_for(&idle), State::Idle); assert!(tooltip_for(&idle).contains("no game")); assert!(menu_header_for(&idle).contains("No game")); } + /// A configuration that cannot be used is the error state on every + /// surface, whatever the session: nothing is watched meanwhile. The + /// menu carries the reason, next to the entry that opens the file. + #[test] + fn a_configuration_fault_overrides_the_session_on_every_surface() { + for session in [Session::Idle, Session::Playing(Some("bf6.exe".to_owned()))] { + let faulty = Facts { + session, + fault: Some("line 3: unknown field `log_levl`".to_owned()), + }; + assert_eq!(state_for(&faulty), State::Error); + assert_eq!( + tooltip_for(&faulty), + "GameModeExecutor - configuration error" + ); + assert_eq!( + menu_header_for(&faulty), + "Configuration error: line 3: unknown field `log_levl`" + ); + } + } + /// A title Windows tracks but does not describe. All three have to say /// something, and the same something -- an empty space would read as a bug. #[test] fn an_unnamed_game_still_reads_sensibly() { - let unnamed = Session::Playing(None); + let unnamed = sound(Session::Playing(None)); assert_eq!(state_for(&unnamed), State::Active); assert_eq!(menu_header_for(&unnamed), UNNAMED); assert!(tooltip_for(&unnamed).contains(UNNAMED)); @@ -1091,7 +1177,7 @@ mod tests { /// not always short. Windows truncates silently, so we do it visibly. #[test] fn a_very_long_name_is_cut_to_fit() { - let long = Session::Playing(Some("x".repeat(400))); + let long = sound(Session::Playing(Some("x".repeat(400)))); let text = truncate(&tooltip_for(&long), 127); assert!(text.encode_utf16().count() <= 127, "{}", text.len()); assert!(text.ends_with('\u{2026}'), "{text}"); @@ -1105,6 +1191,40 @@ mod tests { assert!(text.chars().all(|c| c == 'é' || c == '\u{2026}'), "{text}"); } + /// A session with no configuration fault. + fn sound(session: Session) -> Facts { + Facts { + session, + fault: None, + } + } + + /// The two sinks write process-wide state, so the tests that drive them + /// take turns. + static SURFACES: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + /// The fault sink stores the summary, the surfaces switch to the error + /// state over whatever the session is, and `None` gives them back. + /// Window `0`, as below. + #[test] + fn the_fault_sink_overlays_the_session_and_lifts_again() { + let _turn = SURFACES + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let path = std::path::Path::new("config.toml"); + let fault = crate::config::Config::parse("[general]\nlog_levl = 1\n", path).unwrap_err(); + let sink = fault_sink(0); + + sink(Some(&fault)); + assert_eq!(current_state(), State::Error); + assert!(tooltip().contains("configuration error")); + assert!(menu_header().starts_with("Configuration error: line 2: unknown field")); + + sink(None); + assert_eq!(current_state(), State::Idle); + assert!(tooltip().contains("no game")); + } + /// The engine reports on every refinement, and most of those land on the /// same name. Without the early return the shell would be asked to redraw /// an identical icon each time. @@ -1113,6 +1233,9 @@ mod tests { /// is what lets the plumbing be tested without a window. #[test] fn the_sink_carries_changes_and_swallows_repeats() { + let _turn = SURFACES + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let sink = session_sink(0); let signal = crate::detect::GameSignal { source: "test", @@ -1124,22 +1247,28 @@ mod tests { use crate::engine::Session as Engine; sink(&Engine::Playing(Some(signal.clone()))); - assert_eq!(session(), Session::Playing(Some("bf6.exe".to_owned()))); + assert_eq!( + facts().session, + Session::Playing(Some("bf6.exe".to_owned())) + ); assert_eq!(current_state(), State::Active); assert!(tooltip().contains("bf6.exe")); // The same thing again changes nothing. sink(&Engine::Playing(Some(signal))); - assert_eq!(session(), Session::Playing(Some("bf6.exe".to_owned()))); + assert_eq!( + facts().session, + Session::Playing(Some("bf6.exe".to_owned())) + ); // A game Windows tracks but does not name is still a game. sink(&Engine::Playing(None)); - assert_eq!(session(), Session::Playing(None)); + assert_eq!(facts().session, Session::Playing(None)); assert_eq!(current_state(), State::Active); assert!(tooltip().contains("does not name")); sink(&Engine::Idle); - assert_eq!(session(), Session::Idle); + assert_eq!(facts().session, Session::Idle); assert_eq!(current_state(), State::Idle); assert!(tooltip().contains("no game")); } diff --git a/src/win.rs b/src/win.rs index e93c22c..0c216cd 100644 --- a/src/win.rs +++ b/src/win.rs @@ -8,12 +8,16 @@ use std::time::Duration; use anyhow::{Context, Result}; use windows::Win32::Foundation::{ CloseHandle, ERROR_ALREADY_EXISTS, GetLastError, HANDLE, HWND, LPARAM, LRESULT, WAIT_OBJECT_0, - WPARAM, + WAIT_TIMEOUT, WPARAM, +}; +use windows::Win32::Storage::FileSystem::{ + FILE_NOTIFY_CHANGE_FILE_NAME, FILE_NOTIFY_CHANGE_LAST_WRITE, FILE_NOTIFY_CHANGE_SIZE, + FindCloseChangeNotification, FindFirstChangeNotificationW, FindNextChangeNotification, }; use windows::Win32::System::LibraryLoader::GetModuleHandleW; use windows::Win32::System::Threading::{ - CreateEventW, CreateMutexW, INFINITE, OpenMutexW, SYNCHRONIZATION_SYNCHRONIZE, SetEvent, - WaitForSingleObject, + CreateEventW, CreateMutexW, INFINITE, OpenMutexW, ResetEvent, SYNCHRONIZATION_SYNCHRONIZE, + SetEvent, WaitForMultipleObjects, WaitForSingleObject, }; use windows::Win32::UI::HiDpi::{ DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2, SetProcessDpiAwarenessContext, @@ -36,18 +40,34 @@ pub enum StopReason { /// development loop. The session stays open in the marker and the next /// watcher resumes it, so nothing runs twice. Decided 2026-09-18. Handover, + /// The configuration file changed: the engine stops so that one built + /// on the new file can take its place, in the same process. A session + /// that is open stays open, as for a handover, and the next engine + /// resumes it. Only ever the reason of a signal made with + /// [`StopSignal::child_of`]. Decided 2026-09-19. + Reload, } /// A manual-reset event used to unblock every wait in the program at once. /// /// Waiting on a kernel event rather than checking a flag on a timer is what /// keeps the watcher at zero wake-ups while a game is running. +/// +/// A signal can be the child of another: it is then set when either event +/// is, and the parent's reason wins. That is how one engine run stops for +/// a reload without the process-wide stop ever being reset -- the child's +/// own event carries the reload and is reset between runs; the parent +/// carries *Quit*, the logoff and `stop`, and stays set once set. pub struct StopSignal { event: HANDLE, /// Set before the event when the stop is a handover. The first reason to /// arrive wins: a *Quit* after a handover request still hands over, a /// handover after a *Quit* has nothing left to hand. handover: AtomicBool, + /// Set before the event when the stop is a reload of this run only. + reload: AtomicBool, + /// The signal this one also answers to. + parent: Option>, } // SAFETY: a Win32 event handle is a kernel object; signalling and waiting on @@ -65,9 +85,22 @@ impl StopSignal { Ok(Self { event, handover: AtomicBool::new(false), + reload: AtomicBool::new(false), + parent: None, }) } + /// A signal that is also set whenever `parent` is, and can be set on + /// its own for a reload and reset again with [`take_reload`], leaving + /// the parent as it was. + /// + /// [`take_reload`]: StopSignal::take_reload + pub fn child_of(parent: &Arc) -> Result { + let mut child = Self::new()?; + child.parent = Some(Arc::clone(parent)); + Ok(child) + } + /// Stop, and restore: the stop commands run if a game is on. pub fn signal(&self) { // SAFETY: the event is open for as long as `self` lives. @@ -82,29 +115,80 @@ impl StopSignal { self.signal(); } + /// Stop this run only, to start again on a changed configuration. Nothing + /// to do when a stop is already under way: the process is leaving, and + /// the file is read afresh at the next start. + pub fn signal_reload(&self) { + if self.is_set() { + return; + } + self.reload.store(true, Ordering::SeqCst); + self.signal(); + } + + /// Whether the stop under way is a reload, and if so, clear it so the + /// next run waits afresh. A parent that is set is never cleared: the + /// process is stopping, whatever this run was told. + pub fn take_reload(&self) -> bool { + if self.parent.as_ref().is_some_and(|parent| parent.is_set()) || !self.own_is_set() { + return false; + } + if !self.reload.swap(false, Ordering::SeqCst) { + return false; + } + // SAFETY: as for `signal`. + let _ = unsafe { ResetEvent(self.event) }; + true + } + pub fn reason(&self) -> StopReason { + if let Some(parent) = &self.parent + && parent.is_set() + { + return parent.reason(); + } if self.handover.load(Ordering::SeqCst) { StopReason::Handover + } else if self.reload.load(Ordering::SeqCst) { + StopReason::Reload } else { StopReason::Restore } } pub fn is_set(&self) -> bool { + self.own_is_set() || self.parent.as_ref().is_some_and(|parent| parent.is_set()) + } + + fn own_is_set(&self) -> bool { // SAFETY: as for `signal`. unsafe { WaitForSingleObject(self.event, 0) == WAIT_OBJECT_0 } } + /// Park until the stop is signalled. + pub fn wait(&self) { + while !self.wait_timeout(Duration::MAX) {} + } + /// Wait up to `timeout`. Returns true when the stop was signalled, which /// callers treat as "give up and return". pub fn wait_timeout(&self, timeout: Duration) -> bool { let millis = timeout.as_millis().min(u128::from(INFINITE - 1)) as u32; - // SAFETY: as for `signal`. - unsafe { WaitForSingleObject(self.event, millis) == WAIT_OBJECT_0 } + let handles = self.handles(); + // SAFETY: every handle is an event open for as long as `self` -- and + // its parent, which it holds -- lives. + let result = unsafe { WaitForMultipleObjects(&handles, false, millis) }; + result != WAIT_TIMEOUT && result.0 < WAIT_OBJECT_0.0 + handles.len() as u32 } - pub(crate) fn handle(&self) -> HANDLE { - self.event + /// The events to wait on: this signal's own, and its parent's when it + /// has one. For a wait that also watches something else. + pub(crate) fn handles(&self) -> Vec { + let mut handles = vec![self.event]; + if let Some(parent) = &self.parent { + handles.extend(parent.handles()); + } + handles } } @@ -264,6 +348,9 @@ pub fn close_session_window(reason: StopReason) -> Result<()> { message: match reason { StopReason::Restore => WM_CLOSE, StopReason::Handover => WM_HANDOVER, + // A reload is the watcher's own business, between its engine + // runs; nothing asks it of another process. + StopReason::Reload => anyhow::bail!("a reload cannot be asked of a running watcher"), }, found: false, }; @@ -476,3 +563,84 @@ impl Drop for SingleInstance { unsafe { _ = CloseHandle(self.handle) }; } } + +// --------------------------------------------------------------------------- + +/// What a wait on a [`FolderWatch`] came back with. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FolderEvent { + /// Something in the folder was written, renamed, created or removed. + Changed, + /// Nothing happened within the timeout. + TimedOut, + /// The stop signal was set. + Stopped, +} + +/// A change notification on one folder: a handle Windows signals when a +/// file in it is written, renamed, created or removed. +/// +/// `FindFirstChangeNotificationW` rather than `ReadDirectoryChangesW`: the +/// program does not need to know *which* file changed, only that the folder +/// holding the configuration did, and a waitable handle is all that takes. +/// The watch does not descend into subfolders. +pub struct FolderWatch { + handle: HANDLE, +} + +// SAFETY: a change notification handle is a kernel object; waiting on it +// from the thread that watches, rather than the one that opened it, is what +// it is for, and the struct holds nothing else. +unsafe impl Send for FolderWatch {} + +impl FolderWatch { + pub fn open(dir: &std::path::Path) -> Result { + let name = HSTRING::from(dir.as_os_str()); + // SAFETY: `name` is a NUL-terminated string that outlives the call; + // the handle that comes back is owned by `FolderWatch` and closed + // on drop. + let handle = unsafe { + FindFirstChangeNotificationW( + &name, + false, + FILE_NOTIFY_CHANGE_FILE_NAME + | FILE_NOTIFY_CHANGE_LAST_WRITE + | FILE_NOTIFY_CHANGE_SIZE, + ) + } + .with_context(|| format!("cannot watch `{}` for changes", dir.display()))?; + Ok(Self { handle }) + } + + /// Park until the folder changes, `stop` is set, or `timeout` passes. + /// A change re-arms the handle before returning, so the next wait sees + /// the next change. + pub fn wait(&self, stop: &StopSignal, timeout: Option) -> FolderEvent { + let millis = match timeout { + Some(timeout) => timeout.as_millis().min(u128::from(INFINITE - 1)) as u32, + None => INFINITE, + }; + let mut handles = vec![self.handle]; + handles.extend(stop.handles()); + // SAFETY: the notification handle lives as long as `self`, the + // events as long as `stop`. + let result = unsafe { WaitForMultipleObjects(&handles, false, millis) }; + if result == WAIT_OBJECT_0 { + // SAFETY: re-arms the handle opened in `open`, still open. + let _ = unsafe { FindNextChangeNotification(self.handle) }; + FolderEvent::Changed + } else if result == WAIT_TIMEOUT { + FolderEvent::TimedOut + } else { + FolderEvent::Stopped + } + } +} + +impl Drop for FolderWatch { + fn drop(&mut self) { + // SAFETY: the handle came from `FindFirstChangeNotificationW` and is + // closed once. + unsafe { _ = FindCloseChangeNotification(self.handle) }; + } +} From 19a565c471704f0807efb05e09ab2348b2b0cba3 Mon Sep 17 00:00:00 2001 From: Geoffrey Vancoetsem <10533139+geeooff@users.noreply.github.com> Date: Sat, 19 Sep 2026 02:03:30 +0200 Subject: [PATCH 2/5] Say that the configuration reloads and what a red icon means Getting started no longer tells people to stop and restart the watcher after an edit; the recipes say the same, with install-task kept for the zip's first install. The icon table and the troubleshooting section gain the red, slashed icon; How it works explains the reload, the handover it rides on and why an unusable file freezes the program rather than falling back; the reference notes what applies live and that the exit codes are the commands'. The changelog's Unreleased section carries the three lines for the person running it. Lot 9's page records the mechanics as built -- the engine restarted through the handover, not the RwLock the plan named, and why -- and the measurement of 2026-09-19; Lot 6's reserved state has its meaning. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 6 +- CHANGELOG.md | 19 ++++- docs/design/06-notification-icon.md | 5 +- docs/design/09-robustness.md | 75 +++++++++++++++++-- docs/design/README.md | 2 +- docs/getting-started.md | 38 +++++++--- docs/how-it-works.md | 29 +++++++ .../recipes/fancontrol-fan-profiles/README.md | 7 +- docs/recipes/windows-power-plan/README.md | 7 +- docs/reference.md | 22 +++++- 10 files changed, 176 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2ecebaf..53ecc8b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,9 +73,9 @@ deleted. otherwise. - **The tray renders state and holds no rule.** What the icon, the tooltip and the menu show comes from objects that own the rules — the engine's - session, `update`'s phase — and the tray asks them what to draw and which - action a click means. A rule written in the menu code is in the wrong - place and cannot be tested. + session, `update`'s phase, the supervisor's verdict on the configuration + — and the tray asks them what to draw and which action a click means. A + rule written in the menu code is in the wrong place and cannot be tested. - **The setup commands are a contract with three callers.** `stop`, `init`, `install-task` and `uninstall-task` are sequenced by the package (`scripts/msi.ps1`), by the zip's after-exit shell in `update`, and by diff --git a/CHANGELOG.md b/CHANGELOG.md index 0390c80..62634a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,24 @@ a section is written. ## [Unreleased] -Nothing yet. +### Added + +- The watcher reads `config.toml` again whenever you save it, within a + second: no more stopping and restarting it after a change. A game in + progress is not disturbed, and the commands that run when it ends are the + ones you just saved. `log_dir` is the one setting that waits for the next + start; the log says so. +- A configuration that cannot be used shows as a **red, slashed icon**, and + the first line of the icon's menu says what is wrong — the line number and + the parser's words, or *the file is missing*. Nothing runs until it is + fixed; *Edit configuration* opens the file, and saving a good one brings + the icon back. + +### Changed + +- The watcher starts whatever the configuration file says, rather than + exiting with a code and no icon when the file is wrong at logon. `validate` + still reports the exit codes 3 and 4 for scripts. ## [0.2.0] - 2026-09-18 diff --git a/docs/design/06-notification-icon.md b/docs/design/06-notification-icon.md index f2374bb..e72ec44 100644 --- a/docs/design/06-notification-icon.md +++ b/docs/design/06-notification-icon.md @@ -128,7 +128,8 @@ because the size is logged at all. The idle icon first carried a diagonal slash, which in Windows iconography reads as *disabled* — and idle is the state the program spends nearly all its time in. The slash moved to a distinct **error** state, reserved and unused -until [Lot 9](09-robustness.md) gives it a meaning, so nobody borrows it for -anything else. The frames were checked rather than trusted: eight PNG frames +until [Lot 9](09-robustness.md) gave it a meaning on 2026-09-19 — the +configuration cannot be used and nothing is watched — so nobody borrowed it +for anything else meanwhile. The frames were checked rather than trusted: eight PNG frames per `.ico` at 32-bit alpha, no C2PA payload, and the four luminance figures from the design notes reproduce exactly. diff --git a/docs/design/09-robustness.md b/docs/design/09-robustness.md index c2f9b21..06b83f9 100644 --- a/docs/design/09-robustness.md +++ b/docs/design/09-robustness.md @@ -1,10 +1,11 @@ # Lot 9 — Robustness **Status: partly done.** The session marker is built and verified; the -configuration-fault design is decided and waiting; two smaller items remain. +configuration faults and the live reload are built and measured, waiting +for a real game session; two smaller items remain. - [x] Restore at the next start what a logoff could not — done 2026-09-16, a race fixed and re-verified 2026-09-17 -- [ ] Configuration faults shown in the tray, and live reload — designed, below +- [ ] Configuration faults shown in the tray, and live reload — built 2026-09-19 and measured without a game, below; closes on a reload during a real game session - [ ] Stop timing the refinement; let the OS say when — below - [ ] `ShutdownBlockReasonCreate`, so Windows' shutdown screen says what is being restored rather than naming the process - [ ] Behaviour across two games launched back to back @@ -144,11 +145,71 @@ editing goes through a staged copy, the only way to put an invalid file on disk is to edit it by hand outside the program, and then a frozen program with a red icon is the honest answer. -**Mechanics.** `serve` loads the configuration itself and takes the path rather -than a `Config`; the engine reads an `Arc>` at each use, so a -swap needs no wake-up. The tray gains a fault overlay on top of the session — -two different axes — and finally sets `State::Error`. Written so Lot 12 is -small: the watcher takes a path and an "apply" action. +**Mechanics, as planned.** `serve` loads the configuration itself and takes +the path rather than a `Config`; the engine reads an `Arc>` +at each use, so a swap needs no wake-up. The tray gains a fault overlay on +top of the session — two different axes — and finally sets `State::Error`. +Written so Lot 12 is small: the watcher takes a path and an "apply" action. + +**Mechanics, as built — 2026-09-19.** The `RwLock` was not built. Reading +the configuration at each use would have covered a *valid* change and left +the *invalid* one to new engine states: frozen while idle, frozen mid-game +with the writer's exit meaning nothing, then a recovery to re-run once the +file is valid again — each a branch in the loop and a scenario nobody had +written. The handover from [Lot 13](13-updating.md) already had every one of +those: an engine that stops with the session left open in the marker, and a +start that looks for the writer before recovering. So a change to the file +is a **handover from one engine to the next in the same process**: + +- `service` runs a supervisor on the worker thread — one engine per usable + configuration, built on the file as it is. The engine is unchanged but + for one line: a stop whose reason is `Reload` returns the way `Handover` + does, marker kept, nothing run. +- `StopSignal` gained a third reason and a **child**: a signal that is set + when either its own event or its parent's is, with the parent's reason + winning. The engine runs on a child of the process-wide stop; the child's + own event carries the reload and is reset between engines + (`take_reload`); the parent carries *Quit*, the logoff and `stop`, and is + never reset. That is what keeps a *Quit* arriving during a reload from + being lost, without a lock around the wait. +- `config::watch` is the thread: `FindFirstChangeNotificationW` on the + folder, waited on with the process stop, a 250 ms settle after the last + notification because editors write in several steps, and a comparison of + the file's *bytes* with what is running — a folder touched or a file + written back unchanged is not a reload, a file that no longer parses is. +- A file that cannot be used is a `LoadError` with a one-line `summary` + — `line 3: unknown field `log_levl`, expected one of …`, `the file is + missing`, `detection.poll_interval must be greater than zero` — reported + to the tray through a `FaultSink` beside the session sink. The tray reads + both facts together: a fault is `State::Error` on every surface, the + tooltip *configuration error*, the menu's first line `Configuration + error: ` and the summary, cut at 160 characters. The supervisor then parks + on the child signal: the next change or the process stop ends that, and + nothing else. +- What a reload applies to the log itself: `log_level` follows live through + a `reload::Layer` around the filter and an atomic for the fields, unless + `--log-level` or `RUST_LOG` fixed it at start; `log_dir` cannot follow — + the file is open — and is said at `warn` to wait for the next start. +- `Config::load` no longer makes the watcher exit: the command line runs + `serve` on the path alone, and the exit codes 3 and 4 are the other + commands'. The recovery that closes a session now reports *Idle* to the + tray, since after a reload the icon may still show the session the last + engine left open; a first start swallows it as a repeat. + +**Measured 2026-09-19, 01:58–02:00**, a development build on a scratch +configuration while the installed watcher was stopped, with no game: a +misspelt key at `T`, the `ERROR` line and the icon refreshed to `Error` at +`T + 250 ms` on the nose, the settle; the file fixed with `log_level` moved +to `info`, *Configuration reloaded* and the fields gone from the lines that +followed; back to `debug`, *Log level changed* and the fields back; a +`poll_interval` of zero, the validation's own sentence in the menu line; +the file deleted, *the file is missing*; the file back with `log_dir` +moved, the reload and the `warn` that the log waits; then `stop`, *Stopped*. +Six changes, six reloads, one process, 54 seconds. What is not measured is +the case the design is for: a reload while a game is running, which is a +real session with the marker resumed by the next engine — the scenario +`a_reload_mid_game_is_a_handover_to_the_next_engine` pins it, the field +run closes the item. ## Stop timing the refinement diff --git a/docs/design/README.md b/docs/design/README.md index b9e49b4..554577a 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -20,7 +20,7 @@ session rather than when the code compiles. Each has its own page. | 6 | [Notification area icon](06-notification-icon.md) | done | | 7 | [Icon, tooltip and menu as one state](07-tray-state.md) | done | | 8 | [Distribution](08-distribution.md) | done | -| 9 | [Robustness](09-robustness.md) | partly done | +| 9 | [Robustness](09-robustness.md) | partly done; faults and live reload built, closing on a real session | | 10 | [Configuration window](10-configuration-window.md) | proposed | | 11 | [Documentation for the people who use it](11-user-documentation.md) | done | | 12 | [Editing the configuration without breaking it](12-editing-on-a-copy.md) | proposed | diff --git a/docs/getting-started.md b/docs/getting-started.md index b919832..23b39fe 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -106,20 +106,31 @@ gamemode-executor trigger start gamemode-executor trigger stop ``` -The watcher reads the file when it starts, so after editing it, restart it: +Save the file, and that is all: the watcher notices within a second, reads +it again and writes `Configuration reloaded` in the log. Nothing to restart, +and a game in progress is not disturbed — the commands that run when it ends +are the ones you just saved. The one setting that waits for the next start +is `log_dir`, since the log is already open; the log says so. + +If the file cannot be used, the icon turns **red, with a slash**, and the +first line of its menu says what is wrong — the line number and the +parser's words, such as `Configuration error: line 3: unknown field +'log_levl'`. Nothing runs until you fix it: not the old commands, not their +stop half. **Edit configuration** still opens the file, and saving a good +one brings the icon back to grey. The same words are in the log, marked +`ERROR`. + +**That is the end of the setup.** Play. The commands fire by themselves. + +From the zip, one more command the first time, to register the task that +starts the watcher at every logon and to start it now: ```bash -gamemode-executor stop gamemode-executor install-task ``` -The first is **Quit** from the icon's menu, typed. The second starts it again -— and, from the zip, registers the task that starts it at every logon, once. -No administrator rights, no password, no window. Doing this while a game is -running? `stop --handover` instead of `stop`: the game session is left to the -new watcher, which takes it up where it was without running anything. - -**That is the end of the setup.** Play. The commands fire by themselves. +No administrator rights, no password, no window. The installer did this for +you. Want a complete worked example rather than a blank page? [Recipes](recipes/) has one per job, each with a `config.toml` you can copy straight over. @@ -133,6 +144,7 @@ the clock. It is the only thing this program ever puts on screen. | --- | --- | | **grey controller** | running, no game. What you will see almost all the time. | | **green controller** | a game is detected | +| **red controller, slashed** | the configuration cannot be used and nothing is watched until it is fixed; the menu's first line says what is wrong | Hover it and the tooltip names the game. Right-click and the first line of the menu says the same — it is greyed out because it is an answer, not a button. @@ -272,6 +284,14 @@ Check the log. A command that fails to start is recorded with the reason, and it never prevents the others from running. The usual cause is a wrong path, or a program that needs administrator rights (see above). +**The icon is red, with a slash.** +The configuration file cannot be used, and the first line of the icon's menu +says why — a line number and what the parser found there, or *the file is +missing*. Nothing runs until it is fixed: **Edit configuration** opens the +file, and the moment a usable one is saved the icon is grey again and the +log says `Configuration reloaded`. `gamemode-executor validate` tells the +same story in a terminal, with the parser's full account. + **I logged off during a game and the fans stayed loud.** They calm down at your next logon. Windows does not let the stop commands run once the session is ending, so the watcher runs them the moment it starts diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 241caee..84677c4 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -188,6 +188,35 @@ commands instead, as after a logoff. `gamemode-executor status` shows whether that file is there, and where. +## Changing the configuration + +The watcher does not read the file once and forget it. A small thread waits +on the folder's change notification — Windows' own, nothing polled — and +when the file's bytes change it stops the running engine and starts one on +the new file. Editors save in several steps, so the read waits a quarter of +a second after the last change; the log then says `Configuration reloaded`. +A file written back unchanged is not a reload. + +A game in progress survives it, by the same handover an update uses: the +engine stops without running anything, leaving the session open in the +[marker](#logging-off-mid-game), and the next engine finds the presence +writer still running and takes the session up where it was. The stop +commands that run when the game ends are the new ones. + +A file that cannot be used — a typo, a value the program refuses, a file +that is gone — does not stop the program and does not fall back to the last +good one. It **disables it outright**: the icon turns red, the menu's first +line says what is wrong, and nothing is watched until a usable file is +saved. Frozen, deliberately. Falling back to the last good configuration +would mean the program runs something other than what the file says, with +nothing on screen to say so; frozen with a red icon is an unambiguous +state, and *Edit configuration* is right there. While it is frozen a game +that ends gets no stop commands, and the session marker remembers that: the +moment a usable file is saved, the new engine settles what was left — the +stop commands run if the game is gone, the session resumes if it is still +on. `log_dir` is the one setting a reload cannot apply, because the log is +already open; it takes effect at the next start, and the log says so. + ## Removing it Uninstalling from *Programs and Features* removes what the installer put diff --git a/docs/recipes/fancontrol-fan-profiles/README.md b/docs/recipes/fancontrol-fan-profiles/README.md index 408f17e..ddeefa1 100644 --- a/docs/recipes/fancontrol-fan-profiles/README.md +++ b/docs/recipes/fancontrol-fan-profiles/README.md @@ -325,14 +325,13 @@ FanControl records the active configuration in a file called `CACHE`, in its ConvertFrom-Json).CurrentConfigFileName ``` -When both work, restart the watcher so it reads the file: +When both work, save the file: the watcher reads it again by itself and +the log says `Configuration reloaded`. From the zip, on a first install, +one more command registers the logon task and starts the watcher: ```bash -gamemode-executor stop gamemode-executor install-task ``` - -From the zip, the second line also registers the logon task, the first time. Done. Play a game and the fans follow. ## Worth knowing diff --git a/docs/recipes/windows-power-plan/README.md b/docs/recipes/windows-power-plan/README.md index 1a6b322..b6c6de7 100644 --- a/docs/recipes/windows-power-plan/README.md +++ b/docs/recipes/windows-power-plan/README.md @@ -81,15 +81,14 @@ powercfg /getactivescheme gamemode-executor trigger stop ``` -When both work, restart the watcher so it reads the file: +When both work, save the file: the watcher reads it again by itself and +the log says `Configuration reloaded`. From the zip, on a first install, +one more command registers the logon task and starts the watcher: ```bash -gamemode-executor stop gamemode-executor install-task ``` -From the zip, the second line also registers the logon task, the first time. - ## Worth knowing **On a laptop, Windows may override you.** Some machines switch plans by diff --git a/docs/reference.md b/docs/reference.md index 500e163..408f93a 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -89,6 +89,14 @@ args = ["/setactive", "SCHEME_BALANCED"] Write Windows paths between single quotes: TOML takes those literally, so backslashes need no doubling. +The watcher reads the file again whenever it changes — within about a +second of a save, the log says `Configuration reloaded` — and applies +everything but `log_dir`, which waits for the next start. A file it cannot +use disables it until one it can is saved: the icon turns red and its +menu's first line carries the reason; nothing runs meanwhile, and the exit +codes below are for the commands, since the watcher no longer exits over +the file. + ### Actions Each event has a mode and a list of commands. `series` runs each command after @@ -147,6 +155,10 @@ with no prompt. | 4 | configuration invalid: syntax or validation | | 5 | another instance is already running | +`3` and `4` come from `validate`, `status`, `trigger` and the setup commands, +which need a usable file. The watcher itself starts whatever the file says +and shows the fault in its icon instead. + ## The log `%LOCALAPPDATA%\GameModeExecutor\logs\gamemode-executor.log` unless `log_dir` @@ -164,9 +176,13 @@ One log serves two readers, and `log_level` is the dial between them: | `trace` | technician | Raw measurements. | `info` is reserved for what the program is for: a game detected, named or -gone, the watcher starting or stopping, a session recovered at start — and -what was done to this machine to set it up, which is the same story one -chapter earlier. Nothing else competes with those lines. +gone, the watcher starting or stopping, a session recovered at start, the +configuration reloaded — and what was done to this machine to set it up, +which is the same story one chapter earlier. Nothing else competes with +those lines. A configuration the watcher cannot use is an `error`, the one +line in the log that asks something of you: `The configuration cannot be +used, so nothing is watched until it is fixed: line 3: unknown field +'log_levl'`. Each line is `time LEVEL category message`, with the category one of `watcher`, `game`, `commands`, `setup` or `update`: From 82afe99522922b3fdfbdd9c5735df8fb7bea5e4d Mon Sep 17 00:00:00 2001 From: Geoffrey Vancoetsem <10533139+geeooff@users.noreply.github.com> Date: Sun, 20 Sep 2026 14:58:29 +0200 Subject: [PATCH 3/5] Refuse a misspelt log_level, and say a resumed session by its name Two things the field run of 2026-09-20 found. `log_level = "debg"` passed validation, gave the filter no directive it knew, and the log fell back to errors alone -- quiet for ninety seconds, the reload line that would have explained it filtered out with the rest. The five levels are now validated, case-insensitively, and a sixth word is a fault with the icon and the menu line. And the debug line at the writer's exit, after a resume, read "the game was never named" with a 34 s session: the resumed signal has a name and no process id, and the engine's clock starts at the resume. It now says the game was known by name only, from the resumed session, with the time since the resume. Lot 9's page records the run -- a reload and a fault, each during a Starfield session -- and closes the item. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 +++++ docs/design/09-robustness.md | 52 ++++++++++++++++++++++++++++++------ docs/design/README.md | 2 +- src/config.rs | 32 ++++++++++++++++++++++ src/engine.rs | 25 ++++++++++++----- 5 files changed, 102 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62634a8..f658fd3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,12 @@ a section is written. exiting with a code and no icon when the file is wrong at logon. `validate` still reports the exit codes 3 and 4 for scripts. +### Fixed + +- A misspelt `log_level` — `"debg"` — used to be accepted and to leave a + log with nothing but errors in it, as if the program had gone quiet. It is + now a configuration error like any other, named in the icon's menu. + ## [0.2.0] - 2026-09-18 The program updates itself from the icon, and a game session survives it. diff --git a/docs/design/09-robustness.md b/docs/design/09-robustness.md index 06b83f9..1dc747b 100644 --- a/docs/design/09-robustness.md +++ b/docs/design/09-robustness.md @@ -1,11 +1,11 @@ # Lot 9 — Robustness **Status: partly done.** The session marker is built and verified; the -configuration faults and the live reload are built and measured, waiting -for a real game session; two smaller items remain. +configuration faults and the live reload are built and verified in the +field; two smaller items remain. - [x] Restore at the next start what a logoff could not — done 2026-09-16, a race fixed and re-verified 2026-09-17 -- [ ] Configuration faults shown in the tray, and live reload — built 2026-09-19 and measured without a game, below; closes on a reload during a real game session +- [x] Configuration faults shown in the tray, and live reload — built 2026-09-19, measured without a game and then verified across two Starfield sessions on 2026-09-20, below - [ ] Stop timing the refinement; let the OS say when — below - [ ] `ShutdownBlockReasonCreate`, so Windows' shutdown screen says what is being restored rather than naming the process - [ ] Behaviour across two games launched back to back @@ -205,11 +205,47 @@ followed; back to `debug`, *Log level changed* and the fields back; a `poll_interval` of zero, the validation's own sentence in the menu line; the file deleted, *the file is missing*; the file back with `log_dir` moved, the reload and the `warn` that the log waits; then `stop`, *Stopped*. -Six changes, six reloads, one process, 54 seconds. What is not measured is -the case the design is for: a reload while a game is running, which is a -real session with the marker resumed by the next engine — the scenario -`a_reload_mid_game_is_a_handover_to_the_next_engine` pins it, the field -run closes the item. +Six changes, six reloads, one process, 54 seconds. + +**Verified in the field 2026-09-20, 14:41–14:54**, by the maintainer on +the installed copy, with the release build of the branch copied over it: + +- A misspelt key while idle: the `ERROR` line, the red icon, the tooltip + and the menu line; fixed, *Configuration reloaded*, the icon grey. The + maintainer's remark that the menu line is long — the parser's list of + expected fields — is accepted as it is, for want of a better single line. +- **A reload during a game.** Starfield detected at 14:47:32, the start + commands run; the stop action renamed in the file at 14:49:01: *Stopping + for a reload*, *Configuration reloaded*, *A session was left open with + Starfield.exe still running, so it resumes where it was* — no command + run, no beep, the icon green throughout. The game quit at 14:49:40 and + the stop commands that ran were the renamed ones: `FanControl - Idle + (reloaded)`. +- **A fault during a game.** Starfield again at 14:51:45; `gpu_sample` + misspelt at 14:52:43: the engine stopped, the red icon, and the game + quit into a frozen watcher — nothing ran, the fans stayed on the gaming + configuration, as the strict rule says. The file fixed at 14:54:39: + *Configuration reloaded*, then *The last session ended with Starfield.exe + still running and its stop commands never ran, so they run now*, and + the idle configuration came back by itself. Between the two, the icon + showed *playing Starfield.exe* for 6 ms — the session the stopped engine + had left in the tray, until the recovery reported *Idle* — which is the + reason that report exists. + +Two defects the run found, both fixed the same day: + +- `log_level = "debg"` was **not** a fault. `validate` did not look at the + value, the filter took no directive from it and fell back to `error` + alone, and the log went quiet from 14:42:33 to 14:44:07 — the *reloaded* + line that should have said what happened was itself filtered out. A + pre-existing hole, first seen because the reload made the file easy to + break: the five levels are now validated, case-insensitively, and a + sixth word is a fault with the icon and the menu line like any other. +- The debug line at the writer's exit, after a resume, read *the game was + never named* with a session of 34 s: the resumed signal has a name and + no process id, and the engine's clock started at the resume. It now says + the game was known by name only, from the resumed session, and gives the + time since the resume rather than a session length it cannot know. ## Stop timing the refinement diff --git a/docs/design/README.md b/docs/design/README.md index 554577a..03c2a16 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -20,7 +20,7 @@ session rather than when the code compiles. Each has its own page. | 6 | [Notification area icon](06-notification-icon.md) | done | | 7 | [Icon, tooltip and menu as one state](07-tray-state.md) | done | | 8 | [Distribution](08-distribution.md) | done | -| 9 | [Robustness](09-robustness.md) | partly done; faults and live reload built, closing on a real session | +| 9 | [Robustness](09-robustness.md) | partly done; faults and live reload done, two smaller items open | | 10 | [Configuration window](10-configuration-window.md) | proposed | | 11 | [Documentation for the people who use it](11-user-documentation.md) | done | | 12 | [Editing the configuration without breaking it](12-editing-on-a-copy.md) | proposed | diff --git a/src/config.rs b/src/config.rs index 0790f8b..9656d0f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -20,6 +20,9 @@ use crate::win::{FolderEvent, FolderWatch, StopSignal}; pub const CONFIG_FILE_NAME: &str = "config.toml"; pub const APP_DIR_NAME: &str = "GameModeExecutor"; +/// The five positions of `log_level`, the ones the log's filter reads. +pub const LOG_LEVELS: [&str; 5] = ["error", "warn", "info", "debug", "trace"]; + /// Root of the configuration file. #[derive(Debug, Clone, Default, Deserialize, Serialize)] #[serde(default, deny_unknown_fields)] @@ -274,6 +277,18 @@ impl Config { } pub fn validate(&self) -> Result<()> { + // A level the filter does not know used to be accepted and to leave + // an `error`-only log, silently -- found on 2026-09-20 with "debg". + // The dial has five positions and a sixth is a fault like any other. + if !LOG_LEVELS + .iter() + .any(|level| level.eq_ignore_ascii_case(&self.general.log_level)) + { + anyhow::bail!( + "general.log_level must be one of error, warn, info, debug or trace, not `{}`", + self.general.log_level + ); + } if self.detection.poll_interval.is_zero() { anyhow::bail!("detection.poll_interval must be greater than zero"); } @@ -565,6 +580,23 @@ mode = \"concurrent\" ); } + /// "debg" used to pass validation and leave a log with nothing but + /// errors in it, which the person then read as the program having gone + /// quiet. The log's own filter is what decides the five words. + #[test] + fn a_misspelt_log_level_is_a_fault_not_a_silent_log() { + let path = Path::new("config.toml"); + let error = Config::parse("[general]\nlog_level = \"debg\"\n", path).unwrap_err(); + assert!(matches!(error, LoadError::Invalid { .. }), "{error:?}"); + assert!(error.summary().contains("`debg`"), "{}", error.summary()); + for level in LOG_LEVELS { + for spelling in [level.to_owned(), level.to_ascii_uppercase()] { + let text = format!("[general]\nlog_level = \"{spelling}\"\n"); + Config::parse(&text, path).unwrap(); + } + } + } + #[test] fn unknown_keys_are_rejected() { assert!( diff --git a/src/engine.rs b/src/engine.rs index f5119c0..35c7763 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -248,7 +248,7 @@ impl Engine { continue; } WaitOutcome::WriterExited => { - self.log_writer_exit(session_start, signal.as_ref()); + self.log_writer_exit(session_start, signal.as_ref(), fresh); } } match self.writer_returns(stop) { @@ -378,26 +378,39 @@ impl Engine { /// log jumps straight from the start to the stop, and telling "Windows was /// slow" from "we were slow" needs Steam's own logs. So say whether the /// game we identified was already gone when Windows finally let go. - fn log_writer_exit(&self, session_start: Instant, signal: Option<&GameSignal>) { + /// + /// A resumed session has a name from the marker and no process id, and + /// this engine only saw the end of it: said as such, with the time since + /// the resume rather than a session length it cannot know. Seen on + /// 2026-09-20, when a session resumed after a reload was logged as + /// never named, 34 s long. + fn log_writer_exit(&self, session_start: Instant, signal: Option<&GameSignal>, fresh: bool) { let elapsed = session_start.elapsed(); let named = signal .and_then(|signal| signal.process_id) .map(|pid| (pid, self.sensor.is_running(pid))); - match named { - Some((pid, true)) => tracing::debug!( + match (named, signal) { + (Some((pid, true)), _) => tracing::debug!( target: target::GAME, pid, session = ?elapsed, "Windows released the presence writer while the identified game is still running" ), - Some((pid, false)) => tracing::debug!( + (Some((pid, false)), _) => tracing::debug!( target: target::GAME, pid, session = ?elapsed, "Windows released the presence writer; the identified game had already \ exited, so the wait since then was Windows, not this program" ), - None => tracing::debug!( + (None, Some(signal)) if !fresh => tracing::debug!( + target: target::GAME, + since_resumed = ?elapsed, + "Windows released the presence writer; {} was known by name only, from the \ + session this engine resumed, so whether it had already exited was not checked", + signal.name() + ), + (None, _) => tracing::debug!( target: target::GAME, session = ?elapsed, "Windows released the presence writer; the game was never named" From 848847002b28d49be4e8d70d8e74a270a8e7d661 Mon Sep 17 00:00:00 2001 From: Geoffrey Vancoetsem <10533139+geeooff@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:13:33 +0200 Subject: [PATCH 4/5] Say a configuration fault, and its end, with a notification On the maintainer's remark after the field run: the red icon is easy to miss at logon and the menu line was too long to read. A fault is now said with a silent notification carrying the shell's error glyph and the whole summary, at start and at every reload that fails, and the end of a fault is said too, so the person knows the watcher is watching again; a reload that stays usable says nothing. The menu keeps a headline, the summary cut before the parser's list of expected fields. The supervisor reports which transition each read is -- faulty, restored, usable -- because the notice depends on what came before; the tray owns the wording. The Discreet principle in AGENTS.md now names the two things the program says unasked. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 9 +- CHANGELOG.md | 11 +-- docs/design/09-robustness.md | 13 +++ docs/getting-started.md | 31 ++++--- docs/how-it-works.md | 15 ++-- src/config.rs | 41 ++++++++- src/service.rs | 17 +++- src/tray.rs | 169 ++++++++++++++++++++++++++++------- 8 files changed, 240 insertions(+), 66 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 53ecc8b..36da81a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,10 +30,11 @@ These decide most questions before they are asked. option first and argue for a fallback only if it protects something concrete. - **Discreet.** No dialogs, no windows, no sounds. The icon, its tooltip and - its menu are the whole user interface, plus a silent notification to - answer something the user clicked -- a menu closes on a click, as every - Windows menu does, and the answer has to reach them somewhere; the log is - the rest. + its menu are the whole user interface, plus a silent notification in two + cases only: to answer something the user clicked -- a menu closes on a + click, as every Windows menu does, and the answer has to reach them + somewhere -- and to say that the configuration cannot be used, and then + that it can again, the one state that needs them. The log is the rest. - **No elevation, no service, no telemetry, and no network the user did not ask for.** Recorded as non-goals in the design record with their reasons. The one connection the program ever opens is *Check for updates*, diff --git a/CHANGELOG.md b/CHANGELOG.md index f658fd3..e0cb17f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,11 +22,12 @@ a section is written. progress is not disturbed, and the commands that run when it ends are the ones you just saved. `log_dir` is the one setting that waits for the next start; the log says so. -- A configuration that cannot be used shows as a **red, slashed icon**, and - the first line of the icon's menu says what is wrong — the line number and - the parser's words, or *the file is missing*. Nothing runs until it is - fixed; *Edit configuration* opens the file, and saving a good one brings - the icon back. +- A configuration that cannot be used shows as a **red, slashed icon**, a + silent notification with the error glyph says what is wrong in full — the + line number, the parser's words and what it expected — and the first line + of the icon's menu keeps the short of it. Nothing runs until it is fixed; + *Edit configuration* opens the file, and saving a good one brings the + icon back, with a notification saying the watcher is watching again. ### Changed diff --git a/docs/design/09-robustness.md b/docs/design/09-robustness.md index 1dc747b..2c26ee6 100644 --- a/docs/design/09-robustness.md +++ b/docs/design/09-robustness.md @@ -232,6 +232,19 @@ the installed copy, with the release build of the branch copied over it: had left in the tray, until the recovery reported *Idle* — which is the reason that report exists. +**Said with a notification, decided 2026-09-20** on the maintainer's +remark after the run: the red icon is easy to miss at logon and the menu +line was too long to read. So a fault is said, silently, with the shell's +error glyph and the whole summary — at start and at every reload that +fails — and the end of a fault is said too, so the person knows the +watcher is back; a reload that stays usable says nothing. The menu line +keeps a headline, the summary cut before the parser's list of expected +fields. This is the second thing the program ever says unasked, beside the +answer to a click; the principle in `AGENTS.md` names both. The tray +decides the wording, the supervisor decides which transition it is — a +`Report` of *faulty*, *restored* or *usable* — since the notice depends on +what came before, which only the supervisor knows. + Two defects the run found, both fixed the same day: - `log_level = "debg"` was **not** a fault. `validate` did not look at the diff --git a/docs/getting-started.md b/docs/getting-started.md index 23b39fe..b15ea8f 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -112,13 +112,16 @@ and a game in progress is not disturbed — the commands that run when it ends are the ones you just saved. The one setting that waits for the next start is `log_dir`, since the log is already open; the log says so. -If the file cannot be used, the icon turns **red, with a slash**, and the -first line of its menu says what is wrong — the line number and the -parser's words, such as `Configuration error: line 3: unknown field -'log_levl'`. Nothing runs until you fix it: not the old commands, not their -stop half. **Edit configuration** still opens the file, and saving a good -one brings the icon back to grey. The same words are in the log, marked -`ERROR`. +If the file cannot be used, the icon turns **red, with a slash**, a +notification with the error glyph says what is wrong in full — the line +number and the parser's words, and what it expected instead — and the +first line of the icon's menu keeps the short of it, such as +`Configuration error: line 3: unknown field 'log_levl'`. Nothing runs until +you fix it: not the old commands, not their stop half. **Edit +configuration** still opens the file, and saving a good one brings the icon +back to grey, with a notification saying the watcher is watching again. +The same words are in the log, marked `ERROR`. The notifications are +silent, and Windows keeps them in its notification centre. **That is the end of the setup.** Play. The commands fire by themselves. @@ -285,12 +288,14 @@ never prevents the others from running. The usual cause is a wrong path, or a program that needs administrator rights (see above). **The icon is red, with a slash.** -The configuration file cannot be used, and the first line of the icon's menu -says why — a line number and what the parser found there, or *the file is -missing*. Nothing runs until it is fixed: **Edit configuration** opens the -file, and the moment a usable one is saved the icon is grey again and the -log says `Configuration reloaded`. `gamemode-executor validate` tells the -same story in a terminal, with the parser's full account. +The configuration file cannot be used. A notification said why when it +happened — it is still in Windows' notification centre — and the first line +of the icon's menu keeps the short of it: a line number and what the parser +found there, or *the file is missing*. Nothing runs until it is fixed: +**Edit configuration** opens the file, and the moment a usable one is saved +the icon is grey again, a notification says so, and the log says +`Configuration reloaded`. `gamemode-executor validate` tells the same story +in a terminal, with the parser's full account. **I logged off during a game and the fans stayed loud.** They calm down at your next logon. Windows does not let the stop commands run diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 84677c4..8180c8f 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -205,9 +205,11 @@ commands that run when the game ends are the new ones. A file that cannot be used — a typo, a value the program refuses, a file that is gone — does not stop the program and does not fall back to the last -good one. It **disables it outright**: the icon turns red, the menu's first -line says what is wrong, and nothing is watched until a usable file is -saved. Frozen, deliberately. Falling back to the last good configuration +good one. It **disables it outright**: the icon turns red, a notification +says what is wrong, the menu's first line keeps the short of it, and +nothing is watched until a usable file is saved; when one is, a +notification says the watcher is back. Frozen, deliberately. Falling back +to the last good configuration would mean the program runs something other than what the file says, with nothing on screen to say so; frozen with a red icon is an unambiguous state, and *Edit configuration* is right there. While it is frozen a game @@ -267,10 +269,11 @@ release is honest; the program has no code signature, and the design record says why. A file it downloads carries no mark of the web, so Windows' SmartScreen never sees it: the program vouches for it, through the hash. -The notification that answers *Check for updates* is the one time the +The notification that answers *Check for updates* is one of two times the program shows anything beyond its icon: a menu closes when you click in it, -so the answer has to reach you somewhere. It is silent, it respects your -quiet hours, and it only ever answers something you clicked. +so the answer has to reach you somewhere. The other is a configuration that +cannot be used, and its end — the one thing that needs you. Both are +silent and respect your quiet hours; nothing else is ever said. ## What it does not do diff --git a/src/config.rs b/src/config.rs index 9656d0f..4e2a463 100644 --- a/src/config.rs +++ b/src/config.rs @@ -240,12 +240,40 @@ impl LoadError { Self::Invalid { reason, .. } => reason.clone(), } } + + /// The summary without the parser's list of what it expected instead -- + /// `line 3: unknown field `log_levl`` -- for the one menu line, where + /// the list ran off the screen. The notification and the log keep the + /// whole of it. Asked for by the maintainer on 2026-09-20. + pub fn headline(&self) -> String { + let summary = self.summary(); + match summary.find(", expected one of") { + Some(cut) => summary[..cut].to_owned(), + None => summary, + } + } } -/// Told when the configuration becomes unusable, with why, and when it is -/// usable again, with `None`. The tray draws it; the supervisor in -/// `service` decides it. -pub type FaultSink = Arc) + Send + Sync>; +/// What the supervisor tells the tray about the configuration, each time +/// it reads the file. The tray draws the state and says the transitions; +/// which transition it is, is the supervisor's to know. +#[derive(Debug, Clone, Copy)] +pub enum Report<'a> { + /// The file cannot be used: the icon shows it and a notification says + /// why, at start and at every reload that fails -- the one thing in + /// the program that needs the person, so the one thing that is said + /// unasked. Decided 2026-09-20. + Faulty(&'a LoadError), + /// The file is usable again after a fault: the icon back, and a + /// notification says the watcher is watching again. + Restored, + /// The file is usable and was before: the icon as it is, nothing said. + Usable, +} + +/// Told what the supervisor found each time it read the file. The tray +/// draws it; the supervisor in `service` decides it. +pub type FaultSink = Arc) + Send + Sync>; impl Config { pub fn load(path: &Path) -> Result { @@ -631,6 +659,11 @@ log_levl = 1 "{summary}" ); assert!(!summary.contains('\n'), "one line: {summary:?}"); + assert!( + summary.contains("expected one of"), + "the whole of it: {summary}" + ); + assert_eq!(error.headline(), "line 3: unknown field `log_levl`"); // The console still gets the parser's own account, caret and all. assert!(format!("{:#}", anyhow::Error::new(error)).contains("not usable")); } diff --git a/src/service.rs b/src/service.rs index bcb85b7..6d0c60c 100644 --- a/src/service.rs +++ b/src/service.rs @@ -37,7 +37,7 @@ use std::time::{Duration, Instant}; use anyhow::{Context, Result}; -use crate::config::{self, Config, FaultSink, LoadError}; +use crate::config::{self, Config, FaultSink, LoadError, Report}; use crate::win::{SessionWindow, SingleInstance, StopReason, StopSignal}; use crate::{engine, logging, sensor, tray, update, win}; @@ -267,6 +267,10 @@ impl Supervised { /// cannot be used is shown and waited on; nothing runs meanwhile, not /// even the stop commands of a session that ends, which is what /// "disabled outright" means and why the marker is the right memory. + /// + /// The tray is told which transition each read is -- a fault, the end + /// of one, or a usable file that was usable before -- because it says + /// the first two with a notification and not the third. fn run( &mut self, sensor: &sensor::Windows, @@ -274,10 +278,16 @@ impl Supervised { stop: &StopSignal, ) -> Result<()> { let mut first = true; + let mut faulty = false; loop { match loaded { Ok(config) => { - (self.faults)(None); + (self.faults)(if faulty { + Report::Restored + } else { + Report::Usable + }); + faulty = false; // The log first, so the line below is written the way // the new file asks. self.follow(&config); @@ -296,7 +306,8 @@ impl Supervised { engine.run(stop)?; } Err(fault) => { - (self.faults)(Some(&fault)); + (self.faults)(Report::Faulty(&fault)); + faulty = true; tracing::error!( target: logging::target::WATCHER, path = %self.path.display(), diff --git a/src/tray.rs b/src/tray.rs index 9873ed1..b6adc6a 100644 --- a/src/tray.rs +++ b/src/tray.rs @@ -24,7 +24,7 @@ use anyhow::{Context, Result}; use windows::Win32::Foundation::{HWND, LPARAM, LRESULT, POINT, WPARAM}; use windows::Win32::UI::HiDpi::{GetDpiForWindow, GetSystemMetricsForDpi}; use windows::Win32::UI::Shell::{ - NIF_ICON, NIF_INFO, NIF_MESSAGE, NIF_SHOWTIP, NIF_TIP, NIIF_INFO, NIIF_NOSOUND, + NIF_ICON, NIF_INFO, NIF_MESSAGE, NIF_SHOWTIP, NIF_TIP, NIIF_ERROR, NIIF_INFO, NIIF_NOSOUND, NIIF_RESPECT_QUIET_TIME, NIM_ADD, NIM_DELETE, NIM_MODIFY, NIM_SETVERSION, NOTIFY_ICON_DATA_FLAGS, NOTIFY_ICON_INFOTIP_FLAGS, NOTIFYICON_VERSION_4, NOTIFYICONDATAW, Shell_NotifyIconW, ShellExecuteW, @@ -51,6 +51,10 @@ const WM_SESSION: u32 = WM_APP + 3; /// `WM_APP + 4` is `win`'s handover message. const WM_UPDATE: u32 = WM_APP + 5; +/// Posted by the supervisor when the configuration's verdict left a notice +/// to show: a fault, or the end of one. +const WM_CONFIG: u32 = WM_APP + 6; + /// One wording for a game Windows flags but does not name, shared by the /// tooltip and the menu and agreeing with what the log already says. Three /// surfaces disagreeing about the same fact is worse than any of them being @@ -79,6 +83,48 @@ static SESSION: std::sync::Mutex = std::sync::Mutex::new(Session::Idle) /// `Some`, so it takes precedence over the session on every surface. static FAULT: std::sync::Mutex> = std::sync::Mutex::new(None); +/// A notification waiting to be shown for the configuration, read by the +/// window's thread with nothing borrowed, as the updater's is. +static CONFIG_NOTICE: std::sync::Mutex> = std::sync::Mutex::new(None); + +/// What a notification looks like from the icon: the shell's info or error +/// glyph, a title and a text. The text holds 255 characters. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Notice { + pub kind: NoticeKind, + pub title: String, + pub text: String, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NoticeKind { + Info, + Error, +} + +/// The two notices the configuration can leave. Wording lives here, with +/// the other words the icon shows; when to say them is the supervisor's. +fn config_notice(report: &crate::config::Report<'_>) -> Option { + use crate::config::Report; + match report { + Report::Faulty(fault) => Some(Notice { + kind: NoticeKind::Error, + title: "Configuration error".to_owned(), + text: format!( + "{}\n\nNothing runs until the file is fixed: right-click the icon, Edit \ + configuration.", + fault.summary() + ), + }), + Report::Restored => Some(Notice { + kind: NoticeKind::Info, + title: "Configuration fixed".to_owned(), + text: "The file can be used again and the watcher is watching for games.".to_owned(), + }), + Report::Usable => None, + } +} + /// The two facts every surface is drawn from, read together so the icon, /// the tooltip and the menu cannot disagree about either. #[derive(Clone, Debug, PartialEq, Eq)] @@ -135,33 +181,55 @@ pub fn session_sink(window: isize) -> crate::engine::SessionSink { }) } -/// Hand this to the supervisor so it reports the configuration's faults -/// here: the summary is stored and the window's thread redraws from it. +/// Hand this to the supervisor so it reports what it found in the +/// configuration: the fault's headline is stored for the surfaces, and a +/// fault or the end of one leaves a notice; the window's thread redraws +/// and shows it. pub fn fault_sink(window: isize) -> crate::config::FaultSink { - Arc::new(move |fault: Option<&crate::config::LoadError>| { - let next = fault.map(crate::config::LoadError::summary); - { + Arc::new(move |report: crate::config::Report<'_>| { + let next = match report { + crate::config::Report::Faulty(fault) => Some(fault.headline()), + _ => None, + }; + let changed = { let mut held = FAULT .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - if *held == next { - return; - } + let changed = *held != next; *held = next; - } + changed + }; + let said = match config_notice(&report) { + Some(notice) => { + *CONFIG_NOTICE + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(notice); + true + } + None => false, + }; // SAFETY: posting carries no pointers, and a window that is gone makes // the call fail, which is ignored. unsafe { - let _ = PostMessageW( - Some(HWND(window as *mut std::ffi::c_void)), - WM_SESSION, - WPARAM(0), - LPARAM(0), - ); + let window = HWND(window as *mut std::ffi::c_void); + if changed { + let _ = PostMessageW(Some(window), WM_SESSION, WPARAM(0), LPARAM(0)); + } + if said { + let _ = PostMessageW(Some(window), WM_CONFIG, WPARAM(0), LPARAM(0)); + } } }) } +/// The configuration's pending notice, once. +fn take_config_notice() -> Option { + CONFIG_NOTICE + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .take() +} + /// Hand this to the updater so it wakes the window's thread when an /// outcome left a notice; the thread reads the notice itself. pub fn update_sink(window: isize) -> Arc { @@ -507,6 +575,8 @@ enum Plan { ReAdd, /// The updater has something to say; the notice is read with no borrow. Notify, + /// The configuration broke or was fixed; the notice is read the same way. + NotifyConfig, } /// Add the icon. Call once, from the thread owning `window`. @@ -589,7 +659,13 @@ pub fn dispatch(message: u32, wparam: WPARAM, lparam: LPARAM) -> Option } Plan::Notify => { if let Some(notice) = crate::update::take_notice() { - notify(¬ice.title, ¬ice.text); + notify(NoticeKind::Info, ¬ice.title, ¬ice.text); + } + Some(LRESULT(0)) + } + Plan::NotifyConfig => { + if let Some(notice) = take_config_notice() { + notify(notice.kind, ¬ice.title, ¬ice.text); } Some(LRESULT(0)) } @@ -621,6 +697,7 @@ impl Tray { // supervisor says the configuration broke or was fixed. WM_SESSION => Plan::Reload, WM_UPDATE => Plan::Notify, + WM_CONFIG => Plan::NotifyConfig, _ => Plan::Ignore, } } @@ -648,17 +725,22 @@ impl Tray { // --------------------------------------------------------------------------- /// A notification from the icon: the answer to something the user clicked, -/// since the menu they clicked in closed under them as every menu does. -/// Silent, and held back during quiet hours; Windows shows it as a toast -/// and keeps it in the notification centre. Never for anything the user -/// did not ask for. -fn notify(title: &str, text: &str) { +/// since the menu they clicked in closed under them as every menu does -- +/// or, since 2026-09-20, the one thing that needs them unasked: a +/// configuration that cannot be used, and its end. Silent, and held back +/// during quiet hours; Windows shows it as a toast and keeps it in the +/// notification centre. Never for anything else. +fn notify(kind: NoticeKind, title: &str, text: &str) { let Some(mut data) = TRAY.with(|cell| cell.borrow().as_ref().map(Tray::data)) else { return; }; data.uFlags = NOTIFY_ICON_DATA_FLAGS(data.uFlags.0 | NIF_INFO.0); + let glyph = match kind { + NoticeKind::Info => NIIF_INFO, + NoticeKind::Error => NIIF_ERROR, + }; data.dwInfoFlags = - NOTIFY_ICON_INFOTIP_FLAGS(NIIF_INFO.0 | NIIF_NOSOUND.0 | NIIF_RESPECT_QUIET_TIME.0); + NOTIFY_ICON_INFOTIP_FLAGS(glyph.0 | NIIF_NOSOUND.0 | NIIF_RESPECT_QUIET_TIME.0); let title_w = wide(title); let len = title_w.len().min(data.szInfoTitle.len() - 1); data.szInfoTitle[..len].copy_from_slice(&title_w[..len]); @@ -670,7 +752,7 @@ fn notify(title: &str, text: &str) { // borrowed while the shell handles it. if unsafe { Shell_NotifyIconW(NIM_MODIFY, &data) }.as_bool() { tracing::debug!( - target: crate::logging::target::UPDATE, + target: crate::logging::target::WATCHER, title, "Notification shown" ); @@ -1203,11 +1285,14 @@ mod tests { /// take turns. static SURFACES: std::sync::Mutex<()> = std::sync::Mutex::new(()); - /// The fault sink stores the summary, the surfaces switch to the error - /// state over whatever the session is, and `None` gives them back. - /// Window `0`, as below. + /// The fault sink stores the headline, the surfaces switch to the error + /// state over whatever the session is, and a usable file gives them + /// back. A fault and a restoration each leave one notice, with the + /// whole summary and the error glyph for the fault; a file that was + /// usable and still is leaves none. Window `0`, as below. #[test] - fn the_fault_sink_overlays_the_session_and_lifts_again() { + fn the_fault_sink_overlays_the_session_and_says_the_transitions() { + use crate::config::Report; let _turn = SURFACES .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -1215,14 +1300,36 @@ mod tests { let fault = crate::config::Config::parse("[general]\nlog_levl = 1\n", path).unwrap_err(); let sink = fault_sink(0); - sink(Some(&fault)); + sink(Report::Faulty(&fault)); assert_eq!(current_state(), State::Error); assert!(tooltip().contains("configuration error")); - assert!(menu_header().starts_with("Configuration error: line 2: unknown field")); + assert_eq!( + menu_header(), + "Configuration error: line 2: unknown field `log_levl`", + "the menu line stops before the parser's list" + ); + let notice = take_config_notice().expect("a fault is said"); + assert_eq!(notice.kind, NoticeKind::Error); + assert!(notice.text.contains("expected one of"), "{}", notice.text); + assert!( + notice.text.contains("Edit configuration"), + "{}", + notice.text + ); + assert!(take_config_notice().is_none(), "said once"); - sink(None); + sink(Report::Restored); assert_eq!(current_state(), State::Idle); assert!(tooltip().contains("no game")); + let notice = take_config_notice().expect("the end of a fault is said"); + assert_eq!(notice.kind, NoticeKind::Info); + + sink(Report::Usable); + assert_eq!(current_state(), State::Idle); + assert!( + take_config_notice().is_none(), + "a reload that stays usable is not said" + ); } /// The engine reports on every refinement, and most of those land on the From ad3bafd70bc2014014efc020609b70cb52027250 Mon Sep 17 00:00:00 2001 From: Geoffrey Vancoetsem <10533139+geeooff@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:28:14 +0200 Subject: [PATCH 5/5] Remember a configuration fault across a restart, so the fix is said The maintainer broke the file, stopped the watcher, fixed the file and started it again: no word, since the new process had never seen the fault. A fault is now noted in a second file beside the session marker, `configuration-fault`, removed when a usable file is read; a start that removes one says the fault is over, and a start on a file that was usable all along stays silent as before. `status` reports the file and `purge` removes it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 3 +- docs/design/09-robustness.md | 7 +++- docs/reference.md | 1 + src/cli.rs | 12 +++++++ src/marker.rs | 70 ++++++++++++++++++++++++++++++++++++ src/purge.rs | 8 +++++ src/service.rs | 33 +++++++++++++++-- 7 files changed, 130 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0cb17f..6137901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,8 @@ a section is written. line number, the parser's words and what it expected — and the first line of the icon's menu keeps the short of it. Nothing runs until it is fixed; *Edit configuration* opens the file, and saving a good one brings the - icon back, with a notification saying the watcher is watching again. + icon back, with a notification saying the watcher is watching again — + also when the fix came while the watcher was stopped. ### Changed diff --git a/docs/design/09-robustness.md b/docs/design/09-robustness.md index 2c26ee6..d02c1af 100644 --- a/docs/design/09-robustness.md +++ b/docs/design/09-robustness.md @@ -243,7 +243,12 @@ fields. This is the second thing the program ever says unasked, beside the answer to a click; the principle in `AGENTS.md` names both. The tray decides the wording, the supervisor decides which transition it is — a `Report` of *faulty*, *restored* or *usable* — since the notice depends on -what came before, which only the supervisor knows. +what came before, which only the supervisor knows. What came before +includes the last process: the maintainer broke the file, stopped, fixed +it, started again and got no word, so a fault is noted in a second file +beside the session marker, `configuration-fault`, removed when a usable +file is read, and a start that removes one says the fault is over. `purge` +removes it with the rest. Two defects the run found, both fixed the same day: diff --git a/docs/reference.md b/docs/reference.md index 408f93a..1f97d3d 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -236,6 +236,7 @@ syntax — `RUST_LOG=game=debug` for the detection lines alone. | Configuration | next to the executable, or `%APPDATA%\GameModeExecutor\config.toml` | yours; roams with the profile | | Log | `%LOCALAPPDATA%\GameModeExecutor\logs\` | disposable | | Session marker | `%LOCALAPPDATA%\GameModeExecutor\pending-stop-actions` | present while a game session is open; left behind by a logoff, shutdown, crash or handover, and settled at the next start — the session resumed if the game is still on, closed if it is gone. `status` reports it. | +| Fault marker | `%LOCALAPPDATA%\GameModeExecutor\configuration-fault` | present while the configuration cannot be used; removed when a usable one is read, which is how a watcher started on a file fixed meanwhile knows to say the fault is over | | Logon task | `\GameModeExecutor\Watcher` in Task Scheduler | records the absolute path of the executable; removed with the package, kept through an upgrade | | Updates | `%LOCALAPPDATA%\GameModeExecutor\updates\` | a downloaded release and the installer's log while an update runs; emptied when the next watcher starts, the log kept if the update failed | diff --git a/src/cli.rs b/src/cli.rs index b671e52..93aa94e 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -401,6 +401,18 @@ fn status() -> Result<()> { }, None => println!("Session marker : unavailable, no local profile"), } + // Left by a watcher that found the file unusable; a usable read removes + // it. Present here, the watcher is frozen or was when it last looked. + if let Some(marker) = marker::FaultMarker::in_local_dir() { + if marker.path().is_file() { + println!( + "Configuration fault : PRESENT - the watcher found the file unusable when it last read it ({})", + marker.path().display() + ); + } else { + println!("Configuration fault : none ({})", marker.path().display()); + } + } // Naming only, never detection. let known = KnownGames::load(); diff --git a/src/marker.rs b/src/marker.rs index 29b4e1c..3176ad2 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -29,6 +29,14 @@ use std::path::{Path, PathBuf}; /// comment lines inside are for whoever opens it anyway. pub const FILE_NAME: &str = "pending-stop-actions"; +/// The second file at the same root: "the configuration could not be used +/// when the watcher last looked". Written when a fault is found, removed +/// when a usable file is read -- and *that* removal, at a start, is what +/// tells the watcher to say the fault is over rather than start in silence. +/// The maintainer broke the file, stopped, fixed it, started again, and got +/// no word on 2026-09-20; a fault outlives the process, so its memory must. +pub const FAULT_FILE_NAME: &str = "configuration-fault"; + #[derive(Clone)] pub struct Marker { path: PathBuf, @@ -93,6 +101,54 @@ impl Marker { } } +/// "The configuration could not be used when the watcher last looked": +/// presence is the signal, the contents are for whoever opens the file. +#[derive(Clone)] +pub struct FaultMarker { + path: PathBuf, +} + +impl FaultMarker { + pub fn in_dir(dir: &Path) -> Self { + Self { + path: dir.join(FAULT_FILE_NAME), + } + } + + /// Beside the session marker, or `None` when Windows offers no local + /// profile. + pub fn in_local_dir() -> Option { + crate::config::local_dir().map(|dir| Self::in_dir(&dir)) + } + + pub fn path(&self) -> &Path { + &self.path + } + + /// Record a fault. Overwrites, so the file says the latest one. + pub fn note(&self, summary: &str, since: &str) -> io::Result<()> { + let text = format!( + "# GameModeExecutor: the configuration could not be used, so nothing is watched.\n\ + # Removed by the watcher once a usable file is read.\n\ + fault = {summary}\nsince = {since}\n" + ); + if let Some(dir) = self.path.parent() { + fs::create_dir_all(dir)?; + } + fs::write(&self.path, text) + } + + /// The configuration is usable: forget the fault. Says whether there + /// was one to forget, which is what a start needs to know. + pub fn clear(&self) -> io::Result { + match fs::remove_file(&self.path) { + Ok(()) => Ok(true), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(error), + } + } +} + /// Lenient on purpose: the file is written by this program, but a person may /// have opened it, and its presence matters more than its contents. fn parse(text: &str) -> Pending { @@ -126,6 +182,20 @@ mod tests { dir } + /// A fault is remembered across processes: noted, then cleared once, + /// and the clearing says whether there was anything to clear. + #[test] + fn a_fault_is_remembered_until_a_usable_file_clears_it() { + let marker = FaultMarker::in_dir(&scratch().join("fresh")); + assert!(!marker.clear().unwrap(), "nothing to forget at first"); + marker.note("line 3: unknown field `x`", "now").unwrap(); + assert!(marker.path().is_file()); + let text = fs::read_to_string(marker.path()).unwrap(); + assert!(text.contains("fault = line 3: unknown field `x`"), "{text}"); + assert!(marker.clear().unwrap(), "there was a fault to forget"); + assert!(!marker.clear().unwrap(), "and only once"); + } + #[test] fn opening_creates_the_folder_when_nothing_else_has() { // The log may be configured elsewhere, so the local folder can be diff --git a/src/purge.rs b/src/purge.rs index 35ef8cf..bb7774c 100644 --- a/src/purge.rs +++ b/src/purge.rs @@ -56,6 +56,7 @@ pub struct Layout { pub config_candidates: Vec, pub log: Option, pub marker: Option, + pub fault_marker: Option, pub local_dir: Option, pub roaming_dir: Option, pub exe_dir: Option, @@ -97,6 +98,9 @@ impl Plan { if let Some(marker) = &layout.marker { push(marker); } + if let Some(marker) = &layout.fault_marker { + push(marker); + } // Deepest first, so a parent is judged after its children are gone. // The log's folder counts as ours only inside the program's own @@ -216,6 +220,9 @@ pub fn discover(config: Option<&config::Config>, config_path: &Path) -> Layout { config_candidates: candidates, log: log_dir.map(|dir| dir.join(logging::LOG_FILE_NAME)), marker: local_dir.as_ref().map(|dir| dir.join(marker::FILE_NAME)), + fault_marker: local_dir + .as_ref() + .map(|dir| dir.join(marker::FAULT_FILE_NAME)), local_dir, roaming_dir: config::roaming_dir(), exe_dir: std::env::current_exe() @@ -334,6 +341,7 @@ mod tests { config_candidates: vec![exe_dir.join("config.toml"), roaming.join("config.toml")], log: Some(local.join("logs").join("gamemode-executor.log")), marker: Some(local.join(marker::FILE_NAME)), + fault_marker: Some(local.join(marker::FAULT_FILE_NAME)), local_dir: Some(local.clone()), roaming_dir: Some(roaming.clone()), exe_dir: Some(exe_dir.clone()), diff --git a/src/service.rs b/src/service.rs index 6d0c60c..d0a964a 100644 --- a/src/service.rs +++ b/src/service.rs @@ -165,6 +165,7 @@ pub fn serve(config_path: &Path, level: Option<&str>, console: bool) -> Result<( // State, so it lives with the local profile and not with the log, which // the user may have sent elsewhere and is entitled to empty. let marker = crate::marker::Marker::in_local_dir(); + let fault_marker = crate::marker::FaultMarker::in_local_dir(); // One engine run stops on this; the process stops on `stop`, which it // answers to as well. let run_stop = Arc::new(StopSignal::child_of(&stop)?); @@ -195,6 +196,7 @@ pub fn serve(config_path: &Path, level: Option<&str>, console: bool) -> Result<( sink, faults, marker, + fault_marker, }; let worker = std::thread::spawn(move || { let outcome = @@ -254,6 +256,9 @@ struct Supervised { sink: engine::SessionSink, faults: FaultSink, marker: Option, + /// Remembers a fault across processes, so a start on a file fixed while + /// the watcher was stopped still says the fault is over. + fault_marker: Option, } impl Supervised { @@ -270,7 +275,8 @@ impl Supervised { /// /// The tray is told which transition each read is -- a fault, the end /// of one, or a usable file that was usable before -- because it says - /// the first two with a notification and not the third. + /// the first two with a notification and not the third. "Before" + /// includes the last process: the fault marker carries it across. fn run( &mut self, sensor: &sensor::Windows, @@ -282,7 +288,19 @@ impl Supervised { loop { match loaded { Ok(config) => { - (self.faults)(if faulty { + let remembered = match &self.fault_marker { + Some(marker) => marker.clear().unwrap_or_else(|error| { + tracing::warn!( + target: logging::target::WATCHER, + path = %marker.path().display(), + error = %error, + "Cannot remove the configuration-fault marker" + ); + false + }), + None => false, + }; + (self.faults)(if faulty || remembered { Report::Restored } else { Report::Usable @@ -308,6 +326,17 @@ impl Supervised { Err(fault) => { (self.faults)(Report::Faulty(&fault)); faulty = true; + if let Some(marker) = &self.fault_marker + && let Err(error) = marker.note(&fault.summary(), &logging::local_now()) + { + tracing::warn!( + target: logging::target::WATCHER, + path = %marker.path().display(), + error = %error, + "Cannot write the configuration-fault marker, so a start after the \ + fix will not say the fault is over" + ); + } tracing::error!( target: logging::target::WATCHER, path = %self.path.display(),