refactor(core): explicit Rank discriminants, checked arithmetic, possible_instructions
Build and Deploy / build-and-push (push) Successful in 3m55s
Android Release / build-apk (push) Successful in 4m37s

- Add Rank=1..13 explicit discriminants so `rank as u8 == rank.value()`; collapse 13-arm value() match to `self as u8`
- Add Rank::RANKS and Suit::SUITS iteration constants
- Add Rank::checked_add / checked_sub (const fn, type-safe boundary enforcement); update rules.rs to use them
- Add GameState::possible_instructions() enumerating all valid move_cards triples (foundation for hints/solver)
- Fix waste buffer card peeking through during draw-slide animation by setting Visibility::Hidden on the buffer entity in sync_cards

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-18 13:25:13 -07:00
parent e1c8ae0743
commit 456b4d42e3
4 changed files with 314 additions and 44 deletions
+88 -33
View File
@@ -10,6 +10,9 @@ pub enum Suit {
} }
impl Suit { impl Suit {
/// All four suits in declaration order.
pub const SUITS: [Self; 4] = [Self::Clubs, Self::Diamonds, Self::Hearts, Self::Spades];
/// Returns `true` for red suits (Diamonds, Hearts). /// Returns `true` for red suits (Diamonds, Hearts).
pub fn is_red(self) -> bool { pub fn is_red(self) -> bool {
matches!(self, Suit::Diamonds | Suit::Hearts) matches!(self, Suit::Diamonds | Suit::Hearts)
@@ -24,38 +27,63 @@ impl Suit {
/// Card rank, Ace through King. /// Card rank, Ace through King.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub enum Rank { pub enum Rank {
Ace, Ace = 1,
Two, Two = 2,
Three, Three = 3,
Four, Four = 4,
Five, Five = 5,
Six, Six = 6,
Seven, Seven = 7,
Eight, Eight = 8,
Nine, Nine = 9,
Ten, Ten = 10,
Jack, Jack = 11,
Queen, Queen = 12,
King, King = 13,
} }
impl Rank { impl Rank {
/// All thirteen ranks in ascending order.
pub const RANKS: [Self; 13] = [
Self::Ace, Self::Two, Self::Three, Self::Four, Self::Five,
Self::Six, Self::Seven, Self::Eight, Self::Nine, Self::Ten,
Self::Jack, Self::Queen, Self::King,
];
/// Numeric value: Ace = 1, King = 13. /// Numeric value: Ace = 1, King = 13.
pub fn value(self) -> u8 { pub fn value(self) -> u8 {
match self { self as u8
Rank::Ace => 1, }
Rank::Two => 2,
Rank::Three => 3, const fn new(n: u8) -> Option<Self> {
Rank::Four => 4, match n {
Rank::Five => 5, 1 => Some(Self::Ace),
Rank::Six => 6, 2 => Some(Self::Two),
Rank::Seven => 7, 3 => Some(Self::Three),
Rank::Eight => 8, 4 => Some(Self::Four),
Rank::Nine => 9, 5 => Some(Self::Five),
Rank::Ten => 10, 6 => Some(Self::Six),
Rank::Jack => 11, 7 => Some(Self::Seven),
Rank::Queen => 12, 8 => Some(Self::Eight),
Rank::King => 13, 9 => Some(Self::Nine),
10 => Some(Self::Ten),
11 => Some(Self::Jack),
12 => Some(Self::Queen),
13 => Some(Self::King),
_ => None,
}
}
/// Returns the rank `n` steps above `self`, or `None` if it would exceed King.
pub const fn checked_add(self, n: u8) -> Option<Self> {
Self::new((self as u8).saturating_add(n))
}
/// Returns the rank `n` steps below `self`, or `None` if it would go below Ace.
pub const fn checked_sub(self, n: u8) -> Option<Self> {
match (self as u8).checked_sub(n) {
Some(v) => Self::new(v),
None => None,
} }
} }
} }
@@ -79,16 +107,43 @@ mod tests {
#[test] #[test]
fn rank_values_are_sequential() { fn rank_values_are_sequential() {
let ranks = [ for (i, r) in Rank::RANKS.iter().enumerate() {
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 (i, r) in ranks.iter().enumerate() {
assert_eq!(r.value(), (i + 1) as u8); assert_eq!(r.value(), (i + 1) as u8);
} }
} }
#[test]
fn rank_as_u8_matches_value() {
for r in Rank::RANKS {
assert_eq!(r as u8, r.value());
}
}
#[test]
fn rank_checked_add_boundary() {
assert_eq!(Rank::King.checked_add(1), None);
assert_eq!(Rank::Queen.checked_add(1), Some(Rank::King));
assert_eq!(Rank::Ace.checked_add(1), Some(Rank::Two));
assert_eq!(Rank::Five.checked_add(3), Some(Rank::Eight));
}
#[test]
fn rank_checked_sub_boundary() {
assert_eq!(Rank::Ace.checked_sub(1), None);
assert_eq!(Rank::Two.checked_sub(1), Some(Rank::Ace));
assert_eq!(Rank::King.checked_sub(1), Some(Rank::Queen));
assert_eq!(Rank::Five.checked_sub(3), Some(Rank::Two));
}
#[test]
fn suit_suits_contains_all_four() {
assert_eq!(Suit::SUITS.len(), 4);
assert!(Suit::SUITS.contains(&Suit::Clubs));
assert!(Suit::SUITS.contains(&Suit::Diamonds));
assert!(Suit::SUITS.contains(&Suit::Hearts));
assert!(Suit::SUITS.contains(&Suit::Spades));
}
#[test] #[test]
fn suit_red_and_black_are_complementary() { fn suit_red_and_black_are_complementary() {
for suit in [Suit::Clubs, Suit::Diamonds, Suit::Hearts, Suit::Spades] { for suit in [Suit::Clubs, Suit::Diamonds, Suit::Hearts, Suit::Spades] {
+159
View File
@@ -440,6 +440,91 @@ impl GameState {
}) })
} }
/// Returns all currently valid `move_cards` calls as `(from, to, count)` triples.
///
/// Does not include stock draws — callers check `piles[&PileType::Stock]` directly.
/// Every returned triple is guaranteed to succeed when passed to `move_cards`.
pub fn possible_instructions(&self) -> Vec<(PileType, PileType, usize)> {
if self.is_won {
return Vec::new();
}
let mut moves = Vec::new();
// Waste top card → foundation or tableau
if let Some(waste_top) = self.piles.get(&PileType::Waste).and_then(|p| p.cards.last()) {
for slot in 0..4_u8 {
if let Some(f) = self.piles.get(&PileType::Foundation(slot))
&& can_place_on_foundation(waste_top, f)
{
moves.push((PileType::Waste, PileType::Foundation(slot), 1));
}
}
for dst in 0..7_usize {
if let Some(t) = self.piles.get(&PileType::Tableau(dst))
&& can_place_on_tableau(waste_top, t)
{
moves.push((PileType::Waste, PileType::Tableau(dst), 1));
}
}
}
// Tableau sources
for src in 0..7_usize {
let Some(src_pile) = self.piles.get(&PileType::Tableau(src)) else { continue };
if src_pile.cards.is_empty() {
continue;
}
let run_len = src_pile.cards.iter().rev().take_while(|c| c.face_up).count();
if run_len == 0 {
continue;
}
for count in 1..=run_len {
let seq_start = src_pile.cards.len() - count;
if !is_valid_tableau_sequence(&src_pile.cards[seq_start..]) {
continue;
}
let bottom = &src_pile.cards[seq_start];
if count == 1 {
for slot in 0..4_u8 {
if let Some(f) = self.piles.get(&PileType::Foundation(slot))
&& can_place_on_foundation(bottom, f)
{
moves.push((PileType::Tableau(src), PileType::Foundation(slot), 1));
}
}
}
for dst in 0..7_usize {
if dst == src {
continue;
}
if let Some(t) = self.piles.get(&PileType::Tableau(dst))
&& can_place_on_tableau(bottom, t)
{
moves.push((PileType::Tableau(src), PileType::Tableau(dst), count));
}
}
}
}
// Foundation top → tableau (only when house rule is enabled)
if self.take_from_foundation {
for slot in 0..4_u8 {
let Some(f) = self.piles.get(&PileType::Foundation(slot)) else { continue };
let Some(top) = f.cards.last() else { continue };
for dst in 0..7_usize {
if let Some(t) = self.piles.get(&PileType::Tableau(dst))
&& can_place_on_tableau(top, t)
{
moves.push((PileType::Foundation(slot), PileType::Tableau(dst), 1));
}
}
}
}
moves
}
/// Returns the next `(from, to)` move that advances auto-complete, or /// Returns the next `(from, to)` move that advances auto-complete, or
/// `None` if no such move exists (or `is_auto_completable` is not set). /// `None` if no such move exists (or `is_auto_completable` is not set).
/// ///
@@ -1366,4 +1451,78 @@ mod tests {
.unwrap_err(); .unwrap_err();
assert!(matches!(err, MoveError::RuleViolation(_))); assert!(matches!(err, MoveError::RuleViolation(_)));
} }
// --- possible_instructions ---
#[test]
fn possible_instructions_empty_when_won() {
let mut g = new_game();
g.is_won = true;
assert!(g.possible_instructions().is_empty());
}
#[test]
fn possible_instructions_includes_ace_to_foundation() {
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();
}
g.piles.get_mut(&PileType::Tableau(0)).unwrap().cards.push(Card {
id: 1, suit: Suit::Clubs, rank: Rank::Ace, face_up: true,
});
let moves = g.possible_instructions();
assert!(
moves.contains(&(PileType::Tableau(0), PileType::Foundation(0), 1)),
"Ace must be moveable to empty foundation slot 0; got {moves:?}"
);
}
#[test]
fn possible_instructions_all_valid_on_fresh_game() {
// Every triple returned must actually succeed when applied to a clone of the state.
let g = new_game();
for (from, to, count) in g.possible_instructions() {
let mut clone = g.clone();
assert!(
clone.move_cards(from.clone(), to.clone(), count).is_ok(),
"instruction ({from:?}, {to:?}, {count}) from possible_instructions must succeed"
);
}
}
#[test]
fn possible_instructions_no_face_down_sources() {
let g = new_game();
for (from, _, count) in g.possible_instructions() {
if let PileType::Tableau(i) = from {
let pile = &g.piles[&PileType::Tableau(i)];
let run_len = pile.cards.iter().rev().take_while(|c| c.face_up).count();
assert!(
count <= run_len,
"count {count} exceeds face-up run {run_len} for Tableau({i})"
);
}
}
}
#[test]
fn possible_instructions_waste_top_included() {
let mut g = new_game();
// Clear board, put a King on waste, and an empty tableau pile — waste→tableau must appear.
g.piles.get_mut(&PileType::Stock).unwrap().cards.clear();
for i in 0..7 {
g.piles.get_mut(&PileType::Tableau(i)).unwrap().cards.clear();
}
g.piles.get_mut(&PileType::Waste).unwrap().cards.push(Card {
id: 99, suit: Suit::Spades, rank: Rank::King, face_up: true,
});
let moves = g.possible_instructions();
// King goes on any of the 7 empty tableau piles
assert!(
(0..7).any(|dst| moves.contains(&(PileType::Waste, PileType::Tableau(dst), 1))),
"King on waste must be moveable to an empty tableau column"
);
}
} }
+6 -6
View File
@@ -1,4 +1,4 @@
use crate::card::Card; use crate::card::{Card, Rank};
use crate::pile::Pile; use crate::pile::Pile;
/// Returns `true` if `card` can be placed on the foundation `pile`. /// Returns `true` if `card` can be placed on the foundation `pile`.
@@ -12,8 +12,8 @@ use crate::pile::Pile;
#[must_use] #[must_use]
pub fn can_place_on_foundation(card: &Card, pile: &Pile) -> bool { pub fn can_place_on_foundation(card: &Card, pile: &Pile) -> bool {
match pile.cards.last() { match pile.cards.last() {
None => card.rank.value() == 1, None => card.rank == Rank::Ace,
Some(top) => card.suit == top.suit && card.rank.value() == top.rank.value() + 1, Some(top) => card.suit == top.suit && card.rank.checked_sub(1) == Some(top.rank),
} }
} }
@@ -23,10 +23,10 @@ pub fn can_place_on_foundation(card: &Card, pile: &Pile) -> bool {
#[must_use] #[must_use]
pub fn can_place_on_tableau(card: &Card, pile: &Pile) -> bool { pub fn can_place_on_tableau(card: &Card, pile: &Pile) -> bool {
match pile.cards.last() { match pile.cards.last() {
None => card.rank.value() == 13, None => card.rank == Rank::King,
Some(top) => { Some(top) => {
top.face_up top.face_up
&& card.rank.value() + 1 == top.rank.value() && card.rank.checked_add(1) == Some(top.rank)
&& card.suit.is_red() != top.suit.is_red() && card.suit.is_red() != top.suit.is_red()
} }
} }
@@ -41,7 +41,7 @@ pub fn can_place_on_tableau(card: &Card, pile: &Pile) -> bool {
#[must_use] #[must_use]
pub fn is_valid_tableau_sequence(cards: &[Card]) -> bool { pub fn is_valid_tableau_sequence(cards: &[Card]) -> bool {
cards.windows(2).all(|w| { cards.windows(2).all(|w| {
w[0].rank.value() == w[1].rank.value() + 1 && w[0].suit.is_red() != w[1].suit.is_red() w[0].rank.checked_sub(1) == Some(w[1].rank) && w[0].suit.is_red() != w[1].suit.is_red()
}) })
} }
+61 -5
View File
@@ -691,6 +691,22 @@ fn sync_cards(
) { ) {
let positions = card_positions(game, layout); let positions = card_positions(game, layout);
// The waste buffer card exists only to keep its entity alive while the new
// top card's slide animation plays — it must never be visible to the player.
// Without this, the buffer sits at waste_base uncovered during the animation
// and its rank/suit peek behind the incoming card.
let waste_buffer_id: Option<u32> = {
let visible = match game.draw_mode {
DrawMode::DrawOne => 1_usize,
DrawMode::DrawThree => 3_usize,
};
game.piles
.get(&PileType::Waste)
.filter(|w| w.cards.len() > visible)
.and_then(|w| w.cards.get(w.cards.len().saturating_sub(visible + 1)))
.map(|c| c.id)
};
// Map card_id -> (Entity, current_translation, has_card_animation) for // Map card_id -> (Entity, current_translation, has_card_animation) for
// in-place updates. The `has_card_animation` flag lets `update_card_entity` // in-place updates. The `has_card_animation` flag lets `update_card_entity`
// skip the snap/slide path on cards that are already being driven by a // skip the snap/slide path on cards that are already being driven by a
@@ -711,17 +727,26 @@ fn sync_cards(
} }
} }
// For each card in the current state: spawn or update its entity. // For each card in the current state: spawn or update its entity, then
// apply visibility. The waste buffer card is hidden so it cannot peek
// behind the incoming top card during the draw slide animation.
for (card, position, z) in positions { for (card, position, z) in positions {
match existing.get(&card.id) { let entity = match existing.get(&card.id) {
Some(&(entity, cur, has_anim)) => { Some(&(entity, cur, has_anim)) => {
update_card_entity( update_card_entity(
&mut commands, entity, card, position, z, layout, &mut commands, entity, card, position, z, layout,
slide_secs, back_colour, color_blind, high_contrast, cur, has_anim, card_images, selected_back, font_handle, slide_secs, back_colour, color_blind, high_contrast, cur, has_anim, card_images, selected_back, font_handle,
) );
entity
} }
None => spawn_card_entity(&mut commands, card, position, z, layout, back_colour, color_blind, high_contrast, card_images, selected_back, font_handle), None => spawn_card_entity(&mut commands, card, position, z, layout, back_colour, color_blind, high_contrast, card_images, selected_back, font_handle),
} };
let visibility = if waste_buffer_id == Some(card.id) {
Visibility::Hidden
} else {
Visibility::Inherited
};
commands.entity(entity).insert(visibility);
} }
} }
@@ -831,7 +856,7 @@ fn spawn_card_entity(
card_images: Option<&CardImageSet>, card_images: Option<&CardImageSet>,
selected_back: usize, selected_back: usize,
font_handle: Option<&Handle<Font>>, font_handle: Option<&Handle<Font>>,
) { ) -> Entity {
let sprite = card_sprite(card, layout.card_size, back_colour, card_images, selected_back); let sprite = card_sprite(card, layout.card_size, back_colour, card_images, selected_back);
let mut entity = commands.spawn(( let mut entity = commands.spawn((
@@ -840,6 +865,7 @@ fn spawn_card_entity(
Transform::from_xyz(pos.x, pos.y, z), Transform::from_xyz(pos.x, pos.y, z),
Visibility::default(), Visibility::default(),
)); ));
let entity_id = entity.id();
// Every card gets a subtle drop-shadow child so the play surface reads // Every card gets a subtle drop-shadow child so the play surface reads
// as physical instead of flat. Spawned in idle state; the drag-tracking // as physical instead of flat. Spawned in idle state; the drag-tracking
// system retunes its offset / alpha when this card joins the dragged // system retunes its offset / alpha when this card joins the dragged
@@ -880,6 +906,7 @@ fn spawn_card_entity(
// Suppress unused-variable warning when not building for Android. // Suppress unused-variable warning when not building for Android.
#[cfg(not(target_os = "android"))] #[cfg(not(target_os = "android"))]
let _ = font_handle; let _ = font_handle;
entity_id
} }
#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)]
@@ -2357,6 +2384,35 @@ mod tests {
} }
} }
/// The waste buffer card (slot below top) must be at the *same* XY as the
/// top card so that hiding it (`Visibility::Hidden`) leaves no visible gap.
#[test]
fn waste_draw_one_buffer_card_at_same_xy_as_top() {
use solitaire_core::game_state::DrawMode;
let mut g = GameState::new(42, DrawMode::DrawOne);
// Draw 3 times so the waste pile has 3 cards and the buffer exists.
for _ in 0..3 {
let _ = g.draw();
}
let waste_ids: std::collections::HashSet<u32> =
g.piles[&PileType::Waste].cards.iter().map(|c| c.id).collect();
let layout = crate::layout::compute_layout(Vec2::new(1280.0, 800.0), 0.0, 0.0, true);
let positions = card_positions(&g, &layout);
let waste_rendered: Vec<_> = positions
.iter()
.filter(|(card, _, _)| waste_ids.contains(&card.id))
.collect();
// Buffer (slot 0) + top (slot 1) = 2 rendered waste cards.
assert_eq!(waste_rendered.len(), 2, "Draw-One with 3 waste cards must render exactly 2");
// Both must share the same XY so that hiding the buffer leaves no gap.
let (_, pos0, _) = waste_rendered[0];
let (_, pos1, _) = waste_rendered[1];
assert!(
(pos0.x - pos1.x).abs() < 1e-3 && (pos0.y - pos1.y).abs() < 1e-3,
"buffer and top card must be at the same XY; got buffer={pos0:?} top={pos1:?}"
);
}
#[test] #[test]
fn card_positions_tableau_cards_are_fanned_downward() { fn card_positions_tableau_cards_are_fanned_downward() {
let g = GameState::new(42, solitaire_core::game_state::DrawMode::DrawOne); let g = GameState::new(42, solitaire_core::game_state::DrawMode::DrawOne);