refactor(core): make KlondikeInstruction the move currency
Build and Deploy / build-and-push (push) Failing after 1m1s
Web E2E / web-e2e (push) Failing after 3m26s

Remove the (from, to, count) tuple as an internal move-passing wrapper.
Game logic now stays in KlondikeInstruction space end to end:

- Add GameState::apply_instruction, the native apply path. move_cards
  becomes a thin pile-coordinate adapter that converts to an instruction
  and delegates, so move bookkeeping (validation, score/recycle history,
  undo snapshot) lives in one place instead of being duplicated.
- next_auto_complete_move matches DstFoundation directly instead of
  projecting every candidate to pile coordinates.
- proptests and the storage round-trip test apply instructions directly
  rather than round-tripping instruction -> tuple -> move_cards.

The single instruction -> pile decode is renamed instruction_to_highlight
-> instruction_to_piles and kept in core: decoding a tableau run length
needs upstream pile-stack types core does not re-export, so relocating it
would duplicate the logic across engine and wasm. The two rendering edges
(engine hint highlight, wasm debug move list) call this one decoder; the
engine's hint_piles is a thin delegation to it.

Also includes the CardEntityIndex render-side index and a SelectionPlugin
init_resource fix so update_selection_highlight no longer panics in test
harnesses that omit CardPlugin.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
funman300
2026-06-10 16:58:28 -07:00
parent dc4cf45ea0
commit ef1efdc3b5
9 changed files with 219 additions and 110 deletions
+54
View File
@@ -165,6 +165,29 @@ pub struct CardEntity {
pub card: Card,
}
/// Render-side index mapping each live board card to its [`CardEntity`].
///
/// Maintained exclusively by [`rebuild_card_entity_index`] in `PostUpdate`,
/// after the card-sync authority ([`sync_cards_on_change`]) and the resize
/// re-snap have flushed their spawn/despawn `Commands`. Consumers treat it as
/// read-only and must still call `Query::get(entity)` for components beyond the
/// `Entity` id (the map yields only the id).
///
/// Keyed by `Card`, which is unique across all live board entities in
/// single-deck Klondike. Transient entities (drag shadow, labels) carry no
/// `CardEntity` component, so the rebuild — which scans `&CardEntity` — never
/// records them.
#[derive(Resource, Debug, Default)]
pub struct CardEntityIndex(pub HashMap<Card, Entity>);
impl CardEntityIndex {
/// Resolve a card to its live entity, if one is currently spawned.
#[inline]
pub fn get(&self, card: &Card) -> Option<Entity> {
self.0.get(card).copied()
}
}
/// Marker for the text child inside a card entity.
#[derive(Component, Debug)]
pub struct CardLabel;
@@ -431,6 +454,7 @@ impl Plugin for CardPlugin {
// ensure it exists here. Under `DefaultPlugins` the call is a no-op.
app.init_resource::<ButtonInput<MouseButton>>()
.init_resource::<ResizeThrottle>()
.init_resource::<CardEntityIndex>()
.add_message::<SettingsChangedEvent>()
.add_message::<CardFlippedEvent>()
.add_message::<CardFaceRevealedEvent>()
@@ -466,6 +490,36 @@ impl Plugin for CardPlugin {
);
app.add_systems(Update, resize_android_corner_labels);
app.add_systems(PostUpdate, rebuild_card_entity_index);
}
}
/// Rebuild the [`CardEntityIndex`] from the live `CardEntity` set.
///
/// Runs in `PostUpdate` so that all spawn/despawn `Commands` issued by
/// [`sync_cards_on_change`] and [`snap_cards_on_window_resize`] in `Update`
/// have been flushed at the `Update -> PostUpdate` apply-deferred boundary.
/// Rebuilding from scratch (rather than incrementally patching at every
/// spawn/despawn site — waste cards churn on every draw) keeps a single writer
/// and makes a stale entry structurally impossible.
///
/// Gated to changed frames only: `Changed<CardEntity>` fires the frame a card
/// is spawned, `RemovedComponents<CardEntity>` the frame one is despawned. The
/// `card` field is write-once (never mutated in place), so card-reposition
/// frames don't trip `Changed` and correctly skip the O(52) rebuild.
fn rebuild_card_entity_index(
mut index: ResMut<CardEntityIndex>,
cards: Query<(Entity, &CardEntity)>,
changed: Query<(), Changed<CardEntity>>,
removed: RemovedComponents<CardEntity>,
) {
if changed.is_empty() && removed.is_empty() {
return;
}
let map = &mut index.0;
map.clear();
for (entity, ce) in &cards {
map.insert(ce.card.clone(), entity);
}
}
+36 -21
View File
@@ -26,7 +26,7 @@ use bevy::prelude::*;
use bevy::window::PrimaryWindow;
#[cfg(not(target_os = "android"))]
use bevy::window::{MonitorSelection, WindowMode};
use solitaire_core::{Foundation, KlondikePile, Tableau};
use solitaire_core::{Foundation, KlondikeInstruction, KlondikePile, Tableau};
use solitaire_core::card::{Card, Suit};
use solitaire_core::game_state::GameState;
@@ -350,7 +350,7 @@ pub fn find_heuristic_hint(
}
let idx = hint_cycle.0 % hints.len();
hint_cycle.0 = hint_cycle.0.wrapping_add(1);
let (from, to, _count) = hints[idx];
let (from, to) = hints[idx];
Some((from, to))
}
@@ -1642,30 +1642,47 @@ fn handle_double_tap(
/// Build the complete list of legal moves available in `game`, ordered so that
/// upstream `klondike` priorities are preserved.
///
/// Each entry is `(from, to, count)` — the same triple used by
/// [`MoveRequestEvent`]. The list may be empty when no move exists at all
/// (game is stuck).
/// Each entry is `(from, to)` — the source and destination piles a hint
/// should highlight. Only single-card moves are surfaced; multi-card tableau
/// runs are filtered out by [`hint_piles`]. The list may be empty when no
/// move exists at all (game is stuck).
///
/// This is the backing data for the cycling hint system: the H key steps
/// through `hints[HintCycleIndex % hints.len()]` on each press.
pub fn all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile, usize)> {
pub fn all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile)> {
if game.has_test_pile_overrides() {
return legacy_all_hints(game);
}
game.possible_instructions()
.into_iter()
.filter(|(_, _, count)| *count == 1)
.filter_map(|instruction| hint_piles(game, instruction))
.collect()
}
/// Project a [`KlondikeInstruction`] to the `(source, destination)` piles a
/// hint should highlight, or `None` for a no-op or multi-card move.
///
/// Delegates the instruction→pile decode to the single owner of that mapping,
/// [`GameState::instruction_to_piles`], and keeps only single-card moves
/// (`count == 1`) — the hint highlight can represent exactly one source card.
pub(crate) fn hint_piles(
game: &GameState,
instruction: KlondikeInstruction,
) -> Option<(KlondikePile, KlondikePile)> {
match game.instruction_to_piles(instruction)? {
(from, to, 1) => Some((from, to)),
_ => None,
}
}
/// Legacy hint enumeration used only when test pile overrides are active.
///
/// `possible_instructions()` reflects the internal upstream `Session` state.
/// In test fixtures that inject synthetic piles via `set_test_*`, these
/// synthetic piles can diverge from the session state; this fallback preserves
/// deterministic test semantics in those fixtures.
fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile, usize)> {
fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile)> {
let sources: Vec<KlondikePile> = {
let mut s = vec![KlondikePile::Stock];
for tableau in tableaus() {
@@ -1674,7 +1691,7 @@ fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile, usize)
s
};
let mut hints: Vec<(KlondikePile, KlondikePile, usize)> = Vec::new();
let mut hints: Vec<(KlondikePile, KlondikePile)> = Vec::new();
// Pass 1 — foundation moves (highest priority, shown first).
for from in &sources {
@@ -1685,7 +1702,7 @@ fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile, usize)
for foundation in foundations() {
let dest = KlondikePile::Foundation(foundation);
if game.can_move_cards(from, &dest, 1) {
hints.push((*from, dest, 1));
hints.push((*from, dest));
break;
}
}
@@ -1700,14 +1717,14 @@ fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile, usize)
};
let already_has_foundation_hint = hints
.iter()
.any(|(f, t, _)| f == from && matches!(t, KlondikePile::Foundation(_)));
.any(|(f, t)| f == from && matches!(t, KlondikePile::Foundation(_)));
if already_has_foundation_hint {
continue;
}
for tableau in tableaus() {
let dest = KlondikePile::Tableau(tableau);
if game.can_move_cards(from, &dest, 1) {
hints.push((*from, dest, 1));
hints.push((*from, dest));
break;
}
}
@@ -1727,7 +1744,7 @@ fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile, usize)
for tableau in tableaus() {
let dest = KlondikePile::Tableau(tableau);
if game.can_move_cards(&from, &dest, 1) {
hints.push((from, dest, 1));
hints.push((from, dest));
break;
}
}
@@ -1742,9 +1759,9 @@ fn legacy_all_hints(game: &GameState) -> Vec<(KlondikePile, KlondikePile, usize)
let waste_can_recycle = stock_cards.is_empty() && !waste_cards.is_empty();
if stock_non_empty || waste_can_recycle {
// Stock→Waste is not a real pile-to-pile move, but we reuse the
// triple to signal "draw". The H handler only reads `from` to
// pair to signal "draw". The H handler only reads `from` to
// locate the card to highlight; we point at the stock pile.
hints.push((KlondikePile::Stock, KlondikePile::Stock, 1));
hints.push((KlondikePile::Stock, KlondikePile::Stock));
}
}
@@ -1793,9 +1810,9 @@ const fn tableau_number(tableau: Tableau) -> u8 {
/// Find one valid move in the current game state.
///
/// Returns `(from, to, count)` for the first legal move found, or `None` if
/// Returns `(from, to)` for the first legal move found, or `None` if
/// no move is available. This is a convenience wrapper over [`all_hints`].
pub fn find_hint(game: &GameState) -> Option<(KlondikePile, KlondikePile, usize)> {
pub fn find_hint(game: &GameState) -> Option<(KlondikePile, KlondikePile)> {
all_hints(game).into_iter().next()
}
@@ -2166,10 +2183,9 @@ mod tests {
let hint = find_hint(&game);
assert!(hint.is_some(), "should find a hint");
let (from, to, count) = hint.unwrap();
let (from, to) = hint.unwrap();
assert_eq!(from, KlondikePile::Tableau(Tableau::Tableau1));
assert_eq!(to, KlondikePile::Foundation(Foundation::Foundation1));
assert_eq!(count, 1);
}
// -----------------------------------------------------------------------
@@ -2212,10 +2228,9 @@ mod tests {
let hints = all_hints(&game);
assert_eq!(hints.len(), 1, "exactly one hint: draw from stock");
let (from, to, count) = &hints[0];
let (from, to) = &hints[0];
assert_eq!(*from, KlondikePile::Stock, "hint must come from Stock");
assert_eq!(*to, KlondikePile::Stock, "hint must point to Waste");
assert_eq!(*count, 1);
}
// `all_hints` must be empty when both stock and waste are empty and no
+3 -6
View File
@@ -30,7 +30,7 @@ use solitaire_data::solver::try_solve_from_state;
use crate::card_plugin::CardEntity;
use crate::events::{HintVisualEvent, InfoToastEvent, StateChangedEvent};
use crate::input_plugin::{emit_hint_visuals, find_heuristic_hint};
use crate::input_plugin::{emit_hint_visuals, find_heuristic_hint, hint_piles};
use crate::resources::{GameStateResource, HintCycleIndex};
/// In-flight async work for the H-key hint.
@@ -93,7 +93,7 @@ struct HintTask {
enum HintTaskOutput {
/// Solver verdict was winnable; here is the first move on the solution
/// path. Converted to highlighted `(from, to)` piles by the poll system
/// via [`GameState::instruction_to_move`].
/// via [`crate::input_plugin::hint_piles`].
SolverMove(KlondikeInstruction),
/// Solver was `Unwinnable` or `Inconclusive`. The poll system
/// runs the legacy heuristic against the live `GameState` so the
@@ -153,10 +153,7 @@ pub fn poll_pending_hint_task(
// Resolve the solver's first move to highlighted piles; fall back to the
// live-state heuristic when there's no solver move or it maps to a no-op.
let solver_pair = match output {
HintTaskOutput::SolverMove(instruction) => g
.0
.instruction_to_move(instruction)
.map(|(from, to, _count)| (from, to)),
HintTaskOutput::SolverMove(instruction) => hint_piles(&g.0, instruction),
HintTaskOutput::NeedsHeuristic => None,
};
let (from, to) = match solver_pair.or_else(|| find_heuristic_hint(&g.0, &mut hint_cycle)) {
+22 -21
View File
@@ -41,7 +41,7 @@ use solitaire_core::{Foundation, KlondikePile, Tableau};
use solitaire_core::card::Card;
use solitaire_core::game_state::GameState;
use crate::card_plugin::CardEntity;
use crate::card_plugin::CardEntityIndex;
use crate::events::{InfoToastEvent, MoveRequestEvent, StateChangedEvent};
use crate::game_plugin::GameMutation;
use crate::input_plugin::{best_destination, best_tableau_destination_for_stack};
@@ -147,8 +147,12 @@ pub struct SelectionPlugin;
impl Plugin for SelectionPlugin {
fn build(&self, app: &mut App) {
// `CardEntityIndex` is owned and kept current by `CardPlugin`; this
// call is a no-op there. It is declared here so `update_selection_highlight`
// can read it via `Res<>` even in harnesses that omit `CardPlugin`.
app.init_resource::<SelectionState>()
.init_resource::<KeyboardDragState>()
.init_resource::<CardEntityIndex>()
.add_systems(
Update,
(
@@ -657,7 +661,7 @@ fn update_selection_highlight(
kbd_drag: Res<KeyboardDragState>,
game: Res<GameStateResource>,
layout: Option<Res<LayoutResource>>,
card_entities: Query<(Entity, &CardEntity)>,
card_index: Res<CardEntityIndex>,
highlights: Query<Entity, With<SelectionHighlight>>,
) {
// Always despawn any existing highlight first.
@@ -695,7 +699,7 @@ fn update_selection_highlight(
{
spawn_highlight_on_card(
&mut commands,
&card_entities,
&card_index,
&card,
card_size,
source_color,
@@ -712,7 +716,7 @@ fn update_selection_highlight(
if let Some(card) = top_face_up_card(dest, &game.0) {
spawn_highlight_on_card(
&mut commands,
&card_entities,
&card_index,
&card,
card_size,
dest_color,
@@ -741,27 +745,24 @@ fn pile_cards(game: &GameState, pile: &KlondikePile) -> Vec<(Card, bool)> {
/// the matching `CardEntity::card`. No-op if no entity matches.
fn spawn_highlight_on_card(
commands: &mut Commands,
card_entities: &Query<(Entity, &CardEntity)>,
card_index: &CardEntityIndex,
card: &Card,
card_size: Vec2,
color: Color,
) {
for (entity, card_entity) in card_entities {
if card_entity.card == *card {
commands.entity(entity).with_children(|b| {
b.spawn((
SelectionHighlight,
Sprite {
color,
custom_size: Some(card_size + Vec2::splat(4.0)),
..default()
},
Transform::from_xyz(0.0, 0.0, -0.01),
Visibility::default(),
));
});
break;
}
if let Some(entity) = card_index.get(card) {
commands.entity(entity).with_children(|b| {
b.spawn((
SelectionHighlight,
Sprite {
color,
custom_size: Some(card_size + Vec2::splat(4.0)),
..default()
},
Transform::from_xyz(0.0, 0.0, -0.01),
Visibility::default(),
));
});
}
}