feat(replay): wire ESC accelerator for stop, gate pause modal
ESC during an active replay now stops it (mirrors the existing Stop button click). UI-first contract from CLAUDE.md §3.3 holds for the keyboard accelerator: every keybind the footer surfaces points at a wired action. Cross-plugin coordination: pause_plugin's `toggle_pause` already listens for ESC and would otherwise open the pause modal on the same press. Resolved by adding a fourth defer-if check to the existing modal-stack pattern in `toggle_pause` — `replay_state.is_some_and(|s| s.is_playing())` slots in right after `other_modal_scrims` and before `selection`. Symmetric shape to the existing forfeit / modal-scrim / selection / game-over / drag gates. Footer hint extended from `[SPACE] pause/resume` to `[SPACE] pause/resume · [ESC] stop` in lockstep — the "only-wired-keybinds" discipline holds. 3 new tests: - esc_keyboard_stops_active_replay (positive: Esc → Inactive, overlay despawns next frame) - esc_keyboard_is_noop_when_not_playing (negative: doesn't fire on Inactive state, lets global Esc listeners own those frames) - keybind_footer_hint_lists_space_and_esc (footer text contains both keybinds) Plus updated helper-pin test for the new hint string. Existing pause_plugin tests unaffected (they don't insert a ReplayPlaybackState resource so the new gate is a no-op for them). Tests: 1240 → 1243 (+3). Clippy clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -30,6 +30,7 @@ use crate::events::{
|
||||
use crate::font_plugin::FontResource;
|
||||
use crate::game_plugin::{GameOverScreen, GameStatePath};
|
||||
use crate::progress_plugin::ProgressResource;
|
||||
use crate::replay_playback::ReplayPlaybackState;
|
||||
use crate::resources::{DragState, GameStateResource};
|
||||
use crate::selection_plugin::{SelectionKeySet, SelectionState};
|
||||
use crate::settings_plugin::{SettingsChangedEvent, SettingsResource, SettingsStoragePath};
|
||||
@@ -154,6 +155,7 @@ fn toggle_pause(
|
||||
mut drag: Option<ResMut<DragState>>,
|
||||
mut changed: MessageWriter<StateChangedEvent>,
|
||||
selection: Option<Res<SelectionState>>,
|
||||
replay_state: Option<Res<ReplayPlaybackState>>,
|
||||
) {
|
||||
let PauseModalQueries {
|
||||
pause_screens: screens,
|
||||
@@ -184,6 +186,15 @@ fn toggle_pause(
|
||||
if !other_modal_scrims.is_empty() {
|
||||
return;
|
||||
}
|
||||
// If a replay is currently playing, let `replay_overlay::handle_stop_keyboard`
|
||||
// own the Esc press — that handler stops the replay. Without this guard a
|
||||
// single Esc both stops the replay AND opens the pause modal on top of the
|
||||
// (now empty) board, leaving the player on a screen they didn't ask for.
|
||||
// The HUD-button path is gated too; clicking Pause while watching a replay
|
||||
// is almost always an accident.
|
||||
if replay_state.is_some_and(|s| s.is_playing()) {
|
||||
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()) {
|
||||
|
||||
@@ -286,6 +286,7 @@ impl Plugin for ReplayOverlayPlugin {
|
||||
handle_pause_button,
|
||||
handle_step_button,
|
||||
handle_pause_keyboard,
|
||||
handle_stop_keyboard,
|
||||
handle_stop_button,
|
||||
)
|
||||
.chain(),
|
||||
@@ -776,11 +777,12 @@ fn keybind_footer_mode_text() -> &'static str {
|
||||
/// Pure helper — returns the keybind-hint text shown on the right
|
||||
/// side of the keybind-hint footer row. Lists only the keys that
|
||||
/// are *actually wired* today: the Space accelerator for
|
||||
/// pause/resume. Future commits that wire ESC for stop or ← / → for
|
||||
/// scrub will extend this string — the footer never lists
|
||||
/// unimplemented keybinds (would lie to users).
|
||||
/// pause/resume and the ESC accelerator for stop. Future commits
|
||||
/// that wire ← / → for prev/next move will extend this string —
|
||||
/// the footer never lists unimplemented keybinds (would lie to
|
||||
/// users).
|
||||
fn keybind_footer_hint_text() -> &'static str {
|
||||
"[SPACE] pause/resume"
|
||||
"[SPACE] pause/resume \u{00B7} [ESC] stop" // · separator
|
||||
}
|
||||
|
||||
/// Pure helper — returns the WIN MOVE marker's left-edge position as
|
||||
@@ -1083,6 +1085,37 @@ fn handle_pause_keyboard(
|
||||
toggle_pause_replay_playback(&mut state);
|
||||
}
|
||||
|
||||
/// Watches `Esc` for the keyboard stop accelerator. UI-first
|
||||
/// contract from CLAUDE.md §3.3 is satisfied by the on-screen
|
||||
/// Stop button; this is the optional accelerator.
|
||||
///
|
||||
/// Cross-plugin coordination: `pause_plugin::toggle_pause` also
|
||||
/// listens for `Esc` and would otherwise open the pause modal on
|
||||
/// the same press. The conflict is resolved by `toggle_pause`
|
||||
/// gating itself on `ReplayPlaybackState::is_playing()` —
|
||||
/// symmetrical to the existing `forfeit_screens` /
|
||||
/// `other_modal_scrims` defer-if pattern in that system. So during
|
||||
/// an active replay this handler owns the `Esc` press and the
|
||||
/// pause modal stays closed.
|
||||
///
|
||||
/// No-op when the playback isn't `Playing` (the resource may still
|
||||
/// exist as `Inactive` or `Completed`; only `Playing` means a
|
||||
/// replay is on screen for the player to stop).
|
||||
fn handle_stop_keyboard(
|
||||
mut commands: Commands,
|
||||
keys: Option<Res<ButtonInput<KeyCode>>>,
|
||||
mut state: ResMut<ReplayPlaybackState>,
|
||||
) {
|
||||
let Some(keys) = keys else { return };
|
||||
if !keys.just_pressed(KeyCode::Escape) {
|
||||
return;
|
||||
}
|
||||
if !state.is_playing() {
|
||||
return;
|
||||
}
|
||||
stop_replay_playback(&mut commands, &mut state);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -1916,8 +1949,8 @@ mod tests {
|
||||
);
|
||||
assert_eq!(
|
||||
keybind_footer_hint_text(),
|
||||
"[SPACE] pause/resume",
|
||||
"hint text must list only wired keybinds (Space → pause/resume)",
|
||||
"[SPACE] pause/resume \u{00B7} [ESC] stop",
|
||||
"hint text must list both wired keybinds (Space → pause/resume, Esc → stop) separated by a middle dot",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2167,6 +2200,82 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// Pressing Esc while a replay is playing resets the state to
|
||||
/// `Inactive` (same end-state as clicking the Stop button).
|
||||
/// Mirrors `space_keyboard_toggles_paused_flag` for the stop
|
||||
/// accelerator.
|
||||
#[test]
|
||||
fn esc_keyboard_stops_active_replay() {
|
||||
let mut app = headless_app();
|
||||
// The keyboard handler reads `Option<Res<ButtonInput<KeyCode>>>`
|
||||
// and no-ops when missing — provide it for this test.
|
||||
app.init_resource::<ButtonInput<KeyCode>>();
|
||||
set_state(&mut app, running_state(5, 0));
|
||||
app.update();
|
||||
assert_eq!(overlay_root_count(&mut app), 1);
|
||||
|
||||
app.world_mut()
|
||||
.resource_mut::<ButtonInput<KeyCode>>()
|
||||
.press(KeyCode::Escape);
|
||||
app.update();
|
||||
|
||||
assert!(
|
||||
matches!(
|
||||
app.world().resource::<ReplayPlaybackState>(),
|
||||
ReplayPlaybackState::Inactive
|
||||
),
|
||||
"Esc must reset state to Inactive while replay is Playing",
|
||||
);
|
||||
|
||||
// One more tick — `react_to_state_change` despawns the overlay
|
||||
// in response to the state going Inactive.
|
||||
app.update();
|
||||
assert_eq!(
|
||||
overlay_root_count(&mut app),
|
||||
0,
|
||||
"overlay must despawn the frame after Esc stops the replay",
|
||||
);
|
||||
}
|
||||
|
||||
/// Esc is a no-op when the replay isn't `Playing` — covers
|
||||
/// `Inactive` (no replay attached) and `Completed` (auto-clear
|
||||
/// underway). The handler must stay quiet so the global Esc
|
||||
/// listeners (pause modal, etc.) own those frames.
|
||||
#[test]
|
||||
fn esc_keyboard_is_noop_when_not_playing() {
|
||||
let mut app = headless_app();
|
||||
app.init_resource::<ButtonInput<KeyCode>>();
|
||||
// Resource defaults to Inactive — no replay attached.
|
||||
app.update();
|
||||
|
||||
app.world_mut()
|
||||
.resource_mut::<ButtonInput<KeyCode>>()
|
||||
.press(KeyCode::Escape);
|
||||
app.update();
|
||||
|
||||
// State stays Inactive — no spurious mutation.
|
||||
assert!(matches!(
|
||||
app.world().resource::<ReplayPlaybackState>(),
|
||||
ReplayPlaybackState::Inactive
|
||||
));
|
||||
}
|
||||
|
||||
/// The keybind-footer hint text now lists both wired
|
||||
/// accelerators (Space + Esc). Lock the format so a future edit
|
||||
/// that drops one or the other has to also update this test.
|
||||
#[test]
|
||||
fn keybind_footer_hint_lists_space_and_esc() {
|
||||
let hint = keybind_footer_hint_text();
|
||||
assert!(
|
||||
hint.contains("[SPACE]"),
|
||||
"hint must surface the Space accelerator; got {hint:?}",
|
||||
);
|
||||
assert!(
|
||||
hint.contains("[ESC]"),
|
||||
"hint must surface the Esc accelerator; got {hint:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn space_keyboard_toggles_paused_flag() {
|
||||
let mut app = headless_app();
|
||||
|
||||
Reference in New Issue
Block a user