fix(ui): address PR #52 review — 10 items across Hoshe, Tyre, Araminta (#535)

Critical: bump PROTOCOL_VERSION 8→9 for conversation_events/ended fields.

Hoshe review:
- Dirty flag (_log_dirty) prevents per-frame O(n) BBCode rebuild
- BBCode injection: _escape_bbcode() replaces [ with [lb] on server text
- D-064 regression: dialogue_active cleared in fade callback, not before
- YAML quoting: remove unnecessary quotes from numeric values

Tyre review:
- Carry-forward for dialogue_response, conversation_events, conversation_ended
  in receive_bytes() — arrays merge, scalar falls through
- pause_requested/unpause_requested signals route through main.gd input
  recording (_pending_record_inputs) for #507 replay determinism
- Fix version comments: dialogue_response is v8 (#305), not v9
- Remove dead _active_overheard dictionary

Araminta review:
- Passive lines: ┃ glyph prefix + _desaturate() for name colours
- Active conversation entries pinned (no timeout), unpinned with timestamp
  reset on conversation end
- _enforce_contrast(): minimum luminance floor for name colour readability
- Simplified 1-on-1 attribution: "Speaker:" instead of "Speaker → You:"

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-02-20 20:51:40 +01:00
co-authored by Claude Opus 4.6
parent 2edc7c3098
commit 952f994d59
6 changed files with 170 additions and 51 deletions
+3 -3
View File
@@ -37,11 +37,11 @@ speech_color: "#c8d0e0"
# Base opacity multiplier for non-player-centric lines.
# 1.0 = full opacity, 0.0 = invisible.
# ============================================================
passive_opacity: "0.9"
passive_opacity: 0.9
# ============================================================
# ENTRY TIMING
# All entry types share the same lifetime and fade duration.
# ============================================================
entry_lifetime_seconds: "15.0"
entry_fade_seconds: "3.0"
entry_lifetime_seconds: 15.0
entry_fade_seconds: 3.0
+4 -2
View File
@@ -59,10 +59,12 @@ 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}]
# v8 fields (#305, D-028): NPC follow-up after player dialogue choice
var dialogue_response: Variant = null # {line_id, text, speaker_entity_id}
# v9 fields (#535, D-078): Overheard NPC-to-NPC conversations
var conversation_events: Array = [] # [{speaker_id, target_id, speaker_name, target_name, occluded_line}]
var conversation_ended: Array = [] # [{speaker_id, target_id}]
var dialogue_response: Variant = null # {line_id, text, speaker_entity_id} — NPC follow-up after player choice
# #126, D-018: Medium-range sound events for fog-edge directional indicators.
# Format: [{x, y, event_type, range_category}] — server sends current medium events per tick.
@@ -186,7 +188,7 @@ func apply_snapshot(snapshot: Dictionary) -> void:
else:
conversation_ended = []
# v9: dialogue_response (#535, D-028) — NPC follow-up after player choice
# v8: dialogue_response (#305, D-028) — NPC follow-up after player choice
if snapshot.has("dialogue_response") and snapshot.dialogue_response is Dictionary:
dialogue_response = snapshot.dialogue_response
else:
+11
View File
@@ -241,6 +241,17 @@ func receive_bytes(bytes: PackedByteArray) -> void:
snapshot["current_monologue"] = _last_snapshot["current_monologue"]
if snapshot.get("current_dialogue") == null and _last_snapshot.get("current_dialogue") != null:
snapshot["current_dialogue"] = _last_snapshot["current_dialogue"]
# #535: Carry forward one-shot dialogue events (arrays merge, scalar falls through)
if snapshot.get("dialogue_response") == null and _last_snapshot.get("dialogue_response") != null:
snapshot["dialogue_response"] = _last_snapshot["dialogue_response"]
var old_conv_events: Array = _last_snapshot.get("conversation_events", [])
if old_conv_events.size() > 0:
var new_conv_events: Array = snapshot.get("conversation_events", [])
snapshot["conversation_events"] = old_conv_events + new_conv_events
var old_conv_ended: Array = _last_snapshot.get("conversation_ended", [])
if old_conv_ended.size() > 0:
var new_conv_ended: Array = snapshot.get("conversation_ended", [])
snapshot["conversation_ended"] = old_conv_ended + new_conv_ended
_last_snapshot = snapshot
# Drain the outbound buffer. Returns raw input entries for batch encoding.
+16
View File
@@ -62,6 +62,8 @@ func _ready() -> void:
dialogue_box.option_selected.connect(_on_dialogue_option_selected)
dialogue_box.dialogue_dismissed.connect(_on_dialogue_dismissed)
dialogue_box.confrontation_monologue.connect(_on_confrontation_monologue)
dialogue_box.pause_requested.connect(_on_dialogue_pause_requested)
dialogue_box.unpause_requested.connect(_on_dialogue_unpause_requested)
# #496: Print gauntlet session summary on disconnect
if gauntlet_hud:
@@ -389,6 +391,20 @@ func _on_confrontation_monologue(text: String, duration: float) -> void:
monologue_display.show_monologue(text, duration, 3, true)
# D-061: Auto-pause on dialogue open — routed through input recording (#507, Tyre #3)
func _on_dialogue_pause_requested() -> void:
var input := {"action": InputMapper.Action.PAUSE, "timestamp_msec": Time.get_ticks_msec()}
SimBridge.send_input(input)
_pending_record_inputs.append(input)
# D-061: Auto-unpause on dialogue close — routed through input recording (#507, Tyre #3)
func _on_dialogue_unpause_requested() -> void:
var input := {"action": InputMapper.Action.UNPAUSE, "timestamp_msec": Time.get_ticks_msec()}
SimBridge.send_input(input)
_pending_record_inputs.append(input)
# D-064: Handle walk-away → send WalkAway{npc_id} to server
func _on_dialogue_dismissed() -> void:
SimBridge.send_input({
+1 -1
View File
@@ -11,7 +11,7 @@ class_name Protocol
## Protocol version — must match server PROTOCOL_VERSION in bridge/types.rs.
## Reject snapshots where version != this value.
const PROTOCOL_VERSION: int = 8
const PROTOCOL_VERSION: int = 9
# -- Decode: bytes from server → GDScript types --------------------------------
+135 -45
View File
@@ -7,20 +7,25 @@ extends Control
# InsertOverlay (CanvasLayer 10, z-layer 6) — diegetic, insert-styled.
# D-063: Confrontation options render italic, trigger monologue beat before send.
# D-064: Walk-away via WASD dismisses active conversation (log entries persist).
# D-078: Overheard NPC-NPC lines shown at reduced opacity, no response options.
# dialogue_active held until fade completes (D-064 auto-pause spec).
# D-078: Overheard NPC-NPC lines prefixed with ┃ glyph + desaturated colours.
signal option_selected(response_id: String, text: String)
signal dialogue_dismissed # Walk-away or conversation end
signal confrontation_monologue(text: String, duration: float) # D-063: beat monologue
signal pause_requested # D-061: auto-pause — main.gd routes through input recording (#507)
signal unpause_requested # D-061: auto-unpause
@onready var panel: PanelContainer = $PanelContainer
@onready var dialogue_log: RichTextLabel = $PanelContainer/MarginContainer/VBoxContainer/DialogueLog
@onready var options_container: VBoxContainer = $PanelContainer/MarginContainer/VBoxContainer/OptionsContainer
# -- Log state --
var _log_entries: Array[Dictionary] = [] # {speaker, target, text, is_passive, timestamp_msec}
var _active_overheard: Dictionary = {} # "speaker_id:target_id" -> true
# Entry format: {speaker, target, text, is_passive, pinned, timestamp_msec}
# pinned entries do not expire (active conversation lines, Araminta review).
var _log_entries: Array[Dictionary] = []
var _in_player_conversation: bool = false
var _log_dirty: bool = false # Dirty flag — prevents per-frame O(n) BBCode rebuild (Hoshe #1)
# -- Option state --
var _option_controls: Array[Control] = []
@@ -54,6 +59,9 @@ const CONFRONTATION_BEAT_DURATION: float = 1.5 # D-063: pause before sending
const CONFRONTATION_DIM_ALPHA: float = 0.7 # D-063: dialogue box dims during beat
const CONFRONTATION_MONOLOGUE_KEY: String = "dialogue.confrontation_beat"
const PLAYER_NAME: String = "You"
const PASSIVE_GLYPH: String = "\u2503 " # ┃ prefix for overheard lines (Araminta review)
const PASSIVE_DESATURATION: float = 0.4 # Desaturate passive name colours by this factor
const MIN_CONTRAST_LUMINANCE: float = 0.25 # Floor for name colour brightness against dark BG
# D-064: movement actions that trigger walk-away
const _WALK_AWAY_ACTIONS: Array[StringName] = [
@@ -74,6 +82,9 @@ func _ready() -> void:
func _process(_delta: float) -> void:
_expire_entries()
if _log_dirty:
_log_dirty = false
_rebuild_log()
func _unhandled_input(event: InputEvent) -> void:
@@ -152,16 +163,19 @@ func _update_layout() -> void:
## Append a dialogue line to the log.
## speaker/target: display names. text: the spoken line.
## is_passive: true for overheard NPC-NPC (renders at reduced opacity).
## is_passive: true for overheard NPC-NPC (renders with ┃ prefix + desaturated).
## Active conversation entries are pinned (no timeout) while _in_player_conversation.
func append_line(speaker: String, target: String, text: String, is_passive: bool = false) -> void:
var pinned := not is_passive and _in_player_conversation
_log_entries.append({
"speaker": speaker,
"target": target,
"text": text,
"is_passive": is_passive,
"pinned": pinned,
"timestamp_msec": Time.get_ticks_msec(),
})
_rebuild_log()
_log_dirty = true
_ensure_visible()
@@ -172,18 +186,12 @@ func append_conversation_event(event: Dictionary) -> void:
var text: String = event.get("occluded_line", "")
if text.is_empty():
return
var pair_key := "%s:%s" % [event.get("speaker_id", 0), event.get("target_id", 0)]
_active_overheard[pair_key] = true
append_line(speaker, target, text, true)
## Handle conversation_ended — remove pair from active tracking.
func on_conversation_ended(event: Dictionary) -> void:
var pair_key := "%s:%s" % [event.get("speaker_id", 0), event.get("target_id", 0)]
_active_overheard.erase(pair_key)
# Also try the reverse pair
var reverse_key := "%s:%s" % [event.get("target_id", 0), event.get("speaker_id", 0)]
_active_overheard.erase(reverse_key)
## Handle conversation_ended — no-op currently (entries expire via timeout).
func on_conversation_ended(_event: Dictionary) -> void:
pass
## Append the player's chosen response to the log.
@@ -221,23 +229,36 @@ func show_dialogue(npc_name: String, speech: String, options: Array = []) -> voi
# D-069: Dialogue dip
AudioManager.apply_dip("dialogue")
# D-061: auto-pause in single-player
SimBridge.send_input({"action": InputMapper.Action.PAUSE, "timestamp_msec": Time.get_ticks_msec()})
# D-061: auto-pause — signal to main.gd for input recording (#507)
pause_requested.emit()
## End active player conversation — clears options but preserves log.
## D-064: dialogue_active held until fade completes.
func _end_player_conversation() -> void:
_in_player_conversation = false
_clear_options()
mouse_filter = Control.MOUSE_FILTER_IGNORE
# Unpin all entries and reset their timestamps so timeout starts now (Araminta review)
var now := Time.get_ticks_msec()
for entry in _log_entries:
if entry.pinned:
entry.pinned = false
entry.timestamp_msec = now
_log_dirty = true
# D-069: Clear dialogue/confrontation dip
AudioManager.clear_dip()
# D-061: unpause
SimBridge.send_input({"action": InputMapper.Action.UNPAUSE, "timestamp_msec": Time.get_ticks_msec()})
# D-061: unpause — signal to main.gd for input recording (#507)
unpause_requested.emit()
GameState.dialogue_active = false
# D-064: dialogue_active stays true — cleared after fade completes in hide callback.
# If log entries still exist, panel stays visible and entries expire via timeout.
# If no entries, hide immediately with fade.
if _log_entries.is_empty():
hide_dialogue()
## Hide the entire panel with fade.
@@ -247,6 +268,7 @@ func hide_dialogue() -> void:
if _in_player_conversation:
_end_player_conversation()
return # _end_player_conversation may call hide_dialogue if log is empty
_is_showing = false
@@ -256,6 +278,7 @@ func hide_dialogue() -> void:
_active_tween.tween_property(panel, "modulate:a", 0.0, FADE_OUT)
_active_tween.tween_callback(func():
visible = false
GameState.dialogue_active = false # D-064: unblock movement after fade completes
)
@@ -364,10 +387,11 @@ func _rebuild_log() -> void:
var now := Time.get_ticks_msec()
var bbcode := ""
for entry in _log_entries:
var age_sec: float = (now - entry.timestamp_msec) / 1000.0
var alpha: float = 1.0
if age_sec > _entry_lifetime:
alpha = clampf(1.0 - (age_sec - _entry_lifetime) / _entry_fade, 0.0, 1.0)
if not entry.pinned:
var age_sec: float = (now - entry.timestamp_msec) / 1000.0
if age_sec > _entry_lifetime:
alpha = clampf(1.0 - (age_sec - _entry_lifetime) / _entry_fade, 0.0, 1.0)
if entry.is_passive:
alpha *= _passive_opacity
if alpha <= 0.0:
@@ -380,32 +404,83 @@ func _rebuild_log() -> void:
## Format a single log entry as BBCode.
## Hoshe #2: escape BBCode brackets in server-sourced strings.
## Araminta: passive lines get ┃ prefix + desaturated colours.
## Non-blocking: 1-on-1 player dialogue simplifies to "Speaker:" (no → You).
func _format_entry(entry: Dictionary, alpha: float) -> String:
var speaker: String = _escape_bbcode(entry.speaker)
var target: String = _escape_bbcode(entry.target)
var text: String = _escape_bbcode(entry.text)
var is_passive: bool = entry.is_passive
var speaker_color := _color_for_name(entry.speaker)
var target_color := _color_for_name(entry.target)
# Apply alpha to all colors in this line
# Desaturate passive name colours (Araminta review)
if is_passive:
speaker_color = _desaturate(speaker_color, PASSIVE_DESATURATION)
target_color = _desaturate(target_color, PASSIVE_DESATURATION)
var sc := _color_with_alpha(speaker_color, alpha)
var tc := _color_with_alpha(target_color, alpha)
var ac := _color_with_alpha(_arrow_color, alpha)
var tc := _color_with_alpha(target_color, alpha)
var txc := _color_with_alpha(_speech_color, alpha)
return "[color=%s][b]%s[/b][/color][color=%s] \u2192 [/color][color=%s][b]%s[/b][/color][color=%s]: %s[/color]" % [
sc, entry.speaker, ac, tc, entry.target, txc, entry.text
]
var prefix := PASSIVE_GLYPH if is_passive else ""
# Non-blocking: simplify 1-on-1 player dialogue — no arrow for Speaker → You or You → Speaker
var involves_player := entry.speaker == PLAYER_NAME or entry.target == PLAYER_NAME
if involves_player and not is_passive:
# Just "Speaker: text" or "You: text"
return "%s[color=%s][b]%s[/b][/color][color=%s]: %s[/color]" % [
prefix, sc, speaker, txc, text
]
else:
# Full "Speaker → Target: text" for overheard
return "%s[color=%s][b]%s[/b][/color][color=%s] \u2192 [/color][color=%s][b]%s[/b][/color][color=%s]: %s[/color]" % [
prefix, sc, speaker, ac, tc, target, txc, text
]
## Get a stable color for a character name.
## Escape BBCode bracket characters in server-sourced text (Hoshe #2).
static func _escape_bbcode(text: String) -> String:
return text.replace("[", "[lb]")
## Get a stable color for a character name, with contrast floor enforcement.
func _color_for_name(char_name: String) -> Color:
if char_name == PLAYER_NAME:
return _player_color
if _npc_colors.is_empty():
return _speech_color
var idx := char_name.hash() % _npc_colors.size()
# hash() can return negative in GDScript — use abs
if idx < 0:
idx = -idx
return _npc_colors[idx]
var idx := absi(char_name.hash()) % _npc_colors.size()
var color := _npc_colors[idx]
return _enforce_contrast(color)
## Enforce minimum luminance so name colours remain readable against dark BG (Araminta #3).
static func _enforce_contrast(color: Color) -> Color:
var lum := color.r * 0.299 + color.g * 0.587 + color.b * 0.114
if lum < MIN_CONTRAST_LUMINANCE:
var boost := MIN_CONTRAST_LUMINANCE / maxf(lum, 0.001)
return Color(
minf(color.r * boost, 1.0),
minf(color.g * boost, 1.0),
minf(color.b * boost, 1.0),
color.a
)
return color
## Desaturate a colour by a factor (0.0 = no change, 1.0 = full greyscale).
static func _desaturate(color: Color, amount: float) -> Color:
var grey := color.r * 0.299 + color.g * 0.587 + color.b * 0.114
return Color(
lerpf(color.r, grey, amount),
lerpf(color.g, grey, amount),
lerpf(color.b, grey, amount),
color.a
)
## Convert a Color to a hex string with alpha baked in.
@@ -415,29 +490,44 @@ static func _color_with_alpha(base: Color, alpha: float) -> String:
# -- Entry expiry --
## Remove fully expired entries. Hide panel if nothing remains.
## Remove fully expired entries. Mark dirty if any fading entries exist.
## Pinned entries (active conversation) skip expiry entirely (Araminta #2).
func _expire_entries() -> void:
if _log_entries.is_empty():
return
var now := Time.get_ticks_msec()
var total_lifetime_msec: int = int((_entry_lifetime + _entry_fade) * 1000.0)
var had_entries := _log_entries.size() > 0
var removed := false
var has_fading := false
# Remove expired entries from the front (oldest first)
# Remove expired non-pinned entries from the front (oldest first)
while _log_entries.size() > 0:
var age: int = now - _log_entries[0].timestamp_msec
var entry: Dictionary = _log_entries[0]
if entry.pinned:
break # Pinned entries never expire
var age: int = now - entry.timestamp_msec
if age < total_lifetime_msec:
# Check if this entry is in the fading phase
if age > int(_entry_lifetime * 1000.0):
has_fading = true
break
_log_entries.remove_at(0)
removed = true
# Rebuild if entries were removed
if had_entries and _log_entries.size() == 0:
_rebuild_log()
if not _in_player_conversation:
hide_dialogue()
elif had_entries:
# Rebuild to update fading entries
_rebuild_log()
# Check remaining entries for fading state
if not has_fading:
for entry in _log_entries:
if not entry.pinned:
var age: int = now - entry.timestamp_msec
if age > int(_entry_lifetime * 1000.0):
has_fading = true
break
if removed or has_fading:
_log_dirty = true
if removed and _log_entries.is_empty() and not _in_player_conversation:
hide_dialogue()
# -- Visibility --