From d5d869a6c888682ae89b75d642b103d872868db8 Mon Sep 17 00:00:00 2001 From: funman300 Date: Tue, 19 May 2026 16:27:04 -0700 Subject: [PATCH] fix(multi): resolve 16 bugs from comprehensive rules and code review Core (solitaire_core): - fix(core): auto-complete now requires waste empty to prevent deadlock - fix(core): reject multi-card moves from waste pile (Klondike rule) - fix(core): reject foundation-to-foundation moves (score farming exploit) - fix(core): undo restores score from snapshot baseline, not live score - feat(scoring): add +5 flip bonus when face-down tableau card is exposed - feat(scoring): add recycle penalty (Draw-1: -100/pass, Draw-3: -20/pass) Engine (solitaire_engine): - fix(engine): remove TokioRuntimeResource::default() panic; degrade gracefully - fix(engine): add ModalScrim guard to handle_new_game spawn site - fix(engine): add ModalScrim guard to spawn_restore_prompt spawn site - fix(engine): add ModalScrim guard to check_no_moves spawn site Server / Web (solitaire_server): - fix(web): correct draw_mode casing in replay submission (DrawOne/DrawThree) - fix(web): correct mode casing in replay submission (Classic) for leaderboard - fix(web): trim recorded_at to YYYY-MM-DD for NaiveDate deserialization - fix(server): move /avatars route outside auth middleware (was always 401) Data / Sync (solitaire_data, solitaire_sync): - fix(data): namespace Android token file under APP_DIR_NAME with migration - fix(data): Android token store now multi-user (HashMap); no silent overwrite - fix(sync): draw_one_wins + draw_three_wins invariant preserved after merge Co-Authored-By: Claude Sonnet 4.6 --- solitaire_core/src/game_state.rs | 230 +++++++++++++++++++-- solitaire_core/src/scoring.rs | 44 +++++ solitaire_data/src/android_keystore.rs | 241 ++++++++++++++++------- solitaire_engine/src/analytics_plugin.rs | 18 +- solitaire_engine/src/avatar_plugin.rs | 15 +- solitaire_engine/src/game_plugin.rs | 17 +- solitaire_engine/src/resources.rs | 32 +-- solitaire_server/src/lib.rs | 4 +- solitaire_server/web/game.js | 6 +- solitaire_sync/src/merge.rs | 59 +++++- 10 files changed, 531 insertions(+), 135 deletions(-) diff --git a/solitaire_core/src/game_state.rs b/solitaire_core/src/game_state.rs index 1c0c18a..6910b5f 100644 --- a/solitaire_core/src/game_state.rs +++ b/solitaire_core/src/game_state.rs @@ -5,7 +5,7 @@ use crate::deck::{deal_klondike, Deck}; use crate::error::MoveError; use crate::pile::{Pile, PileType}; use crate::rules::{can_place_on_foundation, can_place_on_tableau, is_valid_tableau_sequence}; -use crate::scoring::{compute_time_bonus as scoring_time_bonus, score_move, score_undo as scoring_undo}; +use crate::scoring::{compute_time_bonus as scoring_time_bonus, score_flip, score_move, score_recycle, score_undo as scoring_undo}; const MAX_UNDO_STACK: usize = 64; @@ -247,6 +247,13 @@ impl GameState { stock.cards.push(card); } self.recycle_count = self.recycle_count.saturating_add(1); + if self.mode != GameMode::Zen { + let penalty = score_recycle( + self.recycle_count, + self.draw_mode == DrawMode::DrawThree, + ); + self.score = (self.score + penalty).max(0); + } self.move_count = self.move_count.saturating_add(1); return Ok(()); } @@ -308,6 +315,11 @@ impl GameState { match &to { PileType::Foundation(_) => { + if matches!(&from, PileType::Foundation(_)) { + return Err(MoveError::RuleViolation( + "cannot move between foundation slots".into(), + )); + } if count != 1 { return Err(MoveError::RuleViolation( "only one card can move to foundation at a time".into(), @@ -331,6 +343,11 @@ impl GameState { )); } } + if matches!(&from, PileType::Waste) && count != 1 { + return Err(MoveError::RuleViolation( + "only the top waste card may be moved".into(), + )); + } let dest = self.piles.get(&to).ok_or(MoveError::InvalidDestination)?; if !can_place_on_tableau(&bottom_card, dest) { return Err(MoveError::RuleViolation("invalid tableau placement".into())); @@ -367,7 +384,8 @@ impl GameState { .cards .split_off(move_start); - // Flip the newly exposed top card of the source pile + // Flip the newly exposed top card of the source pile; award +5 per Windows scoring. + let mut flipped = false; if let Some(top) = self.piles .get_mut(&from) .ok_or(MoveError::InvalidSource)? @@ -376,11 +394,13 @@ impl GameState { && !top.face_up { top.face_up = true; + flipped = true; } self.piles.get_mut(&to).ok_or(MoveError::InvalidDestination)?.cards.append(&mut moved); - self.score = (self.score + score_delta).max(0); + let flip_bonus = if flipped && self.mode != GameMode::Zen { score_flip() } else { 0 }; + self.score = (self.score + score_delta + flip_bonus).max(0); self.move_count = self.move_count.saturating_add(1); self.is_won = self.check_win(); @@ -407,7 +427,7 @@ impl GameState { self.score = if self.mode == GameMode::Zen { 0 } else { - (self.score + scoring_undo()).max(0) + (snapshot.score + scoring_undo()).max(0) }; self.move_count = snapshot.move_count; self.is_won = false; @@ -441,11 +461,15 @@ impl GameState { /// Returns `true` when stock and waste are empty and all tableau cards are face-up. /// At that point the game can be completed without further player input. pub fn check_auto_complete(&self) -> bool { - // Stock must be empty; waste may still have cards (they are resolved - // by draw() calls inside next_auto_complete_move / auto_complete_step). + // All three conditions must hold: stock empty, waste empty, and all + // tableau cards face-up. Requiring waste empty avoids the deadlock + // where the waste top cannot reach a foundation directly. if self.piles.get(&PileType::Stock).is_none_or(|p| !p.cards.is_empty()) { return false; } + if self.piles.get(&PileType::Waste).is_none_or(|p| !p.cards.is_empty()) { + return false; + } (0..7).all(|i| { self.piles .get(&PileType::Tableau(i)) @@ -548,11 +572,10 @@ impl GameState { /// # Precondition /// /// This function is only called when `is_auto_completable` is `true`. - /// Auto-completability requires the waste pile to be empty, as enforced by - /// [`check_auto_complete`](Self::check_auto_complete) — it returns `false` - /// whenever `piles[Waste]` is non-empty. Therefore, skipping the waste pile - /// in this scan is intentional and correct: by the time this function is - /// reached, there are guaranteed to be no cards there to move. + /// Auto-completability requires both stock and waste to be empty, as + /// enforced by [`check_auto_complete`](Self::check_auto_complete). The + /// waste-pile check in this function is therefore a safety net only; under + /// normal operation the waste is guaranteed empty when this is reached. pub fn next_auto_complete_move(&self) -> Option<(PileType, PileType)> { if !self.is_auto_completable || self.is_won { return None; @@ -1134,10 +1157,11 @@ mod tests { } #[test] - fn auto_complete_true_when_stock_empty_waste_has_cards() { - // Waste no longer blocks auto-complete — draw() drains it during - // auto-complete steps. Only stock-not-empty and face-down tableau - // cards block the flag. + fn auto_complete_blocked_when_waste_has_cards() { + // Waste must also be empty for auto-complete to engage. A non-empty + // waste pile — even with all tableau cards face-up and stock empty — + // must return false to prevent a deadlock where the waste top cannot + // reach a foundation directly. let mut g = new_game(); g.piles.get_mut(&PileType::Stock).unwrap().cards.clear(); g.piles.get_mut(&PileType::Waste).unwrap().cards.push(Card { @@ -1151,7 +1175,7 @@ mod tests { c.face_up = true; } } - assert!(g.check_auto_complete()); + assert!(!g.check_auto_complete()); } #[test] @@ -1517,6 +1541,126 @@ mod tests { } } + // --- Flip bonus (+5) --- + + #[test] + fn flip_bonus_awarded_when_face_down_card_exposed() { + let mut g = new_game(); + g.piles.get_mut(&PileType::Stock).unwrap().cards.clear(); + g.piles.get_mut(&PileType::Waste).unwrap().cards.clear(); + for i in 0..7 { g.piles.get_mut(&PileType::Tableau(i)).unwrap().cards.clear(); } + // Tableau(0): hidden Ace under a face-up 5♠ + g.piles.get_mut(&PileType::Tableau(0)).unwrap().cards = vec![ + Card { id: 1, suit: Suit::Hearts, rank: Rank::Ace, face_up: false }, + Card { id: 2, suit: Suit::Spades, rank: Rank::Five, face_up: true }, + ]; + // Tableau(1): 6♥ — 5♠ can land here + g.piles.get_mut(&PileType::Tableau(1)).unwrap().cards = vec![ + Card { id: 3, suit: Suit::Hearts, rank: Rank::Six, face_up: true }, + ]; + let score_before = g.score; + g.move_cards(PileType::Tableau(0), PileType::Tableau(1), 1).unwrap(); + assert_eq!(g.score, score_before + 5, "flip bonus must be +5 when a face-down card is exposed"); + assert!(g.piles[&PileType::Tableau(0)].cards[0].face_up, "exposed card must now be face-up"); + } + + #[test] + fn flip_bonus_not_awarded_when_source_pile_empties() { + let mut g = new_game(); + g.piles.get_mut(&PileType::Stock).unwrap().cards.clear(); + g.piles.get_mut(&PileType::Waste).unwrap().cards.clear(); + for i in 0..7 { g.piles.get_mut(&PileType::Tableau(i)).unwrap().cards.clear(); } + // Only a King in Tableau(0); moving it leaves pile empty — nothing to flip + g.piles.get_mut(&PileType::Tableau(0)).unwrap().cards = vec![ + Card { id: 1, suit: Suit::Spades, rank: Rank::King, face_up: true }, + ]; + let score_before = g.score; + g.move_cards(PileType::Tableau(0), PileType::Tableau(1), 1).unwrap(); + assert_eq!(g.score, score_before, "no flip bonus when source pile becomes empty"); + } + + #[test] + fn flip_bonus_suppressed_in_zen_mode() { + let mut g = GameState::new_with_mode(42, DrawMode::DrawOne, GameMode::Zen); + g.piles.get_mut(&PileType::Stock).unwrap().cards.clear(); + g.piles.get_mut(&PileType::Waste).unwrap().cards.clear(); + for i in 0..7 { g.piles.get_mut(&PileType::Tableau(i)).unwrap().cards.clear(); } + g.piles.get_mut(&PileType::Tableau(0)).unwrap().cards = vec![ + Card { id: 1, suit: Suit::Hearts, rank: Rank::Ace, face_up: false }, + Card { id: 2, suit: Suit::Spades, rank: Rank::Five, face_up: true }, + ]; + g.piles.get_mut(&PileType::Tableau(1)).unwrap().cards = vec![ + Card { id: 3, suit: Suit::Hearts, rank: Rank::Six, face_up: true }, + ]; + g.move_cards(PileType::Tableau(0), PileType::Tableau(1), 1).unwrap(); + assert_eq!(g.score, 0, "zen mode must suppress flip bonus"); + } + + // --- Recycle penalty --- + + #[test] + fn recycle_penalty_draw1_first_pass_free() { + let mut g = new_game(); // DrawOne + g.score = 200; + while !g.piles[&PileType::Stock].cards.is_empty() { g.draw().unwrap(); } + g.draw().unwrap(); // first recycle — free + assert_eq!(g.recycle_count, 1); + assert_eq!(g.score, 200, "first recycle in Draw-1 must be free"); + } + + #[test] + fn recycle_penalty_draw1_second_pass_costs_100() { + let mut g = new_game(); // DrawOne + g.score = 200; + // First recycle (free) + while !g.piles[&PileType::Stock].cards.is_empty() { g.draw().unwrap(); } + g.draw().unwrap(); + // Second recycle (-100) + while !g.piles[&PileType::Stock].cards.is_empty() { g.draw().unwrap(); } + g.draw().unwrap(); + assert_eq!(g.recycle_count, 2); + assert_eq!(g.score, 100, "second recycle in Draw-1 must cost -100"); + } + + #[test] + fn recycle_penalty_draw3_three_passes_free() { + let mut g = GameState::new(42, DrawMode::DrawThree); + g.score = 200; + for _ in 0..3 { + while !g.piles[&PileType::Stock].cards.is_empty() { g.draw().unwrap(); } + g.draw().unwrap(); + } + assert_eq!(g.recycle_count, 3); + assert_eq!(g.score, 200, "first 3 recycles in Draw-3 must be free"); + } + + #[test] + fn recycle_penalty_draw3_fourth_pass_costs_20() { + let mut g = GameState::new(42, DrawMode::DrawThree); + g.score = 200; + for _ in 0..3 { + while !g.piles[&PileType::Stock].cards.is_empty() { g.draw().unwrap(); } + g.draw().unwrap(); + } + // Fourth recycle (-20) + while !g.piles[&PileType::Stock].cards.is_empty() { g.draw().unwrap(); } + g.draw().unwrap(); + assert_eq!(g.recycle_count, 4); + assert_eq!(g.score, 180, "fourth recycle in Draw-3 must cost -20"); + } + + #[test] + fn recycle_penalty_suppressed_in_zen_mode() { + let mut g = GameState::new_with_mode(42, DrawMode::DrawOne, GameMode::Zen); + // Two recycles — second would normally cost -100 in classic mode + for _ in 0..2 { + while !g.piles[&PileType::Stock].cards.is_empty() { g.draw().unwrap(); } + g.draw().unwrap(); + } + assert_eq!(g.recycle_count, 2); + assert_eq!(g.score, 0, "zen mode must suppress recycle penalty"); + } + #[test] fn possible_instructions_waste_top_included() { let mut g = new_game(); @@ -1535,4 +1679,58 @@ mod tests { "King on waste must be moveable to an empty tableau column" ); } + + // --- P2: waste multi-card move must be rejected --- + + #[test] + fn waste_multi_card_move_returns_rule_violation() { + let mut g = new_game(); + g.piles.get_mut(&PileType::Waste).unwrap().cards = vec![ + Card { id: 1, suit: Suit::Hearts, rank: Rank::Ace, face_up: true }, + Card { id: 2, suit: Suit::Spades, rank: Rank::King, face_up: true }, + ]; + for i in 0..7 { g.piles.get_mut(&PileType::Tableau(i)).unwrap().cards.clear(); } + let result = g.move_cards(PileType::Waste, PileType::Tableau(0), 2); + assert!(matches!(result, Err(MoveError::RuleViolation(_))), + "moving 2 cards from waste must be rejected"); + } + + // --- P3: foundation-to-foundation move must be rejected --- + + #[test] + fn foundation_to_foundation_move_returns_rule_violation() { + let mut g = new_game(); + g.piles.get_mut(&PileType::Stock).unwrap().cards.clear(); + g.piles.get_mut(&PileType::Waste).unwrap().cards.clear(); + for i in 0..7 { g.piles.get_mut(&PileType::Tableau(i)).unwrap().cards.clear(); } + // Place Ace of Clubs on Foundation(0), leave Foundation(1) empty. + g.piles.get_mut(&PileType::Foundation(0)).unwrap().cards = vec![ + Card { id: 1, suit: Suit::Clubs, rank: Rank::Ace, face_up: true }, + ]; + // Attempting to move Ace from Foundation(0) to Foundation(1) must fail. + let result = g.move_cards(PileType::Foundation(0), PileType::Foundation(1), 1); + assert!(matches!(result, Err(MoveError::RuleViolation(_))), + "moving between foundation slots must be rejected"); + } + + // --- P4: undo must not retain points from the undone move --- + + #[test] + fn undo_does_not_retain_score_from_undone_move() { + let mut g = new_game(); + g.piles.get_mut(&PileType::Stock).unwrap().cards.clear(); + g.piles.get_mut(&PileType::Waste).unwrap().cards.clear(); + for i in 0..7 { g.piles.get_mut(&PileType::Tableau(i)).unwrap().cards.clear(); } + // Place an Ace on Tableau(0) — moving it to Foundation earns +10. + g.piles.get_mut(&PileType::Tableau(0)).unwrap().cards = vec![ + Card { id: 1, suit: Suit::Clubs, rank: Rank::Ace, face_up: true }, + ]; + assert_eq!(g.score, 0); + g.move_cards(PileType::Tableau(0), PileType::Foundation(0), 1).unwrap(); + assert_eq!(g.score, 10, "moving Ace to foundation earns +10"); + // Undo must roll back to snapshot.score (0) minus the penalty, not keep the +10. + g.undo().unwrap(); + // snapshot.score was 0, so result is max(0, 0 - 15) = 0 + assert_eq!(g.score, 0, "undo must not retain points from the undone move"); + } } diff --git a/solitaire_core/src/scoring.rs b/solitaire_core/src/scoring.rs index ee340ee..f3aa5f9 100644 --- a/solitaire_core/src/scoring.rs +++ b/solitaire_core/src/scoring.rs @@ -5,7 +5,11 @@ use crate::pile::PileType; /// Windows XP Standard scoring: /// - +10 for any card reaching a foundation pile /// - +5 for a waste → tableau move +/// - -15 for a foundation → tableau (take-from-foundation) move /// - 0 for all other moves +/// +/// Note: the +5 flip bonus for exposing a face-down tableau card is applied +/// separately in `game_state::move_cards` because it depends on post-move state. pub fn score_move(from: &PileType, to: &PileType) -> i32 { match to { PileType::Foundation(_) => 10, @@ -23,6 +27,21 @@ pub fn score_undo() -> i32 { -15 } +/// Score bonus awarded when a face-down tableau card is flipped face-up: +5. +pub fn score_flip() -> i32 { + 5 +} + +/// Score penalty for recycling the waste pile back to stock. +/// +/// Windows standard: the first N recycles are free (N=1 for Draw-1, N=3 for Draw-3). +/// Subsequent recycles cost -100 (Draw-1) or -20 (Draw-3). +/// `recycle_count` is the new total count **after** this recycle. +pub fn score_recycle(recycle_count: u32, is_draw_three: bool) -> i32 { + let (free, penalty) = if is_draw_three { (3_u32, -20_i32) } else { (1_u32, -100_i32) }; + if recycle_count > free { penalty } else { 0 } +} + /// Time bonus added to the score on a win: `700_000 / elapsed_seconds`. /// Returns 0 when `elapsed_seconds` is 0 to avoid division by zero. pub fn compute_time_bonus(elapsed_seconds: u64) -> i32 { @@ -93,4 +112,29 @@ mod tests { let bonus = compute_time_bonus(1); assert!(bonus >= 0, "time bonus must be non-negative after u64→i32 cast"); } + + #[test] + fn flip_bonus_is_five() { + assert_eq!(score_flip(), 5); + } + + #[test] + fn recycle_draw1_first_pass_free() { + assert_eq!(score_recycle(1, false), 0); + } + + #[test] + fn recycle_draw1_second_pass_penalised() { + assert_eq!(score_recycle(2, false), -100); + } + + #[test] + fn recycle_draw3_third_pass_free() { + assert_eq!(score_recycle(3, true), 0); + } + + #[test] + fn recycle_draw3_fourth_pass_penalised() { + assert_eq!(score_recycle(4, true), -20); + } } diff --git a/solitaire_data/src/android_keystore.rs b/solitaire_data/src/android_keystore.rs index 388ee7b..b549205 100644 --- a/solitaire_data/src/android_keystore.rs +++ b/solitaire_data/src/android_keystore.rs @@ -2,7 +2,10 @@ /// /// Tokens are serialised to JSON, encrypted with AES-256/GCM/NoPadding using a /// device-bound key from the Android Keystore, and written atomically to -/// `{data_dir}/auth_tokens.bin` as `[12-byte IV][ciphertext+GCM-tag]`. +/// `{data_dir}/ferrous_solitaire/auth_tokens.bin` as `[12-byte IV][ciphertext+GCM-tag]`. +/// +/// The file stores a `HashMap` (keyed by username) so that +/// multiple accounts can coexist without silently overwriting each other. /// /// The Keystore key survives app restarts but is destroyed on uninstall (or if /// the user changes biometric/lock credentials, in which case decryption fails @@ -15,6 +18,7 @@ use jni::{ JNIEnv, JavaVM, }; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use std::path::PathBuf; use crate::auth_tokens::TokenError; @@ -280,21 +284,30 @@ fn decrypt_gcm( // --------------------------------------------------------------------------- fn token_file_path() -> Option { + crate::platform::data_dir() + .map(|d| d.join(crate::APP_DIR_NAME).join("auth_tokens.bin")) +} + +/// Path where the token file lived before the APP_DIR_NAME subdirectory was +/// introduced. Used only during the one-time migration in `read_map`. +fn legacy_token_file_path() -> Option { crate::platform::data_dir().map(|d| d.join("auth_tokens.bin")) } -fn read_file_bytes() -> Result, TokenError> { - let path = token_file_path() - .ok_or_else(|| TokenError::KeychainUnavailable("no data dir".into()))?; +fn read_file_bytes_from(path: &PathBuf) -> Result, TokenError> { if !path.exists() { return Err(TokenError::NotFound(String::new())); } - std::fs::read(&path).map_err(|e| TokenError::Keyring(format!("read auth_tokens.bin: {e}"))) + std::fs::read(path).map_err(|e| TokenError::Keyring(format!("read auth_tokens.bin: {e}"))) } fn write_file_bytes(data: &[u8]) -> Result<(), TokenError> { let path = token_file_path() .ok_or_else(|| TokenError::KeychainUnavailable("no data dir".into()))?; + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .map_err(|e| TokenError::Keyring(format!("create dir: {e}")))?; + } let tmp = path.with_extension("bin.tmp"); std::fs::write(&tmp, data) .map_err(|e| TokenError::Keyring(format!("write auth_tokens.bin.tmp: {e}")))?; @@ -302,29 +315,88 @@ fn write_file_bytes(data: &[u8]) -> Result<(), TokenError> { .map_err(|e| TokenError::Keyring(format!("rename auth_tokens: {e}"))) } -fn load_blob(username: &str) -> Result { - let data = read_file_bytes().map_err(|e| match e { - TokenError::NotFound(_) => TokenError::NotFound(username.to_string()), - other => other, - })?; +/// Decrypt raw bytes from the file and deserialise as `HashMap`. +/// +/// Migration strategy: +/// 1. If the new-path file exists, read and decrypt it. +/// - Try to deserialise as `HashMap`. +/// - On parse failure (old single-blob format), try `TokenBlob` and convert. +/// 2. If the new-path file does NOT exist but the legacy-path file does, migrate: +/// - Read and decrypt the legacy file. +/// - Deserialise as `TokenBlob` (the only format the legacy path ever used). +/// - Write the result to the new path as a single-entry map. +/// - Delete the legacy file (best-effort; leave it if removal fails). +/// 3. If neither file exists, return an empty map. +fn read_map() -> Result, TokenError> { + let new_path = token_file_path() + .ok_or_else(|| TokenError::KeychainUnavailable("no data dir".into()))?; + let legacy_path = legacy_token_file_path(); - if data.len() < 12 { - return Err(TokenError::Keyring("auth_tokens.bin corrupt (too short)".into())); + // --- 1. New path exists --- + if new_path.exists() { + let data = read_file_bytes_from(&new_path).map_err(|e| match e { + TokenError::NotFound(_) => TokenError::NotFound(String::new()), + other => other, + })?; + if data.len() < 12 { + return Err(TokenError::Keyring("auth_tokens.bin corrupt (too short)".into())); + } + let plaintext = with_jvm(|env| { + let key = load_or_create_key(env)?; + decrypt_gcm(env, &key, &data) + })?; + // Try the current multi-user format first. + if let Ok(map) = serde_json::from_slice::>(&plaintext) { + return Ok(map); + } + // Fall back: old single-blob format written by an earlier binary. + if let Ok(blob) = serde_json::from_slice::(&plaintext) { + let mut map = HashMap::new(); + map.insert(blob.username.clone(), blob); + return Ok(map); + } + return Err(TokenError::Keyring("auth_tokens.bin unrecognised format".into())); } - let plaintext = with_jvm(|env| { + // --- 2. Legacy path migration --- + if let Some(ref lpath) = legacy_path { + if lpath.exists() { + let data = read_file_bytes_from(lpath).map_err(|e| match e { + TokenError::NotFound(_) => TokenError::NotFound(String::new()), + other => other, + })?; + if data.len() >= 12 { + let plaintext = with_jvm(|env| { + let key = load_or_create_key(env)?; + decrypt_gcm(env, &key, &data) + })?; + if let Ok(blob) = serde_json::from_slice::(&plaintext) { + let mut map = HashMap::new(); + map.insert(blob.username.clone(), blob); + // Write to the new location, then remove the legacy file. + if write_map_inner(&map).is_ok() { + let _ = std::fs::remove_file(lpath); + } + return Ok(map); + } + } + // Legacy file corrupt or unrecognised — treat as empty. + } + } + + // --- 3. No file found --- + Ok(HashMap::new()) +} + +/// Serialise and encrypt a map, then write it atomically. +fn write_map_inner(map: &HashMap) -> Result<(), TokenError> { + let plaintext = serde_json::to_vec(map) + .map_err(|e| TokenError::Keyring(format!("JSON encode: {e}")))?; + let encrypted = with_jvm(|env| { let key = load_or_create_key(env)?; - decrypt_gcm(env, &key, &data) + encrypt_gcm(env, &key, &plaintext) })?; - - let blob: TokenBlob = serde_json::from_slice(&plaintext) - .map_err(|e| TokenError::Keyring(format!("JSON decode: {e}")))?; - - if blob.username != username { - return Err(TokenError::NotFound(username.to_string())); - } - - Ok(blob) + write_file_bytes(&encrypted) } // --------------------------------------------------------------------------- @@ -333,77 +405,106 @@ fn load_blob(username: &str) -> Result { /// Encrypt and store `access_token` and `refresh_token` for `username`. /// -/// Overwrites any previously stored tokens. +/// If tokens already exist for other usernames they are preserved. +/// Any previously stored tokens for `username` are silently replaced. pub fn store_tokens( username: &str, access_token: &str, refresh_token: &str, ) -> Result<(), TokenError> { - let blob = TokenBlob { - username: username.to_string(), - access_token: access_token.to_string(), - refresh_token: refresh_token.to_string(), + let mut map = match read_map() { + Ok(m) => m, + // If the file is missing or corrupt, start with an empty map so we + // do not block a fresh login. + Err(TokenError::NotFound(_)) => HashMap::new(), + Err(e) => return Err(e), }; - let plaintext = serde_json::to_vec(&blob) - .map_err(|e| TokenError::Keyring(format!("JSON encode: {e}")))?; - let encrypted = with_jvm(|env| { - let key = load_or_create_key(env)?; - encrypt_gcm(env, &key, &plaintext) - })?; + map.insert( + username.to_string(), + TokenBlob { + username: username.to_string(), + access_token: access_token.to_string(), + refresh_token: refresh_token.to_string(), + }, + ); - write_file_bytes(&encrypted) + write_map_inner(&map) } /// Return the stored access token for `username`. /// -/// Returns [`TokenError::NotFound`] if no token has been stored yet. +/// Returns [`TokenError::NotFound`] if no token has been stored for this username. pub fn load_access_token(username: &str) -> Result { - load_blob(username).map(|b| b.access_token) + let mut map = read_map()?; + map.remove(username) + .map(|b| b.access_token) + .ok_or_else(|| TokenError::NotFound(username.to_string())) } /// Return the stored refresh token for `username`. /// -/// Returns [`TokenError::NotFound`] if no token has been stored yet. +/// Returns [`TokenError::NotFound`] if no token has been stored for this username. pub fn load_refresh_token(username: &str) -> Result { - load_blob(username).map(|b| b.refresh_token) + let mut map = read_map()?; + map.remove(username) + .map(|b| b.refresh_token) + .ok_or_else(|| TokenError::NotFound(username.to_string())) } -/// Delete stored tokens and remove the Keystore key for `username`. +/// Delete stored tokens for `username`. +/// +/// If other usernames have stored tokens they are left untouched. +/// When this is the last entry in the map the Keystore key is also removed so +/// a future re-login generates a fresh key. /// /// Missing file or missing Keystore entry are silently ignored. -pub fn delete_tokens(_username: &str) -> Result<(), TokenError> { - if let Some(path) = token_file_path() { - if path.exists() { - std::fs::remove_file(&path) - .map_err(|e| TokenError::Keyring(format!("delete auth_tokens.bin: {e}")))?; +pub fn delete_tokens(username: &str) -> Result<(), TokenError> { + let mut map = match read_map() { + Ok(m) => m, + Err(TokenError::NotFound(_)) => return Ok(()), // nothing to delete + Err(e) => return Err(e), + }; + + map.remove(username); + + if map.is_empty() { + // No more users — remove the file and the Keystore key. + if let Some(path) = token_file_path() { + if path.exists() { + std::fs::remove_file(&path) + .map_err(|e| TokenError::Keyring(format!("delete auth_tokens.bin: {e}")))?; + } } - } - // Remove the Keystore key so a future re-login generates a fresh key. - with_jvm(|env| { - let ks_class = env.find_class("java/security/KeyStore")?; - let ks_type = JValueOwned::from(env.new_string("AndroidKeyStore")?); - let ks = env - .call_static_method( - &ks_class, - "getInstance", - "(Ljava/lang/String;)Ljava/security/KeyStore;", - &[ks_type.borrow()], + // Remove the Keystore key so a future re-login generates a fresh key. + with_jvm(|env| { + let ks_class = env.find_class("java/security/KeyStore")?; + let ks_type = JValueOwned::from(env.new_string("AndroidKeyStore")?); + let ks = env + .call_static_method( + &ks_class, + "getInstance", + "(Ljava/lang/String;)Ljava/security/KeyStore;", + &[ks_type.borrow()], + )? + .l()?; + + let null = JObject::null(); + env.call_method( + &ks, + "load", + "(Ljava/security/KeyStore$LoadStoreParameter;)V", + &[JValue::Object(&null)], )? - .l()?; + .v()?; - let null = JObject::null(); - env.call_method( - &ks, - "load", - "(Ljava/security/KeyStore$LoadStoreParameter;)V", - &[JValue::Object(&null)], - )? - .v()?; - - let alias = JValueOwned::from(env.new_string(KEY_ALIAS)?); - env.call_method(&ks, "deleteEntry", "(Ljava/lang/String;)V", &[alias.borrow()])? - .v() - }) + let alias = JValueOwned::from(env.new_string(KEY_ALIAS)?); + env.call_method(&ks, "deleteEntry", "(Ljava/lang/String;)V", &[alias.borrow()])? + .v() + }) + } else { + // Other users still exist — just rewrite the map without this user. + write_map_inner(&map) + } } diff --git a/solitaire_engine/src/analytics_plugin.rs b/solitaire_engine/src/analytics_plugin.rs index e7f6c07..23f9433 100644 --- a/solitaire_engine/src/analytics_plugin.rs +++ b/solitaire_engine/src/analytics_plugin.rs @@ -45,19 +45,29 @@ pub struct AnalyticsPlugin; impl Plugin for AnalyticsPlugin { fn build(&self, app: &mut App) { app.init_resource::() - .init_resource::() .add_systems(Startup, init_analytics) .add_systems( Update, ( react_to_settings_change, - on_game_won, - on_forfeit, on_new_game, on_achievement_unlocked, - tick_flush_timer, ), ); + + // Build the shared Tokio runtime; skip network flush systems if the OS + // refuses to create threads (resource-limited / sandboxed environments). + match TokioRuntimeResource::new() { + Ok(rt) => { + app.insert_resource(rt).add_systems( + Update, + (on_game_won, on_forfeit, tick_flush_timer), + ); + } + Err(e) => { + bevy::log::warn!("analytics_plugin: Tokio runtime unavailable — analytics flush disabled: {e}"); + } + } } } diff --git a/solitaire_engine/src/avatar_plugin.rs b/solitaire_engine/src/avatar_plugin.rs index 207966c..608eb46 100644 --- a/solitaire_engine/src/avatar_plugin.rs +++ b/solitaire_engine/src/avatar_plugin.rs @@ -48,10 +48,21 @@ pub struct AvatarPlugin; impl Plugin for AvatarPlugin { fn build(&self, app: &mut App) { app.add_message::() - .init_resource::() .init_resource::() .init_resource::() - .add_systems(Update, (handle_avatar_fetch, poll_avatar_task)); + .add_systems(Update, poll_avatar_task); + + // Build the shared Tokio runtime; skip avatar download if the OS + // refuses to create threads (resource-limited / sandboxed environments). + match TokioRuntimeResource::new() { + Ok(rt) => { + app.insert_resource(rt) + .add_systems(Update, handle_avatar_fetch); + } + Err(e) => { + bevy::log::warn!("avatar_plugin: Tokio runtime unavailable — avatar fetch disabled: {e}"); + } + } } } diff --git a/solitaire_engine/src/game_plugin.rs b/solitaire_engine/src/game_plugin.rs index bbe6c51..f3101e7 100644 --- a/solitaire_engine/src/game_plugin.rs +++ b/solitaire_engine/src/game_plugin.rs @@ -32,7 +32,7 @@ use crate::font_plugin::FontResource; use crate::resources::{DragState, GameStateResource, SyncStatusResource}; use crate::ui_modal::{ spawn_modal, spawn_modal_actions, spawn_modal_body_text, spawn_modal_button, - spawn_modal_header, ButtonVariant, + spawn_modal_header, ButtonVariant, ModalScrim, }; use crate::ui_theme; @@ -431,6 +431,7 @@ fn handle_new_game( game_over_screens: Query>, layout: Option>, mut card_transforms: Query<&mut Transform, With>, + scrims: Query<(), With>, ) { for ev in new_game.read() { // If an active game is in progress, intercept and show a confirm dialog. @@ -440,8 +441,12 @@ fn handle_new_game( // duplicates) or if the event itself was already confirmed by the // player pressing Y on the modal — without the `confirmed` check the // modal would be respawned the frame after the despawn flushes. + // Also skip if any other modal scrim is currently open (global guard). let confirm_already_open = !confirm_screens.is_empty(); if needs_confirm && !confirm_already_open && !ev.confirmed { + if !scrims.is_empty() { + return; + } // Despawn any stale game-over overlay before showing confirm dialog. for entity in &game_over_screens { commands.entity(entity).despawn(); @@ -576,10 +581,14 @@ fn spawn_restore_prompt_if_pending( splash: Query<(), With>, existing: Query<(), With>, font_res: Option>, + scrims: Query<(), With>, ) { if pending.0.is_none() || !splash.is_empty() || !existing.is_empty() { return; } + if !scrims.is_empty() { + return; + } spawn_modal( &mut commands, RestorePromptScreen, @@ -1100,6 +1109,7 @@ fn check_no_moves( mut already_fired: Local, game_over_screens: Query>, font_res: Option>, + scrims: Query<(), With>, ) { // Reset the debounce flag on every state change so if something changes // we re-evaluate on the next state change. @@ -1131,8 +1141,9 @@ fn check_no_moves( let no_moves_msg = "No moves available \u{2014} press D to draw or N for a new game"; toast.write(InfoToastEvent(no_moves_msg.to_string())); *already_fired = true; - // Only spawn the overlay if one does not already exist. - if game_over_screens.is_empty() { + // Only spawn the overlay if one does not already exist, and no other + // modal scrim is currently open (global ModalScrim guard). + if game_over_screens.is_empty() && scrims.is_empty() { spawn_game_over_screen(&mut commands, game.0.score, font_res.as_deref()); } } diff --git a/solitaire_engine/src/resources.rs b/solitaire_engine/src/resources.rs index 3e3c986..da6359a 100644 --- a/solitaire_engine/src/resources.rs +++ b/solitaire_engine/src/resources.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use bevy::math::Vec2; -use bevy::prelude::{warn, Resource}; +use bevy::prelude::Resource; use chrono::{DateTime, Utc}; use solitaire_core::game_state::GameState; use solitaire_core::pile::PileType; @@ -146,33 +146,3 @@ impl TokioRuntimeResource { } } -impl Default for TokioRuntimeResource { - fn default() -> Self { - // Try multi-threaded first; fall back to current-thread (single - // worker) if the OS refuses to create additional threads. Neither - // path uses `.expect()` so this never panics at startup. - match tokio::runtime::Builder::new_multi_thread() - .worker_threads(2) - .enable_all() - .build() - { - Ok(rt) => Self(Arc::new(rt)), - Err(e) => { - warn!( - "sync: failed to build multi-thread Tokio runtime ({e}); \ - falling back to current-thread runtime" - ); - // current_thread runtime never spawns OS threads, so it - // succeeds even under tight sandboxing. - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .expect( - "current-thread Tokio runtime failed — \ - the process cannot do any async I/O", - ); - Self(Arc::new(rt)) - } - } - } -} diff --git a/solitaire_server/src/lib.rs b/solitaire_server/src/lib.rs index 5c4fdb7..b0376c3 100644 --- a/solitaire_server/src/lib.rs +++ b/solitaire_server/src/lib.rs @@ -146,7 +146,6 @@ fn build_router_inner(state: AppState, rate_limit: bool) -> Router { .route("/api/account", delete(auth::delete_account)) .route("/api/me", get(auth::get_me)) .route("/api/me/avatar", put(auth::upload_avatar)) - .nest_service("/avatars", ServeDir::new("avatars")) .layer(axum_middleware::from_fn_with_state( state.clone(), middleware::require_auth, @@ -198,7 +197,8 @@ fn build_router_inner(state: AppState, rate_limit: bool) -> Router { .route("/api/daily-challenge", get(challenge::daily_challenge)) .route("/api/replays/recent", get(replays::recent)) .route("/api/replays/{id}", get(replays::get_by_id)) - .route("/health", get(health)); + .route("/health", get(health)) + .nest_service("/avatars", ServeDir::new("avatars")); // Replay web UI: a single HTML page served at `/replays/:id` plus a // ServeDir for the static assets (`web/index.html`, `web/replay.css`, diff --git a/solitaire_server/web/game.js b/solitaire_server/web/game.js index b3909d5..bfd660c 100644 --- a/solitaire_server/web/game.js +++ b/solitaire_server/web/game.js @@ -431,12 +431,12 @@ async function submitReplay(s) { const payload = { schema_version: 1, seed: Math.round(game.seed()), - draw_mode: drawThree ? "draw_three" : "draw_one", - mode: "classic", + draw_mode: drawThree ? "DrawThree" : "DrawOne", + mode: "Classic", time_seconds: elapsedSecs, final_score: s.score, move_count: s.move_count, - recorded_at: new Date().toISOString(), + recorded_at: new Date().toISOString().slice(0, 10), moves: [], }; try { diff --git a/solitaire_sync/src/merge.rs b/solitaire_sync/src/merge.rs index 7379115..3ff09cb 100644 --- a/solitaire_sync/src/merge.rs +++ b/solitaire_sync/src/merge.rs @@ -135,8 +135,21 @@ fn merge_stats( fastest_win_seconds: local.fastest_win_seconds.min(remote.fastest_win_seconds), lifetime_score: local.lifetime_score.max(remote.lifetime_score), best_single_score: local.best_single_score.max(remote.best_single_score), - draw_one_wins: local.draw_one_wins.max(remote.draw_one_wins), - draw_three_wins: local.draw_three_wins.max(remote.draw_three_wins), + // Take per-mode win counts from whichever side contributed `games_won` + // (the side with the higher total). Independent max() calls can push + // draw_one_wins + draw_three_wins above games_won when the two sides + // have complementary win histories (e.g. local has 20 draw-one wins, + // remote has 20 draw-three wins, each with games_won = 20). + draw_one_wins: if local.games_won >= remote.games_won { + local.draw_one_wins + } else { + remote.draw_one_wins + }, + draw_three_wins: if local.games_won >= remote.games_won { + local.draw_three_wins + } else { + remote.draw_three_wins + }, // Per-mode bests. Bests take max; fastest times take a *zero-aware* // min — see [`min_ignore_zero`] for the rationale (0 means "no win // yet" for these fields, unlike the lifetime `fastest_win_seconds` @@ -505,17 +518,55 @@ mod tests { } #[test] - fn stats_draw_mode_wins_take_max() { + fn stats_draw_mode_wins_taken_from_winning_side() { + // Both sides have equal games_won (default 0), so local is chosen (>=). + // Per-mode counts come entirely from that one side — no cross-side max. let mut local = default_payload(); + local.stats.games_won = 25; local.stats.draw_one_wins = 20; local.stats.draw_three_wins = 5; let mut remote = default_payload(); + remote.stats.games_won = 15; remote.stats.draw_one_wins = 15; remote.stats.draw_three_wins = 8; + // local has more wins, so local's per-mode counts are used. let (merged, _) = merge(&local, &remote); + assert_eq!(merged.stats.games_won, 25); assert_eq!(merged.stats.draw_one_wins, 20); - assert_eq!(merged.stats.draw_three_wins, 8); + assert_eq!(merged.stats.draw_three_wins, 5); + assert!( + merged.stats.draw_one_wins + merged.stats.draw_three_wins + <= merged.stats.games_won, + "draw-mode win counts must not exceed total wins" + ); + } + + #[test] + fn merge_stats_draw_mode_wins_do_not_exceed_total() { + // local: 20 draw-one wins, 0 draw-three, games_won = 20 + // remote: 0 draw-one wins, 20 draw-three, games_won = 20 + // Without the fix, independent max() calls yield draw_one=20, draw_three=20, + // games_won=20 — the breakdown sums to 40, double the actual total. + let mut local = default_payload(); + local.stats.games_won = 20; + local.stats.draw_one_wins = 20; + local.stats.draw_three_wins = 0; + + let mut remote = default_payload(); + remote.stats.games_won = 20; + remote.stats.draw_one_wins = 0; + remote.stats.draw_three_wins = 20; + + let (merged, _) = merge(&local, &remote); + assert!( + merged.stats.draw_one_wins + merged.stats.draw_three_wins <= merged.stats.games_won, + "draw-mode win counts must not exceed total wins after merge: \ + draw_one={}, draw_three={}, games_won={}", + merged.stats.draw_one_wins, + merged.stats.draw_three_wins, + merged.stats.games_won, + ); } #[test]