diff --git a/AGENTS.md b/AGENTS.md index 2ecebaf..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*, @@ -73,9 +74,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..6137901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,32 @@ 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**, 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 — + also when the fix came while the watcher was stopped. + +### 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. + +### 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 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/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..d02c1af 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 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 — designed, below +- [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 @@ -144,11 +145,125 @@ 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. + +**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. + +**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. 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: + +- `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 b9e49b4..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 | +| 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/docs/getting-started.md b/docs/getting-started.md index b919832..b15ea8f 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -106,20 +106,34 @@ 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**, 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. + +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 +147,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 +287,16 @@ 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. 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 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..8180c8f 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -188,6 +188,37 @@ 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, 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 +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 @@ -238,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/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..1f97d3d 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`: @@ -220,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 40d9a38..93aa94e 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"), } } @@ -395,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/config.rs b/src/config.rs index 7df16cf..4e2a463 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,15 +1,28 @@ //! 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"; +/// 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)] @@ -149,45 +162,161 @@ 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 {} +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, + } + } +} -/// The configuration file is present but unusable, whether it failed to parse -/// or failed validation. -#[derive(Debug)] -pub struct Invalid(pub PathBuf); +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::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()) + /// 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, + } } } -impl std::error::Error for Invalid {} +/// 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 { - 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) } 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"); } @@ -247,6 +376,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. @@ -414,6 +608,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!( @@ -425,4 +636,113 @@ 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:?}"); + 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")); + } + + #[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..35c7763 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, @@ -243,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) { @@ -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 @@ -362,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" 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..3176ad2 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -29,6 +29,15 @@ 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, } @@ -92,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 { @@ -125,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/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..d0a964a 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, Report}; 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,49 @@ 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(); + 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)?); + // 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, + fault_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 +219,167 @@ 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, + /// 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 { + /// 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. + /// + /// 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. "Before" + /// includes the last process: the fault marker carries it across. + fn run( + &mut self, + sensor: &sensor::Windows, + mut loaded: Result, + stop: &StopSignal, + ) -> Result<()> { + let mut first = true; + let mut faulty = false; + loop { + match loaded { + Ok(config) => { + 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 + }); + faulty = false; + // 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)(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(), + "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 +419,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..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, @@ -43,13 +43,18 @@ 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. /// `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 @@ -72,11 +77,73 @@ 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); + +/// 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)] +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 +181,55 @@ pub fn session_sink(window: isize) -> crate::engine::SessionSink { }) } +/// 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 |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); + 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 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 { @@ -260,11 +376,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 +394,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 +499,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, } @@ -444,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`. @@ -526,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)) } @@ -554,9 +693,11 @@ 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, + WM_CONFIG => Plan::NotifyConfig, _ => Plan::Ignore, } } @@ -584,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]); @@ -606,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" ); @@ -1066,22 +1212,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 +1259,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 +1273,65 @@ 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 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_says_the_transitions() { + use crate::config::Report; + 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(Report::Faulty(&fault)); + assert_eq!(current_state(), State::Error); + assert!(tooltip().contains("configuration error")); + 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(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 /// same name. Without the early return the shell would be asked to redraw /// an identical icon each time. @@ -1113,6 +1340,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 +1354,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) }; + } +}