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 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-17 20:28:46 -07:00
parent 1eb40433a9
commit 69c6e88188
5 changed files with 53 additions and 35 deletions
+3 -1
View File
@@ -31,7 +31,8 @@ mod pile_map_serde {
use crate::pile::{Pile, PileType}; use crate::pile::{Pile, PileType};
pub fn serialize<S: Serializer>(map: &HashMap<PileType, Pile>, s: S) -> Result<S::Ok, S::Error> { pub fn serialize<S: Serializer>(map: &HashMap<PileType, Pile>, s: S) -> Result<S::Ok, S::Error> {
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) entries.serialize(s)
} }
@@ -154,6 +155,7 @@ pub struct GameState {
/// [`GAME_STATE_SCHEMA_VERSION`]. /// [`GAME_STATE_SCHEMA_VERSION`].
#[serde(default = "schema_v1")] #[serde(default = "schema_v1")]
pub schema_version: u32, pub schema_version: u32,
#[serde(skip)]
undo_stack: VecDeque<StateSnapshot>, undo_stack: VecDeque<StateSnapshot>,
} }
+1 -1
View File
@@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize};
use crate::card::{Card, Suit}; use crate::card::{Card, Suit};
/// Identifies which pile on the board a set of cards belongs to. /// 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 { pub enum PileType {
/// The face-down draw pile. /// The face-down draw pile.
Stock, Stock,
+5 -5
View File
@@ -111,12 +111,12 @@ impl MatomoClient {
} }
fn url_encode(s: &str) -> String { fn url_encode(s: &str) -> String {
s.chars() s.bytes()
.flat_map(|c| match c { .flat_map(|b| match b {
'A'..='Z' | 'a'..='z' | '0'..='9' | '-' | '_' | '.' | '~' => { b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
vec![c] vec![b as char]
} }
c => format!("%{:02X}", c as u32).chars().collect(), b => format!("%{b:02X}").chars().collect(),
}) })
.collect() .collect()
} }
+1 -1
View File
@@ -12,7 +12,7 @@ pub mod progress;
pub mod stats; pub mod stats;
pub use achievements::AchievementRecord; pub use achievements::AchievementRecord;
pub use merge::merge; pub use merge::{merge, merge_at};
pub use progress::{level_for_xp, PlayerProgress}; pub use progress::{level_for_xp, PlayerProgress};
pub use stats::StatsSnapshot; pub use stats::StatsSnapshot;
+43 -27
View File
@@ -3,13 +3,18 @@
//! All functions are free of I/O and side effects — safe to call from any //! All functions are free of I/O and side effects — safe to call from any
//! context including unit tests and the Bevy main thread. //! 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::{AchievementRecord, ConflictReport, PlayerProgress, StatsSnapshot, SyncPayload};
use crate::progress::{level_for_xp, DAILY_CHALLENGE_HISTORY_CAP}; use crate::progress::{level_for_xp, DAILY_CHALLENGE_HISTORY_CAP};
/// Merge two [`SyncPayload`]s into a single authoritative result. /// 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: /// The merge strategy is additive and conflict-free for most fields:
/// - Counters: take the maximum (games_played, games_won, etc.) /// - Counters: take the maximum (games_played, games_won, etc.)
/// - Best records: take the minimum for times, maximum for scores/xp /// - 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 /// Fields that cannot be merged deterministically (e.g. diverged streak
/// counts) are recorded in [`ConflictReport`] entries returned alongside /// counts) are recorded in [`ConflictReport`] entries returned alongside
/// the merged payload. Data is never silently discarded. /// the merged payload. Data is never silently discarded.
pub fn merge_at(
local: &SyncPayload,
remote: &SyncPayload,
resolved_at: DateTime<Utc>,
) -> (SyncPayload, Vec<ConflictReport>) {
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 /// # Examples
/// ``` /// ```
@@ -45,30 +82,7 @@ use crate::progress::{level_for_xp, DAILY_CHALLENGE_HISTORY_CAP};
/// assert!(conflicts.is_empty()); /// assert!(conflicts.is_empty());
/// ``` /// ```
pub fn merge(local: &SyncPayload, remote: &SyncPayload) -> (SyncPayload, Vec<ConflictReport>) { pub fn merge(local: &SyncPayload, remote: &SyncPayload) -> (SyncPayload, Vec<ConflictReport>) {
let mut conflicts = Vec::new(); merge_at(local, remote, Utc::now())
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)
} }
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@@ -78,6 +92,7 @@ pub fn merge(local: &SyncPayload, remote: &SyncPayload) -> (SyncPayload, Vec<Con
fn merge_stats( fn merge_stats(
local: &StatsSnapshot, local: &StatsSnapshot,
remote: &StatsSnapshot, remote: &StatsSnapshot,
resolved_at: DateTime<Utc>,
conflicts: &mut Vec<ConflictReport>, conflicts: &mut Vec<ConflictReport>,
) -> StatsSnapshot { ) -> StatsSnapshot {
// win_streak_current cannot be merged deterministically — record conflict // win_streak_current cannot be merged deterministically — record conflict
@@ -137,7 +152,7 @@ fn merge_stats(
local.challenge_fastest_win_seconds, local.challenge_fastest_win_seconds,
remote.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( fn merge_progress(
local: &PlayerProgress, local: &PlayerProgress,
remote: &PlayerProgress, remote: &PlayerProgress,
resolved_at: DateTime<Utc>,
conflicts: &mut Vec<ConflictReport>, conflicts: &mut Vec<ConflictReport>,
) -> PlayerProgress { ) -> PlayerProgress {
// daily_challenge_streak cannot be merged deterministically. // daily_challenge_streak cannot be merged deterministically.
@@ -312,7 +328,7 @@ fn merge_progress(
challenge_index, challenge_index,
daily_challenge_history, daily_challenge_history,
daily_challenge_longest_streak, daily_challenge_longest_streak,
last_modified: Utc::now(), last_modified: resolved_at,
} }
} }