diff --git a/client/scripts/main.gd b/client/scripts/main.gd index 34a553f59..c6aa484aa 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -49,6 +49,10 @@ func _ready() -> void: # Connect to simulation (test mode sets CONNECTED immediately). # Guard: Option A flow leaves SimBridge CONNECTED when main.tscn loads — don't drop it. + # Option A state handoff: main_menu polls and applies the snapshot first, + # seeding GameState (including bookmark_catalog) via the autoload before the + # scene swap. main.tscn then re-applies the next snapshot on top. Both paths + # write through GameState — which is autoloaded, so catalog state survives. if SimBridge.state == SimBridge.ConnectionState.DISCONNECTED: SimBridge.connect_to_sim() @@ -267,19 +271,9 @@ func _process(delta: float) -> void: elif err != OK: push_error("main.gd: LOAD_GAME send_input failed: %s" % error_string(err)) continue - # #528: ESC/OPEN_MENU — priority chain: MetaStack modal → implant app → settings dialog + # #528: ESC/OPEN_MENU — delegate to ordered priority chain if input.action == InputMapper.Action.OPEN_MENU: - if MetaStack.handle_escape(): - continue - if HudGroups.is_implant_active(): - HudGroups.close_app() - continue - if settings_dialog: - if settings_dialog.is_open(): - settings_dialog.close() - else: - MetaStack.push(settings_dialog) - settings_dialog.open() + _handle_menu_key() continue if input.action == InputMapper.Action.INTERACT: # D-057: prefer interaction list (multi-verb), fall back to prompt (v0.1) @@ -314,6 +308,24 @@ func _process(delta: float) -> void: _pending_record_inputs.clear() +# #528: ESC/OPEN_MENU priority chain — first handler to consume wins. +# Order matters: MetaStack modal > open implant app > settings dialog. +# Adding a fourth handler: append a new step here; don't re-inline in _process. +func _handle_menu_key() -> void: + if MetaStack.handle_escape(): + return + if HudGroups.is_implant_active(): + HudGroups.close_app() + return + if settings_dialog == null: + return + if settings_dialog.is_open(): + settings_dialog.close() + else: + MetaStack.push(settings_dialog) + settings_dialog.open() + + # #496: Finalize gauntlet stats on disconnect func _on_connection_state_changed( _old_state: SimBridge.ConnectionState, new_state: SimBridge.ConnectionState diff --git a/client/ui/meta/meta_screen.gd b/client/ui/meta/meta_screen.gd index 4d47e17ca..7ece24053 100644 --- a/client/ui/meta/meta_screen.gd +++ b/client/ui/meta/meta_screen.gd @@ -22,8 +22,7 @@ func open() -> void: return _phase = Phase.OPENING visible = true - if captures_input: - mouse_filter = Control.MOUSE_FILTER_STOP + mouse_filter = Control.MOUSE_FILTER_STOP if captures_input else Control.MOUSE_FILTER_IGNORE on_open() _phase = Phase.OPEN @@ -34,6 +33,7 @@ func close() -> void: _phase = Phase.CLOSING on_close() visible = false + mouse_filter = Control.MOUSE_FILTER_IGNORE _phase = Phase.HIDDEN closed.emit() @@ -43,7 +43,16 @@ func is_open() -> bool: ## Called by MetaStack when ESC is pressed with this screen on top. -## Return true to consume the event (prevent stack pop); false to allow pop. +## +## Return true: consumed-and-held — keep this screen open (e.g. "are you sure" +## prompt was shown, do not close the underlying screen). +## Return false: did nothing internally — let MetaStack close this screen. +## +## To express "consume-and-hold" — the screen must remain open and ESC must NOT +## fall through to gameplay/implant — set `closable_by_escape = false` instead. +## MetaStack treats that as: call on_escape (to let the screen react), do not +## pop, return true so the event stops here. Returning true from `on_escape` is +## the per-event variant; setting the flag is the screen-wide variant. func on_escape() -> bool: escape_pressed.emit() return false diff --git a/client/ui/meta/meta_stack.gd b/client/ui/meta/meta_stack.gd index 1462502b6..bb8e0cc80 100644 --- a/client/ui/meta/meta_stack.gd +++ b/client/ui/meta/meta_stack.gd @@ -38,12 +38,18 @@ func is_active() -> bool: ## Handle ESC key. Call from main.gd before HudGroups ESC handling. ## Returns true if the event was consumed (callers must return after). +## +## A screen on the stack always consumes the event. `closable_by_escape = false` +## means "I refuse to close on ESC" — not "pass the event through to the +## implant/gameplay layer." Otherwise an un-escapable screen (e.g. LoadingScreen) +## would leak ESC to main.gd and open the settings dialog behind it. func handle_escape() -> bool: var t = top() if t == null: return false if not t.closable_by_escape: - return t.on_escape() + t.on_escape() + return true if t.on_escape(): return true t.close() diff --git a/client/ui/meta/screens/bug_report/bug_report_dialog.gd b/client/ui/meta/screens/bug_report/bug_report_dialog.gd index 56c2bced9..405f64212 100644 --- a/client/ui/meta/screens/bug_report/bug_report_dialog.gd +++ b/client/ui/meta/screens/bug_report/bug_report_dialog.gd @@ -37,6 +37,7 @@ const RING_SIZE := 60 var _line_edit: LineEdit = null var _captured_screenshot: Image = null +var _completed: bool = false # set by _on_text_submitted; suppresses on_close cancel # #507: Pre-allocated ring buffers (no per-tick allocation after _ready). # Input ring: replay-format PlayerInput arrays, one per tick. @@ -210,6 +211,7 @@ func _get_filled_snapshot_count() -> int: func start_capture() -> void: if is_open(): return + _completed = false # reset completion flag for this capture session # Capture screenshot BEFORE showing the dialog overlay _captured_screenshot = get_viewport().get_texture().get_image() MetaStack.push(self) @@ -236,17 +238,24 @@ func on_close() -> void: _line_edit.queue_free() _line_edit = null _captured_screenshot = null + # Any close path that did not complete is a cancel — covers ESC, MetaStack + # pop, programmatic close(). _completed flips to true in _on_text_submitted + # right before capture_completed fires, so the two signals stay exclusive. + if not _completed: + capture_cancelled.emit() + _completed = false func on_escape() -> bool: - capture_cancelled.emit() + # on_close() will emit capture_cancelled — don't double-emit here. return false # let MetaStack close func _on_text_submitted(text: String) -> void: _save_report(text) - close() + _completed = true capture_completed.emit() + close() func _save_report(description: String) -> void: diff --git a/client/ui/meta/screens/debug_console/debug_console.gd b/client/ui/meta/screens/debug_console/debug_console.gd index c2166a646..8fc2f6697 100644 --- a/client/ui/meta/screens/debug_console/debug_console.gd +++ b/client/ui/meta/screens/debug_console/debug_console.gd @@ -108,10 +108,11 @@ func _unhandled_input(event: InputEvent) -> void: _toggle() return if is_open(): - # Consume all keyboard events — prevent movement/action leaking through - get_viewport().set_input_as_handled() - if event.keycode == KEY_ESCAPE: - close() + # Consume keyboard events so movement/action don't leak to main.gd, + # but let ESC fall through to OPEN_MENU → MetaStack.handle_escape(). + # MetaStack finds this console at the top of the stack and closes it. + if event.keycode != KEY_ESCAPE: + get_viewport().set_input_as_handled() func _on_input_key(event: InputEvent) -> void: diff --git a/client/ui/meta/screens/main_menu/main_menu.gd b/client/ui/meta/screens/main_menu/main_menu.gd index 479a90fa7..55b1badd6 100644 --- a/client/ui/meta/screens/main_menu/main_menu.gd +++ b/client/ui/meta/screens/main_menu/main_menu.gd @@ -3,6 +3,13 @@ extends MetaScreen ## New Game: opens character creation screen, then starts game. ## Continue: loads most recent save directory. ## Load Game: shows sorted save list for manual selection (#257). +## +## LoadingScreen lifecycle: this scene instantiates its own LoadingScreen child +## (see `_ensure_loading_screen`). main.tscn has a separate `$MetaLayer/LoadingScreen`. +## Safe today because main_menu.tscn and main.tscn never co-exist — the scene +## transition in `_start_game` replaces the tree wholesale. If that invariant +## ever changes (e.g. embedding the menu as an overlay), promote LoadingScreen +## to an autoload to enforce single-instance across the MetaStack. const GAME_SCENE := "res://scenes/main.tscn" const CHARACTER_CREATION_SCENE := "res://scenes/character_creation.tscn" diff --git a/decisions/architecture.md b/decisions/architecture.md index 384b2876c..ffb340960 100644 --- a/decisions/architecture.md +++ b/decisions/architecture.md @@ -751,7 +751,7 @@ Technical foundation decisions that constrain implementation: engine, client-ser ### D-192: Drop PROTOCOL_VERSION lockstep handshake -- **Decision:** Remove the `version` field from the snapshot envelope, the `PROTOCOL_VERSION` constants on both server (`server/src/bridge/types.rs`) and client (`client/scripts/protocol/protocol.gd`), and the version-mismatch guard in `Protocol.decode_snapshot()`. Genuine schema mismatches surface as MessagePack decode errors or missing-field errors at the consumer; that signal is sufficient for our deployment model. +- **Decision:** Deprecate the snapshot envelope `version` field, the `PROTOCOL_VERSION` constants on both server (`server/src/bridge/types.rs`) and client (`client/scripts/protocol/protocol.gd`), and the version-mismatch guard in `Protocol.decode_snapshot()`. Removal is tracked in ticket **#868** (server + client coordinated, sprint 37 or later). Once removed, genuine schema mismatches will surface as MessagePack decode errors or missing-field errors at the consumer; that signal is sufficient for our deployment model. Until #868 lands, the field and guard stay in place — they are no longer load-bearing, but removing them requires coordinated edits on both sides and fresh fixture regeneration. - **Rationale:** The version constants were designed for a network deployment where client and server can ship out of sync. Our actual deployment is a subprocess: the Godot client launches the Rust server it was built with. They are *always* in sync at runtime — the version check has never caught a real mismatch in the field, only dev-time forgetfulness. The cost has been measurable: every protocol-shaping sprint requires bumping two constants in lockstep, and we accumulated tautological tests asserting `PROTOCOL_VERSION == N` (deleted in sprint 36 — see ticket from this D-record). Removing the handshake makes the per-sprint cost zero. **Reversibility:** When/if networked multiplayer arrives (no firm date — see [D-005](#d-005-architecture-godot-client--rust-server-via-subprocess)), the natural fit is a one-time handshake at connection time (a single client-version vs. server-version exchange in the connection protocol), not a per-snapshot version stamp. So even the multiplayer path doesn't argue for keeping the per-snapshot field — that field would be doubly redundant once a connection-time check exists. The design space hasn't been narrowed. - **What we lose:** A single eager, human-readable error at connect time ("client v22 ↔ server v23"). A genuine dev-time schema drift will now surface as a downstream decode/missing-field error, possibly seconds into a session rather than at handshake. - **What we keep:** All field-presence and roundtrip tests in `test_protocol_bridge.gd`, `test_signal_sprint24.gd`, etc. — these cover the *behavior* the version constant was meant to gate. Decode failure in `Messagepack.decode()` still rejects malformed payloads.