refactor: remove card.rs / card_to_id; use card_game::Card directly (#83)

card_to_id was a frankenstein 0..=51 id shim. Replace it with card_game::Card:
- feedback_anim deal jitter now seeds off a hash of the Card itself
- radial_menu RightClickRadialState.cards: Vec<u32> -> Vec<Card>
- wasm CardSnapshot.id: u32 -> Card (serialises transparently as a plain JS
  number, the same opaque key the renderer already used; new test asserts the
  JSON id field is a number)
- wasm DebugInvariantReport deck-completeness check reworked from a [bool;52]
  index into a HashSet<Card> + Card::new reference deck; the out-of-range check
  is dropped since a Card is always valid

Delete card.rs entirely: the Card/Deck/Rank/Suit re-exports move to the crate
root and the 69 `solitaire_core::card::` import paths flatten to `solitaire_core::`.

The JS card.id is purely an opaque identity key (Map key / dataset.cardId, no
arithmetic, card faces render from rank+suit), so the value change is safe.
cargo test --workspace and clippy --workspace --all-targets -- -D warnings green.

Closes #83

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
funman300
2026-06-11 19:33:47 -07:00
parent 5c992cbdca
commit e0a858d4e8
23 changed files with 132 additions and 137 deletions
+57 -37
View File
@@ -21,7 +21,7 @@
use chrono::NaiveDate;
use solitaire_core::{Foundation, KlondikePile, Tableau};
use serde::{Deserialize, Serialize};
use solitaire_core::card::Suit;
use solitaire_core::{Card, Deck, Rank, Suit};
use solitaire_core::error::MoveError;
use solitaire_core::{DrawStockConfig, game_state::{GameMode, GameState}};
use solitaire_core::klondike_adapter::{
@@ -77,9 +77,11 @@ pub struct StateSnapshot {
/// means the card back is drawn; in that case `suit` and `rank` are
/// still set (so the renderer doesn't need separate "unknown" data),
/// just hidden visually.
#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)]
#[derive(Debug, Clone, Serialize, PartialEq, Eq)]
pub struct CardSnapshot {
pub id: u32,
/// Stable per-card identity for the JS renderer (an opaque key). Serialises
/// as the upstream `Card`'s transparent integer value.
pub id: Card,
/// `"clubs" | "diamonds" | "hearts" | "spades"`.
pub suit: &'static str,
/// 1-13, where 1 is Ace and 13 is King.
@@ -87,14 +89,10 @@ pub struct CardSnapshot {
pub face_up: bool,
}
// Stable 0..=51 card identity, shared with the desktop engine via
// solitaire_core so replay snapshots are identical across platforms.
use solitaire_core::card::card_to_id;
impl From<&(solitaire_core::card::Card, bool)> for CardSnapshot {
fn from((card, face_up): &(solitaire_core::card::Card, bool)) -> Self {
impl From<&(Card, bool)> for CardSnapshot {
fn from((card, face_up): &(Card, bool)) -> Self {
Self {
id: card_to_id(card),
id: card.clone(),
suit: match card.suit() {
Suit::Clubs => "clubs",
Suit::Diamonds => "diamonds",
@@ -318,9 +316,10 @@ pub enum DebugMove {
pub struct DebugInvariantReport {
pub state_ok: bool,
pub total_cards_seen: usize,
pub duplicate_card_ids: Vec<u32>,
pub missing_card_ids: Vec<u32>,
pub out_of_range_card_ids: Vec<u32>,
/// Cards that appeared more than once across all piles.
pub duplicate_cards: Vec<Card>,
/// Cards from the full single-deck set that are absent from the board.
pub missing_cards: Vec<Card>,
pub stock_has_face_up_cards: bool,
pub waste_has_face_down_cards: bool,
pub foundation_has_face_down_cards: bool,
@@ -389,24 +388,15 @@ fn invariant_report_for_game(game: &GameState, legal_moves: &[DebugMove]) -> Deb
game.pile(KlondikePile::Tableau(Tableau::Tableau7)),
];
let mut seen = [false; 52];
let mut duplicate_card_ids = Vec::new();
let mut out_of_range_card_ids = Vec::new();
let mut seen: std::collections::HashSet<Card> = std::collections::HashSet::new();
let mut duplicate_cards = Vec::new();
let mut total_cards_seen = 0_usize;
let mut feed = |cards: &[(solitaire_core::card::Card, bool)]| {
let mut feed = |cards: &[(Card, bool)]| {
for (card, _) in cards {
total_cards_seen += 1;
let id = card_to_id(card);
if id >= 52 {
out_of_range_card_ids.push(id);
continue;
}
let idx = id as usize;
if seen[idx] {
duplicate_card_ids.push(id);
} else {
seen[idx] = true;
if !seen.insert(card.clone()) {
duplicate_cards.push(card.clone());
}
}
};
@@ -420,9 +410,22 @@ fn invariant_report_for_game(game: &GameState, legal_moves: &[DebugMove]) -> Deb
feed(pile);
}
let missing_card_ids = (0_u32..52_u32)
.filter(|id| !seen[*id as usize])
.collect::<Vec<_>>();
// Reference set: the full 52-card single deck, using whichever deck id the
// dealt cards carry. Any of those 52 not on the board is missing.
let deck = seen
.iter()
.next()
.map(|c| c.deck())
.unwrap_or_else(|| Deck::new(0).expect("deck id 0 is valid"));
let mut missing_cards = Vec::new();
for suit in Suit::SUITS {
for rank in Rank::RANKS {
let card = Card::new(deck, suit, rank);
if !seen.contains(&card) {
missing_cards.push(card);
}
}
}
let stock_has_face_up_cards = stock.iter().any(|(_, face_up)| *face_up);
let waste_has_face_down_cards = waste.iter().any(|(_, face_up)| !*face_up);
@@ -444,9 +447,8 @@ fn invariant_report_for_game(game: &GameState, legal_moves: &[DebugMove]) -> Deb
let soft_lock = !game.is_won() && stock.is_empty() && waste.is_empty() && legal_moves.is_empty();
let state_ok = duplicate_card_ids.is_empty()
&& missing_card_ids.is_empty()
&& out_of_range_card_ids.is_empty()
let state_ok = duplicate_cards.is_empty()
&& missing_cards.is_empty()
&& !stock_has_face_up_cards
&& !waste_has_face_down_cards
&& !foundation_has_face_down_cards
@@ -455,9 +457,8 @@ fn invariant_report_for_game(game: &GameState, legal_moves: &[DebugMove]) -> Deb
DebugInvariantReport {
state_ok,
total_cards_seen,
duplicate_card_ids,
missing_card_ids,
out_of_range_card_ids,
duplicate_cards,
missing_cards,
stock_has_face_up_cards,
waste_has_face_down_cards,
foundation_has_face_down_cards,
@@ -893,6 +894,25 @@ mod tests {
use std::collections::HashSet;
use std::fmt::Write;
/// The JS card renderer reads `card.id` as an opaque identity key. After the
/// `card_to_id` removal, `CardSnapshot.id` is a `card_game::Card`, which is
/// `#[serde(transparent)]` over `NonZeroU8` — so it must still serialise as a
/// plain JSON number (the same `Serialize` impl `serde_wasm_bindgen` uses).
#[test]
fn card_snapshot_id_serialises_as_a_plain_number() {
let card = Card::new(Deck::new(0).unwrap(), Suit::Hearts, Rank::RANKS[0]);
let snap = CardSnapshot::from(&(card, true));
let json = serde_json::to_value(&snap).expect("serialise CardSnapshot");
assert!(
json["id"].is_number(),
"card.id must serialise as a JSON number for the JS opaque key, got {:?}",
json["id"]
);
assert_eq!(json["suit"], "hearts");
assert_eq!(json["rank"], 1);
assert_eq!(json["face_up"], true);
}
fn pick_move_index(moves: &[DebugMove]) -> Option<usize> {
if moves.is_empty() {
return None;
@@ -933,7 +953,7 @@ mod tests {
for card in cards {
let _ = write!(
key,
"{}:{}:{},",
"{:?}:{}:{},",
card.id,
card.rank,
if card.face_up { 1 } else { 0 }