From ef1efdc3b5cc61912038cbd4b06818118b3201e0 Mon Sep 17 00:00:00 2001 From: funman300 Date: Wed, 10 Jun 2026 16:58:28 -0700 Subject: [PATCH] refactor(core): make KlondikeInstruction the move currency 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) --- solitaire_core/src/game_state.rs | 102 +++++++++++++++++------ solitaire_core/src/lib.rs | 7 +- solitaire_core/src/proptest_tests.rs | 44 ++++------ solitaire_data/src/storage.rs | 12 +-- solitaire_engine/src/card_plugin.rs | 54 ++++++++++++ solitaire_engine/src/input_plugin.rs | 57 ++++++++----- solitaire_engine/src/pending_hint.rs | 9 +- solitaire_engine/src/selection_plugin.rs | 43 +++++----- solitaire_wasm/src/lib.rs | 1 + 9 files changed, 219 insertions(+), 110 deletions(-) diff --git a/solitaire_core/src/game_state.rs b/solitaire_core/src/game_state.rs index 58c752f..bd624c7 100644 --- a/solitaire_core/src/game_state.rs +++ b/solitaire_core/src/game_state.rs @@ -826,14 +826,24 @@ impl GameState { } } - /// Converts an upstream [`KlondikeInstruction`] into the engine's - /// `(from, to, count)` pile-move form, resolving multi-card tableau moves - /// against the live board. Returns `None` for no-op instructions - /// (foundation→foundation, or a tableau move of zero cards). + /// Decodes an upstream [`KlondikeInstruction`] into the `(from, to, count)` + /// pile coordinates of `solitaire_core`'s own pile model, against the live + /// board. Returns `None` for no-op instructions (foundation→foundation, or a + /// tableau move of zero cards). /// - /// Used by the hint system to render a solver's recommended first move, - /// and internally by [`Self::possible_instructions`]. - pub fn instruction_to_move( + /// This is the single, canonical translation from the upstream instruction + /// move-currency to core's [`KlondikePile`] vocabulary. It lives in core + /// because decoding a tableau-run length requires upstream pile-stack types + /// (`KlondikePileStack`/`SkipCards`) that the engine and wasm crates do not + /// see — relocating it would duplicate this logic across both crates. The + /// two edges that genuinely need on-screen positions call it: the engine's + /// hint highlight (which pile to glow) and the wasm debug move list (pile + /// names + run length serialized to the browser harness). + /// + /// Game logic — auto-complete, move application, the property tests — stays + /// in instruction space and never calls this; applying a move uses + /// [`Self::apply_instruction`]. + pub fn instruction_to_piles( &self, instruction: KlondikeInstruction, ) -> Option<(KlondikePile, KlondikePile, usize)> { @@ -931,6 +941,28 @@ impl GameState { } let instruction = self.instruction_for_move(from, to, count)?; + self.apply_instruction(instruction) + } + + /// Apply an upstream [`KlondikeInstruction`] directly to the live session. + /// + /// This is the native apply path for moves that already exist in + /// instruction form — solver hints, auto-complete, replay, and the property + /// tests. User drag-and-drop enters through [`Self::move_cards`], which is a + /// thin adapter that converts pile coordinates to an instruction and + /// delegates here, so the move bookkeeping (rule validation, score history, + /// recycle accounting, undo snapshot) lives in exactly one place. + /// + /// Returns [`MoveError::RuleViolation`] if the instruction is illegal in the + /// current position, or [`MoveError::GameAlreadyWon`] once the game is over. + pub fn apply_instruction( + &mut self, + instruction: KlondikeInstruction, + ) -> Result<(), MoveError> { + if self.is_won() { + return Err(MoveError::GameAlreadyWon); + } + let config = self.validation_config(); if !self .session @@ -941,12 +973,16 @@ impl GameState { return Err(MoveError::RuleViolation("move violates rules".into())); } - let (score_delta, _) = self.pre_instruction_score_delta(instruction); + let (score_delta, is_recycle) = self.pre_instruction_score_delta(instruction); self.score_history.push(self.score); - self.is_recycle_history.push(false); + self.is_recycle_history.push(is_recycle); self.session.process_instruction(instruction); + + if is_recycle { + self.recycle_count = self.recycle_count.saturating_add(1); + } self.score = (self.score + score_delta).max(0); Ok(()) } @@ -996,8 +1032,13 @@ impl GameState { self.session.state().state().is_win_trivial() } - /// Returns all currently valid `(from, to, count)` moves. - pub fn possible_instructions(&self) -> Vec<(KlondikePile, KlondikePile, usize)> { + /// Returns all currently valid moves as upstream [`KlondikeInstruction`]s, + /// ordered by the `klondike` solver's move priority. + /// + /// This is the engine's move currency. Callers that need on-screen pile + /// positions — hint highlighting and the wasm debug move list — decode each + /// instruction with [`Self::instruction_to_piles`] at their UI edge. + pub fn possible_instructions(&self) -> Vec { if self.is_won() { return Vec::new(); } @@ -1008,7 +1049,6 @@ impl GameState { .state() .get_sorted_moves(&config) .into_iter() - .filter_map(|instruction| self.instruction_to_move(instruction)) .collect() } @@ -1061,20 +1101,20 @@ impl GameState { return None; } + // A foundation-bound single-card move is exactly a `DstFoundation` + // instruction whose source is not itself a foundation. Match the + // instruction variant directly rather than projecting every candidate + // to `(from, to, count)` pile coordinates — auto-complete is pure game + // logic and never needs on-screen positions. self.possible_instructions() .into_iter() - .find_map(|(from, to, count)| { - if count != 1 { - return None; - } - if matches!(from, KlondikePile::Foundation(_)) { - return None; - } - if matches!(to, KlondikePile::Foundation(_)) { - Some((from, to)) - } else { - None + .find_map(|instruction| match instruction { + KlondikeInstruction::DstFoundation(dst) + if !matches!(dst.src, KlondikePile::Foundation(_)) => + { + Some((dst.src, KlondikePile::Foundation(dst.foundation))) } + _ => None, }) } @@ -1098,6 +1138,16 @@ impl GameState { mod tests { use super::*; + /// Resolve every legal instruction to its `(from, to, count)` piles for + /// tests that assert against pile positions. Mirrors what a UI edge does + /// via [`GameState::instruction_to_piles`]. + fn legal_pile_moves(game: &GameState) -> Vec<(KlondikePile, KlondikePile, usize)> { + game.possible_instructions() + .into_iter() + .filter_map(|instruction| game.instruction_to_piles(instruction)) + .collect() + } + fn find_foundation_return_position() -> Option<(GameState, KlondikePile, KlondikePile)> { const MAX_SEED: u64 = 512; const MAX_STEPS: usize = 160; @@ -1107,7 +1157,7 @@ mod tests { game.take_from_foundation = true; for _ in 0..MAX_STEPS { - let moves = game.possible_instructions(); + let moves = legal_pile_moves(&game); if let Some((from, to, _count)) = moves.iter().cloned().find(|(from, to, count)| { *count == 1 && matches!(from, KlondikePile::Foundation(_)) @@ -1226,7 +1276,7 @@ mod tests { game.take_from_foundation = true; assert!(game.can_move_cards(&from, &to, 1)); assert!( - game.possible_instructions() + legal_pile_moves(&game) .iter() .any(|(f, t, c)| *f == from && *t == to && *c == 1) ); @@ -1241,7 +1291,7 @@ mod tests { game.take_from_foundation = false; assert!(!game.can_move_cards(&from, &to, 1)); assert!( - game.possible_instructions() + legal_pile_moves(&game) .iter() .all(|(f, t, _)| !matches!(f, KlondikePile::Foundation(_)) || !matches!(t, KlondikePile::Tableau(_))) diff --git a/solitaire_core/src/lib.rs b/solitaire_core/src/lib.rs index 266a457..136e0d6 100644 --- a/solitaire_core/src/lib.rs +++ b/solitaire_core/src/lib.rs @@ -8,9 +8,10 @@ pub mod klondike_adapter; // downstream crates (engine, wasm) can import from one place without a direct // `klondike` / `card_game` dep. // -// `KlondikePileStack`, `SkipCards`, and `TableauStack` are intentionally NOT -// re-exported — they are only used internally in `klondike_adapter.rs` and do -// not appear in any public method signature. +// `KlondikePileStack`, `SkipCards` and `TableauStack` are intentionally NOT +// re-exported — they are only used internally (in `klondike_adapter.rs` and +// when decoding instructions to piles in `instruction_to_piles`) and do not +// appear in any public method signature. pub use card_game::{Card, Session}; pub use klondike::{Foundation, Klondike, KlondikeInstruction, KlondikePile, Tableau}; pub use klondike_adapter::DrawMode; diff --git a/solitaire_core/src/proptest_tests.rs b/solitaire_core/src/proptest_tests.rs index c0a58aa..596daa7 100644 --- a/solitaire_core/src/proptest_tests.rs +++ b/solitaire_core/src/proptest_tests.rs @@ -60,27 +60,23 @@ fn draw_mode_strategy() -> impl Strategy { /// /// Each action is `(draw_flag, move_index)`: /// - `draw_flag = true` → call `game.draw()` -/// - `draw_flag = false` → pick the `move_index % len`th legal move from -/// `possible_instructions()` and execute it. +/// - `draw_flag = false` → pick the `move_index % len`th legal instruction +/// from `possible_instructions()` and apply it via `apply_instruction()`. /// -/// `possible_instructions()` may return `(Stock, Stock, 1)` for the -/// RotateStock / draw action. `move_cards(Stock, Stock, 1)` is rejected by -/// the `from == to` guard, so those are dispatched to `game.draw()`. +/// `possible_instructions()` may return `RotateStock`, which +/// `apply_instruction()` dispatches to `game.draw()`; ordinary instructions +/// are equivalent to `move_cards(from, to, count)`. fn apply_random_actions(game: &mut GameState, actions: &[(bool, usize)]) { for &(do_draw, idx) in actions { if do_draw { let _ = game.draw(); } else { - let instructions = game.possible_instructions(); - if instructions.is_empty() { + let moves = game.possible_instructions(); + if moves.is_empty() { continue; } - let (from, to, count) = instructions[idx % instructions.len()]; - if from == to { - let _ = game.draw(); - } else { - let _ = game.move_cards(from, to, count); - } + let instruction = moves[idx % moves.len()]; + let _ = game.apply_instruction(instruction); } } } @@ -92,16 +88,12 @@ fn apply_one_move(game: &mut GameState, move_idx: usize) -> bool { if game.is_won() { return false; } - let instructions = game.possible_instructions(); - if instructions.is_empty() { + let moves = game.possible_instructions(); + if moves.is_empty() { return game.draw().is_ok(); } - let (from, to, count) = instructions[move_idx % instructions.len()]; - if from == to { - game.draw().is_ok() - } else { - game.move_cards(from, to, count).is_ok() - } + let instruction = moves[move_idx % moves.len()]; + game.apply_instruction(instruction).is_ok() } // --------------------------------------------------------------------------- @@ -258,17 +250,13 @@ proptest! { let mut game = GameState::new(seed, draw_mode); apply_random_actions(&mut game, &setup_actions); - for (from, to, count) in game.possible_instructions() { + for instruction in game.possible_instructions() { // Clone so each move is tried from the same starting state. let mut trial = game.clone(); - let result = if from == to { - trial.draw() - } else { - trial.move_cards(from, to, count) - }; + let result = trial.apply_instruction(instruction); prop_assert!( result.is_ok(), - "possible_instructions() reported ({from:?} → {to:?} ×{count}) \ + "possible_instructions() reported {instruction:?} \ as legal but the call returned Err: {result:?}", ); } diff --git a/solitaire_data/src/storage.rs b/solitaire_data/src/storage.rs index 2938d5b..aa0a3aa 100644 --- a/solitaire_data/src/storage.rs +++ b/solitaire_data/src/storage.rs @@ -506,7 +506,7 @@ mod tests { /// will cause at least one pile to disagree. #[test] fn game_state_v4_mid_game_round_trip() { - use solitaire_core::KlondikePile; + use solitaire_core::KlondikeInstruction; use solitaire_core::game_state::GameState; let path = gs_path("v4_mid_game"); @@ -524,11 +524,13 @@ mod tests { // Execute the first available DstTableau or DstFoundation move so the // instruction history contains a type other than RotateStock. - let moves = gs.possible_instructions(); - if let Some((from, to, count)) = moves.iter().copied().find(|(_, to, _)| { - matches!(to, KlondikePile::Tableau(_) | KlondikePile::Foundation(_)) + if let Some(instruction) = gs.possible_instructions().into_iter().find(|i| { + matches!( + i, + KlondikeInstruction::DstTableau(_) | KlondikeInstruction::DstFoundation(_) + ) }) { - let _ = gs.move_cards(from, to, count); + let _ = gs.apply_instruction(instruction); } // Undo once: verifies that `undo_count` is persisted and that the diff --git a/solitaire_engine/src/card_plugin.rs b/solitaire_engine/src/card_plugin.rs index e9218c6..546e074 100644 --- a/solitaire_engine/src/card_plugin.rs +++ b/solitaire_engine/src/card_plugin.rs @@ -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); + +impl CardEntityIndex { + /// Resolve a card to its live entity, if one is currently spawned. + #[inline] + pub fn get(&self, card: &Card) -> Option { + 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::>() .init_resource::() + .init_resource::() .add_message::() .add_message::() .add_message::() @@ -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` fires the frame a card +/// is spawned, `RemovedComponents` 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, + cards: Query<(Entity, &CardEntity)>, + changed: Query<(), Changed>, + removed: RemovedComponents, +) { + 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); } } diff --git a/solitaire_engine/src/input_plugin.rs b/solitaire_engine/src/input_plugin.rs index 136dd84..5477b2c 100644 --- a/solitaire_engine/src/input_plugin.rs +++ b/solitaire_engine/src/input_plugin.rs @@ -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 = { 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 diff --git a/solitaire_engine/src/pending_hint.rs b/solitaire_engine/src/pending_hint.rs index de131a0..63e5b90 100644 --- a/solitaire_engine/src/pending_hint.rs +++ b/solitaire_engine/src/pending_hint.rs @@ -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)) { diff --git a/solitaire_engine/src/selection_plugin.rs b/solitaire_engine/src/selection_plugin.rs index 2db9368..b84bd95 100644 --- a/solitaire_engine/src/selection_plugin.rs +++ b/solitaire_engine/src/selection_plugin.rs @@ -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::() .init_resource::() + .init_resource::() .add_systems( Update, ( @@ -657,7 +661,7 @@ fn update_selection_highlight( kbd_drag: Res, game: Res, layout: Option>, - card_entities: Query<(Entity, &CardEntity)>, + card_index: Res, highlights: Query>, ) { // 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(), + )); + }); } } diff --git a/solitaire_wasm/src/lib.rs b/solitaire_wasm/src/lib.rs index a06fc4d..bf6527f 100644 --- a/solitaire_wasm/src/lib.rs +++ b/solitaire_wasm/src/lib.rs @@ -357,6 +357,7 @@ fn legal_moves_for_game(game: &GameState) -> Vec { let mut moves: Vec = game .possible_instructions() .into_iter() + .filter_map(|instruction| game.instruction_to_piles(instruction)) .map(|(from, to, count)| DebugMove::Move { from: pile_name(from), to: pile_name(to),