fix(client): address PR #40 review — 13 suggestions

- Rename _insert to insert_state in main.gd (Hoshe #1)
- Add after_each() to test_bug_report_ring_buffer.gd for GameState
  cleanup on assertion failure (Hoshe #2)
- Add after_each() to test_insert_off_behavior.gd for stance/interaction
  restore on assertion failure (Hoshe #3)
- Fix assertion message: "unknown" → "unavailable" (Hoshe #4)
- Document memory ceiling of 60 JSON snapshots in ring buffer (Hoshe #5)
- Add precision warning for u64 rng_seed via JSON float (Hoshe #6)
- Promote _action_enum_to_wire to public action_enum_to_wire (Tyre #1)
- Add @warning_ignore for unused _tick parameter (Tyre #5)
- Document insert_active assumption for future no-insert characters (Tyre #4)
- Restructure OQ-07 decision amendments as bullet points (Tyre #7)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-02-19 12:19:41 +01:00
co-authored by Claude Opus 4.6
parent cf8a4922e5
commit 44d9941f04
11 changed files with 63 additions and 27 deletions
+3 -1
View File
@@ -42,7 +42,9 @@ var gauntlet_mode: bool = false # true when snapshot includes gauntlet_mode fla
# 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: always true (both detective and smuggler have inserts). Server may send false in future.
# 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.
+4 -4
View File
@@ -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"
+4 -4
View File
@@ -84,13 +84,13 @@ func _process(_delta: float) -> void:
# 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 := GameState.insert_active
var insert_state := GameState.insert_active
if cursor_renderer and cursor_renderer.has_method("set_insert_active"):
cursor_renderer.set_insert_active(_insert)
cursor_renderer.set_insert_active(insert_state)
if interaction_list and interaction_list.has_method("set_insert_active"):
interaction_list.set_insert_active(_insert)
interaction_list.set_insert_active(insert_state)
if interaction_prompt and interaction_prompt.has_method("set_insert_active"):
interaction_prompt.set_insert_active(_insert)
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
+1 -1
View File
@@ -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("")
+16 -1
View File
@@ -18,6 +18,21 @@ var BugReportDialogScript = load("res://ui/bug_report_dialog.gd")
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:
@@ -285,7 +300,7 @@ func test_seed_written_on_capture() -> void:
# 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 'unknown' if seed unavailable)"
"_get_current_seed() must not return null (use 0 or 'unavailable' if seed unavailable)"
).is_true()
+2 -2
View File
@@ -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()
+7
View File
@@ -16,6 +16,13 @@ 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:
+3 -3
View File
@@ -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)
+6 -6
View File
@@ -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("")
+8 -3
View File
@@ -41,6 +41,9 @@ var _active: bool = false
# 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)
@@ -91,8 +94,7 @@ func record_tick(tick: int, snapshot_json: String, mapper_inputs: Array) -> void
## Push replay-format inputs for one tick. inputs is Array of {tick, action} dicts.
## Overwrites oldest entry when buffer is full (circular eviction).
## tick param is unused here — each input dict carries its own tick field.
## Kept in the signature so tests and callers can pass tick for symmetry with record_tick().
@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
@@ -133,6 +135,9 @@ func _format_snapshots_jsonl() -> String:
## 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
@@ -145,7 +150,7 @@ func _get_current_seed() -> Variant:
## {"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)
var wire: String = SimBridge.action_enum_to_wire(action_enum)
if wire.is_empty():
return {} # Client-only action (BUG_REPORT, OPEN_MENU)
+9 -2
View File
@@ -212,7 +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.** The 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. But the insert does not process these 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.
- **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
@@ -225,7 +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 — the list suppression is independent of cursor state. `GameState.insert_active` drives this at runtime, wired via `main.gd` on each snapshot.
- **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