fix(core): validate moved tableau stack forms a legal run
move_cards only checked that the *bottom* card of a moved stack landed legally on the destination — the cards above the bottom went through unverified. A player could lift an arbitrary selection from one column and drop it on another whenever the bottom happened to match, even if the upper cards didn't form a descending alternating-colour sequence. Adds is_valid_tableau_sequence(&[Card]) -> bool to rules.rs (4 lines) and one call site in move_cards's tableau-destination branch. One focused test covering single-card / valid-run / same-colour / rank-gap cases. Reported by Quat: "stack 4 onto stack 2" was accepted when illegal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,7 @@ use crate::card::Card;
|
|||||||
use crate::deck::{deal_klondike, Deck};
|
use crate::deck::{deal_klondike, Deck};
|
||||||
use crate::error::MoveError;
|
use crate::error::MoveError;
|
||||||
use crate::pile::{Pile, PileType};
|
use crate::pile::{Pile, PileType};
|
||||||
use crate::rules::{can_place_on_foundation, can_place_on_tableau};
|
use crate::rules::{can_place_on_foundation, can_place_on_tableau, is_valid_tableau_sequence};
|
||||||
use crate::scoring::{compute_time_bonus as scoring_time_bonus, score_move, score_undo as scoring_undo};
|
use crate::scoring::{compute_time_bonus as scoring_time_bonus, score_move, score_undo as scoring_undo};
|
||||||
|
|
||||||
const MAX_UNDO_STACK: usize = 64;
|
const MAX_UNDO_STACK: usize = 64;
|
||||||
@@ -283,6 +283,18 @@ impl GameState {
|
|||||||
if !can_place_on_tableau(&bottom_card, dest) {
|
if !can_place_on_tableau(&bottom_card, dest) {
|
||||||
return Err(MoveError::RuleViolation("invalid tableau placement".into()));
|
return Err(MoveError::RuleViolation("invalid tableau placement".into()));
|
||||||
}
|
}
|
||||||
|
// The previous check only validates that the *bottom* of the
|
||||||
|
// moved stack lands on the destination's top card. Without
|
||||||
|
// this guard, a player could lift an arbitrary multi-card
|
||||||
|
// selection from one column and drop it onto another whenever
|
||||||
|
// the bottom card happens to match — even if the cards
|
||||||
|
// above the bottom don't form a legal descending
|
||||||
|
// alternating-colour run.
|
||||||
|
if !is_valid_tableau_sequence(&from_pile.cards[start..]) {
|
||||||
|
return Err(MoveError::RuleViolation(
|
||||||
|
"moved cards must form a valid tableau run".into(),
|
||||||
|
));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
_ => return Err(MoveError::InvalidDestination),
|
_ => return Err(MoveError::InvalidDestination),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -30,6 +30,18 @@ pub fn can_place_on_tableau(card: &Card, pile: &Pile) -> bool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if `cards` is a legal tableau run on its own — every
|
||||||
|
/// adjacent pair descends by one rank and alternates colour. A single
|
||||||
|
/// card is trivially valid. The destination check is separate; this
|
||||||
|
/// only validates the sequence's *internal* structure, which the tableau
|
||||||
|
/// move path must enforce so a player can't smuggle an arbitrary stack
|
||||||
|
/// onto another column when the bottom card happens to land legally.
|
||||||
|
pub fn is_valid_tableau_sequence(cards: &[Card]) -> bool {
|
||||||
|
cards.windows(2).all(|w| {
|
||||||
|
w[0].rank.value() == w[1].rank.value() + 1 && w[0].suit.is_red() != w[1].suit.is_red()
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -174,4 +186,26 @@ mod tests {
|
|||||||
let p = pile_with(PileType::Tableau(0), vec![top]);
|
let p = pile_with(PileType::Tableau(0), vec![top]);
|
||||||
assert!(!can_place_on_tableau(&c, &p));
|
assert!(!can_place_on_tableau(&c, &p));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn tableau_sequence_validation() {
|
||||||
|
// Single card is trivially a valid sequence.
|
||||||
|
assert!(is_valid_tableau_sequence(&[card(Suit::Hearts, Rank::Five)]));
|
||||||
|
// Valid descending alternating-colour run K♠ Q♥ J♣.
|
||||||
|
assert!(is_valid_tableau_sequence(&[
|
||||||
|
card(Suit::Spades, Rank::King),
|
||||||
|
card(Suit::Hearts, Rank::Queen),
|
||||||
|
card(Suit::Clubs, Rank::Jack),
|
||||||
|
]));
|
||||||
|
// Same colour twice (Q♠ on K♠) — invalid.
|
||||||
|
assert!(!is_valid_tableau_sequence(&[
|
||||||
|
card(Suit::Spades, Rank::King),
|
||||||
|
card(Suit::Spades, Rank::Queen),
|
||||||
|
]));
|
||||||
|
// Rank gap (K♠ → J♥) — invalid.
|
||||||
|
assert!(!is_valid_tableau_sequence(&[
|
||||||
|
card(Suit::Spades, Rank::King),
|
||||||
|
card(Suit::Hearts, Rank::Jack),
|
||||||
|
]));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user