diff --git a/.github/scripts/check-package-contents.sh b/.github/scripts/check-package-contents.sh index 39a03c0..07dc491 100755 --- a/.github/scripts/check-package-contents.sh +++ b/.github/scripts/check-package-contents.sh @@ -3,7 +3,8 @@ # Check what the built bigame-mode package installs: every file Big Game Mode # needs at runtime, with sane modes and owners, and the references between # them (desktop entry -> binary and icon, D-Bus activation -> systemd unit -> -# binary, bus policy -> bus name, AppStream -> desktop entry). +# binary, bus policy -> bus name, AppStream -> desktop entry, Polkit policy -> +# its source with the translations merged in). # # .github/scripts/check-package-contents.sh # @@ -163,6 +164,29 @@ if [[ -f "$bus_policy" ]]; then fi fi +polkit_policy="$root/usr/share/polkit-1/actions/${app_id}.policy" +if [[ -f "$polkit_policy" ]]; then + # The installed policy is the source with the catalogues merged in: the + # same actions, each with its English description and message as written. + source_policy="$repo/data/${app_id}.policy" + if [[ "$(grep -o '' "$polkit_policy")" == "$(grep -E '<(description|message)>' "$source_policy")" ]]; then + ok "Polkit policy keeps every English description and message" + else + fail "Polkit policy's English descriptions or messages differ from the source" + fi + # polkit matches pt_BR, never msgfmt's pt-BR. + if grep -qE 'xml:lang="[a-z]{2,3}-[A-Z]{2}"' "$polkit_policy"; then + fail "Polkit policy has a translation tagged ll-CC, which polkit never serves" + else + ok "Polkit policy's translations are tagged as polkit looks them up" + fi +fi + if [[ -f "$metainfo" ]]; then launchable="$(sed -n 's:.*\(.*\).*:\1:p' "$metainfo")" want_same "AppStream launchable" "$launchable" "${app_id}.desktop" diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index 50cfc69..501b59b 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -5,7 +5,7 @@ name: Backend tests # # Not run on its own for pushes or pull requests: build-package.yml calls it # beside the package, so each push or pull request is one run with both -# jobs (see docs/ci-workflows-audit.md). A manual run checks any branch. +# jobs (see docs/CI.md). A manual run checks any branch. on: workflow_call: diff --git a/.github/workflows/build-package.yml b/.github/workflows/build-package.yml index fa500ed..687d812 100644 --- a/.github/workflows/build-package.yml +++ b/.github/workflows/build-package.yml @@ -12,7 +12,7 @@ name: Build Package # stable-*, and manual runs of such a branch. A failed run is not built, and # a branch with '/' breaks their recipe, so pull requests are checked by # package-check.yml, which calls this workflow under another name. -# See docs/ci-workflows-audit.md. +# See docs/CI.md. on: push: @@ -245,7 +245,7 @@ jobs: echo "errors=$errors" >> "$GITHUB_OUTPUT" echo "warnings=$warnings" >> "$GITHUB_OUTPUT" # Errors fail the build. Warnings are reviewed in - # docs/ci-workflows-audit.md and shown in the run summary: the + # docs/CI.md and shown in the run summary: the # owned empty directories, libraries GTK already pulls in, and # programs or services the package runs rather than links. if (( errors )); then diff --git a/.github/workflows/package-check.yml b/.github/workflows/package-check.yml index 9935578..bf6f10a 100644 --- a/.github/workflows/package-check.yml +++ b/.github/workflows/package-check.yml @@ -4,7 +4,7 @@ name: Package check # and the package), for pull requests and manual runs of any branch, as one # run. Under this name a successful run is not handed to the BigLinux package # builders, which act on "Build Package" runs only (see build-package.yml). -# See docs/ci-workflows-audit.md. +# See docs/CI.md. on: pull_request: diff --git a/.gitignore b/.gitignore index 9657a5f..8fd1b92 100644 --- a/.gitignore +++ b/.gitignore @@ -35,6 +35,8 @@ target/ .serena/ .claude/ .DS_Store +Thumbs.db +*.save __pycache__/ .pytest_cache/ .mypy_cache/ diff --git a/README.md b/README.md index 7451868..f7ce18c 100644 --- a/README.md +++ b/README.md @@ -9,9 +9,9 @@ One switch for per-game performance, presets for what games should favour, AI Graphics with full backup and undo, and a page that shows — with evidence — what is really in effect. -[![Version](https://img.shields.io/badge/version-2.3.1-6c5ce7?style=for-the-badge)](https://github.com/biglinux/bigamemode/releases) +[![Version](https://img.shields.io/badge/version-2.3.2-6c5ce7?style=for-the-badge)](https://github.com/biglinux/bigamemode/releases) [![License](https://img.shields.io/badge/license-GPL--3.0--or--later-2ea44f?style=for-the-badge)](LICENSE) -[![Rust](https://img.shields.io/badge/Rust-1.85%2B-dea584?style=for-the-badge&logo=rust&logoColor=white)](https://www.rust-lang.org/) +[![Rust](https://img.shields.io/badge/Rust-1.88%2B-dea584?style=for-the-badge&logo=rust&logoColor=white)](https://www.rust-lang.org/) [![GTK4](https://img.shields.io/badge/GTK4-libadwaita-4a86cf?style=for-the-badge&logo=gnome&logoColor=white)](https://gnome.pages.gitlab.gnome.org/libadwaita/) [![Languages](https://img.shields.io/badge/languages-29-f39c12?style=for-the-badge)](#languages) @@ -47,26 +47,48 @@ a way back. > One rule runs through the project: **nothing is offered that the machine > cannot do, and nothing is called an improvement without a measurement.** -## What's new in 2.3.1 - -- **Processors without AVX2/BMI2** (Sandy Bridge, Ivy Bridge and older): - falcond 2.0.14 built for x86-64-v3 dies there with an illegal instruction - ([#4](https://github.com/biglinux/bigamemode/issues/4)). Big Game Mode now - names that failure (Home, Details, the tray and the support report) instead - of suggesting a restart, and **Turbo still comes on** with the general - settings, as on a system without falcond. That falcond build is not started - again until the package changes; a falcond built for the x86-64 baseline is - used as soon as it is installed. See - [docs/issue-4-falcond-sigill.md](docs/issue-4-falcond-sigill.md). -- **A falcond that crashes at start** no longer stays enabled to crash again - at every boot, and Turbo can be turned off after it. -- **Game detection:** the Steam runtime's library cache tool (`ldconfig`) is - no longer taken for the running game. -- **Details:** fix commands with `&&` no longer log a markup error. -- **Packaging:** the BigLinux package builders receive only the branches they - can build (`main`, `testing-*`, `stable-*`); pull requests are checked by a - separate workflow. See - [docs/ci-workflows-audit.md](docs/ci-workflows-audit.md). +## What's new in 2.3.2 + +A stability release, from a full review of the code, the helper and the +package. + +- **Turbo on processors falcond cannot run on** (Sandy Bridge, Ivy Bridge and + older with a falcond built for x86-64-v3) no longer switches itself off a + few seconds after it starts, while the power profile stayed at performance. + See [CPU compatibility](docs/CPU-COMPATIBILITY.md). +- **Nothing left behind:** turning Turbo off or changing the preset reaches + Steam and Heroic once they are closed, and deleting a profile takes its + launch options out of Steam, Heroic and Lutris. Profiles says which launcher + to close when one was open. +- **Your settings stay yours:** only what Big Game Mode added is removed, + account by account (a native and a Flatpak Steam are two installs); your + own launch options, Lutris' MangoHud line, and MangoHud and vkBasalt files + (also ones that are not UTF-8, and dotfile-manager symlinks) are kept. +- **AI Graphics** makes one change at a time to a game, so a double click can + no longer mix two installs' backups; Restore from Profiles works as on the + AI Graphics page. +- **Idle cost:** one systemd reading for the tray and Home, no property cache + traffic, slower polling while a game runs, and nothing woken while the + window is hidden. +- **Game detection:** Electron/Chromium helpers, the Steam runtime's locale + tools, Windows installers (`msiexec`, .NET) and crash reporters are not + taken for the game. +- **A stricter helper:** `/run` and the machine-wide `/sys` trees read-only, + memory and task limits, no systemd reload when nothing changes, profiles + named after system processes refused. Its Polkit messages are now + translated with the rest of the application. +- **Quality:** Rust 1.88 declared (the real minimum), French, Chinese and + other languages get their own "label: value" punctuation, and the About + dialog names the developer. + +### 2.3.1 + +- Processors without AVX2/BMI2: a falcond built for x86-64-v3 is named as + the cause instead of a retry being suggested, and Turbo comes on with the + general settings + ([#4](https://github.com/biglinux/bigamemode/issues/4)). +- A falcond that crashes at start no longer stays enabled; the Steam + runtime's `ldconfig` is not taken for the game. ### 2.3.0 @@ -311,7 +333,7 @@ terminal without opening a window (`--network` adds the DNS measurements). | A frame generation option is disabled | lsfg-vk is not installed or your `Lossless.dll` is not usable by it; the row says which | | AI Graphics left a game broken | Profiles → the game's menu → **Restore the game's graphics**, or AI Graphics → **Repair** | | No tray icon on GNOME | Install and enable an AppIndicator extension | -| "falcond is not compatible with this processor" | The installed falcond was built for a newer x86-64 level than this processor's (falcond 2.0.14 built for x86-64-v3 stops with SIGILL without AVX2/BMI2). Turning Turbo off and on again does not help: update falcond once a build for the x86-64 baseline is available. [CPU compatibility](docs/CPU-COMPATIBILITY.md) | +| "falcond is not compatible with this processor" | The installed falcond was built for a newer x86-64 level than this processor's (falcond 2.0.14 built for x86-64-v3 stops with SIGILL without AVX2/BMI2). Turbo still comes on with the general settings; per-game profiles need a falcond built for the x86-64 baseline. Turning Turbo off and on again does not help. [CPU compatibility](docs/CPU-COMPATIBILITY.md) | | Something else | Details → **Support report**, or `bigame-ui --diagnostics`, and the **Logs** page | ## How it works @@ -350,8 +372,9 @@ More in [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md). ``` - **The interface never runs as root.** Only a small helper on the system bus - is privileged, confined by systemd (`ProtectSystem=strict`, `/sys` read-only - except `/sys/devices`, one capability). + is privileged, confined by systemd (`ProtectSystem=strict`, `/run` and `/sys` + read-only except the CPU frequency and device attributes it sets, one + capability, memory and task limits). - **Every privileged method is authorised by Polkit first** (without Polkit, access is denied) and every argument is validated on the root side. - **Narrow, atomic writes** to one known place each, never through a path the @@ -402,7 +425,7 @@ languages are welcome. ## Development -Requires Rust 1.85 or newer, GTK 4.14+, libadwaita 1.7+, +Requires Rust 1.88 or newer, GTK 4.14+, libadwaita 1.7+, `glib-compile-resources` (glib2) and, for the catalogues, gettext and Python 3. ```bash @@ -471,7 +494,7 @@ template, compiles the catalogues and runs `cargo test --release --frozen --workspace` in `check()`. One run checks every pull request and every push (details in -[`docs/ci-workflows-audit.md`](docs/ci-workflows-audit.md)), with two jobs in +[`docs/CI.md`](docs/CI.md)), with two jobs in parallel: - the code (`.github/workflows/backend-tests.yml`): formatting, Clippy with @@ -489,7 +512,7 @@ as *Package check*, which is not handed to them. ## Author -**Rafael Ruscher** · +**Rafael Ruscher**, author and lead developer · I have always loved games, and I am a firm believer in gaming on Linux. In recent years, with Valve's constant work, compatibility has become almost diff --git a/bigame-engine/Cargo.lock b/bigame-engine/Cargo.lock index fc9e6dd..0e49550 100644 --- a/bigame-engine/Cargo.lock +++ b/bigame-engine/Cargo.lock @@ -59,7 +59,7 @@ checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" [[package]] name = "bigame-core" -version = "2.3.1" +version = "2.3.2" dependencies = [ "anyhow", "libc", @@ -77,7 +77,7 @@ dependencies = [ [[package]] name = "bigame-daemon" -version = "2.3.1" +version = "2.3.2" dependencies = [ "anyhow", "bigame-core", @@ -92,7 +92,7 @@ dependencies = [ [[package]] name = "bigame-ui" -version = "2.3.1" +version = "2.3.2" dependencies = [ "anyhow", "bigame-core", diff --git a/bigame-engine/Cargo.toml b/bigame-engine/Cargo.toml index c9e24f9..314e3ef 100644 --- a/bigame-engine/Cargo.toml +++ b/bigame-engine/Cargo.toml @@ -3,11 +3,11 @@ resolver = "2" members = ["bigame-core", "bigame-daemon", "bigame-ui"] [workspace.package] -version = "2.3.1" +version = "2.3.2" authors = ["Rafael Ruscher "] edition = "2024" license = "GPL-3.0-or-later" -rust-version = "1.85" +rust-version = "1.88" [workspace.lints.clippy] pedantic = { level = "warn", priority = -1 } diff --git a/bigame-engine/bigame-core/examples/detect.rs b/bigame-engine/bigame-core/examples/detect.rs index d38ee26..677781c 100644 --- a/bigame-engine/bigame-core/examples/detect.rs +++ b/bigame-engine/bigame-core/examples/detect.rs @@ -29,12 +29,13 @@ fn main() { " outputs={:?} hwmon={:?}", g.connected_outputs, g.hwmon ); + let sample = bigame_core::gpu_telemetry::sample(g); println!( - " dpm={:?} busy={:?} temp={:?} power_uw={:?}", + " dpm={:?} busy={:?} temp_c={:?} power_w={:?}", g.dpm_level(), - g.busy_percent(), - g.hwmon_u64("temp1_input"), - g.hwmon_u64("power1_average") + sample.busy_pct, + sample.temp_c, + sample.power_w ); } println!("render {:?}", hw.render_gpu().map(|g| &g.card)); diff --git a/bigame-engine/bigame-core/examples/falcond_control.rs b/bigame-engine/bigame-core/examples/falcond_control.rs index 1735cfe..c105641 100644 --- a/bigame-engine/bigame-core/examples/falcond_control.rs +++ b/bigame-engine/bigame-core/examples/falcond_control.rs @@ -9,14 +9,13 @@ fn main() -> anyhow::Result<()> { use bigame_core::turbo; let what = std::env::args().nth(1).unwrap_or_else(|| "status".into()); - if what != "status" { - if let Some(secs) = std::env::var("BIGAME_WAIT") + if what != "status" + && let Some(secs) = std::env::var("BIGAME_WAIT") .ok() .and_then(|s| s.parse().ok()) - { - println!("pid {}", std::process::id()); - std::thread::sleep(std::time::Duration::from_secs(secs)); - } + { + println!("pid {}", std::process::id()); + std::thread::sleep(std::time::Duration::from_secs(secs)); } match what.as_str() { "release" => { diff --git a/bigame-engine/bigame-core/src/benchmark/lab.rs b/bigame-engine/bigame-core/src/benchmark/lab.rs index 80f4c58..befb15d 100644 --- a/bigame-engine/bigame-core/src/benchmark/lab.rs +++ b/bigame-engine/bigame-core/src/benchmark/lab.rs @@ -234,7 +234,6 @@ impl Session { /// The report a person reads. #[must_use] - #[allow(clippy::too_many_lines)] pub fn to_markdown(&self, comparisons: &[Comparison]) -> String { let mut out = String::new(); let _ = writeln!(out, "# {} — {}\n", self.workload, self.date); diff --git a/bigame-engine/bigame-core/src/benchmark/mod.rs b/bigame-engine/bigame-core/src/benchmark/mod.rs index 7dfdb6f..6588fe0 100644 --- a/bigame-engine/bigame-core/src/benchmark/mod.rs +++ b/bigame-engine/bigame-core/src/benchmark/mod.rs @@ -123,11 +123,11 @@ pub fn parse_mangohud_csv(content: &str) -> Result { push_col(&mut capture.cpu_temp, &cols, index("cpu_temp")); push_col(&mut capture.gpu_power, &cols, index("gpu_power")); - if let Some(i) = i_elapsed { - if let Some(v) = cols.get(i).and_then(|c| c.parse::().ok()) { - first_elapsed.get_or_insert(v); - last_elapsed = Some(v); - } + if let Some(i) = i_elapsed + && let Some(v) = cols.get(i).and_then(|c| c.parse::().ok()) + { + first_elapsed.get_or_insert(v); + last_elapsed = Some(v); } } diff --git a/bigame-engine/bigame-core/src/benchmark/result.rs b/bigame-engine/bigame-core/src/benchmark/result.rs index 16371ea..2d9cf51 100644 --- a/bigame-engine/bigame-core/src/benchmark/result.rs +++ b/bigame-engine/bigame-core/src/benchmark/result.rs @@ -63,7 +63,7 @@ impl ArmSummary { let mut sorted = runs.clone(); sorted.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal)); - let median = if n % 2 == 0 { + let median = if n.is_multiple_of(2) { f64::midpoint(sorted[n / 2 - 1], sorted[n / 2]) } else { sorted[n / 2] diff --git a/bigame-engine/bigame-core/src/booster/knob.rs b/bigame-engine/bigame-core/src/booster/knob.rs index bf3e9fe..50b9d3d 100644 --- a/bigame-engine/bigame-core/src/booster/knob.rs +++ b/bigame-engine/bigame-core/src/booster/knob.rs @@ -11,7 +11,7 @@ //! job. use std::fmt; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; @@ -88,8 +88,7 @@ impl Knob { pub fn read(&self) -> Option { match self { Self::PowerProfile => crate::dbus::power_profile_get(), - Self::CpuGovernor => read_sysfs(&cpu_attr_path(0, "scaling_governor")), - Self::CpuEpp => read_sysfs(&cpu_attr_path(0, "energy_performance_preference")), + Self::CpuGovernor | Self::CpuEpp => self.read_cpus().uniform(), Self::GpuDpmLevel { card } => read_sysfs(&PathBuf::from(format!( "/sys/class/drm/{card}/device/power_dpm_force_performance_level" ))), @@ -97,6 +96,24 @@ impl Knob { } } + /// The sysfs attribute of a knob set on every online CPU. + fn per_cpu_attr(&self) -> Option<&'static str> { + match self { + Self::CpuGovernor => Some("scaling_governor"), + Self::CpuEpp => Some("energy_performance_preference"), + _ => None, + } + } + + /// A per-CPU knob as every online CPU holds it. cpu0 alone is not the + /// knob: a write that failed on some CPUs leaves the others changed, + /// and reading cpu0 back at its baseline would call that restored. + fn read_cpus(&self) -> CpuValues { + self.per_cpu_attr().map_or(CpuValues::Unreadable, |attr| { + read_cpus(Path::new(CPU_ROOT), attr) + }) + } + /// Values this knob will accept on this machine. /// /// Empty means "this knob does not exist here", which the planner treats as @@ -226,7 +243,16 @@ impl Knob { /// changed", and it is mandatory for every applied change. #[must_use] pub fn verify(&self, expected: &str) -> Verification { - match self.read() { + let read = if self.per_cpu_attr().is_some() { + match self.read_cpus() { + CpuValues::Uniform(v) => Some(v), + CpuValues::Mixed(actual) => return Verification::Mismatch { actual }, + CpuValues::Unreadable => None, + } + } else { + self.read() + }; + match read { Some(actual) if actual == expected => Verification::Confirmed, Some(actual) => Verification::Mismatch { actual }, None => Verification::Unreadable, @@ -281,8 +307,64 @@ impl Verification { // ── sysfs helpers ──────────────────────────────────────────────────────────── +const CPU_ROOT: &str = "/sys/devices/system/cpu"; + fn cpu_attr_path(cpu: u32, attr: &str) -> PathBuf { - PathBuf::from(format!("/sys/devices/system/cpu/cpu{cpu}/cpufreq/{attr}")) + PathBuf::from(format!("{CPU_ROOT}/cpu{cpu}/cpufreq/{attr}")) +} + +/// What the online CPUs hold for one cpufreq attribute. +#[derive(Debug, Clone, PartialEq, Eq)] +enum CpuValues { + /// Every CPU that has the attribute holds this value. + Uniform(String), + /// They disagree: each distinct value, in CPU order, for the report. + Mixed(String), + /// No CPU has it, or one that has it could not be read. + Unreadable, +} + +impl CpuValues { + /// The one value, if there is one. Mixed values are no baseline (one + /// write cannot put them back) and never "already restored". + fn uniform(self) -> Option { + match self { + Self::Uniform(v) => Some(v), + Self::Mixed(_) | Self::Unreadable => None, + } + } +} + +/// `attr` on every CPU under `root` that has it, as the daemon writes it: +/// a CPU without the attribute is offline or not driven by cpufreq. +fn read_cpus(root: &Path, attr: &str) -> CpuValues { + let Ok(entries) = std::fs::read_dir(root) else { + return CpuValues::Unreadable; + }; + let mut cpus: Vec<(u32, PathBuf)> = entries + .flatten() + .filter_map(|e| { + let name = e.file_name(); + let index = name.to_str()?.strip_prefix("cpu")?.parse().ok()?; + let path = e.path().join("cpufreq").join(attr); + path.exists().then_some((index, path)) + }) + .collect(); + cpus.sort_by_key(|(index, _)| *index); + let mut values: Vec = Vec::new(); + for (_, path) in &cpus { + let Some(value) = read_sysfs(path) else { + return CpuValues::Unreadable; + }; + if !values.contains(&value) { + values.push(value); + } + } + match values.len() { + 0 => CpuValues::Unreadable, + 1 => CpuValues::Uniform(values.remove(0)), + _ => CpuValues::Mixed(values.join(", ")), + } } fn read_sysfs(path: &PathBuf) -> Option { @@ -411,6 +493,74 @@ mod tests { ); } + /// A sysfs CPU tree in a private folder: `cpus` as (directory, value of + /// `scaling_governor`, `None` for a CPU without cpufreq). + fn cpu_tree(tag: &str, cpus: &[(&str, Option<&str>)]) -> PathBuf { + let root = std::env::temp_dir().join(format!( + "bigame_knob_{tag}_{}_{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(root.join("cpufreq")).unwrap(); + std::fs::create_dir_all(root.join("cpuidle")).unwrap(); + for (cpu, value) in cpus { + let dir = root.join(cpu); + std::fs::create_dir_all(&dir).unwrap(); + if let Some(v) = value { + std::fs::create_dir_all(dir.join("cpufreq")).unwrap(); + std::fs::write(dir.join("cpufreq/scaling_governor"), format!("{v}\n")).unwrap(); + } + } + root + } + + #[test] + fn a_per_cpu_knob_is_read_on_every_cpu() { + let root = cpu_tree( + "uniform", + &[ + ("cpu0", Some("powersave")), + ("cpu1", Some("powersave")), + ("cpu10", Some("powersave")), + // Offline: no cpufreq, as the daemon skips it too. + ("cpu2", None), + ], + ); + assert_eq!( + read_cpus(&root, "scaling_governor"), + CpuValues::Uniform("powersave".into()) + ); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn a_write_that_failed_on_some_cpus_is_not_a_baseline() { + // cpu0 refused the write, the others took it: cpu0 alone reads as + // restored while the rest stay at performance. + let root = cpu_tree( + "mixed", + &[ + ("cpu0", Some("powersave")), + ("cpu1", Some("performance")), + ("cpu2", Some("performance")), + ], + ); + let values = read_cpus(&root, "scaling_governor"); + assert_eq!(values, CpuValues::Mixed("powersave, performance".into())); + // Never captured, and never "already correct": restore writes it. + assert_eq!(values.uniform(), None); + assert_eq!( + read_cpus(&root, "energy_performance_preference"), + CpuValues::Unreadable + ); + assert_eq!( + read_cpus(&root.join("absent"), "scaling_governor"), + CpuValues::Unreadable + ); + let _ = std::fs::remove_dir_all(&root); + } + #[test] fn titles_are_translatable_and_carry_the_card() { let gpu = Knob::GpuDpmLevel { diff --git a/bigame-engine/bigame-core/src/capabilities.rs b/bigame-engine/bigame-core/src/capabilities.rs index af82b88..7dea180 100644 --- a/bigame-engine/bigame-core/src/capabilities.rs +++ b/bigame-engine/bigame-core/src/capabilities.rs @@ -149,12 +149,13 @@ impl GamescopeCaps { let token = token.trim_end_matches(','); let Some(name) = token.strip_prefix("--") else { // Short options: `-F` in " -F, --filter". - if let Some(short) = token.strip_prefix('-') { - if short.len() == 1 && short.chars().all(char::is_alphabetic) { - let s = short.to_owned(); - if !flags.contains(&s) { - flags.push(s); - } + if let Some(short) = token.strip_prefix('-') + && short.len() == 1 + && short.chars().all(char::is_alphabetic) + { + let s = short.to_owned(); + if !flags.contains(&s) { + flags.push(s); } } continue; @@ -352,10 +353,10 @@ pub fn gamescope_cached() -> Option { let mut cache = CACHE .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - if let Some((k, caps)) = cache.as_ref() { - if *k == key { - return caps.clone(); - } + if let Some((k, caps)) = cache.as_ref() + && *k == key + { + return caps.clone(); } let caps = detect_gamescope(); *cache = Some((key, caps.clone())); diff --git a/bigame-engine/bigame-core/src/config.rs b/bigame-engine/bigame-core/src/config.rs index 87a3354..5404387 100644 --- a/bigame-engine/bigame-core/src/config.rs +++ b/bigame-engine/bigame-core/src/config.rs @@ -73,7 +73,7 @@ impl Default for FalcondConfig { /// Read falcond config from disk. /// /// # Errors -/// Returns error if file is unreadable or contains invalid TOML. +/// Returns an error if the file cannot be read. pub fn read() -> Result { read_from(Path::new(CONFIG_PATH)) } @@ -83,15 +83,15 @@ pub fn read() -> Result { /// Supports both `otter_conf` (bare identifiers) and TOML (quoted strings) formats. /// /// # Errors -/// Returns error if file is unreadable or unparseable. +/// Returns an error if the file cannot be read; a file in neither format +/// reads as falcond's defaults, as falcond itself treats it. pub fn read_from(path: &Path) -> Result { let content = std::fs::read_to_string(path) .with_context(|| format!("read falcond config: {}", path.display()))?; - // Try TOML first (backwards compat with old configs) + // A file in quoted TOML still reads; otherwise it is falcond's own format. if let Ok(cfg) = toml::from_str::(&content) { return Ok(cfg); } - // Fall back to otter_conf key=value parsing (bare identifiers) Ok(parse_otter_conf(&content)) } @@ -108,7 +108,6 @@ fn parse_otter_conf(content: &str) -> FalcondConfig { }; let key = key.trim(); let val = val.trim(); - // Strip optional quotes (handle both `"none"` and `none`) let val = val.trim_matches('"'); match key { "enable_performance_mode" => cfg.enable_performance_mode = val == "true", @@ -118,7 +117,6 @@ fn parse_otter_conf(content: &str) -> FalcondConfig { "profile_mode" => cfg.profile_mode = val.to_string(), "poll_interval_ms" => cfg.poll_interval_ms = val.parse().unwrap_or(9000), "system_processes" => { - // Parse ["str1", "str2"] array syntax let inner = val.trim_start_matches('[').trim_end_matches(']'); if !inner.is_empty() { cfg.system_processes = inner @@ -128,7 +126,7 @@ fn parse_otter_conf(content: &str) -> FalcondConfig { .collect(); } } - _ => {} // Ignore unknown fields + _ => {} } } cfg diff --git a/bigame-engine/bigame-core/src/dbus.rs b/bigame-engine/bigame-core/src/dbus.rs index 7f360b6..1046737 100644 --- a/bigame-engine/bigame-core/src/dbus.rs +++ b/bigame-engine/bigame-core/src/dbus.rs @@ -1,7 +1,5 @@ //! D-Bus integration with `PowerProfiles` and falcond status daemons. -#![allow(clippy::missing_errors_doc)] - use std::sync::OnceLock; /// Cached system bus connection (for `PowerProfiles`). diff --git a/bigame-engine/bigame-core/src/dbus_client.rs b/bigame-engine/bigame-core/src/dbus_client.rs index fa38dc3..854dd49 100644 --- a/bigame-engine/bigame-core/src/dbus_client.rs +++ b/bigame-engine/bigame-core/src/dbus_client.rs @@ -13,6 +13,11 @@ use zbus::proxy; /// zbus would otherwise derive them from the Rust function names, which makes /// the wire contract hostage to a refactor — and does not always produce the /// obvious spelling (`set_vcache_mode` derives to `SetVcacheMode`). +/// +/// Every privileged call carries `ALLOW_INTERACTIVE_AUTHORIZATION` +/// (`allow_interactive_auth`): the user is at the application and can answer +/// a password prompt, and the helper lets Polkit show one only for a call +/// that says so. #[proxy( interface = "com.biglinux.BiGameMode", default_service = "com.biglinux.BiGameMode", @@ -23,41 +28,41 @@ pub trait BiGameDaemon { /// /// `name` must be a bare profile name; the helper rejects anything /// containing a path separator or `..`. - #[zbus(name = "SaveProfile")] + #[zbus(name = "SaveProfile", allow_interactive_auth)] async fn save_profile(&self, name: &str, payload: &str) -> zbus::Result<()>; /// Delete a per-game profile by bare name. - #[zbus(name = "DeleteProfile")] + #[zbus(name = "DeleteProfile", allow_interactive_auth)] async fn delete_profile(&self, name: &str) -> zbus::Result<()>; /// Replace falcond's global configuration and ask it to reload. - #[zbus(name = "ApplyFalcondConfig")] + #[zbus(name = "ApplyFalcondConfig", allow_interactive_auth)] async fn apply_falcond_config(&self, config_payload: &str) -> zbus::Result<()>; /// Set the AMD 3D V-Cache mode. - #[zbus(name = "SetVCacheMode")] + #[zbus(name = "SetVCacheMode", allow_interactive_auth)] async fn set_vcache_mode(&self, mode: &str) -> zbus::Result<()>; /// Set the CPU frequency governor on every online CPU. - #[zbus(name = "SetCpuGovernor")] + #[zbus(name = "SetCpuGovernor", allow_interactive_auth)] async fn set_cpu_governor(&self, governor: &str) -> zbus::Result<()>; /// Set the Energy Performance Preference on every online CPU. - #[zbus(name = "SetCpuEpp")] + #[zbus(name = "SetCpuEpp", allow_interactive_auth)] async fn set_cpu_epp(&self, epp: &str) -> zbus::Result<()>; /// Set `power_dpm_force_performance_level` for one DRM card. - #[zbus(name = "SetGpuDpmLevel")] + #[zbus(name = "SetGpuDpmLevel", allow_interactive_auth)] async fn set_gpu_dpm_level(&self, card: &str, level: &str) -> zbus::Result<()>; /// Turn the game performance backend (falcond) on or off, persistently. /// Returns systemd's active state for the unit afterwards. - #[zbus(name = "SetGameBackend")] + #[zbus(name = "SetGameBackend", allow_interactive_auth)] async fn set_game_backend(&self, enabled: bool) -> zbus::Result; /// Return falcond to its state before Big Game Mode first changed it. /// Returns whether there was anything to hand back. - #[zbus(name = "ReleaseGameBackend")] + #[zbus(name = "ReleaseGameBackend", allow_interactive_auth)] async fn release_game_backend(&self) -> zbus::Result; /// Liveness probe. diff --git a/bigame-engine/bigame-core/src/diagnostics.rs b/bigame-engine/bigame-core/src/diagnostics.rs index bace9c6..2d45a87 100644 --- a/bigame-engine/bigame-core/src/diagnostics.rs +++ b/bigame-engine/bigame-core/src/diagnostics.rs @@ -30,10 +30,11 @@ use crate::hardware::{Chassis, Hardware, PowerSource, Session}; #[must_use] pub fn redact_paths(text: &str) -> String { let mut out = text.to_owned(); - if let Ok(home) = std::env::var("HOME") { - if !home.is_empty() && home != "/" { - out = out.replace(&home, "~"); - } + if let Ok(home) = std::env::var("HOME") + && !home.is_empty() + && home != "/" + { + out = out.replace(&home, "~"); } if let Ok(user) = std::env::var("USER") { // A very short username would match far too much unrelated text. @@ -173,10 +174,10 @@ fn section_bigame(out: &mut String) { let _ = writeln!(out, "── Big Game Mode ──"); let reader = crate::systemd::Reader::shared(); let unit = reader.and_then(|r| r.unit_state(crate::turbo::BACKEND_UNIT)); - // As Home reads it (turbo::state): falcond's unit when it is installed, - // Booster's journal without it. + // As Home reads it (turbo::state): falcond's unit when it governs Turbo, + // Booster's journal without it or with a build that crashed here. let turbo = match &unit { - Some(u) if u.is_installed() => on_off(u.is_active()), + Some(u) if crate::turbo::backend_governs(u) => on_off(u.is_active()), Some(_) => on_off(crate::booster::BoosterEngine::is_active()), None => "unknown (systemd did not answer)", }; @@ -946,10 +947,10 @@ mod tests { if let Ok(home) = std::env::var("HOME") { assert!(!text.contains(&home), "home directory leaked"); } - if let Ok(user) = std::env::var("USER") { - if user.len() >= 3 { - assert!(!text.contains(&user), "username leaked"); - } + if let Ok(user) = std::env::var("USER") + && user.len() >= 3 + { + assert!(!text.contains(&user), "username leaked"); } // The hostname is never collected, so it must not appear either. if let Ok(host) = std::fs::read_to_string("/etc/hostname") { diff --git a/bigame-engine/bigame-core/src/fg.rs b/bigame-engine/bigame-core/src/fg.rs index 0c82f03..348414d 100644 --- a/bigame-engine/bigame-core/src/fg.rs +++ b/bigame-engine/bigame-core/src/fg.rs @@ -261,7 +261,11 @@ fn write_table(path: &Path, table: &Table) -> Result<()> { /// lsfg-vk's file in `format`, with anything in another layout converted. fn read_config_as(format: Format) -> Result { - let mut t = read_table(&config_path())?; + read_config_at(&config_path(), format) +} + +fn read_config_at(path: &Path, format: Format) -> Result
{ + let mut t = read_table(path)?; migrate(format, &mut t); Ok(t) } @@ -271,7 +275,11 @@ fn read_config() -> Result
{ } fn write_config(format: Format, t: &Table) -> Result<()> { - write_table(&config_path(), &finished(format, t.clone())) + write_config_at(&config_path(), format, t) +} + +fn write_config_at(path: &Path, format: Format, t: &Table) -> Result<()> { + write_table(path, &finished(format, t.clone())) } /// `t` as the file lsfg-vk reads: its version, and for 2.x the `[global]` @@ -420,10 +428,11 @@ fn migrate_to_v2(t: &mut Table) { .get("multiplier") .and_then(Value::as_integer) .is_some_and(|m| m < 2); - if p.contains_key("pacing") && p.contains_key("present_mode") { - if let Some(f) = p.get("flow_scale").and_then(Value::as_float) { - p.insert("flow_scale".into(), Value::Float(round_flow(f))); - } + if p.contains_key("pacing") + && p.contains_key("present_mode") + && let Some(f) = p.get("flow_scale").and_then(Value::as_float) + { + p.insert("flow_scale".into(), Value::Float(round_flow(f))); } p.retain(|k, _| V2_PROFILE.contains(&k)); !off @@ -514,7 +523,11 @@ struct State { } fn read_state() -> State { - let t = read_table(&state_path()).unwrap_or_default(); + read_state_at(&state_path()) +} + +fn read_state_at(path: &Path) -> State { + let t = read_table(path).unwrap_or_default(); State { managed: t .get("managed") @@ -535,13 +548,17 @@ fn read_state() -> State { } fn write_state(s: &State) -> Result<()> { + write_state_at(&state_path(), s) +} + +fn write_state_at(path: &Path, s: &State) -> Result<()> { let mut t = Table::new(); t.insert( "managed".into(), Value::Array(s.managed.iter().map(|m| Value::String(m.clone())).collect()), ); t.insert("paused".into(), Value::Array(s.paused.clone())); - write_table(&state_path(), &t) + write_table(path, &t) } fn paused_exe(g: &Value) -> Option<&str> { @@ -750,10 +767,10 @@ pub fn write_profile( (25..=100).contains(&flow_scale_pct), UserError::plain(N_("flow_scale_pct must be 25–100")) ); - let mut t = read_config_as(format)?; - upsert( + put_in_force( format, - &mut t, + &config_path(), + &state_path(), name, Values { multiplier: multiplier.clamp(2, 20), @@ -762,14 +779,30 @@ pub fn write_profile( hdr, present: present_mode, }, - )?; - write_config(format, &t)?; - let mut s = read_state(); + ) +} + +/// Write `name`'s entry into lsfg-vk's file at `config`, managed by +/// Big Game Mode in the state at `state`. The state is written first: an +/// entry in lsfg-vk's file that the state does not list is one the general +/// switch would never set aside, while a name listed without an entry is +/// passed over. +fn put_in_force( + format: Format, + config: &Path, + state: &Path, + name: &str, + values: Values, +) -> Result<()> { + let mut t = read_config_at(config, format)?; + upsert(format, &mut t, name, values)?; + let mut s = read_state_at(state); if !s.managed.iter().any(|m| m == name) { s.managed.push(name.to_owned()); } s.paused.retain(|g| paused_exe(g) != Some(name)); - write_state(&s) + write_state_at(state, &s)?; + write_config_at(config, format, &t) } /// Write `name`'s entry where it belongs: in lsfg-vk's file when the general @@ -1194,9 +1227,14 @@ pub fn is_active_for_game(name: &str) -> bool { /// # Errors /// Returns error if the files cannot be read or written. pub fn disable_all_profiles() -> Result { - let format = read_format(); - let mut t = read_config_as(format)?; - let mut s = read_state(); + disable_all_at(read_format(), &config_path(), &state_path()) +} + +/// [`disable_all_profiles`] on lsfg-vk's file at `config` and the state at +/// `state`. +fn disable_all_at(format: Format, config: &Path, state: &Path) -> Result { + let mut t = read_config_at(config, format)?; + let mut s = read_state_at(state); let mut changed = false; for name in s.managed.clone() { if let Some(g) = take(format, &mut t, &name) { @@ -1205,13 +1243,20 @@ pub fn disable_all_profiles() -> Result { Format::V1 => g, Format::V2 => entry_table(Format::V1, &name, values_of(Format::V2, &g)), }; + // Set aside by an earlier try whose write of lsfg-vk's file + // failed: replaced, not kept twice. + s.paused.retain(|p| paused_exe(p) != Some(name.as_str())); s.paused.push(Value::Table(kept)); changed = true; } } if changed { - write_config(format, &t)?; - write_state(&s)?; + // The entries are kept before they leave lsfg-vk's file: written the + // other way round, a failed state write lost them. Left in the file + // after a failed write, they are still in force, and putting them + // back passes over an entry that is already there. + write_state_at(state, &s)?; + write_config_at(config, format, &t)?; } Ok(changed) } @@ -1607,4 +1652,84 @@ multiplier = 4 } std::fs::remove_dir_all(&dir).unwrap(); } + + /// A private folder per test, with the paths of lsfg-vk's file and of + /// Big Game Mode's state in folders of their own. + fn fg_files(tag: &str) -> (PathBuf, PathBuf, PathBuf) { + let dir = std::env::temp_dir().join(format!( + "bgm-fg-{tag}-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("lsfg-vk")).unwrap(); + std::fs::create_dir_all(dir.join("bigame-mode")).unwrap(); + let config = dir.join("lsfg-vk").join("conf.toml"); + let state = dir.join("bigame-mode").join("lsfg-vk.toml"); + (dir, config, state) + } + + /// `path`'s folder read-only (`true`) or writable again. + fn read_only(path: &Path, on: bool) { + use std::os::unix::fs::PermissionsExt as _; + let mode = if on { 0o555 } else { 0o755 }; + std::fs::set_permissions( + path.parent().unwrap(), + std::fs::Permissions::from_mode(mode), + ) + .unwrap(); + } + + fn in_file(config: &Path, exe: &str) -> bool { + find( + Format::V1, + &read_config_at(config, Format::V1).unwrap(), + exe, + ) + .is_some() + } + + #[test] + fn an_entry_is_listed_before_it_is_in_force() { + let (dir, config, state) = fg_files("in-force"); + put_in_force(Format::V1, &config, &state, "SOTTR.exe", values(2, 70, 0)).unwrap(); + assert!(in_file(&config, "SOTTR.exe")); + assert_eq!(read_state_at(&state).managed, ["SOTTR.exe"]); + // The state cannot be written: lsfg-vk's file does not get an entry + // the general switch would never set aside. + read_only(&state, true); + let failed = put_in_force(Format::V1, &config, &state, "Hades.exe", values(2, 70, 0)); + read_only(&state, false); + assert!(failed.is_err()); + assert!(!in_file(&config, "Hades.exe")); + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn entries_are_kept_before_they_leave_lsfg_vks_file() { + let (dir, config, state) = fg_files("set-aside"); + put_in_force(Format::V1, &config, &state, "SOTTR.exe", values(2, 70, 0)).unwrap(); + // The state cannot be written: the entry stays in force, not lost. + read_only(&state, true); + let failed = disable_all_at(Format::V1, &config, &state); + read_only(&state, false); + assert!(failed.is_err()); + assert!(in_file(&config, "SOTTR.exe")); + assert!(read_state_at(&state).paused.is_empty()); + // lsfg-vk's file cannot be written: the entry is kept in the state + // and still in force there. + read_only(&config, true); + let failed = disable_all_at(Format::V1, &config, &state); + read_only(&config, false); + assert!(failed.is_err()); + assert!(in_file(&config, "SOTTR.exe")); + assert_eq!(read_state_at(&state).paused.len(), 1); + // Tried again: set aside once, not twice, and out of lsfg-vk's file. + assert!(disable_all_at(Format::V1, &config, &state).unwrap()); + assert!(!in_file(&config, "SOTTR.exe")); + let s = read_state_at(&state); + assert_eq!(s.paused.len(), 1); + assert_eq!(paused_exe(&s.paused[0]), Some("SOTTR.exe")); + std::fs::remove_dir_all(&dir).unwrap(); + } } diff --git a/bigame-engine/bigame-core/src/game_settings.rs b/bigame-engine/bigame-core/src/game_settings.rs index 69730c6..7cd6d6e 100644 --- a/bigame-engine/bigame-core/src/game_settings.rs +++ b/bigame-engine/bigame-core/src/game_settings.rs @@ -35,9 +35,11 @@ pub struct GameSettings { /// upscaler (`crate::steam_gamescope::set_wine_fsr_off`). #[serde(skip_serializing_if = "std::ops::Not::not")] pub steam_wine_fsr_off: bool, - /// The variables Big Game Mode put in front of the game's Steam launch - /// options for its own launch settings, so it can replace or remove - /// exactly those (`crate::steam_gamescope`). + /// The variables last asked for in front of the game's Steam launch + /// options for its own launch settings (`crate::steam_gamescope`). What + /// really went in, account by account, is recorded in the state + /// directory's `steam/env-added.toml`; this field stands for every + /// account only for a game set up before that record existed. #[serde(skip_serializing_if = "Option::is_none")] pub steam_env: Option, /// The game's own launch settings (Gamescope, Wine FSR, vkBasalt), over diff --git a/bigame-engine/bigame-core/src/games.rs b/bigame-engine/bigame-core/src/games.rs index 716f9c0..5a3a432 100644 --- a/bigame-engine/bigame-core/src/games.rs +++ b/bigame-engine/bigame-core/src/games.rs @@ -223,14 +223,33 @@ fn canonical(path: &Path) -> PathBuf { /// the same disk the game is loading from. #[must_use] pub fn detect_all() -> Vec { - static CACHE: std::sync::Mutex> = std::sync::Mutex::new(None); let sources = Sources::of_home(&crate::paths::home_dir()); - let mut cache = CACHE + let mut cache = LIBRARY .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); cached_scan(&mut cache, &sources, std::time::Instant::now()).0 } +/// [`detect_all`] for a caller that looks every minute and only wants to +/// hear of a change: the library and the number of its scan, when that is +/// not `known`. +/// +/// Scanned again only when a watched file changed, never for age alone (as +/// [`detect_all`] also does): what the files do not record is not worth a walk +/// of every install folder every few minutes, all day, to a caller such as +/// the running-game watch. Another caller's scan is a change here too. +#[must_use] +pub fn detect_all_changed(known: Option) -> Option<(u64, Vec)> { + let sources = Sources::of_home(&crate::paths::home_dir()); + let mut cache = LIBRARY + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + changed_scan(&mut cache, &sources, std::time::Instant::now(), known) +} + +/// The library as last scanned, for every caller. +static LIBRARY: std::sync::Mutex> = std::sync::Mutex::new(None); + /// Every game in `sources`, folded and sorted. fn detect_in(sources: &Sources) -> Vec { let mut games = steam_games_in(&sources.steam_roots); @@ -372,6 +391,8 @@ const LIBRARY_MAX_AGE: std::time::Duration = std::time::Duration::from_secs(300) /// A scan and what it was read from. struct Cached { + /// Counts the scans, so a caller can tell a new one from the one it has. + scan: u64, sources: Sources, /// The watched files' modification times, taken before the scan. sources_stamp: Vec>, @@ -402,27 +423,55 @@ fn cached_scan( sources: &Sources, now: std::time::Instant, ) -> (Vec, bool) { + let scanned = refresh(cache, sources, now, true); + let games = cache.as_ref().map(|c| c.games.clone()).unwrap_or_default(); + (games, scanned) +} + +/// [`detect_all_changed`] over `cache`. +fn changed_scan( + cache: &mut Option, + sources: &Sources, + now: std::time::Instant, + known: Option, +) -> Option<(u64, Vec)> { + refresh(cache, sources, now, false); + cache + .as_ref() + .filter(|c| Some(c.scan) != known) + .map(|c| (c.scan, c.games.clone())) +} + +/// Scan `sources` again into `cache` unless the scan there is current: of +/// the same sources, none of whose watched files changed since, and — when +/// `expire` — younger than [`LIBRARY_MAX_AGE`]. `true` when it scanned. +fn refresh( + cache: &mut Option, + sources: &Sources, + now: std::time::Instant, + expire: bool, +) -> bool { let sources_stamp: Vec<_> = sources.watched().iter().map(|p| stamp(p)).collect(); - if let Some(c) = cache.as_ref() { - if c.sources == *sources - && now.saturating_duration_since(c.at) < LIBRARY_MAX_AGE - && c.sources_stamp == sources_stamp - && c.games_stamp == games_stamp(&c.games) - { - return (c.games.clone(), false); - } + if let Some(c) = cache.as_ref() + && c.sources == *sources + && (!expire || now.saturating_duration_since(c.at) < LIBRARY_MAX_AGE) + && c.sources_stamp == sources_stamp + && c.games_stamp == games_stamp(&c.games) + { + return false; } // Stamped before reading: a change during the scan shows on the next // call as a difference, never as a stale result kept for good. let games = detect_in(sources); *cache = Some(Cached { + scan: cache.as_ref().map_or(0, |c| c.scan.wrapping_add(1)), sources: sources.clone(), sources_stamp, games_stamp: games_stamp(&games), at: now, - games: games.clone(), + games, }); - (games, true) + true } /// Keep the first of every game, in the order given. @@ -502,12 +551,16 @@ pub fn is_generic_program(name: &str) -> bool { GENERIC_PROGRAMS.contains(&lower.as_str()) || lower.starts_with("google-chrome") || lower.starts_with("electron") - // python3, python3.13, perl5.40: versioned interpreters. - || ["python", "pypy", "ruby", "perl", "lua", "luajit"].iter().any(|p| { - lower - .strip_prefix(p) - .is_some_and(|v| v.chars().all(|c| c.is_ascii_digit() || c == '.')) - }) + || ["python", "pypy", "ruby", "perl", "lua", "luajit"] + .iter() + .any(|p| is_versioned(&lower, p)) +} + +/// Whether lowercase `name` is `program` by a versioned name: python3, +/// python3.13, perl5.40. +pub(crate) fn is_versioned(name: &str, program: &str) -> bool { + name.strip_prefix(program) + .is_some_and(|v| v.chars().all(|c| c.is_ascii_digit() || c == '.')) } /// Main categories an entry cannot carry and be a game, whatever else it @@ -529,9 +582,9 @@ pub struct MenuGame { pub name: String, /// The process name falcond will see (the basename of the program). /// - /// For a Flatpak this is the application's `command`, which - /// [`menu_game`] can only take from an explicit `--command=`; otherwise - /// it is empty until [`menu_games_in`] reads the application's metadata. + /// For a Flatpak this is the application's `command`, which the entry's + /// `Exec` names only in an explicit `--command=`; otherwise it is empty + /// until [`menu_games_in`] reads the application's metadata. pub program: String, /// `Exec` as an argument vector, field codes (`%U`, `%f`, …) removed. pub argv: Vec, @@ -578,13 +631,8 @@ impl From for DetectedGame { /// a launcher (`steam steam://rungameid/…`) are not games here: their process /// is the launcher, and Steam's are found by their tree. A `flatpak run` /// entry is a game whose process is the Flatpak's own command; whether the -/// Flatpak is installed is for [`menu_games_in`] to check. -#[must_use] -pub fn menu_game(content: &str) -> Option { - menu_game_at(content, None) -} - -/// [`menu_game`] for the entry at `location`, which `%k` stands for. +/// Flatpak is installed is for [`menu_games_in`] to check. `location` is +/// where the entry is, which `%k` stands for. fn menu_game_at(content: &str, location: Option<&Path>) -> Option { let mut in_entry = false; let (mut exec, mut name, mut categories, mut icon) = (None, None, None, None); @@ -947,15 +995,6 @@ fn flatpak_command(app_id: &str, installations: &[PathBuf]) -> Option { }) } -/// Every game in the application menu: `XDG_DATA_HOME` and each of -/// `XDG_DATA_DIRS`, the first entry of a name winning, as the menu does; -/// each checked to exist (see [`menu_games_in`]). -#[must_use] -pub fn menu_games() -> Vec { - let sources = Sources::of_home(&crate::paths::home_dir()); - menu_games_in(&sources.applications, &sources.path_dirs, &sources.flatpak) -} - /// The games among the `.desktop` entries of `applications`, keeping only /// those whose program is really there: an executable file, found in /// `path_dirs` when the entry names it without a path, or a Flatpak present @@ -1385,22 +1424,14 @@ pub fn acf_value(content: &str, key: &str) -> Option { vdf::first_value(content, key).filter(|v| !v.is_empty()) } -/// The installed game a running process belongs to, from any launcher: the -/// game whose install folder holds the process, else the one game that -/// lists its executable. What names it and shows its cover when the process -/// alone says neither. -#[must_use] -pub fn installed_game_for_process( - process_name: &str, - install_path: Option<&Path>, -) -> Option { - game_among(&detect_all(), process_name, install_path).cloned() -} - -/// [`installed_game_for_process`] for a running game, also by where its -/// executable is: a Wine game's `Z:\…` or `C:\…` path is mapped to the -/// file in its prefix, so two games that both run as `Game.exe` (RPG Maker) -/// are told apart by folder rather than guessed by name. +/// The installed game a running game belongs to, from any launcher: what +/// names it and shows its cover when the process alone says neither. +/// +/// First by where its executable is: a Wine game's `Z:\…` or `C:\…` path is +/// mapped to the file in its prefix, so two games that both run as +/// `Game.exe` (RPG Maker) are told apart by folder rather than guessed by +/// name. Then the game installed where the running game's launcher says it +/// is, else the one game that lists its executable. #[must_use] pub fn installed_game_for_running(game: &crate::running::GameIdentity) -> Option { let games = detect_all(); @@ -1522,10 +1553,10 @@ fn find_file_named(dir: &Path, filename: &str, depth: u32) -> Option { } for entry in std::fs::read_dir(dir).ok()?.flatten() { let path = entry.path(); - if path.is_dir() { - if let Some(found) = find_file_named(&path, filename, depth - 1) { - return Some(found); - } + if path.is_dir() + && let Some(found) = find_file_named(&path, filename, depth - 1) + { + return Some(found); } } None @@ -1956,10 +1987,9 @@ fn executables_for_file(launch_file: &Path) -> Vec { if let Some(name) = launch_file .file_name() .map(|n| n.to_string_lossy().into_owned()) + && !names.contains(&name) { - if !names.contains(&name) { - names.push(name); - } + names.push(name); } names } @@ -2558,10 +2588,10 @@ pub fn heroic_games(configs: &[PathBuf]) -> Vec { } } for mut entry in entries { - if entry.art.is_empty() { - if let Some(found) = entry.app_name.as_ref().and_then(|a| art.get(a)) { - entry.art.clone_from(found); - } + if entry.art.is_empty() + && let Some(found) = entry.app_name.as_ref().and_then(|a| art.get(a)) + { + entry.art.clone_from(found); } let Some(game) = heroic_game(base, entry) else { continue; @@ -2815,6 +2845,11 @@ mod tests { use std::fs; use std::os::unix::fs::PermissionsExt; + /// [`menu_game_at`] for an entry read from nowhere in particular. + fn menu_game(content: &str) -> Option { + menu_game_at(content, None) + } + const STK_DESKTOP: &str = "[Desktop Entry]\nName=SuperTuxKart\nName[pt_BR]=SuperTuxKart\nExec=supertuxkart\nIcon=supertuxkart\nType=Application\nCategories=Game;ArcadeGame;\nActions=SoftwareRender;\n\n[Desktop Action SoftwareRender]\nName=Software Render\nExec=SoftwareRender supertuxkart\n"; fn tempdir(name: &str) -> PathBuf { @@ -4256,6 +4291,44 @@ mod tests { let _ = fs::remove_dir_all(&home); } + #[test] + fn a_caller_told_of_changes_hears_only_of_a_new_scan() { + let home = tempdir("changed"); + let steamapps = home.join(".local/share/Steam/steamapps"); + write( + &steamapps.join("appmanifest_1.acf"), + manifest("1", "One", "One", "4").as_bytes(), + ); + write(&steamapps.join("common/One/One.exe"), &vec![0; BIG]); + let sources = Sources { + applications: Vec::new(), + path_dirs: Vec::new(), + flatpak: Vec::new(), + ..Sources::of_home(&home) + }; + let mut cache = None; + let now = std::time::Instant::now(); + let (scan, games) = changed_scan(&mut cache, &sources, now, None).unwrap(); + assert_eq!(games.len(), 1); + // However old, an unchanged library is neither scanned nor news. + let later = now + LIBRARY_MAX_AGE * 3; + assert_eq!(changed_scan(&mut cache, &sources, later, Some(scan)), None); + assert_eq!(cache.as_ref().map(|c| c.at), Some(now)); + // A scan another caller made for age is news. + assert!(cached_scan(&mut cache, &sources, later).1); + let (newer, _) = changed_scan(&mut cache, &sources, later, Some(scan)).unwrap(); + assert_ne!(newer, scan); + // So is a change in what it is read from. + write( + &steamapps.join("appmanifest_2.acf"), + manifest("2", "Two", "Two", "4").as_bytes(), + ); + write(&steamapps.join("common/Two/Two.exe"), &vec![0; BIG]); + let (_, games) = changed_scan(&mut cache, &sources, later, Some(newer)).unwrap(); + assert_eq!(games.len(), 2); + let _ = fs::remove_dir_all(&home); + } + #[test] fn another_homes_launchers_are_found_at_their_default_places() { let home = tempdir("xdg"); diff --git a/bigame-engine/bigame-core/src/gamescope.rs b/bigame-engine/bigame-core/src/gamescope.rs index 1a31ce3..a2799a7 100644 --- a/bigame-engine/bigame-core/src/gamescope.rs +++ b/bigame-engine/bigame-core/src/gamescope.rs @@ -352,10 +352,11 @@ impl Config { } } - if let FrameLimit::NestedRefresh(hz) = self.frame_limit { - if hz > 0 && want("r", N_("frame rate not limited"), &mut unsupported) { - args.extend(["-r".into(), hz.to_string()]); - } + if let FrameLimit::NestedRefresh(hz) = self.frame_limit + && hz > 0 + && want("r", N_("frame rate not limited"), &mut unsupported) + { + args.extend(["-r".into(), hz.to_string()]); } if self.mangoapp && want("mangoapp", N_("overlay not shown"), &mut unsupported) { diff --git a/bigame-engine/bigame-core/src/graphics/backend.rs b/bigame-engine/bigame-core/src/graphics/backend.rs index c449321..ad3ecaa 100644 --- a/bigame-engine/bigame-core/src/graphics/backend.rs +++ b/bigame-engine/bigame-core/src/graphics/backend.rs @@ -260,16 +260,16 @@ pub fn check(backend: Backend, r: &Report) -> Availability { detail: Text::plain(N_("a 64-bit game; this one is 32-bit")), }); } - if let Some(api) = r.api.api { - if !c.apis.contains(&api) { - missing.push(Missing { - what: N_("Graphics API"), - detail: Text::with( - N_("needs %s; this game renders with %s"), - [apis(c.apis), api_name(api).to_owned()], - ), - }); - } + if let Some(api) = r.api.api + && !c.apis.contains(&api) + { + missing.push(Missing { + what: N_("Graphics API"), + detail: Text::with( + N_("needs %s; this game renders with %s"), + [apis(c.apis), api_name(api).to_owned()], + ), + }); } if c.needs_ffx_api && r.native.ffx_api.is_none() { missing.push(Missing { diff --git a/bigame-engine/bigame-core/src/graphics/fsr4_upgrade.rs b/bigame-engine/bigame-core/src/graphics/fsr4_upgrade.rs index cf63d76..d428880 100644 --- a/bigame-engine/bigame-core/src/graphics/fsr4_upgrade.rs +++ b/bigame-engine/bigame-core/src/graphics/fsr4_upgrade.rs @@ -38,7 +38,9 @@ pub const GE_VARIABLE: &str = "PROTON_FSR4_UPGRADE=1"; /// game, so that only that is taken out again. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Added { - /// The account (its `userdata` id). + /// The account: its `localconfig.vdf`. A record from before 2.3.2 holds + /// its `userdata` id, which stands for that id's account in every Steam + /// install (the native and the Flatpak each have their own). pub account: String, /// `%command%` went in with the variable: the options had none. #[serde(default, skip_serializing_if = "std::ops::Not::not")] @@ -163,6 +165,14 @@ pub fn apply(process: &str, app_id: Option<&str>, on: bool) -> Result { Ok(Applied::SteamLaunchOptions(last)) } +impl Added { + /// Whether this record is of the account `id` whose configuration is + /// `config`. + fn names(&self, id: &str, config: &std::path::Path) -> bool { + self.account == id || std::path::Path::new(&self.account) == config + } +} + /// The launch option switched in every account it belongs in, as one /// change: every account's new options are worked out before any is /// written, and an account that cannot be written puts back the ones @@ -190,7 +200,7 @@ fn apply_to_accounts( (false, None) => accounts.iter().collect(), (false, Some(list)) => accounts .iter() - .filter(|(id, _)| list.iter().any(|a| a.account == *id)) + .filter(|(id, config)| list.iter().any(|a| a.names(id, config))) .collect(), }; // Every account's new options first. @@ -203,7 +213,7 @@ fn apply_to_accounts( match with_upgrade(¤t) { Some((options, command)) => { now_added.push(Added { - account: id.clone(), + account: config.to_string_lossy().into_owned(), command, }); options @@ -212,7 +222,7 @@ fn apply_to_accounts( } } else { let command = - added.is_none_or(|list| list.iter().any(|a| a.account == *id && a.command)); + added.is_none_or(|list| list.iter().any(|a| a.names(id, config) && a.command)); without_upgrade(¤t, command) }; if wanted != current { @@ -403,6 +413,51 @@ mod tests { assert_eq!(store.get("b").as_deref(), Some("FSR4_UPGRADE=1 %command%")); } + #[test] + fn one_account_in_two_steam_installs_is_recorded_by_its_file() { + // The same account id in the native Steam and in the Flatpak; the + // user typed the variable in the Flatpak's options themselves. + let store = Accounts::new(&[("/native", "-x"), ("/flatpak", "FSR4_UPGRADE=1 %command%")]); + let read = |c: &std::path::Path| store.0.borrow().get(c).cloned(); + let write = |c: &std::path::Path, v: &str| { + store.0.borrow_mut().insert(c.to_path_buf(), v.to_owned()); + Ok(()) + }; + let accounts = vec![ + ("1234".to_owned(), std::path::PathBuf::from("/native")), + ("1234".to_owned(), std::path::PathBuf::from("/flatpak")), + ]; + let (_, record) = apply_to_accounts(&accounts, true, None, &read, &write).unwrap(); + assert_eq!( + record, + Some(vec![Added { + account: "/native".into(), + command: true + }]) + ); + apply_to_accounts(&accounts, false, record.as_deref(), &read, &write).unwrap(); + assert_eq!(store.get("/native").as_deref(), Some("-x")); + assert_eq!( + store.get("/flatpak").as_deref(), + Some("FSR4_UPGRADE=1 %command%"), + "the user's own, in the other install, stays" + ); + + // A record from before names the id: both installs' accounts. + write( + std::path::Path::new("/native"), + "FSR4_UPGRADE=1 %command% -x", + ) + .unwrap(); + let old = [Added { + account: "1234".into(), + command: false, + }]; + apply_to_accounts(&accounts, false, Some(&old), &read, &write).unwrap(); + assert_eq!(store.get("/native").as_deref(), Some("%command% -x")); + assert_eq!(store.get("/flatpak").as_deref(), Some("%command%")); + } + #[test] fn an_account_that_cannot_be_written_puts_back_the_ones_before_it() { let store = Accounts::new(&[("a", "-x"), ("b", "-y")]); diff --git a/bigame-engine/bigame-core/src/graphics/manifest.rs b/bigame-engine/bigame-core/src/graphics/manifest.rs index dcf91f4..cdcd16b 100644 --- a/bigame-engine/bigame-core/src/graphics/manifest.rs +++ b/bigame-engine/bigame-core/src/graphics/manifest.rs @@ -281,7 +281,7 @@ impl Manifest { // The game's folder entry must survive a crash too. std::fs::File::open(state_dir)?.sync_all()?; } - let tmp = dir.join("manifest.json.tmp"); + let tmp = dir.join(unique_name("manifest.json.tmp-")); { let mut f = std::fs::File::create(&tmp).with_context(|| format!("create {}", tmp.display()))?; @@ -347,6 +347,17 @@ impl Manifest { } } +/// `prefix` followed by a name no other change running now uses: this +/// process's id and a count. A temporary file or folder two changes at once +/// named alike (two Big Game Mode processes, or two threads of one) would be +/// written, renamed or deleted by each under the other. +#[must_use] +pub(crate) fn unique_name(prefix: &str) -> String { + static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + format!("{prefix}{}-{n}", std::process::id()) +} + /// Whether `a` and `b` name the same folder, links resolved. A folder that /// is not there is compared as written. #[must_use] @@ -483,6 +494,36 @@ mod tests { assert!(m.managed); } + #[test] + fn saves_at_once_never_share_a_temporary_file() { + let dir = tempfile::tempdir().unwrap(); + let p = Manifest::path(dir.path(), "steam-1"); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write( + &p, + r#"{"schema":1,"game_key":"steam-1","install_root":"/g","source":{"component":"optiscaler","version":"1","url":null,"archive_sha256":null},"started_at":0,"state":"installed","entries":[]}"#, + ) + .unwrap(); + let m = Manifest::load(dir.path(), "steam-1").unwrap().unwrap(); + // One shared temporary name: one thread's rename takes the file the + // other is about to rename, which then fails. + std::thread::scope(|s| { + for _ in 0..4 { + s.spawn(|| { + for _ in 0..25 { + m.save(dir.path()).unwrap(); + } + }); + } + }); + let left: Vec<_> = std::fs::read_dir(p.parent().unwrap()) + .unwrap() + .map(|e| e.unwrap().file_name()) + .collect(); + assert_eq!(left, ["manifest.json"]); + assert_eq!(Manifest::load(dir.path(), "steam-1").unwrap(), Some(m)); + } + #[test] fn game_keys_are_stable_and_safe() { assert_eq!( diff --git a/bigame-engine/bigame-core/src/graphics/mod.rs b/bigame-engine/bigame-core/src/graphics/mod.rs index c8b63bc..e6b4dfe 100644 --- a/bigame-engine/bigame-core/src/graphics/mod.rs +++ b/bigame-engine/bigame-core/src/graphics/mod.rs @@ -608,9 +608,25 @@ pub fn analyze(target: &Target, cfg: &config::AiGraphicsConfig) -> Analysis { /// Returns an error when the game runs, nothing is installed, or the /// registry cannot be written. pub fn switch_game_setting_on_again(target: &Target) -> anyhow::Result { - ensure_closed(target)?; - let m = installed_manifest(&state_dir(), target)?; - ingame::switch_on_again(&m.settings) + let state = state_dir(); + locked(&state, target, || { + ensure_closed(target)?; + let m = installed_manifest(&state, target)?; + ingame::switch_on_again(&m.settings) + }) +} + +/// Run `change` to `target`'s files or record under the game's lock +/// ([`transaction::lock`]), so a second change to the game — a double click, +/// the page and the Profiles menu at once — waits for the first and then +/// finds what it left, instead of backing up its files as the originals. +fn locked( + state: &Path, + target: &Target, + change: impl FnOnce() -> anyhow::Result, +) -> anyhow::Result { + let _lock = transaction::lock(state, &target.key())?; + change() } fn status_of( @@ -876,9 +892,7 @@ fn apply_release( ) -> anyhow::Result { let key = target.key(); let staging = state.join(&key).join("staging"); - let next = state - .join(&key) - .join(format!("staging-{}", std::process::id())); + let next = state.join(&key).join(manifest::unique_name("staging-")); remove_dir_if_there(&next)?; let applied = optiscaler::payload(cached, o, exe_dir, &next, carry).and_then(|files| { transaction::apply( @@ -952,15 +966,17 @@ pub fn install( version: &config::VersionPolicy, ) -> anyhow::Result { let (state, cache) = (state_dir(), optiscaler::cache_dir()); - let done = install_in( - &state, - &cache, - &gamedb::GameDb::load(), - target, - plan, - version, - &[], - ); + let done = locked(&state, target, || { + install_in( + &state, + &cache, + &gamedb::GameDb::load(), + target, + plan, + version, + &[], + ) + }); tidy_cache(&state, &cache); done } @@ -1135,14 +1151,16 @@ pub fn reinstall( let (state, cache) = (state_dir(), optiscaler::cache_dir()); // Once at the end: between its restore and its install no game names // the release it is about to install again. - let done = reinstall_in( - &state, - &cache, - &gamedb::GameDb::load(), - target, - plan, - version, - ); + let done = locked(&state, target, || { + reinstall_in( + &state, + &cache, + &gamedb::GameDb::load(), + target, + plan, + version, + ) + }); tidy_cache(&state, &cache); done } @@ -1212,7 +1230,9 @@ pub fn update( to: &optiscaler::Release, ) -> anyhow::Result { let (state, cache) = (state_dir(), optiscaler::cache_dir()); - let done = update_in(&state, &cache, &gamedb::GameDb::load(), target, plan, to); + let done = locked(&state, target, || { + update_in(&state, &cache, &gamedb::GameDb::load(), target, plan, to) + }); tidy_cache(&state, &cache); done } @@ -1360,13 +1380,25 @@ pub struct Restoration { /// settings stay recorded for the next Restore. A prefix that is gone has no /// settings to put back. /// +/// Big Game Mode's `WINE_FULLSCREEN_FSR=0` in the game's Steam launch options +/// went in with `OptiScaler` (Apply), and goes with it — whichever Restore +/// runs, the page's or the Profiles menu's. With Steam open nothing is +/// written: the switch stays, harmless, until the next Restore. +/// /// # Errors /// Returns an error if the game is running, nothing is installed, the record /// is not this game's folder's, or a file cannot be restored. pub fn restore(target: &Target) -> anyhow::Result { let state = state_dir(); - let done = restore_in(&state, target); + let done = locked(&state, target, || restore_in(&state, target)); tidy_cache(&state, &optiscaler::cache_dir()); + if done.is_ok() + && crate::game_settings::load(&target.process).is_ok_and(|s| s.steam_wine_fsr_off) + && let Err(e) = crate::steam_gamescope::set_wine_fsr_off(&target.process, false) + { + tracing::warn!(target: "graphics", game = %target.process, error = %format!("{e:#}"), + "Big Game Mode's Wine FSR switch could not be taken out of the launch options"); + } done } @@ -1433,7 +1465,7 @@ fn restore_in(state: &Path, target: &Target) -> anyhow::Result { /// As [`restore`], and when the game's own settings could not be put back. pub fn remove(target: &Target) -> anyhow::Result> { let state = state_dir(); - let done = remove_in(&state, target); + let done = locked(&state, target, || remove_in(&state, target)); tidy_cache(&state, &optiscaler::cache_dir()); done } @@ -1459,12 +1491,15 @@ fn remove_in(state: &Path, target: &Target) -> anyhow::Result anyhow::Result> { - repair_in( - &state_dir(), - &optiscaler::cache_dir(), - &gamedb::GameDb::load(), - target, - ) + let state = state_dir(); + locked(&state, target, || { + repair_in( + &state, + &optiscaler::cache_dir(), + &gamedb::GameDb::load(), + target, + ) + }) } fn repair_in( @@ -1892,6 +1927,44 @@ mod tests { } } + #[test] + fn two_applies_at_once_never_take_ours_for_the_games_original() { + let dir = tempfile::tempdir().unwrap(); + let (state, cache) = (dir.path().join("state"), dir.path().join("cache")); + cached_release(&cache, "1.0.0"); + let game = steam_game(dir.path(), "library/Game"); + std::fs::write(game.install_root.join("dxgi.dll"), b"the game's own").unwrap(); + let db = gamedb::GameDb::from_texts(None); + let plan = optiscaler_plan(); + let version = config::VersionPolicy::Pinned("1.0.0".into()); + // A double click: two Apply runs of one game, side by side. + let done: Vec = std::thread::scope(|s| { + let runs: Vec<_> = (0..2) + .map(|_| { + s.spawn(|| { + locked(&state, &game, || { + install_in(&state, &cache, &db, &game, &plan, &version, &[]) + }) + .is_ok() + }) + }) + .collect(); + runs.into_iter().map(|r| r.join().unwrap()).collect() + }); + // The second waits, finds the first's install and is refused. + assert_eq!(done.iter().filter(|ok| **ok).count(), 1, "{done:?}"); + restore_in(&state, &game).unwrap(); + assert_eq!( + std::fs::read(game.install_root.join("dxgi.dll")).unwrap(), + b"the game's own" + ); + assert!( + manifest::Manifest::load(&state, &game.key()) + .unwrap() + .is_none() + ); + } + #[test] fn a_game_moved_to_another_library_is_restored_where_it_is_now() { let s = installed_game(); @@ -2089,12 +2162,12 @@ mod tests { // Something is installed: the transaction refuses. assert!(apply_release(&s.state, &s.game, &other, &cached, Path::new(""), &[]).is_err()); assert_eq!(std::fs::read_to_string(&staged).unwrap(), before); - assert!( - !s.state - .join(&key) - .join(format!("staging-{}", std::process::id())) - .exists() - ); + let left: Vec<_> = std::fs::read_dir(s.state.join(&key)) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .filter(|n| n.starts_with("staging-")) + .collect(); + assert!(left.is_empty(), "{left:?}"); } #[test] diff --git a/bigame-engine/bigame-core/src/graphics/optiscaler.rs b/bigame-engine/bigame-core/src/graphics/optiscaler.rs index c0ddc9d..2647c29 100644 --- a/bigame-engine/bigame-core/src/graphics/optiscaler.rs +++ b/bigame-engine/bigame-core/src/graphics/optiscaler.rs @@ -621,10 +621,10 @@ pub fn get_ini(text: &str, section: &str, key: &str) -> Option { if !inside || t.starts_with(';') || t.starts_with('#') { continue; } - if let Some((k, v)) = t.split_once('=') { - if k.trim().eq_ignore_ascii_case(key) { - return Some(v.trim().to_owned()); - } + if let Some((k, v)) = t.split_once('=') + && k.trim().eq_ignore_ascii_case(key) + { + return Some(v.trim().to_owned()); } } None @@ -1054,10 +1054,11 @@ impl LogFindings { pub fn read_log(text: &str) -> LogFindings { let mut f = LogFindings::default(); for line in text.lines() { - if let Some(rest) = line.split("OptiScaler v").nth(1) { - if line.contains(" loaded") && f.version.is_none() { - f.version = rest.split_whitespace().next().map(str::to_owned); - } + if let Some(rest) = line.split("OptiScaler v").nth(1) + && line.contains(" loaded") + && f.version.is_none() + { + f.version = rest.split_whitespace().next().map(str::to_owned); } if line.contains("Running on Wine") { f.wine = true; @@ -1068,13 +1069,12 @@ pub fn read_log(text: &str) -> LogFindings { .next() .map(str::to_owned); } - if let Some(rest) = line.split("Creating new ").nth(1) { - if let Some(name) = rest + if let Some(rest) = line.split("Creating new ").nth(1) + && let Some(name) = rest .strip_suffix(" upscaler") .or_else(|| rest.split_once(" upscaler").map(|(n, _)| n)) - { - f.upscalers.push(name.trim().to_owned()); - } + { + f.upscalers.push(name.trim().to_owned()); } if line.contains("Fsr4Update:") { f.fsr4 = line.find("RDNA4:").map(|i| line[i..].trim().to_owned()); diff --git a/bigame-engine/bigame-core/src/graphics/pe.rs b/bigame-engine/bigame-core/src/graphics/pe.rs index 256df1e..b8acb40 100644 --- a/bigame-engine/bigame-core/src/graphics/pe.rs +++ b/bigame-engine/bigame-core/src/graphics/pe.rs @@ -220,7 +220,7 @@ fn parse_with( let mut imports = Vec::new(); if let Some(t) = dir(1).and_then(|(rva, size)| table(rva, size, 20)) { - for desc in t.chunks_exact(20).take(MAX_DESCRIPTORS) { + for desc in t.as_chunks::<20>().0.iter().take(MAX_DESCRIPTORS) { if desc.iter().all(|&x| x == 0) { break; } @@ -232,7 +232,7 @@ fn parse_with( let mut delay_imports = Vec::new(); if let Some(t) = dir(13).and_then(|(rva, size)| table(rva, size, 32)) { - for desc in t.chunks_exact(32).take(MAX_DESCRIPTORS) { + for desc in t.as_chunks::<32>().0.iter().take(MAX_DESCRIPTORS) { let (attributes, name_rva) = (u32_at(desc, 0)?, u32_at(desc, 4)?); if name_rva == 0 { break; diff --git a/bigame-engine/bigame-core/src/graphics/plan.rs b/bigame-engine/bigame-core/src/graphics/plan.rs index 3a30717..774aeec 100644 --- a/bigame-engine/bigame-core/src/graphics/plan.rs +++ b/bigame-engine/bigame-core/src/graphics/plan.rs @@ -215,13 +215,15 @@ pub fn plan(r: &Report, cfg: &AiGraphicsConfig, ctx: &Context) -> Plan { let mut p = plan_for_gpu(r, cfg, ctx); // Two GPUs and the game not running: the plan is for the card games are // expected to use, which is only confirmed once the game has it open. - if cfg.mode != Mode::Off && r.gpus.len() > 1 && !r.gpus.iter().any(|g| g.renders_game) { - if let Some(g) = r.gpu() { - p.steps.push(Step::Note(Text::with( + if cfg.mode != Mode::Off + && r.gpus.len() > 1 + && !r.gpus.iter().any(|g| g.renders_game) + && let Some(g) = r.gpu() + { + p.steps.push(Step::Note(Text::with( N_("this computer has more than one GPU: the plan is for %s, the one DXVK and VKD3D-Proton pick for Windows games; it is confirmed when the game runs"), [g.name.clone()], ))); - } } p } diff --git a/bigame-engine/bigame-core/src/graphics/report.rs b/bigame-engine/bigame-core/src/graphics/report.rs index ef923d4..22bc3b1 100644 --- a/bigame-engine/bigame-core/src/graphics/report.rs +++ b/bigame-engine/bigame-core/src/graphics/report.rs @@ -400,21 +400,20 @@ impl Report { /// running game shows, or its files say, is never replaced. #[must_use] pub fn with_listing(mut self, entry: Option) -> Self { - if let Some(e) = &entry { - if let Some(api) = e.api { - if self.api.confidence > Confidence::Detected { - self.api.api = Some(api); - self.api.confidence = Confidence::Detected; - self.api.evidence.push(match e.origin { - super::gamedb::Origin::Carried => Text::plain(N_( - "Big Game Mode's game list names the API this game renders with by default", - )), - super::gamedb::Origin::User => Text::plain(N_( - "your game list names the API this game renders with", - )), - }); + if let Some(e) = &entry + && let Some(api) = e.api + && self.api.confidence > Confidence::Detected + { + self.api.api = Some(api); + self.api.confidence = Confidence::Detected; + self.api.evidence.push(match e.origin { + super::gamedb::Origin::Carried => Text::plain(N_( + "Big Game Mode's game list names the API this game renders with by default", + )), + super::gamedb::Origin::User => { + Text::plain(N_("your game list names the API this game renders with")) } - } + }); } self.listed = entry; self diff --git a/bigame-engine/bigame-core/src/graphics/scan.rs b/bigame-engine/bigame-core/src/graphics/scan.rs index 5641a83..45f5932 100644 --- a/bigame-engine/bigame-core/src/graphics/scan.rs +++ b/bigame-engine/bigame-core/src/graphics/scan.rs @@ -512,13 +512,13 @@ fn anti_cheat_near(root: &Path, exe: Option<&Path>) -> Vec { let Ok(ft) = entry.file_type() else { continue }; let name = entry.file_name().to_string_lossy().to_ascii_lowercase(); let child = rel.join(entry.file_name()); - if let Some(ac) = anti_cheat_marker(&name, ft.is_dir()) { - if !found.iter().any(|a| a.name == ac) { - found.push(AntiCheat { - name: ac.to_owned(), - evidence: child.clone(), - }); - } + if let Some(ac) = anti_cheat_marker(&name, ft.is_dir()) + && !found.iter().any(|a| a.name == ac) + { + found.push(AntiCheat { + name: ac.to_owned(), + evidence: child.clone(), + }); } if ft.is_dir() { stack.push((child, left - 1)); @@ -598,10 +598,10 @@ fn choose_executable( .filter(|(p, dir)| !dir && has_ext(&lower_name(p), "exe")) .map(|(p, _)| p) .collect(); - if let Some(hint) = hint.map(str::to_ascii_lowercase) { - if let Some(e) = exes.iter().find(|e| lower_name(e) == hint) { - return Some((*e).clone()); - } + if let Some(hint) = hint.map(str::to_ascii_lowercase) + && let Some(e) = exes.iter().find(|e| lower_name(e) == hint) + { + return Some((*e).clone()); } // Unreal's crash reporter is a `-Win64-Shipping.exe` too. if let Some(e) = exes.iter().find(|e| { @@ -708,8 +708,10 @@ fn wide_strings_with(bytes: &[u8], needle: &str) -> Vec { end += 2; } let s: String = bytes[start..end] - .chunks_exact(2) - .map(|c| char::from(c[0])) + .as_chunks::<2>() + .0 + .iter() + .map(|[c, _]| char::from(*c)) .collect(); if !out.contains(&s) { out.push(s); @@ -792,10 +794,9 @@ pub fn runs_as(root: &Path, process: &str) -> Vec { if let Some(real) = resolve_ci(root, process) .and_then(|exe| follow_bootstrap(root, &exe)) .and_then(|p| p.file_name().map(|n| n.to_string_lossy().into_owned())) + && !real.eq_ignore_ascii_case(process) { - if !real.eq_ignore_ascii_case(process) { - names.push(real); - } + names.push(real); } names } @@ -930,13 +931,13 @@ fn scan_limited(root: &Path, exe_hint: Option<&str>, max_entries: usize) -> Game let mut exe_dir_dlls = Vec::new(); for (rel, is_dir) in &files { let name = lower_name(rel); - if let Some(ac) = anti_cheat_marker(&name, *is_dir) { - if !anti_cheat.iter().any(|a: &AntiCheat| a.name == ac) { - anti_cheat.push(AntiCheat { - name: ac.to_owned(), - evidence: rel.clone(), - }); - } + if let Some(ac) = anti_cheat_marker(&name, *is_dir) + && !anti_cheat.iter().any(|a: &AntiCheat| a.name == ac) + { + anti_cheat.push(AntiCheat { + name: ac.to_owned(), + evidence: rel.clone(), + }); } if *is_dir { continue; @@ -994,13 +995,12 @@ fn scan_limited(root: &Path, exe_hint: Option<&str>, max_entries: usize) -> Game if let Some(ac) = executable .as_ref() .and_then(|e| known_protected_executable(&lower_name(e))) + && !anti_cheat.iter().any(|a| a.name == ac) { - if !anti_cheat.iter().any(|a| a.name == ac) { - anti_cheat.push(AntiCheat { - name: ac.to_owned(), - evidence: executable.clone().unwrap_or_default(), - }); - } + anti_cheat.push(AntiCheat { + name: ac.to_owned(), + evidence: executable.clone().unwrap_or_default(), + }); } components.sort_by(|a, b| a.kind.cmp(&b.kind).then_with(|| a.path.cmp(&b.path))); proxies.sort_by(|a, b| a.slot.cmp(&b.slot)); diff --git a/bigame-engine/bigame-core/src/graphics/support.rs b/bigame-engine/bigame-core/src/graphics/support.rs index 17e4c3d..c83623f 100644 --- a/bigame-engine/bigame-core/src/graphics/support.rs +++ b/bigame-engine/bigame-core/src/graphics/support.rs @@ -175,22 +175,21 @@ fn contents(target: &Target, a: &Analysis, masks: &Masks) -> Result Result { + use std::os::fd::AsRawFd; + manifest::check_relative(Path::new(game_key))?; + let dir = state_dir.join(".locks"); + std::fs::create_dir_all(&dir).with_context(|| format!("create {}", dir.display()))?; + let path = dir.join(game_key); + let f = std::fs::OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(&path) + .with_context(|| format!("lock {}", path.display()))?; + loop { + // SAFETY: flock on a descriptor this function owns, kept open by the + // file returned. + if unsafe { libc::flock(f.as_raw_fd(), libc::LOCK_EX) } == 0 { + return Ok(f); + } + let e = std::io::Error::last_os_error(); + if e.kind() != std::io::ErrorKind::Interrupted { + return Err(e).with_context(|| format!("lock {}", path.display())); + } + } +} + +/// Create a backup folder for an apply that starts at `now`, named after +/// the second it starts in — which the manifest records to find it again — +/// or the first later second whose folder is not there yet. A folder left +/// from an earlier install in the same second holds originals a removal +/// kept (files another program changed since); sharing it, this install's +/// removal would delete them with its own backups. +fn new_backup_root(state_dir: &Path, game_key: &str, now: u64) -> Result<(u64, PathBuf)> { + let parent = Manifest::backup_dir(state_dir, game_key); + std::fs::create_dir_all(&parent).with_context(|| format!("create {}", parent.display()))?; + for at in now..now.saturating_add(1000) { + let root = parent.join(at.to_string()); + match std::fs::create_dir(&root) { + Ok(()) => return Ok((at, root)), + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(e) => return Err(e).with_context(|| format!("create {}", root.display())), + } + } + bail!("no free backup folder in {}", parent.display()) +} + /// Copy `src` over `target` atomically: a temporary file in the target's own /// folder (same filesystem, so the rename is atomic), synced, then renamed. fn place(src: &Path, target: &Path) -> Result<()> { @@ -153,10 +215,14 @@ enum Found { Missing, /// A plain file, with its hash. File(String), - /// A symlink, at the path or on the way to it, or something that is not - /// a plain file: a mod manager deploys its files as links, or the user - /// moved a folder elsewhere. Not Big Game Mode's to follow or remove. + /// A symlink at the path, or something that is not a plain file: a mod + /// manager deploys its files as links. Not Big Game Mode's to follow or + /// remove, and nothing of Big Game Mode's is there any more. Foreign, + /// Behind a folder on the way that is a symlink now: what is there may + /// still be the file that was placed, and the link is not Big Game + /// Mode's to follow. + Unreachable, } /// What is at `rel` under `root`, and the path it was looked for at. @@ -167,7 +233,16 @@ enum Found { fn found_at(root: &Path, rel: &Path) -> Result<(PathBuf, Found)> { manifest::check_relative(rel)?; let Ok(target) = resolve_inside(root, rel) else { - return Ok((root.join(rel), Found::Foreign)); + let on_the_way = rel + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .is_some_and(|p| resolve_inside(root, p).is_err()); + let found = if on_the_way { + Found::Unreachable + } else { + Found::Foreign + }; + return Ok((root.join(rel), found)); }; let found = match std::fs::symlink_metadata(&target) { Ok(m) if m.file_type().is_symlink() || !m.is_file() => Found::Foreign, @@ -342,20 +417,17 @@ pub fn apply( if files.is_empty() { bail!(UserError::plain(N_("nothing to install"))); } - let started_at = crate::unix_now(); - let backup_root = Manifest::backup_dir(state_dir, game_key).join(started_at.to_string()); - // 1. Check, and hash what will be placed. A file already there under // another letter case is the one replaced. let mut files = files.to_vec(); let mut targets = Vec::with_capacity(files.len()); for f in &mut files { let mut target = resolve_inside(install_root, &f.path)?; - if let Some(name) = spelling_on_disk(&target)? { - if Some(name.as_os_str()) != target.file_name() { - f.path.set_file_name(&name); - target = resolve_inside(install_root, &f.path)?; - } + if let Some(name) = spelling_on_disk(&target)? + && Some(name.as_os_str()) != target.file_name() + { + f.path.set_file_name(&name); + target = resolve_inside(install_root, &f.path)?; } if !f.source.is_file() { bail!("missing payload file {}", f.source.display()); @@ -373,6 +445,7 @@ pub fn apply( } // 2. Back up originals, verified, before anything changes. + let (started_at, backup_root) = new_backup_root(state_dir, game_key, crate::unix_now())?; let entries = back_up_originals(files, &targets, &backup_root)?; let created_dirs = dirs_to_create(install_root, files); @@ -453,7 +526,8 @@ pub fn apply( /// The game's own settings the manifest records are not undone here /// ([`super::ingame::restore`] is the caller's): while there are any, the /// manifest is not deleted but kept with them alone, so they always have a -/// record. +/// record. So are the entries behind a folder that became a symlink: the +/// file there may still be the one placed. /// /// # Errors /// Returns an error if the game's folder is not there (it was moved, or its @@ -473,10 +547,31 @@ pub fn rollback(state_dir: &Path, m: &Manifest) -> Result> { let mut backups_still_needed = false; let mut kept_now = Vec::new(); let mut touched = std::collections::BTreeSet::new(); + // Entries still recorded after this rollback. + let mut unreachable = Vec::new(); + // One folder for every edited config this rollback keeps, named apart + // from any other rollback's: two in the same second would otherwise + // write over each other's copies. + let edited_root = Manifest::backup_dir(state_dir, &m.game_key).join(manifest::unique_name( + &format!("edited-{}-", crate::unix_now()), + )); for e in &m.entries { let (target, found) = found_at(&m.install_root, &e.path)?; let original = e.replaced.as_ref(); let current = match found { + Found::Unreachable => { + // Deleting the record would leave what may still be + // OptiScaler in the game with nothing that knows it: the + // entry stays recorded with its original's backup, the game + // still reads as changed, and Restore finishes once the + // folder is a folder again. + backups_still_needed |= original.is_some(); + tracing::warn!(target: "graphics", file = %target.display(), + "behind a link now; left in place and still recorded"); + unreachable.push(e.clone()); + outcomes.push(FileOutcome::KeptChanged(e.path.clone())); + continue; + } Found::Foreign => { backups_still_needed |= original.is_some(); if let Some(b) = original { @@ -521,9 +616,7 @@ pub fn rollback(state_dir: &Path, m: &Manifest) -> Result> { FileOutcome::KeptChanged(e.path.clone()) } FileKind::Config => { - let keep = Manifest::backup_dir(state_dir, &m.game_key) - .join(format!("edited-{}", crate::unix_now())) - .join(&e.path); + let keep = edited_root.join(&e.path); std::fs::create_dir_all(keep.parent().context("no parent")?)?; std::fs::copy(&target, &keep)?; restore_or_remove(&target, original)?; @@ -568,28 +661,45 @@ pub fn rollback(state_dir: &Path, m: &Manifest) -> Result> { for dir in touched.iter().filter(|d| d.is_dir()) { sync_dir(dir)?; } - if !kept_now.is_empty() { - if let Err(e) = remember_kept(state_dir, m, &kept_now) { - tracing::warn!(target: "graphics", game = %m.game_key, error = %format!("{e:#}"), + if !kept_now.is_empty() + && let Err(e) = remember_kept(state_dir, m, &kept_now) + { + tracing::warn!(target: "graphics", game = %m.game_key, error = %format!("{e:#}"), "the list of kept copies could not be written"); - } } - if m.settings.is_empty() { + let files_left = !unreachable.is_empty(); + if m.settings.is_empty() && !files_left { Manifest::delete(state_dir, &m.game_key)?; } else { - // Files gone, the game's own settings still to put back: the record - // stays, with nothing else in it. + // The game's own settings still to put back, or files behind a + // link still in the game: the record stays, with only them in it — + // and the folders the install created around those files, removed + // with them later. An apply cut short stays one, for the recovery + // at the next start to try again. + let created_dirs = m + .created_dirs + .iter() + .filter(|d| unreachable.iter().any(|e| e.path.starts_with(d))) + .cloned() + .collect(); Manifest { - state: State::Installed, - entries: Vec::new(), - created_dirs: Vec::new(), + state: if files_left { + m.state + } else { + State::Installed + }, + entries: unreachable, + created_dirs, generated: Vec::new(), ..m.clone() } .save(state_dir)?; } - // The configured payload copies are only needed while installed. - let _ = std::fs::remove_dir_all(state_dir.join(&m.game_key).join("staging")); + // The configured payload copies are only needed while installed (Repair + // and Apply read the staged ini). + if !files_left { + let _ = std::fs::remove_dir_all(state_dir.join(&m.game_key).join("staging")); + } if !backups_still_needed { let _ = std::fs::remove_dir_all( Manifest::backup_dir(state_dir, &m.game_key).join(m.started_at.to_string()), @@ -638,6 +748,10 @@ pub fn kept(state_dir: &Path) -> Vec { } fn remember_kept(state_dir: &Path, m: &Manifest, now: &[(PathBuf, PathBuf)]) -> Result<()> { + // One list for every game, and a game's lock only keeps changes to that + // game apart: two games restored at once would each write the list + // without the other's copies. + let _list = lock(state_dir, ".kept")?; let mut all = kept(state_dir); let at = crate::unix_now(); for (file, copy) in now { @@ -650,7 +764,7 @@ fn remember_kept(state_dir: &Path, m: &Manifest, now: &[(PathBuf, PathBuf)]) -> }); } let path = kept_path(state_dir); - let tmp = path.with_extension("json.tmp"); + let tmp = state_dir.join(manifest::unique_name("kept.json.tmp-")); std::fs::write(&tmp, serde_json::to_string_pretty(&all)?)?; std::fs::rename(&tmp, &path)?; Ok(()) @@ -698,11 +812,21 @@ pub fn recover(state_dir: &Path) -> Result> }; for d in dirs.flatten() { let key = d.file_name().to_string_lossy().into_owned(); - if let Ok(Some(m)) = Manifest::load(state_dir, &key) { - if m.state == State::Applying { - tracing::warn!(target: "graphics", game = %key, "interrupted apply found; rolling back"); - done.push((key, rollback(state_dir, &m))); - } + let applying = + |m: &Option| m.as_ref().is_some_and(|m| m.state == State::Applying); + if !Manifest::load(state_dir, &key).is_ok_and(|m| applying(&m)) { + continue; + } + // An apply of this process may be running (the page is open while + // this runs at start): read again once it is done, under the lock. + let Ok(_lock) = lock(state_dir, &key) else { + continue; + }; + if let Ok(Some(m)) = Manifest::load(state_dir, &key) + && m.state == State::Applying + { + tracing::warn!(target: "graphics", game = %key, "interrupted apply found; rolling back"); + done.push((key, rollback(state_dir, &m))); } } Ok(done) @@ -1290,9 +1414,120 @@ mod tests { b"cfg", "not followed" ); + // What is behind the link may still be OptiScaler: the game stays + // recorded as changed, with that file alone. + let left = Manifest::load(&fx.state, "g").unwrap().unwrap(); + assert_eq!(left.state, State::Installed); + assert_eq!( + left.entries + .iter() + .map(|e| e.path.clone()) + .collect::>(), + [PathBuf::from("bin/OptiScaler.ini")] + ); + // The folder is a folder again: the next Restore finishes. + std::fs::remove_file(fx.game.join("bin")).unwrap(); + std::fs::rename(&elsewhere, fx.game.join("bin")).unwrap(); + assert_eq!( + remove(&fx.state, "g").unwrap(), + [FileOutcome::Removed("bin/OptiScaler.ini".into())] + ); + assert!(!fx.game.join("bin/OptiScaler.ini").exists()); assert!(Manifest::load(&fx.state, "g").unwrap().is_none()); } + #[test] + fn a_link_in_place_of_the_file_itself_ends_the_record() { + let fx = fixture(); + let files = [planned(&fx, "dxgi.dll", b"ours", FileKind::Binary)]; + apply(&fx.state, &g(&fx), src(), &files, &[]).unwrap(); + // A mod manager deployed its own dxgi.dll as a link: nothing of + // Big Game Mode's is left there to keep a record of. + std::fs::remove_file(fx.game.join("dxgi.dll")).unwrap(); + std::os::unix::fs::symlink(fx.payload.join("dxgi.dll"), fx.game.join("dxgi.dll")).unwrap(); + assert_eq!( + remove(&fx.state, "g").unwrap(), + [FileOutcome::KeptChanged("dxgi.dll".into())] + ); + assert!(Manifest::load(&fx.state, "g").unwrap().is_none()); + } + + #[test] + fn a_backup_folder_from_the_same_second_is_never_shared() { + let fx = fixture(); + // Originals an earlier removal kept, in folders named after the + // seconds this apply may start in. + let now = crate::unix_now(); + let backups = Manifest::backup_dir(&fx.state, "g"); + for at in now..now + 10 { + std::fs::create_dir_all(backups.join(at.to_string())).unwrap(); + std::fs::write(backups.join(at.to_string()).join("dxgi.dll"), b"kept").unwrap(); + } + std::fs::write(fx.game.join("dxgi.dll"), b"original").unwrap(); + let files = [planned(&fx, "dxgi.dll", b"ours", FileKind::Binary)]; + let m = apply(&fx.state, &g(&fx), src(), &files, &[]).unwrap(); + assert!(m.started_at >= now + 10, "{}", m.started_at); + let backup = m.entries[0].replaced.as_ref().unwrap(); + assert!( + backup + .path + .starts_with(backups.join(m.started_at.to_string())) + ); + remove(&fx.state, "g").unwrap(); + assert_eq!(read(&fx.game.join("dxgi.dll")), b"original"); + for at in now..now + 10 { + assert_eq!( + read(&backups.join(at.to_string()).join("dxgi.dll")), + b"kept" + ); + } + assert!(!backups.join(m.started_at.to_string()).exists()); + } + + #[test] + fn edited_configs_two_removals_keep_are_never_written_over() { + let fx = fixture(); + let files = [planned(&fx, "OptiScaler.ini", b"a=1", FileKind::Config)]; + let mut copies = Vec::new(); + for edit in [b"a=2", b"a=3"] { + apply(&fx.state, &g(&fx), src(), &files, &[]).unwrap(); + std::fs::write(fx.game.join("OptiScaler.ini"), edit).unwrap(); + let out = remove(&fx.state, "g").unwrap(); + let [FileOutcome::EditedCopyKept(_, copy)] = out.as_slice() else { + panic!("{out:?}") + }; + copies.push(copy.clone()); + } + assert_ne!(copies[0], copies[1]); + assert_eq!(read(&copies[0]), b"a=2"); + assert_eq!(read(&copies[1]), b"a=3"); + } + + #[test] + fn a_games_lock_keeps_a_second_change_waiting() { + let fx = fixture(); + let first = lock(&fx.state, "g").unwrap(); + let (tx, rx) = std::sync::mpsc::channel(); + let state = fx.state.clone(); + let waiter = std::thread::spawn(move || { + let _second = lock(&state, "g").unwrap(); + tx.send(()).unwrap(); + }); + // Another game is not held up. + drop(lock(&fx.state, "other").unwrap()); + assert!( + rx.recv_timeout(std::time::Duration::from_millis(200)) + .is_err(), + "a second lock of one game was given while the first was held" + ); + drop(first); + rx.recv_timeout(std::time::Duration::from_secs(5)).unwrap(); + waiter.join().unwrap(); + // Not a folder a removal or the recovery takes for a game's. + assert!(Manifest::load(&fx.state, ".locks").unwrap().is_none()); + assert!(lock(&fx.state, "../g").is_err()); + } + #[test] fn what_is_kept_after_a_removal_is_listed() { let fx = fixture(); diff --git a/bigame-engine/bigame-core/src/graphics/versions.rs b/bigame-engine/bigame-core/src/graphics/versions.rs index 2ef8cdd..4705192 100644 --- a/bigame-engine/bigame-core/src/graphics/versions.rs +++ b/bigame-engine/bigame-core/src/graphics/versions.rs @@ -84,12 +84,6 @@ fn save(cache: &Path, known: &Known) -> Result<()> { Ok(()) } -fn now() -> u64 { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map_or(0, |d| d.as_secs()) -} - /// Fetch the release list from GitHub and save it. /// /// `curl` with an argument vector, no `~/.curlrc`, HTTPS only, size-capped, @@ -131,7 +125,7 @@ pub fn refresh(cache: &Path) -> Result { )) ); let known = Known { - fetched_at: now(), + fetched_at: crate::unix_now(), releases, }; save(cache, &known)?; @@ -145,7 +139,7 @@ pub fn refresh(cache: &Path) -> Result { #[must_use] pub fn load_fresh(cache: &Path) -> Known { let known = load(cache); - if !known.stale(now()) { + if !known.stale(crate::unix_now()) { return known; } match refresh(cache) { @@ -206,10 +200,10 @@ pub fn for_installed(cache: &Path, source: &Source) -> Result { .is_none_or(|h| h.eq_ignore_ascii_case(&r.sha256)) }; let recommended = Release::recommended(); - if let Some(c) = optiscaler::cached_version(cache, &source.version) { - if same(&c.release) { - return Ok(c.release); - } + if let Some(c) = optiscaler::cached_version(cache, &source.version) + && same(&c.release) + { + return Ok(c.release); } if same(&recommended) { return Ok(recommended); diff --git a/bigame-engine/bigame-core/src/hardware.rs b/bigame-engine/bigame-core/src/hardware.rs index 4331435..1c20cae 100644 --- a/bigame-engine/bigame-core/src/hardware.rs +++ b/bigame-engine/bigame-core/src/hardware.rs @@ -217,27 +217,6 @@ impl Gpu { .ok() .map(|s| s.trim().to_owned()) } - - /// Read an integer from this card's hwmon directory. - #[must_use] - pub fn hwmon_u64(&self, attr: &str) -> Option { - let dir = self.hwmon.as_ref()?; - std::fs::read_to_string(dir.join(attr)) - .ok()? - .trim() - .parse() - .ok() - } - - /// Current GPU utilisation percentage (`gpu_busy_percent`), AMD only. - #[must_use] - pub fn busy_percent(&self) -> Option { - std::fs::read_to_string(self.device_path.join("gpu_busy_percent")) - .ok()? - .trim() - .parse() - .ok() - } } // ── Display ────────────────────────────────────────────────────────────────── @@ -560,16 +539,16 @@ fn detect_gpus_in(drm: &Path, pci: &Path) -> Vec { // virtio-gpu's card hangs off `virtio0`, a child of the PCI device; // without the address that device would be listed again as a second // GPU with no card. - if slot.is_empty() { - if let Some(parent) = display_pci_ancestor(&device_path) { - if pci_id.is_empty() { - pci_id = pci_id_of(&parent).unwrap_or_default(); - } - slot = parent - .file_name() - .map(|n| n.to_string_lossy().into_owned()) - .unwrap_or_default(); + if slot.is_empty() + && let Some(parent) = display_pci_ancestor(&device_path) + { + if pci_id.is_empty() { + pci_id = pci_id_of(&parent).unwrap_or_default(); } + slot = parent + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); } let vram_total_bytes = read_trim(device_path.join("mem_info_vram_total")).and_then(|s| s.parse::().ok()); diff --git a/bigame-engine/bigame-core/src/health.rs b/bigame-engine/bigame-core/src/health.rs index cc6b007..4a7c32a 100644 --- a/bigame-engine/bigame-core/src/health.rs +++ b/bigame-engine/bigame-core/src/health.rs @@ -616,13 +616,11 @@ pub fn collect() -> Vec { if backend .as_ref() .is_some_and(crate::systemd::UnitState::is_active) - { - if let Some(c) = crate::systemd::Reader::shared() + && let Some(c) = crate::systemd::Reader::shared() .and_then(|r| r.restarts(crate::turbo::BACKEND_UNIT)) .and_then(restart_check) - { - out.push(c); - } + { + out.push(c); } if status.as_ref().is_some_and(|s| s.dmem_cgroup.is_none()) && caps.falcond_installed { let kernel_can = Path::new("/sys/fs/cgroup/dmem.capacity").exists(); @@ -637,15 +635,16 @@ pub fn collect() -> Vec { None, )); } - if let Some(s) = &status { - if s.profile_mode == "handheld" && !matches!(hw.chassis, Chassis::Handheld) { - out.push(check( + if let Some(s) = &status + && s.profile_mode == "handheld" + && !matches!(hw.chassis, Chassis::Handheld) + { + out.push(check( N_("falcond profile set"), Status::Warning, N_("handheld profiles on a machine that is not a handheld: games run in power-saving mode"), advice(N_("Turn Turbo on — or off and on again if it is already on — to switch falcond to its desktop profiles")), )); - } } // sched-ext diff --git a/bigame-engine/bigame-core/src/heroic_launch.rs b/bigame-engine/bigame-core/src/heroic_launch.rs index eea7bb0..3d93e72 100644 --- a/bigame-engine/bigame-core/src/heroic_launch.rs +++ b/bigame-engine/bigame-core/src/heroic_launch.rs @@ -416,10 +416,10 @@ fn undo(game: &mut Map, previous: &Written) { } } if let Some(gs) = game.get_mut(GAMESCOPE).and_then(Value::as_object_mut) { - if let Some(words) = &previous.options { - if let Some(Value::String(opts)) = gs.get_mut(OPTIONS) { - *opts = without_words(opts, words); - } + if let Some(words) = &previous.options + && let Some(Value::String(opts)) = gs.get_mut(OPTIONS) + { + *opts = without_words(opts, words); } undo_keys(gs, &previous.gamescope); } diff --git a/bigame-engine/bigame-core/src/launcher.rs b/bigame-engine/bigame-core/src/launcher.rs index afb4361..b6ffa7d 100644 --- a/bigame-engine/bigame-core/src/launcher.rs +++ b/bigame-engine/bigame-core/src/launcher.rs @@ -338,10 +338,10 @@ impl LaunchPlan { // it, and DXVK takes the last value. if let (Some(theirs), Some(ours)) = (host.env.get("DXVK_CONFIG"), env.get_mut("DXVK_CONFIG")) + && !is_a_presets(&host.env, "DXVK_CONFIG") + && !theirs.trim().is_empty() { - if !is_a_presets(&host.env, "DXVK_CONFIG") && !theirs.trim().is_empty() { - *ours = format!("{}; {ours}", theirs.trim().trim_end_matches(';')); - } + *ours = format!("{}; {ours}", theirs.trim().trim_end_matches(';')); } } @@ -996,10 +996,11 @@ fn collect_upscaling_env(upscaling: &UpscalingSettings, env: &mut HashMap Option<(Level, &str)> { /// gvfs, inside the UI, reports each volume monitor the system has masked or /// not installed, every time GIO starts, with "failed" in the wording. /// Wine's `fixme:` lines are notes to Wine's developers. falcond's `sudo` -/// calls open and close a PAM session each time. +/// calls open and close a PAM session each time. The kernel's DRM drivers +/// note at every boot that their planes can show a kernel panic ("Registered +/// 5 planes with drm panic"): a capability, not a panic. lsfg-vk 2's layer +/// is implicit and stands aside in every process without a profile, the UI +/// included; the Vulkan loader reports each time that it skipped it. KDE +/// writes `gtk-application-prefer-dark-theme` into GTK's settings, which +/// libadwaita ignores, and says so at every start. const NOISE: &[&str] = &[ "for remote volume monitor with dbus name", ":fixme:", "pam_unix(", + "planes with drm panic", + "failed to find 'vkgetinstanceprocaddr' in layer \"liblsfg-vk-layer.so\"", + "gtk-application-prefer-dark-theme with libadwaita is unsupported", ]; /// What a sched-ext scheduler prints each time it starts, and what the @@ -604,10 +613,10 @@ fn log_files() -> Vec<(PathBuf, FileFormat)> { home.join(".var/app/com.valvesoftware.Steam/.local/share/Steam"), ] { // ~/.steam/steam is normally a link to ~/.local/share/Steam. - if let Ok(path) = root.join("logs/console_log.txt").canonicalize() { - if seen.insert(path.clone()) { - out.push((path, FileFormat::SteamConsole)); - } + if let Ok(path) = root.join("logs/console_log.txt").canonicalize() + && seen.insert(path.clone()) + { + out.push((path, FileFormat::SteamConsole)); } } let state = crate::graphics::state_dir(); @@ -1227,6 +1236,52 @@ mod tests { ); } + #[test] + fn a_driver_that_can_show_a_panic_is_not_a_panic() { + // Real lines, printed by the kernel at every boot at priority 6. + for line in [ + "amdgpu 0000:01:00.0: [drm] Registered 5 planes with drm panic", + "simple-framebuffer simple-framebuffer.0: [drm] Registered 1 planes with drm panic", + ] { + assert_eq!(classify(Some(6), line), Level::Debug, "{line}"); + } + assert_eq!( + classify(Some(0), "Kernel panic - not syncing: Fatal exception"), + Level::Error + ); + } + + #[test] + fn what_other_components_say_at_every_start_is_not_a_problem() { + // Real lines, from bigame-ui's own output, at the priority the + // journal gave them. + assert_eq!( + classify( + Some(4), + "Vulkan: Loader Message: loader_create_device_chain: Failed to find \ + 'vkGetInstanceProcAddr' in layer \"liblsfg-vk-layer.so\". Skipping layer." + ), + Level::Debug + ); + assert_eq!( + classify( + Some(4), + "Using GtkSettings:gtk-application-prefer-dark-theme with libadwaita is \ + unsupported. Please use AdwStyleManager:color-scheme instead." + ), + Level::Debug + ); + // Another layer the loader cannot load is still an error. + assert_eq!( + classify( + Some(4), + "Vulkan: Loader Message: loader_create_device_chain: Failed to find \ + 'vkGetInstanceProcAddr' in layer \"libVkLayer_other.so\". Skipping layer." + ), + Level::Error + ); + } + #[test] fn the_levels_of_game_components_are_read_from_their_own_prefixes() { // Real lines from Steam's stream. diff --git a/bigame-engine/bigame-core/src/mangohud.rs b/bigame-engine/bigame-core/src/mangohud.rs index 38df45e..73eb96b 100644 --- a/bigame-engine/bigame-core/src/mangohud.rs +++ b/bigame-engine/bigame-core/src/mangohud.rs @@ -15,8 +15,8 @@ //! //! * **Steam** — the game's launch options, edited with Steam closed, backed //! up and read back (`steam::set_launch_options`). Options the user wrote -//! stay; only the word this module added (recorded in Big Game Mode's state -//! directory) is ever removed. +//! stay; only the word this module added, in that account (recorded in +//! Big Game Mode's state directory), is ever removed. //! * **Heroic** — the game's `GamesConfig/.json`, with the rest of the //! game's launch settings (`crate::heroic_launch`): Forced is Heroic's own //! `MangoHud` switch (`showMangohud`, its `mangohud --dlsym` wrapper), On @@ -26,7 +26,8 @@ //! settings in memory and writes them back when one changes, so the file //! is written with Heroic closed. //! * **Lutris** — the game's YAML: Lutris's own *FPS counter (`MangoHud`)* -//! option, `system: mangohud: true`, for On and Forced alike. +//! option, `system: mangohud: true`, for On and Forced alike. The line the +//! game had before is recorded, and Off puts it back. //! //! A launcher running as a Flatpak cannot see the system's `mangohud`: it //! needs Flathub's `org.freedesktop.Platform.VulkanLayer.MangoHud` for its @@ -119,38 +120,97 @@ fn added_path() -> std::path::PathBuf { crate::paths::state_home().join("bigame-mode/mangohud/steam-added.toml") } -/// The word recorded in `path` as added for `process`. With no record (an -/// older version wrote the options), the word for the mode it saved -/// (`saved`): it added that word whether or not the user had typed it. -fn read_added(path: &std::path::Path, process: &str, saved: Mode) -> Result> { - let records = read_records(path)?; - Ok(match records.get(process) { - Some(word) => Some(word.clone()).filter(|w| !w.is_empty()), +/// What is recorded of the words added for one game. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(untagged)] +enum SteamRecord { + /// The word in each account, by app (since 2.3.2). + Accounts(crate::steam::Inserted), + /// One word for every account, as versions before 2.3.2 recorded it. + Game(String), +} + +/// The word added for `process` in `account`'s options for `app`. With no +/// record (an older version wrote the options), the word for the mode it +/// saved (`saved`): it added that word whether or not the user had typed it. +fn added_in(record: Option<&SteamRecord>, saved: Mode, account: &str, app: &str) -> Option { + match record { + Some(SteamRecord::Accounts(by)) => by + .get(account) + .and_then(|apps| apps.get(app)) + .filter(|w| !w.is_empty()) + .cloned(), + Some(SteamRecord::Game(word)) => Some(word.clone()).filter(|w| !w.is_empty()), None => legacy_added(saved), - }) + } } -fn read_records(path: &std::path::Path) -> Result> { +/// A record file of Big Game Mode's state; empty when there is none. +/// +/// # Errors +/// Returns an error when it is there but cannot be read or parsed: a +/// broken record is said, not taken for "nothing added". +pub(crate) fn read_record( + path: &std::path::Path, +) -> Result { use anyhow::Context; match std::fs::read_to_string(path) { Ok(text) => toml::from_str(&text).with_context(|| format!("parse {}", path.display())), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(std::collections::BTreeMap::new()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(T::default()), Err(e) => Err(e).with_context(|| format!("read {}", path.display())), } } -/// Record `word` as added for `process` in `path`. -fn write_added(path: &std::path::Path, process: &str, word: Option<&str>) -> Result<()> { +/// Write a record file of Big Game Mode's state. +/// +/// # Errors +/// Returns an error when it cannot be written. +pub(crate) fn write_record(path: &std::path::Path, record: &T) -> Result<()> { use anyhow::Context; - let mut records = read_records(path)?; - records.insert(process.to_owned(), word.unwrap_or_default().to_owned()); if let Some(dir) = path.parent() { std::fs::create_dir_all(dir).with_context(|| format!("create {}", dir.display()))?; } - let tmp = path.with_extension("toml.new"); - std::fs::write(&tmp, toml::to_string(&records)?) - .with_context(|| format!("write {}", tmp.display()))?; - std::fs::rename(&tmp, path).with_context(|| format!("replace {}", path.display())) + replace_file(path, toml::to_string(record)?.as_bytes()) +} + +/// Every account's new options for `mode` in the game's apps, worked out +/// before any is written, with what to record and the last account's +/// options. Each account takes out only the word recorded as added there. +fn steam_plan( + accounts: &[std::path::PathBuf], + apps: &[String], + read: &dyn Fn(&std::path::Path, &str) -> Option, + record: Option<&SteamRecord>, + saved: Mode, + mode: Mode, +) -> (Vec, crate::steam::Inserted, String) { + let mut changes = Vec::new(); + let mut added = crate::steam::Inserted::new(); + let mut last = String::new(); + for config in accounts { + let account = config.to_string_lossy().into_owned(); + for app in apps { + let current = read(config, app).unwrap_or_default(); + let before = added_in(record, saved, &account, app); + let (wanted, add) = launch_options(¤t, before.as_deref(), mode); + if let Some(word) = add { + added + .entry(account.clone()) + .or_default() + .insert(app.clone(), word.to_owned()); + } + if wanted != current { + changes.push(crate::steam::Change { + config: config.clone(), + app: app.clone(), + before: current, + after: wanted.clone(), + }); + } + last = wanted; + } + } + (changes, added, last) } /// The word an older version added for a game saved at `mode`. @@ -213,37 +273,37 @@ pub fn apply(process: &str, mode: Mode) -> Result { return Ok(Applied::SteamRunning); } let users = crate::steam::users(&crate::paths::home_dir()); - if !apps.is_empty() && mode != Mode::Off && crate::steam::any_flatpak(&users) { - if let Some(command) = crate::steam::flatpak_mangohud_missing() { - anyhow::bail!(UserError::with( - N_( - "Steam's Flatpak finds MangoHud only in Flathub's MangoHud extension, which is not installed. Nothing was written. Install it and restart Steam: %s" - ), - [command] - )); - } + if !apps.is_empty() + && mode != Mode::Off + && crate::steam::any_flatpak(&users) + && let Some(command) = crate::steam::flatpak_mangohud_missing() + { + anyhow::bail!(UserError::with( + N_( + "Steam's Flatpak finds MangoHud only in Flathub's MangoHud extension, which is not installed. Nothing was written. Install it and restart Steam: %s" + ), + [command] + )); } let mut settings = crate::game_settings::load(process)?; let record = added_path(); - let added = read_added(&record, process, settings.mangohud)?; + let mut records: std::collections::BTreeMap = read_record(&record)?; // The launch options first, the saved choice after: if Steam's file cannot // be written, the choice stays as it was instead of claiming a mode the // game will not get. - let mut last = String::new(); - let mut now_added = None; - for user in &users { - for app in &apps { - let current = crate::steam::launch_options(&user.config, app).unwrap_or_default(); - let (wanted, add) = launch_options(¤t, added.as_deref(), mode); - if wanted != current { - crate::steam::set_launch_options(&user.config, app, &wanted)?; - } - now_added = now_added.or(add); - last = wanted; - } - } + let accounts: Vec = users.into_iter().map(|u| u.config).collect(); + let (changes, added, last) = steam_plan( + &accounts, + &apps, + &|config, app| crate::steam::launch_options(config, app), + records.get(process), + settings.mangohud, + mode, + ); + crate::steam::set_all(&changes)?; if !apps.is_empty() { - write_added(&record, process, now_added)?; + records.insert(process.to_owned(), SteamRecord::Accounts(added)); + write_record(&record, &records)?; } settings.mangohud = mode; crate::game_settings::save(process, &settings)?; @@ -284,12 +344,14 @@ fn apply_to_launcher( } } LauncherRef::Lutris { config_file } => { - let current = std::fs::read_to_string(config_file)?; - let wanted = lutris_config(¤t, mode); - if wanted != current { - write_keeping_backup(config_file, &wanted)?; - } let mut settings = crate::game_settings::load(process)?; + lutris_apply( + config_file, + &lutris_record_path(), + settings.mangohud, + mode, + &write_keeping_backup, + )?; settings.mangohud = mode; crate::game_settings::save(process, &settings)?; } @@ -334,9 +396,158 @@ pub(crate) fn write_keeping_backup_in( } else if let Some(dir) = file.parent() { std::fs::create_dir_all(dir)?; } - let tmp = file.with_extension("bigame-new"); - std::fs::write(&tmp, text).with_context(|| format!("write {}", tmp.display()))?; - std::fs::rename(&tmp, file).with_context(|| format!("replace {}", file.display())) + replace_file(file, text.as_bytes()) +} + +/// Replace `path` with `content`: written whole to a temporary of its own +/// beside it, flushed, renamed over it and the folder flushed, so a crash +/// leaves the old file or the new one, never half of either, and two +/// writers never share a temporary. The file keeps its permissions, and a +/// dotfile manager's symlink (stow, chezmoi) stays a symlink: the file it +/// points to is the one replaced. +pub(crate) fn replace_file(path: &std::path::Path, content: &[u8]) -> Result<()> { + use anyhow::Context; + use std::io::Write as _; + use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _}; + static SEQUENCE: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let path = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); + let dir = path.parent().context("path has no parent")?; + let name = path + .file_name() + .context("path has no file name")? + .to_string_lossy(); + let tmp = dir.join(format!( + ".{name}.bigame-{}-{}.tmp", + std::process::id(), + SEQUENCE.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); + let mode = std::fs::metadata(&path) + .ok() + .map(|m| m.permissions().mode() & 0o777); + let written = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .custom_flags(libc::O_NOFOLLOW) + .mode(mode.unwrap_or(0o644)) + .open(&tmp) + .and_then(|mut f| { + // The umask narrowed the mode it was created with. + if let Some(mode) = mode { + f.set_permissions(std::fs::Permissions::from_mode(mode))?; + } + f.write_all(content)?; + f.sync_all() + }); + let renamed = written + .with_context(|| format!("write {}", tmp.display())) + .and_then(|()| { + std::fs::rename(&tmp, &path).with_context(|| format!("replace {}", path.display())) + }); + if let Err(e) = renamed { + if let Err(cleanup) = std::fs::remove_file(&tmp) + && cleanup.kind() != std::io::ErrorKind::NotFound + { + tracing::warn!(path = %tmp.display(), error = %cleanup, "could not remove a temporary file"); + } + return Err(e); + } + // The file is in place; only the rename's durability is at stake. + if let Err(e) = std::fs::File::open(dir).and_then(|d| d.sync_all()) { + tracing::warn!(path = %dir.display(), error = %e, "could not flush a folder"); + } + Ok(()) +} + +/// Where the `mangohud` line each Lutris game had before Big Game Mode first +/// switched the option on is recorded, by the game's file (an empty line +/// for none). +fn lutris_record_path() -> std::path::PathBuf { + crate::paths::state_home().join("bigame-mode/mangohud/lutris-before.toml") +} + +/// The line Big Game Mode writes for On and Forced. +const LUTRIS_ON: &str = " mangohud: true"; + +/// Write `mode` into the Lutris game file `file`, with the game saved at +/// `saved` until now. The first time the option goes on, the line the game +/// had is recorded in `record`; Off puts that line back, and takes out +/// only Big Game Mode's own `mangohud: true` — a value the user changed +/// since in Lutris, or set before with nothing from Big Game Mode, stays. +fn lutris_apply( + file: &std::path::Path, + record: &std::path::Path, + saved: Mode, + mode: Mode, + write: &dyn Fn(&std::path::Path, &str) -> Result<()>, +) -> Result<()> { + let current = std::fs::read_to_string(file)?; + let mut before: std::collections::BTreeMap = read_record(record)?; + let key = file.to_string_lossy().into_owned(); + let wanted = if mode == Mode::Off { + match before.get(&key) { + Some(line) => lutris_off(¤t, Some(line.as_str()).filter(|l| !l.is_empty())), + // An older version wrote the option and kept no record of it. + None if saved != Mode::Off => lutris_off(¤t, None), + None => current.clone(), + } + } else { + if !before.contains_key(&key) { + // Recorded before the file changes: a failure between the two + // must not lose what the user had. + let line = lutris_line(¤t).unwrap_or_default().to_owned(); + before.insert(key.clone(), line); + write_record(record, &before)?; + } + lutris_config(¤t, mode) + }; + if wanted != current { + write(file, &wanted)?; + } + if mode == Mode::Off && before.remove(&key).is_some() { + write_record(record, &before)?; + } + Ok(()) +} + +/// Where the `mangohud` option of the `system:` section is in `lines`. +fn lutris_line_at(lines: &[&str]) -> Option { + let header = lines + .iter() + .position(|l| *l == "system:" || l.starts_with("system: "))?; + (header + 1..lines.len()) + .take_while(|&i| lines[i].is_empty() || lines[i].starts_with(' ')) + .find(|&i| { + lines[i].starts_with(" ") + && !lines[i].starts_with(" ") + && lines[i].trim_start().starts_with("mangohud:") + }) +} + +/// The `mangohud` line of a Lutris game YAML's `system:` section. +fn lutris_line(current: &str) -> Option<&str> { + let lines: Vec<&str> = current.lines().collect(); + lutris_line_at(&lines).map(|i| lines[i]) +} + +/// A Lutris game YAML with Big Game Mode's `mangohud: true` taken out: +/// `before`, the line the game had before it, put back, or the line +/// removed when it had none. A `mangohud` line other than Big Game Mode's +/// is the user's, and the file stays as it is. +#[must_use] +pub fn lutris_off(current: &str, before: Option<&str>) -> String { + let mut lines: Vec<&str> = current.lines().collect(); + let Some(at) = lutris_line_at(&lines).filter(|&i| lines[i] == LUTRIS_ON) else { + return current.to_owned(); + }; + let Some(line) = before else { + return lutris_config(current, Mode::Off); + }; + lines[at] = line; + let mut text = lines.join("\n"); + if current.ends_with('\n') { + text.push('\n'); + } + text } /// A Lutris game YAML with `mode` applied to its `system:` section's @@ -354,7 +565,7 @@ pub fn lutris_config(current: &str, mode: Mode) -> String { None if !on => return current.to_owned(), None => { out.push("system:".into()); - out.push(" mangohud: true".into()); + out.push(LUTRIS_ON.into()); } Some(h) => { // `system: {}` or another flow value: make it a block. @@ -369,7 +580,7 @@ pub fn lutris_config(current: &str, mode: Mode) -> String { out[i].starts_with(" ") && !out[i].starts_with(" ") && t.starts_with("mangohud:") }); match (found, on) { - (Some(i), true) => out[i] = " mangohud: true".into(), + (Some(i), true) => LUTRIS_ON.clone_into(&mut out[i]), (Some(i), false) => { out.remove(i); // A section left with nothing in it goes too. @@ -377,7 +588,7 @@ pub fn lutris_config(current: &str, mode: Mode) -> String { out.remove(h); } } - (None, true) => out.insert(h + 1, " mangohud: true".into()), + (None, true) => out.insert(h + 1, LUTRIS_ON.into()), (None, false) => {} } } @@ -599,7 +810,7 @@ pub enum StyleState { /// Read which style is in place. #[must_use] pub fn current_style() -> StyleState { - style_state_of(std::fs::read_to_string(style_path()).ok().as_deref()) + style_state_of(config_text(&style_path()).as_deref()) } fn style_state_of(text: Option<&str>) -> StyleState { @@ -627,37 +838,92 @@ pub fn set_style(style: Style) -> Result<()> { fn set_style_at(style: Style, path: &std::path::Path, backup: &std::path::Path) -> Result<()> { use anyhow::Context; - let state = style_state_of(std::fs::read_to_string(path).ok().as_deref()); + let state = style_state_of(config_text(path).as_deref()); match style_config(style) { None => { if !matches!(state, StyleState::Style(_)) { return Ok(()); // Already the user's own. } - if backup.exists() { - std::fs::rename(backup, path) - .with_context(|| format!("put back {}", path.display()))?; - } else { + if !put_back(backup, path)? { std::fs::remove_file(path).with_context(|| format!("remove {}", path.display()))?; } } Some(text) => { if state == StyleState::Own { - if let Some(dir) = backup.parent() { - std::fs::create_dir_all(dir)?; - } - std::fs::copy(path, backup).with_context(|| format!("keep {}", path.display()))?; + keep_aside(path, backup)?; } if let Some(dir) = path.parent() { std::fs::create_dir_all(dir)?; } - let tmp = path.with_extension("conf.bigame-new"); - std::fs::write(&tmp, text).with_context(|| format!("write {}", tmp.display()))?; - std::fs::rename(&tmp, path).with_context(|| format!("replace {}", path.display()))?; + replace_file(path, text.as_bytes())?; } } Ok(()) } +// ── A configuration file that may be the user's ───────────────────────────── + +/// The text of the configuration file at `path`, to look for Big Game +/// Mode's marker in its first line; `None` only when nothing is there. A +/// file that is not UTF-8, cannot be read, or is a symlink to nothing is +/// still something the user put there: it reads as text without the +/// marker, so it is kept before anything replaces it. +pub(crate) fn config_text(path: &std::path::Path) -> Option { + match std::fs::read(path) { + Ok(bytes) => Some(String::from_utf8_lossy(&bytes).into_owned()), + Err(e) + if e.kind() == std::io::ErrorKind::NotFound + && std::fs::symlink_metadata(path).is_err() => + { + None + } + Err(_) => Some(String::new()), + } +} + +/// Keep the user's file at `path` in `backup` before Big Game Mode's +/// replaces it: what it holds, byte for byte, or a symlink to nothing as +/// that symlink. +/// +/// # Errors +/// Returns an error, and `path` must then not be replaced, when it cannot +/// be kept. +pub(crate) fn keep_aside(path: &std::path::Path, backup: &std::path::Path) -> Result<()> { + use anyhow::Context; + if let Some(dir) = backup.parent() { + std::fs::create_dir_all(dir).with_context(|| format!("create {}", dir.display()))?; + } + // Copying onto a symlink kept earlier would write where it points. + if std::fs::symlink_metadata(backup).is_ok_and(|m| m.file_type().is_symlink()) { + std::fs::remove_file(backup).with_context(|| format!("remove {}", backup.display()))?; + } + let dangling = std::fs::symlink_metadata(path).is_ok_and(|m| m.file_type().is_symlink()) + && std::fs::metadata(path).is_err(); + if dangling { + let target = + std::fs::read_link(path).with_context(|| format!("read link {}", path.display()))?; + std::os::unix::fs::symlink(target, backup) + } else { + std::fs::copy(path, backup).map(drop) + } + .with_context(|| format!("keep {}", path.display())) +} + +/// Put the file kept in `backup` back at `path`; whether one was kept. A +/// symlink at `path` stays: the file it points to is the one put back. +/// +/// # Errors +/// Returns an error when the kept file cannot be moved back. +pub(crate) fn put_back(backup: &std::path::Path, path: &std::path::Path) -> Result { + use anyhow::Context; + if std::fs::symlink_metadata(backup).is_err() { + return Ok(false); + } + let path = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); + std::fs::rename(backup, &path).with_context(|| format!("put back {}", path.display()))?; + Ok(true) +} + #[cfg(test)] mod tests { use super::*; @@ -776,6 +1042,63 @@ mod tests { assert!(!path.exists()); } + #[test] + fn a_file_that_is_not_utf8_or_a_symlink_to_nothing_is_kept() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("MangoHud/MangoHud.conf"); + let backup = dir.path().join("state/MangoHud.conf.user"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + + // Latin-1, as an old editor saved it. + let latin1 = b"# r\xe9glages\nfps\n".to_vec(); + std::fs::write(&path, &latin1).unwrap(); + assert_eq!( + style_state_of(config_text(&path).as_deref()), + StyleState::Own + ); + set_style_at(Style::Basic, &path, &backup).unwrap(); + assert_eq!(std::fs::read(&backup).unwrap(), latin1); + set_style_at(Style::Own, &path, &backup).unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), latin1); + + // A link whose file is gone (an unmounted dotfiles folder). + std::fs::remove_file(&path).unwrap(); + let gone = dir.path().join("unmounted/MangoHud.conf"); + std::os::unix::fs::symlink(&gone, &path).unwrap(); + assert_eq!( + style_state_of(config_text(&path).as_deref()), + StyleState::Own + ); + set_style_at(Style::Full, &path, &backup).unwrap(); + assert_eq!(std::fs::read_link(&backup).unwrap(), gone); + set_style_at(Style::Own, &path, &backup).unwrap(); + assert_eq!(std::fs::read_link(&path).unwrap(), gone); + assert!(std::fs::symlink_metadata(&backup).is_err()); + } + + #[test] + fn a_dotfile_managers_symlink_stays_a_symlink() { + let dir = tempfile::tempdir().unwrap(); + let real = dir.path().join("dotfiles/MangoHud.conf"); + std::fs::create_dir_all(real.parent().unwrap()).unwrap(); + std::fs::write(&real, "fps\n").unwrap(); + let path = dir.path().join("MangoHud/MangoHud.conf"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::os::unix::fs::symlink(&real, &path).unwrap(); + let backup = dir.path().join("state/MangoHud.conf.user"); + + set_style_at(Style::Basic, &path, &backup).unwrap(); + assert_eq!(std::fs::read_link(&path).unwrap(), real); + assert!( + std::fs::read_to_string(&real) + .unwrap() + .starts_with(STYLE_MARKER) + ); + set_style_at(Style::Own, &path, &backup).unwrap(); + assert_eq!(std::fs::read_link(&path).unwrap(), real); + assert_eq!(std::fs::read_to_string(&real).unwrap(), "fps\n"); + } + /// `launch_options` as a chain of saves: what it added is what the next /// call takes out. fn steam(current: &str, added: Option<&str>, mode: Mode) -> (String, Option) { @@ -856,29 +1179,84 @@ mod tests { fn the_record_of_added_words_reads_back() { let dir = crate::tests::tempdir("mangohud_added"); let path = dir.join("state/steam-added.toml"); + let records: std::collections::BTreeMap = read_record(&path).unwrap(); // Nothing recorded: what an older version added for the saved mode. assert_eq!( - read_added(&path, "Game.exe", Mode::Forced) - .unwrap() - .as_deref(), + added_in(records.get("Game.exe"), Mode::Forced, "/a", "10").as_deref(), Some(WRAPPER) ); - write_added(&path, "Game.exe", Some(LAYER)).unwrap(); - write_added(&path, "Other.exe", None).unwrap(); + // A record from before 2.3.2: one word for every account. + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write( + &path, + "\"Game.exe\" = \"MANGOHUD=1\"\n\"Other.exe\" = \"\"\n", + ) + .unwrap(); + let mut records: std::collections::BTreeMap = + read_record(&path).unwrap(); + for account in ["/a", "/b"] { + assert_eq!( + added_in(records.get("Game.exe"), Mode::Forced, account, "10").as_deref(), + Some(LAYER) + ); + } assert_eq!( - read_added(&path, "Game.exe", Mode::Forced) - .unwrap() - .as_deref(), - Some(LAYER) + added_in(records.get("Other.exe"), Mode::On, "/a", "10"), + None + ); + // By account now, next to the older records. + let mut by = crate::steam::Inserted::new(); + by.entry("/a".into()) + .or_default() + .insert("10".into(), LAYER.into()); + records.insert("Game.exe".into(), SteamRecord::Accounts(by)); + write_record(&path, &records).unwrap(); + let records: std::collections::BTreeMap = read_record(&path).unwrap(); + let game = records.get("Game.exe"); + assert_eq!(added_in(game, Mode::On, "/a", "10").as_deref(), Some(LAYER)); + assert_eq!(added_in(game, Mode::On, "/b", "10"), None); + assert_eq!( + added_in(records.get("Other.exe"), Mode::On, "/a", "10"), + None ); - assert_eq!(read_added(&path, "Other.exe", Mode::On).unwrap(), None); std::fs::write(&path, "not = [toml").unwrap(); assert!( - read_added(&path, "Game.exe", Mode::Off).is_err(), + read_record::>(&path).is_err(), "a broken record is said" ); } + #[test] + fn each_account_takes_out_only_what_was_added_there() { + // The user typed MANGOHUD=1 in account "a"; "b" has nothing. + let store = std::cell::RefCell::new(std::collections::HashMap::from([ + ("a".to_owned(), "MANGOHUD=1 %command%".to_owned()), + ("b".to_owned(), "-novid".to_owned()), + ])); + let read = |c: &std::path::Path, _: &str| { + store.borrow().get(c.to_string_lossy().as_ref()).cloned() + }; + let accounts = [std::path::PathBuf::from("a"), "b".into()]; + let apps = ["10".to_owned()]; + let apply = |record: Option<&SteamRecord>, saved, mode| { + let (changes, added, _) = steam_plan(&accounts, &apps, &read, record, saved, mode); + for c in changes { + store + .borrow_mut() + .insert(c.config.to_string_lossy().into_owned(), c.after); + } + SteamRecord::Accounts(added) + }; + let on = apply(None, Mode::Off, Mode::On); + assert_eq!(store.borrow()["a"], "MANGOHUD=1 %command%"); + assert_eq!(store.borrow()["b"], "MANGOHUD=1 %command% -novid"); + let off = apply(Some(&on), Mode::On, Mode::Off); + assert_eq!(store.borrow()["a"], "MANGOHUD=1 %command%", "theirs stays"); + // Steam reads arguments alone as following the command. + assert_eq!(store.borrow()["b"], "%command% -novid"); + assert_eq!(off, SteamRecord::Accounts(crate::steam::Inserted::new())); + } + #[test] fn heroic_gets_its_own_switch_or_the_variable_and_off_takes_out_only_that() { use crate::heroic_launch::{Defaults, Wanted, Written, transform}; @@ -981,6 +1359,126 @@ mod tests { assert_eq!(lutris_config(plain, Mode::Off), plain); } + #[test] + fn a_replaced_file_keeps_its_mode_its_symlink_and_leaves_no_temporary() { + use std::os::unix::fs::PermissionsExt as _; + let dir = tempfile::tempdir().unwrap(); + let real = dir.path().join("dotfiles/game.yml"); + std::fs::create_dir_all(real.parent().unwrap()).unwrap(); + std::fs::write(&real, "old\n").unwrap(); + std::fs::set_permissions(&real, std::fs::Permissions::from_mode(0o600)).unwrap(); + let link = dir.path().join("game.yml"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + + replace_file(&link, b"new\n").unwrap(); + assert!( + std::fs::symlink_metadata(&link) + .unwrap() + .file_type() + .is_symlink() + ); + assert_eq!(std::fs::read_to_string(&real).unwrap(), "new\n"); + assert_eq!( + std::fs::metadata(&real).unwrap().permissions().mode() & 0o777, + 0o600 + ); + // A file that was not there gets an ordinary one. + replace_file(&dir.path().join("fresh.yml"), b"x").unwrap(); + assert_eq!( + std::fs::read_to_string(dir.path().join("fresh.yml")).unwrap(), + "x" + ); + + // Writers at once each use a temporary of their own. + std::thread::scope(|s| { + for i in 0..8 { + let real = ℜ + s.spawn(move || replace_file(real, format!("{i}\n").as_bytes()).unwrap()); + } + }); + let last: u32 = std::fs::read_to_string(&real) + .unwrap() + .trim() + .parse() + .unwrap(); + assert!(last < 8); + for d in [dir.path(), real.parent().unwrap()] { + let names: Vec = std::fs::read_dir(d) + .unwrap() + .flatten() + .map(|e| e.file_name().to_string_lossy().into_owned()) + .collect(); + assert!(!names.iter().any(|n| n.starts_with('.')), "{names:?}"); + } + } + + #[test] + fn the_launchers_file_is_kept_once_and_then_replaced() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("lutris/games/game.yml"); + std::fs::create_dir_all(file.parent().unwrap()).unwrap(); + std::fs::write(&file, "theirs\n").unwrap(); + let backups = dir.path().join("backups"); + write_keeping_backup_in(&file, "ours 1\n", &backups).unwrap(); + write_keeping_backup_in(&file, "ours 2\n", &backups).unwrap(); + assert_eq!(std::fs::read_to_string(&file).unwrap(), "ours 2\n"); + let kept: Vec<_> = std::fs::read_dir(&backups).unwrap().flatten().collect(); + assert_eq!(kept.len(), 1); + assert_eq!(std::fs::read_to_string(kept[0].path()).unwrap(), "theirs\n"); + let names: Vec = std::fs::read_dir(file.parent().unwrap()) + .unwrap() + .flatten() + .map(|e| e.file_name().to_string_lossy().into_owned()) + .collect(); + assert_eq!(names, ["game.yml"]); + } + + #[test] + fn lutris_off_puts_back_what_the_game_had() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("game.yml"); + let record = dir.path().join("state/lutris-before.toml"); + let write = |f: &std::path::Path, t: &str| replace_file(f, t.as_bytes()); + let game = "game:\n exe: /g/x\nsystem:\n env:\n LANG: C\n"; + let run = |before: &str, saved, mode| { + std::fs::write(&file, before).unwrap(); + lutris_apply(&file, &record, saved, mode, &write).unwrap(); + std::fs::read_to_string(&file).unwrap() + }; + for theirs in [ + " mangohud: true", + " mangohud: false", + " mangohud: false # slow", + ] { + let mine = format!("{game}{theirs}\n"); + let on = run(&mine, Mode::Off, Mode::On); + assert!(on.contains(&format!("\n{LUTRIS_ON}\n")), "{on}"); + // Forced after On keeps the first record. + std::fs::write(&file, &on).unwrap(); + lutris_apply(&file, &record, Mode::On, Mode::Forced, &write).unwrap(); + assert_eq!(run(&on, Mode::Forced, Mode::Off), mine, "{theirs}"); + assert!( + read_record::>(&record) + .unwrap() + .is_empty() + ); + } + // None before: the line goes, and the section it added with it. + let plain = "game:\n exe: /g/x\n"; + let on = run(plain, Mode::Off, Mode::On); + assert_eq!(run(&on, Mode::On, Mode::Off), plain); + + // Set by the user, nothing from Big Game Mode: Off leaves it. + let theirs = format!("{game} mangohud: true\n"); + assert_eq!(run(&theirs, Mode::Off, Mode::Off), theirs); + // Changed since in Lutris: theirs now. + run(plain, Mode::Off, Mode::On); + let changed = "game:\n exe: /g/x\nsystem:\n mangohud: false\n"; + assert_eq!(run(changed, Mode::On, Mode::Off), changed); + // An older version's `true`, with no record: it goes. + assert_eq!(run(&on, Mode::On, Mode::Off), plain); + } + #[test] fn the_runtime_branch_is_read_from_the_flatpak_metadata() { let meta = "[Application]\nname=com.heroicgameslauncher.hgl\nruntime=org.freedesktop.Platform/x86_64/25.08\nsdk=org.freedesktop.Sdk/x86_64/25.08\n"; diff --git a/bigame-engine/bigame-core/src/optimization.rs b/bigame-engine/bigame-core/src/optimization.rs index 132e5a2..0eab334 100644 --- a/bigame-engine/bigame-core/src/optimization.rs +++ b/bigame-engine/bigame-core/src/optimization.rs @@ -506,16 +506,16 @@ impl GameOptimization { /// game's own values when it has none yet; the table itself goes, so /// there is one place they live. fn take_legacy_gamescope(&mut self) { - if let Some(old) = self.profile.gamescope.take() { - if !self.launch.sets_gamescope() { - let moved = crate::game_launch::GameLaunch::from_legacy(&old); - self.launch = crate::game_launch::GameLaunch { - wine_fsr: self.launch.wine_fsr, - wine_fsr_mode: self.launch.wine_fsr_mode, - vkbasalt: self.launch.vkbasalt, - ..moved - }; - } + if let Some(old) = self.profile.gamescope.take() + && !self.launch.sets_gamescope() + { + let moved = crate::game_launch::GameLaunch::from_legacy(&old); + self.launch = crate::game_launch::GameLaunch { + wine_fsr: self.launch.wine_fsr, + wine_fsr_mode: self.launch.wine_fsr_mode, + vkbasalt: self.launch.vkbasalt, + ..moved + }; } } diff --git a/bigame-engine/bigame-core/src/overview.rs b/bigame-engine/bigame-core/src/overview.rs index 7c73896..ead854f 100644 --- a/bigame-engine/bigame-core/src/overview.rs +++ b/bigame-engine/bigame-core/src/overview.rs @@ -435,10 +435,10 @@ pub fn running_profile(game: &GameIdentity, mode: &str) -> Option Option { static CACHE: std::sync::Mutex)>> = std::sync::Mutex::new(None); let mut cache = CACHE.lock().ok()?; - if let Some((pid, key)) = cache.as_ref() { - if *pid == game.pid { - return key.clone(); - } + if let Some((pid, key)) = cache.as_ref() + && *pid == game.pid + { + return key.clone(); } // The running game's own file, through its Wine prefix, before its name: // two library games can share a name such as `Game.exe`. diff --git a/bigame-engine/bigame-core/src/processes.rs b/bigame-engine/bigame-core/src/processes.rs index 6ca7c53..87be03e 100644 --- a/bigame-engine/bigame-core/src/processes.rs +++ b/bigame-engine/bigame-core/src/processes.rs @@ -1032,10 +1032,10 @@ fn with_paused(f: impl FnOnce(&mut Vec) -> R) -> R { let list = guard.get_or_insert_with(|| load_paused(&path)); let before = list.clone(); let result = f(list); - if *list != before { - if let Err(e) = save_paused(&path, list) { - tracing::warn!(target: "processes", error = %format!("{e:#}"), "could not save the paused list"); - } + if *list != before + && let Err(e) = save_paused(&path, list) + { + tracing::warn!(target: "processes", error = %format!("{e:#}"), "could not save the paused list"); } result } diff --git a/bigame-engine/bigame-core/src/profiles.rs b/bigame-engine/bigame-core/src/profiles.rs index 644b8f0..eecebc4 100644 --- a/bigame-engine/bigame-core/src/profiles.rs +++ b/bigame-engine/bigame-core/src/profiles.rs @@ -182,15 +182,15 @@ pub fn validate(profile: &GameProfile) -> Vec<&'static str> { } } // Script paths: check they look like absolute paths - if let Some(ref s) = profile.start_script { - if !s.starts_with('/') { - warnings.push(N_("Start script should be an absolute path")); - } + if let Some(ref s) = profile.start_script + && !s.starts_with('/') + { + warnings.push(N_("Start script should be an absolute path")); } - if let Some(ref s) = profile.stop_script { - if !s.starts_with('/') { - warnings.push(N_("Stop script should be an absolute path")); - } + if let Some(ref s) = profile.stop_script + && !s.starts_with('/') + { + warnings.push(N_("Stop script should be an absolute path")); } warnings } @@ -210,12 +210,11 @@ pub fn critical_errors(profile: &GameProfile) -> Vec<&'static str> { if profile.name.contains(std::path::MAIN_SEPARATOR) || profile.name.contains("..") { errors.push(N_("Profile name contains invalid path characters")); } - if let Some(ref gs) = profile.gamescope { - if (gs.render_width == 0) != (gs.render_height == 0) - || (gs.output_width == 0) != (gs.output_height == 0) - { - errors.push(N_("Gamescope resolution needs both width and height")); - } + if let Some(ref gs) = profile.gamescope + && ((gs.render_width == 0) != (gs.render_height == 0) + || (gs.output_width == 0) != (gs.output_height == 0)) + { + errors.push(N_("Gamescope resolution needs both width and height")); } errors } @@ -399,15 +398,15 @@ fn serialize_profile_otter_conf(profile: &GameProfile) -> String { let _ = writeln!(out, "vcache_mode = {}", profile.vcache_mode); let _ = writeln!(out, "idle_inhibit = {}", profile.idle_inhibit); // Strings: quoted - if let Some(ref s) = profile.start_script { - if !s.is_empty() { - let _ = writeln!(out, "start_script = \"{s}\""); - } + if let Some(ref s) = profile.start_script + && !s.is_empty() + { + let _ = writeln!(out, "start_script = \"{s}\""); } - if let Some(ref s) = profile.stop_script { - if !s.is_empty() { - let _ = writeln!(out, "stop_script = \"{s}\""); - } + if let Some(ref s) = profile.stop_script + && !s.is_empty() + { + let _ = writeln!(out, "stop_script = \"{s}\""); } // Big Game Mode's own per-game settings (otter_conf skips unknown keys). // `cpu_governor`, `scx_custom_flags` and `enabled` are not written: nothing @@ -490,18 +489,101 @@ pub fn save_file(profile: &GameProfile) -> Result<()> { Ok(()) } -/// Delete a user profile by name via D-Bus. +/// What deleting a profile did in the game's launchers. `None` (or empty) +/// for a part that had nothing to take out. +#[derive(Debug, Default)] +pub struct Deleted { + /// The game's own launch settings could not be cleared, so a later + /// change in Tuning would write them back into its launchers. + pub settings: Option, + /// `MangoHud`, wherever the game's launcher keeps it. + pub mangohud: Option>, + /// Gamescope and the game's variables in its Steam launch options. + pub steam_launch: Option>, + /// Proton's FSR 4 upgrade in its Steam launch options, one per app. + pub fsr4: Vec>, + /// Everything Big Game Mode wrote into its settings in Heroic. + pub heroic: Option>, +} + +impl Deleted { + /// Steam was running, and kept the game's launch options as they were: + /// they still hold what Big Game Mode wrote. + #[must_use] + pub fn steam_running(&self) -> bool { + use crate::graphics::fsr4_upgrade::Applied as Fsr4; + matches!( + self.mangohud, + Some(Ok(crate::mangohud::Applied::SteamRunning)) + ) || matches!( + self.steam_launch, + Some(Ok(crate::steam_gamescope::Applied::SteamRunning)) + ) || self + .fsr4 + .iter() + .any(|r| matches!(r, Ok(Fsr4::SteamRunning))) + } + + /// The Heroic that was open, and kept the game's settings there as + /// they were. + #[must_use] + pub fn heroic_running(&self) -> Option { + match &self.heroic { + Some(Ok(crate::heroic_launch::Applied::HeroicRunning { launcher, .. })) => { + Some(*launcher) + } + _ => None, + } + } + + /// What failed outright. + #[must_use] + pub fn errors(&self) -> Vec<&anyhow::Error> { + self.settings + .iter() + .chain(self.mangohud.iter().filter_map(|r| r.as_ref().err())) + .chain(self.steam_launch.iter().filter_map(|r| r.as_ref().err())) + .chain(self.fsr4.iter().filter_map(|r| r.as_ref().err())) + .chain(self.heroic.iter().filter_map(|r| r.as_ref().err())) + .collect() + } + + /// Whether everything Big Game Mode wrote for the game is out. + #[must_use] + pub fn complete(&self) -> bool { + !self.steam_running() + && self.heroic_running().is_none() + && !matches!( + self.mangohud, + Some(Ok(crate::mangohud::Applied::LauncherRunning(_))) + ) + && self.errors().is_empty() + } +} + +/// Delete a user profile by name via D-Bus, with what Big Game Mode wrote +/// for the game into its launchers, and say what was left there: with Steam +/// or Heroic open, it stays until the game's launch settings are written +/// again with that launcher closed. /// /// Synchronous for the same reason as [`save`]. /// /// # Errors -/// Returns an error if the profile does not exist or the D-Bus call fails. -pub fn delete(name: &str) -> Result<()> { +/// Returns an error if the profile does not exist or the D-Bus call fails; +/// then nothing else is touched. +pub fn delete(name: &str) -> Result { let path = user_path(name); anyhow::ensure!( path.exists(), UserError::with(N_("profile not found: %s"), [path.display().to_string()]) ); + // The game's settings and its launchers know it by the process the + // profile is for, which the file's name need not be. + let process = load(name) + .ok() + .map(|p| p.name) + .filter(|n| !n.is_empty()) + .unwrap_or_else(|| name.to_owned()); let proxy = crate::dbus_client::daemon_proxy_blocking()?; // The helper reloads falcond itself, through systemd. @@ -513,7 +595,55 @@ pub fn delete(name: &str) -> Result<()> { tracing::warn!(profile = %name, error = %format!("{e:#}"), "the profile is deleted, but its lsfg-vk entry is still there"); } - Ok(()) + Ok(remove_from_launchers(&process)) +} + +/// Take out of the game's launchers what Big Game Mode wrote there for it, +/// through the paths that write it, each asked for nothing. Only what Big +/// Game Mode recorded writing comes out; the user's own options stay. +fn remove_from_launchers(process: &str) -> Deleted { + let mut deleted = Deleted::default(); + let mut mangohud = crate::mangohud::Mode::Off; + let mut fsr4 = false; + match crate::game_settings::load(process) { + Ok(mut settings) => { + mangohud = settings.mangohud; + fsr4 = settings + .steam_fsr4_upgrade + .as_ref() + .is_some_and(|added| !added.is_empty()); + // Cleared first: Heroic's path writes what is saved for the + // game, and Tuning's refresh would put them back into Steam's + // and Heroic's settings for a game with no profile. + if !settings.launch.is_empty() || settings.heroic_fsr4_upgrade { + settings.launch = crate::game_launch::GameLaunch::default(); + settings.heroic_fsr4_upgrade = false; + if let Err(e) = crate::game_settings::save(process, &settings) { + deleted.settings = Some(e); + } + } + } + Err(e) => deleted.settings = Some(e), + } + if mangohud != crate::mangohud::Mode::Off { + deleted.mangohud = Some(crate::mangohud::apply(process, crate::mangohud::Mode::Off)); + } + deleted.steam_launch = Some(crate::steam_gamescope::apply( + process, + crate::steam_gamescope::Wanted::default(), + )); + // Only where it recorded adding it: with no record, a variable in the + // options may be the user's own. + if fsr4 { + deleted.fsr4 = crate::steam_gamescope::steam_apps(process) + .iter() + .map(|app| crate::graphics::fsr4_upgrade::apply(process, Some(app), false)) + .collect(); + } + deleted.heroic = Some(crate::heroic_launch::apply(process, |_| { + crate::heroic_launch::Wanted::default() + })); + deleted } /// Resolve profile path: user dir first, then system. @@ -706,6 +836,48 @@ some_future_falcond_key = 42 dir } + #[test] + fn a_delete_says_what_its_launchers_kept() { + use crate::launchers::Launcher; + let nothing = Deleted::default(); + assert!(nothing.complete()); + + let steam = Deleted { + steam_launch: Some(Ok(crate::steam_gamescope::Applied::Unchanged)), + fsr4: vec![Ok(crate::graphics::fsr4_upgrade::Applied::SteamRunning)], + ..Deleted::default() + }; + assert!(steam.steam_running() && !steam.complete()); + assert_eq!(steam.heroic_running(), None); + + let heroic = Deleted { + heroic: Some(Ok(crate::heroic_launch::Applied::HeroicRunning { + launcher: Launcher::Heroic { flatpak: true }, + game_running: false, + })), + ..Deleted::default() + }; + assert_eq!( + heroic.heroic_running(), + Some(Launcher::Heroic { flatpak: true }) + ); + assert!(!heroic.steam_running() && !heroic.complete()); + + let failed = Deleted { + mangohud: Some(Err(anyhow::anyhow!("read-only"))), + heroic: Some(Ok(crate::heroic_launch::Applied::Written)), + ..Deleted::default() + }; + assert_eq!(failed.errors().len(), 1); + assert!(!failed.complete()); + } + + #[test] + fn deleting_a_profile_that_is_not_there_touches_nothing() { + let err = delete("bigame-test-no-such-profile-x").unwrap_err(); + assert!(err.to_string().contains("profile not found"), "{err}"); + } + #[test] fn a_profile_matches_by_its_name_as_falcond_does() { let r = ProfileRef { diff --git a/bigame-engine/bigame-core/src/running.rs b/bigame-engine/bigame-core/src/running.rs index 02940dd..a17db75 100644 --- a/bigame-engine/bigame-core/src/running.rs +++ b/bigame-engine/bigame-core/src/running.rs @@ -50,7 +50,6 @@ pub struct Proc { /// One directory walk, the owner from the directory itself, two small reads /// per process of the user's, no forks. #[must_use] -#[allow(clippy::similar_names)] // pid and ppid are what /proc calls them pub fn snapshot() -> Vec { snapshot_in(Path::new("/proc")) } @@ -83,11 +82,7 @@ fn snapshot_in(root: &Path) -> Vec { return None; } let raw = std::fs::read(dir.join("cmdline")).ok()?; - let argv0 = raw - .split(|b| *b == 0) - .next() - .map(|a| String::from_utf8_lossy(a).into_owned()) - .unwrap_or_default(); + let argv0 = argv0_of(&raw); // No command line: a kernel thread, or a process on its way // out. Neither is a game, and an exiting one in a game's tree // was once reported as the game, with no name. @@ -112,6 +107,36 @@ fn snapshot_in(root: &Path) -> Vec { .collect() } +/// `argv[0]` of a raw `/proc//cmdline`. +/// +/// Chromium, Electron and CEF helpers rewrite their command line as one +/// space-joined string (`/opt/Heroic/heroic --type=gpu-process …`), so the +/// first NUL-separated field is the whole line and its basename is no +/// program's name. For such a line `argv[0]` ends where the switches begin. +fn argv0_of(raw: &[u8]) -> String { + let first = raw.split(|b| *b == 0).next().unwrap_or_default(); + let joined = !raw + .iter() + .rposition(|b| *b != 0) + .is_some_and(|end| raw[..end].contains(&0)); + let first = String::from_utf8_lossy(first); + if joined + && first.contains(" --type=") + && let Some(cut) = first.find(" --") + { + return first[..cut].to_owned(); + } + first.into_owned() +} + +/// Whether a process is a helper of a Chromium-based program (its GPU +/// process, a renderer, a utility): never the game. A Heroic or Electron +/// game's `--type=gpu-process` submits GPU work, and would be chosen over +/// the program itself. +fn is_chromium_helper(p: &Proc) -> bool { + p.cmdline.contains(" --type=") +} + /// `(ppid, state, utime + stime)` from `/proc//stat`. fn parse_stat(stat: &str) -> Option<(u32, char, u64)> { // Fields after the parenthesised comm, which may contain ") ". @@ -229,6 +254,16 @@ const INFRASTRUCTURE: &[&str] = &[ // the game starts; Debian-based runtimes name the binary ldconfig.real. "ldconfig", "ldconfig.real", + // Short-lived programs the container, Proton and launch scripts run + // before the game: generating locales, parsing options, waiting, asking + // in a dialog. + "localedef", + "getopt", + "env", + "sleep", + "timeout", + "zenity", + "yad", // Wine infrastructure "wineserver", "wine", @@ -251,6 +286,11 @@ const INFRASTRUCTURE: &[&str] = &[ "xalia.exe", "iexplore.exe", "d3ddriverquery64.exe", + // Installing a game's redistributables and compiling its .NET + // assemblies in the prefix, on a first launch. + "msiexec.exe", + "mscorsvw.exe", + "ngen.exe", // Steam's installer-script runner: it runs inside the game's Proton tree // on a first launch, before the game itself, and must not be offered a // profile. @@ -417,9 +457,13 @@ pub fn is_infrastructure(name: &str) -> bool { || STEAM_RUNTIME_PREFIXES.iter().any(|p| lower.starts_with(p)) // Crash handlers by their usual names -- not any name containing // "crash", which would also exclude Crash Bandicoot. - || ["crashhandler", "crash_handler", "crashreport", "crashpad", "crashsender"] + // BugSplat's reporter is BsSndRpt.exe. + || ["crashhandler", "crash_handler", "crashreport", "crashpad", "crashsender", "bssndrpt", "bugsplat"] .iter() .any(|n| lower.contains(n)) + // python3.12: the interpreter Proton's script runs under, by its + // versioned name. + || crate::games::is_versioned(&lower, "python") || lower.contains("launcher") // REDupdater.exe, EA's EADesktopUpdater…: they update, they do not play. || lower.contains("updater") @@ -432,14 +476,15 @@ pub fn is_infrastructure(name: &str) -> bool { /// The processes of a Steam tree the game can be, out of `candidates`. fn game_pool<'a>(candidates: Vec<&'a &'a Proc>, proton: bool) -> Vec<&'a &'a Proc> { // In a Proton tree the game is a Windows binary. A Linux helper inside - // the container -- an overlay, a wrapper -- must not outrank a game - // that is still loading and has burned little CPU yet. + // the container -- an overlay, a wrapper, a program the launch scripts + // run for a moment -- must not outrank a game that is still loading and + // has burned little CPU yet, nor be taken for it before it starts. let windows: Vec<&&Proc> = candidates .iter() .copied() .filter(|p| p.argv0.to_ascii_lowercase().ends_with(".exe")) .collect(); - if proton && !windows.is_empty() { + if proton { // Steam runs its games from the library. A launcher the game // installed in the prefix (CD Projekt's REDlauncher, in // C:\users\…\AppData) runs beside it, and its web page @@ -520,18 +565,8 @@ pub fn identify(procs: &[Proc]) -> Vec { identify_with(procs, &HashMap::new()) } -/// Find the running games in a process list. -/// -/// Steam games are found from their reaper, which names the app id; within -/// that tree the game is the busiest process that is not machinery — a -/// launcher can briefly be the only candidate, and it is excluded by name. -/// Wine games outside Steam are found as busy `.exe` processes under Wine. -/// -/// `native` maps the executable names of games this machine knows about -/// ([`known_native_games`]) to their display names. Without it a native -/// game started from the application menu (`SuperTuxKart` from the -/// repositories, say) is never taken for a game: Home keeps saying *waiting -/// for games* and no profile is offered. +/// [`identify_ranked`] with nothing known of the processes' activity. +#[cfg(test)] #[must_use] pub fn identify_with( procs: &[Proc], @@ -570,21 +605,33 @@ impl Activity { /// The process being played among `pool`: the one drawing, then the /// busiest of late. The GPU is asked only when there is a choice. fn choose<'a>(&self, pool: Vec<&'a Proc>) -> Option<&'a Proc> { - if pool.len() > 1 { - if let Some(renders) = &self.renders { - return pool - .into_iter() - .map(|p| (renders(p.pid), self.recent(p), p)) - .max_by_key(|(drawing, recent, p)| (*drawing, *recent, p.cpu_ticks)) - .map(|(_, _, p)| p); - } + if pool.len() > 1 + && let Some(renders) = &self.renders + { + return pool + .into_iter() + .map(|p| (renders(p.pid), self.recent(p), p)) + .max_by_key(|(drawing, recent, p)| (*drawing, *recent, p.cpu_ticks)) + .map(|(_, _, p)| p); } pool.into_iter() .max_by_key(|p| (self.recent(p), p.cpu_ticks)) } } -/// [`identify_with`], choosing within a tree by [`Activity`]. +/// Find the running games in a process list. +/// +/// Steam games are found from their reaper, which names the app id; within +/// that tree the game is the process that is not machinery chosen by +/// [`Activity`] — a launcher can briefly be the only candidate, and it is +/// excluded by name. Wine games outside Steam are found as busy `.exe` +/// processes under Wine. +/// +/// `native` maps the executable names of games this machine knows about +/// ([`known_native_games`]) to their display names. Without it a native +/// game started from the application menu (`SuperTuxKart` from the +/// repositories, say) is never taken for a game: Home keeps saying *waiting +/// for games* and no profile is offered. #[must_use] pub fn identify_ranked( procs: &[Proc], @@ -604,7 +651,11 @@ pub fn identify_ranked( let proton = proton_tool(&tree); let candidates: Vec<&&Proc> = tree .iter() - .filter(|p| !p.argv0.is_empty() && !is_infrastructure(falcond_name(&p.argv0))) + .filter(|p| { + !p.argv0.is_empty() + && !is_infrastructure(falcond_name(&p.argv0)) + && !is_chromium_helper(p) + }) .collect(); let pool = game_pool(candidates, proton.is_some()); let Some(game) = activity.choose(pool.into_iter().copied().collect()) else { @@ -695,31 +746,54 @@ pub fn identify_ranked( // ── Native games the machine knows about ───────────────────────────────────── /// Executable name → display name for every native game this machine lists: -/// the menu entries in the `Game` category ([`crate::games::menu_games`]), -/// and the process names falcond has a profile for — a profile is falcond's -/// own statement that a process is a game. +/// the games of the library ([`crate::games::detect_all`]) that run here +/// directly, menu entries in the `Game` category among them, and the +/// process names falcond has a profile for — a profile is falcond's own +/// statement that a process is a game. /// -/// Read at most once a minute: detection runs every few seconds, and a game -/// installed meanwhile is picked up on the next read. +/// Looked at most once a minute: detection runs every few seconds, and a +/// game installed meanwhile is picked up on the next look. The library's +/// part is worked out again only when the library was scanned again, which +/// happens only when what it is read from changed +/// ([`crate::games::detect_all_changed`]): a look at an unchanged library +/// costs the stat of its files, not a walk of every install folder. #[must_use] pub fn known_native_games() -> HashMap { use std::sync::Mutex; use std::time::{Duration, Instant}; - static CACHE: Mutex)>> = Mutex::new(None); + #[derive(Default)] + struct Known { + at: Option, + /// The library scan `library` was worked out from. + scan: Option, + library: HashMap, + games: HashMap, + } + static CACHE: Mutex> = Mutex::new(None); let mut cache = CACHE .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - if let Some((at, games)) = cache.as_ref() { - if at.elapsed() < Duration::from_secs(60) { - return games.clone(); - } + let known = cache.get_or_insert_with(Known::default); + if known + .at + .is_some_and(|at| at.elapsed() < Duration::from_secs(60)) + { + return known.games.clone(); } - let games = read_native_games(); - *cache = Some((Instant::now(), games.clone())); + if let Some((scan, library)) = crate::games::detect_all_changed(known.scan) { + known.scan = Some(scan); + known.library = native_games_from(&[], &library); + } + // The library's word over a profile's, as in [`native_games_from`]. + let mut games = native_games_from(&profile_names(), &[]); + games.extend(known.library.iter().map(|(k, v)| (k.clone(), v.clone()))); + known.games.clone_from(&games); + known.at = Some(Instant::now()); games } -fn read_native_games() -> HashMap { +/// The `name` of every falcond profile, the system's and the user's. +fn profile_names() -> Vec { let mut profiles = Vec::new(); let base = Path::new(crate::profiles::SYSTEM_PROFILES_DIR); for dir in [base.to_path_buf(), base.join("user")] { @@ -730,7 +804,7 @@ fn read_native_games() -> HashMap { profiles.extend(profile_name_field(&content)); } } - native_games_from(&profiles, &crate::games::detect_all()) + profiles } /// Names too common among native programs to say which game runs: a @@ -822,37 +896,68 @@ pub fn graphics_from_maps(maps: &str) -> Graphics { } } -/// How long a game's graphics path, once told, is taken as known: its -/// memory map is a large read, and the watcher asks every few seconds. A -/// game that loads its renderer late is read again after this. +/// How long what was learned of a game's process — its graphics path, the +/// card it renders on — is taken as known: its memory map and its +/// descriptors are large reads, the card may take a question to NVML, and +/// the watcher asks every few seconds for as long as the game runs. A game +/// that loads its renderer late is read again after this. const GRAPHICS_KEPT: std::time::Duration = std::time::Duration::from_secs(30); +/// One fact learned of the process being watched, kept for a while: the +/// watcher follows one game, so one process is kept. +struct Kept(Option<(u32, String, std::time::Instant, T)>); + +impl Kept { + const fn new() -> Self { + Self(None) + } + + /// What `read` says of process `pid` running `executable`, read again + /// once `max_age` has passed. A `None` is not kept: what cannot be told + /// yet is asked again the next time. + fn get( + &mut self, + pid: u32, + executable: &str, + max_age: std::time::Duration, + read: impl FnOnce() -> Option, + ) -> Option { + let now = std::time::Instant::now(); + if let Some((p, e, at, value)) = &self.0 + && *p == pid + && e == executable + && now.saturating_duration_since(*at) < max_age + { + return Some(value.clone()); + } + let value = read(); + self.0 = value.clone().map(|v| (pid, executable.to_owned(), now, v)); + value + } +} + /// [`graphics_from_maps`] of process `pid`, read again only after /// [`GRAPHICS_KEPT`] once it is known. fn graphics_of(pid: u32, executable: &str) -> Graphics { - type Seen = HashMap<(u32, String), (std::time::Instant, Graphics)>; - static SEEN: std::sync::Mutex> = std::sync::Mutex::new(None); - let key = (pid, executable.to_owned()); - let now = std::time::Instant::now(); - let mut seen = SEEN - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let seen = seen.get_or_insert_with(HashMap::new); - if let Some((at, graphics)) = seen.get(&key) { - if now.saturating_duration_since(*at) < GRAPHICS_KEPT { - return *graphics; - } - } - let graphics = std::fs::read_to_string(format!("/proc/{pid}/maps")) - .map_or(Graphics::Unknown, |maps| graphics_from_maps(&maps)); - // Only the game being watched is kept. - seen.retain(|(p, _), _| *p == pid); - if graphics == Graphics::Unknown { - seen.remove(&key); - } else { - seen.insert(key, (now, graphics)); - } - graphics + static KEPT: std::sync::Mutex> = std::sync::Mutex::new(Kept::new()); + KEPT.lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(pid, executable, GRAPHICS_KEPT, || { + std::fs::read_to_string(format!("/proc/{pid}/maps")) + .map(|maps| graphics_from_maps(&maps)) + .ok() + .filter(|g| *g != Graphics::Unknown) + }) + .unwrap_or(Graphics::Unknown) +} + +/// [`render_card`] of process `pid`, read again only after +/// [`GRAPHICS_KEPT`] once it is known. +fn render_card_of(pid: u32, executable: &str) -> Option { + static KEPT: std::sync::Mutex> = std::sync::Mutex::new(Kept::new()); + KEPT.lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(pid, executable, GRAPHICS_KEPT, || render_card(pid)) } /// A GPU a process has open. @@ -1105,32 +1210,62 @@ fn drop_enumerated_only( .collect() } +/// What Steam's manifest says of a game: its title, where it is installed, +/// its Proton prefix. +#[derive(Clone, Default)] +struct SteamDetails { + name: Option, + install_path: Option, + compatdata_path: Option, +} + +/// [`SteamDetails`] of app `id`, from the manifest in the library that has +/// it. +fn steam_details(id: &str, home: &Path) -> SteamDetails { + for library in crate::games::steam_libraries(home) { + let steamapps = library.join("steamapps"); + let Ok(text) = std::fs::read_to_string(steamapps.join(format!("appmanifest_{id}.acf"))) + else { + continue; + }; + // The prefix beside the manifest, not the first compatdata/ + // found: Steam leaves stale ones behind when a game moves. + let prefix = steamapps.join("compatdata").join(id); + return SteamDetails { + name: crate::games::acf_value(&text, "name"), + install_path: crate::games::acf_value(&text, "installdir") + .map(|dir| steamapps.join("common").join(dir)), + compatdata_path: prefix.is_dir().then_some(prefix), + }; + } + SteamDetails::default() +} + /// Fill in what the launcher and the live process can say. +/// +/// What was learned of the process is kept ([`Kept`]): the watcher asks +/// again every few seconds for as long as the game runs, and the manifest +/// of a running game does not change; its graphics and card are read again +/// now and then, until they are known and then after [`GRAPHICS_KEPT`]. fn enrich(mut game: GameIdentity) -> GameIdentity { + static STEAM: std::sync::Mutex> = std::sync::Mutex::new(Kept::new()); game.graphics = graphics_of(game.pid, &game.executable); - game.render_card = render_card(game.pid); - if let Some(id) = game.steam_app_id.clone() { - if let Some(home) = std::env::var_os("HOME").map(PathBuf::from) { - for library in crate::games::steam_libraries(&home) { - let steamapps = library.join("steamapps"); - let Ok(text) = - std::fs::read_to_string(steamapps.join(format!("appmanifest_{id}.acf"))) - else { - continue; - }; - if let Some(name) = crate::games::acf_value(&text, "name") { - game.display_name = name; - } - if let Some(dir) = crate::games::acf_value(&text, "installdir") { - game.install_path = Some(steamapps.join("common").join(dir)); - } - // The prefix beside the manifest, not the first compatdata/ - // found: Steam leaves stale ones behind when a game moves. - let prefix = steamapps.join("compatdata").join(&id); - game.compatdata_path = prefix.is_dir().then_some(prefix); - break; - } + game.render_card = render_card_of(game.pid, &game.executable); + if let Some(id) = game.steam_app_id.clone() + && let Some(home) = std::env::var_os("HOME").map(PathBuf::from) + { + let details = STEAM + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(game.pid, &game.executable, std::time::Duration::MAX, || { + Some(steam_details(&id, &home)) + }) + .unwrap_or_default(); + if let Some(name) = details.name { + game.display_name = name; } + game.install_path = details.install_path; + game.compatdata_path = details.compatdata_path; } game } @@ -1471,11 +1606,7 @@ pub fn running_for(pid: u32) -> Option { if hz <= 0 { return None; } - #[allow( - clippy::cast_precision_loss, - clippy::cast_possible_truncation, - clippy::cast_sign_loss - )] + #[allow(clippy::cast_precision_loss)] let started = start_ticks as f64 / hz as f64; #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] Some((uptime - started).max(0.0) as u64) @@ -1548,10 +1679,10 @@ fn matching_profile_in( let Ok(content) = std::fs::read_to_string(&path) else { continue; }; - if let Some(name) = profile_name_field(&content) { - if !name.eq_ignore_ascii_case("proton") { - candidates.push(ProfileMatch { name, path, user }); - } + if let Some(name) = profile_name_field(&content) + && !name.eq_ignore_ascii_case("proton") + { + candidates.push(ProfileMatch { name, path, user }); } } } @@ -2046,6 +2177,110 @@ mod tests { assert!(identify(&tree).is_empty(), "{:?}", identify(&tree)); } + #[test] + fn what_runs_before_a_proton_game_is_not_the_game() { + // A Proton tree before the game starts: the container generating + // locales, a launch script parsing options and waiting, a dialog, + // Proton's interpreter by its versioned name, a Linux helper of + // nobody's list. None of them is the Windows game. + let mut tree = vec![ + reaper(1, "1245620"), + p( + 2, + 1, + "python3|/s/steamapps/common/Proton - Experimental/proton waitforexitandrun x", + 5, + ), + p(3, 2, "/usr/bin/localedef|--add-to-archive", 300), + p(4, 2, "/usr/bin/getopt|-o h", 2), + p(5, 2, "/usr/bin/env|FOO=1 sleep 1", 1), + p(6, 2, "/usr/bin/sleep|1", 1), + p(7, 2, "/usr/bin/timeout|5 true", 1), + p(8, 2, "/usr/bin/zenity|--info", 40), + p(9, 2, "/usr/bin/yad|--text=x", 40), + p(10, 2, "/usr/bin/python3.12|/s/script.py", 90), + p(11, 2, "/s/steamapps/common/G/tools/helper|", 700), + p( + 12, + 2, + "C:\\windows\\system32\\msiexec.exe|/i vcredist.msi", + 900, + ), + p( + 13, + 2, + "C:\\windows\\Microsoft.NET\\Framework64\\v4.0.30319\\mscorsvw.exe|", + 900, + ), + p( + 14, + 2, + "C:\\windows\\Microsoft.NET\\Framework64\\v4.0.30319\\ngen.exe|update", + 900, + ), + p(15, 2, "S:\\steamapps\\common\\G\\BsSndRpt64.exe|", 900), + p(16, 2, "S:\\steamapps\\common\\G\\BugSplatHD64.exe|", 900), + ]; + assert!(identify(&tree).is_empty(), "{:?}", identify(&tree)); + // The game starts: it is the game, however busy the rest are. + tree.push(p(17, 2, "S:\\steamapps\\common\\G\\ELDENRING.exe|", 50)); + let games = identify(&tree); + assert_eq!(games.len(), 1, "{games:?}"); + assert_eq!(games[0].pid, 17); + assert!(!is_infrastructure("Bugsnax.exe")); + assert!(!is_infrastructure("python-game")); + } + + #[test] + fn a_chromium_helpers_joined_command_line_names_its_program() { + // Electron's GPU process, as /proc shows it: one string, no NUL + // between the arguments. + assert_eq!( + argv0_of( + b"/opt/Heroic/heroic --type=gpu-process --user-data-dir=/home/u/.config/heroic\0" + ), + "/opt/Heroic/heroic" + ); + // Arguments NUL-separated as usual are left alone, switches or not. + assert_eq!( + argv0_of(b"/g/My --type=Game/run\0--type=renderer\0"), + "/g/My --type=Game/run" + ); + assert_eq!( + argv0_of(b"C:\\Program Files\\G\\Game.exe\0-dx12\0"), + "C:\\Program Files\\G\\Game.exe" + ); + assert_eq!(argv0_of(b"/g/Game --fullscreen\0"), "/g/Game --fullscreen"); + assert_eq!(argv0_of(b""), ""); + } + + #[test] + fn an_electron_games_helpers_are_not_the_game() { + // An Electron game on Steam: its GPU process submits the GPU work, + // its renderer burns the CPU; the game is the program itself. + let game = "/g/steamapps/common/G/G"; + let tree = vec![ + reaper(1, "4242"), + p(2, 1, &format!("{game}|--no-sandbox"), 300), + p( + 3, + 2, + &format!("{game}|--type=gpu-process --field-trial-handle=1"), + 900, + ), + p(4, 2, &format!("{game}|--type=renderer --lang=en"), 5_000), + p(5, 2, &format!("{game}|--type=utility"), 50), + ]; + let drawing = Activity { + previous: HashMap::new(), + renders: Some(Box::new(|pid| pid == 3)), + }; + let found = identify_ranked(&tree, &HashMap::::new(), &drawing); + assert_eq!(found.len(), 1, "{found:?}"); + assert_eq!(found[0].pid, 2); + assert_eq!(found[0].process_name, "G"); + } + #[test] fn a_process_without_a_command_line_is_not_the_game() { // A process on its way out has an empty cmdline. Left in the pool it @@ -2301,6 +2536,30 @@ mod tests { ); } + #[test] + fn what_is_learned_of_a_process_is_kept_while_it_is_known() { + let reads = std::cell::Cell::new(0); + let read = |value: Option| { + reads.set(reads.get() + 1); + value + }; + let long = std::time::Duration::from_secs(60); + let mut kept = Kept::new(); + // Not known yet: asked again. + assert_eq!(kept.get(7, "G.exe", long, || read(None)), None); + assert_eq!(kept.get(7, "G.exe", long, || read(Some(1))), Some(1)); + assert_eq!(kept.get(7, "G.exe", long, || read(Some(2))), Some(1)); + assert_eq!(reads.get(), 2); + // Another process, or the same pid running something else, is + // read anew. + assert_eq!(kept.get(8, "G.exe", long, || read(Some(3))), Some(3)); + assert_eq!(kept.get(8, "H.exe", long, || read(Some(4))), Some(4)); + // And once its time is up. + let none = std::time::Duration::ZERO; + assert_eq!(kept.get(8, "H.exe", none, || read(Some(5))), Some(5)); + assert_eq!(reads.get(), 5); + } + #[test] fn this_process_has_been_running_a_short_while() { let secs = running_for(std::process::id()).expect("readable"); @@ -2560,6 +2819,24 @@ mod tests { assert_eq!(procs[0].ppid, 4000); assert_eq!(procs[0].cpu_ticks, 1570); assert_eq!(procs[0].cmdline, "C:\\g\\Game.exe -dx12"); + // A Chromium helper that rewrote its command line as one string. + let helper = root.join("4243"); + std::fs::create_dir_all(&helper).unwrap(); + std::fs::write( + helper.join("stat"), + "4243 (heroic) S 4000 4243 0 0 -1 0 0 0 0 0 10 5 0 0 20 0 60 0 99", + ) + .unwrap(); + std::fs::write( + helper.join("cmdline"), + b"/opt/Heroic/heroic --type=gpu-process --user-data-dir=/h/.config/heroic\0", + ) + .unwrap(); + let procs = snapshot_in(&root); + let helper = procs.iter().find(|p| p.pid == 4243).unwrap(); + assert_eq!(helper.argv0, "/opt/Heroic/heroic"); + assert_eq!(falcond_name(&helper.argv0), "heroic"); + assert!(is_chromium_helper(helper)); let _ = std::fs::remove_dir_all(&root); } @@ -2616,6 +2893,13 @@ mod tests { ] ); assert_eq!(known["supertuxkart"], "SuperTuxKart"); + // Built in two parts, as `known_native_games` keeps them, it is the + // same map: the library's word over a profile's. + let profiles = ["cs2".to_owned(), "supertuxkart".to_owned()]; + let mut parts = native_games_from(&profiles, &[]); + parts.extend(native_games_from(&[], &library)); + assert_eq!(parts, native_games_from(&profiles, &library)); + assert_eq!(parts["supertuxkart"], "SuperTuxKart"); } #[test] diff --git a/bigame-engine/bigame-core/src/steam.rs b/bigame-engine/bigame-core/src/steam.rs index fccc8c9..c739355 100644 --- a/bigame-engine/bigame-core/src/steam.rs +++ b/bigame-engine/bigame-core/src/steam.rs @@ -118,8 +118,11 @@ pub fn users(home: &Path) -> Vec { } } } - out.sort_by(|a, b| a.id.cmp(&b.id)); - out.dedup_by(|a, b| a.id == b.id); + // One account signed in to both the native Steam and the Flatpak has a + // `userdata/` in each, and each client reads only its own: they are + // two configurations to write, told apart by their file, not by the id. + out.sort_by(|a, b| a.id.cmp(&b.id).then_with(|| a.config.cmp(&b.config))); + out.dedup_by(|a, b| a.config == b.config); out } @@ -417,6 +420,12 @@ pub fn set_launch_options(config: &Path, app_id: &str, value: &str) -> Result<() write_launch_options(config, app_id, value) } +/// One edit of a `localconfig.vdf` at a time in this process: two saves at +/// once (a profile and AI Graphics) would each read the file, change their +/// own game and write it back, and the second would put back the first +/// one's old options. +static EDIT: std::sync::Mutex<()> = std::sync::Mutex::new(()); + /// Write the launch options without checking whether Steam is running. /// /// Split out from [`set_launch_options`] so the file-editing logic can be @@ -430,6 +439,10 @@ fn write_launch_options(config: &Path, app_id: &str, value: &str) -> Result<()> !app_id.is_empty() && app_id.bytes().all(|b| b.is_ascii_digit()), UserError::with(N_("not a Steam app id: %s"), [app_id]) ); + // Held through the read-back, so what is compared is this write. + let _edit = EDIT + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); // Written in Steam's escapes: a bare quote or newline would corrupt the // file for every game, not just this one. let stored = escape(value); @@ -468,7 +481,9 @@ fn write_launch_options(config: &Path, app_id: &str, value: &str) -> Result<()> } return finish_write(config, &content, &lines, app_id, value); }; - let app_depth = depth(borrowed[from]); + // An empty block's first line is its own closing brace, one level + // out from the keys that belong inside. + let app_depth = depth(borrowed[from]) + usize::from(from == to); let existing = (from..to).find(|i| { pair_key(borrowed[*i]) == Some("LaunchOptions") && depth(borrowed[*i]) == app_depth }); @@ -506,7 +521,7 @@ fn finish_write( if content.ends_with('\n') { out.push('\n'); } - write_atomic(config, out.as_bytes())?; + crate::mangohud::replace_file(config, out.as_bytes()).context("replace localconfig.vdf")?; // Read back rather than trusting the write. let readback = launch_options(config, app_id); @@ -520,13 +535,55 @@ fn finish_write( Ok(()) } -fn write_atomic(path: &Path, content: &[u8]) -> Result<()> { - let dir = path.parent().context("path has no parent")?; - let tmp = dir.join(format!(".localconfig.bigame.{}", std::process::id())); - std::fs::write(&tmp, content).with_context(|| format!("write {}", tmp.display()))?; - if let Err(e) = std::fs::rename(&tmp, path) { - let _ = std::fs::remove_file(&tmp); - return Err(e).context("replace localconfig.vdf"); +// ── Changing several accounts ──────────────────────────────────────────────── + +/// Words Big Game Mode put into games' launch options, by account (its +/// `localconfig.vdf`, as [`SteamUser::config`] names it) and then by app, +/// so that exactly those come out again: a word the user typed in one +/// account is theirs even where Big Game Mode added the same word in +/// another. +pub(crate) type Inserted = + std::collections::BTreeMap>; + +/// One account's new launch options for one game, worked out before any +/// account is written. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Change { + /// The account's `localconfig.vdf`. + pub config: PathBuf, + /// The game's app id. + pub app: String, + /// The options as they read before. + pub before: String, + /// The options to write. + pub after: String, +} + +/// Write every change, or none: an account that cannot be written puts +/// back the ones written before it, so a failure half-way does not leave +/// words in some accounts with no record of them. +/// +/// # Errors +/// Returns the first account's error (Steam running included). +pub(crate) fn set_all(changes: &[Change]) -> Result<()> { + write_all(changes, &|config, app, value| { + set_launch_options(config, app, value) + }) +} + +fn write_all(changes: &[Change], write: &dyn Fn(&Path, &str, &str) -> Result<()>) -> Result<()> { + let mut written: Vec<&Change> = Vec::new(); + for change in changes { + if let Err(e) = write(&change.config, &change.app, &change.after) { + for done in written.iter().rev() { + if let Err(undo) = write(&done.config, &done.app, &done.before) { + tracing::warn!(target: "launch", app = %done.app, error = %format!("{undo:#}"), + "a Steam account's launch options could not be put back"); + } + } + return Err(e); + } + written.push(change); } Ok(()) } @@ -626,12 +683,11 @@ pub fn broken_launch_options(config: &Path) -> Vec { let mut out = Vec::new(); let mut current_app: Option = None; for line in &lines[from..to] { - if depth(line) == app_depth { - if let Some(key) = block_key(line) { - if key.chars().all(|c| c.is_ascii_digit()) { - current_app = Some(key.to_owned()); - } - } + if depth(line) == app_depth + && let Some(key) = block_key(line) + && key.chars().all(|c| c.is_ascii_digit()) + { + current_app = Some(key.to_owned()); } if depth(line) != app_depth + 1 || pair_key(line) != Some("LaunchOptions") { continue; @@ -800,6 +856,103 @@ mod tests { let _ = std::fs::remove_dir_all(path.parent().unwrap()); } + #[test] + fn an_empty_app_block_gets_the_key_inside_it() { + let vdf = VDF.replace( + "\t\t\t\t\t\"1808500\"\n\t\t\t\t\t{\n\t\t\t\t\t\t\"LastPlayed\"\t\t\"1789800000\"\n", + "\t\t\t\t\t\"1808500\"\n\t\t\t\t\t{\n", + ); + assert_ne!(vdf, VDF); + let path = write_temp("empty-block", &vdf); + write_launch_options(&path, "1808500", "MANGOHUD=1 %command%").unwrap(); + let text = std::fs::read_to_string(&path).unwrap(); + assert!( + text.contains( + "\t\t\t\t\t\"1808500\"\n\t\t\t\t\t{\n\t\t\t\t\t\t\"LaunchOptions\"\t\t\"MANGOHUD=1 %command%\"\n\t\t\t\t\t}\n" + ), + "{text}" + ); + assert_eq!( + launch_options(&path, "1808500").as_deref(), + Some("MANGOHUD=1 %command%") + ); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + + #[test] + fn two_games_written_at_once_both_keep_their_options() { + let path = write_temp("concurrent", VDF); + std::thread::scope(|s| { + for (app, value) in [ + ("381210", "MANGOHUD=1 %command%"), + ("1808500", "gamemoderun %command%"), + ] { + let path = &path; + s.spawn(move || { + for _ in 0..20 { + write_launch_options(path, app, value).unwrap(); + } + }); + } + }); + assert_eq!( + launch_options(&path, "381210").as_deref(), + Some("MANGOHUD=1 %command%") + ); + assert_eq!( + launch_options(&path, "1808500").as_deref(), + Some("gamemoderun %command%") + ); + let left: Vec = std::fs::read_dir(path.parent().unwrap()) + .unwrap() + .flatten() + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| n.starts_with('.')) + .collect(); + assert!(left.is_empty(), "{left:?}"); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + + #[test] + fn an_account_that_cannot_be_written_puts_back_the_ones_before_it() { + let store = std::cell::RefCell::new(std::collections::HashMap::from([ + ("a".to_owned(), "-x".to_owned()), + ("b".to_owned(), "-y".to_owned()), + ])); + let change = |c: &str, before: &str| Change { + config: PathBuf::from(c), + app: "10".into(), + before: before.into(), + after: format!("MANGOHUD=1 %command% {before}"), + }; + let changes = [change("a", "-x"), change("b", "-y")]; + let write = |c: &Path, _: &str, v: &str| { + anyhow::ensure!(c != Path::new("b"), "read-only"); + store + .borrow_mut() + .insert(c.to_string_lossy().into_owned(), v.to_owned()); + Ok(()) + }; + assert!(write_all(&changes, &write).is_err()); + assert_eq!(store.borrow()["a"], "-x", "put back"); + assert_eq!(store.borrow()["b"], "-y"); + assert!(write_all(&changes[..1], &write).is_ok()); + assert_eq!(store.borrow()["a"], "MANGOHUD=1 %command% -x"); + } + + #[test] + fn writing_keeps_the_files_permissions() { + use std::os::unix::fs::PermissionsExt as _; + let path = write_temp("mode", VDF); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).unwrap(); + write_launch_options(&path, "381210", "-novid").unwrap(); + assert_eq!( + std::fs::metadata(&path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + #[test] fn writing_leaves_a_backup() { let path = write_temp("backup", VDF); @@ -933,6 +1086,34 @@ mod tests { ); } + #[test] + fn one_account_in_the_native_and_the_flatpak_steam_is_two_configurations() { + let home = tempfile::tempdir().unwrap(); + let account = |root: &str| { + let config = home + .path() + .join(root) + .join("userdata/1234/config/localconfig.vdf"); + std::fs::create_dir_all(home.path().join(root).join("steamapps")).unwrap(); + std::fs::create_dir_all(config.parent().unwrap()).unwrap(); + std::fs::write(&config, VDF).unwrap(); + }; + account(".local/share/Steam"); + account(".var/app/com.valvesoftware.Steam/.local/share/Steam"); + let found = users(home.path()); + assert_eq!(found.len(), 2, "{found:?}"); + assert!(found.iter().all(|u| u.id == "1234")); + assert_eq!(found.iter().filter(|u| u.flatpak()).count(), 1); + // `~/.steam/steam`, a link to the native install, is not a third. + std::fs::create_dir_all(home.path().join(".steam")).unwrap(); + std::os::unix::fs::symlink( + home.path().join(".local/share/Steam"), + home.path().join(".steam/steam"), + ) + .unwrap(); + assert_eq!(users(home.path()).len(), 2); + } + #[test] fn detects_wrappers_that_are_not_installed() { let path = write_temp("broken", VDF); diff --git a/bigame-engine/bigame-core/src/steam_gamescope.rs b/bigame-engine/bigame-core/src/steam_gamescope.rs index a897c0c..df2a1ff 100644 --- a/bigame-engine/bigame-core/src/steam_gamescope.rs +++ b/bigame-engine/bigame-core/src/steam_gamescope.rs @@ -15,8 +15,10 @@ //! owner too, whether Gamescope or `OptiScaler` upscales the game or AI //! Graphics switched Wine FSR off for it. //! -//! Big Game Mode owns only the segment and the variables it wrote (kept in the -//! game's settings); the user's own options are left alone. Where they run +//! Big Game Mode owns only the segment and the variables it wrote (the +//! segment kept in the game's settings, the variables by account in its +//! state directory); the user's own options are left alone, a variable +//! they set already included. Where they run //! a Gamescope of the user's own, Big Game Mode's is not added: two nested //! compositors would scale twice, or not start. vkBasalt, which Gamescope //! would otherwise load for itself, stays in the game (as Big Game Mode's own @@ -28,7 +30,7 @@ use anyhow::Result; use crate::error::UserError; use crate::gamescope::{Config, Mode}; -use crate::steam::{COMMAND, is_command, option_words}; +use crate::steam::{COMMAND, Inserted, is_command, option_words}; use crate::text::N_; /// `gamescope --`, the wrapper for `cfg`, or `None` when Gamescope @@ -140,35 +142,56 @@ pub enum Applied { TheirGamescope(String), } +/// Whether `word` sets a variable (`NAME=value`). +fn is_assignment(word: &str) -> bool { + word.split_once('=').is_some_and(|(name, _)| { + name.starts_with(|c: char| c.is_ascii_alphabetic() || c == '_') + && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') + }) +} + /// `current` launch options with the variables Big Game Mode wrote before -/// (`previous`) taken out and `wanted` put in front of everything. Each -/// variable is taken out once, wherever it is, so one that something else -/// moved or removed does not keep the others in; variables already in front -/// are not put there twice. +/// (`previous`) taken out and `wanted` put in front of everything, and the +/// variables it put in, to record. Each variable is taken out once, +/// wherever it is, so one that something else moved or removed does not +/// keep the others in. A variable the options already set in front — the +/// user's own `ENABLE_VKBASALT=1` — is neither set twice nor recorded: it +/// is not Big Game Mode's to take out later. #[must_use] -pub fn env_options(current: &str, previous: Option<&str>, wanted: Option<&str>) -> String { +pub fn env_options( + current: &str, + previous: Option<&str>, + wanted: Option<&str>, +) -> (String, String) { let mut words = option_words(current); for old in option_words(previous.unwrap_or_default()) { if let Some(at) = words.iter().position(|w| *w == old) { words.remove(at); } } - let wanted = option_words(wanted.unwrap_or_default()); - if wanted.is_empty() { - return joined(&words); - } - if words.starts_with(&wanted) { - return words.join(" "); + let set: Vec<&str> = words + .iter() + .take_while(|w| is_assignment(w)) + .copied() + .collect(); + let added: Vec<&str> = option_words(wanted.unwrap_or_default()) + .into_iter() + .filter(|w| !set.contains(w)) + .collect(); + if added.is_empty() { + return (joined(&words), String::new()); } if !words.iter().any(|w| is_command(w)) { // Plain arguments: they follow the command. words.insert(0, COMMAND); } - wanted - .into_iter() + let options = added + .iter() + .copied() .chain(words) .collect::>() - .join(" ") + .join(" "); + (options, added.join(" ")) } /// What a game's launch options should hold from Big Game Mode: its @@ -229,62 +252,143 @@ pub fn apply(process: &str, wanted: Wanted) -> Result { return Ok(Applied::SteamRunning); } let users = crate::steam::users(&crate::paths::home_dir()); - if wanted.gamescope.is_some() && crate::steam::any_flatpak(&users) { - if let Some(command) = crate::steam::flatpak_gamescope_missing() { - anyhow::bail!(UserError::with( - N_( - "Steam's Flatpak finds Gamescope only in Flathub's Gamescope extension, which is not installed, and this game would not start with it. Nothing was written. Install it and restart Steam: %s" - ), - [command] - )); - } + if wanted.gamescope.is_some() + && crate::steam::any_flatpak(&users) + && let Some(command) = crate::steam::flatpak_gamescope_missing() + { + anyhow::bail!(UserError::with( + N_( + "Steam's Flatpak finds Gamescope only in Flathub's Gamescope extension, which is not installed, and this game would not start with it. Nothing was written. Install it and restart Steam: %s" + ), + [command] + )); + } + let record = env_record_path(); + let mut records: std::collections::BTreeMap = + crate::mangohud::read_record(&record)?; + let accounts: Vec = users.into_iter().map(|u| u.config).collect(); + let plan = plan( + &accounts, + &apps, + &|config, app| crate::steam::launch_options(config, app), + &Previous { + gamescope: settings.steam_gamescope.as_deref(), + env: records.get(process), + legacy_env: previous_env.as_deref(), + }, + &wanted, + ); + crate::steam::set_all(&plan.changes)?; + if records.get(process) != Some(&plan.env) { + records.insert(process.to_owned(), plan.env); + crate::mangohud::write_record(&record, &records)?; } - let mut changes = Vec::new(); - let (mut segment_written, mut theirs) = (false, false); - let mut last = String::new(); - for user in &users { - for app in &apps { - let current = crate::steam::launch_options(&user.config, app).unwrap_or_default(); - let mut with_env = - env_options(¤t, previous_env.as_deref(), wanted.env.as_deref()); + settings.steam_gamescope = wanted.gamescope.filter(|_| plan.segment_written); + settings.steam_env = wanted.env; + settings.steam_wine_fsr_off = wanted.wine_fsr_off; + crate::game_settings::save(process, &settings)?; + Ok(if plan.theirs { + Applied::TheirGamescope(plan.last) + } else { + Applied::Written(plan.last) + }) +} + +/// Where the variables put into each account's launch options are +/// recorded, by game. +fn env_record_path() -> std::path::PathBuf { + crate::paths::state_home().join("bigame-mode/steam/env-added.toml") +} + +/// What Big Game Mode wrote into a game's launch options before. +struct Previous<'a> { + /// Its Gamescope segment. + gamescope: Option<&'a str>, + /// The variables it put into each account, since 2.3.2. + env: Option<&'a Inserted>, + /// With no record by account, the variables an older version wrote, + /// taken for every account's. + legacy_env: Option<&'a str>, +} + +/// Every account's new launch options, worked out before any is written. +struct Plan { + changes: Vec, + /// The variables put in, to record. + env: Inserted, + /// Big Game Mode's Gamescope is in some account's options. + segment_written: bool, + /// Some account runs a Gamescope of the user's own instead. + theirs: bool, + /// The last account's options. + last: String, +} + +fn plan( + accounts: &[std::path::PathBuf], + apps: &[String], + read: &dyn Fn(&std::path::Path, &str) -> Option, + previous: &Previous, + wanted: &Wanted, +) -> Plan { + let mut plan = Plan { + changes: Vec::new(), + env: Inserted::new(), + segment_written: false, + theirs: false, + last: String::new(), + }; + for config in accounts { + let account = config.to_string_lossy().into_owned(); + for app in apps { + let current = read(config, app).unwrap_or_default(); + let previous_env = match previous.env { + Some(by) => by + .get(&account) + .and_then(|a| a.get(app)) + .map(String::as_str), + None => previous.legacy_env, + }; + let (mut with_env, mut added) = + env_options(¤t, previous_env, wanted.env.as_deref()); if wanted.drop_their_wine_fsr_on { + let drop = |w: &&str| *w != "WINE_FULLSCREEN_FSR=1"; let mut words = option_words(&with_env); - words.retain(|w| *w != "WINE_FULLSCREEN_FSR=1"); + words.retain(drop); with_env = joined(&words); + let mut mine = option_words(&added); + mine.retain(drop); + added = mine.join(" "); } - let next = launch_options( - &with_env, - settings.steam_gamescope.as_deref(), - wanted.gamescope.as_deref(), - ); + if !added.is_empty() { + plan.env + .entry(account.clone()) + .or_default() + .insert(app.clone(), added); + } + let next = launch_options(&with_env, previous.gamescope, wanted.gamescope.as_deref()); if let Some(seg) = wanted.gamescope.as_deref() { if option_words(&next) .windows(option_words(seg).len()) .any(|w| w == option_words(seg)) { - segment_written = true; + plan.segment_written = true; } else { - theirs = true; + plan.theirs = true; } } if next != current { - changes.push((user.config.clone(), app.clone(), next.clone())); + plan.changes.push(crate::steam::Change { + config: config.clone(), + app: app.clone(), + before: current, + after: next.clone(), + }); } - last = next; + plan.last = next; } } - for (config, app, next) in &changes { - crate::steam::set_launch_options(config, app, next)?; - } - settings.steam_gamescope = wanted.gamescope.filter(|_| segment_written); - settings.steam_env = wanted.env; - settings.steam_wine_fsr_off = wanted.wine_fsr_off; - crate::game_settings::save(process, &settings)?; - Ok(if theirs { - Applied::TheirGamescope(last) - } else { - Applied::Written(last) - }) + plan } // ── Wine FSR off for one game ─────────────────────────────────────────────── @@ -314,7 +418,8 @@ pub fn current_options(process: &str) -> Option { }) } -fn steam_apps(process: &str) -> Vec { +/// The app ids of the Steam games whose process is `process`. +pub(crate) fn steam_apps(process: &str) -> Vec { crate::games::detect_all() .into_iter() .filter(|g| g.profile_key() == process && g.source == crate::games::Source::Steam) @@ -346,6 +451,120 @@ mod tests { const SEG: &str = "gamescope -w 1280 -h 720 -F fsr -f --"; + /// The options [`env_options`] gives, without what it put in. + fn env_only(current: &str, previous: Option<&str>, wanted: Option<&str>) -> String { + env_options(current, previous, wanted).0 + } + + #[test] + fn a_variable_the_user_set_is_not_recorded_and_stays() { + for mine in [ + "ENABLE_VKBASALT=1 %command%", + "WINE_FULLSCREEN_FSR=0 %command% -dx12", + "WINE_FULLSCREEN_FSR=1 WINE_FULLSCREEN_FSR_MODE=quality %command%", + ] { + let (on, added) = env_options(mine, None, Some(mine.split(" %").next().unwrap())); + assert_eq!((on.as_str(), added.as_str()), (mine, ""), "{mine}"); + } + // Only the missing one goes in, and only it is recorded. + let (on, added) = env_options( + "ENABLE_VKBASALT=1 %command%", + None, + Some("WINE_FULLSCREEN_FSR=0 ENABLE_VKBASALT=1"), + ); + assert_eq!(on, "WINE_FULLSCREEN_FSR=0 ENABLE_VKBASALT=1 %command%"); + assert_eq!(added, "WINE_FULLSCREEN_FSR=0"); + assert_eq!( + env_only(&on, Some(&added), None), + "ENABLE_VKBASALT=1 %command%" + ); + // After a wrapper it is no variable of the game's: ours goes in front. + let (on, added) = env_options( + "gamemoderun ENABLE_VKBASALT=1", + None, + Some("ENABLE_VKBASALT=1"), + ); + assert_eq!( + on, + "ENABLE_VKBASALT=1 %command% gamemoderun ENABLE_VKBASALT=1" + ); + assert_eq!(added, "ENABLE_VKBASALT=1"); + assert!(is_assignment("_A1=x") && !is_assignment("1A=x") && !is_assignment("-x=1")); + } + + #[test] + fn each_account_takes_out_only_the_variables_put_into_it() { + // One account in two Steam installs: the user typed vkBasalt's + // variable in the Flatpak's options themselves. + let store = std::cell::RefCell::new(std::collections::HashMap::from([ + ("/native".to_owned(), "-novid".to_owned()), + ( + "/flatpak".to_owned(), + "ENABLE_VKBASALT=1 %command%".to_owned(), + ), + ])); + let read = |c: &std::path::Path, _: &str| { + store.borrow().get(c.to_string_lossy().as_ref()).cloned() + }; + let accounts = [std::path::PathBuf::from("/native"), "/flatpak".into()]; + let apps = ["10".to_owned()]; + let run = |previous: &Previous, wanted: &Wanted| { + let plan = plan(&accounts, &apps, &read, previous, wanted); + for c in &plan.changes { + store + .borrow_mut() + .insert(c.config.to_string_lossy().into_owned(), c.after.clone()); + } + plan.env + }; + let on = Wanted { + env: Some("ENABLE_VKBASALT=1".into()), + ..Wanted::default() + }; + let none = Previous { + gamescope: None, + env: None, + legacy_env: None, + }; + let record = run(&none, &on); + assert_eq!( + store.borrow()["/native"], + "ENABLE_VKBASALT=1 %command% -novid" + ); + assert_eq!(store.borrow()["/flatpak"], "ENABLE_VKBASALT=1 %command%"); + assert_eq!(record.len(), 1); + assert_eq!(record["/native"]["10"], "ENABLE_VKBASALT=1"); + + let record = run( + &Previous { + env: Some(&record), + ..none + }, + &Wanted::default(), + ); + assert_eq!(store.borrow()["/native"], "%command% -novid"); + assert_eq!( + store.borrow()["/flatpak"], + "ENABLE_VKBASALT=1 %command%", + "the user's own stays" + ); + assert!(record.is_empty()); + + // An older version's record stands for every account, as it did. + store + .borrow_mut() + .insert("/native".into(), "ENABLE_VKBASALT=1 %command%".into()); + run( + &Previous { + legacy_env: Some("ENABLE_VKBASALT=1"), + ..none + }, + &Wanted::default(), + ); + assert_eq!(store.borrow()["/native"], ""); + assert_eq!(store.borrow()["/flatpak"], ""); + } + #[test] fn the_wrapper_goes_in_front_of_the_command_and_the_users_options_stay() { assert_eq!( @@ -367,27 +586,24 @@ mod tests { #[test] fn a_games_variables_go_in_front_of_everything_and_come_out_again() { let env = "WINE_FULLSCREEN_FSR=0 ENABLE_VKBASALT=1"; - assert_eq!(env_options("", None, Some(env)), format!("{env} %command%")); + assert_eq!(env_only("", None, Some(env)), format!("{env} %command%")); let mine = "MANGOHUD=1 gamemoderun %command% -dx12"; - let on = env_options(mine, None, Some(env)); + let on = env_only(mine, None, Some(env)); assert_eq!(on, format!("{env} {mine}")); - assert_eq!(env_options(&on, Some(env), None), mine); + assert_eq!(env_only(&on, Some(env), None), mine); // Replaced, not added twice. assert_eq!( - env_options(&on, Some(env), Some("ENABLE_VKBASALT=0")), + env_only(&on, Some(env), Some("ENABLE_VKBASALT=0")), format!("ENABLE_VKBASALT=0 {mine}") ); - assert_eq!( - env_options(&format!("{env} %command%"), Some(env), None), - "" - ); + assert_eq!(env_only(&format!("{env} %command%"), Some(env), None), ""); // One variable removed by something else: the other still goes. let moved = on.replacen("WINE_FULLSCREEN_FSR=0 ", "", 1); - assert_eq!(env_options(&moved, Some(env), None), mine); + assert_eq!(env_only(&moved, Some(env), None), mine); // Written already (a write that stopped half-way): not twice. - assert_eq!(env_options(&on, None, Some(env)), on); + assert_eq!(env_only(&on, None, Some(env)), on); // With the Gamescope wrapper, each in its place. - let both = launch_options(&env_options(mine, None, Some(env)), None, Some(SEG)); + let both = launch_options(&env_only(mine, None, Some(env)), None, Some(SEG)); assert_eq!( both, format!("{env} MANGOHUD=1 gamemoderun {SEG} %command% -dx12") @@ -511,7 +727,7 @@ mod tests { fn a_quoted_command_is_the_command() { let mine = "gamemoderun \"%command%\" -name 'A B'"; assert_eq!( - env_options(mine, None, Some("ENABLE_VKBASALT=1")), + env_only(mine, None, Some("ENABLE_VKBASALT=1")), format!("ENABLE_VKBASALT=1 {mine}") ); assert_eq!( diff --git a/bigame-engine/bigame-core/src/systemd.rs b/bigame-engine/bigame-core/src/systemd.rs index 717bf07..ac9351b 100644 --- a/bigame-engine/bigame-core/src/systemd.rs +++ b/bigame-engine/bigame-core/src/systemd.rs @@ -212,6 +212,13 @@ impl UnitState { } } +/// How the unit and service proxies read their properties: each one asked +/// for, when read. zbus's default cache subscribes to the object's +/// `PropertiesChanged` and fetches every property of the interface on the +/// first read — hundreds for a service — for proxies that read five between +/// them and are dropped, every ten seconds. +const UNCACHED: zbus::proxy::CacheProperties = zbus::proxy::CacheProperties::No; + /// Read a unit's state. /// /// # Errors @@ -224,6 +231,7 @@ pub async fn unit_state(connection: &zbus::Connection, unit: &str) -> zbus::Resu let path = manager.load_unit(unit).await?; let proxy = UnitProxy::builder(connection) .path(path.clone())? + .cache_properties(UNCACHED) .build() .await?; let active_state = proxy.active_state().await?; @@ -232,6 +240,7 @@ pub async fn unit_state(connection: &zbus::Connection, unit: &str) -> zbus::Resu let service = async { let s = ServiceProxy::builder(connection) .path(path)? + .cache_properties(UNCACHED) .build() .await?; zbus::Result::Ok(ServiceRun { @@ -290,6 +299,7 @@ impl Reader { let proxy = UnitProxyBlocking::builder(&self.connection) .path(path.clone()) .ok()? + .cache_properties(UNCACHED) .build() .ok()?; let active_state = proxy.active_state().ok()?; @@ -306,6 +316,7 @@ impl Reader { let s = ServiceProxyBlocking::builder(&self.connection) .path(path) .ok()? + .cache_properties(UNCACHED) .build() .ok()?; Some(ServiceRun { diff --git a/bigame-engine/bigame-core/src/turbo.rs b/bigame-engine/bigame-core/src/turbo.rs index 5c1055f..f79163b 100644 --- a/bigame-engine/bigame-core/src/turbo.rs +++ b/bigame-engine/bigame-core/src/turbo.rs @@ -79,8 +79,10 @@ pub async fn state() -> Result { /// Whether Turbo's state is falcond's: it is installed, and not a build that /// crashed on this processor. Otherwise Turbo is the Booster's, as without -/// falcond. -fn backend_governs(unit: &crate::systemd::UnitState) -> bool { +/// falcond. Every surface that reads Turbo from falcond's unit asks this +/// first: an inactive falcond that does not govern is not a Turbo off. +#[must_use] +pub fn backend_governs(unit: &crate::systemd::UnitState) -> bool { unit.is_installed() && !crate::falcond_compat::crashes_here() } @@ -641,38 +643,37 @@ async fn enable_backend( ) -> bool { // Profile set first, so falcond starts with the right one rather than // loading the wrong set and reloading. - if let Ok(mut config) = crate::config::read() { - if let Some(fixed) = corrected_profile_mode(hardware.chassis, &config.profile_mode) { - progress(Step::ConfiguringProfiles); - let before = config.profile_mode.clone(); - config.profile_mode = fixed.to_owned(); - match crate::config::write(&config).await { - Ok(()) if crate::config::read().is_ok_and(|c| c.profile_mode == fixed) => report - .push( - Kind::ProfileSet, - Section::Verified, - "Big Game Mode", - Text::with( - N_( - "%s → %s: the handheld profiles run games in power-saving mode, \ + if let Ok(mut config) = crate::config::read() + && let Some(fixed) = corrected_profile_mode(hardware.chassis, &config.profile_mode) + { + progress(Step::ConfiguringProfiles); + let before = config.profile_mode.clone(); + config.profile_mode = fixed.to_owned(); + match crate::config::write(&config).await { + Ok(()) if crate::config::read().is_ok_and(|c| c.profile_mode == fixed) => report.push( + Kind::ProfileSet, + Section::Verified, + "Big Game Mode", + Text::with( + N_( + "%s → %s: the handheld profiles run games in power-saving mode, \ and this machine is not a handheld", - ), - [Arg::Raw(before), Arg::Text(desktop_set())], - ), ), - Ok(()) => report.push( - Kind::ProfileSet, - Section::Failed, - "Big Game Mode", - Text::plain(N_("the configuration was written but reads back unchanged")), + [Arg::Raw(before), Arg::Text(desktop_set())], ), - Err(e) => report.push( - Kind::ProfileSet, - Section::Failed, - "Big Game Mode", - crate::error::describe(&e), - ), - } + ), + Ok(()) => report.push( + Kind::ProfileSet, + Section::Failed, + "Big Game Mode", + Text::plain(N_("the configuration was written but reads back unchanged")), + ), + Err(e) => report.push( + Kind::ProfileSet, + Section::Failed, + "Big Game Mode", + crate::error::describe(&e), + ), } } @@ -783,15 +784,21 @@ async fn apply_preset(preset: crate::turbo_preset::Preset, report: &mut Report) /// Take the Turbo preset away and say so in the report (on the blocking /// pool, as [`apply_preset`]). async fn remove_preset(report: &mut Report) { - let preset = crate::turbo_preset::active(); - if preset == crate::turbo_preset::Preset::Standard { + // A record that cannot be read reads as Standard, yet its variables may + // still be in the session. + if !crate::turbo_preset::has_record() { + // A launcher open when the last preset went may be closed now. + let _ = tokio::task::spawn_blocking(crate::turbo_preset::follow_owed).await; return; } + let preset = crate::turbo_preset::active(); let title = Text::plain(N_("Turbo preset")); let removed = tokio::task::spawn_blocking(crate::turbo_preset::deactivate) .await .unwrap_or_else(|e| Err(anyhow::anyhow!("{e}"))); match removed { + // Unreadable, so no preset to name: the log says what was cleared. + Ok(_) if preset == crate::turbo_preset::Preset::Standard => {} Ok(_) => report.push_knob( title, Section::Restored, @@ -906,10 +913,8 @@ async fn wait_for_fresh_status( let fresh = std::fs::metadata(crate::status::status_path()) .and_then(|m| m.modified()) .is_ok_and(|m| m >= since); - if fresh { - if let Some(s) = crate::status::read() { - return Some(s); - } + if fresh && let Some(s) = crate::status::read() { + return Some(s); } tokio::time::sleep(std::time::Duration::from_millis(100)).await; } @@ -1221,10 +1226,10 @@ async fn note_scheduler_left(report: &mut Report) { ); } let path = scheduler_record(); - if let Err(e) = std::fs::remove_file(&path) { - if e.kind() != std::io::ErrorKind::NotFound { - tracing::warn!(target: "turbo", file = %path.display(), error = %e, "could not remove the scheduler note"); - } + if let Err(e) = std::fs::remove_file(&path) + && e.kind() != std::io::ErrorKind::NotFound + { + tracing::warn!(target: "turbo", file = %path.display(), error = %e, "could not remove the scheduler note"); } } @@ -1251,7 +1256,7 @@ pub fn something_left_blocking() -> bool { let unit = crate::systemd::Reader::shared().and_then(|r| r.unit_state(BACKEND_UNIT)); something_left( unit.as_ref(), - crate::turbo_preset::active() != crate::turbo_preset::Preset::Standard, + crate::turbo_preset::has_record(), BoosterEngine::is_active(), ) } @@ -1275,10 +1280,10 @@ pub fn tidy_up_blocking() -> Result<()> { } let preset = crate::turbo_preset::resync(false); let booster = reconcile_blocking(); - if let Ok(n) = &booster { - if *n > 0 { - tracing::info!(target: "turbo", restored = n, "Booster changes put back after falcond stopped"); - } + if let Ok(n) = &booster + && *n > 0 + { + tracing::info!(target: "turbo", restored = n, "Booster changes put back after falcond stopped"); } preset.context("take the Turbo preset away")?; booster.context("put the Booster's changes back")?; diff --git a/bigame-engine/bigame-core/src/turbo_preset.rs b/bigame-engine/bigame-core/src/turbo_preset.rs index 6457e23..e375d6b 100644 --- a/bigame-engine/bigame-core/src/turbo_preset.rs +++ b/bigame-engine/bigame-core/src/turbo_preset.rs @@ -358,16 +358,25 @@ fn write_cas_config(path: &Path) -> Result<()> { } /// Replace `path` with `text` in one rename: a reader sees the old file or -/// the new one, never half of one. -fn write_atomic(path: &Path, text: &str) -> Result<()> { +/// the new one, never half of one, and a crash leaves one of them whole. +pub(crate) fn write_atomic(path: &Path, text: &str) -> Result<()> { use std::io::Write as _; + static SEQUENCE: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + // A dotfile manager's symlink stays a symlink: the file it points to is + // the one replaced. + let path = &std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); let dir = path.parent().context("path has no parent directory")?; std::fs::create_dir_all(dir).with_context(|| format!("create {}", dir.display()))?; let name = path .file_name() .map(|n| n.to_string_lossy().into_owned()) .unwrap_or_default(); - let tmp = dir.join(format!(".{name}.{}", std::process::id())); + // A name of its own per write, so two writers never share a temporary. + let tmp = dir.join(format!( + ".{name}.{}-{}", + std::process::id(), + SEQUENCE.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); let written = (|| { let mut f = std::fs::File::create(&tmp)?; f.write_all(text.as_bytes())?; @@ -375,10 +384,10 @@ fn write_atomic(path: &Path, text: &str) -> Result<()> { std::fs::rename(&tmp, path) })(); if let Err(e) = written { - if let Err(cleanup) = std::fs::remove_file(&tmp) { - if cleanup.kind() != std::io::ErrorKind::NotFound { - tracing::warn!(file = %tmp.display(), error = %cleanup, "could not remove a temporary file"); - } + if let Err(cleanup) = std::fs::remove_file(&tmp) + && cleanup.kind() != std::io::ErrorKind::NotFound + { + tracing::warn!(file = %tmp.display(), error = %cleanup, "could not remove a temporary file"); } return Err(e).with_context(|| format!("write {}", path.display())); } @@ -642,19 +651,84 @@ pub fn activate(preset: Preset) -> Result> { }; write(&path, &Stored { preset, before })?; let set = crate::video_config::sync_session_env(&crate::video_config::load())?; - follow_in_steam(); + follow_in_launchers(); Ok(set) } -/// Steam's launch options carry a game's vkBasalt switch, which follows the -/// preset in force: they are brought in line with it. A game whose options -/// cannot be written keeps the old ones, which is said in the log. -fn follow_in_steam() { - for (game, result) in crate::optimization::refresh_steam_gamescope() { - if let Err(e) = result { - tracing::warn!(%game, error = %format!("{e:#}"), "Steam's launch options do not follow the Turbo preset"); +/// Whether a preset's record is there, readable or not. [`active`] reads an +/// unreadable one as Standard, yet its variables may still be in the +/// session and [`deactivate`] must still run to take them away. +#[must_use] +pub fn has_record() -> bool { + active_path().exists() +} + +/// The mark that the launchers still owe the preset's change: an open +/// launcher would write its own copy back over it, so it was not written. +fn launchers_owed_path() -> PathBuf { + crate::paths::state_home() + .join("bigame-mode") + .join("turbo-preset-launchers-owed") +} + +/// Steam's launch options and Heroic's settings carry a game's vkBasalt +/// switch, which follows the preset in force: they are brought in line with +/// it. While a launcher is open nothing is written to it (it would write its +/// own copy back), so a Turbo off would leave the preset's vkBasalt in its +/// games for good; the mark [`launchers_owed_path`] keeps that owed, and +/// [`resync`] and the next Turbo switch try again until it is written. +fn follow_in_launchers() { + follow_in_launchers_at( + &launchers_owed_path(), + crate::optimization::refresh_steam_gamescope, + crate::optimization::refresh_heroic, + ); +} + +/// [`follow_in_launchers`] with the mark at `owed` and the launchers +/// refreshed by `steam` and `heroic`. Returns whether every game follows. +/// A game whose settings cannot be written stays owed too: what failed may +/// be fixed by the next try, and the mark goes only once nothing is left. +fn follow_in_launchers_at( + owed: &Path, + steam: impl FnOnce() -> Vec<(String, Result)>, + heroic: impl FnOnce() -> Vec<(String, Result)>, +) -> bool { + let mut waiting: Vec = Vec::new(); + let mut failed = false; + for (game, result) in steam() { + match result { + Ok(crate::steam_gamescope::Applied::SteamRunning) => waiting.push(game), + Ok(_) => {} + Err(e) => { + failed = true; + tracing::warn!(%game, error = %format!("{e:#}"), "Steam's launch options do not follow the Turbo preset"); + } + } + } + for (game, result) in heroic() { + match result { + Ok(crate::heroic_launch::Applied::HeroicRunning { .. }) => waiting.push(game), + Ok(_) => {} + Err(e) => { + failed = true; + tracing::warn!(%game, error = %format!("{e:#}"), "Heroic's settings do not follow the Turbo preset"); + } } } + if !waiting.is_empty() { + tracing::warn!(games = %waiting.join(", "), "a launcher is open: these games follow the Turbo preset once it is closed"); + } + let done = waiting.is_empty() && !failed; + let marked = if done { + remove_if_any(owed) + } else { + write_atomic(owed, "") + }; + if let Err(e) = marked { + tracing::warn!(error = %format!("{e:#}"), "could not record whether the launchers follow the Turbo preset"); + } + done } /// Take the preset away: the session goes back to Tuning's variables and @@ -666,41 +740,58 @@ fn follow_in_steam() { /// Returns an error when the session's environment cannot be set or the /// record cannot be removed. pub fn deactivate() -> Result> { - let path = active_path(); - let Some(record) = read_stored(&path) else { - let unreadable = path.exists(); - if unreadable { - // Unreadable: the user's values are lost with it, but a preset's - // variables must still go. - tracing::warn!(file = %path.display(), "the Turbo preset's record cannot be read; clearing its variables"); + deactivate_in( + &active_path(), + &cas_config_path(), + &launchers_owed_path(), + |layer| crate::video_config::sync_session_env_with(&crate::video_config::load(), layer), + follow_in_launchers, + ) +} + +/// [`deactivate`] with the record at `record`, vkBasalt's CAS file at `cas` +/// and the launchers' mark at `owed`; `sync` brings the session to a layer, +/// `follow` brings the launchers along. +fn deactivate_in( + record: &Path, + cas: &Path, + owed: &Path, + sync: impl FnOnce(&Layer) -> Result>, + follow: impl FnOnce(), +) -> Result> { + let layer = if let Some(stored) = read_stored(record) { + Layer { + levers: Levers::default(), + before: stored.before, + owns_preset_keys: true, } - let set = crate::video_config::sync_session_env_with( - &crate::video_config::load(), - &Layer { - owns_preset_keys: unreadable, - ..Layer::default() - }, - )?; - remove_record(&path)?; + } else { + let unreadable = record.exists(); if unreadable { - follow_in_steam(); + // Unreadable: the user's values are lost with it, but a + // preset's variables must still go. + tracing::warn!(file = %record.display(), "the Turbo preset's record cannot be read; clearing its variables"); + } + Layer { + owns_preset_keys: unreadable, + ..Layer::default() } - return Ok(set); }; - let set = crate::video_config::sync_session_env_with( - &crate::video_config::load(), - &Layer { - levers: Levers::default(), - before: record.before, - owns_preset_keys: true, - }, - )?; - remove_record(&path)?; - follow_in_steam(); + let had_record = layer.owns_preset_keys; + let set = sync(&layer)?; + remove_if_any(record)?; + // The session no longer names it, and nothing of Tuning's does: a + // preset leaves nothing behind once it is gone. + if let Err(e) = remove_if_any(cas) { + tracing::warn!(error = %format!("{e:#}"), "could not remove the Turbo preset's vkBasalt file"); + } + if had_record || owed.exists() { + follow(); + } Ok(set) } -fn remove_record(path: &Path) -> Result<()> { +fn remove_if_any(path: &Path) -> Result<()> { match std::fs::remove_file(path) { Ok(()) => Ok(()), Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), @@ -730,22 +821,34 @@ pub fn switch(preset: Preset) -> Result> { /// found switched off from outside: the preset's variables live only in the /// running session, so after a login they are set again while Turbo is on, /// and a preset left behind by a Turbo switched off elsewhere (or a Turbo -/// off whose undo failed) is taken away. +/// off whose undo failed) is taken away. Launchers that were open when the +/// preset last changed are brought along now ([`follow_owed`]). /// /// # Errors /// Returns an error when the session's environment cannot be set. pub fn resync(turbo_on: bool) -> Result<()> { - if !active_path().exists() { - return Ok(()); - } - if turbo_on { + if has_record() { + if !turbo_on { + // Brings the launchers along itself. + deactivate()?; + return Ok(()); + } crate::video_config::sync_session_env(&crate::video_config::load())?; - } else { - deactivate()?; } + follow_owed(); Ok(()) } +/// Bring along the launchers that were open when the preset last changed, +/// if any: one may be closed by now. Steam's launch options and Heroic's +/// settings carry a game's vkBasalt switch, which follows the preset in +/// force, and an open launcher was left out then rather than written over. +pub fn follow_owed() { + if launchers_owed_path().exists() { + follow_in_launchers(); + } +} + /// Whether the session's environment `session` holds `layer`: every /// variable the preset sets at its value, and every switch it turns off off. #[must_use] @@ -981,6 +1084,179 @@ multiplier = 1 assert_eq!(names, ["vkbasalt-cas.conf"]); } + #[test] + fn an_open_launcher_leaves_the_preset_owed_until_it_follows() { + use crate::heroic_launch::Applied as Heroic; + use crate::steam_gamescope::Applied as Steam; + let dir = Scratch::new("owed"); + let owed = dir + .0 + .join("bigame-mode") + .join("turbo-preset-launchers-owed"); + // Steam open: nothing written there, so the change stays owed. + let followed = follow_in_launchers_at( + &owed, + || vec![("SOTTR.exe".to_owned(), Ok(Steam::SteamRunning))], + Vec::new, + ); + assert!(!followed); + assert!(owed.exists()); + // Heroic open: owed as well, even with Steam's games written. + let followed = follow_in_launchers_at( + &owed, + || vec![("SOTTR.exe".to_owned(), Ok(Steam::Written(String::new())))], + || { + vec![( + "Hades.exe".to_owned(), + Ok(Heroic::HeroicRunning { + launcher: crate::launchers::Launcher::Heroic { flatpak: false }, + game_running: false, + }), + )] + }, + ); + assert!(!followed); + assert!(owed.exists()); + // A game that could not be written is tried again too. + let followed = follow_in_launchers_at( + &owed, + || vec![("SOTTR.exe".to_owned(), Err(anyhow::anyhow!("read-only")))], + Vec::new, + ); + assert!(!followed); + assert!(owed.exists()); + // Everything in line: the mark goes. + let followed = follow_in_launchers_at( + &owed, + || vec![("SOTTR.exe".to_owned(), Ok(Steam::Unchanged))], + || vec![("Hades.exe".to_owned(), Ok(Heroic::Written))], + ); + assert!(followed); + assert!(!owed.exists()); + } + + /// The files [`deactivate_in`] works on, in a scratch folder. + struct Files { + record: PathBuf, + cas: PathBuf, + owed: PathBuf, + } + + impl Files { + fn in_(dir: &Scratch) -> Self { + Self { + record: dir.0.join("turbo-preset-active.toml"), + cas: dir.0.join("vkbasalt-cas.conf"), + owed: dir.0.join("turbo-preset-launchers-owed"), + } + } + } + + #[test] + fn taking_a_preset_away_brings_the_launchers_along() { + let dir = Scratch::new("deactivate"); + let f = Files::in_(&dir); + write( + &f.record, + &Stored { + preset: Preset::Enhanced, + before: BTreeMap::from([("PROTON_FSR4_UPGRADE".to_owned(), "1".to_owned())]), + }, + ) + .unwrap(); + write_cas_config(&f.cas).unwrap(); + let mut seen = None; + let mut followed = false; + deactivate_in( + &f.record, + &f.cas, + &f.owed, + |layer| { + seen = Some(layer.clone()); + Ok(Vec::new()) + }, + || followed = true, + ) + .unwrap(); + let layer = seen.unwrap(); + assert_eq!(layer.levers, Levers::default()); + assert_eq!(layer.before["PROTON_FSR4_UPGRADE"], "1"); + assert!(layer.owns_preset_keys); + assert!(!f.record.exists()); + // vkBasalt's CAS file was the preset's alone. + assert!(!f.cas.exists()); + // After the record: the launchers then read no preset in force. + assert!(followed); + // No record and nothing owed: the launchers are left alone. + let mut followed = false; + deactivate_in( + &f.record, + &f.cas, + &f.owed, + |_| Ok(Vec::new()), + || followed = true, + ) + .unwrap(); + assert!(!followed); + // Still owed from an earlier Turbo off: tried again. + std::fs::write(&f.owed, "").unwrap(); + deactivate_in( + &f.record, + &f.cas, + &f.owed, + |_| Ok(Vec::new()), + || followed = true, + ) + .unwrap(); + assert!(followed); + } + + #[test] + fn an_unreadable_record_is_still_taken_away() { + let dir = Scratch::new("deactivate_unreadable"); + let f = Files::in_(&dir); + std::fs::write(&f.record, "preset = [not toml").unwrap(); + assert!(read_stored(&f.record).is_none()); + let mut owned = false; + let mut followed = false; + deactivate_in( + &f.record, + &f.cas, + &f.owed, + |layer| { + owned = layer.owns_preset_keys; + Ok(Vec::new()) + }, + || followed = true, + ) + .unwrap(); + // Its variables are Big Game Mode's to unset, and the launchers follow. + assert!(owned); + assert!(followed); + assert!(!f.record.exists()); + } + + #[test] + fn a_session_that_does_not_change_keeps_the_record() { + let dir = Scratch::new("deactivate_fails"); + let f = Files::in_(&dir); + write(&f.record, &Stored::default()).unwrap(); + write_cas_config(&f.cas).unwrap(); + let mut followed = false; + let result = deactivate_in( + &f.record, + &f.cas, + &f.owed, + |_| anyhow::bail!("the session environment did not change"), + || followed = true, + ); + assert!(result.is_err()); + assert!(f.record.exists(), "kept to retry from"); + // The session may still name it. + assert!(f.cas.exists()); + assert!(!followed); + } + #[test] fn a_cap_is_merged_into_the_users_dxvk_config() { assert_eq!( diff --git a/bigame-engine/bigame-core/src/video_config.rs b/bigame-engine/bigame-core/src/video_config.rs index 5d7275c..0e6e25c 100644 --- a/bigame-engine/bigame-core/src/video_config.rs +++ b/bigame-engine/bigame-core/src/video_config.rs @@ -79,14 +79,10 @@ pub fn save(cfg: &VideoConfig) -> Result> { /// # Errors /// Returns an error if directory creation or file write fails. pub fn save_to(cfg: &VideoConfig, path: &Path) -> Result<()> { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent) - .with_context(|| format!("create config dir: {}", parent.display()))?; - } let content = toml::to_string_pretty(cfg).context("serialize video config")?; - std::fs::write(path, content) - .with_context(|| format!("write video config: {}", path.display()))?; - Ok(()) + // Written in one rename: a file cut short by a crash or a full disk + // reads as no file, and every setting in it would fall back to default. + crate::turbo_preset::write_atomic(path, &content).context("write video config") } /// The session environment file: `~/.config/environment.d/bigame-mode.conf`. @@ -124,34 +120,34 @@ pub const SESSION_KEYS: &[&str] = &[ /// Returns error if directory creation or file I/O fails, or the running /// session's environment cannot be updated. pub fn write_env_file(cfg: &VideoConfig) -> Result<()> { - let path = env_file_path(); - let env = crate::launcher::build_persistent_env(cfg); + write_env_file_at( + &env_file_path(), + &crate::launcher::build_persistent_env(cfg), + )?; + sync_session_env(cfg).context(UserError::plain(N_( + "could not update the running session's environment", + )))?; + Ok(()) +} +/// The environment.d file at `path` holding `env`, or no file for none. +fn write_env_file_at(path: &Path, env: &HashMap) -> Result<()> { if env.is_empty() { if path.exists() { - std::fs::remove_file(&path) + std::fs::remove_file(path) .with_context(|| format!("remove env file: {}", path.display()))?; } - } else { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent) - .with_context(|| format!("create env dir: {}", parent.display()))?; - } - let mut keys: Vec<&String> = env.keys().collect(); - keys.sort(); - let mut content = String::from("# Managed by BiGameMode. Do not edit manually.\n"); - for k in keys { - // environment.d is KEY=VALUE per line, no quoting required for our values. - let _ = writeln!(content, "{}={}", k, env[k]); - } - std::fs::write(&path, content) - .with_context(|| format!("write env file: {}", path.display()))?; + return Ok(()); } - - sync_session_env(cfg).context(UserError::plain(N_( - "could not update the running session's environment", - )))?; - Ok(()) + let mut keys: Vec<&String> = env.keys().collect(); + keys.sort(); + let mut content = String::from("# Managed by BiGameMode. Do not edit manually.\n"); + for k in keys { + // environment.d is KEY=VALUE per line, no quoting required for our values. + let _ = writeln!(content, "{}={}", k, env[k]); + } + // The next login reads it whole or not at all. + crate::turbo_preset::write_atomic(path, &content).context("write env file") } /// Bring the running session to `cfg`'s variables with the Turbo preset in @@ -205,8 +201,7 @@ fn session_env_with( /// # Errors /// Returns an error when the session bus or the manager cannot be reached. pub fn session_environment() -> Result> { - let conn = zbus::blocking::Connection::session().context("session bus")?; - let now: Vec = user_manager(&conn)? + let now: Vec = user_manager(session_bus()?)? .get_property("Environment") .context("read the session environment")?; Ok(now @@ -216,14 +211,30 @@ pub fn session_environment() -> Result> { .collect()) } +/// One session bus connection for the whole process, opened on first use: +/// Home reads the session's environment every few seconds, and a new +/// connection each time is a new authentication with the bus. A failed +/// connect is not remembered, so a later call tries again. +fn session_bus() -> Result<&'static zbus::blocking::Connection> { + static SHARED: std::sync::OnceLock = std::sync::OnceLock::new(); + if let Some(conn) = SHARED.get() { + return Ok(conn); + } + let conn = zbus::blocking::Connection::session().context("session bus")?; + Ok(SHARED.get_or_init(|| conn)) +} + +/// The user manager, its properties read when asked: a caching proxy +/// fetches every property of the manager (`GetAll`) to read one, and then +/// follows their changes. fn user_manager(conn: &zbus::blocking::Connection) -> Result> { - zbus::blocking::Proxy::new( - conn, - "org.freedesktop.systemd1", - "/org/freedesktop/systemd1", - "org.freedesktop.systemd1.Manager", - ) - .context("systemd user manager") + zbus::blocking::proxy::Builder::new(conn) + .destination("org.freedesktop.systemd1") + .and_then(|b| b.path("/org/freedesktop/systemd1")) + .and_then(|b| b.interface("org.freedesktop.systemd1.Manager")) + .map(|b| b.cache_properties(zbus::proxy::CacheProperties::No)) + .and_then(zbus::blocking::proxy::Builder::build) + .context("systemd user manager") } /// What of Big Game Mode's variables ([`SESSION_KEYS`]) an environment really @@ -300,53 +311,93 @@ fn session_change( /// One `UnsetAndSetEnvironment` call on the user manager — no `systemctl` /// process — then a read-back of its environment to confirm it holds. fn sync_session(unset: &[String], set: &[String]) -> Result<()> { - let conn = zbus::blocking::Connection::session().context("session bus")?; - let manager = user_manager(&conn)?; + let manager = user_manager(session_bus()?)?; manager .call_method("UnsetAndSetEnvironment", &(unset, set)) .context("UnsetAndSetEnvironment")?; let now: Vec = manager .get_property("Environment") .context("read the session environment back")?; - let missing: Vec<&String> = set.iter().filter(|a| !now.contains(a)).collect(); - // A detail of a switch that is off (Wine FSR's mode with - // WINE_FULLSCREEN_FSR=0) may stay: the login put it there and systemd - // keeps it, and with its switch at 0 nothing reads it. - let harmless = |k: &str| { - DETAILS - .iter() - .any(|(detail, switch)| *detail == k && set.iter().any(|a| *a == format!("{switch}=0"))) - }; - let left: Vec<&String> = unset - .iter() - .filter(|k| !harmless(k)) - .filter(|k| { - now.iter() - .any(|a| a.split_once('=').is_some_and(|(n, _)| n == k.as_str())) - }) - .collect(); - if !missing.is_empty() || !left.is_empty() { + let held = read_back(unset, set, &now); + if !held.kept_details.is_empty() { + tracing::warn!( + variables = %held.kept_details.join(", "), + "systemd keeps what the login set; these stay until the next login" + ); + } + if !held.missing.is_empty() || !held.left.is_empty() { anyhow::bail!(UserError::with( N_("the session environment did not change: missing %s, still set %s"), - [ - missing - .iter() - .map(|s| s.as_str()) - .collect::>() - .join(", "), - left.iter() - .map(|s| s.as_str()) - .collect::>() - .join(", ") - ] + [held.missing.join(", "), held.left.join(", ")] )); } Ok(()) } +/// What a read-back of the session (`now`) says of a change that asked to +/// unset `unset` and set `set`. +#[derive(Debug, Default, PartialEq, Eq)] +struct ReadBack { + /// Assignments the session does not hold. + missing: Vec, + /// Variables still set that change what a game gets. + left: Vec, + /// A switch's detail (Wine FSR's mode, vkBasalt's file) still set. The + /// login put it there from environment.d, and systemd keeps a value its + /// generator gave (checked on systemd 261); it only tunes a feature the + /// switch turns on or off, so it waits for the next login rather than + /// failing every change until then. + kept_details: Vec, +} + +fn read_back(unset: &[String], set: &[String], now: &[String]) -> ReadBack { + let is_set = |k: &str| { + now.iter() + .any(|a| a.split_once('=').is_some_and(|(n, _)| n == k)) + }; + let (kept_details, left) = unset + .iter() + .filter(|k| is_set(k)) + .cloned() + .partition(|k| DETAILS.iter().any(|(detail, _)| detail == k)); + ReadBack { + missing: set.iter().filter(|a| !now.contains(a)).cloned().collect(), + left, + kept_details, + } +} + #[cfg(test)] mod tests { use super::*; + + #[test] + fn a_detail_the_login_left_waits_for_the_next_login() { + let strings = |v: &[&str]| v.iter().map(|s| (*s).to_owned()).collect::>(); + // More FPS turns Wine FSR on without a mode; the login gave one, and + // systemd keeps it. + let held = read_back( + &strings(&["WINE_FULLSCREEN_FSR_MODE", "DXVK_CONFIG"]), + &strings(&["WINE_FULLSCREEN_FSR=1"]), + &strings(&["WINE_FULLSCREEN_FSR=1", "WINE_FULLSCREEN_FSR_MODE=balanced"]), + ); + assert_eq!( + held, + ReadBack { + kept_details: strings(&["WINE_FULLSCREEN_FSR_MODE"]), + ..ReadBack::default() + } + ); + // A frame cap that stays, or an assignment that did not take, is not + // waved through. + let held = read_back( + &strings(&["DXVK_CONFIG"]), + &strings(&["ENABLE_VKBASALT=0"]), + &strings(&["DXVK_CONFIG=dxgi.maxFrameRate = 60", "ENABLE_VKBASALT=1"]), + ); + assert_eq!(held.left, strings(&["DXVK_CONFIG"])); + assert_eq!(held.missing, strings(&["ENABLE_VKBASALT=0"])); + } use crate::models::{FrameGenBackend, GamescopeFilter}; #[test] @@ -461,6 +512,48 @@ mod tests { let _ = std::fs::remove_dir_all(path.parent().unwrap()); } + #[test] + fn the_files_are_replaced_whole() { + let path = temp_config("atomic"); + let dir = path.parent().unwrap().to_path_buf(); + let mut cfg = VideoConfig::default(); + save_to(&cfg, &path).unwrap(); + cfg.upscaling.gamescope_sharpness = 9; + save_to(&cfg, &path).unwrap(); + assert_eq!(load_from(&path).upscaling.gamescope_sharpness, 9); + let env_file = dir.join("environment.d").join("bigame-mode.conf"); + let env = HashMap::from([ + ("WINE_FULLSCREEN_FSR".to_owned(), "1".to_owned()), + ("ENABLE_VKBASALT".to_owned(), "1".to_owned()), + ]); + write_env_file_at(&env_file, &env).unwrap(); + assert_eq!( + std::fs::read_to_string(&env_file).unwrap(), + "# Managed by BiGameMode. Do not edit manually.\nENABLE_VKBASALT=1\nWINE_FULLSCREEN_FSR=1\n" + ); + // Each in one rename: no temporary file is left beside them. + for folder in [&dir, &dir.join("environment.d")] { + let left: Vec = std::fs::read_dir(folder) + .unwrap() + .flatten() + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| n.starts_with('.')) + .collect(); + assert!(left.is_empty(), "{left:?}"); + } + // A dotfile manager's symlink stays one: its target is replaced. + let kept = dir.join("dotfiles-video.toml"); + std::fs::rename(&path, &kept).unwrap(); + std::os::unix::fs::symlink(&kept, &path).unwrap(); + save_to(&cfg, &path).unwrap(); + assert!(std::fs::symlink_metadata(&path).unwrap().is_symlink()); + assert_eq!(load_from(&kept).upscaling.gamescope_sharpness, 9); + // Nothing to put in the session: no file at all. + write_env_file_at(&env_file, &HashMap::new()).unwrap(); + assert!(!env_file.exists()); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn a_missing_or_corrupt_config_falls_back_to_defaults() { let path = temp_config("corrupt"); diff --git a/bigame-engine/bigame-core/src/vkbasalt.rs b/bigame-engine/bigame-core/src/vkbasalt.rs index 48f050d..95fbe72 100644 --- a/bigame-engine/bigame-core/src/vkbasalt.rs +++ b/bigame-engine/bigame-core/src/vkbasalt.rs @@ -153,7 +153,7 @@ pub fn default_config() -> Option { /// Which style is in place. #[must_use] pub fn current_style() -> StyleState { - state_of(std::fs::read_to_string(user_config()).ok().as_deref()) + state_of(crate::mangohud::config_text(&user_config()).as_deref()) } fn state_of(text: Option<&str>) -> StyleState { @@ -296,16 +296,13 @@ pub fn set_style(style: Style) -> Result<()> { } fn set_style_at(style: Style, path: &Path, backup: &Path, dir: &Path) -> Result<()> { - let state = state_of(std::fs::read_to_string(path).ok().as_deref()); + let state = state_of(crate::mangohud::config_text(path).as_deref()); match style_config(style, dir) { None => { if !matches!(state, StyleState::Style(_)) { return Ok(()); // Already the user's own. } - if backup.exists() { - std::fs::rename(backup, path) - .with_context(|| format!("put back {}", path.display()))?; - } else { + if !crate::mangohud::put_back(backup, path)? { std::fs::remove_file(path).with_context(|| format!("remove {}", path.display()))?; } } @@ -317,17 +314,12 @@ fn set_style_at(style: Style, path: &Path, backup: &Path, dir: &Path) -> Result< ))); } if state == StyleState::Own { - if let Some(parent) = backup.parent() { - std::fs::create_dir_all(parent)?; - } - std::fs::copy(path, backup).with_context(|| format!("keep {}", path.display()))?; + crate::mangohud::keep_aside(path, backup)?; } if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } - let tmp = path.with_extension("conf.bigame-new"); - std::fs::write(&tmp, text).with_context(|| format!("write {}", tmp.display()))?; - std::fs::rename(&tmp, path).with_context(|| format!("replace {}", path.display()))?; + crate::mangohud::replace_file(path, text.as_bytes())?; } } Ok(()) @@ -415,6 +407,50 @@ mod tests { assert!(!backup.exists()); } + #[test] + fn a_file_that_is_not_utf8_or_a_symlink_to_nothing_is_kept() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("vkBasalt/vkBasalt.conf"); + let backup = dir.path().join("state/vkBasalt.conf.user"); + let shaders = dir.path().join("reshade"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + + let latin1 = b"# nitidez m\xe9dia\neffects = cas\n".to_vec(); + std::fs::write(&path, &latin1).unwrap(); + set_style_at(Style::Cas, &path, &backup, &shaders).unwrap(); + assert_eq!(std::fs::read(&backup).unwrap(), latin1); + set_style_at(Style::Own, &path, &backup, &shaders).unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), latin1); + + std::fs::remove_file(&path).unwrap(); + let gone = dir.path().join("unmounted/vkBasalt.conf"); + std::os::unix::fs::symlink(&gone, &path).unwrap(); + set_style_at(Style::Cas, &path, &backup, &shaders).unwrap(); + assert_eq!(std::fs::read_link(&backup).unwrap(), gone); + set_style_at(Style::Own, &path, &backup, &shaders).unwrap(); + assert_eq!(std::fs::read_link(&path).unwrap(), gone); + } + + #[test] + fn a_dotfile_managers_symlink_stays_a_symlink() { + let dir = tempfile::tempdir().unwrap(); + let real = dir.path().join("dotfiles/vkBasalt.conf"); + std::fs::create_dir_all(real.parent().unwrap()).unwrap(); + std::fs::write(&real, "effects = smaa\n").unwrap(); + let path = dir.path().join("vkBasalt/vkBasalt.conf"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::os::unix::fs::symlink(&real, &path).unwrap(); + let backup = dir.path().join("state/vkBasalt.conf.user"); + let shaders = dir.path().join("reshade"); + + set_style_at(Style::Cas, &path, &backup, &shaders).unwrap(); + assert_eq!(std::fs::read_link(&path).unwrap(), real); + assert!(std::fs::read_to_string(&real).unwrap().starts_with(MARKER)); + set_style_at(Style::Own, &path, &backup, &shaders).unwrap(); + assert_eq!(std::fs::read_link(&path).unwrap(), real); + assert_eq!(std::fs::read_to_string(&real).unwrap(), "effects = smaa\n"); + } + #[test] fn without_a_file_of_its_own_ours_goes_away() { let dir = tempfile::tempdir().unwrap(); diff --git a/bigame-engine/bigame-core/src/watch.rs b/bigame-engine/bigame-core/src/watch.rs index 5a4b5d3..0da3e22 100644 --- a/bigame-engine/bigame-core/src/watch.rs +++ b/bigame-engine/bigame-core/src/watch.rs @@ -28,6 +28,8 @@ const EVENTS: u32 = libc::IN_CLOSE_WRITE | libc::IN_MOVED_TO | libc::IN_CREATE | /// A directory watch that reports when one named file changes. pub struct FileWatch { fd: i32, + /// The watched file's name in the directory. + name: Vec, } impl FileWatch { @@ -38,6 +40,7 @@ impl FileWatch { #[must_use] pub fn new(path: &Path) -> Option { let dir = path.parent()?; + let name = path.file_name()?.as_bytes().to_vec(); let mut c_dir = dir.as_os_str().as_bytes().to_vec(); c_dir.push(0); @@ -55,27 +58,67 @@ impl FileWatch { unsafe { libc::close(fd) }; return None; } - Some(Self { fd }) + Some(Self { fd, name }) } - /// Block until the kernel reports an event, then drain the queue. + /// Block until the kernel reports an event on the file, draining the + /// queue as it goes. /// /// Returns `false` when the descriptor fails, which tells the caller to /// stop watching rather than spin. /// - /// Events are not filtered by name here. Draining and letting the caller - /// re-read is simpler than parsing variable-length records, and the caller - /// compares contents anyway — a spurious wakeup costs one file read, while - /// a missed one costs a stale UI. + /// Events on the directory's other files are passed over: falcond 2.0.2 + /// writes to `/tmp`, where every program's temporary file would + /// otherwise wake the watcher for a read of an unchanged status. #[must_use] pub fn wait(&self) -> bool { // Large enough for many queued records; a short read is fine. let mut buf = [0u8; 4096]; - // SAFETY: `self.fd` is a live inotify descriptor and `buf` is a valid - // writable region of the stated length. - let n = unsafe { libc::read(self.fd, buf.as_mut_ptr().cast::(), buf.len()) }; - n > 0 + loop { + // SAFETY: `self.fd` is a live inotify descriptor and `buf` is a + // valid writable region of the stated length. + let n = + unsafe { libc::read(self.fd, buf.as_mut_ptr().cast::(), buf.len()) }; + let Ok(n) = usize::try_from(n) else { + return false; + }; + if n == 0 { + return false; + } + if concerns(&buf[..n], &self.name) { + return true; + } + } + } +} + +/// Whether the inotify records in `events` concern the file `name`, or may: +/// a queue overflow lost events, and a watch the kernel removed (the +/// directory went away) will report nothing again. +fn concerns(events: &[u8], name: &[u8]) -> bool { + // struct inotify_event: wd (i32), mask, cookie, len (u32), then `len` + // bytes of NUL-padded name. + const HEADER: usize = 16; + let mut rest = events; + while rest.len() >= HEADER { + let field = + |at: usize| u32::from_ne_bytes([rest[at], rest[at + 1], rest[at + 2], rest[at + 3]]); + let mask = field(4); + let len = usize::try_from(field(12)).unwrap_or(usize::MAX); + let Some(record) = rest.get(HEADER..HEADER.saturating_add(len)) else { + // A record cut short: say yes rather than miss a change. + return true; + }; + if mask & (libc::IN_Q_OVERFLOW | libc::IN_IGNORED) != 0 { + return true; + } + let event_name = record.split(|b| *b == 0).next().unwrap_or_default(); + if event_name == name { + return true; + } + rest = &rest[HEADER + len..]; } + false } impl Drop for FileWatch { @@ -88,8 +131,8 @@ impl Drop for FileWatch { /// Watch `path` on a background thread, sending its contents on every change. /// /// The current contents are sent immediately, then again on each change. -/// Identical consecutive contents are suppressed, because a directory watch -/// sees writes to neighbouring files too. +/// Identical consecutive contents are suppressed: a rewrite that changes +/// nothing, or the several events of one rename into place, is no change. /// /// Returns `None` when a watch could not be established. #[must_use] @@ -130,13 +173,13 @@ pub fn watch_file(path: &Path) -> Option> { // A zero-byte status file is never meaningful anyway: the next // event carries the actual data. let current = read_capped(&path).filter(|c| !c.is_empty()); - if let Some(content) = current { - if last.as_ref() != Some(&content) { - if tx.send(content.clone()).is_err() { - return; // receiver dropped - } - last = Some(content); + if let Some(content) = current + && last.as_ref() != Some(&content) + { + if tx.send(content.clone()).is_err() { + return; // receiver dropped } + last = Some(content); } if !watch.wait() { return; @@ -242,6 +285,58 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn only_the_watched_file_wakes_the_watch() { + let dir = tempdir("neighbours"); + let file = dir.join("status"); + let watch = FileWatch::new(&file).expect("watch should start"); + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(watch.wait()); + }); + std::thread::sleep(Duration::from_millis(50)); + // Another program's files in the same directory. + std::fs::write(dir.join("status.other"), "x").unwrap(); + std::fs::write(dir.join("sta"), "x").unwrap(); + std::fs::remove_file(dir.join("sta")).unwrap(); + assert!(rx.recv_timeout(Duration::from_millis(300)).is_err()); + std::fs::write(&file, "now").unwrap(); + assert_eq!(rx.recv_timeout(Duration::from_secs(5)), Ok(true)); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn inotify_records_are_matched_by_name() { + let record = |mask: u32, name: &[u8], padded: usize| { + let mut r = Vec::new(); + r.extend_from_slice(&1i32.to_ne_bytes()); + r.extend_from_slice(&mask.to_ne_bytes()); + r.extend_from_slice(&0u32.to_ne_bytes()); + r.extend_from_slice(&u32::try_from(padded).unwrap().to_ne_bytes()); + r.extend_from_slice(name); + r.resize(16 + padded, 0); + r + }; + let other = record(libc::IN_CLOSE_WRITE, b"falcond_status.tmp", 32); + let ours = record(libc::IN_MOVED_TO, b"falcond_status", 16); + assert!(!concerns(&other, b"falcond_status")); + assert!(concerns(&[other.clone(), ours].concat(), b"falcond_status")); + assert!(!concerns( + &record(libc::IN_CREATE, b"falcond", 16), + b"falcond_status" + )); + // Events lost, or the watch gone: the file may have changed. + assert!(concerns( + &record(libc::IN_Q_OVERFLOW, b"", 0), + b"falcond_status" + )); + assert!(concerns( + &record(libc::IN_IGNORED, b"", 0), + b"falcond_status" + )); + assert!(!concerns(&[], b"falcond_status")); + } + #[test] fn unchanged_contents_are_not_resent() { let dir = tempdir("dedup"); diff --git a/bigame-engine/bigame-daemon/src/backend.rs b/bigame-engine/bigame-daemon/src/backend.rs index cc24e43..5bf2914 100644 --- a/bigame-engine/bigame-daemon/src/backend.rs +++ b/bigame-engine/bigame-daemon/src/backend.rs @@ -163,12 +163,51 @@ async fn stays_active(connection: &zbus::Connection) -> anyhow::Result bool { + if enabled { + now.unit_file_state == "enabled" + && now.active_state == "active" + && now.service.as_ref().is_none_or(|s| s.restarts == 0) + } else { + now.unit_file_state == "disabled" && now.active_state == "inactive" + } +} + +/// Whether what `EnableUnitFiles`/`DisableUnitFiles` reported changed a link, +/// so systemd has to reload its unit files. An unchanged unit needs no +/// reload, and a reload is not free: it is PID 1 re-reading every unit. +fn changed(changes: &[(String, String, String)]) -> bool { + !changes.is_empty() +} + +/// Reload systemd's unit files if `changes` says a link changed. +async fn reload_if_changed( + manager: &ManagerProxy<'_>, + changes: &[(String, String, String)], +) -> zbus::Result<()> { + if changed(changes) { + manager.reload().await + } else { + Ok(()) + } +} + /// Turn the backend on or off, persistently. /// /// On: enable the unit, then start it, and see that it stays up. Off: stop it /// — which restores any game profile it holds — then disable it. The state /// systemd reports afterwards is returned so the caller can verify rather -/// than assume. +/// than assume. A unit already in the state asked for is left alone, and +/// systemd reloads its unit files only when a link actually changed: the +/// method needs no password in an active session, so calling it in a loop +/// must not keep PID 1 reloading. /// /// A unit left `failed` by an earlier run is reset first, both ways. On, so a /// fixed falcond starts at once instead of being refused for the earlier @@ -188,23 +227,30 @@ pub async fn set_enabled( if manager.get_unit_file_state(UNIT).await.is_err() { anyhow::bail!("{UNIT} is not installed"); } + // Also when nothing changes below: this is how control is taken back + // after a hand-back, by asking for the state falcond is already in. take_ownership(connection).await?; + // A build that crashed on this processor crashes again, the same way. + if enabled && bigame_core::falcond_compat::crashes_here() { + anyhow::bail!( + "this falcond build crashed on this processor (illegal instruction); \ + it is not started again until the package changes" + ); + } + let before = state(connection).await?; + if already_there(enabled, &before) { + info!(enabled, "game backend already in the state asked for"); + return Ok(before); + } + let reached = if enabled { - // A build that crashed on this processor crashes again, the same way. - if bigame_core::falcond_compat::crashes_here() { - anyhow::bail!( - "this falcond build crashed on this processor (illegal instruction); \ - it is not started again until the package changes" - ); - } - let before = state(connection).await?; if before.is_failed() { info!(failure = ?before.failure(), "starting falcond again after a failure; clearing it first"); manager.reset_failed_unit(UNIT).await?; } - manager.enable_unit_files(&[UNIT], false, false).await?; - manager.reload().await?; + let (_, changes) = manager.enable_unit_files(&[UNIT], false, false).await?; + reload_if_changed(&manager, &changes).await?; let started = match manager.start_unit(UNIT, "replace").await { Ok(_) => match settle(connection, "active").await { Ok(reached) if reached == "active" => match stays_active(connection).await { @@ -248,8 +294,8 @@ pub async fn set_enabled( // A unit left enabled after a failed start would bring Turbo // back on at the next boot, while the UI said it failed. failed => { - manager.disable_unit_files(&[UNIT], false).await?; - manager.reload().await?; + let changes = manager.disable_unit_files(&[UNIT], false).await?; + reload_if_changed(&manager, &changes).await?; match failed { Ok(reached) => reached, Err(e) => return Err(e), @@ -271,8 +317,8 @@ pub async fn set_enabled( if reached != "inactive" { anyhow::bail!("{UNIT} is still {reached}; not disabled yet"); } - manager.disable_unit_files(&[UNIT], false).await?; - manager.reload().await?; + let changes = manager.disable_unit_files(&[UNIT], false).await?; + reload_if_changed(&manager, &changes).await?; reached }; let now = state(connection).await?; @@ -294,23 +340,20 @@ pub async fn release(connection: &zbus::Connection) -> anyhow::Result { - manager.enable_unit_files(&[UNIT], false, false).await?; - } + let changes = match record.unit_file_state.as_str() { + "enabled" => manager.enable_unit_files(&[UNIT], false, false).await?.1, // Enabled until the next boot only, as it was found. - "enabled-runtime" => { - manager.enable_unit_files(&[UNIT], true, false).await?; - } - "disabled" => { - manager.disable_unit_files(&[UNIT], false).await?; + "enabled-runtime" => manager.enable_unit_files(&[UNIT], true, false).await?.1, + "disabled" => manager.disable_unit_files(&[UNIT], false).await?, + other => { + warn!( + state = other, + "prior unit file state not restorable; left as is" + ); + Vec::new() } - other => warn!( - state = other, - "prior unit file state not restorable; left as is" - ), - } - manager.reload().await?; + }; + reload_if_changed(&manager, &changes).await?; if record.was_active { manager.start_unit(UNIT, "replace").await?; } else { @@ -409,6 +452,72 @@ mod tests { assert_eq!(serde_json::from_str::(&json).unwrap(), note); } + fn unit(file: &str, active: &str, restarts: Option) -> UnitState { + UnitState { + unit_file_state: file.into(), + active_state: active.into(), + service: restarts.map(|restarts| bigame_core::systemd::ServiceRun { + result: "success".into(), + main_code: 0, + main_status: 0, + restarts, + }), + } + } + + #[test] + fn a_unit_already_as_asked_is_left_alone() { + assert!(already_there(true, &unit("enabled", "active", Some(0)))); + assert!(already_there(true, &unit("enabled", "active", None))); + assert!(already_there(false, &unit("disabled", "inactive", Some(3)))); + assert!(already_there(false, &unit("disabled", "inactive", None))); + } + + #[test] + fn anything_short_of_the_state_asked_for_takes_the_full_path() { + // On: not enabled, not running, failed, or restarted by systemd + // since it was started (possibly crashing in a loop). + for now in [ + unit("disabled", "active", Some(0)), + unit("enabled-runtime", "active", Some(0)), + unit("enabled", "inactive", Some(0)), + unit("enabled", "activating", Some(0)), + unit("enabled", "failed", Some(0)), + unit("enabled", "active", Some(2)), + unit("masked", "inactive", None), + ] { + assert!(!already_there(true, &now), "{now:?}"); + } + // Off: still enabled, still running or stopping, or failed (which + // has to be reset). + for now in [ + unit("enabled", "inactive", Some(0)), + unit("enabled-runtime", "inactive", Some(0)), + unit("disabled", "active", Some(0)), + unit("disabled", "deactivating", Some(0)), + unit("disabled", "failed", Some(0)), + unit("masked", "inactive", None), + unit("static", "inactive", None), + ] { + assert!(!already_there(false, &now), "{now:?}"); + } + } + + #[test] + fn systemd_reloads_only_when_a_link_changed() { + assert!(!changed(&[])); + assert!(changed(&[( + "symlink".into(), + "/etc/systemd/system/multi-user.target.wants/falcond.service".into(), + "/usr/lib/systemd/system/falcond.service".into(), + )])); + assert!(changed(&[( + "unlink".into(), + "/etc/systemd/system/multi-user.target.wants/falcond.service".into(), + String::new(), + )])); + } + #[test] fn the_controlled_unit_is_fixed() { // Nothing a caller sends chooses the unit; it is a constant. diff --git a/bigame-engine/bigame-daemon/src/main.rs b/bigame-engine/bigame-daemon/src/main.rs index e12d8b5..1e2899f 100644 --- a/bigame-engine/bigame-daemon/src/main.rs +++ b/bigame-engine/bigame-daemon/src/main.rs @@ -46,12 +46,18 @@ struct BiGameDaemon { impl BiGameDaemon { /// Reject the call unless Polkit authorizes this sender for `action`. + /// Polkit may prompt for a password only if the call says its sender can + /// wait for one (`ALLOW_INTERACTIVE_AUTHORIZATION` in the header). async fn authorize( &self, hdr: &zbus::message::Header<'_>, action: &str, ) -> Result<(), zbus::fdo::Error> { - polkit::check(&self.connection, hdr.sender(), action).await + let interactive = hdr + .primary() + .flags() + .contains(zbus::message::Flags::AllowInteractiveAuth); + polkit::check(&self.connection, hdr.sender(), action, interactive).await } } @@ -75,6 +81,8 @@ impl BiGameDaemon { validate::profile_name(name).map_err(invalid)?; validate::profile_payload(payload).map_err(invalid)?; validate::profile_name_matches(name, payload).map_err(invalid)?; + validate::not_a_root_process(name, &validate::root_process_names(Path::new("/proc"))) + .map_err(invalid)?; let dir = Path::new(USER_PROFILES_DIR); std::fs::create_dir_all(dir) @@ -405,7 +413,12 @@ fn find_vcache_attribute() -> Option { .find(|p| p.exists()) } -#[tokio::main] +// Four worker threads, not tokio's default of one per CPU: with that, the +// helper's thread count followed the machine (17 tasks on 16 threads), and +// the unit's `TasksMax=64` would stop it starting on a 64-thread processor. +// Writing calls serialise on one lock anyway, and every wait — Polkit, a +// password prompt, systemd settling — is asynchronous. +#[tokio::main(worker_threads = 4)] async fn main() -> Result<()> { // Under systemd the output is the journal, which timestamps every line // itself and shows colour codes as `[2m…[0m` in `journalctl`. diff --git a/bigame-engine/bigame-daemon/src/polkit.rs b/bigame-engine/bigame-daemon/src/polkit.rs index acfd9a6..af8eb4c 100644 --- a/bigame-engine/bigame-daemon/src/polkit.rs +++ b/bigame-engine/bigame-daemon/src/polkit.rs @@ -49,11 +49,25 @@ trait Authority { ) -> zbus::Result<(bool, bool, HashMap)>; } -/// Allow Polkit to prompt the user. The helper is invoked from an interactive -/// desktop application, so an authentication dialog is the expected behaviour -/// rather than an outright denial. +/// `CheckAuthorization` flag: Polkit may ask the user to authenticate. const ALLOW_USER_INTERACTION: u32 = 1; +/// The `CheckAuthorization` flags for a call whose header did, or did not, +/// carry D-Bus's `ALLOW_INTERACTIVE_AUTHORIZATION` flag. +/// +/// The caller says whether it can wait for a password prompt: Big Game Mode's +/// own client sets the flag on every call, and `busctl` sets it by default. A +/// caller that did not set it gets Polkit's answer without a dialog — its +/// cached or `yes` authorizations still pass — instead of a prompt it never +/// asked for appearing on the user's screen. +fn polkit_flags(interactive: bool) -> u32 { + if interactive { + ALLOW_USER_INTERACTION + } else { + 0 + } +} + /// Authorize `sender` for `action`, or return the D-Bus error to reply with. /// /// # Errors @@ -66,6 +80,7 @@ pub async fn check( connection: &zbus::Connection, sender: Option<&zbus::names::UniqueName<'_>>, action: &str, + interactive: bool, ) -> Result<(), zbus::fdo::Error> { let Some(sender) = sender else { tracing::warn!(action, "refusing a request with no identifiable sender"); @@ -87,7 +102,7 @@ pub async fn check( &("system-bus-name", subject_details), action, HashMap::new(), - ALLOW_USER_INTERACTION, + polkit_flags(interactive), "", ) .await @@ -106,3 +121,14 @@ pub async fn check( ))) } } + +#[cfg(test)] +mod tests { + use super::polkit_flags; + + #[test] + fn polkit_prompts_only_a_caller_that_can_wait_for_it() { + assert_eq!(polkit_flags(true), 1); + assert_eq!(polkit_flags(false), 0); + } +} diff --git a/bigame-engine/bigame-daemon/src/validate.rs b/bigame-engine/bigame-daemon/src/validate.rs index d5cbbd2..4678b59 100644 --- a/bigame-engine/bigame-daemon/src/validate.rs +++ b/bigame-engine/bigame-daemon/src/validate.rs @@ -14,8 +14,9 @@ const MAX_PROFILE_NAME: usize = 128; /// Largest accepted configuration or profile payload (64 KiB). /// -/// Real profiles are a few hundred bytes; this only exists so an unauthenticated -/// bus peer cannot make the helper write an unbounded file. +/// Real profiles are a few hundred bytes. An unauthenticated peer never gets +/// this far — authorization comes first — so this bounds what an authorized +/// caller can make the helper write into a file falcond then parses as root. const MAX_PAYLOAD: usize = 64 * 1024; /// Validate a per-game profile name. @@ -110,31 +111,268 @@ const CONFIG_KEYS: &[&str] = &[ ]; /// Processes no game profile may name: falcond would apply a game's -/// performance mode and scheduler to the session itself. +/// performance mode, scheduler and the rest to the system itself. Exact +/// names, compared without case as falcond compares them, so no game title +/// is caught by a prefix. Kept to programs that are never a game: the +/// service manager and its daemons, the bus and Polkit, login and privilege +/// programs, display servers and compositors, the sound server, network +/// and hardware daemons, shells, and Big Game Mode with what it drives. +/// +/// falcond matches a profile against the name it takes from the command +/// line and against `/proc//comm`, which the kernel cuts to 15 bytes, +/// so a name is also refused when it is that cut form of an entry here +/// (`systemd-journal` for `systemd-journald`). const SYSTEM_PROCESSES: &[&str] = &[ + // Service manager and its own daemons. + "systemd", + "init", + "(sd-pam)", + "systemd-executor", + "systemd-journald", + "systemd-logind", + "systemd-udevd", + "udevd", + "systemd-resolved", + "systemd-networkd", + "systemd-timesyncd", + "systemd-oomd", + "systemd-homed", + "systemd-userdbd", + "systemd-userwork", + "systemd-machined", + "systemd-swap", + // Bus, Polkit, accounts. + "dbus-daemon", + "dbus-broker", + "dbus-broker-launch", + "polkitd", + "polkit-agent-helper-1", + "accounts-daemon", + "rtkit-daemon", + // Logging in and gaining privileges. + "login", + "agetty", + "getty", + "sshd", + "sshd-session", + "sshd-auth", + "sudo", + "su", + "pkexec", + "doas", + "run0", + "passwd", + "unix_chkpwd", + "sddm", + "sddm-helper", + "gdm", + "gdm-session-worker", + "lightdm", + "greetd", + // Display servers, compositors and the desktop shell. "xorg", "xwayland", "kwin_wayland", + "kwin_wayland_wrapper", "kwin_x11", "gnome-shell", "plasmashell", - "systemd", - "init", - "sddm", - "gdm", - "dbus-daemon", - "dbus-broker", - "polkitd", + "ksmserver", + "kded5", + "kded6", + "gamescope", + "gamescope-wl", + "gamescopereaper", + "sway", + "hyprland", + "labwc", + "wayfire", + "niri", + "weston", + "cosmic-comp", + "xfwm4", + "openbox", + "picom", + // Sound. "pipewire", + "pipewire-pulse", "wireplumber", + "pulseaudio", + "jackd", + "jackdbus", + // Network, devices, power, scheduling. + "networkmanager", + "nm-dispatcher", + "wpa_supplicant", + "iwd", + "dhcpcd", + "dhclient", + "connmand", + "modemmanager", + "bluetoothd", + "avahi-daemon", + "firewalld", + "chronyd", + "ntpd", + "crond", + "cron", + "atd", + "cupsd", + "udisksd", + "upowerd", + "colord", + "fwupd", + "boltd", + "switcheroo-control", + "power-profiles-daemon", + "tuned", + "tuned-ppd", + "thermald", + "irqbalance", + "ananicy-cpp", + "earlyoom", + "nvidia-persistenced", + "nvidia-powerd", + "packagekitd", + "pamac-daemon", + "scx_loader", + "auditd", + "rsyslogd", + "syslog-ng", + "dockerd", + "containerd", + "libvirtd", + "kthreadd", + // Shells: every script in the session, root's included, runs in one. + "sh", + "bash", + "zsh", + "fish", + "dash", + "ksh", + "mksh", + "tcsh", + "csh", + "busybox", + // Big Game Mode and what it drives. "falcond", + "gamemoded", "bigame-daemon", "bigame-ui", - "sudo", - "sh", - "bash", ]; +/// The length the kernel cuts `/proc//comm` to. +const COMM_LEN: usize = 15; + +/// Whether `name` is one of `names`, or the `comm` form of one: what +/// falcond would match a process of that name by. +fn names_one_of<'a>(name: &str, names: impl IntoIterator) -> bool { + let name = name.to_ascii_lowercase(); + names.into_iter().any(|known| { + let known = known.to_ascii_lowercase(); + known == name || (name.len() == COMM_LEN && known.starts_with(&name)) + }) +} + +/// The names falcond could match each process running as root by, read +/// from `proc` (normally `/proc`): its `comm`, and the name falcond takes +/// from its command line. Lowercase. +/// +/// Best effort, on top of [`SYSTEM_PROCESSES`]: a process started after the +/// call is not seen, and under the unit's `ProtectProc=invisible` the helper +/// (uid 0 with no `CAP_SYS_PTRACE`) sees only root processes whose ids are +/// all 0 and that have not made themselves non-dumpable — which is most +/// root services, and every kernel thread. Unreadable entries are skipped: +/// the deny-list above is the guarantee, this only widens it. +#[must_use] +pub fn root_process_names(proc: &std::path::Path) -> std::collections::HashSet { + let mut names = std::collections::HashSet::new(); + let Ok(entries) = std::fs::read_dir(proc) else { + return names; + }; + for entry in entries.flatten() { + let dir = entry.path(); + let is_pid = entry + .file_name() + .to_str() + .is_some_and(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())); + if !is_pid { + continue; + } + let Ok(status) = std::fs::read_to_string(dir.join("status")) else { + continue; + }; + if !runs_as_root(&status) { + continue; + } + if let Ok(comm) = std::fs::read_to_string(dir.join("comm")) { + let comm = comm.trim_end_matches('\n'); + if !comm.is_empty() { + names.insert(comm.to_ascii_lowercase()); + } + } + if let Ok(cmdline) = std::fs::read(dir.join("cmdline")) { + let cmdline = String::from_utf8_lossy(&cmdline); + let name = name_from_cmdline(&cmdline); + if !name.is_empty() { + names.insert(name.to_ascii_lowercase()); + } + } + } + names +} + +/// Whether a `/proc//status` text has a real or effective uid of 0. +fn runs_as_root(status: &str) -> bool { + status + .lines() + .find_map(|l| l.strip_prefix("Uid:")) + .is_some_and(|uids| uids.split_whitespace().take(2).any(|u| u == "0")) +} + +/// The process name falcond 2.0.x takes from a NUL-separated command line +/// (`selectProcessNameFromCmdline`): the base name, after `/` or a backslash, of +/// the first argument that ends in `.exe`, or else of the first non-empty +/// one. +fn name_from_cmdline(cmdline: &str) -> &str { + let mut fallback = ""; + for arg in cmdline.split('\0').filter(|a| !a.is_empty()) { + let base = arg.rsplit(['/', '\\']).next().unwrap_or(arg); + if base.is_empty() { + continue; + } + if fallback.is_empty() { + fallback = base; + } + if base.len() >= 4 + && base.is_char_boundary(base.len() - 4) + && base[base.len() - 4..].eq_ignore_ascii_case(".exe") + { + return base; + } + } + fallback +} + +/// Refuse a profile name falcond would match against a process running as +/// root now, whatever it is called: one in `root_names`, as +/// [`root_process_names`] read them. +/// +/// # Errors +/// Returns an error naming the clash. +pub fn not_a_root_process( + name: &str, + root_names: &std::collections::HashSet, +) -> Result<(), String> { + if names_one_of(name, root_names.iter().map(String::as_str)) { + Err(format!( + "{name:?} is the name of a process running as root, not a game: a profile may not name it" + )) + } else { + Ok(()) + } +} + /// One `key = value` assignment of falcond's configuration format, read /// strictly: the key, then the value with its quotes removed. /// @@ -384,7 +622,7 @@ pub fn profile_name_matches(name: &str, payload: &str) -> Result<(), String> { let field = assignments(payload, PROFILE_KEYS, false)? .into_iter() .find_map(|(k, v)| (k == "name").then(|| v.to_owned())); - if SYSTEM_PROCESSES.contains(&name.to_ascii_lowercase().as_str()) { + if names_one_of(name, SYSTEM_PROCESSES.iter().copied()) { return Err(format!( "{name:?} is a system process, not a game: a profile may not name it" )); @@ -444,9 +682,114 @@ mod tests { #[test] fn a_profile_may_not_name_the_session_itself() { use super::profile_name_matches; - assert!(profile_name_matches("Xorg", "name = \"Xorg\"\n").is_err()); - assert!(profile_name_matches("kwin_wayland", "name = \"kwin_wayland\"\n").is_err()); - assert!(profile_name_matches("vkcube", "name = \"vkcube\"\n").is_ok()); + let check = |name: &str| profile_name_matches(name, &format!("name = \"{name}\"\n")); + for name in [ + "Xorg", + "kwin_wayland", + "systemd-logind", + "NetworkManager", + "sshd", + "zsh", + "fish", + "gamescope", + "Hyprland", + // `comm` cuts names to 15 bytes, and falcond matches that too. + "systemd-journal", + "systemd-journald", + "kwin_wayland_wr", + "dbus-broker-lau", + ] { + assert!(check(name).is_err(), "{name}"); + } + // Real games, and names that merely start like a system process. + for name in [ + "vkcube", + "Cyberpunk2077.exe", + "X4", + "sshd-simulator", + "Gamescope Demo.exe", + "systemd-journa", + "SOTTR.exe", + "factorio", + "Proton", + ] { + assert!(check(name).is_ok(), "{name}: {:?}", check(name)); + } + } + + #[test] + fn falcond_reads_the_process_name_from_the_command_line_as_here() { + use super::name_from_cmdline; + // falcond's own test cases (scanner.zig). + assert_eq!( + name_from_cmdline( + "/usr/bin/wine64\0C:\\Program Files\\Cyberpunk 2077\\bin\\x64\\Cyberpunk2077.exe\0--fullscreen\0" + ), + "Cyberpunk2077.exe" + ); + assert_eq!( + name_from_cmdline("/usr/bin/umu-run\0--gameid\x001234\0"), + "umu-run" + ); + assert_eq!( + name_from_cmdline("/usr/lib/systemd/systemd-logind\0"), + "systemd-logind" + ); + assert_eq!(name_from_cmdline("\0\0"), ""); + assert_eq!(name_from_cmdline("GAME.EXE\0"), "GAME.EXE"); + } + + #[test] + fn a_profile_may_not_name_a_process_running_as_root() { + use super::{not_a_root_process, root_process_names}; + // A /proc of three processes: a root daemon, a root process started + // through wine, and the user's game. + let proc = + std::env::temp_dir().join(format!("bigame-daemon-test-proc-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&proc); + let process = |pid: u32, uid: u32, comm: &str, cmdline: &str| { + let dir = proc.join(pid.to_string()); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("status"), + format!("Name:\t{comm}\nUid:\t{uid}\t{uid}\t{uid}\t{uid}\n"), + ) + .unwrap(); + std::fs::write(dir.join("comm"), format!("{comm}\n")).unwrap(); + std::fs::write(dir.join("cmdline"), cmdline).unwrap(); + }; + process(1, 0, "init-daemon", "/usr/lib/init-daemon\0--system\0"); + process( + 40, + 0, + "RootTool.exe", + "/usr/bin/wine\0C:\\tools\\RootTool.exe\0", + ); + process(41, 0, "very-long-servi", "/usr/bin/very-long-service\0"); + process( + 500, + 1000, + "Cyberpunk2077.e", + "/usr/bin/wine64\0Z:\\games\\Cyberpunk2077.exe\0", + ); + // Not a process directory. + std::fs::create_dir_all(proc.join("sys")).unwrap(); + + let root = root_process_names(&proc); + std::fs::remove_dir_all(&proc).unwrap(); + for name in [ + "init-daemon", + "INIT-DAEMON", + "RootTool.exe", + "very-long-servi", + "very-long-service", + ] { + assert!(not_a_root_process(name, &root).is_err(), "{name}"); + } + assert!(not_a_root_process("Cyberpunk2077.exe", &root).is_ok()); + assert!(not_a_root_process("vkcube", &root).is_ok()); + // An unreadable /proc refuses nothing: the deny-list still holds. + assert!(root_process_names(std::path::Path::new("/nonexistent/proc")).is_empty()); } #[test] diff --git a/bigame-engine/bigame-ui/src/app.rs b/bigame-engine/bigame-ui/src/app.rs index cf8ef0a..1a6bec7 100644 --- a/bigame-engine/bigame-ui/src/app.rs +++ b/bigame-engine/bigame-ui/src/app.rs @@ -404,6 +404,50 @@ fn follow_turbo(app: &adw::Application, tray_handle: &std::rc::Rc)>; + +thread_local! { + /// What else follows the status loop's readings of falcond's unit. + static UNIT_LISTENERS: std::cell::RefCell> = + const { std::cell::RefCell::new(Vec::new()) }; +} + +/// Be given every reading of Turbo's backend unit the status loop takes: +/// every ten seconds, with the window hidden too. Home follows Turbo +/// changed elsewhere from the very reading the tray's warning comes from, +/// so systemd is asked once per tick, not once per surface. +pub(crate) fn follow_backend_unit( + listener: impl Fn(Option<&bigame_core::systemd::UnitState>) + 'static, +) { + UNIT_LISTENERS.with(|l| l.borrow_mut().push(Box::new(listener))); +} + +/// What the status loop reads on its worker. +struct StatusReading { + unit: Option, + missing_runtime: Vec, + /// How to install `missing_runtime`, worked out on the worker: asking + /// whether the repositories have them runs pacman. + install: Option>, + install_shell_command: Option, +} + +impl StatusReading { + fn read() -> Self { + let unit = bigame_core::systemd::Reader::shared() + .and_then(|r| r.unit_state(bigame_core::turbo::BACKEND_UNIT)); + let missing_runtime = detect_missing_runtime_packages(); + Self { + unit, + install: install_missing_packages_action(&missing_runtime), + install_shell_command: install_missing_packages_shell_command(&missing_runtime), + missing_runtime, + } + } +} + /// Keep the tray's warning and the error indicator in step with the real /// state. fn start_status_loop( @@ -430,20 +474,15 @@ fn start_status_loop( std::sync::Arc::clone(&error_indicator), ); glib::spawn_future_local(async move { - let reading = gtk4::gio::spawn_blocking(|| { - let unit = bigame_core::systemd::Reader::shared() - .and_then(|r| r.unit_state(bigame_core::turbo::BACKEND_UNIT)); - (unit, detect_missing_runtime_packages()) - }) - .await; + let reading = gtk4::gio::spawn_blocking(StatusReading::read).await; busy.set(false); - if let Ok((unit, missing_runtime)) = reading { - show_status( - unit.as_ref(), - &missing_runtime, - &tray_handle, - &error_indicator, - ); + if let Ok(reading) = reading { + show_status(&reading, &tray_handle, &error_indicator); + UNIT_LISTENERS.with(|l| { + for listener in l.borrow().iter() { + listener(reading.unit.as_ref()); + } + }); } }); } @@ -462,11 +501,12 @@ fn start_status_loop( /// Put one reading on the tray and the error indicator. fn show_status( - unit: Option<&bigame_core::systemd::UnitState>, - missing_runtime: &[String], + reading: &StatusReading, tray_handle: &tray::TrayHandle, error_indicator: &crate::widgets::error_indicator::ErrorIndicator, ) { + let unit = reading.unit.as_ref(); + let missing_runtime = &reading.missing_runtime; let backend_failed = unit.is_some_and(bigame_core::systemd::UnitState::is_failed); let failure = unit.and_then(bigame_core::systemd::UnitState::failure); let warning = if let (true, Some(failure)) = (backend_failed, failure) { @@ -488,13 +528,17 @@ fn show_status( Some(i18n("falcond stopped unexpectedly")) } else if !missing_runtime.is_empty() { let missing_csv = missing_runtime.join(", "); - let action = install_missing_packages_action(missing_runtime); + let action = reading.install.clone(); // The confirmation shows the very command the button runs. - let install_hint = - install_missing_packages_hint(missing_runtime, action.as_deref().map(|a| a.join(" "))); + let install_hint = install_missing_packages_hint( + missing_runtime, + action + .as_deref() + .map(|a| a.join(" ")) + .or_else(|| reading.install_shell_command.clone()), + ); if let Some(cmd) = action { - let copy_cmd = - install_missing_packages_shell_command(missing_runtime).unwrap_or_default(); + let copy_cmd = reading.install_shell_command.clone().unwrap_or_default(); error_indicator.set_error_with_action_and_copy( &i18n("Missing Runtime Dependencies"), &format!( @@ -544,9 +588,11 @@ fn detect_missing_runtime_packages() -> Vec { missing } +/// What to do about the missing packages; `runs` is the command that +/// installs them here, if one does. #[must_use] fn install_missing_packages_hint(missing: &[String], runs: Option) -> String { - if let Some(cmd) = runs.or_else(|| install_missing_packages_shell_command(missing)) { + if let Some(cmd) = runs { return format!( "{}\n1) {}\n2) {}\n3) {}\n\n{}\n{}", i18n("Troubleshooting"), @@ -631,9 +677,9 @@ fn show_about_dialog(app: &adw::Application) { .application_name(NAME) .application_icon(APP_ID) .version(env!("CARGO_PKG_VERSION")) - // The line under the name: what the product is. The people who make - // it are in the credits. - .developer_name(i18n("BigLinux's game mode.")) + // The line under the name, as AppStream's names it: a + // person's name, not translated. + .developer_name("Rafael Ruscher") .website(WEBSITE) .issue_url(format!("{WEBSITE}/issues")) .license_type(gtk4::License::Gpl30) diff --git a/bigame-engine/bigame-ui/src/game_watch.rs b/bigame-engine/bigame-ui/src/game_watch.rs index c02b1e2..30fea1d 100644 --- a/bigame-engine/bigame-ui/src/game_watch.rs +++ b/bigame-engine/bigame-ui/src/game_watch.rs @@ -22,6 +22,12 @@ use bigame_core::running::GameIdentity; /// How often to look when nothing has signalled a change. const POLL: std::time::Duration = std::time::Duration::from_secs(5); +/// How often to look while a game is known: the game should have the +/// machine, a look walks `/proc` and asks what the game renders with, and +/// its exit is mostly caught at once by falcond dropping its profile. A +/// game falcond has no profile for is seen gone up to this much later. +const POLL_PLAYING: std::time::Duration = std::time::Duration::from_secs(15); + /// Returns [`glib::ControlFlow::Break`] to stop listening. type Listener = Box) -> glib::ControlFlow>; @@ -57,14 +63,24 @@ pub fn start() { }); *watch.monitor.borrow_mut() = Some(monitor); } - glib::timeout_add_local(POLL, || { - check(); - glib::ControlFlow::Continue - }); + schedule(); glib::idle_add_local_once(check); }); } +/// Look again after [`POLL`], or [`POLL_PLAYING`] while a game is known. +fn schedule() { + let wait = if current().is_some() { + POLL_PLAYING + } else { + POLL + }; + glib::timeout_add_local_once(wait, || { + check(); + schedule(); + }); +} + /// Look now. pub fn check() { WATCH.with(|watch| { @@ -113,10 +129,11 @@ fn merge( current: Option<&GameIdentity>, mut found: Option, ) -> (Option, bool) { - if let (Some(cur), Some(new)) = (current, found.as_mut()) { - if cur.pid == new.pid && new.render_card.is_none() { - new.render_card.clone_from(&cur.render_card); - } + if let (Some(cur), Some(new)) = (current, found.as_mut()) + && cur.pid == new.pid + && new.render_card.is_none() + { + new.render_card.clone_from(&cur.render_card); } let key = |g: Option<&GameIdentity>| g.map(|g| (g.pid, g.graphics, g.render_card.clone())); let changed = key(current) != key(found.as_ref()); diff --git a/bigame-engine/bigame-ui/src/i18n.rs b/bigame-engine/bigame-ui/src/i18n.rs index 9680340..4825f97 100644 --- a/bigame-engine/bigame-ui/src/i18n.rs +++ b/bigame-engine/bigame-ui/src/i18n.rs @@ -28,11 +28,7 @@ pub fn init() { /// Mark a string for translation without translating it here: for labels /// kept in constants and passed to [`i18n`] where they are shown. -#[allow(non_snake_case)] -#[must_use] -pub const fn N_(s: &'static str) -> &'static str { - s -} +pub use bigame_core::text::N_; /// Translate a string via gettext. #[must_use] @@ -47,6 +43,13 @@ pub fn ni18n(singular: &str, plural: &str, n: usize) -> String { ngettext(singular, plural, u32::try_from(n).unwrap_or(u32::MAX)).replace("%n", &n.to_string()) } +/// "label: value" as the language writes it: French puts a space before the +/// colon, Chinese uses a full-width one. +#[must_use] +pub fn labelled(label: &str, value: &str) -> String { + bigame_core::text::Text::fill(&i18n("%s: %s"), &[label.to_owned(), value.to_owned()]) +} + /// A sentence from bigame-core, translated: its template and every /// translatable value in it through gettext. #[must_use] diff --git a/bigame-engine/bigame-ui/src/profile_offer.rs b/bigame-engine/bigame-ui/src/profile_offer.rs index 99600c5..6e883bd 100644 --- a/bigame-engine/bigame-ui/src/profile_offer.rs +++ b/bigame-engine/bigame-ui/src/profile_offer.rs @@ -163,22 +163,22 @@ pub fn install(app: &adw::Application) { fn game_changed(app: &adw::Application, game: Option<&GameIdentity>) { let Some(game) = game.cloned() else { app.withdraw_notification(NOTIFICATION_ID); - if let Some(name) = LAST_GAME.with(|g| g.borrow_mut().take()) { - if crate::settings::load().notifications_enabled { - // Only Turbo changes anything for a game: with it off there - // is nothing that was put back to announce. - let app = app.clone(); - glib::spawn_future_local(async move { - if !gio::spawn_blocking(turbo_is_on).await.unwrap_or(false) { - return; - } - let n = gio::Notification::new(&i18n("%s closed").replace("%s", &name)); - n.set_body(Some(&i18n( - "Everything the game's profile changed has been put back.", - ))); - app.send_notification(Some("game-exit"), &n); - }); - } + if let Some(name) = LAST_GAME.with(|g| g.borrow_mut().take()) + && crate::settings::load().notifications_enabled + { + // Only Turbo changes anything for a game: with it off there + // is nothing that was put back to announce. + let app = app.clone(); + glib::spawn_future_local(async move { + if !gio::spawn_blocking(turbo_is_on).await.unwrap_or(false) { + return; + } + let n = gio::Notification::new(&i18n("%s closed").replace("%s", &name)); + n.set_body(Some(&i18n( + "Everything the game's profile changed has been put back.", + ))); + app.send_notification(Some("game-exit"), &n); + }); } return; }; @@ -504,16 +504,15 @@ fn present_review( let app = app.clone(); let anchor = window.clone(); dialog.connect_response(None, move |_, response| { - if never.is_active() { - if let Err(e) = decline_forever(&process) { - if let Some(w) = &anchor { - crate::widgets::toast::error( - w, - &i18n("Could not save: %s").replace("%s", &e.to_string()), - "", - ); - } - } + if never.is_active() + && let Err(e) = decline_forever(&process) + && let Some(w) = &anchor + { + crate::widgets::toast::error( + w, + &i18n("Could not save: %s").replace("%s", &e.to_string()), + "", + ); } if response == "create" { create_for(&app, &process); diff --git a/bigame-engine/bigame-ui/src/views/ai_graphics.rs b/bigame-engine/bigame-ui/src/views/ai_graphics.rs index 70e1575..66a2dfa 100644 --- a/bigame-engine/bigame-ui/src/views/ai_graphics.rs +++ b/bigame-engine/bigame-ui/src/views/ai_graphics.rs @@ -28,10 +28,11 @@ use bigame_core::graphics::versions::Offer; use bigame_core::graphics::{self, Analysis, ChoiceState, Target, backend, diagnose, external}; use bigame_core::optimization::Feature; use bigame_core::overview::State; +use bigame_core::text::Text; -use crate::i18n::{error_text, i18n, ni18n, tr}; +use crate::i18n::{error_text, i18n, labelled, ni18n, tr}; use crate::widgets::info::{self, Entry}; -use crate::widgets::notice::{self, Kind, Notice}; +use crate::widgets::notice::{self, Kind, Notice, sentence}; use crate::widgets::status::Chip; struct Page { @@ -82,6 +83,8 @@ struct Page { status: Chip, /// Set while the page moves a control itself. quiet: Cell, + /// Set while a change to the game runs ([`begin`]). + working: Cell, /// Counts analyses, so only the latest one is shown. generation: Cell, /// The upscaler was changed since the last apply. @@ -142,23 +145,20 @@ pub fn status_text(s: &Status) -> String { upscaler_name(upscaler, *fsr_generation) ), Status::NotDetected => i18n("Installed, but the game did not load it"), - Status::Failed { errors } => format!( - "{}: {}", - i18n("Failed"), - errors.first().cloned().unwrap_or_default() + Status::Failed { errors } => labelled( + &i18n("Failed"), + &errors.first().cloned().unwrap_or_default(), ), Status::SettingsLeft => i18n( "Files put back; the game's own setting is still to be put back — press Restore again", ), - Status::Moved { installed_in } => format!( - "{}: {}", - i18n("Installed in another folder"), - installed_in.display() + Status::Moved { installed_in } => labelled( + &i18n("Installed in another folder"), + &installed_in.display().to_string(), ), - Status::Unreadable { error } => format!( - "{}: {}", - i18n("The record of what was installed cannot be read"), - tr(error) + Status::Unreadable { error } => labelled( + &i18n("The record of what was installed cannot be read"), + &tr(error), ), } } @@ -224,15 +224,6 @@ fn wrap_title(group: &adw::PreferencesGroup) { visit(group.upcast_ref::()); } -/// Start a sentence with a capital: core writes steps as clauses ("choose -/// `XeSS` in the game's menu"), and a row title reads as a sentence. -fn sentence(s: &str) -> String { - let mut c = s.chars(); - c.next() - .map(|f| f.to_uppercase().chain(c).collect()) - .unwrap_or_default() -} - fn row(title: &str, subtitle: &str) -> adw::ActionRow { adw::ActionRow::builder() .title(title) @@ -615,9 +606,11 @@ fn render_facts(page: &Rc, a: &Analysis) { .unwrap_or_default(); facts.push(Fact { title: i18n("How the game starts"), - value: i18n("%s starts %s") - .replacen("%s", &stub.display().to_string(), 1) - .replacen("%s", &name, 1), + // A translation may name the game first (%2$s … %1$s). + value: Text::fill( + &i18n("%s starts %s"), + &[stub.display().to_string(), name], + ), info: Some(i18n( "The launcher starts a small Unreal Engine bootstrap, which starts the real game. Big Game Mode looks at the game that runs — its folder is where OptiScaler would go, and its executable says which upscalers the game has — and knows it as running by either name.", )), @@ -758,9 +751,8 @@ fn api_info(r: &Report) -> String { if !evidence.is_empty() { let _ = write!( s, - "\n\n{}: {}", - i18n("How it is known"), - evidence.join(" · ") + "\n\n{}", + labelled(&i18n("How it is known"), &evidence.join(" · ")) ); } s @@ -827,7 +819,7 @@ fn changes(before: &[(String, String)], after: &[(String, String)]) -> Vec (String, &'static str, String) { i18n("Why it is unavailable here:"), missing .iter() - .map(|m| format!("• {}: {}", i18n(m.what), tr(&m.detail))) + .map(|m| format!("• {}", labelled(&i18n(m.what), &tr(&m.detail)))) .collect::>() .join("\n"), i18n( @@ -1495,21 +1487,25 @@ fn found_expander(r: &Report) -> adw::ExpanderRow { details.add_row(&row( "Proton", &format!( - "{} · Windows {} · {}: {} · {}: {}", + "{} · Windows {} · {} · {}", p.tool.clone().unwrap_or_else(|| i18n("unknown build")), p.windows_version.clone().unwrap_or_else(|| "?".into()), - i18n("FSR 4 provider"), - if p.fsr4_provider { - i18n("yes") - } else { - i18n("no") - }, - i18n("AMD HIP runtime"), - if p.hip_runtime { - i18n("yes") - } else { - i18n("no") - }, + labelled( + &i18n("FSR 4 provider"), + &if p.fsr4_provider { + i18n("yes") + } else { + i18n("no") + } + ), + labelled( + &i18n("AMD HIP runtime"), + &if p.hip_runtime { + i18n("yes") + } else { + i18n("no") + } + ), ), )); } @@ -1587,10 +1583,9 @@ fn save_report(page: &Rc, button: >k4::Button, row: &adw::ActionRow) { }); page.overlay.add_toast(toast); } - Ok(Err(e)) => page.overlay.add_toast(adw::Toast::new(&format!( - "{}: {}", - i18n("Could not write the report"), - error_text(&e) + Ok(Err(e)) => page.overlay.add_toast(adw::Toast::new(&labelled( + &i18n("Could not write the report"), + &error_text(&e), ))), Err(_) => page .overlay @@ -1908,10 +1903,9 @@ fn wire_choice(page: &Rc) { Ok(()) => page2.overlay.add_toast(adw::Toast::new(&i18n( "lsfg-vk was turned off for this game", ))), - Err(e) => page2.overlay.add_toast(adw::Toast::new(&format!( - "{}: {}", - i18n("Could not turn lsfg-vk off"), - error_text(&e) + Err(e) => page2.overlay.add_toast(adw::Toast::new(&labelled( + &i18n("Could not turn lsfg-vk off"), + &error_text(&e), ))), } choose_frame_generation(&page2, true); @@ -1970,10 +1964,9 @@ fn wire_choice(page: &Rc) { Ok(()) => page.overlay.add_toast(adw::Toast::new(&i18n( "lsfg-vk was turned off for this game", ))), - Err(e) => page.overlay.add_toast(adw::Toast::new(&format!( - "{}: {}", - i18n("Could not turn lsfg-vk off"), - error_text(&e) + Err(e) => page.overlay.add_toast(adw::Toast::new(&labelled( + &i18n("Could not turn lsfg-vk off"), + &error_text(&e), ))), } refresh(&page); @@ -1994,23 +1987,24 @@ fn wire_choice(page: &Rc) { let page = Rc::clone(page); c.setting_off .add_action(&i18n("Switch it back on"), true, move |_| { + if !begin(&page, &i18n("Writing the game's settings…")) { + return; + } let page = Rc::clone(&page); glib::spawn_future_local(async move { if refuse_while_running(&page, &page.overlay).await { + end(&page); return; } - busy(&page, Some(&i18n("Writing the game's settings…"))); let target = page.target.clone(); let result = gio::spawn_blocking(move || { graphics::switch_game_setting_on_again(&target) }) .await; - busy(&page, None); + end(&page); let text = match result { Ok(Ok(_)) => i18n("Switched back on in the game's settings"), - Ok(Err(e)) => { - format!("{}: {}", i18n("Nothing was changed"), error_text(&e)) - } + Ok(Err(e)) => labelled(&i18n("Nothing was changed"), &error_text(&e)), Err(_) => i18n("Nothing was changed"), }; page.overlay.add_toast(adw::Toast::new(&text)); @@ -2038,7 +2032,7 @@ fn wire_choice(page: &Rc) { Ok(Ok(bigame_core::steam_gamescope::Applied::Written(o))) => { i18n("Steam launch options: %s").replace("%s", &o) } - Ok(Err(e)) => format!("{}: {}", i18n("Nothing was changed"), error_text(&e)), + Ok(Err(e)) => labelled(&i18n("Nothing was changed"), &error_text(&e)), Ok(Ok(_)) | Err(_) => i18n("Nothing was changed"), }; page.overlay.add_toast(adw::Toast::new(&text)); @@ -2104,7 +2098,17 @@ fn choose_frame_generation(page: &Rc, on: bool) { refresh(page); } +/// Show the page as busy with `text`, or not busy. While a change to the +/// game runs, only [`end`] ends it: an analysis started meanwhile (a choice +/// changed during a download) would make the buttons usable again +/// mid-change. fn busy(page: &Page, text: Option<&str>) { + if !page.working.get() { + show_busy(page, text); + } +} + +fn show_busy(page: &Page, text: Option<&str>) { let on = text.is_some(); page.spinner.set_visible(on); page.spinner.set_spinning(on); @@ -2120,6 +2124,29 @@ fn busy(page: &Page, text: Option<&str>) { ] { b.set_sensitive(!on); } + // Update and Go back change the game's files too. + if let Some(versions) = page.versions.borrow().as_ref() { + versions.set_sensitive(!on); + } +} + +/// Start a change to the game's files, showing `text`. Called in the click +/// itself, before anything is awaited: the buttons are grey before a second +/// click can start a second run beside the first. `false` while another +/// change runs. +fn begin(page: &Page, text: &str) -> bool { + if page.working.get() { + return false; + } + show_busy(page, Some(text)); + page.working.set(true); + true +} + +/// The change [`begin`] started is over. +fn end(page: &Page) { + page.working.set(false); + show_busy(page, None); } /// *Check Again*: read the game's folder, the GPU, the running game and @@ -2270,12 +2297,15 @@ fn render_versions(page: &Rc, offer: &Offer) { /// Update to `to`, or go back to the previous version (`None`). fn change_version(page: &Rc, to: Option) { + if !begin(page, &i18n("Downloading, checking and installing…")) { + return; + } let page = page.clone(); glib::spawn_future_local(async move { if refuse_while_running(&page, &page.overlay).await { + end(&page); return; } - busy(&page, Some(&i18n("Downloading, checking and installing…"))); let target = page.target.clone(); let cfg = page.cfg.borrow().clone(); let result = gio::spawn_blocking(move || { @@ -2286,7 +2316,7 @@ fn change_version(page: &Rc, to: Option format!( "{} {} — {}", @@ -2294,7 +2324,7 @@ fn change_version(page: &Rc, to: Option format!("{}: {}", i18n("Not updated"), error_text(&e)), + Ok(Err(e)) => labelled(&i18n("Not updated"), &error_text(&e)), Err(_) => i18n("Not updated"), }; page.overlay.add_toast(adw::Toast::new(&text)); @@ -2302,6 +2332,57 @@ fn change_version(page: &Rc, to: Option