From 69c6e88188e3fd6b0c25885c3827380d4dd38264 Mon Sep 17 00:00:00 2001 From: funman300 Date: Sun, 17 May 2026 20:28:46 -0700 Subject: [PATCH] fix(core,sync,data): deterministic pile serialization, undo skip, url-encode bytes, merge_at - Derive PartialOrd+Ord on PileType and sort pile entries in pile_map_serde before serializing so save-file output is deterministic (M-4) - Add #[serde(skip)] to undo_stack so transient undo history is never written to save files, eliminating unnecessary bloat (M-3) - Add merge_at() accepting an explicit resolved_at timestamp so callers can inject the server-side time; merge() wraps it with Utc::now() for backwards compatibility (M-1) - Fix url_encode to percent-encode UTF-8 bytes rather than Unicode codepoints so multi-byte characters produce RFC 3986-compliant output (M-2) Co-Authored-By: Claude Sonnet 4.6 --- solitaire_core/src/game_state.rs | 4 +- solitaire_core/src/pile.rs | 2 +- solitaire_data/src/matomo_client.rs | 10 ++--- solitaire_sync/src/lib.rs | 2 +- solitaire_sync/src/merge.rs | 70 ++++++++++++++++++----------- 5 files changed, 53 insertions(+), 35 deletions(-) diff --git a/solitaire_core/src/game_state.rs b/solitaire_core/src/game_state.rs index 5d03953..dfbce2a 100644 --- a/solitaire_core/src/game_state.rs +++ b/solitaire_core/src/game_state.rs @@ -31,7 +31,8 @@ mod pile_map_serde { use crate::pile::{Pile, PileType}; pub fn serialize(map: &HashMap, s: S) -> Result { - let entries: Vec<(&PileType, &Pile)> = map.iter().collect(); + let mut entries: Vec<(&PileType, &Pile)> = map.iter().collect(); + entries.sort_by_key(|(k, _)| *k); entries.serialize(s) } @@ -154,6 +155,7 @@ pub struct GameState { /// [`GAME_STATE_SCHEMA_VERSION`]. #[serde(default = "schema_v1")] pub schema_version: u32, + #[serde(skip)] undo_stack: VecDeque, } diff --git a/solitaire_core/src/pile.rs b/solitaire_core/src/pile.rs index baa76bb..a95721f 100644 --- a/solitaire_core/src/pile.rs +++ b/solitaire_core/src/pile.rs @@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize}; use crate::card::{Card, Suit}; /// Identifies which pile on the board a set of cards belongs to. -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] pub enum PileType { /// The face-down draw pile. Stock, diff --git a/solitaire_data/src/matomo_client.rs b/solitaire_data/src/matomo_client.rs index b021349..4868d3b 100644 --- a/solitaire_data/src/matomo_client.rs +++ b/solitaire_data/src/matomo_client.rs @@ -111,12 +111,12 @@ impl MatomoClient { } fn url_encode(s: &str) -> String { - s.chars() - .flat_map(|c| match c { - 'A'..='Z' | 'a'..='z' | '0'..='9' | '-' | '_' | '.' | '~' => { - vec![c] + s.bytes() + .flat_map(|b| match b { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => { + vec![b as char] } - c => format!("%{:02X}", c as u32).chars().collect(), + b => format!("%{b:02X}").chars().collect(), }) .collect() } diff --git a/solitaire_sync/src/lib.rs b/solitaire_sync/src/lib.rs index 12f75f0..92b320c 100644 --- a/solitaire_sync/src/lib.rs +++ b/solitaire_sync/src/lib.rs @@ -12,7 +12,7 @@ pub mod progress; pub mod stats; pub use achievements::AchievementRecord; -pub use merge::merge; +pub use merge::{merge, merge_at}; pub use progress::{level_for_xp, PlayerProgress}; pub use stats::StatsSnapshot; diff --git a/solitaire_sync/src/merge.rs b/solitaire_sync/src/merge.rs index 2198318..168681b 100644 --- a/solitaire_sync/src/merge.rs +++ b/solitaire_sync/src/merge.rs @@ -3,13 +3,18 @@ //! All functions are free of I/O and side effects — safe to call from any //! context including unit tests and the Bevy main thread. -use chrono::{NaiveDate, Utc}; +use chrono::{DateTime, NaiveDate, Utc}; use crate::{AchievementRecord, ConflictReport, PlayerProgress, StatsSnapshot, SyncPayload}; use crate::progress::{level_for_xp, DAILY_CHALLENGE_HISTORY_CAP}; /// Merge two [`SyncPayload`]s into a single authoritative result. /// +/// `resolved_at` is recorded as `last_modified` on the merged payload and all +/// sub-structs. Pass an explicit timestamp when the caller needs deterministic +/// output (e.g. server handlers); use [`merge`] as a convenience wrapper that +/// captures the current time automatically. +/// /// The merge strategy is additive and conflict-free for most fields: /// - Counters: take the maximum (games_played, games_won, etc.) /// - Best records: take the minimum for times, maximum for scores/xp @@ -20,6 +25,38 @@ use crate::progress::{level_for_xp, DAILY_CHALLENGE_HISTORY_CAP}; /// Fields that cannot be merged deterministically (e.g. diverged streak /// counts) are recorded in [`ConflictReport`] entries returned alongside /// the merged payload. Data is never silently discarded. +pub fn merge_at( + local: &SyncPayload, + remote: &SyncPayload, + resolved_at: DateTime, +) -> (SyncPayload, Vec) { + let mut conflicts = Vec::new(); + + if local.user_id != remote.user_id { + conflicts.push(ConflictReport { + field: "user_id".to_string(), + local_value: local.user_id.to_string(), + remote_value: remote.user_id.to_string(), + }); + return (local.clone(), conflicts); + } + + let stats = merge_stats(&local.stats, &remote.stats, resolved_at, &mut conflicts); + let achievements = merge_achievements(&local.achievements, &remote.achievements); + let progress = merge_progress(&local.progress, &remote.progress, resolved_at, &mut conflicts); + + let merged = SyncPayload { + user_id: local.user_id, + stats, + achievements, + progress, + last_modified: resolved_at, + }; + + (merged, conflicts) +} + +/// Convenience wrapper around [`merge_at`] that captures the current UTC time. /// /// # Examples /// ``` @@ -45,30 +82,7 @@ use crate::progress::{level_for_xp, DAILY_CHALLENGE_HISTORY_CAP}; /// assert!(conflicts.is_empty()); /// ``` pub fn merge(local: &SyncPayload, remote: &SyncPayload) -> (SyncPayload, Vec) { - let mut conflicts = Vec::new(); - - if local.user_id != remote.user_id { - conflicts.push(ConflictReport { - field: "user_id".to_string(), - local_value: local.user_id.to_string(), - remote_value: remote.user_id.to_string(), - }); - return (local.clone(), conflicts); - } - - let stats = merge_stats(&local.stats, &remote.stats, &mut conflicts); - let achievements = merge_achievements(&local.achievements, &remote.achievements); - let progress = merge_progress(&local.progress, &remote.progress, &mut conflicts); - - let merged = SyncPayload { - user_id: local.user_id, - stats, - achievements, - progress, - last_modified: Utc::now(), - }; - - (merged, conflicts) + merge_at(local, remote, Utc::now()) } // --------------------------------------------------------------------------- @@ -78,6 +92,7 @@ pub fn merge(local: &SyncPayload, remote: &SyncPayload) -> (SyncPayload, Vec, conflicts: &mut Vec, ) -> StatsSnapshot { // win_streak_current cannot be merged deterministically — record conflict @@ -137,7 +152,7 @@ fn merge_stats( local.challenge_fastest_win_seconds, remote.challenge_fastest_win_seconds, ), - last_modified: Utc::now(), + last_modified: resolved_at, } } @@ -222,6 +237,7 @@ fn merge_achievements( fn merge_progress( local: &PlayerProgress, remote: &PlayerProgress, + resolved_at: DateTime, conflicts: &mut Vec, ) -> PlayerProgress { // daily_challenge_streak cannot be merged deterministically. @@ -312,7 +328,7 @@ fn merge_progress( challenge_index, daily_challenge_history, daily_challenge_longest_streak, - last_modified: Utc::now(), + last_modified: resolved_at, } }