fix(client): address PR #22 review — 3 critical bugs, 2 warnings, 8 suggestions

Critical fixes:
- fog_state: guard _compute_bounds() against all-invalid tiles (negative Rect2i crash)
- fog_shader: read Camera2D zoom dynamically instead of hardcoded Vector2(2,2)
- world_radial: set custom_minimum_size in _ready() from spoke geometry

Warnings:
- fog.gdshader: tighten PERIPHERAL_LOW 0.15→0.55 to match D-059 peripheral band
- Extract color_for_entity_kind() to Constants.gd, decouple CursorRenderer from EntityRenderer

Documentation: shallow copy assumption, gradual decay TODO, tween guard rationale,
fade timing rationale, ToggleInsert TODO, monologue consume-once, hover offset safety,
v6 fixture gap TODO.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-02-15 23:23:11 +01:00
co-authored by Claude Opus 4.6
parent 1b0953db25
commit 9e8ccb13bf
10 changed files with 44 additions and 17 deletions
+6
View File
@@ -62,6 +62,8 @@ func update_from_state() -> void:
var positions: Dictionary = GameState.visible_positions
var sectors: Dictionary = GameState.visibility_sectors
# TODO(v0.2): gradual decay over game-time instead of immediate 255→128
# 1. Clear visibility, then write current LOS
_vis_bytes.fill(0)
for pos in positions:
@@ -93,6 +95,7 @@ func update_from_state() -> void:
_exp_image.set_data(_width, _height, false, Image.FORMAT_R8, _exp_bytes)
exploration_texture.update(_exp_image)
# Shallow copy — correct for Dictionary<Vector2i, bool/String> values
_prev_visible = positions.duplicate()
@@ -108,5 +111,8 @@ func _compute_bounds(tiles: Array) -> Rect2i:
min_y = mini(min_y, int(tile.y))
max_x = maxi(max_x, int(tile.x))
max_y = maxi(max_y, int(tile.y))
# Guard: all tiles invalid (no x/y) — sentinels would produce negative Rect2i
if min_x > max_x:
return Rect2i(0, 0, 1, 1)
# Margin for fog gradient bleed at edges
return Rect2i(min_x - 4, min_y - 4, max_x - min_x + 9, max_y - min_y + 9)
+9
View File
@@ -58,6 +58,15 @@ const ENTITY_COLOR_HOSTILE: Color = Color("#d45d5d") # Hostile/Dangerous —
const ENTITY_COLOR_OBJECT: Color = Color("#8b8ba0") # Static objects — muted grey
const ENTITY_COLOR_PLAYER: Color = Color("#e0e8ff") # Player character (detective)
# D-033 color lookup by entity kind (Phase 1: defaults, Phase 2 #361: relationship-based)
static func color_for_entity_kind(entity_data: Dictionary) -> Color:
var kind_variant: String = entity_data.get("kind", {}).get("variant", "")
match kind_variant:
"Player": return ENTITY_COLOR_PLAYER
"Npc": return ENTITY_COLOR_UNKNOWN
"Object", "Terrain": return ENTITY_COLOR_OBJECT
_: return ENTITY_COLOR_OBJECT
# D-015: Peripheral vision dimming
const PERIPHERAL_ALPHA: float = 0.5
+3 -1
View File
@@ -40,10 +40,12 @@ func _process(_delta: float) -> void:
stance_indicator.update_from_state()
# Show monologue if server sent one this tick (#414)
# Consume-once: set to null after showing to prevent re-display.
# Single monologue per snapshot is guaranteed by server.
if GameState.current_monologue != null and monologue_display:
var mono: Dictionary = GameState.current_monologue
monologue_display.show_monologue(mono.get("text", ""), mono.get("duration_seconds", 5.0))
GameState.current_monologue = null # Consume — don't re-show next frame
GameState.current_monologue = null
# Track camera to player position every frame (D-015: locked, no panning)
# Camera2D smoothing handles interpolation — we just set the target
+6 -1
View File
@@ -128,7 +128,7 @@ func _find_nearest_entity() -> Dictionary:
best_dist = dist
result.id = entity.entity_id
result.kind = entity.get("kind", {}).get("variant", "")
result.color = EntityRenderer._color_for_kind(entity)
result.color = Constants.color_for_entity_kind(entity)
result.offset = (xform * center) - mouse_screen
return result
@@ -191,6 +191,11 @@ func _interpolate() -> void:
# --- Drawing ---
func _draw() -> void:
# _hover_offset is computed in _detect_hover() during _process(), not recalculated
# here. This is safe because Camera2D (tree sibling, earlier in scene order) applies
# its smoothing transform before CursorRenderer._process() reads canvas_transform.
# Nothing modifies the canvas transform between _process() and _draw().
# Bloom pass — wider, semi-transparent glow
if _bloom > 0.01:
var bloom_mod := 1.0
+2 -11
View File
@@ -101,18 +101,9 @@ func _remove_entity_node(entity_id: int) -> void:
entity_node.queue_free()
entity_nodes.erase(entity_id)
# D-033 color by entity kind (Phase 1: defaults by kind, not relationship)
# D-033 color by entity kind — delegates to Constants.color_for_entity_kind
static func _color_for_kind(entity_data: Dictionary) -> Color:
var kind_variant: String = entity_data.get("kind", {}).get("variant", "")
match kind_variant:
"Player":
return Constants.ENTITY_COLOR_PLAYER
"Npc":
return Constants.ENTITY_COLOR_UNKNOWN
"Object", "Terrain":
return Constants.ENTITY_COLOR_OBJECT
_:
return Constants.ENTITY_COLOR_OBJECT
return Constants.color_for_entity_kind(entity_data)
# Add a facing direction indicator triangle to the player entity
func _add_facing_indicator(parent_node: Control) -> void:
+2 -1
View File
@@ -43,7 +43,8 @@ func update_fog() -> void:
# 2. Position ColorRect to cover the current viewport
var vp_size := get_viewport().get_visible_rect().size
var zoom := Vector2(2.0, 2.0) # Must match Camera2D zoom
var cam := get_viewport().get_camera_2d()
var zoom := cam.zoom if cam else Vector2(2.0, 2.0)
var camera_pos := GameState.player_position * TILE_SIZE
var half_view := vp_size / (2.0 * zoom)
_fog_rect.position = camera_pos - half_view
+1 -1
View File
@@ -25,7 +25,7 @@ const vec3 DARK_OVERLAY = vec3(0.02, 0.02, 0.05);
// D-059 thresholds (after bilinear filtering)
// Forward tiles = 1.0, Peripheral = 0.706 (180/255), not-visible = 0.0
const float CLEAR_THRESHOLD = 0.85; // Above this: fully clear
const float PERIPHERAL_LOW = 0.15; // Below this: transition to deep/unexplored
const float PERIPHERAL_LOW = 0.55; // Below this: transition to deep/unexplored
void fragment() {
// Map UV (0-1 across ColorRect) to world pixels, then to tile coordinates
+8
View File
@@ -6,6 +6,14 @@ class_name TestProtocolV6
extends GdUnitTestSuite
const FIXTURE_DIR = "res://tests/fixtures/msgpack/"
# TODO: Add .msgpack fixture files for v6 stance/inventory variations.
# Current fixtures (snapshot_one_npc, snapshot_empty, etc.) only cover default
# Walk stance with empty inventory. Missing fixtures:
# - snapshot with player_stance = Sprint, Careful, Crouch
# - snapshot with populated player_inventory (1-item, 9-item full grid)
# - combined stance + inventory variations
# Stance/inventory decode is tested via in-memory Messagepack.encode() above,
# but regression-safe .msgpack fixtures are needed for bridge contract coverage.
func _load_fixture(name: String) -> PackedByteArray:
+3 -1
View File
@@ -11,6 +11,7 @@ extends Control
signal verb_selected(kind: String, entity_id: int)
const MAX_VERBS := 4
# Intentionally faster than cursor 150ms — text must be readable quickly
const FADE_IN := 0.12
const FADE_OUT := 0.10
const LABEL_HEIGHT := 22
@@ -74,7 +75,7 @@ func update_from_state() -> void:
func _rebuild_labels() -> void:
# Clear existing labels
# Guard: tween callback from previous _hide() may fire after labels already freed
for lbl in _verb_labels:
if is_instance_valid(lbl):
lbl.queue_free()
@@ -113,6 +114,7 @@ func _hide() -> void:
_active_tween.kill()
_active_tween = create_tween()
_active_tween.tween_property(self, "modulate:a", 0.0, FADE_OUT)
# Guard: tween callback from previous _hide() may fire after labels already freed
_active_tween.tween_callback(func():
visible = false
for lbl in _verb_labels:
+4 -1
View File
@@ -38,6 +38,9 @@ var _insert_active: bool = false
func _ready() -> void:
var menu_extent := SPOKE_RADIUS + ICON_SIZE + 20.0 # spoke + icon + label margin
custom_minimum_size = Vector2(menu_extent * 2.0, menu_extent * 2.0)
size = custom_minimum_size
visible = false
mouse_filter = Control.MOUSE_FILTER_STOP
@@ -117,7 +120,7 @@ func _confirm_selection() -> void:
func _activate_insert() -> void:
# Send Pause to freeze game while viewing insert data
# TODO(v7): replace PAUSE toggle with dedicated ToggleInsert action in protocol
if not _insert_active:
_insert_active = true
_send_pause()