funman300/Ferrous-Solitaire
1
1
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 48 Wiki Activity

refactor(core,engine,wasm): canonical FOUNDATIONS/TABLEAUS consts; adopt upstream SUITS/RANKS

Browse Source 
Review findings 1+2 (Quat-underuse lens, 2026-07-06):

- solitaire_core gains pub const FOUNDATIONS / TABLEAUS — the canonical
  iteration source for the upstream pile enums (upstream klondike has no
  Foundation::ALL, and inherent impls cannot be added to foreign types).
  Deletes three identical private const-fn copies (radial_menu,
  table_plugin, input_plugin) and the hand-enumerated variants in
  card_plugin::sync::all_cards and solitaire_wasm.

- Hand-rolled [Suit; 4] / [Rank; 13] arrays replaced with upstream
  Suit::SUITS / Rank::RANKS. The order-sensitive CardImageSet indexing
  is re-keyed through canonical card_plugin::{suit_index, rank_index}
  helpers that match upstream order, with regression tests asserting
  the correspondence — one ordering everywhere instead of three
  divergent local ones.

Net -177 lines. No behaviour change; all consumers go through the
canonical helpers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-07-06 19:58:33 -07:00
parent 15bb136c79
commit 7a5f03987d
11 changed files with 120 additions and 297 deletions
Download Patch File Download Diff File Expand all files Collapse all files
solitaire_engine/src/assets/card_face_svg.rs
+5 -3
View File
@@ -74,9 +74,11 @@ pub const ALL_RANKS: [Rank; 13] = [
Rank::King,
];
/// Every suit in `Clubs, Diamonds, Hearts, Spades` order — matches
/// `card_plugin::load_card_images` so the suit index used here lines
/// up with `CardImageSet.faces[suit]`.
/// Iteration order for the SVG generator and the pin test only —
/// output files are keyed by `suit_filename`, so no runtime index
/// depends on this order. Kept local (not `Suit::SUITS`) because
/// reordering would churn the pinned snapshot ordering for no
/// benefit.
pub const ALL_SUITS: [Suit; 4] = [Suit::Clubs, Suit::Diamonds, Suit::Hearts, Suit::Spades];
/// The rank component of the on-disk filename — `A`, `2`..`10`, `J`,
solitaire_engine/src/card_plugin/mod.rs
+21 -2
View File
@@ -137,6 +137,23 @@ pub const RED_SUIT_COLOUR_HC: Color = Color::srgb(1.000, 0.408, 0.408);
/// high-contrast boost path.
pub const BLACK_SUIT_COLOUR: Color = Color::srgb(0.910, 0.910, 0.910);
/// Canonical outer index of `s` in [`CardImageSet::faces`].
///
/// Derived from the upstream `card_game::Suit` discriminants (0..=3 in
/// `Suit::SUITS` order), so every reader and writer of `faces` computes
/// the same layout from the same source. Three hand-rolled copies of this
/// mapping once lived in card_plugin and theme/plugin and were one
/// reorder away from drawing the wrong art.
pub(crate) const fn suit_index(s: solitaire_core::Suit) -> usize {
s as usize
}
/// Canonical inner index of `r` in [`CardImageSet::faces`] — upstream
/// `card_game::Rank` discriminants are 1..=13 in `Rank::RANKS` order.
pub(crate) const fn rank_index(r: solitaire_core::Rank) -> usize {
r as usize - 1
}
/// Pre-loaded [`Handle<Image>`]s for card face and back PNG textures.
///
/// Loaded once at startup by [`load_card_images`]. When this resource is
@@ -146,8 +163,10 @@ pub const BLACK_SUIT_COLOUR: Color = Color::srgb(0.910, 0.910, 0.910);
pub struct CardImageSet {
/// Per-card face images indexed by `[suit][rank]`.
///
/// Suit order: Clubs=0, Diamonds=1, Hearts=2, Spades=3.
/// Rank order: Ace=0, Two=1 … King=12.
/// Layout is pinned to the upstream declaration order — index with
/// [`suit_index`] / [`rank_index`], never a hand-rolled match.
/// Suit order: `Suit::SUITS` (Spades=0, Hearts=1, Clubs=2, Diamonds=3).
/// Rank order: `Rank::RANKS` (Ace=0 … King=12).
pub faces: [[Handle<Image>; 13]; 4],
/// One handle per unlockable card-back design (indices 0–4). These
/// correspond to the legacy `assets/cards/backs/back_N.png` art, indexed
solitaire_engine/src/card_plugin/sync.rs
+9 -54
View File
@@ -105,25 +105,12 @@ pub(super) fn load_card_images(asset_server: Option<Res<AssetServer>>, mut comma
return;
};
const SUITS: [Suit; 4] = [Suit::Clubs, Suit::Diamonds, Suit::Hearts, Suit::Spades];
const RANKS: [Rank; 13] = [
Rank::Ace,
Rank::Two,
Rank::Three,
Rank::Four,
Rank::Five,
Rank::Six,
Rank::Seven,
Rank::Eight,
Rank::Nine,
Rank::Ten,
Rank::Jack,
Rank::Queen,
Rank::King,
];
// faces[suit_index(s)][rank_index(r)] — see the canonical helpers in
// card_plugin::mod; building from SUITS/RANKS order matches them.
let faces: [[Handle<Image>; 13]; 4] = std::array::from_fn(|si| {
std::array::from_fn(|ri| asset_server.load(card_face_asset_path(RANKS[ri], SUITS[si])))
std::array::from_fn(|ri| {
asset_server.load(card_face_asset_path(Rank::RANKS[ri], Suit::SUITS[si]))
})
});
let backs =
std::array::from_fn(|i| asset_server.load(format!("cards/backs/classic/back_{i}.png")));
@@ -149,27 +136,8 @@ pub(super) fn card_sprite(
) -> Sprite {
if let Some(set) = card_images {
let image = if face_up {
let suit_idx = match card.suit() {
Suit::Clubs => 0,
Suit::Diamonds => 1,
Suit::Hearts => 2,
Suit::Spades => 3,
};
let rank_idx = match card.rank() {
Rank::Ace => 0,
Rank::Two => 1,
Rank::Three => 2,
Rank::Four => 3,
Rank::Five => 4,
Rank::Six => 5,
Rank::Seven => 6,
Rank::Eight => 7,
Rank::Nine => 8,
Rank::Ten => 9,
Rank::Jack => 10,
Rank::Queen => 11,
Rank::King => 12,
};
let suit_idx = suit_index(card.suit());
let rank_idx = rank_index(card.rank());
set.faces[suit_idx][rank_idx].clone()
} else if let Some(theme_back) = &set.theme_back {
// Active theme provides its own back — always wins over the
@@ -519,23 +487,10 @@ pub(super) fn all_cards(game: &GameState) -> Vec<(Card, bool)> {
let mut cards: Vec<(Card, bool)> = Vec::with_capacity(52);
cards.extend(game.stock_cards());
cards.extend(game.waste_cards());
for foundation in [
Foundation::Foundation1,
Foundation::Foundation2,
Foundation::Foundation3,
Foundation::Foundation4,
] {
for foundation in solitaire_core::FOUNDATIONS {
cards.extend(game.pile(KlondikePile::Foundation(foundation)));
}
for tableau in [
Tableau::Tableau1,
Tableau::Tableau2,
Tableau::Tableau3,
Tableau::Tableau4,
Tableau::Tableau5,
Tableau::Tableau6,
Tableau::Tableau7,
] {
for tableau in solitaire_core::TABLEAUS {
cards.extend(game.pile(KlondikePile::Tableau(tableau)));
}
cards
solitaire_engine/src/input_plugin/mod.rs
+9 -27
View File
@@ -28,6 +28,7 @@ use bevy::window::PrimaryWindow;
#[cfg(not(target_os = "android"))]
use bevy::window::{MonitorSelection, WindowMode};
use solitaire_core::{Foundation, KlondikeInstruction, KlondikePile, Tableau};
use solitaire_core::{FOUNDATIONS, TABLEAUS};
use solitaire_core::{Card, Suit};
use solitaire_core::game_state::GameState;
@@ -1329,13 +1330,13 @@ const DOUBLE_TAP_FLASH_SECS: f32 = 0.35;
pub fn best_destination(card: &Card, game: &GameState) -> Option<KlondikePile> {
let source = game.pile_containing_card(card.clone())?;
for foundation in foundations() {
for foundation in FOUNDATIONS {
let dest = KlondikePile::Foundation(foundation);
if game.can_move_cards(&source, &dest, 1) {
return Some(dest);
}
}
for tableau in tableaus() {
for tableau in TABLEAUS {
let dest = KlondikePile::Tableau(tableau);
if game.can_move_cards(&source, &dest, 1) {
return Some(dest);
@@ -1356,7 +1357,7 @@ pub fn best_tableau_destination_for_stack(
game: &GameState,
stack_count: usize,
) -> Option<(KlondikePile, usize)> {
for tableau in tableaus() {
for tableau in TABLEAUS {
let dest = KlondikePile::Tableau(tableau);
if game.can_move_cards(from, &dest, stack_count) {
return Some((dest, stack_count));
@@ -1692,7 +1693,7 @@ pub(crate) fn hint_piles(
fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile)> {
let sources: Vec<KlondikePile> = {
let mut s = vec![KlondikePile::Stock];
for tableau in tableaus() {
for tableau in TABLEAUS {
s.push(KlondikePile::Tableau(tableau));
}
s
@@ -1706,7 +1707,7 @@ fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile)> {
let Some(_card) = from_pile.last().filter(|(_, face_up)| *face_up) else {
continue;
};
for foundation in foundations() {
for foundation in FOUNDATIONS {
let dest = KlondikePile::Foundation(foundation);
if game.can_move_cards(from, &dest, 1) {
hints.push((*from, dest));
@@ -1728,7 +1729,7 @@ fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile)> {
if already_has_foundation_hint {
continue;
}
for tableau in tableaus() {
for tableau in TABLEAUS {
let dest = KlondikePile::Tableau(tableau);
if game.can_move_cards(from, &dest, 1) {
hints.push((*from, dest));
@@ -1742,13 +1743,13 @@ fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile)> {
// should never hint Foundation→Foundation. Here we handle the return path
// separately so the guarded `take_from_foundation` rule is respected.
if game.take_from_foundation {
for foundation in foundations() {
for foundation in FOUNDATIONS {
let from = KlondikePile::Foundation(foundation);
let from_pile = pile_cards(game, &from);
let Some(_card) = from_pile.last().filter(|(_, face_up)| *face_up) else {
continue;
};
for tableau in tableaus() {
for tableau in TABLEAUS {
let dest = KlondikePile::Tableau(tableau);
if game.can_move_cards(&from, &dest, 1) {
hints.push((from, dest));
@@ -1782,26 +1783,7 @@ fn pile_cards(game: &GameState, pile: &KlondikePile) -> Vec<(Card, bool)> {
}
}
const fn foundations() -> [Foundation; 4] {
[
Foundation::Foundation1,
Foundation::Foundation2,
Foundation::Foundation3,
Foundation::Foundation4,
]
}
const fn tableaus() -> [Tableau; 7] {
[
Tableau::Tableau1,
Tableau::Tableau2,
Tableau::Tableau3,
Tableau::Tableau4,
Tableau::Tableau5,
Tableau::Tableau6,
Tableau::Tableau7,
]
}
const fn tableau_number(tableau: Tableau) -> u8 {
match tableau {
solitaire_engine/src/pending_hint.rs
+3 -24
View File
@@ -220,32 +220,11 @@ mod tests {
] {
game.set_test_foundation_cards(foundation, Vec::new());
}
for tableau in [
Tableau::Tableau1,
Tableau::Tableau2,
Tableau::Tableau3,
Tableau::Tableau4,
Tableau::Tableau5,
Tableau::Tableau6,
Tableau::Tableau7,
] {
for tableau in solitaire_core::TABLEAUS {
game.set_test_tableau_cards(tableau, Vec::new());
}
let suits = [Suit::Clubs, Suit::Diamonds, Suit::Hearts, Suit::Spades];
let ranks_below_king = [
Rank::Ace,
Rank::Two,
Rank::Three,
Rank::Four,
Rank::Five,
Rank::Six,
Rank::Seven,
Rank::Eight,
Rank::Nine,
Rank::Ten,
Rank::Jack,
Rank::Queen,
];
let suits = Suit::SUITS;
let ranks_below_king = &Rank::RANKS[..12]; // everything below King
for (foundation, suit) in [
Foundation::Foundation1,
Foundation::Foundation2,
solitaire_engine/src/radial_menu.rs
+3 -21
View File
@@ -48,6 +48,7 @@ use bevy::math::Vec2;
use bevy::prelude::*;
use bevy::window::PrimaryWindow;
use solitaire_core::{Foundation, KlondikePile, Tableau};
use solitaire_core::{FOUNDATIONS, TABLEAUS};
use solitaire_core::Card;
use solitaire_core::game_state::GameState;
@@ -254,13 +255,13 @@ pub fn legal_destinations_for_card(
game: &GameState,
) -> Vec<KlondikePile> {
let mut out = Vec::new();
for foundation in foundations() {
for foundation in FOUNDATIONS {
let dest = KlondikePile::Foundation(foundation);
if game.can_move_cards(source_pile, &dest, 1) {
out.push(dest);
}
}
for tableau in tableaus() {
for tableau in TABLEAUS {
let dest = KlondikePile::Tableau(tableau);
if game.can_move_cards(source_pile, &dest, 1) {
out.push(dest);
@@ -360,26 +361,7 @@ fn pile_cards(game: &GameState, pile: &KlondikePile) -> Vec<(Card, bool)> {
}
const fn foundations() -> [Foundation; 4] {
[
Foundation::Foundation1,
Foundation::Foundation2,
Foundation::Foundation3,
Foundation::Foundation4,
]
}
const fn tableaus() -> [Tableau; 7] {
[
Tableau::Tableau1,
Tableau::Tableau2,
Tableau::Tableau3,
Tableau::Tableau4,
Tableau::Tableau5,
Tableau::Tableau6,
Tableau::Tableau7,
]
}
/// Builds the `(destination, anchor)` list for a fresh radial open.
///
solitaire_engine/src/table_plugin.rs
+6 -23
View File
@@ -6,7 +6,8 @@
use bevy::prelude::*;
use bevy::window::WindowResized;
use solitaire_core::{Foundation, KlondikePile, Tableau};
use solitaire_core::KlondikePile;
use solitaire_core::{FOUNDATIONS, TABLEAUS};
use solitaire_core::Suit;
use crate::events::{HintVisualEvent, StateChangedEvent};
@@ -280,10 +281,10 @@ fn spawn_pile_markers(commands: &mut Commands, layout: &Layout) {
let mut piles: Vec<KlondikePile> = Vec::with_capacity(12);
piles.push(KlondikePile::Stock);
for foundation in foundations() {
for foundation in FOUNDATIONS {
piles.push(KlondikePile::Foundation(foundation));
}
for tableau in tableaus() {
for tableau in TABLEAUS {
piles.push(KlondikePile::Tableau(tableau));
}
@@ -576,31 +577,13 @@ fn pile_cards(
}
}
const fn foundations() -> [Foundation; 4] {
[
Foundation::Foundation1,
Foundation::Foundation2,
Foundation::Foundation3,
Foundation::Foundation4,
]
}
const fn tableaus() -> [Tableau; 7] {
[
Tableau::Tableau1,
Tableau::Tableau2,
Tableau::Tableau3,
Tableau::Tableau4,
Tableau::Tableau5,
Tableau::Tableau6,
Tableau::Tableau7,
]
}
#[cfg(test)]
mod tests {
use super::*;
use crate::game_plugin::GamePlugin;
use solitaire_core::{Foundation, Tableau};
/// Minimal headless app — omits windowing so pile markers are spawned with
/// the default 1280×800 layout and no camera is created.
@@ -940,7 +923,7 @@ mod tests {
#[test]
fn suit_symbol_all_four_are_distinct() {
let symbols: Vec<&str> = [Suit::Spades, Suit::Hearts, Suit::Diamonds, Suit::Clubs]
let symbols: Vec<&str> = Suit::SUITS
.iter()
.map(suit_symbol)
.collect();
solitaire_engine/src/theme/mod.rs
+4 -16
View File
@@ -62,22 +62,10 @@ impl CardKey {
/// Iterator over all 52 valid keys, in suit-major / rank-ascending order.
/// Used to enumerate the manifest's required entries.
pub fn all() -> impl Iterator<Item = CardKey> {
const SUITS: [Suit; 4] = [Suit::Clubs, Suit::Diamonds, Suit::Hearts, Suit::Spades];
const RANKS: [Rank; 13] = [
Rank::Ace,
Rank::Two,
Rank::Three,
Rank::Four,
Rank::Five,
Rank::Six,
Rank::Seven,
Rank::Eight,
Rank::Nine,
Rank::Ten,
Rank::Jack,
Rank::Queen,
Rank::King,
];
// Order-independent enumeration — consumers check completeness and
// round-trips, never positions.
const SUITS: [Suit; 4] = Suit::SUITS;
const RANKS: [Rank; 13] = Rank::RANKS;
SUITS
.into_iter()
.flat_map(|s| RANKS.into_iter().map(move |r| CardKey::new(s, r)))
solitaire_engine/src/theme/plugin.rs
+11 -58
View File
@@ -17,7 +17,7 @@ use solitaire_core::{Rank, Suit};
use crate::assets::{
bundled_theme_url, classic_theme_svg_bytes, dark_theme_svg_bytes, rasterize_svg, user_theme_dir,
};
use crate::card_plugin::CardImageSet;
use crate::card_plugin::{CardImageSet, rank_index, suit_index};
use crate::events::StateChangedEvent;
use super::loader::CardThemeLoader;
@@ -242,22 +242,8 @@ fn sync_card_image_set_with_active_theme(
/// `theme_back` when present, so writing here is sufficient to make
/// every face-down card pick up the theme's art on the next sync.
fn apply_theme_to_card_image_set(theme: &CardTheme, image_set: &mut CardImageSet) {
for suit in [Suit::Clubs, Suit::Diamonds, Suit::Hearts, Suit::Spades] {
for rank in [
Rank::Ace,
Rank::Two,
Rank::Three,
Rank::Four,
Rank::Five,
Rank::Six,
Rank::Seven,
Rank::Eight,
Rank::Nine,
Rank::Ten,
Rank::Jack,
Rank::Queen,
Rank::King,
] {
for suit in Suit::SUITS {
for rank in Rank::RANKS {
if let Some(handle) = theme.faces.get(&CardKey::new(suit, rank)) {
image_set.faces[suit_index(suit)][rank_index(rank)] = handle.clone();
}
@@ -266,36 +252,7 @@ fn apply_theme_to_card_image_set(theme: &CardTheme, image_set: &mut CardImageSet
image_set.theme_back = Some(theme.back.clone());
}
/// Index used by [`CardImageSet::faces`] for a given suit. Mirrors
/// the `card_plugin` doc comment: Clubs=0, Diamonds=1, Hearts=2, Spades=3.
const fn suit_index(s: Suit) -> usize {
match s {
Suit::Clubs => 0,
Suit::Diamonds => 1,
Suit::Hearts => 2,
Suit::Spades => 3,
}
}
/// Index used by [`CardImageSet::faces`] for a given rank.
/// Ace=0, Two=1 … King=12.
const fn rank_index(r: Rank) -> usize {
match r {
Rank::Ace => 0,
Rank::Two => 1,
Rank::Three => 2,
Rank::Four => 3,
Rank::Five => 4,
Rank::Six => 5,
Rank::Seven => 6,
Rank::Eight => 7,
Rank::Nine => 8,
Rank::Ten => 9,
Rank::Jack => 10,
Rank::Queen => 11,
Rank::King => 12,
}
}
/// Switches the active theme to the one served at
/// `themes://<theme_id>/theme.ron`. Returns the new `Handle<CardTheme>`
@@ -446,21 +403,17 @@ mod tests {
}
#[test]
fn suit_index_ranges_match_card_plugin_layout() {
assert_eq!(suit_index(Suit::Clubs), 0);
assert_eq!(suit_index(Suit::Diamonds), 1);
assert_eq!(suit_index(Suit::Hearts), 2);
assert_eq!(suit_index(Suit::Spades), 3);
fn suit_index_matches_upstream_suits_order() {
for (i, s) in Suit::SUITS.iter().enumerate() {
assert_eq!(suit_index(*s), i, "faces outer layout = Suit::SUITS order");
}
}
#[test]
fn rank_index_starts_at_ace_zero_and_ends_at_king_twelve() {
assert_eq!(rank_index(Rank::Ace), 0);
assert_eq!(rank_index(Rank::Two), 1);
assert_eq!(rank_index(Rank::Ten), 9);
assert_eq!(rank_index(Rank::Jack), 10);
assert_eq!(rank_index(Rank::Queen), 11);
assert_eq!(rank_index(Rank::King), 12);
fn rank_index_matches_upstream_ranks_order() {
for (i, r) in Rank::RANKS.iter().enumerate() {
assert_eq!(rank_index(*r), i, "faces inner layout = Rank::RANKS order");
}
}
#[test]