fix(client): correct 5 bugs found in pre-PR review
- sound_indicator_renderer: draw_colored_polygon → draw_polygon (runtime crash) - sound_indicator_renderer: append+dedup instead of clear — indicators now survive 3.5s instead of dying after one server tick - dialogue_box: wire Constants.DIALOGUE_MAX_WIDTH (640px) instead of hardcoded 832px MAX_WIDTH_PX - dialogue_box.tscn: update default offsets to ±320 (was ±416) - decisions/perception.md: fix footer typo (1920px → 640px) - tests updated for new append/dedup indicator behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -13,8 +13,9 @@ extends Node2D
|
||||
## Danger #d45d5d — alert, gunshot, explosion, threat
|
||||
##
|
||||
## Each indicator lives for INDICATOR_LIFETIME seconds and fades out over
|
||||
## the last FADE_DURATION seconds. New events from the same tick replace
|
||||
## any previously received events (server sends current medium events per tick).
|
||||
## the last FADE_DURATION seconds. New events are appended each tick;
|
||||
## expired indicators are removed by _process(). Deduplication prevents
|
||||
## the same source position from stacking multiple arrows.
|
||||
|
||||
const TILE_SIZE: int = Constants.TILE_SIZE
|
||||
|
||||
@@ -47,20 +48,32 @@ func _process(delta: float) -> void:
|
||||
queue_redraw()
|
||||
|
||||
|
||||
## Update medium-range sound events from snapshot.
|
||||
## Append new medium-range sound events from snapshot.
|
||||
## events: Array of {x: float, y: float, event_type: String}
|
||||
## Only Medium range_category events should be passed.
|
||||
## Deduplicates by tile position — if an indicator already exists at (x,y),
|
||||
## its timer resets instead of spawning a duplicate arrow.
|
||||
func update_sound_events(events: Array) -> void:
|
||||
_indicators.clear()
|
||||
for evt in events:
|
||||
if not evt.has("x") or not evt.has("y"):
|
||||
continue
|
||||
_indicators.append({
|
||||
"x": float(evt.x),
|
||||
"y": float(evt.y),
|
||||
"event_type": evt.get("event_type", ""),
|
||||
"elapsed": 0.0,
|
||||
})
|
||||
var ex: float = float(evt.x)
|
||||
var ey: float = float(evt.y)
|
||||
# Deduplicate: reset timer if an indicator already exists at this tile
|
||||
var found := false
|
||||
for ind in _indicators:
|
||||
if is_equal_approx(ind.x, ex) and is_equal_approx(ind.y, ey):
|
||||
ind.elapsed = 0.0
|
||||
ind.event_type = evt.get("event_type", "")
|
||||
found = true
|
||||
break
|
||||
if not found:
|
||||
_indicators.append({
|
||||
"x": ex,
|
||||
"y": ey,
|
||||
"event_type": evt.get("event_type", ""),
|
||||
"elapsed": 0.0,
|
||||
})
|
||||
if not _indicators.is_empty():
|
||||
queue_redraw()
|
||||
|
||||
@@ -122,7 +135,7 @@ func _rect_edge_point(center: Vector2, dir: Vector2, half: Vector2, inset: float
|
||||
func _draw_arrow(pos: Vector2, dir: Vector2, color: Color) -> void:
|
||||
var perp := Vector2(-dir.y, dir.x)
|
||||
var base_center := pos - dir * ARROW_LEN
|
||||
draw_colored_polygon(
|
||||
draw_polygon(
|
||||
PackedVector2Array([pos, base_center - perp * ARROW_HALF, base_center + perp * ARROW_HALF]),
|
||||
PackedColorArray([color, color, color])
|
||||
)
|
||||
|
||||
@@ -438,24 +438,25 @@ func test_sound_indicator_drops_events_without_position() -> void:
|
||||
assert_that(renderer._indicators.size()).is_equal(1)
|
||||
renderer.queue_free()
|
||||
|
||||
func test_sound_indicator_replaces_events_on_update() -> void:
|
||||
func test_sound_indicator_appends_new_events() -> void:
|
||||
var renderer := _make_sound_indicator_renderer()
|
||||
renderer.update_sound_events([
|
||||
{"x": 1.0, "y": 1.0, "event_type": "Voice"},
|
||||
{"x": 2.0, "y": 2.0, "event_type": "Footstep"},
|
||||
])
|
||||
assert_that(renderer._indicators.size()).is_equal(2)
|
||||
# New update replaces all previous events
|
||||
# New update appends — existing indicators persist until they expire
|
||||
renderer.update_sound_events([{"x": 5.0, "y": 5.0, "event_type": "Footstep"}])
|
||||
assert_that(renderer._indicators.size()).is_equal(1)
|
||||
assert_that(renderer._indicators.size()).is_equal(3)
|
||||
renderer.queue_free()
|
||||
|
||||
func test_sound_indicator_empty_update_clears_indicators() -> void:
|
||||
func test_sound_indicator_empty_update_preserves_existing() -> void:
|
||||
var renderer := _make_sound_indicator_renderer()
|
||||
renderer.update_sound_events([{"x": 3.0, "y": 3.0, "event_type": "Voice"}])
|
||||
assert_that(renderer._indicators.size()).is_equal(1)
|
||||
# Empty update does not clear existing indicators — they expire via _process
|
||||
renderer.update_sound_events([])
|
||||
assert_that(renderer._indicators.size()).is_equal(0)
|
||||
assert_that(renderer._indicators.size()).is_equal(1)
|
||||
renderer.queue_free()
|
||||
|
||||
func test_sound_indicator_color_voice() -> void:
|
||||
@@ -479,6 +480,15 @@ func test_sound_indicator_color_neutral_for_unknown() -> void:
|
||||
assert_that(renderer._color_for_type("Unknown")).is_equal(SoundIndicatorRenderer.COLOR_NEUTRAL)
|
||||
renderer.queue_free()
|
||||
|
||||
func test_sound_indicator_deduplicates_same_position() -> void:
|
||||
var renderer := _make_sound_indicator_renderer()
|
||||
renderer.update_sound_events([{"x": 5.0, "y": 5.0, "event_type": "Voice"}])
|
||||
assert_that(renderer._indicators.size()).is_equal(1)
|
||||
# Same position again — should reset timer, not add a duplicate
|
||||
renderer.update_sound_events([{"x": 5.0, "y": 5.0, "event_type": "Voice"}])
|
||||
assert_that(renderer._indicators.size()).is_equal(1)
|
||||
renderer.queue_free()
|
||||
|
||||
func test_sound_indicator_elapsed_starts_at_zero() -> void:
|
||||
var renderer := _make_sound_indicator_renderer()
|
||||
renderer.update_sound_events([{"x": 10.0, "y": 5.0, "event_type": "Voice"}])
|
||||
|
||||
@@ -200,9 +200,25 @@ func test_indicator_does_not_crash_for_empty_events() -> void:
|
||||
indicator.set_script(script)
|
||||
add_child(indicator)
|
||||
indicator.update_sound_events([])
|
||||
# Empty update on fresh renderer — no indicators exist, no crash
|
||||
assert_that(indicator._indicators.size()).is_equal(0)
|
||||
indicator.queue_free()
|
||||
|
||||
func test_indicator_existing_survive_empty_update() -> void:
|
||||
## Existing indicators persist through an empty update (expire via _process only).
|
||||
var script: GDScript = _load_indicator_script()
|
||||
if script == null:
|
||||
push_warning("TestSoundIndicators: sound_indicator_renderer.gd not found")
|
||||
return
|
||||
var indicator := Node2D.new()
|
||||
indicator.set_script(script)
|
||||
add_child(indicator)
|
||||
indicator.update_sound_events([{"x": 5.0, "y": 5.0, "event_type": "Voice"}])
|
||||
assert_that(indicator._indicators.size()).is_equal(1)
|
||||
indicator.update_sound_events([])
|
||||
assert_that(indicator._indicators.size()).is_equal(1)
|
||||
indicator.queue_free()
|
||||
|
||||
func test_indicator_stores_valid_medium_events() -> void:
|
||||
## #126: update_sound_events must store events with x and y fields.
|
||||
var script: GDScript = _load_indicator_script()
|
||||
|
||||
@@ -28,7 +28,7 @@ const FADE_IN: float = 0.2
|
||||
const FADE_OUT: float = 0.3 # D-064: 300ms fade on walk-away
|
||||
const MAX_OPTIONS: int = 3 # D-061: max 3 response options visible
|
||||
const MAX_HEIGHT_RATIO: float = 0.2 # D-061: max 20% viewport height
|
||||
const MAX_WIDTH_PX: float = 832.0 # D-061: max-width cap (~65% of 1280)
|
||||
const MAX_WIDTH_PX: float = Constants.DIALOGUE_MAX_WIDTH # D-076: 640px (OQ-29)
|
||||
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"
|
||||
|
||||
@@ -20,9 +20,9 @@ anchor_left = 0.5
|
||||
anchor_top = 1.0
|
||||
anchor_right = 0.5
|
||||
anchor_bottom = 1.0
|
||||
offset_left = -416.0
|
||||
offset_left = -320.0
|
||||
offset_top = -200.0
|
||||
offset_right = 416.0
|
||||
offset_right = 320.0
|
||||
grow_horizontal = 2
|
||||
grow_vertical = 0
|
||||
|
||||
|
||||
@@ -358,4 +358,4 @@ How the player observes and interacts with the world: camera, fog, line-of-sight
|
||||
|
||||
---
|
||||
|
||||
*30 decisions. Last updated: 2026-02-19 (D-076: OQ-29 resolved — dialogue max-width 1920px)*
|
||||
*30 decisions. Last updated: 2026-02-19 (D-076: OQ-29 resolved — dialogue max-width 640px)*
|
||||
|
||||
Reference in New Issue
Block a user