diff --git a/client/scripts/autoloads/game_state.gd b/client/scripts/autoloads/game_state.gd index 78866ba2f..6be09bc9f 100644 --- a/client/scripts/autoloads/game_state.gd +++ b/client/scripts/autoloads/game_state.gd @@ -40,6 +40,17 @@ var dialogue_active: bool = false var room_id: Variant = null # String room_id from snapshot, null in non-gauntlet mode var gauntlet_mode: bool = false # true when snapshot includes gauntlet_mode flag +# OQ-07 (#522): Insert active state — false suppresses verb labels (z-layer 6). +# Cursor shape changes still fire when false (D-056 option a). +# v0.1 assumption: always true — both playable characters (detective and smuggler) +# have neural inserts. Future characters without inserts would receive false from +# the server's "insert_active" snapshot field, disabling all z-layer-6 UI. +var insert_active: bool = true + +# #507: RNG seed for replay determinism — populated from snapshot "rng_seed" field. +# Null in v0.1 (server does not yet send this field; protocol change required). +var rng_seed: Variant = null + # v7 fields (#431, D-059/D-060) var pending_recognitions: Array = [] # [{entity_id, x, y, z, remaining_ticks, total_delay_ticks}] @@ -132,6 +143,20 @@ func apply_snapshot(snapshot: Dictionary) -> void: else: room_id = null + # OQ-07 (#522): insert_active — defaults true (v0.1 always has insert). + # Server may send false for characters without an insert in future sprints. + if snapshot.has("insert_active") and snapshot.insert_active is bool: + insert_active = snapshot.insert_active + else: + insert_active = true + + # #507: rng_seed — server sends current RNG seed for replay determinism. + # Field: "rng_seed" (u64 as integer). Null if server does not include it. + if snapshot.has("rng_seed"): + rng_seed = snapshot.rng_seed + else: + rng_seed = null + # v2: visible_tiles with visibility sectors # Derives visible_positions when not explicitly provided (real server mode) if snapshot.has("visible_tiles") and snapshot.visible_tiles is Array and snapshot.visible_tiles.size() > 0: diff --git a/client/scripts/autoloads/sim_bridge.gd b/client/scripts/autoloads/sim_bridge.gd index 4c7c8b98b..df6724770 100644 --- a/client/scripts/autoloads/sim_bridge.gd +++ b/client/scripts/autoloads/sim_bridge.gd @@ -179,7 +179,7 @@ func send_input(player_input: Dictionary) -> Error: return ERR_CONNECTION_ERROR if test_mode: var action: int = player_input.get("action", -1) - var wire_name: String = _action_enum_to_wire(action) + var wire_name: String = action_enum_to_wire(action) if not wire_name.is_empty(): if wire_name == "SetFacing": # D-054: Use action_data.facing from the input dict, not InputMapper global @@ -192,9 +192,9 @@ func send_input(player_input: Dictionary) -> Error: else: _test_input_queue.append(wire_name) return OK - var action_name := _action_enum_to_wire(player_input.get("action", -1)) + var action_name := action_enum_to_wire(player_input.get("action", -1)) if action_name.is_empty(): - # _action_enum_to_wire already emits push_warning for invalid actions + # action_enum_to_wire already emits push_warning for invalid actions return ERR_INVALID_PARAMETER # Use the server's current tick so drain_for_tick processes this input immediately. # The client-side timestamp_msec is only useful for ordering within a frame. @@ -251,7 +251,7 @@ func drain_outbound() -> Array[Dictionary]: # Map InputMapper.Action enum values to wire-format action names (matching Rust PlayerAction). # OPEN_MENU is client-only — no Rust equivalent, not sent over the wire. -static func _action_enum_to_wire(action: int) -> String: +static func action_enum_to_wire(action: int) -> String: match action: InputMapper.Action.MOVE_NORTH: return "MoveNorth" InputMapper.Action.MOVE_NORTHEAST: return "MoveNortheast" diff --git a/client/scripts/main.gd b/client/scripts/main.gd index 8368bb2a5..ed72d567b 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -22,6 +22,7 @@ var _last_monologue_tick: int = -1 # Prevent re-consuming monologue when same t var _last_dialogue_tick: int = -1 var _flash_rect: ColorRect = null # #502/#501: ephemeral screen flash overlay (shared: teleport preempts amber) var _teleport_in_progress: bool = false # #501: defer smoothing re-enable by one frame after teleport +var _pending_record_inputs: Array = [] # #507: accumulates server-bound inputs across frames; flushed into record_tick() on snapshot arrival func _ready() -> void: print("The Settled Reach — client initialized") @@ -81,6 +82,16 @@ func _process(_delta: float) -> void: if world_renderer and world_renderer.has_method("update_from_state"): world_renderer.update_from_state() + # OQ-07 (#522): propagate insert state to all z-layer-6 display nodes. + # Cursor shape still fires (D-056 option a) — only verb labels suppressed. + var insert_state := GameState.insert_active + if cursor_renderer and cursor_renderer.has_method("set_insert_active"): + cursor_renderer.set_insert_active(insert_state) + if interaction_list and interaction_list.has_method("set_insert_active"): + interaction_list.set_insert_active(insert_state) + if interaction_prompt and interaction_prompt.has_method("set_insert_active"): + interaction_prompt.set_insert_active(insert_state) + # D-057: Update interaction list from game state # Suppress during dialogue — player is in conversation, verb list is noise if interaction_list and interaction_list.has_method("update_from_state"): @@ -135,6 +146,8 @@ func _process(_delta: float) -> void: camera.reset_smoothing() # Send queued input to simulation + # #507: Server-bound inputs are accumulated into _pending_record_inputs across frames. + # At 60fps/10tps, inputs on non-snapshot frames must not be lost from the ring buffer. var inputs = InputMapper.flush_queue() for input in inputs: # #495: F12 WRONG button — client-only, trigger bug report capture @@ -164,6 +177,18 @@ func _process(_delta: float) -> void: "verb": null, } SimBridge.send_input(input) + _pending_record_inputs.append(input) + + # #507: Record tick data to ring buffer — once per server tick (snapshot arrival). + # Flushes all inputs accumulated since the last snapshot (across multiple display frames), + # then clears the accumulator for the next tick. + if snapshot != null and bug_report_dialog and bug_report_dialog.has_method("record_tick"): + bug_report_dialog.record_tick( + GameState.current_tick, + JSON.stringify(GameState.current_snapshot), + _pending_record_inputs + ) + _pending_record_inputs.clear() # Consume-once per tick: show monologue text, then clear. diff --git a/client/scripts/rendering/cursor_renderer.gd b/client/scripts/rendering/cursor_renderer.gd index a68036ff9..690f05d46 100644 --- a/client/scripts/rendering/cursor_renderer.gd +++ b/client/scripts/rendering/cursor_renderer.gd @@ -11,6 +11,9 @@ enum State { DEFAULT, ENTITY_HOVER, OBJECT_HOVER, WEAPON_AIM } var current_state: State = State.DEFAULT var hovered_entity_id: int = -1 var weapon_mode_active: bool = false +# OQ-07 (#522): when false, verb labels are suppressed (should_show_interactions → false). +# Cursor shape transitions still fire — the character's body still orients to targets. +var insert_active: bool = true signal state_changed(new_state: State) signal hovered_entity_changed(entity_id: int) @@ -305,8 +308,15 @@ func get_z_layer() -> int: func should_show_interactions() -> bool: + # OQ-07: insert off suppresses verb labels even though cursor shape still changes + if not insert_active: + return false return not weapon_mode_active or _shift_held +func set_insert_active(active: bool) -> void: + insert_active = active + + func get_interaction_range() -> int: return 2 # D-056: ~2 sim tiles diff --git a/client/tests/test_anti_tedium.gd b/client/tests/test_anti_tedium.gd index f9c493d09..0f7791add 100644 --- a/client/tests/test_anti_tedium.gd +++ b/client/tests/test_anti_tedium.gd @@ -518,7 +518,7 @@ func test_bug_report_not_double_activatable() -> void: func test_bug_report_action_not_on_wire() -> void: # BUG_REPORT is client-only — must not produce a wire-format action name. - var wire_name := SimBridge._action_enum_to_wire(InputMapper.Action.BUG_REPORT) + var wire_name := SimBridge.action_enum_to_wire(InputMapper.Action.BUG_REPORT) assert_that(wire_name).override_failure_message( "BUG_REPORT must not produce a wire action name (client-only)" ).is_equal("") diff --git a/client/tests/test_bug_report_ring_buffer.gd b/client/tests/test_bug_report_ring_buffer.gd new file mode 100644 index 000000000..f3f3afb5e --- /dev/null +++ b/client/tests/test_bug_report_ring_buffer.gd @@ -0,0 +1,361 @@ +## #507: WRONG button ring buffer tests. +## Tests the 60-tick rolling history upgrade to bug_report_dialog.gd. +## Covers ring buffer capacity, circular overwrite, JSONL format compliance, +## replay compatibility, seed file, and regression against MVP behavior. +## +## MVP baseline (must still pass): snapshot.json, render.txt, description.txt. +## New outputs: inputs.jsonl, snapshots.jsonl, seed.txt. +## +## Spec refs: D-030 (testability), D-020 (ObserverSnapshot boundary) +## Sprint 11 client.md §Notes #507 +class_name TestBugReportRingBuffer +extends GdUnitTestSuite + + +var BugReportDialogScript = load("res://ui/bug_report_dialog.gd") + +# Expected ring buffer capacity per spec. +const EXPECTED_CAPACITY := 60 + + +func after_each() -> void: + # Reset GameState fields mutated by tests to prevent cross-test leakage. + GameState.current_tick = 0 + GameState.room_id = null + GameState.player_stance = "Walk" + GameState.player_facing = "South" + GameState.player_position = Vector2.ZERO + GameState.visible_entities = [] + GameState.visible_tiles = [] + GameState.game_time = {} + GameState.current_monologue = null + GameState.current_dialogue = null + GameState.rng_seed = null + + +# -- Helpers ------------------------------------------------------------------- + +func _make_dialog() -> Control: + var dialog = Control.new() + dialog.set_script(BugReportDialogScript) + auto_free(dialog) + add_child(dialog) + return dialog + + +## Create a minimal valid PlayerInput dict matching the replay.rs wire format. +## replay.rs expects: {"tick": N, "action": "MoveNorth"} or action as dict. +func _make_input(tick: int, action: String = "MoveNorth") -> Dictionary: + return {"tick": tick, "action": action} + + +## Make a minimal snapshot JSON string for snapshot buffer testing. +func _make_snapshot_json(tick: int) -> String: + return JSON.stringify({ + "tick": tick, + "version": Protocol.PROTOCOL_VERSION, + "entities": [], + }) + + +## Check a string is valid JSON array — mirrors replay.rs line expectation. +func _is_valid_json_array(line: String) -> bool: + if line.is_empty(): + return false + var result = JSON.parse_string(line) + return result != null and result is Array + + +# -- Ring buffer capacity ------------------------------------------------------ + +func test_buffer_capacity_is_60() -> void: + # Spec: "Maintain a 60-entry circular buffer" + var dialog = _make_dialog() + if not dialog.has_method("_get_buffer_capacity"): + push_warning("TestBugReportRingBuffer: _get_buffer_capacity not found — awaiting #507") + return + assert_that(dialog._get_buffer_capacity()).override_failure_message( + "Ring buffer capacity must be 60 per spec" + ).is_equal(EXPECTED_CAPACITY) + + +func test_snapshot_buffer_capacity_is_60() -> void: + # Parallel snapshot buffer must match input buffer capacity. + var dialog = _make_dialog() + if not dialog.has_method("_get_snapshot_buffer_capacity"): + push_warning("TestBugReportRingBuffer: _get_snapshot_buffer_capacity not found — awaiting #507") + return + assert_that(dialog._get_snapshot_buffer_capacity()).is_equal(EXPECTED_CAPACITY) + + +# -- Pre-allocation ----------------------------------------------------------- + +func test_input_buffer_preallocated_at_ready() -> void: + # Spec: "pre-allocate the 60-slot arrays at startup. Do not allocate on every tick." + var dialog = _make_dialog() + if not dialog.has_method("_get_input_buffer"): + push_warning("TestBugReportRingBuffer: _get_input_buffer not found — awaiting #507") + return + var buf = dialog._get_input_buffer() + assert_that(buf is Array).override_failure_message( + "Input buffer must be an Array" + ).is_true() + assert_that(buf.size()).override_failure_message( + "Input buffer must be pre-allocated at capacity (%d slots)" % EXPECTED_CAPACITY + ).is_equal(EXPECTED_CAPACITY) + + +# -- Push and fill behavior --------------------------------------------------- + +func test_push_fills_input_buffer() -> void: + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_inputs"): + push_warning("TestBugReportRingBuffer: _push_tick_inputs not found — awaiting #507") + return + if not dialog.has_method("_get_filled_input_count"): + push_warning("TestBugReportRingBuffer: _get_filled_input_count not found — awaiting #507") + return + for i in range(10): + dialog._push_tick_inputs(i, [_make_input(i)]) + assert_that(dialog._get_filled_input_count()).override_failure_message( + "10 pushed ticks should produce filled count of 10" + ).is_equal(10) + + +func test_push_fills_snapshot_buffer() -> void: + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_snapshot"): + push_warning("TestBugReportRingBuffer: _push_tick_snapshot not found — awaiting #507") + return + if not dialog.has_method("_get_filled_snapshot_count"): + push_warning("TestBugReportRingBuffer: _get_filled_snapshot_count not found — awaiting #507") + return + for i in range(10): + dialog._push_tick_snapshot(_make_snapshot_json(i)) + assert_that(dialog._get_filled_snapshot_count()).is_equal(10) + + +# -- Circular overwrite ------------------------------------------------------- + +func test_circular_overwrite_evicts_oldest() -> void: + # After 61 pushes, filled count must be 60 (oldest evicted). + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_inputs") or not dialog.has_method("_get_filled_input_count"): + push_warning("TestBugReportRingBuffer: ring buffer API not found — awaiting #507") + return + for i in range(EXPECTED_CAPACITY + 1): + dialog._push_tick_inputs(i, [_make_input(i)]) + assert_that(dialog._get_filled_input_count()).override_failure_message( + "After 61 pushes, buffer must hold exactly 60 entries (oldest evicted)" + ).is_equal(EXPECTED_CAPACITY) + + +func test_circular_overwrite_keeps_newest_inputs() -> void: + # After 61 pushes, the flushed JSONL must contain tick 1..60 (not tick 0). + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_inputs") or not dialog.has_method("_format_inputs_jsonl"): + push_warning("TestBugReportRingBuffer: ring buffer flush API not found — awaiting #507") + return + # Push 61 ticks. Tick 0 should be evicted; ticks 1-60 should be present. + for i in range(EXPECTED_CAPACITY + 1): + dialog._push_tick_inputs(i, [_make_input(i, "MoveNorth")]) + var jsonl: String = dialog._format_inputs_jsonl() + # Tick 0 action would be at position 0, but after 61 pushes, position 0 was overwritten. + # We check that the output has exactly 60 lines. + var lines: PackedStringArray = jsonl.split("\n", false) + # Filter out blank lines (PackedStringArray has no filter — convert to Array first) + var non_blank: Array = Array(lines).filter(func(l): return not l.strip_edges().is_empty()) + assert_that(non_blank.size()).override_failure_message( + "After 61 pushes, JSONL output must have exactly 60 non-blank lines" + ).is_equal(EXPECTED_CAPACITY) + + +func test_exact_60_pushes_no_eviction() -> void: + # After exactly 60 pushes, all 60 are present. + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_inputs") or not dialog.has_method("_get_filled_input_count"): + push_warning("TestBugReportRingBuffer: ring buffer API not found — awaiting #507") + return + for i in range(EXPECTED_CAPACITY): + dialog._push_tick_inputs(i, [_make_input(i)]) + assert_that(dialog._get_filled_input_count()).override_failure_message( + "After exactly 60 pushes, all 60 entries should be present" + ).is_equal(EXPECTED_CAPACITY) + + +# -- JSONL format compliance (replay.rs contract) ---------------------------- + +func test_inputs_jsonl_each_line_is_json_array() -> void: + # replay.rs: "Each line is a JSON array of PlayerInput for one tick." + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_inputs") or not dialog.has_method("_format_inputs_jsonl"): + push_warning("TestBugReportRingBuffer: JSONL API not found — awaiting #507") + return + for i in range(3): + dialog._push_tick_inputs(i, [_make_input(i)]) + var jsonl: String = dialog._format_inputs_jsonl() + var lines: PackedStringArray = jsonl.split("\n", false) + var non_blank: Array = Array(lines).filter(func(l): return not l.strip_edges().is_empty()) + for line in non_blank: + assert_that(_is_valid_json_array(line)).override_failure_message( + "Each JSONL line must be a valid JSON array, got: %s" % line + ).is_true() + + +func test_inputs_jsonl_idle_tick_is_empty_array() -> void: + # replay.rs: "Empty array = idle tick (no input sent)." + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_inputs") or not dialog.has_method("_format_inputs_jsonl"): + push_warning("TestBugReportRingBuffer: JSONL API not found — awaiting #507") + return + dialog._push_tick_inputs(0, []) # Idle tick: no inputs + var jsonl: String = dialog._format_inputs_jsonl() + var lines: PackedStringArray = jsonl.split("\n", false) + var non_blank: Array = Array(lines).filter(func(l): return not l.strip_edges().is_empty()) + assert_that(non_blank.size()).override_failure_message( + "One idle tick should produce exactly 1 JSONL line" + ).is_equal(1) + assert_that(non_blank[0].strip_edges()).override_failure_message( + "Idle tick line must be an empty JSON array '[]'" + ).is_equal("[]") + + +func test_inputs_jsonl_has_tick_field() -> void: + # replay.rs PlayerInput expects {"tick": N, "action": "..."} + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_inputs") or not dialog.has_method("_format_inputs_jsonl"): + push_warning("TestBugReportRingBuffer: JSONL API not found — awaiting #507") + return + dialog._push_tick_inputs(42, [_make_input(42, "MoveEast")]) + var jsonl: String = dialog._format_inputs_jsonl() + # Parse the first line and verify structure + var lines: PackedStringArray = jsonl.split("\n", false) + var non_blank: Array = Array(lines).filter(func(l): return not l.strip_edges().is_empty()) + assert_that(non_blank.size()).is_greater(0) + var arr = JSON.parse_string(non_blank[0]) + assert_that(arr is Array).is_true() + assert_that(arr.size() > 0).is_true() + var first_input: Dictionary = arr[0] + assert_that(first_input.has("tick")).override_failure_message( + "Each PlayerInput in JSONL must have 'tick' field" + ).is_true() + assert_that(first_input.has("action")).override_failure_message( + "Each PlayerInput in JSONL must have 'action' field" + ).is_true() + + +func test_inputs_jsonl_multiple_actions_per_tick() -> void: + # replay.rs supports multiple PlayerInput per tick (one array entry per action). + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_inputs") or not dialog.has_method("_format_inputs_jsonl"): + push_warning("TestBugReportRingBuffer: JSONL API not found — awaiting #507") + return + dialog._push_tick_inputs(0, [ + _make_input(0, "MoveNorth"), + _make_input(0, "Pause"), + ]) + var jsonl: String = dialog._format_inputs_jsonl() + var lines: PackedStringArray = jsonl.split("\n", false) + var non_blank: Array = Array(lines).filter(func(l): return not l.strip_edges().is_empty()) + assert_that(non_blank.size()).is_equal(1) + var arr = JSON.parse_string(non_blank[0]) + assert_that(arr is Array).is_true() + assert_that(arr.size()).override_failure_message( + "Tick with 2 actions must have 2 entries in the JSONL array" + ).is_equal(2) + + +func test_snapshots_jsonl_each_line_is_valid_json() -> void: + # Each snapshot line should be valid JSON (parsed snapshot). + var dialog = _make_dialog() + if not dialog.has_method("_push_tick_snapshot") or not dialog.has_method("_format_snapshots_jsonl"): + push_warning("TestBugReportRingBuffer: snapshots JSONL API not found — awaiting #507") + return + for i in range(3): + dialog._push_tick_snapshot(_make_snapshot_json(i)) + var jsonl: String = dialog._format_snapshots_jsonl() + var lines: PackedStringArray = jsonl.split("\n", false) + var non_blank: Array = Array(lines).filter(func(l): return not l.strip_edges().is_empty()) + assert_that(non_blank.size()).is_equal(3) + for line in non_blank: + var parsed = JSON.parse_string(line) + assert_that(parsed != null).override_failure_message( + "Each snapshot line must be valid JSON, got: %s" % line + ).is_true() + assert_that(parsed is Dictionary).override_failure_message( + "Each snapshot line must parse to a Dictionary" + ).is_true() + + +# -- Seed file ---------------------------------------------------------------- + +func test_seed_written_on_capture() -> void: + # Spec: "Include the server's current RNG seed... Output as seed.txt" + # We can't easily verify file output in headless mode, but we verify + # the dialog has the method to produce the seed value. + var dialog = _make_dialog() + if not dialog.has_method("_get_current_seed"): + push_warning("TestBugReportRingBuffer: _get_current_seed not found — awaiting #507") + return + # Should return some Variant (int or String) without crashing + var seed_val = dialog._get_current_seed() + assert_that(seed_val != null).override_failure_message( + "_get_current_seed() must not return null (use 0 or 'unavailable' if seed unavailable)" + ).is_true() + + +# -- Room metadata regression ------------------------------------------------- + +func test_description_txt_still_contains_room_id() -> void: + # Regression: MVP description.txt must still include room metadata. + GameState.current_tick = 5 + GameState.room_id = "warehouse_01" + GameState.player_stance = "Walk" + GameState.player_facing = "North" + GameState.player_position = Vector2(10.0, 10.0) + GameState.visible_entities = [] + GameState.visible_tiles = [] + GameState.game_time = {} + GameState.current_monologue = null + GameState.current_dialogue = null + var dialog = _make_dialog() + # Call the render text method to verify room shows up in render output + # (description.txt itself writes via FileAccess, difficult in headless — test the render path) + if dialog.has_method("_render_snapshot_text"): + # The text render is for render.txt, but description.txt room comes from GameState.room_id + # We indirectly verify by checking that the game state has the right fields. + assert_that(GameState.room_id).is_equal("warehouse_01") + GameState.room_id = null + + +# -- MVP regression ----------------------------------------------------------- + +func test_render_snapshot_text_still_works() -> void: + # Regression: MVP _render_snapshot_text() must still work after upgrade. + GameState.current_tick = 10 + GameState.player_position = Vector2(5.0, 5.0) + GameState.player_facing = "East" + GameState.player_stance = "Careful" + GameState.visible_entities = [] + GameState.visible_tiles = [] + GameState.game_time = {} + GameState.current_monologue = null + GameState.current_dialogue = null + var dialog = _make_dialog() + if not dialog.has_method("_render_snapshot_text"): + push_warning("TestBugReportRingBuffer: _render_snapshot_text missing — regression risk") + return + var text: String = dialog._render_snapshot_text() + assert_that(text.length() > 0).is_true() + assert_that(text.contains("t10")).is_true() + + +func test_dialog_is_active_api_unchanged() -> void: + # Regression: public API from MVP must be unchanged. + var dialog = _make_dialog() + assert_that(dialog.has_method("start_capture")).override_failure_message( + "start_capture() must still be present (MVP regression)" + ).is_true() + assert_that(dialog.has_method("is_active")).override_failure_message( + "is_active() must still be present (MVP regression)" + ).is_true() diff --git a/client/tests/test_cursor_states.gd b/client/tests/test_cursor_states.gd index 2b5358127..726be1b72 100644 --- a/client/tests/test_cursor_states.gd +++ b/client/tests/test_cursor_states.gd @@ -305,6 +305,46 @@ func test_cursor_changes_require_los() -> void: cursor.queue_free() +# -- OQ-07: Insert-off behavior (D-056 amendment, #522) ---------------------- +# Option (a): cursor shape still changes, verb labels suppressed. + +func test_insert_off_cursor_still_changes_shape() -> void: + # OQ-07 option (a): cursor state machine still fires when insert is off. + # The character's body orients toward targets even without insert data. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if cursor.has_method("set_insert_active") and cursor.has_method("set_hover_target"): + cursor.set_insert_active(false) + cursor.set_hover_target({"entity_id": 2, "kind": "Npc", "relationship": "Unknown"}) + assert_that(str(cursor.get_state())).is_equal("EntityHover") + cursor.queue_free() + + +func test_insert_off_suppresses_interactions() -> void: + # OQ-07 option (a): with insert off, should_show_interactions() returns false. + # Verb labels (z-layer 6) are suppressed — no actionable insert data. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if cursor.has_method("set_insert_active") and cursor.has_method("should_show_interactions"): + cursor.set_insert_active(false) + assert_that(cursor.should_show_interactions()).is_false() + cursor.queue_free() + + +func test_insert_on_restores_interaction_display() -> void: + # Re-enabling insert allows interactions to show again. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if cursor.has_method("set_insert_active") and cursor.has_method("should_show_interactions"): + cursor.set_insert_active(false) + cursor.set_insert_active(true) + assert_that(cursor.should_show_interactions()).is_true() + cursor.queue_free() + + # -- Interaction range (D-056) ------------------------------------------------ func test_click_interaction_range() -> void: diff --git a/client/tests/test_hub_teleport.gd b/client/tests/test_hub_teleport.gd index 12ce5f24b..fe727e930 100644 --- a/client/tests/test_hub_teleport.gd +++ b/client/tests/test_hub_teleport.gd @@ -129,13 +129,13 @@ func test_gauntlet_mode_transitions_off() -> void: func test_teleport_hub_wire_name() -> void: # TELEPORT_HUB must encode to "TeleportToHub" on the wire (matching Rust PlayerAction) - var wire_name := SimBridge._action_enum_to_wire(InputMapper.Action.TELEPORT_HUB) + var wire_name := SimBridge.action_enum_to_wire(InputMapper.Action.TELEPORT_HUB) assert_that(wire_name).is_equal("TeleportToHub") func test_teleport_hub_wire_not_empty() -> void: # Wire name must not be empty (empty = client-only, not sent to server) - var wire_name := SimBridge._action_enum_to_wire(InputMapper.Action.TELEPORT_HUB) + var wire_name := SimBridge.action_enum_to_wire(InputMapper.Action.TELEPORT_HUB) assert_that(wire_name.is_empty()).is_false() diff --git a/client/tests/test_insert_off_behavior.gd b/client/tests/test_insert_off_behavior.gd new file mode 100644 index 000000000..bfd4ffe89 --- /dev/null +++ b/client/tests/test_insert_off_behavior.gd @@ -0,0 +1,400 @@ +## #522: OQ-07 resolution — no-insert interaction behavior. +## Tests that when insert_active == false, interaction labels disappear +## (diegetic test from D-056 and D-057), and that cursor and interaction +## list agree on insert state. +## +## D-056: "Interaction labels render on z-layer 6 (insert overlay). +## If the insert is off, labels disappear." +## D-057: "Labels render on z-layer 6. If insert is off, labels disappear." +## +## OQ-07 resolution (expected option a): cursor shape still changes +## (character physically orients), but verb labels are suppressed. +## +## Spec refs: D-056, D-057, D-048, D-049 +## Ticket #522 +class_name TestInsertOffBehavior +extends GdUnitTestSuite + + +func after_each() -> void: + # Reset GameState fields mutated by tests to prevent cross-test leakage. + GameState.nearby_interactions = [] + GameState.player_stance = "Walk" + GameState.insert_active = true + + +# -- Helpers ------------------------------------------------------------------- + +func _make_cursor() -> Node: + for path in ["res://ui/cursor_state_machine.tscn", "res://scenes/cursor.tscn", "res://ui/cursor.tscn"]: + if ResourceLoader.exists(path): + var scene = load(path) + var cursor = scene.instantiate() + add_child(cursor) + return cursor + return null + + +func _make_cursor_or_skip() -> Node: + var cursor = _make_cursor() + if cursor == null: + push_warning("TestInsertOffBehavior: cursor scene not found — test skipped (awaiting #522)") + return cursor + + +func _make_interaction_list() -> Node: + for path in ["res://ui/interaction_list.tscn", "res://ui/entity_interaction_list.tscn", + "res://scenes/interaction_list.tscn"]: + if ResourceLoader.exists(path): + var scene = load(path) + var node = scene.instantiate() + add_child(node) + return node + return null + + +func _make_list_or_skip() -> Node: + var list = _make_interaction_list() + if list == null: + push_warning("TestInsertOffBehavior: interaction list scene not found — test skipped (awaiting #522)") + return list + + +func _set_test_interactions() -> void: + GameState.nearby_interactions = [{ + "entity_id": 2, + "entity_type": "Npc", + "distance": 1, + "verbs": [ + {"kind": "Talk", "label": "Talk", "priority": 1, "available": true}, + ], + }] + + +func _clear_test_interactions() -> void: + GameState.nearby_interactions = [] + + +# -- Interaction list: diegetic test (D-056, D-057) --------------------------- + +func test_insert_off_hides_interaction_list() -> void: + # D-056/D-057 diegetic test: "If the insert is off, labels disappear." + var list = _make_list_or_skip() + if list == null: + return + _set_test_interactions() + if list.has_method("set_insert_active"): + list.set_insert_active(false) + if list.has_method("update_from_state"): + list.update_from_state() + if list.has_method("is_showing"): + assert_that(list.is_showing()).override_failure_message( + "D-056/D-057 diegetic test: interaction list must be hidden when insert is off" + ).is_false() + elif list.has_method("get_visible_verb_count"): + assert_that(list.get_visible_verb_count()).override_failure_message( + "Interaction list must show 0 verbs when insert is off" + ).is_equal(0) + list.queue_free() + _clear_test_interactions() + + +func test_insert_on_shows_interaction_list() -> void: + # When insert is on (normal state), verbs should be visible. + var list = _make_list_or_skip() + if list == null: + return + _set_test_interactions() + if list.has_method("set_insert_active"): + list.set_insert_active(true) + if list.has_method("update_from_state"): + list.update_from_state() + if list.has_method("is_showing"): + assert_that(list.is_showing()).override_failure_message( + "Interaction list must be visible when insert is on and verbs present" + ).is_true() + list.queue_free() + _clear_test_interactions() + + +func test_insert_reenable_restores_list() -> void: + # Toggling insert off then on restores verb list visibility. + var list = _make_list_or_skip() + if list == null: + return + _set_test_interactions() + if not list.has_method("set_insert_active"): + list.queue_free() + _clear_test_interactions() + return + list.set_insert_active(false) + if list.has_method("update_from_state"): + list.update_from_state() + list.set_insert_active(true) + if list.has_method("update_from_state"): + list.update_from_state() + if list.has_method("is_showing"): + assert_that(list.is_showing()).override_failure_message( + "Re-enabling insert must restore list visibility" + ).is_true() + list.queue_free() + _clear_test_interactions() + + +# -- Cursor: insert-off behavior (OQ-07 resolution option a) ------------------ + +func test_insert_off_suppresses_should_show_interactions() -> void: + # OQ-07 option (a): labels are suppressed when insert is off. + # Regardless of cursor visual state, should_show_interactions() must return false. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if not cursor.has_method("set_insert_active"): + push_warning("TestInsertOffBehavior: cursor.set_insert_active not found — awaiting #522") + cursor.queue_free() + return + cursor.set_insert_active(false) + if cursor.has_method("should_show_interactions"): + assert_that(cursor.should_show_interactions()).override_failure_message( + "should_show_interactions() must return false when insert is off" + ).is_false() + cursor.queue_free() + + +func test_insert_on_restores_should_show_interactions() -> void: + # After re-enabling insert, interactions should show again (not in weapon mode). + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if not cursor.has_method("set_insert_active"): + push_warning("TestInsertOffBehavior: cursor.set_insert_active not found — awaiting #522") + cursor.queue_free() + return + cursor.set_insert_active(false) + cursor.set_insert_active(true) + if cursor.has_method("should_show_interactions"): + assert_that(cursor.should_show_interactions()).override_failure_message( + "should_show_interactions() must return true after re-enabling insert" + ).is_true() + cursor.queue_free() + + +func test_insert_off_at_startup_no_corruption() -> void: + # Insert can be off from the start — no state machine corruption. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if not cursor.has_method("set_insert_active") or not cursor.has_method("get_state"): + cursor.queue_free() + return + cursor.set_insert_active(false) + # Cursor must be in a valid state (not null/error) + var state_str := str(cursor.get_state()) + assert_that(["Default", "EntityHover", "ObjectHover", "WeaponAim"].has(state_str)).override_failure_message( + "Cursor must be in a valid state after insert-off at startup, got: %s" % state_str + ).is_true() + cursor.queue_free() + + +# -- OQ-07 option (a): cursor shape still changes, labels suppressed ---------- + +func test_insert_off_option_a_cursor_still_transitions() -> void: + # Option (a): cursor shape changes even when insert off + # (character physically orients to target, just no labels). + # If implementation chose option (b) instead, this test would fail — that's informative. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if not cursor.has_method("set_insert_active") or not cursor.has_method("set_hover_target"): + push_warning("TestInsertOffBehavior: set_insert_active or set_hover_target not found — awaiting #522") + cursor.queue_free() + return + cursor.set_insert_active(false) + cursor.set_hover_target({"entity_id": 2, "kind": "Npc", "relationship": "Unknown", "in_los": true}) + # Option (a): cursor SHOULD be in EntityHover despite insert being off + # Option (b): cursor would stay in Default + # Test documents expected behavior — fail message explains which option is active + if cursor.has_method("get_state"): + var state := str(cursor.get_state()) + # If this assertion fails, option (b) was implemented (cursor full-suppressed). + # Neither (a) nor (b) is wrong — this test documents which was chosen. + # Consult decisions/perception.md for the OQ-07 amendment. + assert_that(state).override_failure_message( + "OQ-07 option (a): cursor should still transition to EntityHover when insert is off. " + + "If this fails, option (b) was implemented (full suppression) — update this test to assert 'Default' instead." + ).is_equal("EntityHover") + cursor.queue_free() + + +# -- Edge cases --------------------------------------------------------------- + +func test_insert_off_plus_weapon_mode_stays_suppressed() -> void: + # Insert-off AND weapon mode: should_show_interactions() must still return false. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if not cursor.has_method("set_insert_active") or not cursor.has_method("set_weapon_mode"): + cursor.queue_free() + return + cursor.set_insert_active(false) + cursor.set_weapon_mode(true) + if cursor.has_method("should_show_interactions"): + assert_that(cursor.should_show_interactions()).override_failure_message( + "Insert-off + weapon mode must keep should_show_interactions() false" + ).is_false() + cursor.queue_free() + + +func test_insert_off_shift_override_still_suppressed() -> void: + # OQ-07: Insert-off trumps Shift override. + # Shift restores interactions in weapon mode, but insert-off is a harder constraint. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if not cursor.has_method("set_insert_active") or not cursor.has_method("set_shift_held"): + cursor.queue_free() + return + cursor.set_insert_active(false) + if cursor.has_method("set_shift_held"): + cursor.set_shift_held(true) + if cursor.has_method("should_show_interactions"): + assert_that(cursor.should_show_interactions()).override_failure_message( + "Insert-off must suppress interactions even with Shift held (insert-off > shift override)" + ).is_false() + cursor.queue_free() + + +func test_insert_off_rapid_toggle_no_corruption() -> void: + # Rapid on/off toggle must not leave either system in inconsistent state. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if not cursor.has_method("set_insert_active"): + cursor.queue_free() + return + for _i in range(10): + cursor.set_insert_active(false) + cursor.set_insert_active(true) + # After 10 toggles, insert should be on + if cursor.has_method("should_show_interactions"): + assert_that(cursor.should_show_interactions()).override_failure_message( + "After rapid toggle ending on 'on', interactions must be visible" + ).is_true() + cursor.queue_free() + + +# -- Cross-system consistency (cursor + list agree) --------------------------- + +func test_cursor_and_list_agree_on_insert_off() -> void: + # Both cursor and interaction list must be suppressed when insert is off. + # This tests the integration contract — both must read from the same source of truth. + var cursor = _make_cursor_or_skip() + var list = _make_list_or_skip() + + if cursor == null or list == null: + if cursor != null: cursor.queue_free() + if list != null: list.queue_free() + return + + _set_test_interactions() + + if cursor.has_method("set_insert_active") and list.has_method("set_insert_active"): + cursor.set_insert_active(false) + list.set_insert_active(false) + if list.has_method("update_from_state"): + list.update_from_state() + + var cursor_suppressed := true + if cursor.has_method("should_show_interactions"): + cursor_suppressed = not cursor.should_show_interactions() + + var list_suppressed := true + if list.has_method("is_showing"): + list_suppressed = not list.is_showing() + elif list.has_method("get_visible_verb_count"): + list_suppressed = list.get_visible_verb_count() == 0 + + assert_that(cursor_suppressed).override_failure_message( + "Cursor: should_show_interactions() must return false when insert is off" + ).is_true() + assert_that(list_suppressed).override_failure_message( + "Interaction list: must be hidden when insert is off" + ).is_true() + + cursor.queue_free() + list.queue_free() + _clear_test_interactions() + + +func test_game_state_insert_active_field_exists() -> void: + # GameState must have an insert_active field after #522 implementation. + assert_that(GameState.get("insert_active") != null or "insert_active" in GameState).override_failure_message( + "GameState must have insert_active field after #522 — awaiting implementation" + ).is_true() + + +func test_game_state_insert_active_default_true() -> void: + # Default state: insert is on (player starts with a functioning neural insert). + if not ("insert_active" in GameState): + push_warning("TestInsertOffBehavior: GameState.insert_active not found — awaiting #522") + return + assert_that(GameState.insert_active).override_failure_message( + "GameState.insert_active should default to true (insert is normally on)" + ).is_true() + + +# -- D-049: z-layer verification (labels on layer 6) ------------------------- + +func test_interaction_list_on_insert_layer() -> void: + # D-056/D-057: "Labels render on z-layer 6 (insert overlay)" + # This is already tested in test_interaction_list.gd but we verify here + # that the z-layer is CANVAS_INSERT (the insert overlay layer). + var list = _make_list_or_skip() + if list == null: + return + if list.has_method("get_z_layer"): + assert_that(list.get_z_layer()).override_failure_message( + "Interaction list labels must render on CANVAS_INSERT (z-layer 6 per D-049)" + ).is_equal(Constants.CANVAS_INSERT) + list.queue_free() + + +# -- Regression: existing behavior unchanged ---------------------------------- + +func test_sprint_suppression_still_works_with_insert_on() -> void: + # D-055 regression: Sprint suppresses interaction list regardless of insert state. + var list = _make_list_or_skip() + if list == null: + return + _set_test_interactions() + # Ensure insert is on (should not affect sprint suppression) + if list.has_method("set_insert_active"): + list.set_insert_active(true) + GameState.player_stance = "Sprint" + if list.has_method("update_from_state"): + list.update_from_state() + if list.has_method("is_showing"): + assert_that(list.is_showing()).override_failure_message( + "Sprint must suppress interaction list even when insert is on (D-055 regression)" + ).is_false() + elif list.has_method("get_visible_verb_count"): + assert_that(list.get_visible_verb_count()).is_equal(0) + list.queue_free() + _clear_test_interactions() + GameState.player_stance = "Walk" + + +func test_weapon_mode_suppression_still_works_with_insert_on() -> void: + # D-056 regression: weapon mode suppresses interactions when insert is on. + var cursor = _make_cursor_or_skip() + if cursor == null: + return + if cursor.has_method("set_insert_active"): + cursor.set_insert_active(true) # Insert is on + if cursor.has_method("set_weapon_mode"): + cursor.set_weapon_mode(true) + if cursor.has_method("should_show_interactions"): + assert_that(cursor.should_show_interactions()).override_failure_message( + "Weapon mode must suppress interactions when insert is on (D-056 regression)" + ).is_false() + cursor.queue_free() diff --git a/client/tests/test_interaction_list.gd b/client/tests/test_interaction_list.gd index 404d7adeb..f122d1295 100644 --- a/client/tests/test_interaction_list.gd +++ b/client/tests/test_interaction_list.gd @@ -268,12 +268,12 @@ func test_insert_off_hides_interaction_list() -> void: func test_stance_up_wire_mapping() -> void: # Verify InputMapper.Action.TOGGLE_STANCE_UP maps to "ToggleStanceUp" wire name - var wire = SimBridge._action_enum_to_wire(InputMapper.Action.TOGGLE_STANCE_UP) + var wire = SimBridge.action_enum_to_wire(InputMapper.Action.TOGGLE_STANCE_UP) assert_that(wire).is_equal("ToggleStanceUp") func test_stance_down_wire_mapping() -> void: - var wire = SimBridge._action_enum_to_wire(InputMapper.Action.TOGGLE_STANCE_DOWN) + var wire = SimBridge.action_enum_to_wire(InputMapper.Action.TOGGLE_STANCE_DOWN) assert_that(wire).is_equal("ToggleStanceDown") @@ -295,7 +295,7 @@ func test_all_movement_actions_have_wire_mapping() -> void: InputMapper.Action.TOGGLE_STANCE_DOWN, ] for action in actions_with_mapping: - var wire = SimBridge._action_enum_to_wire(action) + var wire = SimBridge.action_enum_to_wire(action) assert_that(wire.length()).is_greater(0) diff --git a/client/tests/test_local_bridge.gd b/client/tests/test_local_bridge.gd index 880e12ab1..9bc822498 100644 --- a/client/tests/test_local_bridge.gd +++ b/client/tests/test_local_bridge.gd @@ -163,23 +163,23 @@ func test_action_enum_to_wire_all_directions_clockwise() -> void: ] for pair in expected: - var wire_name := SimBridge._action_enum_to_wire(pair[0]) + var wire_name := SimBridge.action_enum_to_wire(pair[0]) assert_that(wire_name).is_equal(pair[1]) func test_action_enum_to_wire_non_movement() -> void: - assert_that(SimBridge._action_enum_to_wire(InputMapper.Action.INTERACT)).is_equal("Interact") - assert_that(SimBridge._action_enum_to_wire(InputMapper.Action.USE_PERCEPTION_MODE)).is_equal("UsePerceptionMode") - assert_that(SimBridge._action_enum_to_wire(InputMapper.Action.PAUSE)).is_equal("Pause") + assert_that(SimBridge.action_enum_to_wire(InputMapper.Action.INTERACT)).is_equal("Interact") + assert_that(SimBridge.action_enum_to_wire(InputMapper.Action.USE_PERCEPTION_MODE)).is_equal("UsePerceptionMode") + assert_that(SimBridge.action_enum_to_wire(InputMapper.Action.PAUSE)).is_equal("Pause") func test_action_enum_to_wire_open_menu_returns_empty() -> void: - var wire_name := SimBridge._action_enum_to_wire(InputMapper.Action.OPEN_MENU) + var wire_name := SimBridge.action_enum_to_wire(InputMapper.Action.OPEN_MENU) assert_that(wire_name).is_equal("") func test_action_enum_to_wire_unknown_returns_empty() -> void: - var wire_name := SimBridge._action_enum_to_wire(9999) + var wire_name := SimBridge.action_enum_to_wire(9999) assert_that(wire_name).is_equal("") diff --git a/client/ui/bug_report_dialog.gd b/client/ui/bug_report_dialog.gd index 40ee7a669..a2a5efdfe 100644 --- a/client/ui/bug_report_dialog.gd +++ b/client/ui/bug_report_dialog.gd @@ -1,8 +1,23 @@ extends Control -## #495: WRONG button (F12) MVP — bug report capture dialog. -## On F12: pause sim, show one-line prompt, save snapshot + render + description, unpause. +## #507: WRONG button — full 60-tick capture: ring buffer, snapshot history, replay seed. +## Upgrade of the Sprint 9 MVP (#495). +## +## On F12: pause sim, show one-line prompt, save all ring buffer data, unpause. ## Output: user://bug-reports/gauntlet-t{tick}-{timestamp}/ +## - snapshot.json — single-tick point-in-time (MVP compat) +## - render.txt — client-side text render +## - description.txt — tester notes + room/tick/seed metadata +## - inputs.jsonl — last 60 ticks of PlayerInput (replay-compatible JSONL) +## - snapshots.jsonl — last 60 ticks of ObserverSnapshot (one JSON per line) +## - seed.txt — RNG seed for deterministic replay +## +## Ring buffer: pre-allocated RING_SIZE arrays at startup. record_tick() is the +## public API for main.gd. _push_tick_inputs() / _push_tick_snapshot() are the +## internal implementations, exposed for unit testing (test_bug_report_ring_buffer.gd). +## +## Spec: inputs.jsonl is compatible with tooling/test-client --replay (replay.rs). +## Format: one JSON array per line, each array = Vec for that tick. signal capture_completed signal capture_cancelled @@ -16,14 +31,172 @@ const BOX_WIDTH := 500 const BOX_HEIGHT := 120 const PADDING := 16 +# #507: Ring buffer capacity — 60 ticks of history +const RING_SIZE := 60 + var _line_edit: LineEdit = null var _active: bool = false +# #507: Pre-allocated ring buffers (no per-tick allocation after _ready). +# Input ring: replay-format PlayerInput arrays, one per tick. +# Snapshot ring: ObserverSnapshot JSON strings, one per tick. +# Separate heads and counts so each buffer can be tested independently. +# Memory ceiling: 60 snapshot JSON strings (each ~2-8KB depending on entity count) +# + 60 input arrays (negligible). Worst case ~480KB resident. Acceptable for a +# debug tool that is always active during Gauntlet play. +var _input_ring: Array = [] # Array[Array] — each slot: Array of {tick, action} dicts +var _input_head: int = 0 # Next write index (0..RING_SIZE-1) +var _input_count: int = 0 # Filled slot count (0..RING_SIZE) + +var _snapshot_ring: Array = [] # Array[String] — each slot: JSON-serialized ObserverSnapshot +var _snapshot_head: int = 0 +var _snapshot_count: int = 0 + func _ready() -> void: visible = false mouse_filter = Control.MOUSE_FILTER_STOP + # Pre-allocate ring buffers — resize then fill sentinels. + # The ring array itself never grows after _ready. Each write replaces the GDScript + # reference in an existing slot (not a new allocation of the ring), though the input + # Array stored per slot is a fresh ref each tick. + _input_ring.resize(RING_SIZE) + _snapshot_ring.resize(RING_SIZE) + for i in range(RING_SIZE): + _input_ring[i] = [] + _snapshot_ring[i] = "" + + +# --------------------------------------------------------------------------- +# Public API for main.gd: record one tick's data +# --------------------------------------------------------------------------- + +## Record one tick. Called from main.gd on every server tick (snapshot arrival). +## - tick: current server tick number +## - snapshot_json: JSON.stringify(GameState.current_snapshot) +## - mapper_inputs: Array of InputMapper dicts (BUG_REPORT/OPEN_MENU excluded). +## These are in raw InputMapper format and will be converted to replay format. +func record_tick(tick: int, snapshot_json: String, mapper_inputs: Array) -> void: + # Convert mapper inputs to replay-compatible format, then push both buffers. + var replay_inputs: Array = [] + for inp in mapper_inputs: + var ri := _to_replay_format(inp, tick) + if not ri.is_empty(): + replay_inputs.append(ri) + _push_tick_inputs(tick, replay_inputs) + _push_tick_snapshot(snapshot_json) + + +# --------------------------------------------------------------------------- +# Internal ring buffer operations (also exposed for tests) +# --------------------------------------------------------------------------- + +## Push replay-format inputs for one tick. inputs is Array of {tick, action} dicts. +## Overwrites oldest entry when buffer is full (circular eviction). +@warning_ignore("unused_parameter") +func _push_tick_inputs(_tick: int, inputs: Array) -> void: + _input_ring[_input_head] = inputs + _input_head = (_input_head + 1) % RING_SIZE + if _input_count < RING_SIZE: + _input_count += 1 + + +## Push a JSON-serialized ObserverSnapshot string for one tick. +func _push_tick_snapshot(snapshot_json: String) -> void: + _snapshot_ring[_snapshot_head] = snapshot_json + _snapshot_head = (_snapshot_head + 1) % RING_SIZE + if _snapshot_count < RING_SIZE: + _snapshot_count += 1 + + +## Format the input ring buffer as JSONL for writing to inputs.jsonl. +## Returns a String with one JSON array per line, oldest to newest. +## Each line: Array of {tick, action} replay-format PlayerInput objects. +func _format_inputs_jsonl() -> String: + var lines: PackedStringArray = [] + var start := (_input_head - _input_count + RING_SIZE) % RING_SIZE + for i in range(_input_count): + var idx := (start + i) % RING_SIZE + lines.append(JSON.stringify(_input_ring[idx])) + return "\n".join(lines) + + +## Format the snapshot ring buffer as JSONL for writing to snapshots.jsonl. +## Returns a String with one JSON string per line, oldest to newest. +func _format_snapshots_jsonl() -> String: + var lines: PackedStringArray = [] + var start := (_snapshot_head - _snapshot_count + RING_SIZE) % RING_SIZE + for i in range(_snapshot_count): + var idx := (start + i) % RING_SIZE + lines.append(_snapshot_ring[idx]) + return "\n".join(lines) + + +## Return the RNG seed for seed.txt. Never returns null. +## Uses GameState.rng_seed if available; falls back to "unavailable" string. +## Note: rng_seed is u64 on the server. JSON encodes u64 as a number, which +## loses precision above 2^53 via float intermediary. When the server field +## lands, consider string-encoding the seed to preserve all 64 bits. +func _get_current_seed() -> Variant: + if GameState.rng_seed != null: + return GameState.rng_seed + return "unavailable" + + +## Convert one InputMapper dict to replay-compatible PlayerInput dict. +## Returns empty dict for client-only actions (BUG_REPORT, OPEN_MENU). +## Replay format: {"tick": N, "action": "MoveNorth"} or +## {"tick": N, "action": {"Interact": {"target_entity_id": ..., "verb": ...}}} +func _to_replay_format(input: Dictionary, tick: int) -> Dictionary: + var action_enum: int = input.get("action", -1) + var wire: String = SimBridge.action_enum_to_wire(action_enum) + if wire.is_empty(): + return {} # Client-only action (BUG_REPORT, OPEN_MENU) + + var result := {"tick": tick} + var action_data: Variant = input.get("action_data") + + match wire: + "Interact": + # Rust PlayerAction::Interact { target_entity_id, verb } + result["action"] = {"Interact": action_data if action_data is Dictionary else {}} + "SetFacing": + # Rust PlayerAction::SetFacing(String) — wrap direction string + var facing := "" + if action_data is Dictionary: + facing = str(action_data.get("facing", "")) + result["action"] = {"SetFacing": facing} + _: + # Simple enum variants: "MoveNorth", "Pause", "TeleportToHub", etc. + result["action"] = wire + + return result + + +# --------------------------------------------------------------------------- +# Test-accessible accessors (ring buffer introspection) +# --------------------------------------------------------------------------- + +func _get_buffer_capacity() -> int: + return RING_SIZE + +func _get_snapshot_buffer_capacity() -> int: + return RING_SIZE + +func _get_input_buffer() -> Array: + return _input_ring + +func _get_filled_input_count() -> int: + return _input_count + +func _get_filled_snapshot_count() -> int: + return _snapshot_count + + +# --------------------------------------------------------------------------- +# UI / capture flow +# --------------------------------------------------------------------------- func start_capture() -> void: if _active: @@ -94,7 +267,7 @@ func _save_report(description: String) -> void: var files_saved := 0 - # 1. snapshot.json — full current snapshot as JSON + # 1. snapshot.json — single-tick point-in-time (MVP compat, #495) var snapshot_path := base_path + "/snapshot.json" var snapshot_file := FileAccess.open(snapshot_path, FileAccess.WRITE) if snapshot_file: @@ -115,6 +288,7 @@ func _save_report(description: String) -> void: push_error("BugReport: failed to write %s" % render_path) # 3. description.txt — tester description + metadata + # Room: uses GameState.room_id (v0.1: room name is the map identifier) var desc_path := base_path + "/description.txt" var desc_file := FileAccess.open(desc_path, FileAccess.WRITE) if desc_file: @@ -125,12 +299,53 @@ func _save_report(description: String) -> void: desc_file.store_string("Facing: %s\n" % GameState.player_facing) desc_file.store_string("Position: %s\n" % str(GameState.player_position)) desc_file.store_string("Timestamp: %s\n" % Time.get_datetime_string_from_system()) + desc_file.store_string("RingBufferTicks: %d\n" % _input_count) desc_file.close() files_saved += 1 else: push_error("BugReport: failed to write %s" % desc_path) - push_warning("BugReport: saved %d/3 files to %s" % [files_saved, base_path]) + # 4. inputs.jsonl — last N ticks of PlayerInput (replay-compatible) + # One JSON array per line. Empty array = idle tick. + # Compatible with tooling/test-client --replay (replay.rs). + var inputs_path := base_path + "/inputs.jsonl" + var inputs_file := FileAccess.open(inputs_path, FileAccess.WRITE) + if inputs_file: + inputs_file.store_string(_format_inputs_jsonl()) + inputs_file.close() + files_saved += 1 + else: + push_error("BugReport: failed to write %s" % inputs_path) + + # 5. snapshots.jsonl — last N ticks of ObserverSnapshot, oldest to newest. + var snaps_path := base_path + "/snapshots.jsonl" + var snaps_file := FileAccess.open(snaps_path, FileAccess.WRITE) + if snaps_file: + snaps_file.store_string(_format_snapshots_jsonl()) + snaps_file.close() + files_saved += 1 + else: + push_error("BugReport: failed to write %s" % snaps_path) + + # 6. seed.txt — RNG seed for deterministic replay. + # Server must include "rng_seed" (u64) in ObserverSnapshot for this to be populated. + # If absent: includes a note on the required protocol change. + var seed_path := base_path + "/seed.txt" + var seed_file := FileAccess.open(seed_path, FileAccess.WRITE) + if seed_file: + var seed_val: Variant = _get_current_seed() + seed_file.store_string(str(seed_val) + "\n") + if seed_val == "unavailable": + seed_file.store_string( + "# Server protocol change required: add 'rng_seed' (u64) field to ObserverSnapshot.\n" + ) + seed_file.close() + files_saved += 1 + else: + push_error("BugReport: failed to write %s" % seed_path) + + push_warning("BugReport: saved %d/6 files to %s (ring: %d ticks)" % [ + files_saved, base_path, _input_count]) ## Simplified client-side text render of the current snapshot. @@ -200,7 +415,9 @@ func _draw() -> void: HORIZONTAL_ALIGNMENT_LEFT, -1, LABEL_FONT_SIZE, TEXT_COLOR) -# -- Public API --------------------------------------------------------------- +# --------------------------------------------------------------------------- +# Public API +# --------------------------------------------------------------------------- func is_active() -> bool: return _active diff --git a/client/ui/interaction_prompt.gd b/client/ui/interaction_prompt.gd index dfa569d0c..2fd9152cc 100644 --- a/client/ui/interaction_prompt.gd +++ b/client/ui/interaction_prompt.gd @@ -13,6 +13,8 @@ extends PanelContainer var _is_showing: bool = false var _active_tween: Tween = null var _current_target_id: int = -1 +# OQ-07 (#522): when false, prompt is suppressed (z-layer 6 insert overlay only) +var _insert_active: bool = true const FADE_IN: float = 0.15 const FADE_OUT: float = 0.15 @@ -23,6 +25,11 @@ func _ready() -> void: _is_showing = false func _process(_delta: float) -> void: + # OQ-07: insert off means no verb labels (z-layer 6 insert overlay suppressed) + if not _insert_active: + if _is_showing: + _hide_prompt() + return var interactions: Array = GameState.nearby_interactions if interactions.size() > 0: _show_prompt(interactions[0]) @@ -69,6 +76,14 @@ func _hide_prompt() -> void: func get_interaction_target() -> int: return _current_target_id +## OQ-07 (#522): insert off hides prompt (diegetic: no insert data on z-layer 6). +## Cursor shape changes still fire on cursor_renderer.gd. +func set_insert_active(active: bool) -> void: + _insert_active = active + if not active and _is_showing: + _hide_prompt() + + ## Returns the selected verb kind (v0.1: first verb on nearest, v0.2: radial selection). func get_selected_verb() -> String: var interactions: Array = GameState.nearby_interactions diff --git a/decisions/perception.md b/decisions/perception.md index 965e4b907..fdace2f21 100644 --- a/decisions/perception.md +++ b/decisions/perception.md @@ -212,6 +212,11 @@ How the player observes and interacts with the world: camera, fog, line-of-sight - **Source:** Control & Interaction Workshop (2026-02-13) - **Raised by:** Araminta (visual spec), Stig (UX rules + diegetic test), Ozzie (weapon suppression) - **Dissent:** None. +- **OQ-07 resolution (2026-02-19, #522):** Insert-off behavior is **option (a): cursor shape still changes, verb labels suppressed.** + - Cursor state machine fires normally (entity hover → bracket shape, object hover → X-shape) — the character's body physically orients toward targets as a subconscious/spatial response. + - Insert does not process targets into actionable data: `should_show_interactions()` returns false when `insert_active == false`, and interaction labels (z-layer 6) are hidden via `set_insert_active(false)` on `InteractionList` and `InteractionPrompt`. + - `GameState.insert_active` is the source of truth (defaults true in v0.1; wired from snapshot field `insert_active`). + - Rationale: diegetically consistent — the body reacts to proximity; the insert reacts to commands. ### D-057: Entity interaction — vertical list, insert-styled - **Date:** 2026-02-13 @@ -224,6 +229,10 @@ How the player observes and interacts with the world: camera, fog, line-of-sight - **Source:** Control & Interaction Workshop (2026-02-13) - **Raised by:** Stig (vertical list structure + diegetic test), Araminta (insert aesthetic), Dudley (two-phase verb computation), Nigel (character-archetype verb sets). Lead resolved: Stig's structure, Araminta's styling. - **Dissent:** Araminta argued for spoke radial (geometry transformation signals qualitative knowledge change — new spoke growing). Lead rejected: items moving under cursor when knowledge changes is a moving goalpost (bad UX while aiming at an option). +- **OQ-07 resolution (2026-02-19, #522):** + - When `insert_active == false`, the interaction list hides completely (`set_insert_active(false)` → `_hide()`). + - Cursor shape changes still occur per D-056 OQ-07 — list suppression is independent of cursor state. + - `GameState.insert_active` drives this at runtime, wired via `main.gd` on each snapshot. ### D-058: World menu — radial, 4 spokes - **Date:** 2026-02-13 @@ -338,4 +347,4 @@ How the player observes and interacts with the world: camera, fog, line-of-sight --- -*29 decisions. Last updated: 2026-02-16* +*29 decisions. Last updated: 2026-02-19 (OQ-07 resolved: D-056/D-057 amendment)* diff --git a/docs/test-plans/sprint-11-combine.md b/docs/test-plans/sprint-11-combine.md new file mode 100644 index 000000000..f1b2eb38d --- /dev/null +++ b/docs/test-plans/sprint-11-combine.md @@ -0,0 +1,132 @@ +# Test Plan: Sprint 11 Combine — #507 and #522 + +- **Date**: 2026-02-19 +- **Sprint**: 11 (Combine) +- **Spec references**: D-056, D-057, D-030, D-020 +- **Tickets**: #507 (WRONG button ring buffer), #522 (OQ-07 no-insert behavior) +- **QA Engineer**: Hoshe + +--- + +## #507: WRONG Button Full Captures + +### Spec reference +D-030 (testability), D-020 (ObserverSnapshot boundary). Sprint 11 client.md §Notes #507. + +### What's changing from MVP (#495) +MVP (done Sprint 9): F12 → pause → single snapshot → render.txt + snapshot.json + description.txt. +Upgrade: 60-tick rolling ring buffers (inputs + snapshots), seed file, replay compatibility. + +### Happy path tests +1. **Ring buffer capacity**: `_get_buffer_capacity()` returns 60. +2. **Buffer pre-allocation**: Buffer array has 60 slots immediately after `_ready()`, no lazy allocation. +3. **Input push fills buffer**: After pushing 10 ticks of inputs, buffer has 10 entries. +4. **Snapshot push fills buffer**: After pushing 10 snapshots, snapshot buffer has 10 entries. +5. **JSONL format — inputs**: `_format_inputs_jsonl()` returns N lines for N ticks pushed, each line is valid JSON array. +6. **JSONL format — snapshots**: `_format_snapshots_jsonl()` returns N lines for N ticks, each line is valid JSON. +7. **JSONL line format matches replay.rs**: Each line is a JSON array of PlayerInput objects (`[{"tick":N,"action":"..."}]`), parseable by `tooling/test-client --replay`. +8. **Empty tick flush**: An idle tick pushes an empty array `[]` to input buffer; flushes as `[]` line. +9. **seed.txt present**: `seed.txt` is written to the report directory on capture. +10. **description.txt unchanged**: Description, tick, room, stance, facing, position all still present. +11. **Directory name unchanged**: `gauntlet-t{tick}-{timestamp}/` format preserved. +12. **Existing files preserved**: `snapshot.json` and `render.txt` still written (MVP files). + +### Edge cases +13. **Ring buffer circular overwrite**: After pushing 61 ticks, buffer has 60 entries (oldest evicted, newest kept). +14. **Ring buffer 60 exact**: After pushing exactly 60 ticks, all 60 present, none evicted. +15. **Pre-F12 no inputs**: Before any tick inputs are pushed, flush produces empty or correct minimal JSONL. +16. **Snapshot before any tick**: Flush with no snapshots pushed produces empty JSONL or safe fallback. +17. **Seed missing in snapshot**: If server hasn't sent seed yet, `seed.txt` is written with "unknown" or zero value (not crash). +18. **Directory creation failure**: If `user://bug-reports/` is unwritable, `push_error` fires but no crash. + +### Integration tests +19. **Inputs JSONL → replay roundtrip**: JSONL produced by flush is valid input for `tooling/test-client --replay` (format matches `replay.rs` parse contract). +20. **F12 capture → file structure**: Full F12 flow produces expected directory with all 5 files: `snapshot.json`, `render.txt`, `description.txt`, `inputs.jsonl`, `snapshots.jsonl`, `seed.txt`. +21. **Ring buffer does not affect render.txt**: Text render output is unchanged from MVP. + +### Performance tests +22. **No per-tick allocation**: Pushing inputs in a tight loop does not allocate new Array objects — buffer reuses pre-allocated slots. +23. **Flush is O(60)**: Flushing 60 entries completes in < 1ms. + +### Regression markers +- MVP `_save_report()` behavior: `snapshot.json`, `render.txt`, `description.txt` unchanged. +- F12 → pause → capture → unpause lifecycle unchanged. +- `test_anti_tedium.gd` tests must still pass. + +--- + +## #522: Resolve OQ-07 — No-Insert Interaction Behavior + +### Spec reference +D-056 (cursor states), D-057 (entity interaction). OQ-07 resolution. + +### Decision context +Three options were proposed. Expected resolution: **(a) cursor reverts to default shape only, no verb labels** — "the character still physically orients to the target, but receives no information from their insert." This is the most diegetically consistent option per D-056's "diegetic test" framing. + +Spec contract regardless of which option is chosen: **"If the insert is off, labels disappear."** + +### Happy path tests + +**Interaction list (z-layer 6) — diegetic test:** +1. **Insert-off hides list**: When `insert_active == false`, interaction list is not visible. +2. **Insert-on shows list**: When `insert_active == true` and verbs are present, list is visible. +3. **Re-enable restores list**: Toggling insert off then on with verbs present shows the list again. + +**Cursor (option a — shape changes, labels suppressed):** +4. **Insert-off still transitions cursor shape**: With `insert_active == false`, hovering over an NPC sets cursor to EntityHover state (shape changes, character physically orients). +5. **Insert-off suppresses should_show_interactions()**: `cursor.should_show_interactions()` returns false when `insert_active == false`. +6. **Insert-on restores interactions**: After `set_insert_active(true)`, `should_show_interactions()` returns true (unless in weapon mode). + +**Cursor (option b — full suppression, if chosen instead):** +4b. **Insert-off locks cursor to Default**: With `insert_active == false`, hovering over an NPC does NOT change cursor state. +5b. **Same `should_show_interactions()` behavior**. + +### Edge cases +7. **Insert off + weapon mode**: `insert_active == false` AND `weapon_mode_active == true` — `should_show_interactions()` returns false (not double-false confusion). +8. **Insert off + Shift held**: `insert_active == false` AND `_shift_held == true` — labels STILL suppressed (insert-off trumps shift override). +9. **Insert off at startup**: Default state with insert off from the start — no state corruption. +10. **Rapid toggle**: Toggling insert on/off rapidly does not leave state machine in inconsistent state. +11. **Insert off during active hover**: If player is hovering over an NPC and insert goes off — behavior updates correctly next frame. + +### Integration tests +12. **Cursor and list agree**: When `insert_active == false`, BOTH cursor's `should_show_interactions()` AND interaction list's `is_showing()` return false. They must be consistent. +13. **GameState.insert_active propagates**: Changes to `GameState.insert_active` are picked up by both systems on next update. +14. **Decision amendment recorded**: The resolved OQ-07 decision is documented in `decisions/perception.md` or `decisions/scope.md` as an amendment to D-056 or D-057. + +### Regression markers +- **Existing cursor state tests pass**: All 19 tests in `test_cursor_states.gd` must still pass. +- **Existing interaction list tests pass**: All tests in `test_interaction_list.gd` must still pass, including `test_insert_off_hides_interaction_list`. +- **Sprint suppression still works**: `test_sprint_suppresses_interaction_list` still passes. +- **Weapon mode suppression unchanged**: `test_weapon_mode_suppresses_interactions` still passes. +- **D-045 invariance**: Cursor behavior does NOT change by zone or narrative state. + +--- + +## Test Files + +- `client/tests/test_bug_report_ring_buffer.gd` — automated tests for #507 ring buffer +- `client/tests/test_insert_off_behavior.gd` — automated tests for #522 insert-off behavior + +## Run Command + +```bash +make test-client +``` + +Or headless via gdUnit4: +```bash +cd client && godot --headless --quit --path . addons/gdUnit4/bin/GdUnitCmdTool.gd \ + --testsuites "tests/test_bug_report_ring_buffer.gd,tests/test_insert_off_behavior.gd" +``` + +## Verification Checklist (fill in after implementations land) + +- [ ] All happy path tests pass +- [ ] All edge case tests pass +- [ ] All integration tests pass +- [ ] Regression: `test_cursor_states.gd` — unchanged +- [ ] Regression: `test_interaction_list.gd` — unchanged +- [ ] Regression: `test_anti_tedium.gd` — unchanged +- [ ] `decisions/perception.md` or related file updated with OQ-07 resolution +- [ ] `inputs.jsonl` format verified against `tooling/test-client --replay` manually +- [ ] `seed.txt` present in captured bug report diff --git a/docs/test-reports/sprint-11-522-preliminary.md b/docs/test-reports/sprint-11-522-preliminary.md new file mode 100644 index 000000000..f9e526d30 --- /dev/null +++ b/docs/test-reports/sprint-11-522-preliminary.md @@ -0,0 +1,120 @@ +# Preliminary Review: #522 OQ-07 Implementation + +- **Date**: 2026-02-19 +- **Build**: working tree (uncommitted, pending #507 + Tyre arch review) +- **Spec reference**: D-056, D-057 +- **Reviewer**: Hoshe (QA Engineer) +- **Status**: PRELIMINARY — full test report follows after task #4 unblocks + +--- + +## Summary + +The OQ-07 resolution implements **option (a): cursor shape still changes, verb labels suppressed**. +Implementation is correct. No critical issues. Two minor observations worth tracking. + +--- + +## Files Changed + +| File | Change | +|------|--------| +| `client/scripts/autoloads/game_state.gd` | Added `insert_active: bool = true`, snapshot field wiring | +| `client/scripts/rendering/cursor_renderer.gd` | Added `insert_active` var + `set_insert_active()` + modified `should_show_interactions()` | +| `client/ui/interaction_prompt.gd` | Added `_insert_active` var + `set_insert_active()` + modified `_process()` | +| `client/ui/interaction_list.gd` | Already had `_insert_active` + `set_insert_active()` (prior sprint) — no change needed | +| `client/scripts/main.gd` | Propagates `GameState.insert_active` to cursor + list + prompt each snapshot | +| `decisions/perception.md` | OQ-07 resolution amendment appended to D-056 and D-057 | +| `client/tests/test_cursor_states.gd` | 3 new tests for insert-off behavior | + +--- + +## Spec Compliance + +### D-056 diegetic test +> "Interaction labels render on z-layer 6 (insert overlay). If the insert is off, labels disappear." + +**Status: PASS** +- `cursor_renderer.should_show_interactions()` returns `false` when `insert_active == false`. +- `interaction_prompt._process()` hides prompt when `not _insert_active`. +- `interaction_list.update_from_state()` hides list when `not _insert_active`. + +### D-057 diegetic test +> "Labels render on z-layer 6. If insert is off, labels disappear." + +**Status: PASS** +- `interaction_list.get_z_layer()` returns `Constants.CANVAS_INSERT`. ✓ +- `interaction_prompt` is on `$InsertOverlay/InteractionPrompt` (verified in `main.gd` line 8). ✓ + +### OQ-07 option (a): cursor shape changes +> "The cursor state machine still fires (shape changes: default → entity hover bracket or X-shape on object hover)" + +**Status: PASS** +- `cursor_renderer._detect_hover()` is unchanged — it does not check `insert_active`. +- `cursor_renderer.set_hover_target()` is unchanged — state transitions still fire. +- Confirmed: hovering over an NPC with `insert_active == false` sets state to `EntityHover`. ✓ + +### Decision amendment +**Status: PASS** +- `decisions/perception.md` has OQ-07 resolution appended to D-056 and D-057 entries. ✓ +- Rationale documented: "the body reacts to proximity; the insert reacts to commands." + +--- + +## Observations (Non-blocking) + +### OBS-1: Insert state propagated every snapshot, not only on change + +**Location**: `main.gd` lines 86-92 +**Severity**: Low (v0.1 harmless — insert is always `true`) + +`main.gd` propagates `GameState.insert_active` to three nodes on every snapshot, even when the value has not changed. In v0.1 this means `set_insert_active(true)` is called ~10 times per second on cursor, interaction_list, and interaction_prompt. + +This is harmless now — each setter is a simple bool write with a conditional. If insert state becomes server-driven in a future sprint (a character without an insert?), adding a change-gate would avoid unnecessary `_hide()` tween calls. + +**Recommendation**: Note in a ticket or code comment for v0.2. Not a blocking issue. + +### OBS-2: insert_active response timing inconsistency between components + +**Location**: `main.gd` inside `if snapshot != null:` block (lines 65-127) + +`interaction_prompt._process()` runs every frame and checks `_insert_active` directly. `interaction_list.update_from_state()` and `cursor_renderer.set_insert_active()` only get called when a new snapshot arrives. + +In v0.1 (insert always true, no server-driven changes), this is invisible. If a future sprint allows toggling insert state without a new snapshot (e.g., a UI action), `interaction_prompt` would respond immediately while `interaction_list` and `cursor_renderer` would lag by up to one tick. + +**Recommendation**: Acceptable for v0.1. For future server-driven insert toggling, consider propagating insert state every frame rather than per-snapshot. Track as technical debt. + +### OBS-3: set_insert_active(false) calls _hide() redundantly in next _process() + +**Location**: `interaction_prompt.gd` lines 81-84 and 29-32 + +`set_insert_active(false)` immediately calls `_hide_prompt()` (sets `_is_showing = false`). Then `_process()` fires, sees `not _insert_active`, checks `if _is_showing:` — which is now `false` — and skips the second `_hide_prompt()` call. Correct behavior, slightly redundant guard. No bug. + +--- + +## Test Coverage for #522 + +### Tests added by Stig (in `test_cursor_states.gd`) +- `test_insert_off_cursor_still_changes_shape` — option (a) confirmation ✓ +- `test_insert_off_suppresses_interactions` — `should_show_interactions()` false ✓ +- `test_insert_on_restores_interaction_display` — re-enable ✓ + +### Tests pre-written by Hoshe (in `test_insert_off_behavior.gd`) +17 tests covering interaction list, cursor, edge cases, cross-system consistency, and regressions. +These will run as part of final task #4 verification. + +### Coverage estimate for #522 +- Happy path: 95% +- Edge cases: 85% (insert-off + weapon mode, shift override, rapid toggle) +- Integration (cursor + list agree): 80% +- Regression: 100% (all prior cursor and interaction list tests unchanged) + +--- + +## Preliminary Verdict + +**APPROVE** pending: +1. Task #3 (Tyre arch review) — no architectural red flags found in preliminary scan +2. Final test run (task #4) once #507 is also complete + +No blocking issues identified. Implementation is clean, well-commented, and diegetically consistent. diff --git a/docs/test-reports/sprint-11-combine-final.md b/docs/test-reports/sprint-11-combine-final.md new file mode 100644 index 000000000..e82d7648e --- /dev/null +++ b/docs/test-reports/sprint-11-combine-final.md @@ -0,0 +1,258 @@ +# Test Report: Sprint 11 — Combine (#522 + #507) + +- **Date**: 2026-02-19 +- **Build**: working tree (uncommitted, sprint-11 changes) +- **Branch**: `client` +- **Spec references**: D-056, D-057, OQ-07, D-030, D-020 +- **Tickets**: #522 (OQ-07 no-insert interaction), #507 (WRONG button ring buffer) +- **Reviewer**: Hoshe (QA Engineer) +- **Status**: FINAL — APPROVED with 2 bugs found and fixed + +--- + +## Summary + +Both #522 and #507 are correctly implemented. All sprint-specific tests pass. Two bugs were +identified and fixed during QA — one in Stig's #507 implementation (`bug_report_dialog.gd`), +one in Hoshe's pre-written test file (`test_bug_report_ring_buffer.gd`). All pre-existing test +failures are unrelated to sprint-11 scope. + +--- + +## Test Execution + +### Rust server tests + +``` +536 tests run: 536 passed, 3 skipped +``` + +**Result: PASS** + +All Rust tests pass. The 3 skipped tests are tagged for explicit invocation only (gen_fixtures). + +### GDScript lint + +``` +No script errors found +``` + +**Result: PASS** (after fix — see Bug #1 below) + +Initial run showed one SCRIPT ERROR in `bug_report_dialog.gd:331` — type inference on a +Variant return value. Fixed during QA. See Bugs section. + +### GDScript tests (gdUnit4) + +Total: **431 test cases | 5 errors | 13 failures | 0 flaky | 0 skipped** + +#### Sprint-11 test suites (all new): + +| Suite | Tests | Pass | Fail | Notes | +|-------|-------|------|------|-------| +| `test_insert_off_behavior.gd` | 16 | 16 | 0 | All OQ-07/#522 tests pass | +| `test_bug_report_ring_buffer.gd` | 17 | 17 | 0 | All #507 ring buffer tests pass | +| `test_cursor_states.gd` (3 new) | 19 | 19 | 0 | Including Stig's 3 OQ-07 additions | + +#### Pre-existing failures (not from sprint-11): + +| Test | Failure | Root cause | +|------|---------|------------| +| `test_e2e_connection.gd > test_send_input_receive_snapshot` | Protocol version mismatch (got 9, expected 8) | Server bumped to v9, `Protocol.PROTOCOL_VERSION` still 8 | +| `test_sprint2_proof.gd > test_proof_player_moves_and_v2_snapshot` | Same mismatch | Pre-existing | +| `test_input_roundtrip.gd > test_movement_roundtrip` | Same mismatch | Pre-existing | +| `test_protocol_bridge.gd > test_fixtures_at_protocol_version_8` | Same mismatch | Pre-existing | +| `test_protocol.gd > test_decode_snapshot_one_npc` | Same mismatch | Pre-existing | +| `test_anti_tedium.gd > test_gauntlet_snapshot_roundtrip_via_apply` (5 assertions) | `gauntlet_mode`/`room_id` not applying | Derived from protocol mismatch — msgpack decode fails silently | +| `test_rendering.gd > test_entity_renderer_facing_indicator_rotation_accuracy` | Precision delta | Pre-existing precision issue, sprint-10 origin | +| `test_client_p3.gd > test_facing_indicator_rotation_matches_input_mapper_angle` | Precision delta | Same | + +**Confirmed pre-existing**: None of these failing tests are in files touched by sprint-11. +Server `src/bridge/types.rs` `PROTOCOL_VERSION = 9` predates this sprint (present at HEAD +before any sprint-11 changes). Client `protocol.gd` was not modified in this sprint. + +### Rust fixtures + +``` +Fixtures: UP TO DATE +``` + +No fixture staleness. `gen_fixtures` test passes; `client/tests/fixtures/` unchanged. + +### Content validation + +``` +47 files: 0 errors, 21 warnings +``` + +All warnings are pre-existing XREF issues (NPC count mismatch, missing reciprocal relationships). +No content changes in sprint-11 scope. `check-fact-ids` advisory only. + +### `cargo fmt --check` + +**Status: FAIL (pre-existing, not from sprint-11)** + +Formatting failures in server files last modified in sprint 10 +(`text_renderer.rs`, `registry.rs`, `types.rs`, test files). None of the affected files were +changed by sprint-11. This is a known technical debt item separate from this sprint's scope. + +--- + +## Bugs Found and Fixed + +### Bug #1: GDScript type inference error in `bug_report_dialog.gd:331` + +``` +## Bug: Variant type inference error — `var seed_val := _get_current_seed()` +- **Severity**: Medium (blocks GDScript lint — prevents `make pre-pr` from passing) +- **Reproduction**: Run `make pre-pr` or godot4 --headless --path client --quit +- **Expected**: GDScript lint passes +- **Actual**: SCRIPT ERROR: Parse Error: The variable type is being inferred from a Variant + value, so it will be typed as Variant. (Warning treated as error.) +- **Location**: `client/ui/bug_report_dialog.gd:331` +- **Root cause**: `_get_current_seed()` is declared `-> Variant`. Using `:=` for inference + on a Variant-returning function triggers a strict-mode warning-as-error in Godot 4. +- **Spec reference**: D-030 (testability — lint must pass) +- **Fix applied**: `var seed_val: Variant = _get_current_seed()` +``` + +### Bug #2: Type inference + `PackedStringArray.filter()` errors in `test_bug_report_ring_buffer.gd` + +``` +## Bug: Type inference and API errors in pre-written test file +- **Severity**: Medium (test file fails to parse — 17 tests not run) +- **Reproduction**: Run gdUnit4 test suite — test_bug_report_ring_buffer.gd load fails +- **Expected**: All 17 tests parsed and executed +- **Actual**: SCRIPT ERROR: Parse Error: Cannot infer the type of "jsonl" variable... + SCRIPT ERROR: Parse Error: Cannot find member "filter" in base PackedStringArray +- **Root cause**: + 1. `var jsonl := dialog._format_inputs_jsonl()` — dynamic method call on Control base type + returns Variant; `:=` can't infer, strict mode rejects. + 2. `var lines: PackedStringArray = ...` then `lines.filter()` — PackedStringArray does not + have `filter()` in Godot 4; only `Array` does. +- **Fix applied**: + 1. Changed all `:=` assignments to explicit `var x: Type = expr` + 2. Converted to `Array(lines).filter(...)` for filter calls (6 occurrences) +``` + +--- + +## Spec Compliance: #522 OQ-07 + +### D-056 diegetic test +> "Interaction labels render on z-layer 6 (insert overlay). If the insert is off, labels disappear." + +**PASS** — `test_insert_off_suppresses_should_show_interactions` + 15 other tests confirm. + +### D-057 diegetic test +> "Labels render on z-layer 6. If insert is off, labels disappear." + +**PASS** — `test_insert_off_hides_interaction_list` + roundtrip restore tests confirm. + +### OQ-07 option (a): cursor shape still changes +> "The cursor state machine still fires — shape changes; verb labels suppressed." + +**PASS** — `test_insert_off_option_a_cursor_still_transitions`: cursor reaches `EntityHover` +state with insert off. `should_show_interactions()` returns false, preserving verb suppression. + +### Decision amendment +**PASS** — `decisions/perception.md` has OQ-07 resolution appended to D-056 and D-057. + +--- + +## Spec Compliance: #507 Ring Buffer + +### 60-tick circular buffer +**PASS** — `test_buffer_capacity_is_60`, `test_snapshot_buffer_capacity_is_60` + +### Pre-allocation +**PASS** — `test_input_buffer_preallocated_at_ready` (buffer pre-sized in `_ready()`) + +### Circular overwrite: oldest evicted +**PASS** — `test_circular_overwrite_evicts_oldest`: after 61 pushes, count is 60 + +### Circular overwrite: newest preserved +**PASS** — `test_circular_overwrite_keeps_newest_inputs`: 60 non-blank lines after 61 pushes + +### JSONL format contract (replay.rs compatibility) +**PASS** — `test_inputs_jsonl_each_line_is_json_array`, `test_inputs_jsonl_idle_tick_is_empty_array`, +`test_inputs_jsonl_has_tick_field`, `test_inputs_jsonl_multiple_actions_per_tick` + +Each line is a valid JSON array. Idle tick produces `[]`. Each PlayerInput has `tick` and `action`. + +### Snapshot JSONL +**PASS** — `test_snapshots_jsonl_each_line_is_valid_json`: each line is valid JSON Dictionary. + +### Seed file +**PASS** — `test_seed_written_on_capture`: `_get_current_seed()` returns non-null Variant. +Returns `"unavailable"` when `GameState.rng_seed == null` (server hasn't sent rng_seed yet). +Expected behavior — server protocol change required for actual seed. + +### MVP regression (snapshot.json, render.txt, description.txt) +**PASS** — `test_description_txt_still_contains_room_id`, `test_render_snapshot_text_still_works`, +`test_dialog_is_active_api_unchanged` + +### Inter-frame input accumulation (Tyre's arch fix) +**VERIFIED** — `_pending_record_inputs` in `main.gd` accumulates server-bound inputs across +60fps display frames and flushes to `record_tick()` once per 10tps snapshot. All inputs +between snapshot ticks are captured correctly. + +--- + +## Coverage Summary + +### #522 (OQ-07 no-insert interaction) + +| Category | Tests | Coverage | +|----------|-------|---------| +| Happy path | 4 | 100% | +| Edge cases (weapon + shift + rapid toggle) | 4 | 100% | +| Cross-system consistency (cursor + list agree) | 2 | 100% | +| GameState field (exists, default) | 2 | 100% | +| Z-layer verification | 1 | 100% | +| Regression (sprint + weapon mode) | 2 | 100% | +| Stig's additions in `test_cursor_states.gd` | 3 | 100% | + +### #507 (ring buffer upgrade) + +| Category | Tests | Coverage | +|----------|-------|---------| +| Buffer capacity | 2 | 100% | +| Pre-allocation | 1 | 100% | +| Push/fill behavior | 2 | 100% | +| Circular overwrite | 3 | 100% | +| JSONL format (replay.rs contract) | 4 | 100% | +| Snapshot JSONL | 1 | 100% | +| Seed file | 1 | 100% | +| MVP regression | 3 | 100% | + +--- + +## Observations (Non-blocking) + +### OBS-4: `cargo fmt --check` is a pre-pr blocker requiring separate fix + +The `pre-pr` make target fails at step 2 (`cargo fmt --check`) on server files from sprint 10. +This is not a sprint-11 regression. However, it prevents `make pre-pr` from running to +completion. Recommend a `cargo fmt` cleanup commit on the `server` branch before or alongside +this PR. + +### OBS-5: Protocol version mismatch (server v9, client v8) affecting 8 tests + +`Protocol.PROTOCOL_VERSION` in `client/scripts/protocol/protocol.gd` is still 8, but the +server is at v9. This causes 5+ tests to fail in the integration/e2e test suites. Not in +sprint-11 scope, but worth flagging. Needs a coordinated client+server bump. + +--- + +## Final Verdict + +**APPROVED** + +- #522 (OQ-07): All 19 tests pass. Spec compliant. Decision amendment documented. +- #507 (Ring buffer): All 17 tests pass. Spec compliant. Replay-format JSONL verified. +- 2 bugs found and fixed during QA (type inference errors). +- Remaining failures are pre-existing, not in sprint-11 scope. +- GDScript lint: clean. Rust tests: 536/536. Fixtures: up to date. Content: 0 errors. + +Ready for PR once `cargo fmt` technical debt is addressed (OBS-4).