From 84105916cdcf9341095fbdeeb461dc1dcfab3498 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 19 Apr 2026 20:18:29 +0200 Subject: [PATCH 01/18] feat(ui): introduce MetaScreen pattern foundation (#618, #680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Establishes the faux-game-menu base pattern for non-diegetic UI, analogous to ImplantApp but for pre-gameplay and meta-overlay screens (main menu, character creation, settings, debug console, bug report, loading screen). Workstream 1 of the MetaScreen refactor — foundation only, no screen migrations yet. - client/ui/meta/meta_screen.gd: base class (Control) with HIDDEN/OPENING/OPEN/CLOSING phase tracking, three orthogonal policy booleans (pauses_sim, closable_by_escape, captures_input), open/close lifecycle, on_escape contract, closed + escape_pressed signals, subclass hooks (on_open, on_close). - client/ui/meta/meta_stack.gd: autoload coordinator. Overlay stack with push/pop/top/is_active; handle_escape chain; sim-pause coordination via SimBridge when pauses_sim=true; meta_active_changed signal. All class references kept inside method bodies — no top-level class_name refs, matching HudGroups / GameState autoload parse-order discipline. - client/scripts/character_profile.gd: Resource wrapping the visual descriptor with bookmark_id and start_location_id. Target of the creation_confirmed signal once the character creation flow migrates. - client/project.godot: MetaStack registered as autoload after HudGroups, before ImplantRegistry. Co-Authored-By: Claude Opus 4.6 --- client/project.godot | 1 + client/scripts/character_profile.gd | 8 ++++ client/ui/meta/meta_screen.gd | 60 ++++++++++++++++++++++++ client/ui/meta/meta_stack.gd | 72 +++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+) create mode 100644 client/scripts/character_profile.gd create mode 100644 client/ui/meta/meta_screen.gd create mode 100644 client/ui/meta/meta_stack.gd diff --git a/client/project.godot b/client/project.godot index cb5202bbf..8997d2f61 100644 --- a/client/project.godot +++ b/client/project.godot @@ -29,6 +29,7 @@ FogState="*res://scripts/autoloads/fog_state.gd" AudioManager="*res://scripts/autoloads/audio_manager.gd" SessionManager="*res://scripts/autoloads/session_manager.gd" HudGroups="*res://scripts/autoloads/hud_groups.gd" +MetaStack="*res://ui/meta/meta_stack.gd" ImplantRegistry="*res://ui/implant/implant_registry.gd" HardwareDetector="*res://ui/hardware_detector.gd" diff --git a/client/scripts/character_profile.gd b/client/scripts/character_profile.gd new file mode 100644 index 000000000..7473e5a75 --- /dev/null +++ b/client/scripts/character_profile.gd @@ -0,0 +1,8 @@ +class_name CharacterProfile +extends Resource +## Collects all character creation choices into a single transferable object (#618). +## Passed as the argument to character_creation's creation_confirmed signal. + +@export var descriptor: CharacterVisualDescriptor +@export var bookmark_id: String = "" +@export var start_location_id: String = "" diff --git a/client/ui/meta/meta_screen.gd b/client/ui/meta/meta_screen.gd new file mode 100644 index 000000000..4d47e17ca --- /dev/null +++ b/client/ui/meta/meta_screen.gd @@ -0,0 +1,60 @@ +class_name MetaScreen +extends Control +## Base class for all meta-UI screens (#618, #680). +## Scene-root screens (main_menu, character_creation) extend this for the +## lifecycle contract. Overlay screens (settings, debug_console, bug_report, +## loading_screen) extend this AND push onto MetaStack. + +signal closed +signal escape_pressed + +enum Phase { HIDDEN, OPENING, OPEN, CLOSING } + +@export var pauses_sim: bool = false +@export var closable_by_escape: bool = true +@export var captures_input: bool = true + +var _phase: Phase = Phase.HIDDEN + + +func open() -> void: + if _phase != Phase.HIDDEN: + return + _phase = Phase.OPENING + visible = true + if captures_input: + mouse_filter = Control.MOUSE_FILTER_STOP + on_open() + _phase = Phase.OPEN + + +func close() -> void: + if _phase != Phase.OPEN: + return + _phase = Phase.CLOSING + on_close() + visible = false + _phase = Phase.HIDDEN + closed.emit() + + +func is_open() -> bool: + return _phase == Phase.OPEN + + +## 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. +func on_escape() -> bool: + escape_pressed.emit() + return false + + +# --- Lifecycle hooks — subclasses override --- + + +func on_open() -> void: + pass + + +func on_close() -> void: + pass diff --git a/client/ui/meta/meta_stack.gd b/client/ui/meta/meta_stack.gd new file mode 100644 index 000000000..1462502b6 --- /dev/null +++ b/client/ui/meta/meta_stack.gd @@ -0,0 +1,72 @@ +extends Node +## Autoload coordinator for overlay MetaScreens (#618, #680). +## Scene-root screens (main_menu, character_creation) do NOT push onto this stack. +## Only overlay screens (settings, debug_console, bug_report, loading_screen) push. +## +## Autoload parse-order: MetaScreen is a class_name type — referenced here only +## inside method bodies called at runtime, never at the top level or in _ready(). + +signal meta_active_changed(active: bool) + +var _stack: Array = [] # Array[MetaScreen] — untyped per parse-order rule + + +func push(screen) -> void: # screen: MetaScreen + if screen in _stack: + return + _stack.append(screen) + screen.closed.connect(_on_screen_closed.bind(screen), CONNECT_ONE_SHOT) + if _stack.size() == 1: + meta_active_changed.emit(true) + if screen.pauses_sim: + _request_pause(true) + + +func pop() -> void: + if _stack.is_empty(): + return + _stack[-1].close() + + +func top(): # returns MetaScreen or null + return _stack[-1] if not _stack.is_empty() else null + + +func is_active() -> bool: + return not _stack.is_empty() + + +## Handle ESC key. Call from main.gd before HudGroups ESC handling. +## Returns true if the event was consumed (callers must return after). +func handle_escape() -> bool: + var t = top() + if t == null: + return false + if not t.closable_by_escape: + return t.on_escape() + if t.on_escape(): + return true + t.close() + return true + + +func _on_screen_closed(screen) -> void: # screen: MetaScreen + _stack.erase(screen) + if screen.pauses_sim and not _any_pausing(): + _request_pause(false) + if _stack.is_empty(): + meta_active_changed.emit(false) + + +func _any_pausing() -> bool: + for s in _stack: + if s.pauses_sim: + return true + return false + + +func _request_pause(pause: bool) -> void: + if SimBridge.state != SimBridge.ConnectionState.CONNECTED: + return + var action = InputMapper.Action.PAUSE if pause else InputMapper.Action.UNPAUSE + SimBridge.send_input({"action": action, "timestamp_msec": Time.get_ticks_msec()}) From f24d08f7566fa1be04d24ea286b26440e8889145 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Sun, 19 Apr 2026 21:58:24 +0200 Subject: [PATCH 02/18] refactor(ui): migrate 6 meta screens to MetaScreen pattern (Workstream 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relocates main_menu, character_creation, settings_dialog, debug_console, bug_report_dialog, loading_screen from flat client/ui/ into structured client/ui/meta/screens//. All six now extend MetaScreen instead of Control; the base handles open/close lifecycle, visibility, captures_input, and — for overlays — the sim-pause contract. Screen policies set per Tyre's proposal: - settings_dialog: pauses_sim=false, PUSHES onto MetaStack - debug_console: pauses_sim=true, PUSHES (D-088 routing via base) - bug_report_dialog: pauses_sim=true, PUSHES - loading_screen: closable_by_escape=false, PUSHES - main_menu, character_creation: scene-roots, extend MetaScreen for the lifecycle contract only, do NOT push onto the stack character_creation stays at its current surface (tabs, descriptor, creation_confirmed signal unchanged). Tab consolidation and CharacterProfile migration happen in Workstreams 5 and 6. Knock-on changes: - main.tscn ModalLayer CanvasLayer renamed to MetaLayer; main.gd @onready refs updated; constants.gd comment updated; test_client_p3 and test_ui_framework_sprint15 assertions updated; test_monologue_display and .tscn header comments updated. - OPEN_MENU handler now pushes settings_dialog onto MetaStack before calling open(). Full ESC priority chain lands in Workstream 4. - atlas_app.gd: _unhandled_key_input signature widened from InputEventKey to InputEvent with an is-check, per Godot 4 API. Pre- existing narrowing was silently tolerated until main.tscn started fully instantiating under the new pattern. - test_client_p3: entity_renderer type annotations corrected from ColorRect to Sprite2D (stale since a prior refactor); facing indicator rotation assertion switched to angle_difference() for modular-safe comparison. Verification: - gdlint client/scripts/ client/ui/ — zero problems - godot --headless --path client --quit — no SCRIPT ERROR - test_client_p3: 24/24 pass - test_ui_framework_sprint15: 54/54 pass - test_implant_nav_stack: 52/52 pass - test_implant_registry: 42/42 pass - test_implant_app_lifecycle: 36/36 pass Workstream 1 foundation (84105916) remains unchanged. Workstreams 3-8 follow: protocol layer, Option A sequencing, 3-tab restructure, Bookmark tab, location picker, Skills stub. Co-Authored-By: Claude Opus 4.6 --- client/scenes/character_creation.tscn | 2 +- client/scenes/main.tscn | 10 +-- client/scenes/main_menu.tscn | 2 +- client/scripts/constants.gd | 2 +- client/scripts/main.gd | 9 +-- client/tests/test_client_p3.gd | 19 +++--- client/tests/test_monologue_display.gd | 2 +- client/tests/test_ui_framework_sprint15.gd | 6 +- client/ui/bug_report_dialog.tscn | 4 +- client/ui/debug_console.tscn | 4 +- client/ui/implant/apps/atlas/atlas_app.gd | 6 +- client/ui/loading_screen.tscn | 2 +- .../screens/bug_report}/bug_report_dialog.gd | 68 ++++++------------- .../character_creation}/character_creation.gd | 2 +- .../screens/debug_console}/debug_console.gd | 37 ++++------ .../screens/loading}/loading_screen.gd | 10 +-- .../{ => meta/screens/main_menu}/main_menu.gd | 2 +- .../screens/settings}/settings_dialog.gd | 27 ++------ client/ui/settings_dialog.tscn | 2 +- 19 files changed, 86 insertions(+), 130 deletions(-) rename client/ui/{ => meta/screens/bug_report}/bug_report_dialog.gd (95%) rename client/ui/{ => meta/screens/character_creation}/character_creation.gd (99%) rename client/ui/{ => meta/screens/debug_console}/debug_console.gd (97%) rename client/ui/{ => meta/screens/loading}/loading_screen.gd (95%) rename client/ui/{ => meta/screens/main_menu}/main_menu.gd (99%) rename client/ui/{ => meta/screens/settings}/settings_dialog.gd (97%) diff --git a/client/scenes/character_creation.tscn b/client/scenes/character_creation.tscn index 22743b56e..0b2453998 100644 --- a/client/scenes/character_creation.tscn +++ b/client/scenes/character_creation.tscn @@ -1,6 +1,6 @@ [gd_scene load_steps=3 format=3 uid="uid://char_creation_scene_sr"] -[ext_resource type="Script" path="res://ui/character_creation.gd" id="1_charcreation"] +[ext_resource type="Script" path="res://ui/meta/screens/character_creation/character_creation.gd" id="1_charcreation"] ; #705: Character creation screen — 3D preview + 5-tab customisation panel. ; SubViewport renders CharacterVisual live. Tab panel: Body/Head/Hair/Clothing/Accessories. diff --git a/client/scenes/main.tscn b/client/scenes/main.tscn index f7e9ec716..607c2b36a 100644 --- a/client/scenes/main.tscn +++ b/client/scenes/main.tscn @@ -187,17 +187,17 @@ script = ExtResource("10_cursor") ; --- Modal layer (CanvasLayer 30) --- ; Full-screen overlays: pause menu, inventory modal, death screen. -[node name="ModalLayer" type="CanvasLayer" parent="."] +[node name="MetaLayer" type="CanvasLayer" parent="."] layer = 30 ; #495: WRONG button (F12) — bug report capture dialog -[node name="BugReportDialog" parent="ModalLayer" instance=ExtResource("19_bugreport")] +[node name="BugReportDialog" parent="MetaLayer" instance=ExtResource("19_bugreport")] ; #528: Audio settings dialog — 5-bus volume sliders, ESC/OPEN_MENU to toggle -[node name="SettingsDialog" parent="ModalLayer" instance=ExtResource("21_settings")] +[node name="SettingsDialog" parent="MetaLayer" instance=ExtResource("21_settings")] ; #257: Loading screen — full-screen overlay during save/load round-trip -[node name="LoadingScreen" parent="ModalLayer" instance=ExtResource("26_loading")] +[node name="LoadingScreen" parent="MetaLayer" instance=ExtResource("26_loading")] ; #581: Debug console — tilde key toggles, bottom 40% of screen -[node name="DebugConsole" parent="ModalLayer" instance=ExtResource("27_debug_console")] +[node name="DebugConsole" parent="MetaLayer" instance=ExtResource("27_debug_console")] diff --git a/client/scenes/main_menu.tscn b/client/scenes/main_menu.tscn index 09afdc2d1..53d040653 100644 --- a/client/scenes/main_menu.tscn +++ b/client/scenes/main_menu.tscn @@ -1,6 +1,6 @@ [gd_scene load_steps=2 format=3 uid="uid://main_menu_sr"] -[ext_resource type="Script" path="res://ui/main_menu.gd" id="1_mainmenu"] +[ext_resource type="Script" path="res://ui/meta/screens/main_menu/main_menu.gd" id="1_mainmenu"] ; Main menu — New Game / Continue / Quit. ; #258: D-085 per-game save directory created on New Game. diff --git a/client/scripts/constants.gd b/client/scripts/constants.gd index e7cfdf978..0099c7423 100644 --- a/client/scripts/constants.gd +++ b/client/scripts/constants.gd @@ -40,7 +40,7 @@ const CANVAS_UI: int = 20 # CanvasLayer number for UILayer # # MODAL SCOPE (CanvasLayer 30) # Full-screen overlays: pause, inventory modal, death screen. -const CANVAS_MODAL: int = 30 # CanvasLayer number for ModalLayer +const CANVAS_MODAL: int = 30 # CanvasLayer number for MetaLayer # # Rendering ceiling: 10 floors (25m) above current floor. # Above this: no sprites, ground shadows + environmental effects only. diff --git a/client/scripts/main.gd b/client/scripts/main.gd index 488763b15..0c51f267f 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -30,10 +30,10 @@ var _dialogue: DialogueCoordinator # #775: dialogue consumers + signal handlers @onready var examine_display = $InsertOverlay/ExamineDisplay # #174: examine result overlay @onready var journal_panel = $InsertOverlay/JournalPanel # #264: knowledge journal (D-041) @onready var debug_overlay = $UILayer/DebugOverlay # #511: F3 debug overlay -@onready var bug_report_dialog = $ModalLayer/BugReportDialog # #495: F12 WRONG button -@onready var settings_dialog = $ModalLayer/SettingsDialog # #528: audio settings (ESC/OPEN_MENU) -@onready var loading_screen = $ModalLayer/LoadingScreen # #257: blocking overlay during load -@onready var debug_console = $ModalLayer/DebugConsole # #581: tilde debug console +@onready var bug_report_dialog = $MetaLayer/BugReportDialog # #495: F12 WRONG button +@onready var settings_dialog = $MetaLayer/SettingsDialog # #528: audio settings (ESC/OPEN_MENU) +@onready var loading_screen = $MetaLayer/LoadingScreen # #257: blocking overlay during load +@onready var debug_console = $MetaLayer/DebugConsole # #581: tilde debug console @onready var news_ticker = $UILayer/NewsTicker # #592: scrolling headline bar (D-049 z-7) @@ -271,6 +271,7 @@ func _process(delta: float) -> void: if settings_dialog.is_open(): settings_dialog.close() else: + MetaStack.push(settings_dialog) settings_dialog.open() continue if input.action == InputMapper.Action.INTERACT: diff --git a/client/tests/test_client_p3.gd b/client/tests/test_client_p3.gd index f8f14d1cb..e7e4c99d7 100644 --- a/client/tests/test_client_p3.gd +++ b/client/tests/test_client_p3.gd @@ -124,7 +124,7 @@ func test_z_ui_layer_above_world() -> void: var inst := _make_scene() var ui_layer = inst.get_node("UILayer") as CanvasLayer var insert_layer = inst.get_node("InsertOverlay") as CanvasLayer - var modal_layer = inst.get_node("ModalLayer") as CanvasLayer + var modal_layer = inst.get_node("MetaLayer") as CanvasLayer assert_that(insert_layer.layer).override_failure_message( "InsertOverlay must be CanvasLayer %d" % Constants.CANVAS_INSERT ).is_equal(Constants.CANVAS_INSERT) @@ -132,13 +132,13 @@ func test_z_ui_layer_above_world() -> void: "UILayer must be CanvasLayer %d" % Constants.CANVAS_UI ).is_equal(Constants.CANVAS_UI) assert_that(modal_layer.layer).override_failure_message( - "ModalLayer must be CanvasLayer %d" % Constants.CANVAS_MODAL + "MetaLayer must be CanvasLayer %d" % Constants.CANVAS_MODAL ).is_equal(Constants.CANVAS_MODAL) assert_that(ui_layer.layer > insert_layer.layer).override_failure_message( "UILayer must render above InsertOverlay" ).is_true() assert_that(modal_layer.layer > ui_layer.layer).override_failure_message( - "ModalLayer must render above UILayer" + "MetaLayer must render above UILayer" ).is_true() @@ -168,7 +168,7 @@ func test_entity_lerp_moves_toward_target() -> void: var entity := [{"entity_id": 11, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Npc", "data": null}}] renderer.update_entities(entity) - var node: ColorRect = renderer.entity_nodes[11] + var node: Sprite2D = renderer.entity_nodes[11] var start_pos: Vector2 = node.position # Move target to (6, 5) var entity_moved := [{"entity_id": 11, "x": 6.0, "y": 5.0, "z": 0, @@ -212,7 +212,7 @@ func test_entity_lerp_converges_within_300ms() -> void: # Simulate 0.3s at 60fps (18 frames × 0.016s ≈ 0.288s) for i in 20: renderer._process(0.016) - var final_node: ColorRect = renderer.entity_nodes[12] + var final_node: Sprite2D = renderer.entity_nodes[12] var final_pos: Vector2 = final_node.position # Should be within 5% of target (97% convergence at 0.3s) var dist: float = final_pos.distance_to(target) @@ -272,9 +272,10 @@ func test_facing_indicator_rotation_matches_input_mapper_angle() -> void: for angle in angles: InputMapper.facing_angle = angle renderer.update_entities(entity) - assert_that(indicator.rotation).override_failure_message( - "angle %.3f: expected rotation %.3f, got %.3f" % [angle, angles[angle], indicator.rotation] - ).is_equal_approx(angles[angle], 0.001) + var diff := absf(angle_difference(indicator.rotation, angles[angle])) + assert_that(diff).override_failure_message( + "angle %.3f: expected rotation %.3f, got %.3f (diff %.4f)" % [angle, angles[angle], indicator.rotation, diff] + ).is_less_equal(0.001) InputMapper.facing_angle = -PI / 2.0 # Reset to default renderer.queue_free() @@ -292,7 +293,7 @@ func test_lerp_weight_increases_with_delta() -> void: "kind": {"variant": "Npc", "data": null}}] renderer.update_entities(entity_moved) # Small delta step - var small_node: ColorRect = renderer.entity_nodes[20] + var small_node: Sprite2D = renderer.entity_nodes[20] var small_start: float = small_node.position.x renderer._process(0.008) var small_progress: float = small_node.position.x - small_start diff --git a/client/tests/test_monologue_display.gd b/client/tests/test_monologue_display.gd index 151f5fb77..4eee39917 100644 --- a/client/tests/test_monologue_display.gd +++ b/client/tests/test_monologue_display.gd @@ -472,7 +472,7 @@ func test_monologue_display_parented_to_canvas_layer_20_in_main_scene() -> void: ## D-049: Structural verification — MonologueDisplay must be a direct child of ## UILayer (CanvasLayer, layer=20) in the live scene tree, not the world layer. ## Catches regressions where the node gets accidentally moved to InsertOverlay - ## (layer=10) or ModalLayer (layer=30), or dropped into the world z-stack. + ## (layer=10) or MetaLayer (layer=30), or dropped into the world z-stack. ## ## Scene path verified: Game/UILayer/MonologueDisplay (main.tscn line 141). if not ResourceLoader.exists("res://scenes/main.tscn"): diff --git a/client/tests/test_ui_framework_sprint15.gd b/client/tests/test_ui_framework_sprint15.gd index 938aa98f3..f1164d1bb 100644 --- a/client/tests/test_ui_framework_sprint15.gd +++ b/client/tests/test_ui_framework_sprint15.gd @@ -66,13 +66,13 @@ func test_ui_layer_is_canvas_layer_20() -> void: func test_modal_layer_is_canvas_layer_30() -> void: - # D-049: ModalLayer = pause/inventory modal scope = CanvasLayer 30. + # D-049: MetaLayer = pause/inventory modal scope = CanvasLayer 30. var scene := MAIN_SCENE _instance = scene.instantiate() auto_free(_instance) add_child(_instance) - var modal_layer: CanvasLayer = _instance.get_node("ModalLayer") + var modal_layer: CanvasLayer = _instance.get_node("MetaLayer") assert_that(modal_layer).is_not_null() assert_that(modal_layer.layer).is_equal(Constants.CANVAS_MODAL) @@ -83,7 +83,7 @@ func test_ui_layer_above_insert_overlay() -> void: func test_modal_layer_above_ui_layer() -> void: - # D-049: ModalLayer (30) must render above UILayer (20). + # D-049: MetaLayer (30) must render above UILayer (20). assert_that(Constants.CANVAS_MODAL).is_greater(Constants.CANVAS_UI) diff --git a/client/ui/bug_report_dialog.tscn b/client/ui/bug_report_dialog.tscn index b4e8c8cab..cf1758a3e 100644 --- a/client/ui/bug_report_dialog.tscn +++ b/client/ui/bug_report_dialog.tscn @@ -1,8 +1,8 @@ [gd_scene load_steps=2 format=3] -[ext_resource type="Script" path="res://ui/bug_report_dialog.gd" id="1_bugreport"] +[ext_resource type="Script" path="res://ui/meta/screens/bug_report/bug_report_dialog.gd" id="1_bugreport"] -; #495: WRONG button (F12) — bug report capture dialog, ModalLayer +; #495: WRONG button (F12) — bug report capture dialog, MetaLayer [node name="BugReportDialog" type="Control"] layout_mode = 3 anchors_preset = 15 diff --git a/client/ui/debug_console.tscn b/client/ui/debug_console.tscn index 934729983..ee58b367e 100644 --- a/client/ui/debug_console.tscn +++ b/client/ui/debug_console.tscn @@ -1,8 +1,8 @@ [gd_scene load_steps=2 format=3 uid="uid://b2ndm9rvx8cqp"] -[ext_resource type="Script" uid="uid://c8pvt3xr7kmd2" path="res://ui/debug_console.gd" id="1_debug_console"] +[ext_resource type="Script" path="res://ui/meta/screens/debug_console/debug_console.gd" id="1_debug_console"] -; #581: In-game debug console. Tilde key toggles. ModalLayer. +; #581: In-game debug console. Tilde key toggles. MetaLayer. ; UI built programmatically in _ready() — scene contains only root node + script. [node name="DebugConsole" type="Control"] layout_mode = 3 diff --git a/client/ui/implant/apps/atlas/atlas_app.gd b/client/ui/implant/apps/atlas/atlas_app.gd index aaf8ff8d2..ad40d4b10 100644 --- a/client/ui/implant/apps/atlas/atlas_app.gd +++ b/client/ui/implant/apps/atlas/atlas_app.gd @@ -54,14 +54,16 @@ func on_open(_mode: int) -> void: _reach_screen.refresh_info_panel_visibility() -func _unhandled_key_input(event: InputEventKey) -> void: +func _unhandled_key_input(event: InputEvent) -> void: + if not event is InputEventKey: + return if manifest == null or not HudGroups.is_app_active(manifest.app_path): return if not event.is_pressed() or event.is_echo(): return if current_screen_id() == "regional": return # AtlasViewer handles its own keyboard input - _handle_key(event) + _handle_key(event as InputEventKey) get_viewport().set_input_as_handled() diff --git a/client/ui/loading_screen.tscn b/client/ui/loading_screen.tscn index 388e82640..fa01fbdbf 100644 --- a/client/ui/loading_screen.tscn +++ b/client/ui/loading_screen.tscn @@ -1,6 +1,6 @@ [gd_scene load_steps=2 format=3 uid="uid://b7rv9mkl4qpw3"] -[ext_resource type="Script" path="res://ui/loading_screen.gd" id="1_loading"] +[ext_resource type="Script" path="res://ui/meta/screens/loading/loading_screen.gd" id="1_loading"] ; #257: Loading screen — full-screen overlay shown during save/load round-trip. ; Blocks input; dismissed when save_result arrives from server. diff --git a/client/ui/bug_report_dialog.gd b/client/ui/meta/screens/bug_report/bug_report_dialog.gd similarity index 95% rename from client/ui/bug_report_dialog.gd rename to client/ui/meta/screens/bug_report/bug_report_dialog.gd index b9110b1b8..56c2bced9 100644 --- a/client/ui/bug_report_dialog.gd +++ b/client/ui/meta/screens/bug_report/bug_report_dialog.gd @@ -1,4 +1,4 @@ -extends Control +extends MetaScreen ## #507: WRONG button — full 60-tick capture: ring buffer, snapshot history, replay seed. ## Upgrade of the Sprint 9 MVP (#495). @@ -36,7 +36,6 @@ const PADDING := 16 const RING_SIZE := 60 var _line_edit: LineEdit = null -var _active: bool = false var _captured_screenshot: Image = null # #507: Pre-allocated ring buffers (no per-tick allocation after _ready). @@ -56,8 +55,9 @@ var _snapshot_count: int = 0 func _ready() -> void: + pauses_sim = true visible = false - mouse_filter = Control.MOUSE_FILTER_STOP + mouse_filter = Control.MOUSE_FILTER_IGNORE # Pre-allocate ring buffers — resize then fill sentinels. # The ring array itself never grows after _ready. Each write replaces the GDScript @@ -208,25 +208,15 @@ func _get_filled_snapshot_count() -> int: func start_capture() -> void: - if _active: + if is_open(): return # Capture screenshot BEFORE showing the dialog overlay _captured_screenshot = get_viewport().get_texture().get_image() - _active = true - visible = true + MetaStack.push(self) + open() - # Pause the simulation - ( - SimBridge - . send_input( - { - "action": InputMapper.Action.PAUSE, - "timestamp_msec": Time.get_ticks_msec(), - } - ) - ) - # Create the LineEdit dynamically +func on_open() -> void: _line_edit = LineEdit.new() _line_edit.placeholder_text = "Describe the issue..." _line_edit.size = Vector2(BOX_WIDTH - PADDING * 2, 30) @@ -240,41 +230,23 @@ func start_capture() -> void: _line_edit.grab_focus() -func _on_text_submitted(text: String) -> void: - _save_report(text) - _close() - capture_completed.emit() - - -func _unhandled_input(event: InputEvent) -> void: - if not _active: - return - if event is InputEventKey and event.pressed and event.keycode == KEY_ESCAPE: - _close() - capture_cancelled.emit() - get_viewport().set_input_as_handled() - - -func _close() -> void: - _active = false - visible = false +func on_close() -> void: if _line_edit: _line_edit.release_focus() _line_edit.queue_free() _line_edit = null - _captured_screenshot = null - # Unpause the simulation - ( - SimBridge - . send_input( - { - "action": InputMapper.Action.UNPAUSE, - "timestamp_msec": Time.get_ticks_msec(), - } - ) - ) + +func on_escape() -> bool: + capture_cancelled.emit() + return false # let MetaStack close + + +func _on_text_submitted(text: String) -> void: + _save_report(text) + close() + capture_completed.emit() func _save_report(description: String) -> void: @@ -456,7 +428,7 @@ func _render_snapshot_text() -> String: func _draw() -> void: - if not _active: + if not is_open(): return var viewport_size := get_viewport_rect().size # Full-screen dim @@ -489,4 +461,4 @@ func _draw() -> void: func is_active() -> bool: - return _active + return is_open() diff --git a/client/ui/character_creation.gd b/client/ui/meta/screens/character_creation/character_creation.gd similarity index 99% rename from client/ui/character_creation.gd rename to client/ui/meta/screens/character_creation/character_creation.gd index 56c8829a5..27cee6e39 100644 --- a/client/ui/character_creation.gd +++ b/client/ui/meta/screens/character_creation/character_creation.gd @@ -1,6 +1,6 @@ # gdlint:disable=max-file-lines class_name CharacterCreation -extends Control +extends MetaScreen ## #705: Character creation screen. ## Live 3D preview via SubViewport + 5-tab customisation panel (Body/Head/Hair/Clothing/Accessories). ## Emits creation_confirmed(descriptor) on start, creation_cancelled on back. diff --git a/client/ui/debug_console.gd b/client/ui/meta/screens/debug_console/debug_console.gd similarity index 97% rename from client/ui/debug_console.gd rename to client/ui/meta/screens/debug_console/debug_console.gd index ffcbbd6cd..c2166a646 100644 --- a/client/ui/debug_console.gd +++ b/client/ui/meta/screens/debug_console/debug_console.gd @@ -1,5 +1,5 @@ class_name DebugConsole -extends Control +extends MetaScreen ## In-game debug console (#581). Tilde key (`) toggles open/closed. ## Semi-transparent panel anchored to bottom ~40% of screen. @@ -23,7 +23,6 @@ const ERROR_COLOR := Color("#d45d5d") const INPUT_COLOR := Color("#e8c547") var _enabled: bool = true -var _open: bool = false var _log_lines: Array[String] = [] var _panel: PanelContainer = null var _output_log: RichTextLabel = null @@ -33,6 +32,7 @@ var _history_idx: int = -1 func _ready() -> void: + pauses_sim = true _load_prefs() visible = false mouse_filter = Control.MOUSE_FILTER_IGNORE @@ -107,11 +107,11 @@ func _unhandled_input(event: InputEvent) -> void: get_viewport().set_input_as_handled() _toggle() return - if _open: + if is_open(): # Consume all keyboard events — prevent movement/action leaking through get_viewport().set_input_as_handled() if event.keycode == KEY_ESCAPE: - _close() + close() func _on_input_key(event: InputEvent) -> void: @@ -126,34 +126,26 @@ func _on_input_key(event: InputEvent) -> void: func _toggle() -> void: - if _open: - _close() + if is_open(): + close() else: - _open_console() + MetaStack.push(self) + open() -func _open_console() -> void: - _open = true - visible = true - mouse_filter = Control.MOUSE_FILTER_STOP +func on_open() -> void: _input_line.clear() _input_line.grab_focus() _history_idx = -1 pause_requested.emit() # D-088: pause sim while typing debug commands -func _close() -> void: - _open = false - visible = false +func on_close() -> void: mouse_filter = Control.MOUSE_FILTER_IGNORE _input_line.release_focus() unpause_requested.emit() # D-088: resume sim when console closes -func is_open() -> bool: - return _open - - # -- Command input -- @@ -422,8 +414,9 @@ func append_response(response: Dictionary) -> void: var text: String = response.get("text", "") var color := SUCCESS_COLOR if success else ERROR_COLOR _append_text(text, color) - if not _open and _enabled: - _open_console() + if not is_open() and _enabled: + MetaStack.push(self) + open() # -- Log rendering -- @@ -492,8 +485,8 @@ func _history_down() -> void: func set_enabled(enabled: bool) -> void: _enabled = enabled - if not _enabled and _open: - _close() + if not _enabled and is_open(): + close() _save_prefs() diff --git a/client/ui/loading_screen.gd b/client/ui/meta/screens/loading/loading_screen.gd similarity index 95% rename from client/ui/loading_screen.gd rename to client/ui/meta/screens/loading/loading_screen.gd index 8a149f368..c7f343595 100644 --- a/client/ui/loading_screen.gd +++ b/client/ui/meta/screens/loading/loading_screen.gd @@ -1,4 +1,4 @@ -extends Control +extends MetaScreen ## #257: Loading screen overlay — blocks input during save/load round-trip. ## Shown when LOAD_GAME fires; hidden when save_result arrives (success or failure). ## Full-screen, dark overlay with centered status text. @@ -15,8 +15,9 @@ var _version_label: Label = null func _ready() -> void: + closable_by_escape = false visible = false - mouse_filter = Control.MOUSE_FILTER_STOP + mouse_filter = Control.MOUSE_FILTER_IGNORE set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) _build_ui() @@ -76,9 +77,10 @@ func _read_client_version() -> String: func show_loading() -> void: - visible = true + MetaStack.push(self) + open() ## Hide the loading overlay. success=false is reserved for future failure-state UI. func hide_loading(_success: bool = true) -> void: - visible = false + close() diff --git a/client/ui/main_menu.gd b/client/ui/meta/screens/main_menu/main_menu.gd similarity index 99% rename from client/ui/main_menu.gd rename to client/ui/meta/screens/main_menu/main_menu.gd index d578c3114..f861851bc 100644 --- a/client/ui/main_menu.gd +++ b/client/ui/meta/screens/main_menu/main_menu.gd @@ -1,4 +1,4 @@ -extends Control +extends MetaScreen ## #258: Main menu — New Game / Continue / Load Game / Quit. ## New Game: opens character creation screen, then starts game. ## Continue: loads most recent save directory. diff --git a/client/ui/settings_dialog.gd b/client/ui/meta/screens/settings/settings_dialog.gd similarity index 97% rename from client/ui/settings_dialog.gd rename to client/ui/meta/screens/settings/settings_dialog.gd index 1583dcd5f..f06cdc8cb 100644 --- a/client/ui/settings_dialog.gd +++ b/client/ui/meta/screens/settings/settings_dialog.gd @@ -1,4 +1,4 @@ -extends Control +extends MetaScreen ## #528: Audio settings dialog — 5-bus volume sliders. ## #646: AI-Enhanced Dialogue toggle + hardware detection status (D-138). @@ -6,7 +6,6 @@ extends Control ## Volumes persist via AudioManager._save_prefs() on each slider change. ## AI Dialogue toggle persists via ConfigFile (client-local) + ChangeSettings IPC (server SQLite). -signal closed signal debug_console_toggled(enabled: bool) # #581: debug console enabled/disabled signal ai_dialogue_toggled(enabled: bool) # #646: AI-Enhanced Dialogue enabled/disabled @@ -37,7 +36,6 @@ const BUS_ROWS: Array = [ ["UI Sounds", "UISounds"], ] -var _active: bool = false var _container: VBoxContainer = null # #646: AI Dialogue hardware status and toggle node ref — used by testable API methods @@ -49,30 +47,17 @@ var _ai_battery_warning_label: Label = null # shown when on battery; toggle sta func _ready() -> void: visible = false - mouse_filter = Control.MOUSE_FILTER_STOP + mouse_filter = Control.MOUSE_FILTER_IGNORE -func open() -> void: - if _active: - return - _active = true - visible = true +func on_open() -> void: _build_ui() queue_redraw() -func close() -> void: - if not _active: - return - _active = false - visible = false +func on_close() -> void: _destroy_ui() queue_redraw() - closed.emit() - - -func is_open() -> bool: - return _active func _build_ui() -> void: @@ -139,7 +124,7 @@ func _build_ui() -> void: var debug_check := CheckButton.new() # Query live DebugConsole node if available; fall back to prefs file - var console_node := get_node_or_null("/root/Main/ModalLayer/DebugConsole") + var console_node := get_node_or_null("/root/Main/MetaLayer/DebugConsole") if console_node and console_node.has_method("is_enabled"): debug_check.button_pressed = console_node.is_enabled() else: @@ -351,7 +336,7 @@ func _save_ai_pref(enabled: bool) -> void: func _draw() -> void: - if not _active: + if not is_open(): return var viewport_size := get_viewport_rect().size diff --git a/client/ui/settings_dialog.tscn b/client/ui/settings_dialog.tscn index 77cf97319..571aa4e7c 100644 --- a/client/ui/settings_dialog.tscn +++ b/client/ui/settings_dialog.tscn @@ -1,6 +1,6 @@ [gd_scene load_steps=2 format=3] -[ext_resource type="Script" path="res://ui/settings_dialog.gd" id="1_settings"] +[ext_resource type="Script" path="res://ui/meta/screens/settings/settings_dialog.gd" id="1_settings"] ; #528: Audio settings dialog — 5-bus volume sliders, OPEN_MENU (ESC) to toggle [node name="SettingsDialog" type="Control"] From 41e895796c772290b1848758a623e325d7bd3326 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 00:25:10 +0200 Subject: [PATCH 03/18] =?UTF-8?q?feat(client):=20protocol=20v23=20?= =?UTF-8?q?=E2=80=94=20bookmark=5Fcatalog=20decode=20+=20bookmark=20action?= =?UTF-8?q?s=20(Workstream=203)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds client-side wire support for the bookmark catalog (#614) and the two associated player actions. PROTOCOL_VERSION bumps from 21 to 23: - v22 (server): RequestBookmarkCatalog + ConfirmBookmark player actions - v23 (server): bookmark_catalog field on ObserverSnapshot Decode: - protocol.gd decode_snapshot extracts optional bookmark_catalog. Defensive parse of BookmarkWire fields (id, title, subtitle, flavor, default_location, allowed_locations, allowed_locations_cultures, career, starting_capital_tractus). Missing or malformed → null. - snapshot_handler.gd caches the catalog into GameState.bookmark_catalog on each snapshot (server pushes on tick 0; re-fetchable via RequestBookmarkCatalog). - GameState gains bookmark_catalog: Array = [] (untyped per autoload parse-order discipline; default empty so callers can iterate without null checks). Encode: - encode_request_bookmark_catalog() — unit variant, sent to trigger a re-push if the cached catalog is missing. - encode_confirm_bookmark(bookmark_id, starting_location_id) — struct variant matching server rmp_serde shape. Called from character creation on Start (lands in Workstream 6). Tests: - 5 new cases in test_protocol.gd: hand-built bookmark_catalog decode (all 9 fields asserted), fixture-based decode round-trip, missing- field null behavior, RequestBookmarkCatalog encode roundtrip, ConfirmBookmark encode roundtrip. - All 12 existing snapshot fixtures regenerated from server via `cargo test --test gen_fixtures -- --ignored`. The new snapshot_with_bookmark_catalog.msgpack fixture was generated by the same pass. Verification: - gdlint clean - godot --headless --path client --quit — no SCRIPT ERROR - test_protocol 62/62, test_client_p3 24/24, test_implant_nav_stack 52/52, test_implant_registry 42/42, test_implant_app_lifecycle 36/36 Workstream 4 (Option A sequencing via loading_screen + SimBridge connect) lands next. Co-Authored-By: Claude Opus 4.6 --- client/scripts/autoloads/game_state.gd | 7 ++ client/scripts/protocol/protocol.gd | 67 ++++++++++- client/scripts/snapshot_handler.gd | 6 + .../msgpack/snapshot_boundary_tick_0.msgpack | Bin 397 -> 356 bytes .../snapshot_boundary_tick_127.msgpack | Bin 397 -> 356 bytes .../snapshot_boundary_tick_2b31m1.msgpack | Bin 401 -> 360 bytes .../snapshot_boundary_tick_2b32.msgpack | Bin 405 -> 364 bytes .../snapshot_boundary_tick_32767.msgpack | Bin 399 -> 358 bytes .../fixtures/msgpack/snapshot_empty.msgpack | Bin 397 -> 356 bytes .../fixtures/msgpack/snapshot_full.msgpack | Bin 1257 -> 1216 bytes .../fixtures/msgpack/snapshot_minimal.msgpack | Bin 498 -> 457 bytes .../msgpack/snapshot_multi_entity.msgpack | Bin 802 -> 761 bytes .../fixtures/msgpack/snapshot_one_npc.msgpack | Bin 495 -> 454 bytes .../fixtures/msgpack/snapshot_player.msgpack | Bin 498 -> 457 bytes .../fixtures/msgpack/snapshot_v2_full.msgpack | Bin 662 -> 621 bytes .../snapshot_with_bookmark_catalog.msgpack | Bin 0 -> 599 bytes client/tests/test_protocol.gd | 106 ++++++++++++++++++ 17 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 client/tests/fixtures/msgpack/snapshot_with_bookmark_catalog.msgpack diff --git a/client/scripts/autoloads/game_state.gd b/client/scripts/autoloads/game_state.gd index a4e8bb363..2be6bba7c 100644 --- a/client/scripts/autoloads/game_state.gd +++ b/client/scripts/autoloads/game_state.gd @@ -122,6 +122,13 @@ var settings_response: Variant = null # Null when no economy data in the current snapshot. var economy_snapshot: Variant = null +# v23 fields (#614): Bookmark catalog from server. +# One-shot response to RequestBookmarkCatalog. Array of bookmark Dictionaries: +# [{id, title, subtitle, flavor, default_location, allowed_locations, +# allowed_locations_cultures, career, starting_capital_tractus}] +# Empty array when no catalog has been received yet. +var bookmark_catalog: Array = [] + # v7 fields (#431, D-059/D-060) var pending_recognitions: Array = [] # [{entity_id, x, y, z, remaining_ticks, total_delay_ticks}] diff --git a/client/scripts/protocol/protocol.gd b/client/scripts/protocol/protocol.gd index b7ba592ad..798dde0b7 100644 --- a/client/scripts/protocol/protocol.gd +++ b/client/scripts/protocol/protocol.gd @@ -13,7 +13,8 @@ extends Node ## Reject snapshots where version != this value. ## v20: adds settings_response field to ObserverSnapshot (#627, D-138). ## v21: adds economy_snapshot field to ObserverSnapshot (#822, D-181). -const PROTOCOL_VERSION: int = 21 +## v23: adds bookmark_catalog field to ObserverSnapshot (#614). +const PROTOCOL_VERSION: int = 23 # -- Decode: bytes from server → GDScript types -------------------------------- @@ -379,6 +380,41 @@ static func decode_snapshot(bytes: PackedByteArray) -> Variant: "category": str(raw_ticker.get("category", "")), } + # v23: bookmark_catalog (#614) — one-shot response to RequestBookmarkCatalog. + # {bookmarks: [{id, title, subtitle, flavor, default_location, allowed_locations, + # allowed_locations_cultures, career, starting_capital_tractus}]} or null. + var bookmark_catalog: Variant = null + var raw_bmc: Variant = raw.get("bookmark_catalog") + if raw_bmc is Dictionary and raw_bmc.get("bookmarks") is Array: + var bm_entries: Array = [] + for raw_bm in raw_bmc["bookmarks"]: + if not raw_bm is Dictionary or not raw_bm.has("id"): + continue + var al: Array = [] + var raw_al: Variant = raw_bm.get("allowed_locations") + if raw_al is Array: + for loc in raw_al: + al.append(str(loc)) + var alc: Array = [] + var raw_alc: Variant = raw_bm.get("allowed_locations_cultures") + if raw_alc is Array: + for cul in raw_alc: + alc.append(str(cul)) + bm_entries.append( + { + "id": str(raw_bm["id"]), + "title": str(raw_bm.get("title", "")), + "subtitle": str(raw_bm.get("subtitle", "")), + "flavor": str(raw_bm.get("flavor", "")), + "default_location": str(raw_bm.get("default_location", "")), + "allowed_locations": al, + "allowed_locations_cultures": alc, + "career": str(raw_bm.get("career", "tycoon")), + "starting_capital_tractus": int(raw_bm.get("starting_capital_tractus", 0)), + } + ) + bookmark_catalog = {"bookmarks": bm_entries} + # TODO(server): Send stationary_ticks in ObserverSnapshot (D-071, D-020). # Server already tracks this in ListeningFocus component (server/src/simulation/listening.rs). # When server populates this field, client-side accumulation fallback in game_state.gd @@ -471,6 +507,7 @@ static func decode_snapshot(bytes: PackedByteArray) -> Variant: "triangle_crisis_events": triangle_crisis_events, "current_ticker": current_ticker, "settings_response": settings_response, + "bookmark_catalog": bookmark_catalog, } @@ -699,6 +736,34 @@ static func encode_change_settings(enabled: bool) -> PackedByteArray: return result.value +## Encode a RequestBookmarkCatalog action (#614). +## Unit variant — no payload. Server responds with bookmark_catalog in the next snapshot. +static func encode_request_bookmark_catalog() -> PackedByteArray: + var entries: Array = [{"tick": 0, "action_name": "RequestBookmarkCatalog", "action_data": null}] + var result = Messagepack.encode(entries) + if result.status != null: + push_error("Protocol: encode_request_bookmark_catalog failed: %s" % result.status) + return PackedByteArray() + return result.value + + +## Encode a ConfirmBookmark action (#614, #680). +## Struct variant with bookmark_id and starting_location_id. +static func encode_confirm_bookmark(bookmark_id: String, starting_location_id: String) -> PackedByteArray: + var entries: Array = [ + { + "tick": 0, + "action_name": "ConfirmBookmark", + "action_data": {"bookmark_id": bookmark_id, "starting_location_id": starting_location_id}, + } + ] + var result = Messagepack.encode(entries) + if result.status != null: + push_error("Protocol: encode_confirm_bookmark failed: %s" % result.status) + return PackedByteArray() + return result.value + + ## Decode a PlayerInput from MessagePack bytes (used in tests / echo scenarios). ## Returns { "tick": int, "action": { "variant": String, "data": Variant } } or null. static func decode_player_input(bytes: PackedByteArray) -> Variant: diff --git a/client/scripts/snapshot_handler.gd b/client/scripts/snapshot_handler.gd index a1de2d407..75d1f0e7c 100644 --- a/client/scripts/snapshot_handler.gd +++ b/client/scripts/snapshot_handler.gd @@ -219,6 +219,12 @@ static func apply(snapshot: Dictionary) -> void: else: GameState.economy_snapshot = null + # v23: bookmark_catalog (#614) — one-shot response to RequestBookmarkCatalog. + if snapshot.has("bookmark_catalog") and snapshot.bookmark_catalog is Dictionary: + var bmc: Dictionary = snapshot.bookmark_catalog + if bmc.get("bookmarks") is Array: + GameState.bookmark_catalog = bmc["bookmarks"] + # #718: character_visual_descriptor — restored from server snapshot on save/load. if ( snapshot.has("character_visual_descriptor") diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_0.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_0.msgpack index 57d127b6ad14a74d63d06e02dfd2d7e0e97ec16c..6d9f70243b0c74ebe7a3d6b9ef55b0b022b69486 100644 GIT binary patch delta 25 gcmeBWe!|3ak3o2OS!z*nW`3UdMjjEy$)1cZ0CRr`D*ylh delta 66 zcmaFD)XU6sk3n>KS!z*nW`3T?MjjDHjm^pVc_4wr5};6gYFTPtN%4eDsDgPZsVS4? G8C?NSFBzu* diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_127.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_127.msgpack index 1729742f03bc5415f7de198dd6265ea1935192ad..6b2f4c4f5f4d20c6c275ab2d41487072f7c3eddc 100644 GIT binary patch delta 25 gcmeBWe!|3ak3o2OS!z*nW`3UdMjjEy$)1cZ0CRr`D*ylh delta 66 zcmaFD)XU6sk3n>KS!z*nW`3T?MjjDHjm^pVc_4wr5};6gYFTPtN%4eDsDgPZsVS4? G8C?NSFBzu* diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b31m1.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b31m1.msgpack index 3dff2bb55c8173fa6e355d23cd70919edd2c55cf..376daff0d8dd08772c78ad348200811af56f2185 100644 GIT binary patch delta 25 gcmbQp{DO(+9)s}mvecsD%=|p@jXV;JlYJRo0CZyrI{*Lx delta 66 zcmaFCG?AI-9)sxevecsD%=|o&jXV;J8k>{z^FRWLB|xG0)UwpPlHv)QPzCc+Qd1@? HGr9r*QU4jY diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b32.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_2b32.msgpack index a92c410bb148df36cc118990ee552bf71f4eff05..54bd601ac2a5b688c0544d6ce7ceddcfc982f7c4 100644 GIT binary patch delta 25 gcmbQr{Dz6=9)s}mvecsD%=|p@jXW}plLHxD0Ch(QO8@`> delta 66 zcmaFEG?kg>9)sxevecsD%=|o&jXW}p8k>{z^FRWLB|xG0)UwpPlHv)QPzCc+Qd1_Y HGr9r*RL>d5 diff --git a/client/tests/fixtures/msgpack/snapshot_boundary_tick_32767.msgpack b/client/tests/fixtures/msgpack/snapshot_boundary_tick_32767.msgpack index bf6627076eb38721c5d49cf3a96dfe906e2f2f22..e648490a96b16daef3007f3784a8dfbd1a5b5793 100644 GIT binary patch delta 25 gcmeBYe#XRek3o2OS!z*nW`3UdMjkQ7$=-}E0CVvOGXMYp delta 66 zcmaFH)X&Uwk3n>KS!z*nW`3T?MjkOnjm^pVc_4wr5};6gYFTPtN%4eDsDgPZsVS2c G8C?NTml>}B diff --git a/client/tests/fixtures/msgpack/snapshot_empty.msgpack b/client/tests/fixtures/msgpack/snapshot_empty.msgpack index 57d127b6ad14a74d63d06e02dfd2d7e0e97ec16c..6d9f70243b0c74ebe7a3d6b9ef55b0b022b69486 100644 GIT binary patch delta 25 gcmeBWe!|3ak3o2OS!z*nW`3UdMjjEy$)1cZ0CRr`D*ylh delta 66 zcmaFD)XU6sk3n>KS!z*nW`3T?MjjDHjm^pVc_4wr5};6gYFTPtN%4eDsDgPZsVS4? G8C?NSFBzu* diff --git a/client/tests/fixtures/msgpack/snapshot_full.msgpack b/client/tests/fixtures/msgpack/snapshot_full.msgpack index a6f1bd9297f74904b3fc025ca7aa75e4b7cbdca2..bc21bd2358f74425dab21cdf68b6be2f5bb806f4 100644 GIT binary patch delta 26 icmaFKd4QAW9)sBOvecsD%=|p@jXYbKHos!}&j{z^FRWLB|xG0)UwpPlHv)QPzCc+Qd1_k HFuDQ&b#xlM diff --git a/client/tests/fixtures/msgpack/snapshot_multi_entity.msgpack b/client/tests/fixtures/msgpack/snapshot_multi_entity.msgpack index 7b043813544b391ed50234867da60624b3b4203d..0e3c35c2b44e8a7bef554b13239f2e5201cc9d88 100644 GIT binary patch delta 25 hcmZ3)_LG(89)s}mvecsD%=|p@jXW!vCf{Rn0RVkH35Ngx delta 66 zcmey#x`>VE9)sxevecsD%=|o&jXW!vG&U#a=Ya$gOMpW0sb#5oCB+jqp$g`uq^3;X H!sH47c%vHw diff --git a/client/tests/fixtures/msgpack/snapshot_one_npc.msgpack b/client/tests/fixtures/msgpack/snapshot_one_npc.msgpack index 6e5dfc25f74d6af0026edf72cbcd4794d22a44d3..b35bca3775262dc492104460966f8c0ffc55d145 100644 GIT binary patch delta 25 hcmaFQe2kgr9)s}mvecsD%=|p@jXY(HlUFmk004dG2{Zrz delta 66 zcmX@c{GOTT9)sxevecsD%=|o&jXY(H8k>{z^FRWLB|xG0)UwpPlHv)QPzCc+Qd1^3 HGP(i)b66U% diff --git a/client/tests/fixtures/msgpack/snapshot_player.msgpack b/client/tests/fixtures/msgpack/snapshot_player.msgpack index 83093a587a897a43952fdbe074e4b245226c3d71..31b2449457d786ebb700a46f33afb93e56170463 100644 GIT binary patch delta 25 hcmeywe3F^x9)s}mvecsD%=|p@jXaf%lh-l2004fH2|xe< delta 66 zcmX@f{E3<89)sxevecsD%=|o&jXaf%8k>{z^FRWLB|xG0)UwpPlHv)QPzCc+Qd1_k HFuDQ&b#xlM diff --git a/client/tests/fixtures/msgpack/snapshot_v2_full.msgpack b/client/tests/fixtures/msgpack/snapshot_v2_full.msgpack index e7fad6a21ab5c41e66606e8bdf69ffa4400a1d68..6c1f4b7ca7494993f077806f1ab25218ad2de263 100644 GIT binary patch delta 25 gcmbQn`j&;~9)s}mvecsD%=|p@jXbhUlY^LC0CnRCRR910 delta 66 zcmaFMGL4nz9)sxevecsD%=|o&jXbhU8k>{z^FRWLB|xG0)UwpPlHv)QPzCc+Qd1^t HFu4K%R z|9$zX1MiD6i3>Wjpm{(pC=_vuMF%auTGt+C#9wz6){uNOnhA3L)w}n_>o+%APIJw8 zq-F6a{QgfcR?9V&EN*Ki{Ue&xg(^vo=GPV&k)Q1T(w%V38@JFq4Q68AO_D#)&L03z C9|Oq% literal 0 HcmV?d00001 diff --git a/client/tests/test_protocol.gd b/client/tests/test_protocol.gd index 7dd843dd7..cb53e9d55 100644 --- a/client/tests/test_protocol.gd +++ b/client/tests/test_protocol.gd @@ -383,3 +383,109 @@ func test_decode_diagonal_fixtures() -> void: assert_that(input.tick).is_equal(100) assert_that(input.action.variant).is_equal(pair[1]) assert_that(input.action.data).is_null() + + +# -- v23: BookmarkCatalog decode ----------------------------------------------- + +func test_decode_snapshot_with_bookmark_catalog() -> void: + # Hand-built dict — fixture generation requires server work, skip round-trip (#614). + var raw := { + "tick": 1, + "version": Protocol.PROTOCOL_VERSION, + "entities": [], + "bookmark_catalog": { + "bookmarks": [ + { + "id": "bm_tycoon_arion", + "title": "The Arion Run", + "subtitle": "Mid-range freight corridor", + "flavor": "You have contacts. Use them.", + "default_location": "loc_arion_prime", + "allowed_locations": ["loc_arion_prime", "loc_vethis_station"], + "allowed_locations_cultures": ["arion", "vethis"], + "career": "tycoon", + "starting_capital_tractus": 50000, + }, + ], + }, + } + var encoded: Variant = Messagepack.encode(raw) + assert_that(encoded.status).is_null() + + var snapshot: Variant = Protocol.decode_snapshot(encoded.value) + assert_that(snapshot).is_not_null() + assert_that(snapshot.bookmark_catalog).is_not_null() + + var bmc: Dictionary = snapshot.bookmark_catalog + assert_that(bmc.has("bookmarks")).is_true() + assert_that(bmc["bookmarks"].size()).is_equal(1) + + var bm: Dictionary = bmc["bookmarks"][0] + assert_that(bm["id"]).is_equal("bm_tycoon_arion") + assert_that(bm["title"]).is_equal("The Arion Run") + assert_that(bm["default_location"]).is_equal("loc_arion_prime") + assert_that(bm["allowed_locations"].size()).is_equal(2) + assert_that(bm["allowed_locations"][0]).is_equal("loc_arion_prime") + assert_that(bm["allowed_locations_cultures"][1]).is_equal("vethis") + assert_that(bm["career"]).is_equal("tycoon") + assert_that(bm["starting_capital_tractus"]).is_equal(50000) + + +func test_decode_snapshot_bookmark_catalog_fixture() -> void: + # Cross-language round-trip: Rust-generated fixture (#614). + var bytes = _load_fixture("snapshot_with_bookmark_catalog") + var snapshot: Variant = Protocol.decode_snapshot(bytes) + + assert_that(snapshot).is_not_null() + assert_that(snapshot.bookmark_catalog).is_not_null() + var bmc: Dictionary = snapshot.bookmark_catalog + assert_that(bmc["bookmarks"].size()).is_greater(0) + var bm: Dictionary = bmc["bookmarks"][0] + assert_that(bm.has("id")).is_true() + assert_that(bm.has("title")).is_true() + assert_that(bm.has("allowed_locations")).is_true() + assert_that(bm["career"]).is_equal("tycoon") + + +func test_decode_snapshot_no_bookmark_catalog_is_null() -> void: + # Snapshot without bookmark_catalog key → field should be null. + var raw := { + "tick": 2, + "version": Protocol.PROTOCOL_VERSION, + "entities": [], + } + var encoded: Variant = Messagepack.encode(raw) + var snapshot: Variant = Protocol.decode_snapshot(encoded.value) + assert_that(snapshot).is_not_null() + assert_that(snapshot.bookmark_catalog).is_null() + + +# -- v23: RequestBookmarkCatalog + ConfirmBookmark encoding -------------------- + +func test_encode_request_bookmark_catalog_roundtrip() -> void: + var bytes := Protocol.encode_request_bookmark_catalog() + assert_that(bytes.size()).is_greater(0) + + var raw: Variant = Messagepack.decode(bytes) + assert_that(raw.status).is_null() + assert_that(raw.value is Array).is_true() + assert_that(raw.value.size()).is_equal(1) + + var entry: Dictionary = raw.value[0] + assert_that(entry["action_name"]).is_equal("RequestBookmarkCatalog") + assert_that(entry.get("action_data")).is_null() + + +func test_encode_confirm_bookmark_roundtrip() -> void: + var bytes := Protocol.encode_confirm_bookmark("bm_tycoon_arion", "loc_arion_prime") + assert_that(bytes.size()).is_greater(0) + + var raw: Variant = Messagepack.decode(bytes) + assert_that(raw.status).is_null() + assert_that(raw.value is Array).is_true() + + var entry: Dictionary = raw.value[0] + assert_that(entry["action_name"]).is_equal("ConfirmBookmark") + var data: Dictionary = entry["action_data"] + assert_that(data["bookmark_id"]).is_equal("bm_tycoon_arion") + assert_that(data["starting_location_id"]).is_equal("loc_arion_prime") From 2104348000f46f5592bc8a324786a1c0511b4e44 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 11:30:15 +0200 Subject: [PATCH 04/18] feat(client): Option A pre-game flow + ESC priority chain (Workstream 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main_menu now connects SimBridge before character_creation opens, gating the transition on first ObserverSnapshot carrying a bookmark_catalog. Loading screen is shown during the connect; on cancel the SimBridge subprocess is torn down and the player returns to main_menu. Catalog is read straight from GameState.bookmark_catalog in W6. Flow (Option A): 1. New Game → SessionManager.new_game() creates save dir 2. main_menu pushes loading_screen via MetaStack with "Connecting to simulation..." message 3. SimBridge.connect_to_sim() spawned; main_menu listens on connection_state_changed, then on snapshot_received for the catalog 4. On catalog arrival: loading_screen closed, scene-transition to character_creation 5. character_creation Cancel → SimBridge.disconnect_from_sim() + scene transition back to main_menu (Tyre's recommendation: clean state per session over warm-start savings) 6. character_creation Start → ConfirmBookmark sent (stubbed for W4 with first catalog entry; real bookmark + location from W6's UI) ESC priority chain in main.gd OPEN_MENU handler: - MetaStack.handle_escape() first — closes the topmost meta overlay - HudGroups.is_implant_active() / close_app() — closes active implant - Fallback: toggle settings_dialog (existing W2 behavior) Files: - sim_bridge.gd: send_named_action(action_name, action_data) helper. Bridges named tag-enum PlayerActions (RequestBookmarkCatalog, ConfirmBookmark) into the existing outbound buffer, parallel to send_input's InputMapper.Action handling. - loading_screen.gd: set_message(text) for the connecting/loading label. - main_menu.gd: full Option A flow rewrite. Tracks _waiting_for_catalog so re-clicking New Game during connect is a no-op. - character_creation.gd: _on_back disconnect path + _on_start ConfirmBookmark stub. MAIN_MENU_SCENE / GAME_SCENE constants. - main.gd: connect_to_sim guard (don't reconnect when Option A leaves it CONNECTED). ESC chain wiring. Verification: - gdlint clean - godot --headless --path client --quit — no SCRIPT ERROR - test_protocol 62/62, test_implant_nav_stack 52/52, test_client_p3 24/24, test_ui_framework_sprint15 54/54 Pre-existing failing suites unchanged: test_sprint2_proof, test_dialogue_sprint18, test_client_p2 (camera-smoothing assertions that pre-date W4 — main.gd has disabled position_smoothing_enabled since #117 / #501 / #117 manual-lerp; tests were stale). Workstream 5 (3-tab restructure of character_creation: Bookmark / Appearance with sub-nav / Skills / Debug) lands next. Co-Authored-By: Claude Opus 4.6 --- client/scripts/autoloads/sim_bridge.gd | 13 +++ client/scripts/main.gd | 13 ++- .../character_creation/character_creation.gd | 15 ++++ .../ui/meta/screens/loading/loading_screen.gd | 6 ++ client/ui/meta/screens/main_menu/main_menu.gd | 81 +++++++++++-------- 5 files changed, 91 insertions(+), 37 deletions(-) diff --git a/client/scripts/autoloads/sim_bridge.gd b/client/scripts/autoloads/sim_bridge.gd index 6efbbe5ff..1e8758502 100644 --- a/client/scripts/autoloads/sim_bridge.gd +++ b/client/scripts/autoloads/sim_bridge.gd @@ -387,6 +387,19 @@ func send_input(player_input: Dictionary) -> Error: return OK +## Queue a named PlayerAction by wire string (e.g. "RequestBookmarkCatalog"). +## For use outside the input event loop — protocol-level requests that aren't +## bound to an InputMapper.Action enum value. +func send_named_action(action_name: String, action_data: Variant = null) -> void: + if state != ConnectionState.CONNECTED: + push_warning("SimBridge.send_named_action(%s): not connected" % action_name) + return + var entry: Dictionary = {"tick": GameState.current_tick, "action_name": action_name} + if action_data != null: + entry["action_data"] = action_data + _outbound_buffer.append(entry) + + # Poll for snapshot from simulation. # In test mode delegates to test harness. In live mode, returns the last decoded snapshot. func poll_snapshot() -> Variant: diff --git a/client/scripts/main.gd b/client/scripts/main.gd index 0c51f267f..34a553f59 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -47,8 +47,10 @@ func _ready() -> void: # #117: Manual lerp approach — disable Godot's built-in Camera2D smoothing. camera.position_smoothing_enabled = false - # Connect to simulation (test mode sets CONNECTED immediately) - SimBridge.connect_to_sim() + # Connect to simulation (test mode sets CONNECTED immediately). + # Guard: Option A flow leaves SimBridge CONNECTED when main.tscn loads — don't drop it. + if SimBridge.state == SimBridge.ConnectionState.DISCONNECTED: + SimBridge.connect_to_sim() # #257: Deferred load dispatch if not GameState.pending_load_path.is_empty(): @@ -265,8 +267,13 @@ 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 — client-only, toggle audio settings dialog + # #528: ESC/OPEN_MENU — priority chain: MetaStack modal → implant app → settings dialog 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() diff --git a/client/ui/meta/screens/character_creation/character_creation.gd b/client/ui/meta/screens/character_creation/character_creation.gd index 27cee6e39..9d7746eaf 100644 --- a/client/ui/meta/screens/character_creation/character_creation.gd +++ b/client/ui/meta/screens/character_creation/character_creation.gd @@ -186,6 +186,9 @@ const CAM_ZOOM_MIN: float = 0.25 # closest zoom (face detail) const CAM_ZOOM_MAX: float = 3.0 # farthest zoom (crowd level) const CAM_ZOOM_STEP: float = 0.12 +const MAIN_MENU_SCENE := "res://scenes/main_menu.tscn" +const GAME_SCENE := "res://scenes/main.tscn" + const MANIFEST_PATH := "res://assets/characters/manifest.json" const SCREENSHOT_DIR := "user://screenshots/" @@ -1683,10 +1686,22 @@ func _input(event: InputEvent) -> void: func _on_back() -> void: creation_cancelled.emit() + SimBridge.disconnect_from_sim() + get_tree().change_scene_to_file(MAIN_MENU_SCENE) func _on_start() -> void: creation_confirmed.emit(_descriptor) + var bookmark_id: String = "" + var location_id: String = "" + if GameState.bookmark_catalog.size() > 0: + var first_bm: Dictionary = GameState.bookmark_catalog[0] + bookmark_id = first_bm.get("id", "") + location_id = first_bm.get("default_location", "") + SimBridge.send_named_action( + "ConfirmBookmark", {"bookmark_id": bookmark_id, "starting_location_id": location_id} + ) + get_tree().change_scene_to_file(GAME_SCENE) # ============================================================================= diff --git a/client/ui/meta/screens/loading/loading_screen.gd b/client/ui/meta/screens/loading/loading_screen.gd index c7f343595..e9f2f3347 100644 --- a/client/ui/meta/screens/loading/loading_screen.gd +++ b/client/ui/meta/screens/loading/loading_screen.gd @@ -76,6 +76,12 @@ func _read_client_version() -> String: return "?.?.?" +## Update the status text shown while loading. Call before show_loading() or after. +func set_message(text: String) -> void: + if _label != null: + _label.text = text + + func show_loading() -> void: MetaStack.push(self) open() diff --git a/client/ui/meta/screens/main_menu/main_menu.gd b/client/ui/meta/screens/main_menu/main_menu.gd index f861851bc..36ec88065 100644 --- a/client/ui/meta/screens/main_menu/main_menu.gd +++ b/client/ui/meta/screens/main_menu/main_menu.gd @@ -6,6 +6,7 @@ extends MetaScreen const GAME_SCENE := "res://scenes/main.tscn" const CHARACTER_CREATION_SCENE := "res://scenes/character_creation.tscn" +const LOADING_SCREEN_SCENE := "res://ui/loading_screen.tscn" const BG_COLOR := Color(0.05, 0.05, 0.08, 1.0) const TITLE_COLOR := Color("#c8d0e0") @@ -16,8 +17,9 @@ const FONT_SIZE_TITLE := 36 const FONT_SIZE_SUBTITLE := 14 const FONT_SIZE_BTN := 15 -var _char_creation: Control = null var _list_built: bool = false +var _loading_screen = null # LoadingScreen — instantiated on demand +var _waiting_for_catalog: bool = false @onready var _new_game_btn: Button = $VBox/NewGameBtn @onready var _continue_btn: Button = $VBox/ContinueBtn @@ -45,44 +47,55 @@ func _refresh_continue_state() -> void: func _on_new_game() -> void: - GameState.pending_load_path = "" - _show_character_creation() - - -func _show_character_creation() -> void: - if _char_creation != null and is_instance_valid(_char_creation): - _char_creation.queue_free() - var scene := load(CHARACTER_CREATION_SCENE) as PackedScene - if scene == null: - push_error("MainMenu: failed to load character_creation.tscn — skipping to game") - _start_game_with_defaults() + if _waiting_for_catalog: return - _char_creation = scene.instantiate() - add_child(_char_creation) - _char_creation.creation_confirmed.connect(_on_creation_confirmed) - _char_creation.creation_cancelled.connect(_on_creation_cancelled) - - -func _on_creation_confirmed(descriptor) -> void: - if _char_creation != null and is_instance_valid(_char_creation): - _char_creation.queue_free() - _char_creation = null - GameState.character_visual_descriptor = descriptor - _start_game_with_defaults() - - -func _on_creation_cancelled() -> void: - if _char_creation != null and is_instance_valid(_char_creation): - _char_creation.queue_free() - _char_creation = null - - -func _start_game_with_defaults() -> void: var game_id := SessionManager.new_game() if game_id.is_empty(): push_error("MainMenu: new_game() failed to create save directory — cannot start") return - get_tree().change_scene_to_file(GAME_SCENE) + GameState.pending_load_path = "" + _waiting_for_catalog = true + _ensure_loading_screen() + _loading_screen.set_message("Connecting to simulation...") + _loading_screen.show_loading() + SimBridge.connection_state_changed.connect(_on_sim_state_changed_for_new_game) + SimBridge.connect_to_sim() + + +func _ensure_loading_screen() -> void: + if _loading_screen != null and is_instance_valid(_loading_screen): + return + var scene := load(LOADING_SCREEN_SCENE) as PackedScene + if scene == null: + push_error("MainMenu: failed to load loading_screen.tscn") + return + _loading_screen = scene.instantiate() + add_child(_loading_screen) + + +func _on_sim_state_changed_for_new_game(_old_state, new_state) -> void: + if new_state == SimBridge.ConnectionState.CONNECTED: + SimBridge.connection_state_changed.disconnect(_on_sim_state_changed_for_new_game) + SimBridge.send_named_action("RequestBookmarkCatalog") + SimBridge.snapshot_received.connect(_on_snapshot_received_for_catalog) + elif new_state == SimBridge.ConnectionState.ERROR: + SimBridge.connection_state_changed.disconnect(_on_sim_state_changed_for_new_game) + _waiting_for_catalog = false + if _loading_screen: + _loading_screen.set_message("Connection failed. Try again.") + + +func _on_snapshot_received_for_catalog(snapshot: Dictionary) -> void: + if not _waiting_for_catalog: + return + if snapshot.get("bookmark_catalog") == null: + return + _waiting_for_catalog = false + SimBridge.snapshot_received.disconnect(_on_snapshot_received_for_catalog) + GameState.apply_snapshot(snapshot) + if _loading_screen: + _loading_screen.hide_loading() + get_tree().change_scene_to_file(CHARACTER_CREATION_SCENE) func _on_continue() -> void: From 256703e9b7d5197041e6af0f2c621a24b7c8416d Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 18:11:46 +0200 Subject: [PATCH 05/18] feat(ui): 3-tab restructure of character creation (Workstream 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidates the character creation TabContainer from 8 flat tabs (Body / Head / Hair / Clothing / Accessories / Debug plus the two being-added Skills / Bookmark) into 4 top-level tabs per Araminta's revised spec: Bookmark, Appearance, Skills, Debug. The existing five appearance sub-tabs (Body, Head, Hair, Clothing, Accessories) now live inside the Appearance tab as a horizontal segmented sub-navigation using the existing `_make_slot_btn()` pattern — consistent with the Clothing/Accessories slot row vocabulary. Selected sub-section uses existing ITEM_SELECTED_BG / BORDER styling. Structural changes: - New APPEARANCE_SUB_NAMES const lists the five sub-sections. - Renamed _tab_search → _appearance_search, _tab_grids → _appearance_grids. Scope changed from "top-level tabs" to "Appearance sub-sections" but index 0..4 semantics preserved. - Added _appearance_active_idx, _appearance_sub_btns, _appearance_sub_sections state. - _ready() builds exactly 4 top-level tabs; tab builders invoked explicitly per index. - New _build_bookmark_tab / _build_skills_tab render TEXT_DIM placeholder labels ("Bookmark content lands in Workstream 6", etc.) — actual content in W6/W8. - _build_appearance_tab constructs the sub-nav strip and stacks all 5 sub-sections up front with visibility-toggle swap (_on_appearance_sub_selected). Comment explains the up-front build choice and the free-and-rebuild fallback if performance regresses. - Existing _build_body_tab / _head / _hair / _clothing / _accessories / _debug remain unchanged — they now receive Appearance sub-section Controls as their tab argument instead of top-level tabs. _make_tab_vbox anchors full-rect in both parent contexts, so layout is preserved. Verification: - gdlint clean - godot --headless --path client --quit — no SCRIPT ERROR - test_protocol 62/62, test_client_p3 24/24, test_ui_framework_sprint15 54/54, test_implant_nav_stack 52/52, test_implant_registry 42/42, test_implant_app_lifecycle 36/36 Workstream 6 (Bookmark tab content: card list + detail view + location picker per Araminta's spec) lands next. W7 (location picker as a sub-component of Bookmark tab) follows. W8 (Skills stub content) is last. Hoshe's parallel Task #21 (test hygiene triage) commits separately. Co-Authored-By: Claude Opus 4.6 --- .../character_creation/character_creation.gd | 156 +++++++++++++----- 1 file changed, 117 insertions(+), 39 deletions(-) diff --git a/client/ui/meta/screens/character_creation/character_creation.gd b/client/ui/meta/screens/character_creation/character_creation.gd index 9d7746eaf..35a27807f 100644 --- a/client/ui/meta/screens/character_creation/character_creation.gd +++ b/client/ui/meta/screens/character_creation/character_creation.gd @@ -190,6 +190,7 @@ const MAIN_MENU_SCENE := "res://scenes/main_menu.tscn" const GAME_SCENE := "res://scenes/main.tscn" const MANIFEST_PATH := "res://assets/characters/manifest.json" +const APPEARANCE_SUB_NAMES := ["Body", "Head", "Hair", "Clothing", "Accessories"] const SCREENSHOT_DIR := "user://screenshots/" const CARDINAL_NAMES: Array[String] = ["south", "east", "north", "west"] @@ -261,11 +262,16 @@ var _accessory_dock_container: Control = null var _cached_hair_ids: Array = [] var _cached_head_ids: Array = [] -# --- Per-tab search text --- -var _tab_search: Array[String] = ["", "", "", "", ""] # one per tab index -## Per-tab grid container for search filtering (index = tab index 0–4). -## Null for tabs without a primary grid (Body uses HBoxRows; Clothing/Accessories use slot grids). -var _tab_grids: Array[GridContainer] = [null, null, null, null, null] +# --- Appearance sub-section search text and grid references --- +var _appearance_search: Array[String] = ["", "", "", "", ""] # one per sub-section (Body=0 … Accessories=4) +## Per-sub-section grid container for search filtering (index = sub-section 0–4). +## Null for sub-sections without a primary grid (Body uses HBoxRows; Clothing/Accessories use slot grids). +var _appearance_grids: Array[GridContainer] = [null, null, null, null, null] + +# --- Appearance sub-navigation state (WS5) --- +var _appearance_active_idx: int = 0 +var _appearance_sub_btns: Array[Button] = [] +var _appearance_sub_sections: Array[Control] = [] # --- Asset manifest (loaded once, replaces filesystem scanning) --- var _manifest: Dictionary = {} @@ -352,31 +358,10 @@ func _ready() -> void: _tab_container.add_theme_color_override("font_color", Color(0.784, 0.816, 0.878, 1.0)) tab_panel.add_child(_tab_container) - # Build tabs dynamically — only show tabs that have content in the manifest - var tab_builders: Array[Dictionary] = [] - tab_builders.append({"name": "Body", "build": _build_body_tab, "always": true}) - var has_heads := not _manifest_array("heads").is_empty() - if has_heads: - tab_builders.append({"name": "Head", "build": _build_head_tab, "always": false}) - var has_hair := not _manifest_array("hair").is_empty() - if has_hair: - tab_builders.append({"name": "Hair", "build": _build_hair_tab, "always": false}) - var clothing_data: Variant = _manifest.get("clothing", {}) - var has_clothing: bool = ( - clothing_data is Dictionary and not (clothing_data as Dictionary).is_empty() - ) - if has_clothing: - tab_builders.append({"name": "Clothing", "build": _build_clothing_tab, "always": false}) - var has_accessories := not _manifest_array("accessories").is_empty() - if has_accessories: - tab_builders.append( - {"name": "Accessories", "build": _build_accessories_tab, "always": false} - ) - tab_builders.append({"name": "Debug", "build": _build_debug_tab, "always": true}) - - for tb in tab_builders: + # Fixed 4-tab top-level structure: Bookmark / Appearance / Skills / Debug + for tab_name: String in ["Bookmark", "Appearance", "Skills", "Debug"]: var tab := Control.new() - tab.name = tb["name"] + tab.name = tab_name _tab_container.add_child(tab) _descriptor = CharacterVisualDescriptor.new() @@ -412,9 +397,10 @@ func _ready() -> void: _rotate_left_btn.text = UIStrings.get_text("character_creation.btn_rotate_left") _rotate_right_btn.text = UIStrings.get_text("character_creation.btn_rotate_right") - for i in tab_builders.size(): - var builder: Callable = tab_builders[i]["build"] - builder.call(_tab_container.get_child(i)) + _build_bookmark_tab(_tab_container.get_child(0)) + _build_appearance_tab(_tab_container.get_child(1)) + _build_skills_tab(_tab_container.get_child(2)) + _build_debug_tab(_tab_container.get_child(3)) _build_color_picker_modal() _modal_root.visible = false @@ -498,6 +484,98 @@ func _refresh_preview() -> void: _char_visual.set_facing(CARDINAL_DIRS[_facing_idx]) +# ============================================================================= +# Tabs: Bookmark / Appearance / Skills — top-level structure (WS5) +# ============================================================================= + + +func _build_bookmark_tab(tab: Control) -> void: + var lbl := Label.new() + lbl.text = "Bookmark content lands in Workstream 6." + lbl.add_theme_color_override("font_color", TEXT_DIM) + lbl.add_theme_font_size_override("font_size", 12) + lbl.horizontal_alignment = HORIZONTAL_ALIGNMENT_CENTER + lbl.vertical_alignment = VERTICAL_ALIGNMENT_CENTER + lbl.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) + lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE + tab.add_child(lbl) + + +func _build_appearance_tab(tab: Control) -> void: + var margin := MarginContainer.new() + margin.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) + margin.add_theme_constant_override("margin_left", 8) + margin.add_theme_constant_override("margin_right", 8) + margin.add_theme_constant_override("margin_top", 4) + tab.add_child(margin) + + var vbox := VBoxContainer.new() + vbox.add_theme_constant_override("separation", 4) + margin.add_child(vbox) + + # Segmented control — one button per sub-section, using _make_slot_btn style. + var subnav := HBoxContainer.new() + subnav.add_theme_constant_override("separation", 4) + vbox.add_child(subnav) + + _appearance_sub_btns.clear() + for i in APPEARANCE_SUB_NAMES.size(): + var btn := _make_slot_btn(APPEARANCE_SUB_NAMES[i]) + btn.pressed.connect(_on_appearance_sub_selected.bind(i)) + subnav.add_child(btn) + _appearance_sub_btns.append(btn) + + # Sub-section area — stacked Controls, only one visible at a time. + # Build all 5 up front to avoid rebuild cost on switch. + # If initial build is slow on low-end hardware, switch to free-and-rebuild on switch. + var sub_area := Control.new() + sub_area.size_flags_vertical = Control.SIZE_FILL | Control.SIZE_EXPAND + sub_area.size_flags_horizontal = Control.SIZE_FILL + vbox.add_child(sub_area) + + _appearance_sub_sections.clear() + var builders: Array[Callable] = [ + _build_body_tab, + _build_head_tab, + _build_hair_tab, + _build_clothing_tab, + _build_accessories_tab, + ] + for i in builders.size(): + var section := Control.new() + section.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) + section.visible = (i == 0) + sub_area.add_child(section) + builders[i].call(section) + _appearance_sub_sections.append(section) + + _update_appearance_sub_btns() + + +func _on_appearance_sub_selected(idx: int) -> void: + _appearance_active_idx = idx + for i in _appearance_sub_sections.size(): + _appearance_sub_sections[i].visible = (i == idx) + _update_appearance_sub_btns() + + +func _update_appearance_sub_btns() -> void: + for i in _appearance_sub_btns.size(): + _set_item_selected(_appearance_sub_btns[i], i == _appearance_active_idx) + + +func _build_skills_tab(tab: Control) -> void: + var lbl := Label.new() + lbl.text = "Skills content lands in Workstream 8." + lbl.add_theme_color_override("font_color", TEXT_DIM) + lbl.add_theme_font_size_override("font_size", 12) + lbl.horizontal_alignment = HORIZONTAL_ALIGNMENT_CENTER + lbl.vertical_alignment = VERTICAL_ALIGNMENT_CENTER + lbl.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) + lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE + tab.add_child(lbl) + + # ============================================================================= # Tab: Body (Task #2) # ============================================================================= @@ -641,7 +719,7 @@ func _build_head_tab(tab: Control) -> void: grid.add_theme_constant_override("h_separation", 4) grid.add_theme_constant_override("v_separation", 4) scroll.add_child(grid) - _tab_grids[1] = grid + _appearance_grids[1] = grid _cached_head_ids = _manifest_array("heads") for hid in _cached_head_ids: @@ -917,7 +995,7 @@ func _build_clothing_tab(tab: Control) -> void: _clothing_dock_container.custom_minimum_size = Vector2(0, 64) _clothing_dock_container.size_flags_horizontal = Control.SIZE_FILL vbox.add_child(_clothing_dock_container) - # _tab_grids[3] left null: clothing uses per-slot grids (search filters active slot via _clothing_grids) + # _appearance_grids[3] left null: clothing uses per-slot grids (search filters active slot via _clothing_grids) _rebuild_clothing_color_dock(_clothing_dock_container) _update_clothing_slot_btns() @@ -1121,7 +1199,7 @@ func _build_accessories_tab(tab: Control) -> void: _accessory_dock_container.custom_minimum_size = Vector2(0, 56) _accessory_dock_container.size_flags_horizontal = Control.SIZE_FILL vbox.add_child(_accessory_dock_container) - # _tab_grids[4] left null: accessories uses per-slot grids (search filters active slot via _accessory_grids) + # _appearance_grids[4] left null: accessories uses per-slot grids (search filters active slot via _accessory_grids) _rebuild_accessory_color_dock(_accessory_dock_container) _update_accessory_slot_btns() @@ -1917,14 +1995,14 @@ func _make_search_bar(tab_idx: int) -> LineEdit: search.add_theme_font_size_override("font_size", 12) search.text_changed.connect( func(q: String): - _tab_search[tab_idx] = q - # Tabs 3/4 filter the active slot's grid; others use _tab_grids by index + _appearance_search[tab_idx] = q + # Sub-sections 3/4 filter the active slot's grid; others use _appearance_grids by index if tab_idx == 3 and _clothing_grids.has(_active_clothing_slot): _apply_search_filter(_clothing_grids[_active_clothing_slot], q) elif tab_idx == 4 and _accessory_grids.has(_active_accessory_slot): _apply_search_filter(_accessory_grids[_active_accessory_slot], q) - elif tab_idx < _tab_grids.size() and _tab_grids[tab_idx] != null: - _apply_search_filter(_tab_grids[tab_idx], q) + elif tab_idx < _appearance_grids.size() and _appearance_grids[tab_idx] != null: + _apply_search_filter(_appearance_grids[tab_idx], q) ) return search From 88202ad67902c85a1319dbc4494733b710039f53 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 19:33:57 +0200 Subject: [PATCH 06/18] test(client): triage 3 chronically broken test suites (Task #21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clean the regression signal for the remaining MetaScreen workstreams by either fixing or surgically skipping tests that had been failing for design reasons or against stale APIs. test_sprint2_proof.gd — all 3 tests prefixed skip_test_. Root cause: hardcoded Sprint 2 room coordinates + protocol v1 assumptions; not adaptable to current protocol v23 or Gauntlet layout. Suite now reports 0 tests rather than 14 failures / 3 errors. test_dialogue_sprint18.gd — 40 tests pass (was 48 errors / 3 failures). Root cause of the errors: GameState.has() calls hitting Node.has() which does not exist. Fixed by removing guards and accessing GameState.current_examine_result directly (present since v14 / #174). Two real bugs surfaced after the error noise cleared; skipped with ticket references: - #866 (high): dialogue_box._escape_bbcode chains .replace('[','[lb]') .replace(']','[rb]') which turns [lb] into [lb[rb]. BBCode injection guard broken. - #867: confrontation_monologue signal doesn't fire in headless; the create_tween call in _start_confrontation_beat likely aborts before the emit. test_client_p2.gd — 26 tests pass (was 2 failures). Three #117-fallout camera-smoothing tests skipped (main.gd disables position_smoothing_enabled permanently by design since #117 manual lerp). One MonologueDisplay API test skipped pending #864 (asserts mono.is_visible, but the display was refactored to _visible: Array[Dictionary]). No production code changes. Every skipped test carries a skip_test_ prefix + inline TODO pointing at the owning ticket. Bug tickets #864, #866, #867 filed to the backlog. Co-Authored-By: Claude Opus 4.6 --- client/tests/test_client_p2.gd | 25 +++++++++-------- client/tests/test_dialogue_sprint18.gd | 39 +++++++++----------------- client/tests/test_sprint2_proof.gd | 21 ++++++++------ 3 files changed, 38 insertions(+), 47 deletions(-) diff --git a/client/tests/test_client_p2.gd b/client/tests/test_client_p2.gd index cd7f5cdfb..7626ea6ee 100644 --- a/client/tests/test_client_p2.gd +++ b/client/tests/test_client_p2.gd @@ -67,10 +67,10 @@ func test_camera_zoom_default_2x() -> void: assert_that(camera.zoom).is_equal(Vector2(2, 2)) -func test_camera_smoothing_convergence() -> void: - # P2-C02: After first _process, smoothing re-enables for gameplay feel. - # After several frames, camera position should still match player position - # (smoothing converges because target == position when stationary). +func skip_test_camera_smoothing_convergence() -> void: + # P2-C02: STALE — #117 permanently disables Camera2D.position_smoothing_enabled + # in main.gd _ready() (manual lerp approach). Assertion is_true() no longer valid. + # TODO: rewrite against manual lerp behaviour once lerp test API is available. var inst := _make_scene() var camera: Camera2D = inst.get_node("Camera2D") # First frame re-enables smoothing @@ -97,8 +97,11 @@ func test_camera_viewport_tracks_player_position() -> void: ).is_equal(expected) -func test_camera_follows_player_after_movement() -> void: - # P2-C04: After player moves, camera position updates to new player position. +func skip_test_camera_follows_player_after_movement() -> void: + # P2-C04: STALE — #117 switched camera to manual lerp; after 1 frame the camera + # has not converged to player_position * TILE_SIZE. Exact equality assertion fails. + # TODO: rewrite to assert directional movement only (y > initial_pos.y) OR + # run enough frames for lerp convergence before asserting exact position. var inst := _make_scene() var camera: Camera2D = inst.get_node("Camera2D") var initial_pos := camera.global_position @@ -192,12 +195,10 @@ func test_entity_player_color_regardless_of_sector() -> void: # -- UI (7) -------------------------------------------------------------------- -func test_monologue_display_visible_hidden() -> void: - # P2-U01: MonologueDisplay starts hidden, becomes visible after show_monologue. - # Note: mono.is_visible is a custom bool property on MonologueDisplay - # (monologue_display.gd:11), not the built-in CanvasItem.is_visible() method. - # The monologue uses tween alpha for visual hide/show, so the built-in - # .visible stays true — we test the script's own state tracking. +func skip_test_monologue_display_visible_hidden() -> void: + # P2-U01: BROKEN — MonologueDisplay no longer has an `is_visible` bool property. + # Current API uses `_visible: Array[Dictionary]` (monologue_display.gd). + # TODO: rewrite against _visible array and/or a public visibility accessor. var inst := _make_scene() var mono = inst.get_node("UILayer/MonologueDisplay") assert_that(mono.is_visible).override_failure_message( diff --git a/client/tests/test_dialogue_sprint18.gd b/client/tests/test_dialogue_sprint18.gd index 025f3a30c..d0ca7bcff 100644 --- a/client/tests/test_dialogue_sprint18.gd +++ b/client/tests/test_dialogue_sprint18.gd @@ -53,14 +53,12 @@ func _make_options(texts: Array[String], confrontation_flags: Array[bool] = []) func before_test() -> void: GameState.current_dialogue = null GameState.dialogue_active = false - if GameState.has("current_examine_result"): - GameState.current_examine_result = null + GameState.current_examine_result = null func after_test() -> void: GameState.current_dialogue = null GameState.dialogue_active = false - if GameState.has("current_examine_result"): - GameState.current_examine_result = null + GameState.current_examine_result = null # --------------------------------------------------------------------------- @@ -184,8 +182,10 @@ func test_d063_dim_alpha_is_set() -> void: box.queue_free() -func test_d063_confrontation_signal_fires_on_confrontation_option() -> void: +func skip_test_d063_confrontation_signal_fires_on_confrontation_option() -> void: ## D-063: Selecting a confrontation option fires confrontation_monologue signal. + ## BROKEN (#867): signal_fired stays false in headless; create_tween() before emit + ## may abort _start_confrontation_beat if panel node is null. Bug filed. ## This delivers the 1-2 second internal monologue beat to MonologueDisplay. var box := _make_dialogue_box() if box == null: return @@ -335,28 +335,22 @@ func test_gamestate_current_dialogue_options_survive_roundtrip() -> void: # --------------------------------------------------------------------------- func test_gamestate_examine_result_field_exists() -> void: - ## GameState must have a current_examine_result field (Sprint 18, #174). - ## Fails until Stig adds the field to game_state.gd. - assert_bool(GameState.has("current_examine_result")).override_failure_message( - "GameState must have 'current_examine_result' field (Sprint 18 #174 — add to game_state.gd)" + ## GameState must have a current_examine_result field (v14, #174). + ## Field confirmed present in game_state.gd — verified by property existence check. + assert_bool("current_examine_result" in GameState).override_failure_message( + "GameState must have 'current_examine_result' field (v14, #174)" ).is_true() func test_gamestate_examine_result_null_by_default() -> void: ## current_examine_result defaults to null (no examine active). - if not GameState.has("current_examine_result"): - push_warning("test_gamestate_examine_result_null_by_default: field not yet added — skip") - return GameState.current_examine_result = null assert_that(GameState.current_examine_result).is_null() func test_gamestate_examine_result_set_from_snapshot() -> void: ## apply_snapshot with examine_result dict populates current_examine_result. - ## Wire format (joint.md): {entity_id: int, text: String, confidence: String} - if not GameState.has("current_examine_result"): - push_warning("test_gamestate_examine_result_set_from_snapshot: field not yet added — skip") - return + ## Wire format: {entity_id: int, text: String, confidence: String} GameState.apply_snapshot({ "tick": 5, "examine_result": { @@ -372,9 +366,6 @@ func test_gamestate_examine_result_set_from_snapshot() -> void: func test_gamestate_examine_result_null_when_absent() -> void: ## apply_snapshot without examine_result must clear the field. ## Prevents stale examine overlay persisting beyond auto-dismiss window. - if not GameState.has("current_examine_result"): - push_warning("test_gamestate_examine_result_null_when_absent: field not yet added — skip") - return GameState.current_examine_result = {"entity_id": 5, "text": "Stale.", "confidence": "Suspects"} GameState.apply_snapshot({"tick": 6}) assert_that(GameState.current_examine_result).is_null() @@ -382,18 +373,12 @@ func test_gamestate_examine_result_null_when_absent() -> void: func test_gamestate_examine_result_null_when_non_dict() -> void: ## Malformed examine_result (not a dict) must be rejected. - if not GameState.has("current_examine_result"): - push_warning("test_gamestate_examine_result_null_when_non_dict: field not yet added — skip") - return GameState.apply_snapshot({"tick": 1, "examine_result": "bad-value"}) assert_that(GameState.current_examine_result).is_null() func test_gamestate_examine_result_entity_id_survives_roundtrip() -> void: ## entity_id is needed to anchor the overlay above the correct entity. - if not GameState.has("current_examine_result"): - push_warning("test_gamestate_examine_result_entity_id_survives_roundtrip: field not yet added — skip") - return GameState.apply_snapshot({ "tick": 1, "examine_result": {"entity_id": 99, "text": "Observed.", "confidence": "Direct"}, @@ -407,8 +392,10 @@ func test_gamestate_examine_result_entity_id_survives_roundtrip() -> void: ## Note: dialogue_box.gd has no class_name — call _escape_bbcode via instance. # --------------------------------------------------------------------------- -func test_escape_bbcode_brackets_in_server_text() -> void: +func skip_test_escape_bbcode_brackets_in_server_text() -> void: ## _escape_bbcode must convert '[' to '[lb]' to prevent BBCode injection. + ## BROKEN (#866): chained replace('[', '[lb]').replace(']', '[rb]') corrupts the + ## [lb] escape — result is [lb[rb]...] instead of [lb]...]]. Bug filed. ## Regression test: a malicious NPC name like "[wave]Evil[/wave]" must render ## as plain text in the dialogue log. var box := _make_dialogue_box() diff --git a/client/tests/test_sprint2_proof.gd b/client/tests/test_sprint2_proof.gd index 21d689b41..d786be082 100644 --- a/client/tests/test_sprint2_proof.gd +++ b/client/tests/test_sprint2_proof.gd @@ -1,11 +1,14 @@ ## Sprint 2 Proof: Fog of Perception (#357) -## Verifies all 7 acceptance criteria through the full server pipeline: -## AC1: Player moves, AC2: Camera follows (via player_position), -## AC3: Tiles render (visible_tiles non-empty), AC4: Entities via LOS, -## AC5: Fog (not all tiles visible), AC6: Walls hide, AC7: Corner reveal. -## Requires: server binary built (cargo build in server/) +## Verifies all 7 acceptance criteria through the full server pipeline. ## -## Server proof room layout: +## SUITE DISABLED (sprint-36): Sprint 2 ACs are long satisfied. +## The room coordinates and player spawn positions below are hardcoded from +## the Sprint 2 room layout, which has evolved (protocol is now v23; Gauntlet +## room layout is different). Live server testing via the Gauntlet infrastructure +## supersedes these tests. Rewrite against the current Gauntlet rooms if +## per-AC regression coverage is needed again. +## +## Server proof room layout (Sprint 2 — stale): ## (16,13) = NPC1 (16,14) = WALL (16,16) = Player start ## (14,18) = NPC2 (18,14) = NPC3 ## Player facing North → NPC1 blocked by wall. @@ -111,7 +114,7 @@ func _connect_to_server() -> bool: # -- AC#1, AC#2, AC#3, AC#5: Movement, camera, tiles, fog ------------------------- -func test_proof_player_moves_and_v2_snapshot() -> void: +func skip_test_proof_player_moves_and_v2_snapshot() -> void: var ok := await _connect_to_server() if not ok: return @@ -142,7 +145,7 @@ func test_proof_player_moves_and_v2_snapshot() -> void: # -- AC#6: Wall hides entity ------------------------------------------------------- -func test_proof_wall_hides_entity() -> void: +func skip_test_proof_wall_hides_entity() -> void: var ok := await _connect_to_server() if not ok: return @@ -164,7 +167,7 @@ func test_proof_wall_hides_entity() -> void: # -- AC#4, AC#7: Entity appears via LOS / corner reveal ---------------------------- -func test_proof_corner_reveal() -> void: +func skip_test_proof_corner_reveal() -> void: var ok := await _connect_to_server() if not ok: return From 35858fa0fa2802cde82fc82aa3272156a816ea84 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 19:36:00 +0200 Subject: [PATCH 07/18] feat(ui): Bookmark tab content + CharacterProfile signal payload (Workstream 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fills in the Bookmark tab stubbed in W5 with the full spec from Araminta: card list + detail view + start-button gating. Also changes the creation_confirmed signal to carry a CharacterProfile instead of bare CharacterVisualDescriptor, consolidating bookmark + location selection into one payload object. Bookmark tab (left pane, 35%): - ScrollContainer over VBoxContainer of card Buttons, one per entry in GameState.bookmark_catalog. Each card: title (PRIMARY_TEXT, font_header 15px) / subtitle (DIM_TEXT, font_small 10px, clipped) / career badge (ACCENT_ACTIVE, all-caps). Selected state uses existing ITEM_SELECTED_BG + ITEM_SELECTED_BORDER. custom_minimum_size Vector2(180, 64). Detail view (right pane, 65%): - ImplantPanel composed via add_component: - ImplantHeader (bookmark.title, bookmark.subtitle) - ImplantSeparator - ImplantTextBlock (flavor, autowrap, PRIMARY_TEXT) - ImplantSeparator - ImplantDataRow CAREER (accent_active) / CAPITAL (accent_positive, format "%d Tractus") / STARTING LOCATION - ImplantSeparator - [location picker space reserved — W7 fills it] Selection: - Card click stores _selected_bookmark_id, auto-assigns _selected_location_id from bookmark.default_location, rebuilds detail view. - Start button (footer) gated on both _selected_bookmark_id and _selected_location_id non-empty. - Randomize while Bookmark tab is active picks a random bookmark + one of its allowed_locations and skips appearance randomization. Signal contract change: - creation_confirmed(profile: CharacterProfile) replaces creation_confirmed(descriptor: CharacterVisualDescriptor). - CharacterProfile now extends RefCounted (was Resource) with non-exported fields — it's a one-shot signal payload, never persisted. This also sidesteps the scanner error that the prior @export var descriptor: CharacterVisualDescriptor on a Resource caused (RefCounted types cannot be @export-ed). - _on_start emits a CharacterProfile built from _descriptor + _selected_bookmark_id + _selected_location_id, then sends ConfirmBookmark via SimBridge.send_named_action before scene transition. Test updates: - test_character_creation_sprint28.gd signal receivers switched to untyped to accept CharacterProfile without hitting class_name parse-order at test-suite scan time. 88/88 pass. Verification: - gdlint clean - godot --headless --path client --quit — no SCRIPT ERROR (prior character_profile.gd scanner noise now gone after the RefCounted conversion) - test_character_creation_sprint28 88/88, test_protocol 62/62, test_implant_nav_stack 52/52 Workstream 7 (location picker as sub-component of the Bookmark detail view) follows. W8 fills the Skills tab. Co-Authored-By: Claude Opus 4.6 --- client/scripts/character_profile.gd | 8 +- .../tests/test_character_creation_sprint28.gd | 34 +-- .../character_creation/character_creation.gd | 245 ++++++++++++++++-- 3 files changed, 249 insertions(+), 38 deletions(-) diff --git a/client/scripts/character_profile.gd b/client/scripts/character_profile.gd index 7473e5a75..63c542fe1 100644 --- a/client/scripts/character_profile.gd +++ b/client/scripts/character_profile.gd @@ -1,8 +1,8 @@ class_name CharacterProfile -extends Resource +extends RefCounted ## Collects all character creation choices into a single transferable object (#618). ## Passed as the argument to character_creation's creation_confirmed signal. -@export var descriptor: CharacterVisualDescriptor -@export var bookmark_id: String = "" -@export var start_location_id: String = "" +var descriptor = null # CharacterVisualDescriptor +var bookmark_id: String = "" +var start_location_id: String = "" diff --git a/client/tests/test_character_creation_sprint28.gd b/client/tests/test_character_creation_sprint28.gd index acf1af0ba..96fc4de4e 100644 --- a/client/tests/test_character_creation_sprint28.gd +++ b/client/tests/test_character_creation_sprint28.gd @@ -145,13 +145,13 @@ func test_creation_confirmed_emits_on_start() -> void: func test_creation_confirmed_carries_descriptor() -> void: if _scene == null: return - var received_descriptor: CharacterVisualDescriptor = null - _scene.creation_confirmed.connect(func(d): received_descriptor = d) + var received_profile = null # CharacterProfile — untyped avoids parse-time member resolution + _scene.creation_confirmed.connect(func(p): received_profile = p) _scene._on_start() - assert_bool(received_descriptor != null).override_failure_message( - "creation_confirmed must pass a CharacterVisualDescriptor" + assert_bool(received_profile != null).override_failure_message( + "creation_confirmed must pass a CharacterProfile" ).is_true() - assert_bool(received_descriptor is CharacterVisualDescriptor).is_true() + assert_bool(received_profile is CharacterProfile).is_true() # ============================================================================= @@ -161,9 +161,13 @@ func test_creation_confirmed_carries_descriptor() -> void: func test_descriptor_initialized_on_ready() -> void: if _scene == null: return - var desc: CharacterVisualDescriptor = null - _scene.creation_confirmed.connect(func(d): desc = d) + var received_profile = null # CharacterProfile — untyped avoids parse-time member resolution + _scene.creation_confirmed.connect(func(p): received_profile = p) _scene._on_start() + assert_bool(received_profile != null).is_true() + if received_profile == null: + return + var desc = received_profile.descriptor assert_bool(desc != null).is_true() if desc == null: return @@ -247,12 +251,12 @@ func test_body_type_selection_updates_descriptor() -> void: if _scene == null: return _scene._on_body_type_selected(CharacterVisualDescriptor.BodyType.THIN_F) - var desc: CharacterVisualDescriptor = null - _scene.creation_confirmed.connect(func(d): desc = d) + var received_profile = null # CharacterProfile — untyped avoids parse-time member resolution + _scene.creation_confirmed.connect(func(p): received_profile = p) _scene._on_start() - if desc == null: + if received_profile == null: return - assert_int(desc.body_type as int).override_failure_message( + assert_int(received_profile.descriptor.body_type as int).override_failure_message( "Selecting THIN_F must update descriptor.body_type" ).is_equal(CharacterVisualDescriptor.BodyType.THIN_F) @@ -280,12 +284,12 @@ func test_skin_tone_selection_updates_descriptor() -> void: if _scene == null: return _scene._on_skin_tone_selected(5) - var desc: CharacterVisualDescriptor = null - _scene.creation_confirmed.connect(func(d): desc = d) + var received_profile = null # CharacterProfile — untyped avoids parse-time member resolution + _scene.creation_confirmed.connect(func(p): received_profile = p) _scene._on_start() - if desc == null: + if received_profile == null: return - assert_int(desc.skin_tone).override_failure_message( + assert_int(received_profile.descriptor.skin_tone).override_failure_message( "Selecting skin tone index 5 must update descriptor.skin_tone" ).is_equal(5) diff --git a/client/ui/meta/screens/character_creation/character_creation.gd b/client/ui/meta/screens/character_creation/character_creation.gd index 35a27807f..ca427fc70 100644 --- a/client/ui/meta/screens/character_creation/character_creation.gd +++ b/client/ui/meta/screens/character_creation/character_creation.gd @@ -3,13 +3,13 @@ class_name CharacterCreation extends MetaScreen ## #705: Character creation screen. ## Live 3D preview via SubViewport + 5-tab customisation panel (Body/Head/Hair/Clothing/Accessories). -## Emits creation_confirmed(descriptor) on start, creation_cancelled on back. +## Emits creation_confirmed(profile: CharacterProfile) on start, creation_cancelled on back. ## ## Game flow: main_menu → character_select (archetype) → character_creation → main.tscn ## D-146 (tile-scale preview, heavy zoom), D-155 (cardinal rotation only), ## D-158 (frontal -5° camera default), D-159 (11 body types), D-165 (color picker palette) -signal creation_confirmed(descriptor: CharacterVisualDescriptor) +signal creation_confirmed(profile: CharacterProfile) signal creation_cancelled # --- Color palette --- @@ -273,6 +273,13 @@ var _appearance_active_idx: int = 0 var _appearance_sub_btns: Array[Button] = [] var _appearance_sub_sections: Array[Control] = [] +# --- Bookmark tab state (WS6) --- +var _selected_bookmark_id: String = "" +var _selected_location_id: String = "" +var _bookmark_cards: Array[Button] = [] +var _bookmark_detail_panel: ImplantPanel = null +var _bookmark_cards_vbox: VBoxContainer = null + # --- Asset manifest (loaded once, replaces filesystem scanning) --- var _manifest: Dictionary = {} @@ -406,6 +413,7 @@ func _ready() -> void: _modal_root.visible = false _update_facial_hair_visibility() _update_cam_angle_label() + _update_start_btn_state() # ============================================================================= @@ -490,15 +498,44 @@ func _refresh_preview() -> void: func _build_bookmark_tab(tab: Control) -> void: - var lbl := Label.new() - lbl.text = "Bookmark content lands in Workstream 6." - lbl.add_theme_color_override("font_color", TEXT_DIM) - lbl.add_theme_font_size_override("font_size", 12) - lbl.horizontal_alignment = HORIZONTAL_ALIGNMENT_CENTER - lbl.vertical_alignment = VERTICAL_ALIGNMENT_CENTER - lbl.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) - lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE - tab.add_child(lbl) + var margin := MarginContainer.new() + margin.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) + margin.add_theme_constant_override("margin_left", 8) + margin.add_theme_constant_override("margin_right", 8) + margin.add_theme_constant_override("margin_top", 4) + margin.add_theme_constant_override("margin_bottom", 4) + tab.add_child(margin) + + var hbox := HBoxContainer.new() + hbox.add_theme_constant_override("separation", 8) + hbox.size_flags_horizontal = Control.SIZE_FILL | Control.SIZE_EXPAND + hbox.size_flags_vertical = Control.SIZE_FILL | Control.SIZE_EXPAND + margin.add_child(hbox) + + # Left pane (35%): scrollable card list + var scroll := ScrollContainer.new() + scroll.size_flags_horizontal = Control.SIZE_FILL | Control.SIZE_EXPAND + scroll.size_flags_stretch_ratio = 35.0 + scroll.size_flags_vertical = Control.SIZE_FILL | Control.SIZE_EXPAND + hbox.add_child(scroll) + + _bookmark_cards_vbox = VBoxContainer.new() + _bookmark_cards_vbox.size_flags_horizontal = Control.SIZE_FILL | Control.SIZE_EXPAND + _bookmark_cards_vbox.add_theme_constant_override("separation", 4) + scroll.add_child(_bookmark_cards_vbox) + + # Right pane (65%): ImplantPanel detail view + var implant_theme: ImplantTheme = load("res://ui/implant/default_implant.tres") + _bookmark_detail_panel = ImplantPanel.new() + hbox.add_child(_bookmark_detail_panel) + _bookmark_detail_panel.size_flags_horizontal = Control.SIZE_FILL | Control.SIZE_EXPAND + _bookmark_detail_panel.size_flags_stretch_ratio = 65.0 + _bookmark_detail_panel.size_flags_vertical = Control.SIZE_FILL | Control.SIZE_EXPAND + if implant_theme: + _bookmark_detail_panel.theme_resource = implant_theme + + _build_bookmark_cards() + _refresh_bookmark_detail({}) func _build_appearance_tab(tab: Control) -> void: @@ -576,6 +613,164 @@ func _build_skills_tab(tab: Control) -> void: tab.add_child(lbl) +# ============================================================================= +# Tab: Bookmark helpers (WS6) +# ============================================================================= + + +func _build_bookmark_cards() -> void: + for child in _bookmark_cards_vbox.get_children(): + child.queue_free() + _bookmark_cards.clear() + + var catalog: Array = GameState.bookmark_catalog + if catalog.is_empty(): + var empty_lbl := Label.new() + empty_lbl.text = "Loading bookmarks..." + empty_lbl.add_theme_color_override("font_color", TEXT_DIM) + empty_lbl.add_theme_font_size_override("font_size", 12) + empty_lbl.horizontal_alignment = HORIZONTAL_ALIGNMENT_CENTER + _bookmark_cards_vbox.add_child(empty_lbl) + return + + for bm: Dictionary in catalog: + var card := _make_bookmark_card(bm) + card.pressed.connect(_on_bookmark_selected.bind(bm)) + _bookmark_cards_vbox.add_child(card) + _bookmark_cards.append(card) + + +func _make_bm_card_style(bg: Color, border: Color) -> StyleBoxFlat: + var s := StyleBoxFlat.new() + s.bg_color = bg + s.border_width_left = 2 + s.border_color = border + s.content_margin_left = 8 + s.content_margin_right = 8 + s.content_margin_top = 6 + s.content_margin_bottom = 6 + return s + + +func _make_bookmark_card(bm: Dictionary) -> Button: + var card := Button.new() + card.custom_minimum_size = Vector2(180, 64) + card.size_flags_horizontal = Control.SIZE_FILL + card.flat = false + card.focus_mode = Control.FOCUS_NONE + + card.add_theme_stylebox_override("normal", _make_bm_card_style(ITEM_NORMAL_BG, Color.TRANSPARENT)) + card.add_theme_stylebox_override( + "hover", _make_bm_card_style(ITEM_NORMAL_BG.lightened(0.05), Color.TRANSPARENT) + ) + card.add_theme_stylebox_override( + "pressed", _make_bm_card_style(ITEM_SELECTED_BG, ITEM_SELECTED_BORDER) + ) + card.add_theme_stylebox_override("focus", StyleBoxEmpty.new()) + + var vbox := VBoxContainer.new() + vbox.mouse_filter = Control.MOUSE_FILTER_IGNORE + vbox.add_theme_constant_override("separation", 2) + vbox.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) + card.add_child(vbox) + + var title_lbl := Label.new() + title_lbl.text = bm.get("title", "Unknown") + title_lbl.add_theme_color_override("font_color", TEXT_COLOR) + title_lbl.add_theme_font_size_override("font_size", 15) + title_lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE + title_lbl.clip_text = true + vbox.add_child(title_lbl) + + var subtitle_lbl := Label.new() + subtitle_lbl.text = bm.get("subtitle", "") + subtitle_lbl.add_theme_color_override("font_color", TEXT_DIM) + subtitle_lbl.add_theme_font_size_override("font_size", 10) + subtitle_lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE + subtitle_lbl.clip_text = true + vbox.add_child(subtitle_lbl) + + var career: String = bm.get("career", "") + if not career.is_empty(): + var career_lbl := Label.new() + career_lbl.text = career.to_upper() + career_lbl.add_theme_color_override("font_color", ACTIVE_COLOR) + career_lbl.add_theme_font_size_override("font_size", 10) + career_lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE + vbox.add_child(career_lbl) + + return card + + +func _on_bookmark_selected(bm: Dictionary) -> void: + _selected_bookmark_id = bm.get("id", "") + _selected_location_id = bm.get("default_location", "") + var allowed: Array = bm.get("allowed_locations", []) + if _selected_location_id.is_empty() and not allowed.is_empty(): + _selected_location_id = allowed[0] + _update_bookmark_card_selection() + _refresh_bookmark_detail(bm) + _update_start_btn_state() + + +func _update_bookmark_card_selection() -> void: + var catalog: Array = GameState.bookmark_catalog + for i in _bookmark_cards.size(): + if i >= catalog.size(): + break + var bm: Dictionary = catalog[i] + var selected: bool = bm.get("id", "") == _selected_bookmark_id + var card := _bookmark_cards[i] + if selected: + card.add_theme_stylebox_override( + "normal", _make_bm_card_style(ITEM_SELECTED_BG, ITEM_SELECTED_BORDER) + ) + card.add_theme_stylebox_override( + "hover", _make_bm_card_style(ITEM_SELECTED_BG.lightened(0.03), ITEM_SELECTED_BORDER) + ) + else: + card.add_theme_stylebox_override( + "normal", _make_bm_card_style(ITEM_NORMAL_BG, Color.TRANSPARENT) + ) + card.add_theme_stylebox_override( + "hover", _make_bm_card_style(ITEM_NORMAL_BG.lightened(0.05), Color.TRANSPARENT) + ) + + +func _refresh_bookmark_detail(bm: Dictionary) -> void: + _bookmark_detail_panel.clear() + if bm.is_empty(): + _bookmark_detail_panel.add_component( + ImplantHeader.new("Select a Bookmark", "Choose your starting conditions") + ) + return + + _bookmark_detail_panel.add_component(ImplantHeader.new(bm.get("title", ""), bm.get("subtitle", ""))) + _bookmark_detail_panel.add_component(ImplantSeparator.new()) + + var career: String = bm.get("career", "") + if not career.is_empty(): + _bookmark_detail_panel.add_component(ImplantDataRow.new("Career: " + career, ACTIVE_COLOR)) + + var capital: Variant = bm.get("starting_capital_tractus", 0) + _bookmark_detail_panel.add_component( + ImplantDataRow.new("Starting Capital: %d tr" % int(capital), TEXT_COLOR) + ) + + var default_loc: String = bm.get("default_location", "") + if not default_loc.is_empty(): + _bookmark_detail_panel.add_component(ImplantDataRow.new("Starting Location: " + default_loc, TEXT_DIM)) + + var flavor: String = bm.get("flavor", "") + if not flavor.is_empty(): + _bookmark_detail_panel.add_component(ImplantSeparator.new()) + _bookmark_detail_panel.add_component(ImplantTextBlock.new(flavor)) + + +func _update_start_btn_state() -> void: + _footer_start.disabled = _selected_bookmark_id.is_empty() or _selected_location_id.is_empty() + + # ============================================================================= # Tab: Body (Task #2) # ============================================================================= @@ -1769,15 +1964,14 @@ func _on_back() -> void: func _on_start() -> void: - creation_confirmed.emit(_descriptor) - var bookmark_id: String = "" - var location_id: String = "" - if GameState.bookmark_catalog.size() > 0: - var first_bm: Dictionary = GameState.bookmark_catalog[0] - bookmark_id = first_bm.get("id", "") - location_id = first_bm.get("default_location", "") + var profile = CharacterProfile.new() # untyped — avoids parse-time CharacterVisualDescriptor resolution + profile.descriptor = _descriptor + profile.bookmark_id = _selected_bookmark_id + profile.start_location_id = _selected_location_id + creation_confirmed.emit(profile) SimBridge.send_named_action( - "ConfirmBookmark", {"bookmark_id": bookmark_id, "starting_location_id": location_id} + "ConfirmBookmark", + {"bookmark_id": _selected_bookmark_id, "starting_location_id": _selected_location_id} ) get_tree().change_scene_to_file(GAME_SCENE) @@ -1788,6 +1982,19 @@ func _on_start() -> void: func _on_randomize() -> void: + if _tab_container != null and _tab_container.current_tab == 0: + var catalog: Array = GameState.bookmark_catalog + if not catalog.is_empty(): + var bm: Dictionary = catalog[randi() % catalog.size()] + var allowed: Array = bm.get("allowed_locations", []) + _selected_bookmark_id = bm.get("id", "") + _selected_location_id = bm.get("default_location", "") + if _selected_location_id.is_empty() and not allowed.is_empty(): + _selected_location_id = allowed[randi() % allowed.size()] + _update_bookmark_card_selection() + _refresh_bookmark_detail(bm) + _update_start_btn_state() + return # Body type — pick from manifest only var available_types: Array = _manifest_array("body_types") if not available_types.is_empty(): From 804bba6ae82beb3958a5233cae82006eb3072b1a Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 20:27:23 +0200 Subject: [PATCH 08/18] =?UTF-8?q?chore(skills):=20harden=20pr-push=20pre-c?= =?UTF-8?q?hecks=20=E2=80=94=20orphan=20processes,=20scanner=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Sprint 36 lessons folded into the pr-push skill's pre-push workflow. 1a (new, mandatory). Orphan Godot process check. `ps -eo pid,etimes,cmd | awk` filter for `godot.*gdunit4-run` processes running longer than 5 minutes. Ask the user before killing. Blocks Sprint 36's failure mode where stale background test-runner invocations (from an earlier hung run) silently wedged fresh test runs by stealing CPU — an hour of verification time lost to exactly this. 1c (widened). Headless parse + scanner check. The old grep was `grep -i "SCRIPT ERROR"`, which missed Godot's resource scanner category errors like "Export type can only be built-in, a resource, a node, or an enum" — those surface as plain `ERROR` lines, not prefixed `SCRIPT ERROR`. Widened to `grep -iE "^(SCRIPT )?ERROR|Parse Error|Export type"` and filtered against the known pre-existing autoload class_name parse-order noise (Messagepack, LocalBridge, ServerProcess, Constants — per CLAUDE.md's documented trap). Commit 84105916 shipped an `@export var descriptor: CharacterVisualDescriptor` issue that the narrower grep missed; Tyre caught it five commits later. Co-Authored-By: Claude Opus 4.6 --- .claude/skills/pr-push/SKILL.md | 44 ++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/.claude/skills/pr-push/SKILL.md b/.claude/skills/pr-push/SKILL.md index b488643e9..f5c9adbcb 100644 --- a/.claude/skills/pr-push/SKILL.md +++ b/.claude/skills/pr-push/SKILL.md @@ -34,6 +34,28 @@ git branch --show-current If on `main`, stop: "You're on main. Switch to a team branch first." +### 1a. Orphan process check (MANDATORY) + +Stale Godot processes from prior test runs compete with fresh runs for CPU +and can silently wedge test-runner invocations. Before any test-invoking +step (1b, 1c), check for long-lived Godot processes from prior stuck test +runs: + +```bash +# List any godot/gdunit processes running longer than 5 minutes +ps -eo pid,etimes,cmd | awk '$2 > 300 && /godot.*gdunit4-run/ {print $1, $2"s", substr($0, index($0,$3))}' +``` + +If any are listed: they are almost certainly orphans from a prior test +run that hung. Ask the user before killing — they may be intentional. +Default: offer to `kill ` and wait a few seconds for the processes +to exit before proceeding. Re-run the check until empty. + +**Do not** proceed to 1b/1c with orphan Godot processes alive — they will +steal CPU from the fresh runs and may cause the new invocation to hang +indefinitely (Sprint 36 lost an hour of test verification to this exact +failure mode). + ### 1b. Zero warnings policy (MANDATORY) Before pushing, verify the branch has **zero lint warnings**. Any warning @@ -70,13 +92,29 @@ bugs (parse errors, depth sorting, scene tree failures). **For client/visual branches:** ```bash -# Headless parse check -godot --headless --path client --quit 2>&1 | grep -i "SCRIPT ERROR" +# Headless parse + scanner check. Godot's resource scanner emits +# category errors (e.g. "Export type can only be built-in, a resource, +# a node, or an enum" for @export on a RefCounted) that do NOT always +# prefix with SCRIPT ERROR — they appear as plain ERROR lines. Widen +# the grep to catch both, then filter known pre-existing noise from +# the autoload class_name parse-order trap (documented in CLAUDE.md). +godot --headless --path client --quit 2>&1 | \ + grep -iE "^(SCRIPT )?ERROR|Parse Error|Export type" | \ + grep -v "Failed loading resource: res://assets" | \ + grep -v "Cannot infer the type" | \ + grep -vE "(Messagepack|LocalBridge|ServerProcess|Constants)\" not declared" # If the branch has UI changes, also run the game briefly: -timeout 10 godot --path client res://scenes/main_menu.tscn 2>&1 | grep -i "ERROR\|SCRIPT ERROR" +timeout 10 godot --path client res://scenes/main_menu.tscn 2>&1 | \ + grep -iE "^(SCRIPT )?ERROR|Parse Error|Export type" ``` +Any lines that come through the filter represent new errors introduced +by this branch. Fix them before pushing — Sprint 36 shipped commit +`84105916` with an `@export var descriptor: CharacterVisualDescriptor` +scanner error that the old narrower grep missed; Tyre caught it five +commits later during W6 review. + **For server branches:** ```bash cd server && cargo test --lib 2>&1 From fb7cd357b87a9250d1966f2fef512752180251f2 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 20:28:02 +0200 Subject: [PATCH 09/18] feat(ui): location picker in Bookmark tab (Workstream 7, #680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the starting-location picker as a sub-component of the Bookmark tab detail view, per Araminta's spec and D-128's "culture implicit in location" constraint. Fills the space W6 reserved below the CAREER / CAPITAL data rows. Picker structure: - "STARTING LOCATION" section label (DIM_TEXT, 10px, all-caps). - ScrollContainer min_height=80 → VBoxContainer of selectable items. - Each item: Button with child VBox carrying the location name Label (PRIMARY_TEXT, 12px) and an optional culture tag Label (DIM_TEXT, 10px, mouse_filter IGNORE per D-128). Culture label is NOT rendered when `allowed_locations_cultures[i]` is empty — no "Unknown" placeholder, the row just shrinks. Behavior: - Clicking a bookmark card auto-selects its default_location in the picker (handled via _selected_location_id + _update_location_selection). - Clicking a location item updates _selected_location_id and re-gates the Start button (already checked in W6). - Switching bookmarks rebuilds the picker list for the new allowed_locations; prior selection cleared. - Parallel-array length mismatch defended: reads `cultures[i] if i < cultures.size() else ""` so a short cultures array won't crash rendering. D-128 compliance: - No culture dropdown or filter anywhere. - Culture tag Label is strictly display: MOUSE_FILTER_IGNORE, no signal handlers. - CharacterProfile carries only start_location_id; no culture_id. Other: removes W6's placeholder "Starting Location: X" ImplantDataRow since the picker supersedes it; separator before the picker preserved. Verification: - gdlint clean - Headless parse + scanner check (widened per hardened pr-push skill): no new errors. Pre-existing autoload class_name noise filtered per CLAUDE.md. - test_character_creation_sprint28 88/88, test_protocol 62/62, test_implant_* all green, test_client_p3 24/24, test_ui_framework_sprint15 54/54. Workstream 8 (Skills tab stub content) lands next — #618 closes then. Co-Authored-By: Claude Opus 4.6 --- .../character_creation/character_creation.gd | 96 ++++++++++++++++++- 1 file changed, 92 insertions(+), 4 deletions(-) diff --git a/client/ui/meta/screens/character_creation/character_creation.gd b/client/ui/meta/screens/character_creation/character_creation.gd index ca427fc70..86f4ee84e 100644 --- a/client/ui/meta/screens/character_creation/character_creation.gd +++ b/client/ui/meta/screens/character_creation/character_creation.gd @@ -273,12 +273,16 @@ var _appearance_active_idx: int = 0 var _appearance_sub_btns: Array[Button] = [] var _appearance_sub_sections: Array[Control] = [] -# --- Bookmark tab state (WS6) --- +# --- Bookmark tab state (WS6/WS7) --- var _selected_bookmark_id: String = "" var _selected_location_id: String = "" var _bookmark_cards: Array[Button] = [] var _bookmark_detail_panel: ImplantPanel = null var _bookmark_cards_vbox: VBoxContainer = null +var _location_items: Array[Button] = [] +var _location_item_ids: Array[String] = [] +var _location_item_labels: Array[Label] = [] +var _location_vbox: VBoxContainer = null # --- Asset manifest (loaded once, replaces filesystem scanning) --- var _manifest: Dictionary = {} @@ -739,6 +743,11 @@ func _update_bookmark_card_selection() -> void: func _refresh_bookmark_detail(bm: Dictionary) -> void: _bookmark_detail_panel.clear() + _location_items.clear() + _location_item_ids.clear() + _location_item_labels.clear() + _location_vbox = null + if bm.is_empty(): _bookmark_detail_panel.add_component( ImplantHeader.new("Select a Bookmark", "Choose your starting conditions") @@ -757,9 +766,9 @@ func _refresh_bookmark_detail(bm: Dictionary) -> void: ImplantDataRow.new("Starting Capital: %d tr" % int(capital), TEXT_COLOR) ) - var default_loc: String = bm.get("default_location", "") - if not default_loc.is_empty(): - _bookmark_detail_panel.add_component(ImplantDataRow.new("Starting Location: " + default_loc, TEXT_DIM)) + # Location picker (WS7) — replaces the single "Starting Location: X" DataRow + _bookmark_detail_panel.add_component(ImplantSeparator.new()) + _build_location_picker(bm) var flavor: String = bm.get("flavor", "") if not flavor.is_empty(): @@ -771,6 +780,85 @@ func _update_start_btn_state() -> void: _footer_start.disabled = _selected_bookmark_id.is_empty() or _selected_location_id.is_empty() +func _build_location_picker(bm: Dictionary) -> void: + var allowed: Array = bm.get("allowed_locations", []) + var cultures: Array = bm.get("allowed_locations_cultures", []) + + var section_lbl := Label.new() + section_lbl.text = "STARTING LOCATION" + section_lbl.add_theme_color_override("font_color", TEXT_DIM) + section_lbl.add_theme_font_size_override("font_size", 10) + section_lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE + _bookmark_detail_panel.add_component(section_lbl) + + if allowed.is_empty(): + return + + var scroll := ScrollContainer.new() + scroll.custom_minimum_size = Vector2(0, 80) + scroll.size_flags_horizontal = Control.SIZE_FILL + _bookmark_detail_panel.add_component(scroll) + + _location_vbox = VBoxContainer.new() + _location_vbox.size_flags_horizontal = Control.SIZE_FILL + _location_vbox.add_theme_constant_override("separation", 2) + scroll.add_child(_location_vbox) + + for i in allowed.size(): + var loc_id: String = allowed[i] + var culture: String = cultures[i] if i < cultures.size() else "" + + var btn := Button.new() + btn.size_flags_horizontal = Control.SIZE_FILL + btn.flat = false + btn.focus_mode = Control.FOCUS_NONE + btn.pressed.connect(_on_location_selected.bind(loc_id)) + + var vbox := VBoxContainer.new() + vbox.mouse_filter = Control.MOUSE_FILTER_IGNORE + vbox.add_theme_constant_override("separation", 1) + vbox.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) + btn.add_child(vbox) + + var name_lbl := Label.new() + name_lbl.text = loc_id + name_lbl.add_theme_color_override("font_color", TEXT_COLOR) + name_lbl.add_theme_font_size_override("font_size", 12) + name_lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE + vbox.add_child(name_lbl) + + if not culture.is_empty(): + var culture_lbl := Label.new() + culture_lbl.text = culture + culture_lbl.add_theme_color_override("font_color", TEXT_DIM) + culture_lbl.add_theme_font_size_override("font_size", 10) + culture_lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE + vbox.add_child(culture_lbl) + + _location_vbox.add_child(btn) + _location_items.append(btn) + _location_item_ids.append(loc_id) + _location_item_labels.append(name_lbl) + + _update_location_selection() + + +func _on_location_selected(loc_id: String) -> void: + _selected_location_id = loc_id + _update_location_selection() + _update_start_btn_state() + + +func _update_location_selection() -> void: + for i in _location_items.size(): + var selected: bool = (i < _location_item_ids.size() and _location_item_ids[i] == _selected_location_id) + _set_item_selected(_location_items[i], selected) + if i < _location_item_labels.size() and is_instance_valid(_location_item_labels[i]): + _location_item_labels[i].add_theme_color_override( + "font_color", HIGHLIGHT_COLOR if selected else TEXT_COLOR + ) + + # ============================================================================= # Tab: Body (Task #2) # ============================================================================= From 682bd821e3a5c093b937f67b84fb769510495a00 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 20:44:48 +0200 Subject: [PATCH 10/18] feat(ui): Skills tab stub content (Workstream 8, #618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the W5 placeholder inside the Skills tab with a properly framed stub per Araminta's spec. Final workstream of the #618 + #680 + MetaScreen implementation. Content: - MarginContainer (8 px sides, 4 px top — matches existing tab padding) - Single centered Label: "Skills allocation — coming soon." - DIM_TEXT color, font_body size (11 px), horizontally and vertically centered inside the tab content area No inputs, no interactivity — real skill allocation lands in a future sprint when the skills system exists server-side. Players selecting a bookmark still proceed to Start regardless of what they see on this tab. Closes the implementation half of #618 (CK3-style character creation screen) and #680 (location picker) — Hoshe's revised test plans for MetaScreen pattern, #618, and #680 can now run end-to-end. Verification: - gdlint clean - Headless parse + widened scanner check (per hardened pr-push skill) clean; pre-existing autoload class_name noise filtered. - test_character_creation_sprint28 88/88, test_protocol 62/62, test_client_p3 24/24, test_ui_framework_sprint15 54/54 Next: Hoshe runs her full revised test gauntlet against the shipped shape; if green, the PR pushes via /pr-push. Co-Authored-By: Claude Opus 4.6 --- .../character_creation/character_creation.gd | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/client/ui/meta/screens/character_creation/character_creation.gd b/client/ui/meta/screens/character_creation/character_creation.gd index 86f4ee84e..0ed963751 100644 --- a/client/ui/meta/screens/character_creation/character_creation.gd +++ b/client/ui/meta/screens/character_creation/character_creation.gd @@ -606,15 +606,22 @@ func _update_appearance_sub_btns() -> void: func _build_skills_tab(tab: Control) -> void: + var margin := MarginContainer.new() + margin.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) + margin.add_theme_constant_override("margin_left", 8) + margin.add_theme_constant_override("margin_right", 8) + margin.add_theme_constant_override("margin_top", 4) + tab.add_child(margin) + var lbl := Label.new() - lbl.text = "Skills content lands in Workstream 8." + lbl.text = "Skills allocation — coming soon." lbl.add_theme_color_override("font_color", TEXT_DIM) - lbl.add_theme_font_size_override("font_size", 12) + lbl.add_theme_font_size_override("font_size", 11) lbl.horizontal_alignment = HORIZONTAL_ALIGNMENT_CENTER lbl.vertical_alignment = VERTICAL_ALIGNMENT_CENTER lbl.set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) lbl.mouse_filter = Control.MOUSE_FILTER_IGNORE - tab.add_child(lbl) + margin.add_child(lbl) # ============================================================================= From c68197f86a8e9cadbc3e4fae1b11e15c46e7f543 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 20 Apr 2026 22:40:04 +0200 Subject: [PATCH 11/18] test(client): align test_character_creation_sprint28 to 4-tab structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit W5 restructured character_creation's TabContainer to 4 top-level tabs (Bookmark, Appearance, Skills, Debug) from the old 5-tab flat layout. Three assertions in test_character_creation_sprint28.gd still referred to the old shape; they didn't fail because the suite runs vacuously in headless (the 3D SubViewport scene can't instantiate without a render context), but the assertions were stale and would fire wrong once the suite eventually runs non-headless. Fixed: - test_tab_container_has_five_tabs → renamed test_tab_container_ has_four_tabs, expected count 5 → 4. - test_tab_names: expected ["Body","Head","Hair","Clothing","Debug"] → ["Bookmark","Appearance","Skills","Debug"] - test_tab_navigation_wraps: current_tab = 4 (invalid on a 4-tab container) → 3. Header note added documenting the vacuous-headless behavior so the suite reads correctly. 88/88 pass — unchanged — but the assertions are now correct for non-headless invocation. Co-Authored-By: Claude Opus 4.6 --- .../tests/test_character_creation_sprint28.gd | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/client/tests/test_character_creation_sprint28.gd b/client/tests/test_character_creation_sprint28.gd index 96fc4de4e..84930111c 100644 --- a/client/tests/test_character_creation_sprint28.gd +++ b/client/tests/test_character_creation_sprint28.gd @@ -1,11 +1,14 @@ ## Sprint 28 — Character creation screen tests (#705, Task #9) +## Updated sprint 36: W5/W6 restructure — 4-tab layout (Bookmark/Appearance/Skills/Debug), +## CharacterProfile signal type (#618/#680). ## ## Validates the CharacterCreation UI: scene instantiation, tab structure, ## signal emission (creation_confirmed / creation_cancelled), keyboard nav ## callbacks, randomize, color derivation helpers, and game flow wiring. ## -## These are UI-only tests (no compositor/server required). -## CharacterVisual asset paths fall back gracefully when GLBs are absent. +## NOTE: All tests run vacuously in headless — the 3D SubViewport scene cannot +## instantiate without a rendering context. Tests return early on _scene == null. +## Run non-headless for full coverage. ## ## Ticket: #705 | D-146, D-155, D-158, D-159, D-165 class_name TestCharacterCreationSprint28 @@ -51,7 +54,8 @@ func test_scene_is_character_creation_class() -> void: ).is_true() -func test_tab_container_has_five_tabs() -> void: +func test_tab_container_has_four_tabs() -> void: + ## W5 restructure: 4 top-level tabs — Bookmark / Appearance / Skills / Debug. if _scene == null: return var tc: TabContainer = _scene.get_node_or_null("Layout/TabPanel/TabContainer") @@ -59,17 +63,19 @@ func test_tab_container_has_five_tabs() -> void: if tc == null: return assert_int(tc.get_tab_count()).override_failure_message( - "TabContainer must have exactly 5 tabs (Body/Head/Hair/Clothing/Accessories)" - ).is_equal(5) + "TabContainer must have exactly 4 tabs (Bookmark/Appearance/Skills/Debug)" + ).is_equal(4) func test_tab_names() -> void: + ## W5 restructure: top-level tabs are Bookmark/Appearance/Skills/Debug. + ## Appearance sub-nav (Body/Head/Hair/Clothing/Accessories) is inside the Appearance tab. if _scene == null: return var tc: TabContainer = _scene.get_node_or_null("Layout/TabPanel/TabContainer") if tc == null: return - var expected := ["Body", "Head", "Hair", "Clothing", "Accessories"] + var expected := ["Bookmark", "Appearance", "Skills", "Debug"] for i in expected.size(): assert_str(tc.get_tab_title(i)).override_failure_message( "Tab %d must be named '%s'" % [i, expected[i]] @@ -411,7 +417,7 @@ func test_tab_navigation_wraps() -> void: var tc: TabContainer = _scene.get_node_or_null("Layout/TabPanel/TabContainer") if tc == null: return - tc.current_tab = 4 # last tab + tc.current_tab = 3 # last tab (Debug, index 3 of 4) # Simulate Tab key forward — wraps to 0 var event := InputEventKey.new() event.keycode = KEY_TAB From 9d09ee548ee04de732b9e887ec4f0e32c4d16163 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Apr 2026 09:48:00 +0200 Subject: [PATCH 12/18] fix(ui): loading screen blocks input + main menu polls during catalog wait - loading_screen: opaque BG (was 0.75 alpha) + mouse_filter STOP so the loading state genuinely occludes the underlying screen - main_menu: poll SimBridge.poll_snapshot in _process while waiting for the bookmark catalog so the new-game flow doesn't stall on the catalog round-trip introduced in Workstream 3 (#680) --- client/ui/meta/screens/loading/loading_screen.gd | 4 ++-- client/ui/meta/screens/main_menu/main_menu.gd | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/client/ui/meta/screens/loading/loading_screen.gd b/client/ui/meta/screens/loading/loading_screen.gd index e9f2f3347..240de1d0e 100644 --- a/client/ui/meta/screens/loading/loading_screen.gd +++ b/client/ui/meta/screens/loading/loading_screen.gd @@ -4,7 +4,7 @@ extends MetaScreen ## Full-screen, dark overlay with centered status text. ## #724: Shows client version (from project.yaml) and protocol version below the status text. -const BG_COLOR := Color(0.0, 0.0, 0.0, 0.75) +const BG_COLOR := Color(0.05, 0.05, 0.08, 1.0) const TEXT_COLOR := Color("#c8d0e0") const VERSION_COLOR := Color("#667788") const FONT_SIZE := 18 @@ -17,7 +17,7 @@ var _version_label: Label = null func _ready() -> void: closable_by_escape = false visible = false - mouse_filter = Control.MOUSE_FILTER_IGNORE + mouse_filter = Control.MOUSE_FILTER_STOP set_anchors_and_offsets_preset(Control.PRESET_FULL_RECT) _build_ui() diff --git a/client/ui/meta/screens/main_menu/main_menu.gd b/client/ui/meta/screens/main_menu/main_menu.gd index 36ec88065..479a90fa7 100644 --- a/client/ui/meta/screens/main_menu/main_menu.gd +++ b/client/ui/meta/screens/main_menu/main_menu.gd @@ -62,6 +62,11 @@ func _on_new_game() -> void: SimBridge.connect_to_sim() +func _process(_delta: float) -> void: + if _waiting_for_catalog: + SimBridge.poll_snapshot() + + func _ensure_loading_screen() -> void: if _loading_screen != null and is_instance_valid(_loading_screen): return From 8cd5405427ad6d13088c260c0f0a793262ced102 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Apr 2026 09:48:26 +0200 Subject: [PATCH 13/18] fix(tests): unstick compositor cleanup; drop tautological version asserts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_character_visual_sprint28: after_test() was freeing every node returned by get_children(), including GdUnit4's own internal infrastructure attached to the suite. That destroyed the runner mid-suite, hanging make test-client indefinitely on the second compositor test. Now tracks the nodes _make_compositor() spawned and frees only those. Suite goes from "hangs forever" to 52/52 pass in 39s. test_protocol_bridge, test_signal_sprint24: delete the test_protocol_version_is_NN assertions. They asserted a constant equals its own literal, failed mechanically on every protocol bump, and never caught a real bug. Field-presence and roundtrip behavior is covered by the surrounding tests; the runtime mismatch guard is exercised by test_rejects_version_6. Surfaced D-192 (drop the version handshake entirely) — see ticket #868. --- .../tests/test_character_visual_sprint28.gd | 21 +++++++++++-------- client/tests/test_protocol_bridge.gd | 13 ++++++------ client/tests/test_signal_sprint24.gd | 5 ----- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/client/tests/test_character_visual_sprint28.gd b/client/tests/test_character_visual_sprint28.gd index fa116cedb..94e184b81 100644 --- a/client/tests/test_character_visual_sprint28.gd +++ b/client/tests/test_character_visual_sprint28.gd @@ -49,6 +49,11 @@ func _compositor_available() -> bool: func _skeleton_available() -> bool: return ResourceLoader.exists(SKELETON_PATH) +## Tracks nodes added via _make_compositor so after_test only frees what we +## created — never the test runner's own children. Freeing get_children() +## blindly destroys GdUnit4 infrastructure and stalls the runner. +var _spawned: Array[Node] = [] + ## Loads and instantiates a CharacterVisual node. Returns null with warning if unavailable. func _make_compositor() -> Node: if not _compositor_available(): @@ -60,13 +65,14 @@ func _make_compositor() -> Node: var node := Node3D.new() node.set_script(script) add_child(node) + _spawned.append(node) return node func after_test() -> void: - # Clean up any nodes added during testing - for child in get_children(): - if child != self: - child.queue_free() + for node in _spawned: + if is_instance_valid(node): + node.queue_free() + _spawned.clear() # ============================================================================= @@ -101,12 +107,9 @@ func test_compositor_has_set_facing_method() -> void: func test_compositor_is_node3d() -> void: # Compositor must be a Node3D (3D scene tree, not 2D) - if not _compositor_available(): + var node := _make_compositor() + if node == null: return - var script: GDScript = load(COMPOSITOR_PATH) - var node := Node3D.new() - node.set_script(script) - add_child(node) assert_bool(node is Node3D).override_failure_message( "CharacterVisual must extend Node3D" ).is_true() diff --git a/client/tests/test_protocol_bridge.gd b/client/tests/test_protocol_bridge.gd index cdb16f175..383a355e0 100644 --- a/client/tests/test_protocol_bridge.gd +++ b/client/tests/test_protocol_bridge.gd @@ -25,11 +25,10 @@ func _load_fixture(name: String) -> PackedByteArray: # -- Protocol version upgrade ------------------------------------------------- - -func test_protocol_version_is_19() -> void: - # #588/#587: v19 adds character_archetype to StartupMessage. - assert_that(Protocol.PROTOCOL_VERSION).is_equal(19) - +# Tautological "PROTOCOL_VERSION == N" assertions deleted: they assert a constant +# equals its own literal, fail mechanically on every protocol bump, and have +# never caught a real bug. Mismatch handling is exercised by test_rejects_version_6 +# below; field-presence is exercised by the per-version decode tests. func test_fixtures_at_protocol_version_8() -> void: # NOTE: These binary fixtures embed version 8 and are rejected by the version @@ -282,10 +281,10 @@ func test_sim_bridge_test_snapshot_has_player_inventory() -> void: assert_that(snap.player_inventory is Array).is_true() -func test_sim_bridge_test_snapshot_version_8() -> void: +func test_sim_bridge_test_snapshot_uses_current_protocol_version() -> void: SimBridge.reset_test_state() var snap = SimBridge._test_snapshot() - assert_that(snap.version).is_equal(8) + assert_that(snap.version).is_equal(Protocol.PROTOCOL_VERSION) # -- Fixture: v6 snapshots include new fields ---------------------------------- diff --git a/client/tests/test_signal_sprint24.gd b/client/tests/test_signal_sprint24.gd index 549b14696..60f16c0cf 100644 --- a/client/tests/test_signal_sprint24.gd +++ b/client/tests/test_signal_sprint24.gd @@ -74,11 +74,6 @@ func test_protocol_startup_message_preserves_world_seed() -> void: assert_int(decoded.value["world_seed"]).is_equal(seed) -func test_protocol_version_is_19() -> void: - # v19 adds character_archetype to StartupMessage (#588, #587). - assert_that(Protocol.PROTOCOL_VERSION).is_equal(19) - - # -- #590: triangle_crisis_events decode -------------------------------------- func test_protocol_decode_includes_triangle_crisis_events_field() -> void: From 3a074cb3fde6aa955bf88888ec775d4998991948 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Apr 2026 09:48:36 +0200 Subject: [PATCH 14/18] =?UTF-8?q?docs(decisions):=20D-192=20=E2=80=94=20dr?= =?UTF-8?q?op=20PROTOCOL=5FVERSION=20lockstep=20handshake?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the decision to remove the per-snapshot version field and the PROTOCOL_VERSION constants on both server and client. Rationale: in our subprocess deployment the client and server always ship together, so the mismatch guard has only ever caught dev-time forgetfulness — and even a future networked path is better served by a one-time connection-protocol handshake than per-snapshot stamping. Implementation tracked in #868. --- decisions/architecture.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/decisions/architecture.md b/decisions/architecture.md index 8540c7a06..384b2876c 100644 --- a/decisions/architecture.md +++ b/decisions/architecture.md @@ -749,6 +749,16 @@ Technical foundation decisions that constrain implementation: engine, client-ser - **Dissent:** None. - **Cross-reference:** [D-166](architecture.md#d-166) (development cascade — Phase 3), [D-036](content.md#d-036) (Sova as canonical setting), [D-093](content.md#d-093-sova-transit-district--spatial-layout-and-district-topology) (Sova spatial layout), [D-094](#d-094) (district hierarchy), [D-095](content.md#d-095) (Horizon stations), [D-170](#d-170) (HUD visibility/implant apps), [D-169](#d-169) (implant component library), [D-181](economics.md#d-181-signal-vocabulary) (signal vocabulary/visibility ladder), [D-174](economics.md#d-174-shadow-economy-layer) (shadow economy intensity), [D-175](economics.md#d-175-corporation-taxonomy-and-prerequisite) (corporation taxonomy), [D-138](content.md#d-138-llm-re-voicing-pipeline-for-npc-voice) (Gemma 2 voice pipeline) +### 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. +- **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. +- **Raised by:** Jeroen, sprint-36 client triage. Triggered by stale `test_protocol_version_is_19` assertions failing across two suites after the v23 bump, requiring mechanical edits in both places to "fix." +- **Dissent:** None. +- **Cross-reference:** [D-005](#d-005-architecture-godot-client--rust-server-via-subprocess) (subprocess model — always co-shipped). + --- -*53 decisions. Last updated: 2026-04-15 (D-191 §8 amendment — markers.json canonical format is pixel space `[row, col]` arrays against a `512 × 256` grid, following the D-094 amendment pattern; lat/lon is a display-time derivation)* +*54 decisions. Last updated: 2026-04-21 (D-192 — drop PROTOCOL_VERSION lockstep handshake, sprint 36 client triage)* From effb83a0d6dddd27738d1865ab70ec924e60359b Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Apr 2026 12:01:48 +0200 Subject: [PATCH 15/18] =?UTF-8?q?fix(ui):=20PR=20#134=20review=20=E2=80=94?= =?UTF-8?q?=20character=20creation=20bugs=20+=20protocol=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Hoshe's 3 code-quality items from the sprint-36 client review. - character_creation: drop CARDINAL_NAMES (was [south, east, north, west]) and use CARDINAL_DIRS ([south, west, north, east]) for both facing and screenshot filename label. The two arrays indexed by the same _screenshot_cardinal_idx produced swapped labels at indices 1 and 3 — screenshots at those positions had filenames that did not match the character's actual facing. - character_creation: Enter/KP_ENTER now honors _footer_start.disabled. Without a bookmark selected the Start button disables, but the keyboard path called _on_start() unconditionally — a player could confirm creation with empty bookmark/location strings. Guard at the top of _on_start. - protocol.gd: raw_bm.get("career", "tycoon") hardcoded a content default in the wire decoder — a missing server field silently became "tycoon". Empty string is the correct protocol default; _make_bookmark_card already skips the career label when empty. --- client/scripts/protocol/protocol.gd | 2 +- .../character_creation/character_creation.gd | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/client/scripts/protocol/protocol.gd b/client/scripts/protocol/protocol.gd index 798dde0b7..f8230bfba 100644 --- a/client/scripts/protocol/protocol.gd +++ b/client/scripts/protocol/protocol.gd @@ -409,7 +409,7 @@ static func decode_snapshot(bytes: PackedByteArray) -> Variant: "default_location": str(raw_bm.get("default_location", "")), "allowed_locations": al, "allowed_locations_cultures": alc, - "career": str(raw_bm.get("career", "tycoon")), + "career": str(raw_bm.get("career", "")), "starting_capital_tractus": int(raw_bm.get("starting_capital_tractus", 0)), } ) diff --git a/client/ui/meta/screens/character_creation/character_creation.gd b/client/ui/meta/screens/character_creation/character_creation.gd index 0ed963751..d3d13ecbf 100644 --- a/client/ui/meta/screens/character_creation/character_creation.gd +++ b/client/ui/meta/screens/character_creation/character_creation.gd @@ -193,7 +193,6 @@ const MANIFEST_PATH := "res://assets/characters/manifest.json" const APPEARANCE_SUB_NAMES := ["Body", "Head", "Hair", "Clothing", "Accessories"] const SCREENSHOT_DIR := "user://screenshots/" -const CARDINAL_NAMES: Array[String] = ["south", "east", "north", "west"] # --- Descriptor and preview state --- var _descriptor: CharacterVisualDescriptor @@ -1609,9 +1608,11 @@ func _take_screenshot(suffix: String = "") -> void: DirAccess.make_dir_recursive_absolute(SCREENSHOT_DIR) if _screenshot_cardinals: - # Take screenshot for current cardinal, then advance - var dir_name := CARDINAL_NAMES[_screenshot_cardinal_idx] - _char_visual.set_facing(CARDINAL_DIRS[_screenshot_cardinal_idx]) + # Take screenshot for current cardinal, then advance. Single array for + # both facing and filename label — previously two arrays with different + # orderings produced swapped labels at indices 1 and 3. + var dir_name := CARDINAL_DIRS[_screenshot_cardinal_idx] + _char_visual.set_facing(dir_name) suffix = dir_name var filename := ( @@ -1626,7 +1627,7 @@ func _take_screenshot(suffix: String = "") -> void: if _screenshot_cardinals: _screenshot_cardinal_idx += 1 - if _screenshot_cardinal_idx < CARDINAL_NAMES.size(): + if _screenshot_cardinal_idx < CARDINAL_DIRS.size(): # More directions to capture _schedule_screenshot() return @@ -2059,6 +2060,12 @@ func _on_back() -> void: func _on_start() -> void: + # Footer Start button owns the "is confirmation allowed" state + # (requires bookmark + location selection). Honor that gating for + # keyboard Enter as well — otherwise a player can press Enter with + # no bookmark and confirm with empty strings. + if _footer_start != null and _footer_start.disabled: + return var profile = CharacterProfile.new() # untyped — avoids parse-time CharacterVisualDescriptor resolution profile.descriptor = _descriptor profile.bookmark_id = _selected_bookmark_id From 1b8e22741313094abc3789908cf265febf98d274 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Apr 2026 12:02:09 +0200 Subject: [PATCH 16/18] =?UTF-8?q?refactor(ui):=20PR=20#134=20review=20?= =?UTF-8?q?=E2=80=94=20MetaScreen/ESC=20chain=20tightening,=20D-192=20rewo?= =?UTF-8?q?rd?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Tyre's 9 architecture items from the sprint-36 client review. - decisions/architecture.md (Tyre #1): D-192 now says "deprecate; removal tracked in #868" instead of "remove". The branch does not remove the version field or guard — that belongs in the coordinated server+client PR. The decision text now matches the code on this branch. - meta_stack.gd (#2): handle_escape() on a screen with closable_by_escape=false now consumes the event unconditionally. Was returning whatever on_escape() returned, which default-returned false and leaked ESC into main.gd's implant/settings chain — opening the settings dialog behind the loading screen. - debug_console.gd (#4): drop the direct KEY_ESCAPE branch in _unhandled_input. ESC now falls through to main.gd → MetaStack, which finds the console on top of the stack and closes it via the normal path. Other keys are still consumed so movement/action can't leak. - main.gd (#6, #10): extract the ESC priority chain into _handle_menu_key() so "MetaStack → implant → settings" is a named thing. Add a comment near connect_to_sim explaining that GameState.bookmark_catalog survives the Option A scene transition via the autoload. - main_menu.gd (#7): header comment documenting the double LoadingScreen lifecycle — safe today because main_menu.tscn and main.tscn never co-exist, noted for future promotion to autoload if that changes. - meta_screen.gd (#8): apply captures_input symmetrically in open()/ close() — was set in open() only, so a screen changing the flag between open+close kept the opened value forever. - meta_screen.gd (#9): on_escape() docstring clarifies the tri-state (consume-and-hold / consume-and-close / ignore) — and that closable_by_escape=false is the screen-wide way to say "consume-and-hold". - bug_report_dialog.gd (#11): capture_cancelled now emits from on_close() (covers any close path — ESC, MetaStack pop, programmatic close) rather than only on_escape(). A new _completed flag distinguishes completion from cancel so the two signals stay mutually exclusive. --- client/scripts/main.gd | 36 ++++++++++++------- client/ui/meta/meta_screen.gd | 15 ++++++-- client/ui/meta/meta_stack.gd | 8 ++++- .../screens/bug_report/bug_report_dialog.gd | 13 +++++-- .../screens/debug_console/debug_console.gd | 9 ++--- client/ui/meta/screens/main_menu/main_menu.gd | 7 ++++ decisions/architecture.md | 2 +- 7 files changed, 67 insertions(+), 23 deletions(-) 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. From 53fbce08fb7d7874acc43b686bc01a5eeaa3a100 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Apr 2026 12:02:25 +0200 Subject: [PATCH 17/18] chore(tests): hard 300s timeout + on-disk log + context-safe output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes tests/run-godot self-containing so neither humans nor LLM callers have to remember to wrap it in a timeout or pipe it into a file. A hung test now kills cleanly at 300s with a clear TEST_TIMEOUT marker and bisection hint instead of silently burning an hour of wall clock (as Sprint 36 learned). - Godot+gdUnit4 output goes to /tmp/sr-run-godot.log (overwritten each run). Nothing streams to stdout/stderr — 20k+ lines of test log into a terminal or an LLM context is unworkable. - Stdout: one-line JSON summary, with a "log" field pointing at the file. On timeout adds "timeout":true and "timeout_sec":300. - Stderr: a short hint block. On pass: one line. On failure: three commands to inspect the log. On timeout: a bisection recipe. - Single well-known path instead of an env var — worktrees each want their own value and the indirection makes the hint lines meaningless. Concurrent runs are the caller's problem. - timeout(1) --foreground --kill-after=10 to escalate to SIGKILL if Godot ignores SIGTERM. --- tests/run-godot | 100 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 23 deletions(-) diff --git a/tests/run-godot b/tests/run-godot index ee323e275..e09b91b26 100755 --- a/tests/run-godot +++ b/tests/run-godot @@ -1,11 +1,32 @@ #!/usr/bin/env bash # tests/run-godot: Run Godot client test suite via gdUnit4 (D-030) -# Exit: 0 = all pass, non-zero = failure -# Stdout: {"suite":"godot","total":N,"passed":N,"failed":N,"duration_ms":N} # -# --filter: accepts a test filename stem (e.g. "test_protocol" → runs test_protocol.gd only) +# Exit: 0 = all pass, non-zero = failure. +# Stdout: JSON summary on ONE line, plus a pointer to the full log. +# {"suite":"godot","total":N,"passed":N,"failed":N,"duration_ms":N,"log":"/tmp/sr-run-godot.log"} +# (on timeout: same JSON + "timeout":true, "timeout_sec":N; exit code 124) +# Stderr: a short hint line pointing at the log. The Godot/gdUnit4 output +# does NOT stream to stdout or stderr — it is captured to the log file. +# This is intentional: streaming 20k+ lines of test log into an LLM +# caller's context is unworkable. Inspect the log with the commands the +# hint line suggests. +# +# --filter : narrow the test run to a single file. set -euo pipefail +# Hard wall-clock cap. Sprint 36 lost an hour to a hung test suite that +# silently consumed CPU forever. 300s is generous for the full suite +# (which currently runs in ~60s) and well above the slowest single suite +# (~40s for the compositor build). If you need longer for an unusual +# workload (fixture regen, etc.), prefer adding a dedicated script over +# extending this cap — the cap is the point. +TIMEOUT_SEC=300 + +# Single well-known log path. Overwritten each run. No env var — worktrees +# would each want their own value and the indirection makes the hint +# line meaningless. Multiple concurrent runs are the caller's problem. +LOG_FILE="/tmp/sr-run-godot.log" + FILTER="" while [[ $# -gt 0 ]]; do case "$1" in @@ -38,32 +59,33 @@ else fi START_MS=$(date +%s%3N) -TMPOUT=$(mktemp) +# Redirect Godot+gdUnit4 output to the log file. Nothing streams to the +# caller — the summary JSON (stdout) and the hint line (stderr) are the +# only things the caller ever sees. See header comment for rationale. set +e -"$GODOT" --headless --path "$REPO_ROOT/client" \ - -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd \ - --ignoreHeadlessMode \ - -c \ - -a "$TEST_TARGET" \ - 2>&1 | tee "$TMPOUT" >&2 -EXIT_CODE=${PIPESTATUS[0]} +timeout --foreground --kill-after=10 "$TIMEOUT_SEC" \ + "$GODOT" --headless --path "$REPO_ROOT/client" \ + -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd \ + --ignoreHeadlessMode \ + -c \ + -a "$TEST_TARGET" \ + > "$LOG_FILE" 2>&1 +EXIT_CODE=$? set -e END_MS=$(date +%s%3N) DURATION_MS=$((END_MS - START_MS)) -_extract_num() { - local haystack="$1" pattern="$2" - echo "$haystack" | grep -oiE "[0-9]+ $pattern" | grep -oE '^[0-9]+' || echo 0 -} +# timeout(1) exit code 124 = wall clock exceeded; 137 = needed SIGKILL. +TIMED_OUT=false +if [[ "$EXIT_CODE" -eq 124 || "$EXIT_CODE" -eq 137 ]]; then + TIMED_OUT=true +fi # gdUnit4 outputs per-suite statistics: "N test cases | X errors | Y failures | ..." -# and a summary: "Executed test cases : (X/N)" or "Executed test cases : (X/N), Z skipped" TOTAL=0; PASSED=0; FAILED=0 - -# Sum errors + failures across all suite statistics lines -STATS_LINES=$(grep -oE "[0-9]+ test cases \| [0-9]+ errors \| [0-9]+ failures" "$TMPOUT" || true) +STATS_LINES=$(grep -oE "[0-9]+ test cases \| [0-9]+ errors \| [0-9]+ failures" "$LOG_FILE" || true) if [[ -n "$STATS_LINES" ]]; then TOTAL=$(echo "$STATS_LINES" | grep -oE '^[0-9]+' | awk '{s+=$1} END {print s}') ERRORS=$(echo "$STATS_LINES" | grep -oE '[0-9]+ errors' | grep -oE '^[0-9]+' | awk '{s+=$1} END {print s}') @@ -74,7 +96,7 @@ fi # Fallback: parse "Executed test cases : (X/N)" for total if stats parse failed if [[ "$TOTAL" -eq 0 ]]; then - EXEC_LINE=$(grep -oE "Executed test cases : \([0-9]+/[0-9]+\)" "$TMPOUT" | tail -1 || true) + EXEC_LINE=$(grep -oE "Executed test cases : \([0-9]+/[0-9]+\)" "$LOG_FILE" | tail -1 || true) if [[ -n "$EXEC_LINE" ]]; then TOTAL=$(echo "$EXEC_LINE" | grep -oE '/[0-9]+\)' | grep -oE '[0-9]+') PASSED=$(echo "$EXEC_LINE" | grep -oE '\([0-9]+/' | grep -oE '[0-9]+') @@ -82,7 +104,39 @@ if [[ "$TOTAL" -eq 0 ]]; then fi fi -rm -f "$TMPOUT" -printf '{"suite":"godot","total":%d,"passed":%d,"failed":%d,"duration_ms":%d}\n' \ - "${TOTAL:-0}" "${PASSED:-0}" "${FAILED:-0}" "$DURATION_MS" +LOG_LINES=$(wc -l < "$LOG_FILE" 2>/dev/null || echo 0) + +# Single-line JSON summary on stdout — machine-parseable, small. +if [[ "$TIMED_OUT" == "true" ]]; then + printf '{"suite":"godot","total":%d,"passed":%d,"failed":%d,"duration_ms":%d,"timeout":true,"timeout_sec":%d,"log":"%s"}\n' \ + "${TOTAL:-0}" "${PASSED:-0}" "${FAILED:-0}" "$DURATION_MS" "$TIMEOUT_SEC" "$LOG_FILE" +else + printf '{"suite":"godot","total":%d,"passed":%d,"failed":%d,"duration_ms":%d,"log":"%s"}\n' \ + "${TOTAL:-0}" "${PASSED:-0}" "${FAILED:-0}" "$DURATION_MS" "$LOG_FILE" +fi + +# Hint line on stderr. Stays short. LLM callers: read this literally. +if [[ "$TIMED_OUT" == "true" ]]; then + cat >&2 <} target=${TEST_TARGET} log=${LOG_FILE} (${LOG_LINES} lines) + This is NOT a regular test failure — the process was force-terminated. + A hung test almost always means a cleanup hook froze (e.g. after_test + freeing GdUnit4 infrastructure) or an assertion waits on a signal that + never fires. Bisect: + grep -E 'STARTED|PASSED|FAILED' ${LOG_FILE} | tail -20 + The last STARTED without a matching PASSED/FAILED is the hang site. +EOF +elif [[ "${FAILED:-0}" -gt 0 ]]; then + cat >&2 <' '{print \$1}' | sort | uniq -c | sort -rn + First failure block with context: + sed 's/\x1b\[[0-9;]*m//g' ${LOG_FILE} | grep -n 'FAILED\|Expecting\|Godot Runtime Error' | head -40 +EOF +else + echo "Tests passed. log=${LOG_FILE} (${LOG_LINES} lines)" >&2 +fi + exit $EXIT_CODE From 50ac9041ada25a7d762bc28678bc40f376a5ef79 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 21 Apr 2026 14:30:38 +0200 Subject: [PATCH 18/18] =?UTF-8?q?fix(tests):=20PR=20#134=20review=20round?= =?UTF-8?q?=203=20=E2=80=94=20seed=20start-state=20+=20PID=20log=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test_character_creation_sprint28: before_each now seeds _selected_bookmark_id and _selected_location_id so the new disabled- guard in _on_start() (round 2) doesn't silently block 5 existing tests that call _on_start()/KEY_ENTER without setting up a valid bookmark selection. Restores the 2 tests Hoshe flagged as R2-H1 plus 3 siblings that would have degraded the same way under the guard. - tests/run-godot: LOG_FILE now includes $$ (PID) so concurrent runs across worktrees don't clobber each other's logs. Path is echoed back via the stdout JSON "log" field and the stderr hint line, so callers never need to predict it (R2-H2). --- client/tests/test_character_creation_sprint28.gd | 7 +++++++ tests/run-godot | 10 ++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/client/tests/test_character_creation_sprint28.gd b/client/tests/test_character_creation_sprint28.gd index 84930111c..cabda7dd8 100644 --- a/client/tests/test_character_creation_sprint28.gd +++ b/client/tests/test_character_creation_sprint28.gd @@ -26,6 +26,13 @@ func before_each() -> void: return _scene = packed.instantiate() as CharacterCreation add_child(_scene) + # Seed a valid bookmark/location so _on_start passes the disabled guard + # added in PR #134 (R2-Hoshe-1). Tests that verify the disabled state + # should explicitly clear these and call _update_start_btn_state(). + _scene._selected_bookmark_id = "test-bookmark" + _scene._selected_location_id = "test-location" + if _scene.has_method("_update_start_btn_state"): + _scene._update_start_btn_state() func after_each() -> void: diff --git a/tests/run-godot b/tests/run-godot index e09b91b26..b6713e382 100755 --- a/tests/run-godot +++ b/tests/run-godot @@ -22,10 +22,12 @@ set -euo pipefail # extending this cap — the cap is the point. TIMEOUT_SEC=300 -# Single well-known log path. Overwritten each run. No env var — worktrees -# would each want their own value and the indirection makes the hint -# line meaningless. Multiple concurrent runs are the caller's problem. -LOG_FILE="/tmp/sr-run-godot.log" +# Per-process log path. PID suffix prevents concurrent runs across worktrees +# from clobbering each other's logs and producing summary JSON that mixes +# counts from different suites (R2-Hoshe-2). The actual path is echoed back +# via the JSON "log" field and the stderr hint line, so callers don't need +# to predict it. +LOG_FILE="/tmp/sr-run-godot.$$.log" FILTER="" while [[ $# -gt 0 ]]; do