forked from unom/punktfunk
fix(client/windows): settings stop going stale behind your back, and the log has a door
A field reporter's codec setting "changed by itself" between sessions. Nothing writes the negotiated codec back — what they saw was a stale snapshot. `AppCtx.settings` is loaded ONCE at process start and the page renders from it, but this process is not the file's only writer (the spawned session persists its match-window size, the console UI and Decky save too), so the page showed values another process had already replaced — until a row was touched and `commit`'s rebase pulled the file in, at which point the value visibly jumped. The 2026-07-31 rebase fix covered the whole-file writers and missed two spots: nothing re-based on page ENTRY, and the profile-scope commit arm cloned the snapshot without reloading, so overlay absorption diffed against stale globals. Both now re-base on the file. Two more ways a setting could vanish or cost time: * An older binary's whole-file save DROPPED a newer client's keys — `Settings` had no unknown-key passthrough, unlike `SettingsOverlay`, whose `extra` map already gives profiles exactly that contract. Extended to the globals: additive, empty on every existing store, and an empty map serializes to nothing so no file churns. (`save()` was already temp+rename, so the torn-file → silent-Default reset was closed.) * "Check the client log" never said WHERE. Settings ▸ About grows an Open log folder row (%LOCALAPPDATA%\punktfunk\logs, folder not file so the rotated .old generation is in reach), and the failed-spawn banner now names the path. The 4:4:4 caption said "HEVC only, and only where the host can encode it", which sends people hunting: the host gate is PyroWave or an NVENC backend. It says so now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -623,8 +623,14 @@ pub(crate) fn hosts_page(props: &HostsProps, cx: &mut RenderCx) -> Element {
|
||||
actions.push(
|
||||
icon_btn("Settings", Symbol::Setting)
|
||||
.on_click({
|
||||
let ss = set_screen.clone();
|
||||
move || ss.call(Screen::Settings)
|
||||
let (c, ss) = (ctx.clone(), set_screen.clone());
|
||||
move || {
|
||||
// Re-base the settings snapshot on the file before the page
|
||||
// renders — this process is not its only writer (see
|
||||
// settings::refresh_snapshot).
|
||||
super::settings::refresh_snapshot(&c);
|
||||
ss.call(Screen::Settings)
|
||||
}
|
||||
})
|
||||
.into(),
|
||||
);
|
||||
|
||||
@@ -2,7 +2,8 @@
|
||||
//! Settings).
|
||||
|
||||
use super::style::*;
|
||||
use super::Screen;
|
||||
use super::{AppCtx, Screen};
|
||||
use std::sync::Arc;
|
||||
use windows_reactor::*;
|
||||
|
||||
/// punktfunk's own license (MIT OR Apache-2.0).
|
||||
@@ -15,10 +16,15 @@ const APP_LICENSE: &str = concat!(
|
||||
/// scripts/gen-third-party-notices.sh; the MSIX also ships this under licenses/).
|
||||
const THIRD_PARTY_NOTICES: &str = include_str!("../../../../THIRD-PARTY-NOTICES.txt");
|
||||
|
||||
pub(crate) fn licenses_page(set_screen: &AsyncSetState<Screen>) -> Element {
|
||||
pub(crate) fn licenses_page(ctx: &Arc<AppCtx>, set_screen: &AsyncSetState<Screen>) -> Element {
|
||||
let back_btn = button("Back").accent().icon(Symbol::Back).on_click({
|
||||
let ss = set_screen.clone();
|
||||
move || ss.call(Screen::Settings)
|
||||
let (c, ss) = (ctx.clone(), set_screen.clone());
|
||||
move || {
|
||||
// Back RE-ENTERS the settings page — re-base its snapshot on the file, same
|
||||
// as the hosts page's Settings button (see settings::refresh_snapshot).
|
||||
super::settings::refresh_snapshot(&c);
|
||||
ss.call(Screen::Settings)
|
||||
}
|
||||
});
|
||||
|
||||
let app_card = card(
|
||||
|
||||
@@ -172,6 +172,10 @@ pub(crate) struct Shared {
|
||||
|
||||
pub struct AppCtx {
|
||||
pub(crate) identity: (String, String),
|
||||
/// The settings snapshot the UI renders from. Loaded once at startup, and RE-BASED on
|
||||
/// the file when the settings page is (re)entered (`settings::refresh_snapshot`) and
|
||||
/// inside every `commit` — this process is not the file's only writer (session resize,
|
||||
/// console UI, Decky), so a plain process-lifetime snapshot goes stale on screen.
|
||||
pub(crate) settings: Mutex<Settings>,
|
||||
pub(crate) gamepad: GamepadService,
|
||||
pub(crate) shared: Arc<Shared>,
|
||||
@@ -688,7 +692,7 @@ fn root(cx: &mut RenderCx, ctx: &Arc<AppCtx>) -> Element {
|
||||
&set_settings_rev,
|
||||
nav_progress,
|
||||
),
|
||||
Screen::Licenses => licenses::licenses_page(&set_screen),
|
||||
Screen::Licenses => licenses::licenses_page(ctx, &set_screen),
|
||||
Screen::Help => help::help_page(&set_screen),
|
||||
Screen::Pair => component(pair::pair_page, svc),
|
||||
Screen::SpeedTest => component(speed::speed_page, SpeedProps { svc, state: speed }),
|
||||
|
||||
@@ -411,7 +411,16 @@ fn commit(
|
||||
return;
|
||||
}
|
||||
let mut catalog = ProfilesFile::load();
|
||||
let base = ctx.settings.lock().unwrap().clone();
|
||||
// The same rebase as the global arm above: `base` is what `absorb`'s before/after
|
||||
// effective settings derive from, and the snapshot is not the file — another process
|
||||
// (session resize, console UI, Decky) may have moved a global under us. The historical
|
||||
// rebase fix ("settings saves stop reverting each other") covered the whole-file
|
||||
// writers but missed this arm.
|
||||
let base = {
|
||||
let mut s = ctx.settings.lock().unwrap();
|
||||
*s = Settings::load();
|
||||
s.clone()
|
||||
};
|
||||
let Some(p) = catalog.profiles.iter_mut().find(|p| p.id == scope) else {
|
||||
return; // deleted from under us; the next render falls back to the defaults scope
|
||||
};
|
||||
@@ -425,6 +434,17 @@ fn commit(
|
||||
rev.1.call(rev.0 + 1);
|
||||
}
|
||||
|
||||
/// Re-base the process-lifetime settings snapshot on the file — called from the navigation
|
||||
/// handlers that (re)enter this page, NOT per render pass. `ctx.settings` is loaded once at
|
||||
/// process start and this process is not the file's only writer (a spawned session persists
|
||||
/// its match-window size, the console UI and Decky save too — profiles.rs documents the
|
||||
/// family), so without this the page opens showing values another process already replaced,
|
||||
/// which then visibly "jump" the moment a row is touched and `commit`'s rebase pulls the
|
||||
/// file in. The field report this fixes: a codec setting that "changed by itself".
|
||||
pub(crate) fn refresh_snapshot(ctx: &Arc<AppCtx>) {
|
||||
*ctx.settings.lock().unwrap() = Settings::load();
|
||||
}
|
||||
|
||||
/// Which tier-P rows the profile in scope overrides. Plain bools rather than a lookup so the
|
||||
/// call sites read as `over.codec` — the row and its flag stay visibly paired.
|
||||
#[derive(Default)]
|
||||
@@ -978,6 +998,16 @@ pub(crate) fn settings_page(
|
||||
let ss = set_screen.clone();
|
||||
button("Third-party licenses").on_click(move || ss.call(Screen::Licenses))
|
||||
};
|
||||
// The client log's home (%LOCALAPPDATA%\punktfunk\logs) — the file every "check the
|
||||
// client log" message means, which until this row had no way in from the UI at all.
|
||||
// The folder rather than the file so the rotated `.old` generation is in reach too.
|
||||
// Best-effort, like the log itself: a missing dir or a failed spawn stays silent.
|
||||
let logs_button = button("Open log folder").on_click(|| {
|
||||
if let Some(dir) = crate::logfile::log_dir() {
|
||||
let _ = std::fs::create_dir_all(&dir);
|
||||
let _ = std::process::Command::new("explorer.exe").arg(&dir).spawn();
|
||||
}
|
||||
});
|
||||
let library_toggle = setting_toggle(ctx, scope, (rev, set_rev), s.library_enabled, |s, on| {
|
||||
s.library_enabled = on
|
||||
});
|
||||
@@ -1071,8 +1101,9 @@ pub(crate) fn settings_page(
|
||||
"HDR10, when the host has HDR content and this display supports it. \
|
||||
HEVC only; otherwise the stream stays SDR.",
|
||||
),
|
||||
// Wording shared with the GTK client (its chroma_row) — same setting,
|
||||
// same constraints.
|
||||
// First sentence shared with the GTK client (its chroma_row); the
|
||||
// constraint sentence names the real gate (host: PyroWave || NVENC) —
|
||||
// "where the host can encode it" cost field users the discovery time.
|
||||
described_overridable(
|
||||
(rev, set_rev),
|
||||
scope,
|
||||
@@ -1081,7 +1112,8 @@ pub(crate) fn settings_page(
|
||||
over.enable_444,
|
||||
chroma_toggle,
|
||||
"Full-colour video: crisp small text and thin lines, at more \
|
||||
bandwidth. HEVC only, and only where the host can encode it.",
|
||||
bandwidth. Requires an NVIDIA host (NVENC) or the PyroWave \
|
||||
codec \u{2014} other encoders stream 4:2:0.",
|
||||
),
|
||||
],
|
||||
None,
|
||||
@@ -1348,7 +1380,16 @@ pub(crate) fn settings_page(
|
||||
"About",
|
||||
group(
|
||||
None,
|
||||
vec![about_identity.into(), licenses_button.into()],
|
||||
vec![
|
||||
about_identity.into(),
|
||||
described_labeled(
|
||||
"Diagnostics",
|
||||
logs_button,
|
||||
"The client log (client.log, plus the session\u{2019}s whole \
|
||||
receive/decode/present trail) \u{2014} attach it to a bug report.",
|
||||
),
|
||||
licenses_button.into(),
|
||||
],
|
||||
None,
|
||||
),
|
||||
),
|
||||
|
||||
@@ -21,11 +21,12 @@ const ROTATE_BYTES: u64 = 10 * 1024 * 1024;
|
||||
|
||||
static SINK: OnceLock<Option<Arc<Mutex<File>>>> = OnceLock::new();
|
||||
|
||||
fn log_dir() -> Option<PathBuf> {
|
||||
/// The log directory — Settings ▸ About's "Open log folder" opens it in Explorer.
|
||||
pub(crate) fn log_dir() -> Option<PathBuf> {
|
||||
Some(PathBuf::from(std::env::var_os("LOCALAPPDATA")?).join(r"punktfunk\logs"))
|
||||
}
|
||||
|
||||
/// The log file's path, for the "logs land here" startup line (and any future UI affordance).
|
||||
/// The log file's path, for the "logs land here" startup line and the failed-spawn banner.
|
||||
pub(crate) fn path() -> Option<PathBuf> {
|
||||
Some(log_dir()?.join("client.log"))
|
||||
}
|
||||
|
||||
@@ -105,7 +105,14 @@ fn parse_line(line: &str) -> Option<ChildLine> {
|
||||
/// connect that silently drops back to the host list.
|
||||
pub(crate) fn silent_exit_banner(code: i32) -> Option<String> {
|
||||
(code != 0 && code != -1).then(|| {
|
||||
format!("The session didn't start (punktfunk-session exited with code {code}). Check the client log.")
|
||||
// Name the log's actual location — "check the client log" without a path is a
|
||||
// scavenger hunt (Settings ▸ About's "Open log folder" reaches it too).
|
||||
let log = crate::logfile::path()
|
||||
.map(|p| p.display().to_string())
|
||||
.unwrap_or_else(|| "the client log".into());
|
||||
format!(
|
||||
"The session didn't start (punktfunk-session exited with code {code}). Check {log}."
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ use anyhow::{anyhow, Context, Result};
|
||||
use punktfunk_core::client::NativeClient;
|
||||
use punktfunk_core::quic::endpoint;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
pub fn config_dir() -> Result<PathBuf> {
|
||||
@@ -940,6 +941,14 @@ pub struct Settings {
|
||||
/// the user will be looking at. `0` = never stored → the 1280×720 default.
|
||||
pub last_window_w: u32,
|
||||
pub last_window_h: u32,
|
||||
/// Settings keys this build doesn't model (a newer client's field), carried through a
|
||||
/// load→save round-trip untouched — [`crate::profiles::SettingsOverlay`]'s `extra`
|
||||
/// pattern extended to the globals. Without it, every whole-file writer of this store
|
||||
/// (two shells, the console settings screen, the session's resize callback, Decky)
|
||||
/// running as an OLDER binary silently drops what a newer one persisted. Empty on
|
||||
/// every existing store, and an empty map serializes to nothing, so files don't churn.
|
||||
#[serde(flatten)]
|
||||
pub extra: BTreeMap<String, serde_json::Value>,
|
||||
}
|
||||
|
||||
fn default_codec() -> String {
|
||||
@@ -1034,6 +1043,7 @@ impl Default for Settings {
|
||||
match_window: false,
|
||||
last_window_w: 0,
|
||||
last_window_h: 0,
|
||||
extra: BTreeMap::new(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1208,6 +1218,28 @@ mod tests {
|
||||
assert!(s.echo_cancel);
|
||||
}
|
||||
|
||||
/// A key this build doesn't model (a newer client's setting) survives a load→save
|
||||
/// round trip instead of being dropped by the next whole-file write — the same
|
||||
/// contract `SettingsOverlay.extra` gives profiles. And when there are no unknown
|
||||
/// keys, the flatten map adds nothing, so existing files don't churn.
|
||||
#[test]
|
||||
fn settings_unknown_keys_survive_round_trip() {
|
||||
let newer = r#"{"width":1920,"height":1080,"frob_mode":"fancy","frob_level":3}"#;
|
||||
let s: Settings = serde_json::from_str(newer).unwrap();
|
||||
assert_eq!((s.width, s.height), (1920, 1080));
|
||||
assert_eq!(
|
||||
s.extra.get("frob_mode").and_then(|v| v.as_str()),
|
||||
Some("fancy")
|
||||
);
|
||||
let out = serde_json::to_string(&s).unwrap();
|
||||
assert!(out.contains(r#""frob_mode":"fancy""#), "{out}");
|
||||
assert!(out.contains(r#""frob_level":3"#), "{out}");
|
||||
// No unknown keys → no artifact of the passthrough field in the file.
|
||||
let plain = serde_json::to_string(&Settings::default()).unwrap();
|
||||
assert!(!plain.contains("extra"), "{plain}");
|
||||
assert!(!plain.contains("frob"), "{plain}");
|
||||
}
|
||||
|
||||
/// Stats-tier resolution: a pre-tier store falls back to `show_stats` (off → Off,
|
||||
/// on/absent → Normal), an explicit tier wins, and setting a tier keeps the legacy
|
||||
/// bool in sync so pre-tier binaries reading the same file agree on off vs on.
|
||||
|
||||
Reference in New Issue
Block a user