From 3bb3ddb6f8881caecf657c3c24ad5c9261a89eec Mon Sep 17 00:00:00 2001 From: funman300 Date: Sun, 17 May 2026 20:09:01 -0700 Subject: [PATCH] fix(engine): eliminate panics, fix dismiss hit-test scope, guard home respawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR-2: dismiss_modal_on_scrim_click now queries only the target scrim's Children rather than all ModalCard entities globally. Prevents dismissing the wrong scrim when two overlapping modals are open. CR-5: handle_home_draw_mode_buttons and handle_home_difficulty_toggle now check other_modal_scrims.is_empty() before the despawn+respawn cycle, preventing a concurrent second ModalScrim in the same frame. H-1: solitaire_core::game_state — replaced all panicking piles[&key] index accesses with safe .get().ok_or(MoveError::InvalidSource)?, .get().is_some_and(...), or .get().and_then(...) in draw(), check_auto_complete(), next_auto_complete_move(), foundation_slot_for(). H-5: input_plugin end_drag and touch_end_drag — replaced piles[&target] with .get(&target).is_some_and(...) so missing pile types reject the move rather than panicking. Co-Authored-By: Claude Sonnet 4.6 --- solitaire_core/src/game_state.rs | 24 +++++++++++++----------- solitaire_engine/src/home_plugin.rs | 11 +++++++++++ solitaire_engine/src/input_plugin.rs | 15 ++++++++------- solitaire_engine/src/ui_modal.rs | 18 +++++++++++------- 4 files changed, 43 insertions(+), 25 deletions(-) diff --git a/solitaire_core/src/game_state.rs b/solitaire_core/src/game_state.rs index edf2b4a..5d03953 100644 --- a/solitaire_core/src/game_state.rs +++ b/solitaire_core/src/game_state.rs @@ -224,10 +224,10 @@ impl GameState { return Err(MoveError::GameAlreadyWon); } - let stock_len = self.piles[&PileType::Stock].cards.len(); + let stock_len = self.piles.get(&PileType::Stock).ok_or(MoveError::InvalidSource)?.cards.len(); if stock_len == 0 { - let waste_len = self.piles[&PileType::Waste].cards.len(); + let waste_len = self.piles.get(&PileType::Waste).ok_or(MoveError::InvalidSource)?.cards.len(); if waste_len == 0 { return Err(MoveError::StockEmpty); } @@ -428,14 +428,13 @@ impl GameState { pub fn check_auto_complete(&self) -> bool { // Stock must be empty; waste may still have cards (they are resolved // by draw() calls inside next_auto_complete_move / auto_complete_step). - if !self.piles[&PileType::Stock].cards.is_empty() { + if self.piles.get(&PileType::Stock).is_none_or(|p| !p.cards.is_empty()) { return false; } (0..7).all(|i| { - self.piles[&PileType::Tableau(i)] - .cards - .iter() - .all(|c| c.face_up) + self.piles + .get(&PileType::Tableau(i)) + .is_some_and(|p| p.cards.iter().all(|c| c.face_up)) }) } @@ -461,7 +460,8 @@ impl GameState { // Check waste top first — when stock is exhausted the waste may still // contain cards that can go directly to a foundation. let waste = PileType::Waste; - if let Some((card, slot)) = self.piles[&waste].cards.last() + if let Some((card, slot)) = self.piles.get(&waste) + .and_then(|p| p.cards.last()) .and_then(|c| self.foundation_slot_for(c).map(|s| (c, s))) { let _ = card; // borrow ends here @@ -469,7 +469,8 @@ impl GameState { } for i in 0..7 { let tableau = PileType::Tableau(i); - if let Some(slot) = self.piles[&tableau].cards.last() + if let Some(slot) = self.piles.get(&tableau) + .and_then(|p| p.cards.last()) .and_then(|c| self.foundation_slot_for(c)) { return Some((tableau, PileType::Foundation(slot))); @@ -487,7 +488,7 @@ impl GameState { let mut candidate: Option = None; let mut empty_slot: Option = None; for slot in 0..4_u8 { - let pile = &self.piles[&PileType::Foundation(slot)]; + let Some(pile) = self.piles.get(&PileType::Foundation(slot)) else { continue }; if pile.cards.is_empty() { if empty_slot.is_none() { empty_slot = Some(slot); @@ -501,7 +502,8 @@ impl GameState { if card.rank.value() == 1 { empty_slot } else { None } }); target.filter(|&slot| { - can_place_on_foundation(card, &self.piles[&PileType::Foundation(slot)]) + self.piles.get(&PileType::Foundation(slot)) + .is_some_and(|p| can_place_on_foundation(card, p)) }) } diff --git a/solitaire_engine/src/home_plugin.rs b/solitaire_engine/src/home_plugin.rs index a75f06a..ee745a7 100644 --- a/solitaire_engine/src/home_plugin.rs +++ b/solitaire_engine/src/home_plugin.rs @@ -589,6 +589,7 @@ fn handle_home_draw_mode_buttons( one_buttons: Query<&Interaction, (With, Changed)>, three_buttons: Query<&Interaction, (With, Changed)>, screens: Query>, + other_modal_scrims: Query<(), (With, Without)>, mut settings: Option>, storage_path: Option>, mut changed: MessageWriter, @@ -601,6 +602,12 @@ fn handle_home_draw_mode_buttons( if screens.is_empty() { return; } + // Don't respawn while another modal sits on top — the despawn queues + // immediately but executes at end of frame, so a respawn in the same + // frame would create a second concurrent ModalScrim. + if !other_modal_scrims.is_empty() { + return; + } let want_one = one_buttons.iter().any(|i| *i == Interaction::Pressed); let want_three = three_buttons.iter().any(|i| *i == Interaction::Pressed); if !want_one && !want_three { @@ -658,6 +665,7 @@ fn handle_home_difficulty_toggle( mut commands: Commands, toggles: Query<&Interaction, (With, Changed)>, screens: Query>, + other_modal_scrims: Query<(), (With, Without)>, mut diff_expanded: ResMut, progress: Option>, stats: Option>, @@ -668,6 +676,9 @@ fn handle_home_difficulty_toggle( if screens.is_empty() { return; } + if !other_modal_scrims.is_empty() { + return; + } if !toggles.iter().any(|i| *i == Interaction::Pressed) { return; } diff --git a/solitaire_engine/src/input_plugin.rs b/solitaire_engine/src/input_plugin.rs index 9dec6ae..177623b 100644 --- a/solitaire_engine/src/input_plugin.rs +++ b/solitaire_engine/src/input_plugin.rs @@ -717,13 +717,12 @@ fn end_drag( let ok = match &target { PileType::Foundation(_) => { count == 1 - && can_place_on_foundation( - &bottom_card, - &game.0.piles[&target], - ) + && game.0.piles.get(&target) + .is_some_and(|p| can_place_on_foundation(&bottom_card, p)) } PileType::Tableau(_) => { - can_place_on_tableau(&bottom_card, &game.0.piles[&target]) + game.0.piles.get(&target) + .is_some_and(|p| can_place_on_tableau(&bottom_card, p)) } _ => false, }; @@ -972,10 +971,12 @@ fn touch_end_drag( let ok = match &target { PileType::Foundation(_) => { count == 1 - && can_place_on_foundation(&bottom_card, &game.0.piles[&target]) + && game.0.piles.get(&target) + .is_some_and(|p| can_place_on_foundation(&bottom_card, p)) } PileType::Tableau(_) => { - can_place_on_tableau(&bottom_card, &game.0.piles[&target]) + game.0.piles.get(&target) + .is_some_and(|p| can_place_on_tableau(&bottom_card, p)) } _ => false, }; diff --git a/solitaire_engine/src/ui_modal.rs b/solitaire_engine/src/ui_modal.rs index 0449c01..d0cdd7d 100644 --- a/solitaire_engine/src/ui_modal.rs +++ b/solitaire_engine/src/ui_modal.rs @@ -603,7 +603,7 @@ pub fn dismiss_modal_on_scrim_click( mut commands: Commands, mouse: Option>>, windows: Query<&Window, With>, - scrims: Query, With)>, + scrims: Query<(Entity, &Children), (With, With)>, cards: Query<(&UiGlobalTransform, &ComputedNode), With>, ) { let Some(mouse) = mouse else { return }; @@ -620,15 +620,19 @@ pub fn dismiss_modal_on_scrim_click( // Topmost-only: bail after the first dismissible scrim. Stacked // dismissible modals are not currently a real case, but this guard // keeps the behaviour predictable if they ever arise. - let Some(scrim_entity) = scrims.iter().next() else { + let Some((scrim_entity, scrim_children)) = scrims.iter().next() else { return; }; - let cursor_over_card = cards.iter().any(|(transform, computed)| { - let inv = computed.inverse_scale_factor; - let size_logical = computed.size() * inv; - let centre_logical = transform.translation * inv; - cursor_is_inside_rect(cursor, centre_logical, size_logical) + // Only test the ModalCard(s) that belong to THIS scrim, not cards + // from any other concurrently-open modal. + let cursor_over_card = scrim_children.iter().any(|child| { + cards.get(child).is_ok_and(|(transform, computed)| { + let inv = computed.inverse_scale_factor; + let size_logical = computed.size() * inv; + let centre_logical = transform.translation * inv; + cursor_is_inside_rect(cursor, centre_logical, size_logical) + }) }); if !cursor_over_card {