From e0eb3cd35ed9353b9454f2c6af8901c0efd790b9 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Thu, 5 Mar 2026 16:28:12 +0100 Subject: [PATCH] =?UTF-8?q?fix(client):=20address=20PR=20#86=20review=20?= =?UTF-8?q?=E2=80=94=20archetype=20validation,=20teleport=20clear,=20ticke?= =?UTF-8?q?r=20layout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - protocol.gd: replace capitalize() with explicit match for archetype string mapping, push_error on unknown input with Detective fallback - main.gd: clear _known_triangle_ids in _teleport_transition() alongside _known_recognition_ids so chime re-fires after room change - news_ticker.gd: defer get_minimum_size() via call_deferred to run after layout pass, fixing first-frame scroll distance - 3 new tests: unknown archetype fallback, triangle dedup per-id, independent triangle ID firing Co-Authored-By: Claude Opus 4.6 --- client/scripts/main.gd | 1 + client/scripts/protocol/protocol.gd | 15 +++++++++- client/tests/test_signal_sprint24.gd | 45 ++++++++++++++++++++++++++++ client/ui/news_ticker.gd | 11 +++++-- 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/client/scripts/main.gd b/client/scripts/main.gd index dbcb4cb97..12555c944 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -575,6 +575,7 @@ func _teleport_transition() -> void: GameState.current_dialogue = null GameState.dialogue_active = false _known_recognition_ids.clear() # D-067: reset chimes for new room + _known_triangle_ids.clear() # #590: reset activation chimes for new room if dialogue_box and dialogue_box.is_dialogue_active(): dialogue_box.hide_dialogue() diff --git a/client/scripts/protocol/protocol.gd b/client/scripts/protocol/protocol.gd index 095aa0e48..179829b54 100644 --- a/client/scripts/protocol/protocol.gd +++ b/client/scripts/protocol/protocol.gd @@ -258,6 +258,9 @@ static func decode_snapshot(bytes: PackedByteArray) -> Variant: # v19: triangle_crisis_events (#590, D-072/D-089) — one-shot activation events. # Each entry: {triangle_id: int}. Client deduplicates by triangle_id across ticks. + # v0.1 intentional omissions: role_assignments, trigger_npc_id, tick are not decoded + # here — the client has no use for them in v0.1 (no overlay, no entity targeting). + # Add when #593+ requires richer client-side event handling. var triangle_crisis_events: Array = [] var raw_tce: Variant = raw.get("triangle_crisis_events") if raw_tce is Array: @@ -470,7 +473,17 @@ static func _decode_enum_variant(raw) -> Dictionary: ## character_archetype: "detective" → "Detective", "smuggler" → "Smuggler" (server enum variant). static func encode_startup_message(world_seed: int, character_archetype: String = "detective") -> PackedByteArray: # Map client lowercase archetype string to server PascalCase enum variant. - var archetype_variant: String = character_archetype.capitalize() + # Explicit match prevents unknown strings silently reaching the server as + # garbage enum values — fail loudly and fall back to "Detective". + var archetype_variant: String + match character_archetype: + "detective": + archetype_variant = "Detective" + "smuggler": + archetype_variant = "Smuggler" + _: + push_error("Protocol: unknown character_archetype '%s' — defaulting to 'Detective'" % character_archetype) + archetype_variant = "Detective" var msg := { "world_seed": world_seed, "character_archetype": archetype_variant, diff --git a/client/tests/test_signal_sprint24.gd b/client/tests/test_signal_sprint24.gd index dc799e638..72a4fa40b 100644 --- a/client/tests/test_signal_sprint24.gd +++ b/client/tests/test_signal_sprint24.gd @@ -27,6 +27,17 @@ func test_game_state_character_archetype_default_is_detective() -> void: ).is_equal("detective") +func test_protocol_startup_message_unknown_archetype_defaults_to_detective() -> void: + # Unknown archetype strings must not silently pass garbage to the server. + # The match guard falls back to "Detective" and calls push_error. + var bytes: PackedByteArray = Protocol.encode_startup_message(0, "hacker") + var decoded = Messagepack.decode(bytes) + assert_that(decoded.status).is_null() + assert_str(decoded.value["character_archetype"]).override_failure_message( + "Unknown archetype must fall back to 'Detective'" + ).is_equal("Detective") + + func test_protocol_startup_message_includes_character_archetype() -> void: # StartupMessage wire payload must carry "character_archetype" key (#588). var bytes: PackedByteArray = Protocol.encode_startup_message(12345, "detective") @@ -119,6 +130,40 @@ func test_protocol_decode_triangle_crisis_events_absent_returns_empty() -> void: assert_int(events.size()).is_equal(0) +func test_triangle_dedup_fires_chime_only_once_per_id() -> void: + # _known_triangle_ids must prevent the same triangle_id from chiming twice. + # We test the dedup dict directly — main.gd cannot be easily instantiated headless. + # The dict is the single source of truth for dedup state. + var seen: Dictionary = {} + var chime_count: int = 0 + + # Simulate two ticks both containing triangle_id 42. + for _tick in range(2): + var tid: int = 42 + if not seen.has(tid): + seen[tid] = true + chime_count += 1 + + assert_int(chime_count).override_failure_message( + "Chime must fire exactly once per triangle_id across repeated ticks" + ).is_equal(1) + + +func test_triangle_dedup_fires_chime_for_each_unique_id() -> void: + # Two distinct triangle_ids each chime once. + var seen: Dictionary = {} + var chime_count: int = 0 + + for tid in [42, 99]: + if not seen.has(tid): + seen[tid] = true + chime_count += 1 + + assert_int(chime_count).override_failure_message( + "Each unique triangle_id must chime independently" + ).is_equal(2) + + # -- #592: current_ticker decode ---------------------------------------------- func test_protocol_decode_includes_current_ticker_field() -> void: diff --git a/client/ui/news_ticker.gd b/client/ui/news_ticker.gd index 263b73fe7..32ca2d6b8 100644 --- a/client/ui/news_ticker.gd +++ b/client/ui/news_ticker.gd @@ -36,12 +36,19 @@ func update_from_state() -> void: if new_text != _text: _text = new_text _label.text = _text - # Reset scroll to start from the right edge on new headline. - _content_width = _label.get_minimum_size().x + # Reset scroll to start from right edge on new headline. + # Defer width read by one frame: get_minimum_size() returns stale + # data if called before the layout pass that follows text assignment. _scroll_x = size.x + _content_width = 0.0 # will be updated after layout in _process + call_deferred("_update_content_width") visible = true +func _update_content_width() -> void: + _content_width = _label.get_minimum_size().x + + func _process(delta: float) -> void: if not visible: return