fix(engine): eliminate panics, fix dismiss hit-test scope, guard home respawn
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 <noreply@anthropic.com>
This commit is contained in:
@@ -224,10 +224,10 @@ impl GameState {
|
|||||||
return Err(MoveError::GameAlreadyWon);
|
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 {
|
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 {
|
if waste_len == 0 {
|
||||||
return Err(MoveError::StockEmpty);
|
return Err(MoveError::StockEmpty);
|
||||||
}
|
}
|
||||||
@@ -428,14 +428,13 @@ impl GameState {
|
|||||||
pub fn check_auto_complete(&self) -> bool {
|
pub fn check_auto_complete(&self) -> bool {
|
||||||
// Stock must be empty; waste may still have cards (they are resolved
|
// Stock must be empty; waste may still have cards (they are resolved
|
||||||
// by draw() calls inside next_auto_complete_move / auto_complete_step).
|
// 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;
|
return false;
|
||||||
}
|
}
|
||||||
(0..7).all(|i| {
|
(0..7).all(|i| {
|
||||||
self.piles[&PileType::Tableau(i)]
|
self.piles
|
||||||
.cards
|
.get(&PileType::Tableau(i))
|
||||||
.iter()
|
.is_some_and(|p| p.cards.iter().all(|c| c.face_up))
|
||||||
.all(|c| c.face_up)
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -461,7 +460,8 @@ impl GameState {
|
|||||||
// Check waste top first — when stock is exhausted the waste may still
|
// Check waste top first — when stock is exhausted the waste may still
|
||||||
// contain cards that can go directly to a foundation.
|
// contain cards that can go directly to a foundation.
|
||||||
let waste = PileType::Waste;
|
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)))
|
.and_then(|c| self.foundation_slot_for(c).map(|s| (c, s)))
|
||||||
{
|
{
|
||||||
let _ = card; // borrow ends here
|
let _ = card; // borrow ends here
|
||||||
@@ -469,7 +469,8 @@ impl GameState {
|
|||||||
}
|
}
|
||||||
for i in 0..7 {
|
for i in 0..7 {
|
||||||
let tableau = PileType::Tableau(i);
|
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))
|
.and_then(|c| self.foundation_slot_for(c))
|
||||||
{
|
{
|
||||||
return Some((tableau, PileType::Foundation(slot)));
|
return Some((tableau, PileType::Foundation(slot)));
|
||||||
@@ -487,7 +488,7 @@ impl GameState {
|
|||||||
let mut candidate: Option<u8> = None;
|
let mut candidate: Option<u8> = None;
|
||||||
let mut empty_slot: Option<u8> = None;
|
let mut empty_slot: Option<u8> = None;
|
||||||
for slot in 0..4_u8 {
|
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 pile.cards.is_empty() {
|
||||||
if empty_slot.is_none() {
|
if empty_slot.is_none() {
|
||||||
empty_slot = Some(slot);
|
empty_slot = Some(slot);
|
||||||
@@ -501,7 +502,8 @@ impl GameState {
|
|||||||
if card.rank.value() == 1 { empty_slot } else { None }
|
if card.rank.value() == 1 { empty_slot } else { None }
|
||||||
});
|
});
|
||||||
target.filter(|&slot| {
|
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))
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -589,6 +589,7 @@ fn handle_home_draw_mode_buttons(
|
|||||||
one_buttons: Query<&Interaction, (With<HomeDrawOneButton>, Changed<Interaction>)>,
|
one_buttons: Query<&Interaction, (With<HomeDrawOneButton>, Changed<Interaction>)>,
|
||||||
three_buttons: Query<&Interaction, (With<HomeDrawThreeButton>, Changed<Interaction>)>,
|
three_buttons: Query<&Interaction, (With<HomeDrawThreeButton>, Changed<Interaction>)>,
|
||||||
screens: Query<Entity, With<HomeScreen>>,
|
screens: Query<Entity, With<HomeScreen>>,
|
||||||
|
other_modal_scrims: Query<(), (With<crate::ui_modal::ModalScrim>, Without<HomeScreen>)>,
|
||||||
mut settings: Option<ResMut<SettingsResource>>,
|
mut settings: Option<ResMut<SettingsResource>>,
|
||||||
storage_path: Option<Res<SettingsStoragePath>>,
|
storage_path: Option<Res<SettingsStoragePath>>,
|
||||||
mut changed: MessageWriter<SettingsChangedEvent>,
|
mut changed: MessageWriter<SettingsChangedEvent>,
|
||||||
@@ -601,6 +602,12 @@ fn handle_home_draw_mode_buttons(
|
|||||||
if screens.is_empty() {
|
if screens.is_empty() {
|
||||||
return;
|
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_one = one_buttons.iter().any(|i| *i == Interaction::Pressed);
|
||||||
let want_three = three_buttons.iter().any(|i| *i == Interaction::Pressed);
|
let want_three = three_buttons.iter().any(|i| *i == Interaction::Pressed);
|
||||||
if !want_one && !want_three {
|
if !want_one && !want_three {
|
||||||
@@ -658,6 +665,7 @@ fn handle_home_difficulty_toggle(
|
|||||||
mut commands: Commands,
|
mut commands: Commands,
|
||||||
toggles: Query<&Interaction, (With<HomeDifficultyToggle>, Changed<Interaction>)>,
|
toggles: Query<&Interaction, (With<HomeDifficultyToggle>, Changed<Interaction>)>,
|
||||||
screens: Query<Entity, With<HomeScreen>>,
|
screens: Query<Entity, With<HomeScreen>>,
|
||||||
|
other_modal_scrims: Query<(), (With<crate::ui_modal::ModalScrim>, Without<HomeScreen>)>,
|
||||||
mut diff_expanded: ResMut<DifficultyExpanded>,
|
mut diff_expanded: ResMut<DifficultyExpanded>,
|
||||||
progress: Option<Res<ProgressResource>>,
|
progress: Option<Res<ProgressResource>>,
|
||||||
stats: Option<Res<StatsResource>>,
|
stats: Option<Res<StatsResource>>,
|
||||||
@@ -668,6 +676,9 @@ fn handle_home_difficulty_toggle(
|
|||||||
if screens.is_empty() {
|
if screens.is_empty() {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
if !other_modal_scrims.is_empty() {
|
||||||
|
return;
|
||||||
|
}
|
||||||
if !toggles.iter().any(|i| *i == Interaction::Pressed) {
|
if !toggles.iter().any(|i| *i == Interaction::Pressed) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -717,13 +717,12 @@ fn end_drag(
|
|||||||
let ok = match &target {
|
let ok = match &target {
|
||||||
PileType::Foundation(_) => {
|
PileType::Foundation(_) => {
|
||||||
count == 1
|
count == 1
|
||||||
&& can_place_on_foundation(
|
&& game.0.piles.get(&target)
|
||||||
&bottom_card,
|
.is_some_and(|p| can_place_on_foundation(&bottom_card, p))
|
||||||
&game.0.piles[&target],
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
PileType::Tableau(_) => {
|
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,
|
_ => false,
|
||||||
};
|
};
|
||||||
@@ -972,10 +971,12 @@ fn touch_end_drag(
|
|||||||
let ok = match &target {
|
let ok = match &target {
|
||||||
PileType::Foundation(_) => {
|
PileType::Foundation(_) => {
|
||||||
count == 1
|
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(_) => {
|
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,
|
_ => false,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -603,7 +603,7 @@ pub fn dismiss_modal_on_scrim_click(
|
|||||||
mut commands: Commands,
|
mut commands: Commands,
|
||||||
mouse: Option<Res<ButtonInput<MouseButton>>>,
|
mouse: Option<Res<ButtonInput<MouseButton>>>,
|
||||||
windows: Query<&Window, With<PrimaryWindow>>,
|
windows: Query<&Window, With<PrimaryWindow>>,
|
||||||
scrims: Query<Entity, (With<ModalScrim>, With<ScrimDismissible>)>,
|
scrims: Query<(Entity, &Children), (With<ModalScrim>, With<ScrimDismissible>)>,
|
||||||
cards: Query<(&UiGlobalTransform, &ComputedNode), With<ModalCard>>,
|
cards: Query<(&UiGlobalTransform, &ComputedNode), With<ModalCard>>,
|
||||||
) {
|
) {
|
||||||
let Some(mouse) = mouse else { return };
|
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
|
// Topmost-only: bail after the first dismissible scrim. Stacked
|
||||||
// dismissible modals are not currently a real case, but this guard
|
// dismissible modals are not currently a real case, but this guard
|
||||||
// keeps the behaviour predictable if they ever arise.
|
// 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;
|
return;
|
||||||
};
|
};
|
||||||
|
|
||||||
let cursor_over_card = cards.iter().any(|(transform, computed)| {
|
// Only test the ModalCard(s) that belong to THIS scrim, not cards
|
||||||
let inv = computed.inverse_scale_factor;
|
// from any other concurrently-open modal.
|
||||||
let size_logical = computed.size() * inv;
|
let cursor_over_card = scrim_children.iter().any(|child| {
|
||||||
let centre_logical = transform.translation * inv;
|
cards.get(child).is_ok_and(|(transform, computed)| {
|
||||||
cursor_is_inside_rect(cursor, centre_logical, size_logical)
|
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 {
|
if !cursor_over_card {
|
||||||
|
|||||||
Reference in New Issue
Block a user