diff --git a/client/tests/test_dialogue_speaker_colors.gd b/client/tests/test_dialogue_speaker_colors.gd new file mode 100644 index 000000000..356a809ad --- /dev/null +++ b/client/tests/test_dialogue_speaker_colors.gd @@ -0,0 +1,153 @@ +## Sprint 23 #573: dialogue speaker color binding tests. +## +## Verifies entity-ID-bound speaker color assignment in dialogue_box.gd: +## round-robin palette allocation, same-entity reuse, conversation-end reset, +## and fallback behavior when no entity ID is provided. +## +## D-030: fixture-based, server-free, no subprocess required. +class_name TestDialogueSpeakerColors +extends GdUnitTestSuite + + +func _make_dialogue_box() -> Control: + if not ResourceLoader.exists("res://ui/dialogue_box.tscn"): + push_warning("TestDialogueSpeakerColors: dialogue_box.tscn not found — scene tests skipped") + return null + var node: Control = load("res://ui/dialogue_box.tscn").instantiate() + add_child(node) + return node + + +func before_test() -> void: + GameState.dialogue_active = false + + +func after_test() -> void: + GameState.dialogue_active = false + + +# -- _assign_npc_color: round-robin assignment --------------------------------- + +func test_assign_npc_color_returns_palette_color() -> void: + ## First call for an entity ID should return a color from the NPC palette. + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + var color: Color = box._assign_npc_color(100) + assert_that(color).override_failure_message( + "_assign_npc_color must return a non-default color for a valid entity ID (#573)" + ).is_not_equal(box._speech_color) + + +func test_assign_npc_color_same_entity_returns_same_color() -> void: + ## Repeated calls for the same entity ID must return the same color. + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + var color1: Color = box._assign_npc_color(200) + var color2: Color = box._assign_npc_color(200) + assert_that(color1).override_failure_message( + "_assign_npc_color must return the same color for the same entity ID (#573)" + ).is_equal(color2) + + +func test_assign_npc_color_different_entities_get_different_colors() -> void: + ## Different entity IDs should get different colors (within palette size). + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + var color1: Color = box._assign_npc_color(300) + var color2: Color = box._assign_npc_color(301) + assert_that(color1).override_failure_message( + "Different entity IDs must get different palette colors (#573)" + ).is_not_equal(color2) + + +func test_assign_npc_color_negative_id_returns_speech_color() -> void: + ## Negative entity ID (no entity) should fall back to _speech_color. + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + var color: Color = box._assign_npc_color(-1) + assert_that(color).override_failure_message( + "_assign_npc_color(-1) must return _speech_color fallback (#573)" + ).is_equal(box._speech_color) + + +# -- Color registry cleared on conversation end -------------------------------- + +func test_color_registry_cleared_on_conversation_end() -> void: + ## After hide_dialogue(), the color registry must be empty so next + ## conversation starts fresh (avoids palette exhaustion). + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + box.show_dialogue("NPC", "Hello.", [], 400) + assert_bool(box._npc_entity_colors.has(400)).override_failure_message( + "Entity color should be registered during conversation (#573)" + ).is_true() + box.hide_dialogue() + assert_bool(box._npc_entity_colors.is_empty()).override_failure_message( + "_npc_entity_colors must be cleared after conversation ends (#573)" + ).is_true() + assert_int(box._next_npc_color).override_failure_message( + "_next_npc_color must reset to 0 after conversation ends (#573)" + ).is_equal(0) + + +func test_color_registry_reset_gives_fresh_assignment() -> void: + ## After conversation end + new conversation, same entity ID gets a color + ## (may differ from previous conversation — that's fine, per-conversation). + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + box.show_dialogue("NPC", "Hello.", [], 500) + var color1: Color = box._npc_entity_colors.get(500, Color.BLACK) + box.hide_dialogue() + box.show_dialogue("NPC", "Hi again.", [], 500) + var color2: Color = box._npc_entity_colors.get(500, Color.BLACK) + # Both should be valid palette colors (not BLACK fallback) + assert_that(color1).override_failure_message( + "First conversation color must be a palette color (#573)" + ).is_not_equal(Color.BLACK) + assert_that(color2).override_failure_message( + "Second conversation color must be a palette color (#573)" + ).is_not_equal(Color.BLACK) + + +# -- show_dialogue entity ID threading ---------------------------------------- + +func test_show_dialogue_registers_npc_color() -> void: + ## show_dialogue with a valid entity ID must register the color. + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + box.show_dialogue("Kael", "Welcome.", [], 600) + assert_bool(box._npc_entity_colors.has(600)).override_failure_message( + "show_dialogue must register entity color when npc_entity_id provided (#573)" + ).is_true() + + +func test_show_dialogue_without_entity_id_no_registration() -> void: + ## show_dialogue without entity ID should not register any color. + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + box.show_dialogue("NPC", "Hello.", []) + assert_bool(box._npc_entity_colors.is_empty()).override_failure_message( + "show_dialogue without entity ID must not register colors (#573)" + ).is_true() + + +# -- append_dialogue_response defensive guard ---------------------------------- + +func test_append_dialogue_response_registers_color_if_missing() -> void: + ## append_dialogue_response with a valid entity_id must register the color + ## even if show_dialogue was not called first (defensive guard). + var box := _make_dialogue_box() + if box == null: return + auto_free(box) + box.append_dialogue_response("Voss", "I see.", 700) + assert_bool(box._npc_entity_colors.has(700)).override_failure_message( + "append_dialogue_response must register color for unknown entity ID (#573)" + ).is_true() diff --git a/client/ui/dialogue_box.gd b/client/ui/dialogue_box.gd index da30b0698..fee49943d 100644 --- a/client/ui/dialogue_box.gd +++ b/client/ui/dialogue_box.gd @@ -48,6 +48,8 @@ var _npc_name: String = "" # -- Entity color registry (#573) -- # Maps entity_id → Color for dialogue participants. # Assigned from _npc_colors palette on first encounter; player uses _player_color. +# v0.1: colors are per-conversation — cleared in _end_player_conversation() to avoid +# palette exhaustion (8 entries) across long sessions with 9+ NPCs. var _npc_entity_colors: Dictionary = {} # entity_id -> Color var _npc_entity_id: int = -1 # Entity ID of the current player conversation NPC var _next_npc_color: int = 0 # Round-robin palette index for client-side assignment @@ -194,6 +196,7 @@ func _update_layout() -> void: ## is_passive: true for overheard NPC-NPC (renders with ┃ prefix + desaturated). ## Active conversation entries are pinned (no timeout) while _in_player_conversation. ## speaker_entity_id/target_entity_id: optional entity IDs for stable color lookup (#573). +## TODO Phase 2: 6 positional params is unwieldy — consider dictionary-options overload. func append_line(speaker: String, target: String, text: String, is_passive: bool = false, speaker_entity_id: int = -1, target_entity_id: int = -1) -> void: var pinned := not is_passive and _in_player_conversation var entry: Dictionary = { @@ -280,6 +283,8 @@ func append_player_line(target_npc: String, text: String) -> void: ## Append an NPC follow-up line (from dialogue_response). +## Note: expects show_dialogue() to have been called first to set _npc_entity_id. +## Defensive: if entity_id is valid but not yet registered, _assign_npc_color handles it. func append_dialogue_response(npc_name: String, text: String, entity_id: int = -1) -> void: if entity_id >= 0: _assign_npc_color(entity_id) @@ -335,6 +340,11 @@ func _end_player_conversation() -> void: entry.timestamp_msec = now _log_dirty = true + # #573: Clear per-conversation color registry to avoid palette exhaustion + _npc_entity_colors.clear() + _npc_entity_id = -1 + _next_npc_color = 0 + # D-069: Clear dialogue/confrontation dip — coordinator routes to AudioManager audio_dip_cleared.emit() @@ -544,10 +554,10 @@ func _format_entry(entry: Dictionary, alpha: float) -> String: var text: String = _escape_bbcode(entry.text) var is_passive: bool = entry.is_passive - # Desaturate passive name colours (Araminta review) + # Desaturate passive name colours (Araminta review), re-enforce contrast floor after if is_passive: - speaker_color = _desaturate(speaker_color, PASSIVE_DESATURATION) - target_color = _desaturate(target_color, PASSIVE_DESATURATION) + speaker_color = _enforce_contrast(_desaturate(speaker_color, PASSIVE_DESATURATION)) + target_color = _enforce_contrast(_desaturate(target_color, PASSIVE_DESATURATION)) var sc := _color_with_alpha(speaker_color, alpha) var ac := _color_with_alpha(_arrow_color, alpha) @@ -576,6 +586,8 @@ static func _escape_bbcode(text: String) -> String: ## Assign a palette color to an NPC entity ID on first encounter (#573). ## Returns the same color on subsequent calls for the same entity ID. +## TODO D-033 Phase 2: derive from relationship color — current independent palette +## will need alignment when relationship-based entity colors arrive. func _assign_npc_color(entity_id: int) -> Color: if entity_id < 0: return _speech_color