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::font_plugin::FontResource;
|
||||||
use crate::game_plugin::{GameOverScreen, GameStatePath};
|
use crate::game_plugin::{GameOverScreen, GameStatePath};
|
||||||
use crate::progress_plugin::ProgressResource;
|
use crate::progress_plugin::ProgressResource;
|
||||||
|
use crate::replay_playback::ReplayPlaybackState;
|
||||||
use crate::resources::{DragState, GameStateResource};
|
use crate::resources::{DragState, GameStateResource};
|
||||||
use crate::selection_plugin::{SelectionKeySet, SelectionState};
|
use crate::selection_plugin::{SelectionKeySet, SelectionState};
|
||||||
use crate::settings_plugin::{SettingsChangedEvent, SettingsResource, SettingsStoragePath};
|
use crate::settings_plugin::{SettingsChangedEvent, SettingsResource, SettingsStoragePath};
|
||||||
@@ -154,6 +155,7 @@ fn toggle_pause(
|
|||||||
mut drag: Option<ResMut<DragState>>,
|
mut drag: Option<ResMut<DragState>>,
|
||||||
mut changed: MessageWriter<StateChangedEvent>,
|
mut changed: MessageWriter<StateChangedEvent>,
|
||||||
selection: Option<Res<SelectionState>>,
|
selection: Option<Res<SelectionState>>,
|
||||||
|
replay_state: Option<Res<ReplayPlaybackState>>,
|
||||||
) {
|
) {
|
||||||
let PauseModalQueries {
|
let PauseModalQueries {
|
||||||
pause_screens: screens,
|
pause_screens: screens,
|
||||||
@@ -184,6 +186,15 @@ fn toggle_pause(
|
|||||||
if !other_modal_scrims.is_empty() {
|
if !other_modal_scrims.is_empty() {
|
||||||
return;
|
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
|
// 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.
|
// (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()) {
|
if selection.is_some_and(|s| s.selected_pile.is_some()) {
|
||||||
|
|||||||
@@ -286,6 +286,7 @@ impl Plugin for ReplayOverlayPlugin {
|
|||||||
handle_pause_button,
|
handle_pause_button,
|
||||||
handle_step_button,
|
handle_step_button,
|
||||||
handle_pause_keyboard,
|
handle_pause_keyboard,
|
||||||
|
handle_stop_keyboard,
|
||||||
handle_stop_button,
|
handle_stop_button,
|
||||||
)
|
)
|
||||||
.chain(),
|
.chain(),
|
||||||
@@ -776,11 +777,12 @@ fn keybind_footer_mode_text() -> &'static str {
|
|||||||
/// Pure helper — returns the keybind-hint text shown on the right
|
/// Pure helper — returns the keybind-hint text shown on the right
|
||||||
/// side of the keybind-hint footer row. Lists only the keys that
|
/// side of the keybind-hint footer row. Lists only the keys that
|
||||||
/// are *actually wired* today: the Space accelerator for
|
/// are *actually wired* today: the Space accelerator for
|
||||||
/// pause/resume. Future commits that wire ESC for stop or ← / → for
|
/// pause/resume and the ESC accelerator for stop. Future commits
|
||||||
/// scrub will extend this string — the footer never lists
|
/// that wire ← / → for prev/next move will extend this string —
|
||||||
/// unimplemented keybinds (would lie to users).
|
/// the footer never lists unimplemented keybinds (would lie to
|
||||||
|
/// users).
|
||||||
fn keybind_footer_hint_text() -> &'static str {
|
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
|
/// 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);
|
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
|
// Tests
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -1916,8 +1949,8 @@ mod tests {
|
|||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
keybind_footer_hint_text(),
|
keybind_footer_hint_text(),
|
||||||
"[SPACE] pause/resume",
|
"[SPACE] pause/resume \u{00B7} [ESC] stop",
|
||||||
"hint text must list only wired keybinds (Space → pause/resume)",
|
"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]
|
#[test]
|
||||||
fn space_keyboard_toggles_paused_flag() {
|
fn space_keyboard_toggles_paused_flag() {
|
||||||
let mut app = headless_app();
|
let mut app = headless_app();
|
||||||
|
|||||||
Reference in New Issue
Block a user