jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(ui): clear color registry on conversation end, add speaker color tests (#573)

Browse Source 
- Reset _npc_entity_colors/_npc_entity_id/_next_npc_color in _end_player_conversation()
  to prevent palette exhaustion across long sessions with many unique NPCs
- Re-enforce contrast floor after passive desaturation (_enforce_contrast after _desaturate)
- Add TestDialogueSpeakerColors suite: palette allocation, entity reuse, reset, fallback

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
jpmschweitzer
2026-03-04 09:50:02 +01:00
co-authored by Claude Sonnet 4.6
parent 974bb2b28e
commit d30ab62bc0
2 changed files with 168 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/tests/test_dialogue_speaker_colors.gd
+153
View File
@@ -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()
client/ui/dialogue_box.gd
+15 -3
View File
@@ -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