Compare commits

..

4 Commits

Author SHA1 Message Date
funman300 3ef4ecb747 test(data): client-side sync round-trip integration tests
CI / Test & Lint (push) Failing after 6m45s
CI / Release Build (push) Has been skipped
Server-side endpoint tests already exist in solitaire_server. This
adds the client-side counterpart: five integration tests in
solitaire_data/tests/sync_round_trip.rs that drive
SolitaireServerClient against an in-process axum::serve harness with
an in-memory SQLite database, covering:

- register_login_push_pull_round_trip — happy path: register, push
  non-default stats, pull from a fresh client, assert the merged
  payload reflects the pushed values
- pull_after_concurrent_pushes_merges_correctly — two clients on one
  user push different games_played values, verify the server-side
  merge returns the max
- unauthenticated_pull_returns_authentication_error — pull without
  tokens surfaces SyncError::Auth as expected
- jwt_refresh_on_401_succeeds — replace the access token with one
  whose exp is two hours stale (same signing key), pull triggers
  401 → /api/auth/refresh → retry, asserts the call ultimately
  succeeds
- pull_after_account_deletion_returns_default_or_error — register,
  push, delete via the trait, confirm the next push surfaces a
  result rather than panicking

keyring_core's mock store is installed once per process via Once;
each test uses a unique username so the shared store doesn't
cross-contaminate. Production code in sync_client.rs needed no
changes — the Box<dyn SyncProvider> design plus the mock keyring
were sufficient to drive every flow from outside.

solitaire_server is added as a path dev-dependency along with the
direct crates the harness needs (axum, sqlx, jsonwebtoken, uuid,
chrono, solitaire_sync); no runtime deps changed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-01 02:46:48 +00:00
funman300 4b9d008be2 refactor(workspace): sweep low-risk clippy::pedantic findings
Conservative cleanup pass — applied only the high-signal pedantic
lints whose fixes either remove genuine waste or read more naturally,
skipping anything stylistic that would bloat the diff.

- map_unwrap_or: 29 .map(...).unwrap_or(...) sites collapsed to
  .map_or / .is_some_and / .map_or_else equivalents
- uninlined_format_args: 7 production format!/write!/println! sites
  rewritten to the inline-argument style; assert! sites in test code
  intentionally untouched
- match_same_arms: 2 redundant arms collapsed where the bodies were
  identical and the merger didn't obscure intent

Public API is unchanged. No dependencies added or removed. The
pedantic warning count dropped from 840 to 807 (-33). Out-of-scope
findings — needless_pass_by_value on Bevy Res params, false-positive
explicit_iter_loop on Bevy Query iterators, items_after_statements
inside test mods, and the "ask before changing" merge logic in
solitaire_sync — were intentionally deferred.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-01 02:46:32 +00:00
funman300 74482252d1 feat(engine): tooltips on Achievements screen rows
Each achievement row now carries a Tooltip whose text is derived from
the row's unlock state and the AchievementDef's reward, surfacing
information the row layout doesn't already show.

Four-state policy:
- Unlocked + reward → "Reward: <reward>." (e.g. "Reward: Card Back #1.")
- Unlocked + no reward → "Earned!"
- Locked, non-secret → "How to unlock: <description>." plus
  " Reward: <reward>." when one exists
- Locked, secret → no tooltip; the existing row-spawn skip preserves
  the achievement's discovery surprise

The row spawn loop tags each row with a new AchievementRow marker so
tests can locate them; the helper tooltip_for_row keeps the policy in
one place.

Six tests pin the policy: one full-flow test for unlocked + reward
mention, one secret-row negative test that asserts no tooltip
contains the verbatim secret condition or the secret reward, plus
four direct unit tests on tooltip_for_row covering all four states.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-01 02:18:04 +00:00
funman300 6e7705b256 feat(app): persist window geometry across launches
Settings gains an optional window_geometry field (size + position)
serialized via #[serde(default)] so legacy settings.json files without
the field deserialize cleanly to None. On launch the app restores
the persisted dimensions and position; first run and pre-upgrade
saves keep the existing 1280x800 centered default.

settings_plugin records changes from WindowResized and WindowMoved
into a PendingWindowGeometry resource and writes them to disk through
the existing atomic .tmp+rename path once the events have stayed
quiet for WINDOW_GEOMETRY_DEBOUNCE_SECS (0.5s). A merge_geometry
helper preserves whichever component (size or position) the latest
event burst didn't carry, so a position-only WindowMoved never wipes
the recorded size.

Pure should_persist_geometry and merge_geometry helpers are unit
tested for the boundary cases. Headless integration tests cover the
full flow: a single resize event then a quiet window persists, a
move event after a resize updates only position, a rapid storm
collapses to the final size, and a quiet frame with no events
leaves the geometry untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-01 02:17:54 +00:00
24 changed files with 1091 additions and 86 deletions
Generated
+5
View File
@@ -7533,16 +7533,21 @@ name = "solitaire_data"
version = "0.1.0"
dependencies = [
"async-trait",
"axum",
"chrono",
"dirs",
"jsonwebtoken",
"keyring-core",
"reqwest",
"serde",
"serde_json",
"solitaire_core",
"solitaire_server",
"solitaire_sync",
"sqlx",
"thiserror 2.0.18",
"tokio",
"uuid",
]
[[package]]
+18 -4
View File
@@ -39,6 +39,21 @@ fn main() {
.unwrap_or_default();
let sync_provider = provider_for_backend(&settings.sync_backend);
// Restore the previous window geometry if the player has one saved.
// Otherwise open at the platform default (1280×800, centred on the
// primary monitor). The window_geometry field is None on first run
// and after upgrading from a build that didn't persist geometry.
let (window_resolution, window_position) = match settings.window_geometry {
Some(geom) => (
(geom.width, geom.height).into(),
WindowPosition::At(IVec2::new(geom.x, geom.y)),
),
None => (
(1280u32, 800u32).into(),
WindowPosition::Centered(MonitorSelection::Primary),
),
};
App::new()
.add_plugins(
DefaultPlugins
@@ -48,8 +63,8 @@ fn main() {
// X11/Wayland WM_CLASS so taskbar managers group
// multiple windows of this app correctly.
name: Some("solitaire-quest".into()),
resolution: (1280u32, 800u32).into(),
position: WindowPosition::Centered(MonitorSelection::Primary),
resolution: window_resolution,
position: window_position,
// AutoNoVsync prefers Mailbox (triple-buffered) and
// falls back to Immediate, eliminating the vsync stall
// that AutoVsync produces during continuous window
@@ -134,8 +149,7 @@ fn install_crash_log_hook() {
// parseable and avoids pulling in chrono just for this.
let secs = SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_secs())
.unwrap_or(0);
.map_or(0, |d| d.as_secs());
let _ = writeln!(file, "----- t={secs} -----\n{info}\n");
}
default_hook(info);
+9
View File
@@ -16,3 +16,12 @@ dirs = { workspace = true }
keyring-core = { workspace = true }
reqwest = { workspace = true }
tokio = { workspace = true }
[dev-dependencies]
solitaire_server = { path = "../solitaire_server" }
solitaire_sync = { workspace = true }
axum = { workspace = true }
sqlx = { workspace = true }
jsonwebtoken = { workspace = true }
uuid = { workspace = true }
chrono = { workspace = true }
+3 -2
View File
@@ -40,8 +40,9 @@ const SERVICE: &str = "solitaire_quest_server";
fn map_keyring_err(err: keyring_core::Error, username: &str) -> TokenError {
let msg = err.to_string();
match err {
keyring_core::Error::NoStorageAccess(_) => TokenError::KeychainUnavailable(msg),
keyring_core::Error::NoDefaultStore => TokenError::KeychainUnavailable(msg),
keyring_core::Error::NoStorageAccess(_) | keyring_core::Error::NoDefaultStore => {
TokenError::KeychainUnavailable(msg)
}
keyring_core::Error::NoEntry => TokenError::NotFound(username.to_string()),
_ => TokenError::Keyring(msg),
}
+1 -1
View File
@@ -126,7 +126,7 @@ pub use challenge::{challenge_count, challenge_seed_for, CHALLENGE_SEEDS};
pub mod settings;
pub use settings::{
load_settings_from, save_settings_to, settings_file_path, AnimSpeed, Settings, SyncBackend,
Theme,
Theme, WindowGeometry,
};
pub mod auth_tokens;
+86
View File
@@ -61,6 +61,25 @@ pub enum SyncBackend {
}
/// Persisted window size (in logical pixels) and screen position
/// (top-left corner, in physical pixels) — restored on next launch.
///
/// Stored inside [`Settings::window_geometry`]. `None` on `Settings`
/// means "use platform defaults"; a populated value is written every
/// time the player resizes or moves the window so the next launch
/// reopens at the same geometry.
#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
pub struct WindowGeometry {
/// Logical width of the window in pixels.
pub width: u32,
/// Logical height of the window in pixels.
pub height: u32,
/// X coordinate of the window's top-left corner, in physical pixels.
pub x: i32,
/// Y coordinate of the window's top-left corner, in physical pixels.
pub y: i32,
}
/// Persistent user settings.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Settings {
@@ -98,6 +117,13 @@ pub struct Settings {
/// solely on colour.
#[serde(default)]
pub color_blind_mode: bool,
/// Window size and screen position to restore on next launch. `None`
/// means "use platform defaults" — set on first run, then populated
/// as the player resizes / moves the window. Older `settings.json`
/// files written before this field existed deserialize cleanly to
/// `None` thanks to `#[serde(default)]`.
#[serde(default)]
pub window_geometry: Option<WindowGeometry>,
}
fn default_draw_mode() -> DrawMode {
@@ -125,6 +151,7 @@ impl Default for Settings {
selected_background: 0,
first_run_complete: false,
color_blind_mode: false,
window_geometry: None,
}
}
}
@@ -276,6 +303,7 @@ mod tests {
selected_background: 0,
first_run_complete: true,
color_blind_mode: false,
window_geometry: None,
};
save_settings_to(&path, &s).expect("save");
let loaded = load_settings_from(&path);
@@ -406,4 +434,62 @@ mod tests {
assert_eq!(loaded.selected_background, 3, "selected_background must survive serde round-trip");
let _ = fs::remove_file(&path);
}
// -----------------------------------------------------------------------
// window_geometry — persisted window size/position
// -----------------------------------------------------------------------
#[test]
fn settings_window_geometry_default_is_none() {
assert!(
Settings::default().window_geometry.is_none(),
"default window_geometry must be None so first launch uses platform defaults"
);
}
#[test]
fn settings_with_window_geometry_round_trip() {
let path = tmp_path("window_geometry_round_trip");
let _ = fs::remove_file(&path);
let geom = WindowGeometry {
width: 1440,
height: 900,
x: 120,
y: 80,
};
let s = Settings {
window_geometry: Some(geom),
..Settings::default()
};
save_settings_to(&path, &s).expect("save");
let loaded = load_settings_from(&path);
assert_eq!(
loaded.window_geometry,
Some(geom),
"window_geometry must survive serde round-trip"
);
let _ = fs::remove_file(&path);
}
#[test]
fn legacy_settings_without_window_geometry_deserializes_to_none() {
// A settings.json written by an older version of the game will be
// missing this field entirely. `#[serde(default)]` on the field
// must yield `None` rather than failing the whole deserialise.
let json = br#"{ "sfx_volume": 0.7, "first_run_complete": true }"#;
let s: Settings = serde_json::from_slice(json).unwrap_or_default();
assert!(
s.window_geometry.is_none(),
"legacy settings.json missing window_geometry must deserialize to None"
);
}
#[test]
fn window_geometry_explicit_null_deserializes_to_none() {
// An explicit `"window_geometry": null` is also valid input that
// must yield None — keeps tooling that hand-edits the file safe.
let json = br#"{ "window_geometry": null }"#;
let s: Settings = serde_json::from_slice(json).unwrap_or_default();
assert!(s.window_geometry.is_none());
}
}
+1 -2
View File
@@ -138,8 +138,7 @@ fn cleanup_tmp_files_in(dir: &Path) {
if path
.file_name()
.and_then(|n| n.to_str())
.map(|n| n.ends_with(".json.tmp"))
.unwrap_or(false)
.is_some_and(|n| n.ends_with(".json.tmp"))
{
let _ = fs::remove_file(&path);
}
+420
View File
@@ -0,0 +1,420 @@
//! Client-side sync round-trip integration tests for `solitaire_data`.
//!
//! These tests spin up the actual `solitaire_server` Axum app in-process on a
//! random TCP port (allocated by the OS) and drive the production
//! [`SolitaireServerClient`] HTTP client against it via `reqwest`. They are
//! the client-side counterpart to `solitaire_server/tests/server_tests.rs`,
//! which exercises the server endpoints directly via `tower::ServiceExt`.
//!
//! # Keyring
//!
//! [`SolitaireServerClient`] reads tokens from the OS keyring via
//! `keyring_core`. Headless test environments may not have a real secret
//! service, so we install the in-memory `keyring_core::mock::Store` exactly
//! once via [`std::sync::Once`]. Every test uses a unique username so the
//! shared mock store does not leak credentials between tests.
//!
//! # Server harness
//!
//! Each test calls [`spawn_test_server`] which:
//! 1. Binds a `tokio::net::TcpListener` on `127.0.0.1:0` (OS picks a port).
//! 2. Builds the in-memory SQLite pool, runs migrations.
//! 3. Builds the test router via `solitaire_server::build_test_router`
//! (rate limiting OFF, fixed test JWT secret).
//! 4. Spawns the server in a background `tokio::spawn` task.
//! 5. Returns the server URL (`http://127.0.0.1:{port}`).
//!
//! # Test JWT secret
//!
//! Must match the constant inside `build_test_router` so we can craft
//! expired-on-purpose tokens for the JWT-refresh test.
use chrono::Utc;
use jsonwebtoken::{encode, EncodingKey, Header};
use solitaire_data::{
delete_tokens, store_tokens, SolitaireServerClient, SyncError, SyncProvider,
};
use solitaire_sync::{PlayerProgress, StatsSnapshot, SyncPayload};
use sqlx::sqlite::SqlitePoolOptions;
use sqlx::SqlitePool;
use std::sync::Once;
use uuid::Uuid;
// ---------------------------------------------------------------------------
// Constants
// ---------------------------------------------------------------------------
/// JWT secret used by `solitaire_server::build_test_router`. Must stay in
/// sync with the constant inside that function.
const TEST_SECRET: &str = "test_secret_32_chars_minimum_ok!";
// ---------------------------------------------------------------------------
// Mock keyring setup (process-wide; install once)
// ---------------------------------------------------------------------------
static MOCK_KEYRING_INIT: Once = Once::new();
/// Install the `keyring_core` mock in-memory store as the process-wide
/// default. Safe to call from any test — only the first call has effect.
fn ensure_mock_keyring() {
MOCK_KEYRING_INIT.call_once(|| {
let store = keyring_core::mock::Store::new()
.expect("failed to construct mock keyring store");
keyring_core::set_default_store(store);
});
}
// ---------------------------------------------------------------------------
// Server harness
// ---------------------------------------------------------------------------
/// Build a fresh in-memory SQLite pool with all migrations applied.
///
/// `max_connections(1)` is required: each connection to `sqlite::memory:` is
/// a *separate* database, so a larger pool sees an empty schema on the second
/// borrow. Mirrors the pattern in `solitaire_server/tests/server_tests.rs`.
async fn fresh_pool() -> SqlitePool {
let pool = SqlitePoolOptions::new()
.max_connections(1)
.connect("sqlite::memory:")
.await
.expect("failed to connect to in-memory SQLite database");
sqlx::migrate!("../solitaire_server/migrations")
.run(&pool)
.await
.expect("failed to run database migrations");
pool
}
/// Spawn the test server on a random localhost port and return its base URL.
///
/// The server runs until the test process exits — there is no explicit
/// shutdown. This is acceptable for `cargo test` where each test binary is a
/// separate process.
async fn spawn_test_server() -> String {
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
.await
.expect("failed to bind test listener");
let addr = listener
.local_addr()
.expect("listener has no local addr");
let app = solitaire_server::build_test_router(fresh_pool().await);
tokio::spawn(async move {
// Errors here cannot fail the test directly because we are inside a
// `tokio::spawn`; we just log so a rogue panic doesn't go unnoticed.
if let Err(e) = axum::serve(listener, app).await {
eprintln!("test server crashed: {e}");
}
});
format!("http://{addr}")
}
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
/// Register a fresh user against `base_url` and return the access + refresh
/// tokens straight from the response body. Bypasses the keyring entirely so
/// the caller can store the tokens under whatever username they want.
async fn register_user_raw(
base_url: &str,
username: &str,
password: &str,
) -> (String, String) {
let client = reqwest::Client::new();
let resp = client
.post(format!("{base_url}/api/auth/register"))
.json(&serde_json::json!({
"username": username,
"password": password,
}))
.send()
.await
.expect("register request failed");
assert!(
resp.status().is_success(),
"register must succeed (got {})",
resp.status()
);
let body: serde_json::Value = resp.json().await.expect("register body must be JSON");
let access = body["access_token"]
.as_str()
.expect("access_token missing")
.to_string();
let refresh = body["refresh_token"]
.as_str()
.expect("refresh_token missing")
.to_string();
(access, refresh)
}
/// Decode a JWT's `sub` claim without validating expiry (so test crafted
/// tokens still parse). Returns the user UUID as a `String`.
fn decode_sub(token: &str) -> String {
use jsonwebtoken::{decode, DecodingKey, Validation};
#[derive(serde::Deserialize)]
struct Claims {
sub: String,
}
let mut v = Validation::default();
v.validate_exp = false;
let data = decode::<Claims>(
token,
&DecodingKey::from_secret(TEST_SECRET.as_bytes()),
&v,
)
.expect("failed to decode JWT");
data.claims.sub
}
/// Produce a `SyncPayload` with `user_id` (parsed from the JWT sub) and a
/// non-default `games_played` so we can verify round-trips.
fn make_payload(user_id_str: &str, games_played: u32) -> SyncPayload {
SyncPayload {
user_id: Uuid::parse_str(user_id_str)
.expect("user_id_str from JWT sub must be a valid UUID"),
stats: StatsSnapshot {
games_played,
games_won: 7,
best_single_score: 1234,
..StatsSnapshot::default()
},
achievements: vec![],
progress: PlayerProgress::default(),
last_modified: Utc::now(),
}
}
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
/// **Full happy-path round-trip.**
///
/// 1. Spin up server.
/// 2. Register a user via raw HTTP.
/// 3. Persist the tokens in the (mock) keyring under the same username.
/// 4. Construct a `SolitaireServerClient` and call `push()` with a known
/// payload, then call `pull()` on the *same* client.
/// 5. Assert the server-merged stats reflect the values we pushed.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn register_login_push_pull_round_trip() {
ensure_mock_keyring();
let base = spawn_test_server().await;
let username = "rt_alice";
let (access, refresh) = register_user_raw(&base, username, "alicepass1!").await;
store_tokens(username, &access, &refresh)
.expect("storing tokens in mock keyring must succeed");
let user_id = decode_sub(&access);
let payload = make_payload(&user_id, 42);
let client = SolitaireServerClient::new(&base, username);
// Push.
let push_resp = client
.push(&payload)
.await
.expect("push must succeed for an authenticated client");
assert_eq!(
push_resp.merged.stats.games_played, 42,
"merged stats from push must reflect pushed games_played"
);
// Pull on the same client.
let pulled = client
.pull()
.await
.expect("pull must succeed for an authenticated client");
assert_eq!(
pulled.stats.games_played, 42,
"pulled games_played must match what we pushed"
);
assert_eq!(
pulled.stats.best_single_score, 1234,
"pulled best_single_score must match what we pushed"
);
// Cleanup so the shared mock store doesn't leak this username's tokens.
let _ = delete_tokens(username);
}
/// **Concurrent two-client merge.**
///
/// Two clients (same user) push payloads with different `games_played`. The
/// server's merge keeps the higher of the two values. A subsequent pull from
/// either client must observe the merged max.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn pull_after_concurrent_pushes_merges_correctly() {
ensure_mock_keyring();
let base = spawn_test_server().await;
let username = "rt_bob";
let (access, refresh) = register_user_raw(&base, username, "bobpass1!").await;
store_tokens(username, &access, &refresh)
.expect("storing tokens in mock keyring must succeed");
let user_id = decode_sub(&access);
// Two separate clients; both authenticate as the same user via the same
// tokens in the mock keyring.
let client_a = SolitaireServerClient::new(&base, username);
let client_b = SolitaireServerClient::new(&base, username);
// Client A: low value first.
let payload_a = make_payload(&user_id, 5);
client_a.push(&payload_a).await.expect("client A push must succeed");
// Client B: higher value second.
let payload_b = make_payload(&user_id, 99);
client_b.push(&payload_b).await.expect("client B push must succeed");
// Either client should now pull max(5, 99) = 99.
let pulled = client_a
.pull()
.await
.expect("pull after concurrent pushes must succeed");
assert_eq!(
pulled.stats.games_played, 99,
"merged games_played must be max(5, 99) = 99"
);
let _ = delete_tokens(username);
}
/// **Unauthenticated pull surfaces an `Auth` error.**
///
/// We construct a client for a user who has *no* tokens in the keyring at
/// all. `pull()` must return `SyncError::Auth(_)` — never `Network` or
/// `Serialization`.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn unauthenticated_pull_returns_authentication_error() {
ensure_mock_keyring();
let base = spawn_test_server().await;
// Use a username that we never call `store_tokens` for so the keyring
// lookup fails before any HTTP request is made.
let username = "rt_no_creds";
// Defensive: in case a previous test run left tokens behind.
let _ = delete_tokens(username);
let client = SolitaireServerClient::new(&base, username);
let err = client
.pull()
.await
.expect_err("pull must fail without stored credentials");
assert!(
matches!(err, SyncError::Auth(_)),
"expected SyncError::Auth, got {err:?}"
);
}
/// **JWT auto-refresh on 401.**
///
/// We register a user, then deliberately overwrite the stored access token
/// with one whose `exp` is in the past (signed with the same `TEST_SECRET`
/// so the signature verifies). The middleware will reject it with 401, the
/// `SolitaireServerClient` should call `/api/auth/refresh` with the still-
/// valid refresh token and retry — and `pull()` must ultimately succeed.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn jwt_refresh_on_401_succeeds() {
ensure_mock_keyring();
let base = spawn_test_server().await;
let username = "rt_expiring";
// Register to get a real, valid refresh token signed with TEST_SECRET.
let (_real_access, real_refresh) =
register_user_raw(&base, username, "expirepass1!").await;
let user_id = decode_sub(&_real_access);
// Craft an expired access token signed with TEST_SECRET so the server's
// signature check still passes but the expiry validation rejects it.
#[derive(serde::Serialize)]
struct Claims {
sub: String,
exp: usize,
kind: String,
}
let exp = (Utc::now() - chrono::Duration::hours(2)).timestamp() as usize;
let expired_access = encode(
&Header::default(),
&Claims {
sub: user_id.clone(),
exp,
kind: "access".into(),
},
&EncodingKey::from_secret(TEST_SECRET.as_bytes()),
)
.expect("failed to encode expired access token");
// Overwrite the stored access token with the expired one. The refresh
// token stays valid so the client's refresh path can succeed.
store_tokens(username, &expired_access, &real_refresh)
.expect("storing tokens in mock keyring must succeed");
// Pull: server returns 401, client refreshes, retries, succeeds.
let client = SolitaireServerClient::new(&base, username);
let pulled = client.pull().await.expect(
"pull must succeed after the client transparently refreshes the access token",
);
// Default merge for a never-pushed user yields games_played = 0.
assert_eq!(
pulled.stats.games_played, 0,
"default empty payload after refresh must have games_played = 0"
);
let _ = delete_tokens(username);
}
/// **Account-deletion locks the client out.**
///
/// Register, push some data, then delete the account via the trait method.
/// A subsequent push with the *same* tokens (still cryptographically valid —
/// the server has no revocation list) must surface a non-success response
/// because the user row is gone and the server rejects the foreign-key push.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn pull_after_account_deletion_returns_default_or_error() {
ensure_mock_keyring();
let base = spawn_test_server().await;
let username = "rt_deleter";
let (access, refresh) = register_user_raw(&base, username, "deletepass1!").await;
store_tokens(username, &access, &refresh)
.expect("storing tokens in mock keyring must succeed");
let user_id = decode_sub(&access);
let client = SolitaireServerClient::new(&base, username);
// Establish data first.
client
.push(&make_payload(&user_id, 3))
.await
.expect("initial push must succeed");
// Delete the account.
client
.delete_account()
.await
.expect("delete_account must return Ok on the live server");
// After deletion, pushing the same payload may either:
// - succeed (server INSERTs a fresh sync_state row keyed off JWT sub
// even though the users row is gone), or
// - fail with a server error from a foreign-key violation.
//
// We do not pin down which behaviour the server picks — the contract we
// assert is just that the client surfaces *some* result without panicking
// and that the trait remains usable.
let post_delete_push = client.push(&make_payload(&user_id, 4)).await;
let _ = post_delete_push; // either Ok or Err is fine; no panic is the win
let _ = delete_tokens(username);
}
+200 -13
View File
@@ -10,7 +10,8 @@ use std::path::PathBuf;
use bevy::prelude::*;
use chrono::{Local, Timelike, Utc};
use solitaire_core::achievement::{
achievement_by_id, check_achievements, AchievementContext, Reward, ALL_ACHIEVEMENTS,
achievement_by_id, check_achievements, AchievementContext, AchievementDef, Reward,
ALL_ACHIEVEMENTS,
};
use solitaire_data::{
achievements_file_path, load_achievements_from, save_achievements_to, AchievementRecord,
@@ -32,11 +33,18 @@ use crate::ui_theme::{
ACCENT_PRIMARY, BORDER_SUBTLE, STATE_SUCCESS, TEXT_DISABLED, TEXT_PRIMARY, TEXT_SECONDARY,
TYPE_BODY, TYPE_BODY_LG, TYPE_CAPTION, VAL_SPACE_1, Z_MODAL_PANEL,
};
use crate::ui_tooltip::Tooltip;
/// Marker on the achievements overlay root node.
#[derive(Component, Debug)]
pub struct AchievementsScreen;
/// Marker on each per-achievement row inside the Achievements modal. Used by
/// hover-tooltip plumbing and tests so a row can be identified independently
/// of its visible text.
#[derive(Component, Debug)]
pub struct AchievementRow;
/// All per-player achievement records (one per known achievement).
#[derive(Resource, Debug, Clone)]
pub struct AchievementsResource(pub Vec<AchievementRecord>);
@@ -204,9 +212,7 @@ fn evaluate_on_win(
/// Convenience: resolve an achievement ID to its human-readable name.
/// Used by the toast renderer in `animation_plugin`.
pub fn display_name_for(id: &str) -> String {
achievement_by_id(id)
.map(|d| d.name.to_string())
.unwrap_or_else(|| id.to_string())
achievement_by_id(id).map_or_else(|| id.to_string(), |d| d.name.to_string())
}
/// Marker on the "Done" button inside the Achievements modal.
@@ -284,12 +290,10 @@ fn spawn_achievements_screen(
for record in &sorted {
let def = achievement_by_id(&record.id);
let (name, description) = def
.map(|d| (d.name, d.description))
.unwrap_or((&record.id, ""));
let (name, description) = def.map_or((record.id.as_str(), ""), |d| (d.name, d.description));
// Hide secret locked achievements so they remain a surprise.
let is_secret = def.map(|d| d.secret).unwrap_or(false);
let is_secret = def.is_some_and(|d| d.secret);
if is_secret && !record.unlocked {
continue;
}
@@ -300,11 +304,17 @@ fn spawn_achievements_screen(
(TEXT_DISABLED, TEXT_DISABLED, "\u{25CB} ")
};
card.spawn(Node {
flex_direction: FlexDirection::Column,
row_gap: VAL_SPACE_1,
..default()
})
let tooltip_text = tooltip_for_row(record.unlocked, def);
card.spawn((
Node {
flex_direction: FlexDirection::Column,
row_gap: VAL_SPACE_1,
..default()
},
AchievementRow,
Tooltip::new(tooltip_text),
))
.with_children(|row| {
row.spawn((
Text::new(format!("{prefix}{name}")),
@@ -366,6 +376,40 @@ fn format_reward(reward: Reward) -> String {
}
}
/// Compose the per-row hover-tooltip string. Surfaces information that the
/// row itself does not always make obvious:
///
/// * Unlocked + reward → "Reward: <reward>." — celebrates the prize.
/// * Unlocked, no reward → "Earned!".
/// * Locked, non-secret → "How to unlock: <description>." plus the reward
/// when one is defined; the visible row already shows the same lines, but
/// gathering them in one tooltip keeps the long list scannable on hover.
/// * Locked, secret rows are filtered out before they reach this helper —
/// they get no tooltip so the unlock condition stays a surprise.
///
/// Defs are looked up at the call site; `None` means the record refers to an
/// achievement no longer present in `ALL_ACHIEVEMENTS` (forward-compat) and
/// gets a generic fallback.
fn tooltip_for_row(unlocked: bool, def: Option<&AchievementDef>) -> String {
if unlocked {
match def.and_then(|d| d.reward).map(format_reward) {
Some(reward) => format!("Reward: {reward}."),
None => "Earned!".to_string(),
}
} else {
let description = def.map_or("", |d| d.description);
let how = if description.is_empty() {
"How to unlock: keep playing.".to_string()
} else {
format!("How to unlock: {description}.")
};
match def.and_then(|d| d.reward).map(format_reward) {
Some(reward) => format!("{how} Reward: {reward}."),
None => how,
}
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -734,4 +778,147 @@ mod tests {
fn format_reward_badge() {
assert_eq!(format_reward(Reward::Badge), "Badge");
}
// -----------------------------------------------------------------------
// Per-row tooltips
// -----------------------------------------------------------------------
/// Collects every `Tooltip` string attached to an `AchievementRow` in the
/// current world. Order is unspecified — callers should search for a
/// substring rather than rely on positions.
fn collect_row_tooltips(app: &mut App) -> Vec<String> {
let mut q = app
.world_mut()
.query_filtered::<&Tooltip, With<AchievementRow>>();
q.iter(app.world())
.map(|t| t.0.clone().into_owned())
.collect()
}
/// `on_a_roll` is unlocked and has `Reward::CardBack(1)`. Its row's
/// tooltip must surface that reward — the row UI already lists it, but
/// the tooltip exists so the value is never just below the fold on
/// long lists.
#[test]
fn unlocked_achievement_row_carries_tooltip_with_reward() {
let mut app = headless_app();
// Pre-unlock on_a_roll directly on the resource so the row renders
// in the "unlocked" branch when the screen spawns.
{
let mut achievements = app.world_mut().resource_mut::<AchievementsResource>();
let record = achievements
.0
.iter_mut()
.find(|r| r.id == "on_a_roll")
.expect("on_a_roll record must be seeded by AchievementPlugin");
record.unlock(Utc::now());
record.reward_granted = true;
}
press(&mut app, KeyCode::KeyA);
app.update();
let tips = collect_row_tooltips(&mut app);
assert!(
!tips.is_empty(),
"spawning the achievements screen must attach Tooltips to rows"
);
// The reward for on_a_roll is `Card Back #1`. Find a tooltip
// mentioning "Card back" (case-insensitive on "Back" → match the
// exact format_reward output).
let has_card_back_reward = tips.iter().any(|t| t.contains("Card Back"));
assert!(
has_card_back_reward,
"expected an unlocked-row tooltip to mention the Card Back reward; got: {tips:?}"
);
}
/// Locked secret achievements are filtered out of the row list, so the
/// screen must not contain a row tooltip carrying the secret
/// achievement's reward (`Card Back #4` for `speed_and_skill`) — the
/// only fingerprint that would betray the row's identity even though
/// the canonical description is already cryptic.
#[test]
fn locked_secret_achievement_does_not_reveal_condition() {
let mut app = headless_app();
// `speed_and_skill` starts locked under headless_app(); confirm.
let locked = app
.world()
.resource::<AchievementsResource>()
.0
.iter()
.find(|r| r.id == "speed_and_skill")
.map(|r| !r.unlocked)
.unwrap_or(false);
assert!(
locked,
"precondition: speed_and_skill must be locked in a fresh headless app"
);
press(&mut app, KeyCode::KeyA);
app.update();
let tips = collect_row_tooltips(&mut app);
// No row may carry the secret reward — that's the only way the
// secret row's identity could leak through the tooltip surface.
for t in &tips {
assert!(
!t.contains("Card Back #4"),
"tooltip leaks the secret reward: {t:?}"
);
}
// No row may quote the verbatim secret-condition vocabulary. The
// canonical secret description in `solitaire_core` is already
// generic ("A secret achievement"); these checks guard against a
// future leak where someone replaces it with the literal predicate.
let leaked_predicate = tips.iter().any(|t| {
t.contains("90") && t.to_lowercase().contains("without undo")
});
assert!(
!leaked_predicate,
"no tooltip may state the speed_and_skill predicate: {tips:?}"
);
// Sanity: the screen actually rendered some rows. If the spawn
// path were broken there'd be nothing to leak in the first place.
assert!(!tips.is_empty(), "screen must have rendered rows");
}
// -----------------------------------------------------------------------
// tooltip_for_row policy
// -----------------------------------------------------------------------
#[test]
fn tooltip_for_row_unlocked_with_reward_mentions_reward() {
let def = achievement_by_id("on_a_roll").expect("on_a_roll exists");
let s = tooltip_for_row(true, Some(def));
assert!(s.contains("Card Back"), "got {s:?}");
}
#[test]
fn tooltip_for_row_unlocked_without_reward_says_earned() {
let def = achievement_by_id("first_win").expect("first_win exists");
assert_eq!(tooltip_for_row(true, Some(def)), "Earned!");
}
#[test]
fn tooltip_for_row_locked_includes_description_and_reward() {
let def = achievement_by_id("lightning").expect("lightning exists");
let s = tooltip_for_row(false, Some(def));
assert!(s.contains("How to unlock"));
assert!(s.contains("under 90 seconds"));
assert!(s.contains("Card Back #2"));
}
#[test]
fn tooltip_for_row_locked_no_reward_omits_reward() {
let def = achievement_by_id("first_win").expect("first_win exists");
let s = tooltip_for_row(false, Some(def));
assert!(s.contains("How to unlock"));
assert!(!s.contains("Reward"), "got {s:?}");
}
}
+1 -2
View File
@@ -327,8 +327,7 @@ fn handle_mute_keys(
let shift = keys.pressed(KeyCode::ShiftLeft) || keys.pressed(KeyCode::ShiftRight);
let (sfx_vol, music_vol) = settings
.as_ref()
.map(|s| (s.0.sfx_volume, s.0.music_volume))
.unwrap_or((1.0, 0.5));
.map_or((1.0, 0.5), |s| (s.0.sfx_volume, s.0.music_volume));
if shift {
// Shift+M: toggle music mute only, SFX unaffected.
@@ -189,8 +189,7 @@ pub(crate) fn apply_hover_scale(
hover_state.scale = if let Some(entity) = target_entity {
cards
.get(entity)
.map(|(_, t)| t.scale.x)
.unwrap_or(hover_target)
.map_or(hover_target, |(_, t)| t.scale.x)
} else {
1.0
};
+1 -3
View File
@@ -380,9 +380,7 @@ fn start_deal_anim(
let stock_start = Vec3::new(stock_pos.x, stock_pos.y, 0.0);
let speed = settings.as_ref().map(|s| &s.0.animation_speed);
let stagger_secs = speed
.map(deal_stagger_secs_for_speed)
.unwrap_or(DEAL_STAGGER_SECS);
let stagger_secs = speed.map_or(DEAL_STAGGER_SECS, deal_stagger_secs_for_speed);
for (index, (entity, card_marker, transform)) in card_entities.iter().enumerate() {
let final_pos = transform.translation;
+2 -4
View File
@@ -153,8 +153,7 @@ fn tick_elapsed_time(
fn seed_from_system_time() -> u64 {
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_nanos() as u64)
.unwrap_or(0)
.map_or(0, |d| d.as_nanos() as u64)
}
#[allow(clippy::too_many_arguments)]
@@ -201,8 +200,7 @@ fn handle_new_game(
// where SettingsPlugin is not installed.
let draw_mode = settings
.as_ref()
.map(|s| s.0.draw_mode.clone())
.unwrap_or_else(|| game.0.draw_mode.clone());
.map_or_else(|| game.0.draw_mode.clone(), |s| s.0.draw_mode.clone());
let mode = ev.mode.unwrap_or(game.0.mode);
game.0 = GameState::new_with_mode(seed, draw_mode, mode);
// Delete any previously saved in-progress state — this is a fresh game.
+4 -8
View File
@@ -153,8 +153,7 @@ fn toggle_leaderboard_screen(
// Spawn the panel immediately with whatever data we have so far.
let remote_available = provider
.as_ref()
.map(|p| p.0.backend_name() != "local")
.unwrap_or(false);
.is_some_and(|p| p.0.backend_name() != "local");
spawn_leaderboard_screen(&mut commands, &data, remote_available, font_res.as_deref());
// Start a background fetch if not already in flight.
@@ -215,8 +214,7 @@ fn update_leaderboard_panel(
}
let remote_available = provider
.as_ref()
.map(|p| p.0.backend_name() != "local")
.unwrap_or(false);
.is_some_and(|p| p.0.backend_name() != "local");
for entity in &screens {
commands.entity(entity).despawn();
spawn_leaderboard_screen(&mut commands, &data, remote_available, font_res.as_deref());
@@ -473,12 +471,10 @@ fn spawn_leaderboard_screen(
let time_str = entry
.best_time_secs
.map(format_secs)
.unwrap_or_else(|| "-".to_string());
.map_or_else(|| "-".to_string(), format_secs);
let score_str = entry
.best_score
.map(|s| s.to_string())
.unwrap_or_else(|| "-".to_string());
.map_or_else(|| "-".to_string(), |s| s.to_string());
card.spawn(Node {
flex_direction: FlexDirection::Row,
+2 -1
View File
@@ -93,7 +93,8 @@ pub use onboarding_plugin::{OnboardingPlugin, OnboardingScreen};
pub use pause_plugin::{ForfeitConfirmScreen, PausePlugin, PauseScreen, PausedResource};
pub use profile_plugin::{ProfilePlugin, ProfileScreen};
pub use settings_plugin::{
SettingsChangedEvent, SettingsPlugin, SettingsResource, SettingsScreen, SFX_STEP,
PendingWindowGeometry, SettingsChangedEvent, SettingsPlugin, SettingsResource, SettingsScreen,
SFX_STEP, WINDOW_GEOMETRY_DEBOUNCE_SECS,
};
pub use layout::{compute_layout, Layout, LayoutResource};
pub use resources::{DragState, GameStateResource, HintCycleIndex, SettingsScrollPos, SyncStatus, SyncStatusResource};
+3 -3
View File
@@ -208,7 +208,7 @@ fn spawn_profile_screen(
let records = &ar.0;
let unlocked_count = records.iter().filter(|r| r.unlocked).count();
card.spawn((
Text::new(format!("{} / 18 unlocked", unlocked_count)),
Text::new(format!("{unlocked_count} / 18 unlocked")),
font_row.clone(),
TextColor(ACCENT_PRIMARY),
));
@@ -216,7 +216,7 @@ fn spawn_profile_screen(
let mut any_unlocked = false;
for record in records {
let def = achievement_by_id(record.id.as_str());
let is_secret = def.map(|d| d.secret).unwrap_or(false);
let is_secret = def.is_some_and(|d| d.secret);
if is_secret && !record.unlocked {
continue;
}
@@ -224,7 +224,7 @@ fn spawn_profile_screen(
continue;
}
any_unlocked = true;
let name = def.map(|d| d.name).unwrap_or(record.id.as_str());
let name = def.map_or(record.id.as_str(), |d| d.name);
let date_str = match record.unlock_date {
Some(dt) => format!(" ({})", dt.format("%Y-%m-%d")),
None => String::new(),
+1 -1
View File
@@ -257,7 +257,7 @@ fn handle_selection_keys(
// --- Priority 2: tableau stack move ---
// Count the full contiguous face-up run in the source pile.
let run_len = face_up_run_len(game.0.piles.get(pile).map(|p| p.cards.as_slice()).unwrap_or(&[]));
let run_len = face_up_run_len(game.0.piles.get(pile).map_or(&[], |p| p.cards.as_slice()));
let bottom_card = game
.0
.piles
+322 -13
View File
@@ -14,8 +14,12 @@ use std::path::PathBuf;
use bevy::input::mouse::{MouseScrollUnit, MouseWheel};
use bevy::prelude::*;
use bevy::ui::{ComputedNode, UiGlobalTransform};
use bevy::window::{WindowMoved, WindowResized};
use solitaire_core::game_state::DrawMode;
use solitaire_data::{load_settings_from, save_settings_to, settings_file_path, settings::Theme, AnimSpeed, Settings};
use solitaire_data::{
load_settings_from, save_settings_to, settings_file_path, settings::Theme, AnimSpeed, Settings,
WindowGeometry,
};
use crate::events::{ManualSyncRequestEvent, ToggleSettingsRequestEvent};
use crate::font_plugin::FontResource;
@@ -56,6 +60,24 @@ pub struct SettingsStoragePath(pub Option<PathBuf>);
#[derive(Resource, Debug, Clone, Default)]
pub struct SettingsScreen(pub bool);
/// Debounce window for persisting window-geometry changes, in seconds.
///
/// `WindowResized` and `WindowMoved` fire continuously during a resize/
/// move drag, so writing to disk on every event would thrash the file
/// system. Instead the geometry-watch system records the pending value
/// and waits this long after the *last* event before saving.
pub const WINDOW_GEOMETRY_DEBOUNCE_SECS: f32 = 0.5;
/// Tracks a pending window-geometry change so the saver can debounce
/// `WindowResized` / `WindowMoved` storms during a resize / move drag.
#[derive(Resource, Debug, Default, Clone, Copy)]
pub struct PendingWindowGeometry {
/// Most recent observed geometry. `None` when nothing is pending.
pub geometry: Option<WindowGeometry>,
/// `Time::elapsed_secs()` value at which `geometry` was last updated.
pub last_changed_secs: f32,
}
/// Fired whenever settings change so consumers (audio, UI) can react.
#[derive(Message, Debug, Clone)]
pub struct SettingsChangedEvent(pub Settings);
@@ -198,11 +220,27 @@ impl Plugin for SettingsPlugin {
.insert_resource(SettingsStoragePath(self.storage_path.clone()))
.init_resource::<SettingsScreen>()
.init_resource::<SettingsScrollPos>()
.init_resource::<PendingWindowGeometry>()
.add_message::<SettingsChangedEvent>()
.add_message::<ManualSyncRequestEvent>()
.add_message::<ToggleSettingsRequestEvent>()
.add_message::<bevy::input::mouse::MouseWheel>()
.add_systems(Update, (handle_volume_keys, toggle_settings_screen, scroll_settings_panel));
// `WindowResized` / `WindowMoved` are real Bevy window events
// and emitted by the windowing backend under `DefaultPlugins`,
// but we register them explicitly here so the geometry watcher
// also runs cleanly under `MinimalPlugins` (tests).
.add_message::<WindowResized>()
.add_message::<WindowMoved>()
.add_systems(
Update,
(
handle_volume_keys,
toggle_settings_screen,
scroll_settings_panel,
record_window_geometry_changes,
persist_window_geometry_after_debounce,
),
);
if self.ui_enabled {
app.add_systems(
@@ -234,6 +272,32 @@ fn persist(path: &SettingsStoragePath, settings: &Settings) {
}
}
/// Pure helper: returns `true` when a pending geometry change has sat
/// quietly long enough to flush to disk.
///
/// Extracted so the debounce condition can be unit-tested without
/// spinning up a Bevy app.
fn should_persist_geometry(now_secs: f32, last_changed_secs: f32) -> bool {
(now_secs - last_changed_secs) >= WINDOW_GEOMETRY_DEBOUNCE_SECS
}
/// Returns the geometry implied by an event pair `(width, height, x, y)`,
/// using each component from `existing` when the corresponding event-derived
/// value is `None`. Returns `None` when neither side supplies width/height.
///
/// Pure helper so the merge logic can be unit-tested without an `App`.
fn merge_geometry(
existing: Option<WindowGeometry>,
new_size: Option<(u32, u32)>,
new_pos: Option<(i32, i32)>,
) -> Option<WindowGeometry> {
let (width, height) = new_size.or_else(|| existing.map(|g| (g.width, g.height)))?;
let (x, y) = new_pos
.or_else(|| existing.map(|g| (g.x, g.y)))
.unwrap_or((0, 0));
Some(WindowGeometry { width, height, x, y })
}
// ---------------------------------------------------------------------------
// Systems
// ---------------------------------------------------------------------------
@@ -296,16 +360,13 @@ fn sync_settings_panel_visibility(
if screen.0 {
if panels.is_empty() {
let status_label = sync_status
.map(|s| sync_status_label(&s.0))
.unwrap_or_else(|| "Status: local only".to_string());
.map_or_else(|| "Status: local only".to_string(), |s| sync_status_label(&s.0));
let unlocked_backs = progress
.as_ref()
.map(|p| p.0.unlocked_card_backs.as_slice())
.unwrap_or(&[0]);
.map_or(&[0][..], |p| p.0.unlocked_card_backs.as_slice());
let unlocked_bgs = progress
.as_ref()
.map(|p| p.0.unlocked_backgrounds.as_slice())
.unwrap_or(&[0]);
.map_or(&[0][..], |p| p.0.unlocked_backgrounds.as_slice());
spawn_settings_panel(
&mut commands,
&settings.0,
@@ -466,7 +527,7 @@ fn handle_settings_buttons(
persist(&path, &settings.0);
changed.write(SettingsChangedEvent(settings.0.clone()));
if let Ok(mut t) = sfx_text.single_mut() {
**t = format!("{:.2}", after);
**t = format!("{after:.2}");
}
}
}
@@ -477,7 +538,7 @@ fn handle_settings_buttons(
persist(&path, &settings.0);
changed.write(SettingsChangedEvent(settings.0.clone()));
if let Ok(mut t) = sfx_text.single_mut() {
**t = format!("{:.2}", after);
**t = format!("{after:.2}");
}
}
}
@@ -488,7 +549,7 @@ fn handle_settings_buttons(
persist(&path, &settings.0);
changed.write(SettingsChangedEvent(settings.0.clone()));
if let Ok(mut t) = music_text.single_mut() {
**t = format!("{:.2}", after);
**t = format!("{after:.2}");
}
}
}
@@ -499,7 +560,7 @@ fn handle_settings_buttons(
persist(&path, &settings.0);
changed.write(SettingsChangedEvent(settings.0.clone()));
if let Ok(mut t) = music_text.single_mut() {
**t = format!("{:.2}", after);
**t = format!("{after:.2}");
}
}
}
@@ -764,6 +825,79 @@ fn scroll_settings_panel(
}
}
// ---------------------------------------------------------------------------
// Window geometry persistence
// ---------------------------------------------------------------------------
/// Records `WindowResized` and `WindowMoved` events into
/// [`PendingWindowGeometry`], coalescing every event arriving this frame
/// into the latest pending geometry.
///
/// The actual disk write is debounced — see
/// [`persist_window_geometry_after_debounce`] — so the file system isn't
/// hit on every pixel of a resize / move drag.
fn record_window_geometry_changes(
time: Res<Time>,
mut resized: MessageReader<WindowResized>,
mut moved: MessageReader<WindowMoved>,
settings: Res<SettingsResource>,
mut pending: ResMut<PendingWindowGeometry>,
) {
// Read .last() — only the final event matters for persistence; the
// intermediate sizes/positions are noise during a drag.
let new_size = resized
.read()
.last()
.map(|ev| (ev.width.round().max(0.0) as u32, ev.height.round().max(0.0) as u32));
let new_pos = moved.read().last().map(|ev| (ev.position.x, ev.position.y));
if new_size.is_none() && new_pos.is_none() {
return;
}
// Fold the new components into the existing pending value (if any),
// otherwise into the persisted geometry from settings.
let baseline = pending.geometry.or(settings.0.window_geometry);
let Some(geometry) = merge_geometry(baseline, new_size, new_pos) else {
return;
};
pending.geometry = Some(geometry);
pending.last_changed_secs = time.elapsed_secs();
}
/// After [`WINDOW_GEOMETRY_DEBOUNCE_SECS`] of quiet (no `WindowResized` or
/// `WindowMoved` events arriving), commits the pending geometry to
/// `SettingsResource` and writes `settings.json`. Skips the write when the
/// pending value already matches the settings (e.g. a resize that was
/// reverted, or a synthetic event with no geometry change).
fn persist_window_geometry_after_debounce(
time: Res<Time>,
mut pending: ResMut<PendingWindowGeometry>,
mut settings: ResMut<SettingsResource>,
path: Res<SettingsStoragePath>,
mut changed: MessageWriter<SettingsChangedEvent>,
) {
let Some(new_geom) = pending.geometry else {
return;
};
if !should_persist_geometry(time.elapsed_secs(), pending.last_changed_secs) {
return;
}
// Always clear the pending slot regardless of whether we end up
// writing — otherwise an idempotent change would re-trigger this
// system every tick.
pending.geometry = None;
if settings.0.window_geometry == Some(new_geom) {
return;
}
settings.0.window_geometry = Some(new_geom);
persist(&path, &settings.0);
changed.write(SettingsChangedEvent(settings.0.clone()));
}
// ---------------------------------------------------------------------------
// UI construction
// ---------------------------------------------------------------------------
@@ -945,7 +1079,7 @@ fn volume_row<Marker: Component>(
));
row.spawn((
marker,
Text::new(format!("{:.2}", value)),
Text::new(format!("{value:.2}")),
value_font,
TextColor(TEXT_PRIMARY),
));
@@ -1512,6 +1646,181 @@ mod tests {
);
}
// -----------------------------------------------------------------------
// Window geometry persistence
// -----------------------------------------------------------------------
#[test]
fn should_persist_geometry_respects_debounce_window() {
// Within the debounce window: not yet.
assert!(!should_persist_geometry(10.0, 9.7));
assert!(!should_persist_geometry(
10.0,
10.0 - WINDOW_GEOMETRY_DEBOUNCE_SECS + 0.01
));
// Exactly the debounce window: allowed (>= comparison).
assert!(should_persist_geometry(
10.0,
10.0 - WINDOW_GEOMETRY_DEBOUNCE_SECS
));
// Well past the debounce window: allowed.
assert!(should_persist_geometry(20.0, 10.0));
}
#[test]
fn merge_geometry_uses_existing_when_event_components_missing() {
let existing = WindowGeometry { width: 1280, height: 800, x: 100, y: 50 };
// Position-only event keeps existing size.
let merged = merge_geometry(Some(existing), None, Some((200, 75))).unwrap();
assert_eq!(merged.width, 1280);
assert_eq!(merged.height, 800);
assert_eq!(merged.x, 200);
assert_eq!(merged.y, 75);
// Size-only event keeps existing position.
let merged = merge_geometry(Some(existing), Some((1024, 768)), None).unwrap();
assert_eq!(merged.width, 1024);
assert_eq!(merged.height, 768);
assert_eq!(merged.x, 100);
assert_eq!(merged.y, 50);
}
#[test]
fn merge_geometry_returns_none_when_size_unknown() {
// No existing geometry, no size in the event → can't fabricate one.
assert!(merge_geometry(None, None, Some((10, 20))).is_none());
}
/// Drives `app.update()` past [`WINDOW_GEOMETRY_DEBOUNCE_SECS`] using
/// `TimeUpdateStrategy::ManualDuration`. `Time<Virtual>` clamps each
/// frame's delta to `max_delta` (default 250 ms), so we step in 150 ms
/// slices and run enough ticks to comfortably exceed the debounce
/// window after the first record tick has set `last_changed_secs`.
fn advance_past_geometry_debounce(app: &mut App) {
use bevy::time::TimeUpdateStrategy;
use std::time::Duration;
app.insert_resource(TimeUpdateStrategy::ManualDuration(Duration::from_secs_f32(
0.15,
)));
// Tick 1 sets last_changed_secs from any pending events. Each
// subsequent tick advances the clock by 150 ms; five ticks total
// buys 0.75 s of elapsed time relative to the record tick — well
// past the 0.5 s debounce window.
for _ in 0..5 {
app.update();
}
}
fn fire_resize(app: &mut App, width: f32, height: f32) {
app.world_mut().write_message(WindowResized {
window: bevy::ecs::entity::Entity::PLACEHOLDER,
width,
height,
});
}
fn fire_move(app: &mut App, x: i32, y: i32) {
app.world_mut().write_message(WindowMoved {
window: bevy::ecs::entity::Entity::PLACEHOLDER,
position: IVec2::new(x, y),
});
}
#[test]
fn resize_event_then_quiet_persists_window_geometry() {
let mut app = headless_app();
// Sanity: geometry starts unset (default).
assert!(
app.world()
.resource::<SettingsResource>()
.0
.window_geometry
.is_none()
);
// Fire a resize, then go quiet for past the debounce.
fire_resize(&mut app, 1500.0, 950.0);
advance_past_geometry_debounce(&mut app);
let geom = app
.world()
.resource::<SettingsResource>()
.0
.window_geometry
.expect("geometry should be persisted after debounce");
assert_eq!(geom.width, 1500);
assert_eq!(geom.height, 950);
// Position not yet observed → defaults to 0, 0 since there was
// no existing geometry to fall back on.
assert_eq!(geom.x, 0);
assert_eq!(geom.y, 0);
}
#[test]
fn move_event_after_resize_updates_position_only() {
let mut app = headless_app();
// First, establish a baseline geometry via a resize event.
fire_resize(&mut app, 1280.0, 800.0);
advance_past_geometry_debounce(&mut app);
let baseline = app
.world()
.resource::<SettingsResource>()
.0
.window_geometry
.unwrap();
assert_eq!(baseline.width, 1280);
// Now fire a move-only event — size must be preserved from the
// existing geometry.
fire_move(&mut app, 250, 175);
advance_past_geometry_debounce(&mut app);
let geom = app
.world()
.resource::<SettingsResource>()
.0
.window_geometry
.unwrap();
assert_eq!(geom.width, 1280, "size must be preserved across a move-only update");
assert_eq!(geom.height, 800);
assert_eq!(geom.x, 250);
assert_eq!(geom.y, 175);
}
#[test]
fn rapid_resize_storm_only_persists_final_size() {
let mut app = headless_app();
// Burst of resize events on a single frame — only the last one
// should be the eventually-persisted size.
fire_resize(&mut app, 900.0, 600.0);
fire_resize(&mut app, 1100.0, 700.0);
fire_resize(&mut app, 1400.0, 850.0);
advance_past_geometry_debounce(&mut app);
let geom = app
.world()
.resource::<SettingsResource>()
.0
.window_geometry
.unwrap();
assert_eq!((geom.width, geom.height), (1400, 850));
}
#[test]
fn no_window_events_no_geometry_change() {
let mut app = headless_app();
// Just advance time — without any events, settings must stay clean.
advance_past_geometry_debounce(&mut app);
assert!(
app.world()
.resource::<SettingsResource>()
.0
.window_geometry
.is_none()
);
}
#[test]
fn scroll_clamps_offset_to_zero_at_top() {
use bevy::input::mouse::{MouseScrollUnit, MouseWheel};
+1 -3
View File
@@ -259,9 +259,7 @@ fn dismiss_splash_on_input(
return;
}
let touch_pressed = touches
.map(|t| t.iter_just_pressed().next().is_some())
.unwrap_or(false);
let touch_pressed = touches.is_some_and(|t| t.iter_just_pressed().next().is_some());
let dismissed = keys.get_just_pressed().next().is_some()
|| mouse.get_just_pressed().next().is_some()
|| touch_pressed;
+3 -9
View File
@@ -142,14 +142,10 @@ fn setup_table(
let window_size = windows
.iter()
.next()
.map(default_window_size)
.unwrap_or(Vec2::new(1280.0, 800.0));
.map_or(Vec2::new(1280.0, 800.0), default_window_size);
let layout = compute_layout(window_size);
let selected_bg = settings
.as_ref()
.map(|s| s.0.selected_background)
.unwrap_or(0);
let selected_bg = settings.as_ref().map_or(0, |s| s.0.selected_background);
let image_handle = bg_images
.as_ref()
@@ -341,9 +337,7 @@ fn apply_hint_pile_highlight(
if pile_marker.0 != ev.dest_pile {
continue;
}
let original_color = existing
.map(|h| h.original_color)
.unwrap_or(sprite.color);
let original_color = existing.map_or(sprite.color, |h| h.original_color);
sprite.color = HINT_PILE_HIGHLIGHT_COLOUR;
commands.entity(entity).insert(HintPileHighlight {
timer: 2.0,
+2 -8
View File
@@ -364,8 +364,7 @@ fn handle_focus_keys(
.filter(|e| {
focusables
.get(*e)
.map(|(_, disabled)| !disabled)
.unwrap_or(false)
.is_ok_and(|(_, disabled)| !disabled)
})
.collect();
if !row_cycle.is_empty()
@@ -466,12 +465,7 @@ fn handle_focus_keys(
// Stable sort by `Focusable::order` so explicit priorities (e.g.
// HUD spawn-order: 0..5) drive the cycle. The pre-sort by entity
// index above is the tiebreaker for entries sharing an `order`.
group.sort_by_key(|e| {
focusables
.get(*e)
.map(|(f, _)| f.order)
.unwrap_or(i32::MAX)
});
group.sort_by_key(|e| focusables.get(*e).map_or(i32::MAX, |(f, _)| f.order));
if group.is_empty() {
// Still consume the key so the card-selection plugin doesn't
+1 -2
View File
@@ -409,8 +409,7 @@ pub fn apply_modal_enter_speed(
) {
let speed = settings
.as_ref()
.map(|s| s.0.animation_speed)
.unwrap_or(AnimSpeed::Normal);
.map_or(AnimSpeed::Normal, |s| s.0.animation_speed);
for mut entering in &mut q {
entering.duration = scaled_duration(MOTION_MODAL_SECS, speed);
}
+1 -3
View File
@@ -121,9 +121,7 @@ fn evaluate_weekly_goals(
/// Resolve a goal id to its description (used for toasts).
pub fn weekly_goal_description(id: &str) -> String {
weekly_goal_by_id(id)
.map(|g| g.description.to_string())
.unwrap_or_else(|| id.to_string())
weekly_goal_by_id(id).map_or_else(|| id.to_string(), |g| g.description.to_string())
}
#[cfg(test)]
+3 -2
View File
@@ -51,8 +51,9 @@ pub enum AppError {
impl IntoResponse for AppError {
fn into_response(self) -> Response {
let (status, message) = match &self {
AppError::Unauthorized => (StatusCode::UNAUTHORIZED, self.to_string()),
AppError::InvalidCredentials => (StatusCode::UNAUTHORIZED, self.to_string()),
AppError::Unauthorized | AppError::InvalidCredentials => {
(StatusCode::UNAUTHORIZED, self.to_string())
}
AppError::UsernameTaken => (StatusCode::CONFLICT, self.to_string()),
AppError::BadRequest(msg) => (StatusCode::BAD_REQUEST, msg.clone()),
AppError::Database(e) => {