fix(engine): resolve input coordination bugs in selection/pause/keyboard

- SelectionPlugin: add clear_selection_on_state_change system so undo/move/reject
  never leave a stale selection pointing at the wrong card
- SelectionPlugin: expose SelectionKeySet system set for cross-plugin ordering
- PausePlugin: skip Escape→pause when a card is keyboard-selected; toggle_pause
  now runs before SelectionKeySet so it reads SelectionState before it is cleared
- InputPlugin: guard Space→DrawRequestEvent when SelectionState has an active pile
  so Space executes a card move instead of also drawing from stock
- window: enforce 800×600 minimum via WindowResizeConstraints
- game_state: add precondition doc to next_auto_complete_move (waste exclusion)
- card_plugin: 12 unit tests for constants, face_colour, label_visibility, label_for
- pause_plugin: add paused_resource_default and draw_mode_label exhaustiveness tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-04-28 22:13:10 +00:00
parent ffc79447d4
commit 1ec2593137
6 changed files with 207 additions and 4 deletions
+5
View File
@@ -22,6 +22,11 @@ fn main() {
primary_window: Some(Window { primary_window: Some(Window {
title: "Solitaire Quest".into(), title: "Solitaire Quest".into(),
resolution: (1280u32, 800u32).into(), resolution: (1280u32, 800u32).into(),
resize_constraints: bevy::window::WindowResizeConstraints {
min_width: 800.0,
min_height: 600.0,
..default()
},
..default() ..default()
}), }),
..default() ..default()
+9
View File
@@ -361,6 +361,15 @@ impl GameState {
/// Scans tableau piles 06 in order, returning the first top card that /// Scans tableau piles 06 in order, returning the first top card that
/// can be placed on any foundation pile. The scan order ensures Aces are /// can be placed on any foundation pile. The scan order ensures Aces are
/// resolved before higher ranks that depend on them. /// resolved before higher ranks that depend on them.
///
/// # Precondition
///
/// This function is only called when `is_auto_completable` is `true`.
/// Auto-completability requires the waste pile to be empty, as enforced by
/// [`check_auto_complete`](Self::check_auto_complete) — it returns `false`
/// whenever `piles[Waste]` is non-empty. Therefore, skipping the waste pile
/// in this scan is intentional and correct: by the time this function is
/// reached, there are guaranteed to be no cards there to move.
pub fn next_auto_complete_move(&self) -> Option<(PileType, PileType)> { pub fn next_auto_complete_move(&self) -> Option<(PileType, PileType)> {
if !self.is_auto_completable || self.is_won { if !self.is_auto_completable || self.is_won {
return None; return None;
+123
View File
@@ -1322,6 +1322,129 @@ mod tests {
); );
} }
// -----------------------------------------------------------------------
// Constant sanity bounds (pure)
// -----------------------------------------------------------------------
#[test]
fn tableau_fan_frac_is_in_unit_interval() {
assert!(
TABLEAU_FAN_FRAC > 0.0 && TABLEAU_FAN_FRAC < 1.0,
"TABLEAU_FAN_FRAC must be in (0, 1), got {TABLEAU_FAN_FRAC}"
);
}
#[test]
fn flip_half_secs_is_positive() {
assert!(
FLIP_HALF_SECS > 0.0,
"FLIP_HALF_SECS must be positive, got {FLIP_HALF_SECS}"
);
}
#[test]
fn font_size_frac_is_positive_and_reasonable() {
assert!(
FONT_SIZE_FRAC > 0.0 && FONT_SIZE_FRAC <= 1.0,
"FONT_SIZE_FRAC should be in (0, 1], got {FONT_SIZE_FRAC}"
);
}
// -----------------------------------------------------------------------
// face_colour (pure) — color-blind mode
// -----------------------------------------------------------------------
#[test]
fn face_colour_normal_mode_returns_card_face_colour_for_red_suit() {
let card = Card { id: 0, suit: Suit::Hearts, rank: Rank::King, face_up: true };
assert_eq!(face_colour(&card, false), CARD_FACE_COLOUR);
}
#[test]
fn face_colour_normal_mode_returns_card_face_colour_for_black_suit() {
let card = Card { id: 0, suit: Suit::Spades, rank: Rank::King, face_up: true };
assert_eq!(face_colour(&card, false), CARD_FACE_COLOUR);
}
#[test]
fn face_colour_color_blind_mode_gives_red_suits_a_different_tint() {
let red_card = Card { id: 0, suit: Suit::Diamonds, rank: Rank::Queen, face_up: true };
let cbm_colour = face_colour(&red_card, true);
assert_ne!(
cbm_colour, CARD_FACE_COLOUR,
"color-blind mode must tint red-suit cards differently from the standard face colour"
);
}
#[test]
fn face_colour_color_blind_mode_does_not_change_black_suits() {
let black_card = Card { id: 0, suit: Suit::Clubs, rank: Rank::Jack, face_up: true };
assert_eq!(
face_colour(&black_card, true),
CARD_FACE_COLOUR,
"color-blind mode must not alter black-suit card face colour"
);
}
// -----------------------------------------------------------------------
// label_visibility (pure)
// -----------------------------------------------------------------------
#[test]
fn label_visibility_face_up_is_inherited() {
let card = Card { id: 0, suit: Suit::Clubs, rank: Rank::Ace, face_up: true };
assert_eq!(label_visibility(&card), Visibility::Inherited);
}
#[test]
fn label_visibility_face_down_is_hidden() {
let card = Card { id: 0, suit: Suit::Clubs, rank: Rank::Ace, face_up: false };
assert_eq!(label_visibility(&card), Visibility::Hidden);
}
// -----------------------------------------------------------------------
// label_for — remaining ranks not yet covered
// -----------------------------------------------------------------------
#[test]
fn label_for_all_ranks_contain_suit_letter() {
let suits = [Suit::Clubs, Suit::Diamonds, Suit::Hearts, Suit::Spades];
let letters = ["C", "D", "H", "S"];
for (suit, letter) in suits.iter().zip(letters.iter()) {
let card = Card { id: 0, suit: *suit, rank: Rank::King, face_up: true };
assert!(
label_for(&card).ends_with(letter),
"label for {suit:?} must end with '{letter}'"
);
}
}
#[test]
fn label_for_face_cards_use_letter_prefix() {
let make = |rank| Card { id: 0, suit: Suit::Spades, rank, face_up: true };
assert!(label_for(&make(Rank::Jack)).starts_with('J'));
assert!(label_for(&make(Rank::Queen)).starts_with('Q'));
assert!(label_for(&make(Rank::King)).starts_with('K'));
}
#[test]
fn label_for_numeric_ranks_two_through_nine() {
let make = |rank| Card { id: 0, suit: Suit::Clubs, rank, face_up: true };
let expected = [
(Rank::Two, "2C"),
(Rank::Three, "3C"),
(Rank::Four, "4C"),
(Rank::Five, "5C"),
(Rank::Six, "6C"),
(Rank::Seven, "7C"),
(Rank::Eight, "8C"),
(Rank::Nine, "9C"),
];
for (rank, label) in expected {
assert_eq!(label_for(&make(rank)), label, "rank {rank:?}");
}
}
#[test] #[test]
fn facedown_cards_use_tighter_fan_than_uniform_faceup_fan() { fn facedown_cards_use_tighter_fan_than_uniform_faceup_fan() {
let g = GameState::new(42, solitaire_core::game_state::DrawMode::DrawOne); let g = GameState::new(42, solitaire_core::game_state::DrawMode::DrawOne);
+7 -1
View File
@@ -43,6 +43,7 @@ use crate::pause_plugin::PausedResource;
use crate::progress_plugin::ProgressResource; use crate::progress_plugin::ProgressResource;
use crate::layout::{Layout, LayoutResource}; use crate::layout::{Layout, LayoutResource};
use crate::resources::{DragState, GameStateResource, HintCycleIndex}; use crate::resources::{DragState, GameStateResource, HintCycleIndex};
use crate::selection_plugin::SelectionState;
use crate::time_attack_plugin::TimeAttackResource; use crate::time_attack_plugin::TimeAttackResource;
/// Z-depth used for cards while being dragged — above all resting cards. /// Z-depth used for cards while being dragged — above all resting cards.
@@ -144,6 +145,7 @@ fn handle_keyboard_core(
mut confirm: ResMut<KeyboardConfirmState>, mut confirm: ResMut<KeyboardConfirmState>,
mut ev: CoreKeyboardMessages<'_>, mut ev: CoreKeyboardMessages<'_>,
mut time_attack: Option<ResMut<TimeAttackResource>>, mut time_attack: Option<ResMut<TimeAttackResource>>,
selection: Option<Res<SelectionState>>,
) { ) {
if paused.is_some_and(|p| p.0) { if paused.is_some_and(|p| p.0) {
return; return;
@@ -224,7 +226,11 @@ fn handle_keyboard_core(
} }
} }
if keys.just_pressed(KeyCode::KeyD) || keys.just_pressed(KeyCode::Space) { // Space draws only when no card is keyboard-selected; when a card IS selected,
// SelectionPlugin handles Space to execute the move.
let space_draws = keys.just_pressed(KeyCode::Space)
&& selection.as_ref().is_none_or(|s| s.selected_pile.is_none());
if keys.just_pressed(KeyCode::KeyD) || space_draws {
// Cancel any pending forfeit when the player takes another action. // Cancel any pending forfeit when the player takes another action.
confirm.forfeit_countdown = 0.0; confirm.forfeit_countdown = 0.0;
ev.draw.write(DrawRequestEvent); ev.draw.write(DrawRequestEvent);
+37 -1
View File
@@ -23,6 +23,7 @@ use crate::events::StateChangedEvent;
use crate::game_plugin::{GameOverScreen, GameStatePath}; use crate::game_plugin::{GameOverScreen, GameStatePath};
use crate::progress_plugin::ProgressResource; use crate::progress_plugin::ProgressResource;
use crate::resources::{DragState, GameStateResource}; use crate::resources::{DragState, GameStateResource};
use crate::selection_plugin::{SelectionKeySet, SelectionState};
use crate::settings_plugin::{SettingsChangedEvent, SettingsResource, SettingsStoragePath}; use crate::settings_plugin::{SettingsChangedEvent, SettingsResource, SettingsStoragePath};
use crate::stats_plugin::StatsResource; use crate::stats_plugin::StatsResource;
@@ -58,7 +59,15 @@ impl Plugin for PausePlugin {
app.add_message::<SettingsChangedEvent>() app.add_message::<SettingsChangedEvent>()
.add_message::<StateChangedEvent>() .add_message::<StateChangedEvent>()
.init_resource::<PausedResource>() .init_resource::<PausedResource>()
.add_systems(Update, (toggle_pause, handle_pause_draw_toggle)); .add_systems(
Update,
(
// toggle_pause must see SelectionState *before* handle_selection_keys
// clears it, so it can skip Escape when a card is selected.
toggle_pause.before(SelectionKeySet),
handle_pause_draw_toggle,
),
);
} }
} }
@@ -76,10 +85,16 @@ fn toggle_pause(
settings: Option<Res<SettingsResource>>, settings: Option<Res<SettingsResource>>,
mut drag: Option<ResMut<DragState>>, mut drag: Option<ResMut<DragState>>,
mut changed: MessageWriter<StateChangedEvent>, mut changed: MessageWriter<StateChangedEvent>,
selection: Option<Res<SelectionState>>,
) { ) {
if !keys.just_pressed(KeyCode::Escape) { if !keys.just_pressed(KeyCode::Escape) {
return; return;
} }
// If a card is currently selected, let SelectionPlugin handle this Escape
// (it will clear the selection). Pause must not also open in the same frame.
if selection.is_some_and(|s| s.selected_pile.is_some()) {
return;
}
// If the game-over overlay is visible, let handle_game_over_input consume // If the game-over overlay is visible, let handle_game_over_input consume
// the Escape key (to start a new game). Do not open the pause overlay. // the Escape key (to start a new game). Do not open the pause overlay.
if !game_over_screens.is_empty() { if !game_over_screens.is_empty() {
@@ -415,6 +430,16 @@ mod tests {
); );
} }
// -----------------------------------------------------------------------
// PausedResource default (pure)
// -----------------------------------------------------------------------
#[test]
fn paused_resource_default_is_unpaused() {
let p = PausedResource::default();
assert!(!p.0, "game must start unpaused");
}
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// draw_mode_label (pure function) — Task #64 // draw_mode_label (pure function) — Task #64
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
@@ -429,6 +454,17 @@ mod tests {
assert_eq!(draw_mode_label(DrawMode::DrawThree), "Draw 3"); assert_eq!(draw_mode_label(DrawMode::DrawThree), "Draw 3");
} }
/// Both variants are covered so the match is exhaustive — this test would
/// fail to compile if a new DrawMode variant were added without updating
/// `draw_mode_label`.
#[test]
fn draw_mode_label_covers_all_variants() {
for mode in [DrawMode::DrawOne, DrawMode::DrawThree] {
let label = draw_mode_label(mode);
assert!(!label.is_empty(), "draw_mode_label must never return an empty string");
}
}
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// pause_draw_toggle_flips_draw_mode — Task #64 // pause_draw_toggle_flips_draw_mode — Task #64
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
+26 -2
View File
@@ -22,7 +22,7 @@ use solitaire_core::card::Suit;
use solitaire_core::pile::PileType; use solitaire_core::pile::PileType;
use crate::card_plugin::CardEntity; use crate::card_plugin::CardEntity;
use crate::events::{InfoToastEvent, MoveRequestEvent}; use crate::events::{InfoToastEvent, MoveRequestEvent, StateChangedEvent};
use crate::game_plugin::GameMutation; use crate::game_plugin::GameMutation;
use crate::input_plugin::{best_destination, best_tableau_destination_for_stack}; use crate::input_plugin::{best_destination, best_tableau_destination_for_stack};
use crate::layout::LayoutResource; use crate::layout::LayoutResource;
@@ -42,6 +42,13 @@ pub struct SelectionState {
pub selected_pile: Option<PileType>, pub selected_pile: Option<PileType>,
} }
/// System set label for the key-handling system.
///
/// `PausePlugin` registers `toggle_pause` before this set so it can read
/// [`SelectionState`] before `handle_selection_keys` clears it on Escape.
#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)]
pub struct SelectionKeySet;
/// Marker component placed on the outline sprite used as the keyboard-selection /// Marker component placed on the outline sprite used as the keyboard-selection
/// highlight. /// highlight.
/// ///
@@ -59,7 +66,10 @@ impl Plugin for SelectionPlugin {
.add_systems( .add_systems(
Update, Update,
( (
handle_selection_keys.before(GameMutation), handle_selection_keys
.in_set(SelectionKeySet)
.before(GameMutation),
clear_selection_on_state_change.after(GameMutation),
update_selection_highlight.after(GameMutation), update_selection_highlight.after(GameMutation),
), ),
); );
@@ -329,6 +339,20 @@ fn try_foundation_dest(
None None
} }
/// Clears the selection whenever the game state changes.
///
/// Without this, an undo or a rejected move could leave `selected_pile`
/// pointing at a pile whose top card changed, causing the highlight to
/// trail a different card than the player expects.
fn clear_selection_on_state_change(
mut state_events: MessageReader<StateChangedEvent>,
mut selection: ResMut<SelectionState>,
) {
if state_events.read().next().is_some() {
selection.selected_pile = None;
}
}
/// Maintains the `SelectionHighlight` outline sprite. /// Maintains the `SelectionHighlight` outline sprite.
/// ///
/// When a pile is selected, a cyan sprite is placed at the selected card's /// When a pile is selected, a cyan sprite is placed at the selected card's