From ebc973b5582a8220734158162e12f69d2496d3dd Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 23 Feb 2026 21:01:22 +0100 Subject: [PATCH 1/5] refactor(client): optimize zone_id extraction from O(N) to O(1) lookup (#543) Build _tile_by_coord dictionary from member visible_tiles (covers both test-mode "tiles" key and live-server "visible_tiles" key), then replace the linear scan with a single dict lookup. Net-zero complexity: adds one dict-set per tile in an existing iteration, removes the separate scan loop. Co-Authored-By: Claude Opus 4.6 --- client/scripts/autoloads/game_state.gd | 20 +-- client/tests/test_snapshot_zone_id.gd | 211 +++++++++++++++++++++++++ 2 files changed, 221 insertions(+), 10 deletions(-) create mode 100644 client/tests/test_snapshot_zone_id.gd diff --git a/client/scripts/autoloads/game_state.gd b/client/scripts/autoloads/game_state.gd index 50318fad5..979008f4a 100644 --- a/client/scripts/autoloads/game_state.gd +++ b/client/scripts/autoloads/game_state.gd @@ -240,16 +240,16 @@ func apply_snapshot(snapshot: Dictionary) -> void: medium_sound_events = [] close_sound_events = [] - # D-073 (#529): Extract zone_id from the player's current tile (server-authoritative). - # O(1) via visible_positions dict would be ideal, but tiles are arrays without - # positional indexing — use the same tile iteration below instead. - current_zone_id = "" - var _px := int(player_position.x) - var _py := int(player_position.y) - for _ztile in visible_tiles: - if _ztile is Dictionary and _ztile.get("x") == _px and _ztile.get("y") == _py: - current_zone_id = _ztile.get("zone_id", "") - break + # D-073 (#529): O(1) zone_id lookup. Build coord→tile dict from member visible_tiles + # (populated above from either "tiles" test-mode key or "visible_tiles" live key). + # Must use the member var, not snapshot.visible_tiles, so test mode is covered. + var _tile_by_coord: Dictionary = {} + for vtile in visible_tiles: + if vtile is Dictionary and vtile.has("x") and vtile.has("y"): + _tile_by_coord[Vector2i(vtile.x, vtile.y)] = vtile + var player_pos_key := Vector2i(int(player_position.x), int(player_position.y)) + var player_tile = _tile_by_coord.get(player_pos_key, null) + current_zone_id = player_tile.get("zone_id", "") if player_tile else "" # v2: visible_tiles with visibility sectors # Derives visible_positions when not explicitly provided (real server mode) diff --git a/client/tests/test_snapshot_zone_id.gd b/client/tests/test_snapshot_zone_id.gd new file mode 100644 index 000000000..6f5abf710 --- /dev/null +++ b/client/tests/test_snapshot_zone_id.gd @@ -0,0 +1,211 @@ +## Sprint 16 #543: GameState.current_zone_id population tests. +## Verifies that apply_snapshot() correctly extracts zone_id from the player's +## current tile after the O(N) → O(1) refactor. Behavior must be identical +## before and after the change (net-zero behavioral change per ticket spec). +## Spec refs: D-073, #543, #529. +class_name TestSnapshotZoneId +extends GdUnitTestSuite + + +func before_test() -> void: + GameState.current_zone_id = "" + GameState.player_position = Vector2.ZERO + GameState.visible_tiles = [] + GameState.visible_positions = {} + GameState.visibility_sectors = {} + + +# -- Happy path (S16-Z01) ------------------------------------------------------ + +func test_zone_id_populated_when_player_on_zone_tile() -> void: + # S16-Z01: Player at (5,5), visible_tiles has zone_id "zone_alpha" at (5,5). + # D-073: current_zone_id must be set to the server-authoritative zone_id. + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": [ + {"x": 3, "y": 3, "z": 0, "type": "floor", "zone_id": "zone_foyer"}, + {"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_alpha"}, + {"x": 7, "y": 7, "z": 0, "type": "floor", "zone_id": "zone_beta"}, + ], + }) + assert_that(GameState.current_zone_id).override_failure_message( + "current_zone_id must match zone_id at player tile (5,5)" + ).is_equal("zone_alpha") + assert_that(GameState.current_zone_id.is_empty()).override_failure_message( + "current_zone_id must be non-empty when player is on a zone-tagged tile" + ).is_false() + + +# -- Missing zone_id field (S16-Z02) ------------------------------------------- + +func test_zone_id_empty_when_tile_lacks_zone_field() -> void: + # S16-Z02: Player's tile exists but has no zone_id key → empty string. + # Server may send tiles without zone_id when tile is unzoned. + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": [ + {"x": 5, "y": 5, "z": 0, "type": "floor"}, + ], + }) + assert_that(GameState.current_zone_id).override_failure_message( + "Tile without zone_id field → current_zone_id must default to empty string" + ).is_equal("") + + +# -- Player off-tile (S16-Z03) ------------------------------------------------- + +func test_zone_id_empty_when_player_not_on_any_tile() -> void: + # S16-Z03: Player at (99,99) but no tile at that position → empty string. + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 99.0, "y": 99.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": [ + {"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_alpha"}, + ], + }) + assert_that(GameState.current_zone_id).override_failure_message( + "Player at (99,99) with no matching tile → current_zone_id must be empty" + ).is_equal("") + + +# -- Empty tiles (S16-Z04) ----------------------------------------------------- + +func test_zone_id_empty_on_empty_visible_tiles() -> void: + # S16-Z04: No visible tiles at all → empty string, no crash. + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": [], + }) + assert_that(GameState.current_zone_id).override_failure_message( + "Empty visible_tiles → current_zone_id must be empty string (no crash)" + ).is_equal("") + + +# -- Correct tile selected among many (S16-Z05) -------------------------------- + +func test_zone_id_selects_correct_tile_among_many() -> void: + # S16-Z05: 10x10 tile grid, player at (6,4). Only zone_6_4 must be selected. + # Tests that the lookup doesn't accidentally match a neighboring tile. + var tiles: Array = [] + for tx in range(10): + for ty in range(10): + tiles.append({"x": tx, "y": ty, "z": 0, "type": "floor", + "zone_id": "zone_%d_%d" % [tx, ty]}) + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 6.0, "y": 4.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": tiles, + }) + assert_that(GameState.current_zone_id).override_failure_message( + "Player at (6,4) must get zone_6_4, not a neighboring tile" + ).is_equal("zone_6_4") + + +# -- Zone transition (S16-Z06) ------------------------------------------------- + +func test_zone_id_updates_on_zone_transition() -> void: + # S16-Z06: Player moves from zone_alpha (5,5) to zone_beta (6,5). + # current_zone_id must update on each snapshot. + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": [ + {"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_alpha"}, + {"x": 6, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_beta"}, + ], + }) + assert_that(GameState.current_zone_id).is_equal("zone_alpha") + + GameState.apply_snapshot({ + "tick": 2, + "entities": [ + {"entity_id": 1, "x": 6.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": [ + {"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_alpha"}, + {"x": 6, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_beta"}, + ], + }) + assert_that(GameState.current_zone_id).override_failure_message( + "After moving to (6,5), current_zone_id must update to zone_beta" + ).is_equal("zone_beta") + + +# -- Regression: visible_positions unaffected (S16-Z07) ----------------------- + +func test_visible_positions_unaffected_by_zone_id_refactor() -> void: + # S16-Z07: The O(1) refactor must not break visible_positions population. + # Both zone_id and visible_positions derive from the same visible_tiles loop — + # verify both are correctly populated after a single apply_snapshot(). + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": [ + {"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_alpha", "visibility": "Forward"}, + {"x": 6, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_beta", "visibility": "Peripheral"}, + ], + }) + assert_that(GameState.visible_positions.has(Vector2i(5, 5))).override_failure_message( + "visible_positions must still contain (5,5) after zone_id refactor" + ).is_true() + assert_that(GameState.visible_positions.has(Vector2i(6, 5))).is_true() + assert_that(GameState.current_zone_id).override_failure_message( + "current_zone_id must be zone_alpha after same apply_snapshot call" + ).is_equal("zone_alpha") + + +# -- Fractional player position (S16-Z08) -------------------------------------- + +func test_zone_id_uses_int_truncation_of_player_position() -> void: + # S16-Z08: Player at (5.7, 5.9) → int(5.7)=5, int(5.9)=5 → matches tile (5,5). + # Server sends player coords as floats; zone lookup must truncate to tile index. + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 5.7, "y": 5.9, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "visible_tiles": [ + {"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_alpha"}, + ], + }) + assert_that(GameState.current_zone_id).override_failure_message( + "Player at (5.7,5.9) must match tile (5,5) — int() truncation applies" + ).is_equal("zone_alpha") + + +# -- Test-mode tiles key (S16-Z09) -------------------------------------------- + +func test_zone_id_works_with_test_mode_tiles_key() -> void: + # S16-Z09: Test mode sends "tiles" key, not "visible_tiles". + # The O(1) refactor uses member visible_tiles (covers both paths). + # Regression guard: if _tile_by_coord is moved into the snapshot.visible_tiles + # block only, this test fails — catching the exact regression Tyre flagged. + GameState.apply_snapshot({ + "tick": 1, + "entities": [ + {"entity_id": 1, "x": 5.0, "y": 5.0, "z": 0, "kind": {"variant": "Player", "data": null}}, + ], + "tiles": [ + {"x": 5, "y": 5, "z": 0, "type": "floor", "zone_id": "zone_alpha"}, + ], + }) + assert_that(GameState.current_zone_id).override_failure_message( + "Test-mode 'tiles' key must populate zone_id via member visible_tiles" + ).is_equal("zone_alpha") -- 2.54.0 From a0260176b44a6963ae34401a97c1a9c78715de92 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 23 Feb 2026 21:01:30 +0100 Subject: [PATCH 2/5] feat(client): integrate D-019 angle sprites into entity renderer (#540) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate entity rendering from ColorRect placeholders to Sprite2D with rendered PNGs at -72.5° from horizontal. Key changes: - Sprite2D.centered=false, scale=0.5 for 64px source → 32px runtime - self_modulate for D-033 relationship tinting (modulate.a reserved for D-015 peripheral dimming) - 8-octant to 4-cardinal direction mapping for sprite selection - Feet-anchored ENTITY_OFFSET_Y for correct y-sort with tilted sprites - Facing indicator repositioned to sprite local center (32,32) Co-Authored-By: Claude Opus 4.6 --- client/scripts/rendering/entity_renderer.gd | 123 ++++++++---- client/tests/test_color_shift.gd | 56 +++--- client/tests/test_sprite_integration.gd | 195 ++++++++++++++++++++ 3 files changed, 310 insertions(+), 64 deletions(-) create mode 100644 client/tests/test_sprite_integration.gd diff --git a/client/scripts/rendering/entity_renderer.gd b/client/scripts/rendering/entity_renderer.gd index 020d93d00..f32061438 100644 --- a/client/scripts/rendering/entity_renderer.gd +++ b/client/scripts/rendering/entity_renderer.gd @@ -2,7 +2,7 @@ class_name EntityRenderer extends Node2D # Entity renderer — manages entity sprites under the Entities node -# Creates/updates/removes ColorRect children based on entity data +# Creates/updates/removes Sprite2D children based on entity data # Entity format (from Protocol v2): {entity_id, x, y, z, kind: {variant, data}, visibility} # # Position lerping: entity sprites smoothly slide between tiles instead of snapping. @@ -11,13 +11,15 @@ extends Node2D # # D-033 colors: Phase 1 defaults by entity kind. Phase 2 (#361) will derive # color from RelationshipState via the knowledge graph. +# #540: Sprites at D-019 angle (-72.5° from horizontal). Textures are neutral greyscale; +# self_modulate applies D-033 relationship tinting. modulate.a reserved for D-015 dimming. const TILE_SIZE: int = Constants.TILE_SIZE -# D-044: 24x32 entity footprint within 32x32 visual tile (64x64 source scaled to 32px runtime) +# D-044: 24x32 entity footprint within 32x32 visual tile (64x64 source at 0.5 scale = 32px runtime) const ENTITY_WIDTH: int = 24 const ENTITY_HEIGHT: int = 32 -const ENTITY_OFFSET_X: float = (TILE_SIZE - ENTITY_WIDTH) / 2.0 # center horizontally -const ENTITY_OFFSET_Y: float = (TILE_SIZE - ENTITY_HEIGHT) / 2.0 # center vertically for placeholder. Migration: switch to bottom-anchor (offset = TILE_SIZE - ENTITY_HEIGHT) when real sprites land for correct y-sort ordering. +const ENTITY_OFFSET_X: float = 0.0 # sprite fills tile width at 0.5 scale +const ENTITY_OFFSET_Y: float = TILE_SIZE - ENTITY_HEIGHT # feet-anchored for correct y-sort with D-019 tilt # Lerp speed — framerate-independent exponential smoothing. # At 12.0: ~70% there after 0.1s, ~95% after 0.25s. @@ -27,7 +29,8 @@ const LERP_SPEED: float = 12.0 var entity_nodes: Dictionary = {} # entity_id -> Node2D var _entity_targets: Dictionary = {} # entity_id -> Vector2 (target pixel position) var _entity_relationships: Dictionary = {} # #521: entity_id -> String (last relationship) -var _entity_tweens: Dictionary = {} # #521: entity_id -> {target: Color, elapsed: float} +var _entity_tweens: Dictionary = {} # #521: entity_id -> {from: Color, target: Color, elapsed: float} +var _entity_facing: Dictionary = {} # #540: entity_id -> String ("north"/"east"/"south"/"west") # #521: Color transition duration in seconds (D-033: "0.5s fade") const COLOR_FADE_DURATION: float = 0.5 @@ -48,7 +51,7 @@ func _process(delta: float) -> void: if not node.position.is_equal_approx(target): node.position = node.position.lerp(target, weight) - # #521: Advance color transitions (manual lerp, testable without SceneTree) + # #521: Advance self_modulate transitions (manual lerp, testable without SceneTree) var finished_ids: Array = [] for entity_id in _entity_tweens.keys(): if not entity_nodes.has(entity_id): @@ -57,8 +60,9 @@ func _process(delta: float) -> void: var tween_data: Dictionary = _entity_tweens[entity_id] tween_data.elapsed += delta var t := clampf(tween_data.elapsed / COLOR_FADE_DURATION, 0.0, 1.0) - var node_c: ColorRect = entity_nodes[entity_id] as ColorRect - node_c.color = tween_data.from.lerp(tween_data.target, t) + var node_s: Sprite2D = entity_nodes[entity_id] as Sprite2D + if node_s: + node_s.self_modulate = tween_data.from.lerp(tween_data.target, t) if t >= 1.0: finished_ids.append(entity_id) for eid in finished_ids: @@ -92,15 +96,23 @@ func update_entities(entities: Array) -> void: for entity_id in ids_to_remove: _remove_entity_node(entity_id) -# Create a new entity node with D-033 color and optional facing indicator -func _create_entity_node(entity_id: int, entity_data: Dictionary) -> void: - var entity_node = ColorRect.new() - entity_node.name = "Entity_" + str(entity_id) - entity_node.size = Vector2(ENTITY_WIDTH, ENTITY_HEIGHT) - entity_node.pivot_offset = Vector2(ENTITY_WIDTH / 2.0, ENTITY_HEIGHT / 2.0) - # D-033 color by relationship (#521) - entity_node.color = _color_for_kind(entity_data) +# Create a new entity node with D-033 tint and sprite texture at D-019 angle +func _create_entity_node(entity_id: int, entity_data: Dictionary) -> void: + var entity_node := Sprite2D.new() + entity_node.name = "Entity_" + str(entity_id) + # centered=false: top-left origin aligns with tile grid. + # scale=0.5: maps 64px source texture to 32px runtime (D-043, 2x camera = 64px on screen). + entity_node.centered = false + entity_node.scale = Vector2(0.5, 0.5) + + # Load sprite for current facing direction + var direction := _entity_direction(entity_id, entity_data) + _entity_facing[entity_id] = direction + entity_node.texture = _load_sprite_texture(direction) + + # D-033: self_modulate for relationship tinting; modulate.a is reserved for D-015 dimming. + entity_node.self_modulate = _color_for_kind(entity_data) add_child(entity_node) entity_nodes[entity_id] = entity_node @@ -121,12 +133,13 @@ func _create_entity_node(entity_id: int, entity_data: Dictionary) -> void: _update_entity_node(entity_id, entity_data) -# Update an existing entity node (target position, visibility dimming, facing) + +# Update an existing entity node (target position, sprite direction, visibility dimming, facing) func _update_entity_node(entity_id: int, entity_data: Dictionary) -> void: if not entity_nodes.has(entity_id): return - var entity_node = entity_nodes[entity_id] + var node = entity_nodes[entity_id] # Update target position — the lerp in _process() will smoothly move there. # Server sends tile-center coords (tile 16 → 16.5), floor to get tile index. @@ -136,38 +149,39 @@ func _update_entity_node(entity_id: int, entity_data: Dictionary) -> void: floorf(entity_data.y) * TILE_SIZE + ENTITY_OFFSET_Y ) - # #521: Detect relationship change → fade D-033 color (0.5s via _process) + # #540: Update sprite texture when facing direction changes + var new_dir := _entity_direction(entity_id, entity_data) + if new_dir != _entity_facing.get(entity_id, ""): + _entity_facing[entity_id] = new_dir + (node as Sprite2D).texture = _load_sprite_texture(new_dir) + + # #521: Detect relationship change → fade D-033 self_modulate (0.5s via _process) var new_rel: String = entity_data.get("relationship", "Unknown") - var old_rel: String = _entity_relationships.get(entity_id, "Unknown") - if new_rel != old_rel: + if new_rel != _entity_relationships.get(entity_id, "Unknown"): _entity_relationships[entity_id] = new_rel - var new_color := _color_for_kind(entity_data) _entity_tweens[entity_id] = { - "from": entity_node.color, - "target": new_color, + "from": (node as Sprite2D).self_modulate, + "target": _color_for_kind(entity_data), "elapsed": 0.0, } - # Note: modulate.a (peripheral dimming below) and color (D-033 tint above) - # are compositionally independent — both can change simultaneously without - # interference. If alpha tweening is added later, coordinate with color tween. - - # v2: Peripheral vision dimming (D-015) - # null visibility (v1 backward compat) defaults to full alpha + # D-015: Peripheral vision dimming via modulate.a. + # Independent from self_modulate (D-033 tint) — both can change simultaneously. var visibility: Variant = entity_data.get("visibility") var target_alpha := Constants.PERIPHERAL_ALPHA if visibility == "Peripheral" else 1.0 - if not is_equal_approx(entity_node.modulate.a, target_alpha): - entity_node.modulate.a = target_alpha + if not is_equal_approx(node.modulate.a, target_alpha): + node.modulate.a = target_alpha # D-054: Update facing indicator from client-side mouse angle (not server). # InputMapper.facing_angle is a continuous float — smoother than octant snapping. if entity_id == GameState.player_entity_id: - var indicator = entity_node.get_node_or_null("FacingIndicator") + var indicator = node.get_node_or_null("FacingIndicator") if indicator != null: # facing_angle: 0=East, -PI/2=North. Indicator: 0=North (up). # Rotate from North basis: add PI/2 to convert. indicator.rotation = InputMapper.facing_angle + PI / 2.0 + # Remove an entity node func _remove_entity_node(entity_id: int) -> void: if not entity_nodes.has(entity_id): @@ -179,13 +193,47 @@ func _remove_entity_node(entity_id: int) -> void: _entity_targets.erase(entity_id) _entity_relationships.erase(entity_id) _entity_tweens.erase(entity_id) + _entity_facing.erase(entity_id) + # D-033 color by entity kind — delegates to Constants.color_for_entity_kind static func _color_for_kind(entity_data: Dictionary) -> Color: 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: + +# #540: Map entity to current 4-direction sprite key. +# Player uses GameState.player_facing (8-octant → 4-cardinal). NPCs default "south". +func _entity_direction(entity_id: int, _entity_data: Dictionary) -> String: + if entity_id == GameState.player_entity_id: + return _octant_to_direction(GameState.player_facing) + # NPCs: no facing field in v1 entity format; south is viewer-facing (D-019 angle) + return "south" + + +# Map 8-direction octant string to nearest 4-direction sprite key. +# N/NW → north, NE/E → east, SE/S → south, SW/W → west +static func _octant_to_direction(octant: String) -> String: + match octant: + "North", "Northwest": return "north" + "Northeast", "East": return "east" + "Southeast", "South": return "south" + "Southwest", "West": return "west" + _: return "south" + + +# Load the sprite texture for the given 4-direction key. +# Falls back to null with a push_warning if the asset is missing. +static func _load_sprite_texture(direction: String) -> Texture2D: + var path := "res://assets/sprites/npc_generic_%s_64.png" % direction + if ResourceLoader.exists(path): + return load(path) as Texture2D + push_warning("EntityRenderer: sprite not found: %s" % path) + return null + + +# Add a facing direction indicator triangle to the player entity. +# Indicator position is in Sprite2D local space (64px texture before 0.5 scale → center at (32,32)). +func _add_facing_indicator(parent_node: Node2D) -> void: var indicator := Polygon2D.new() indicator.name = "FacingIndicator" var s := Constants.FACING_INDICATOR_SIZE @@ -197,6 +245,7 @@ func _add_facing_indicator(parent_node: Control) -> void: Vector2(s * 0.6, -offset + s * 0.4), ]) indicator.color = Constants.ENTITY_COLOR_PLAYER - # Position at center of parent ColorRect — rotation around this point - indicator.position = Vector2(ENTITY_WIDTH / 2.0, ENTITY_HEIGHT / 2.0) + # Sprite2D local space: 64px texture at scale 0.5 → center of visible sprite at (32,32). + # Indicator rotates around this point to track player facing direction. + indicator.position = Vector2(32.0, 32.0) parent_node.add_child(indicator) diff --git a/client/tests/test_color_shift.gd b/client/tests/test_color_shift.gd index e619ab3ad..2b5ae7d7b 100644 --- a/client/tests/test_color_shift.gd +++ b/client/tests/test_color_shift.gd @@ -135,43 +135,43 @@ func test_npc_uses_relationship_color_unknown() -> void: var renderer: Node2D = _make_entity_renderer() var entities: Array = [_make_entity(2, "Npc", "Unknown")] renderer.update_entities(entities) - var node: ColorRect = renderer.entity_nodes[2] as ColorRect + var node: Sprite2D = renderer.entity_nodes[2] as Sprite2D # Unknown -> teal (both Phase 1 and Phase 2 produce the same result) - assert_that(node.color).is_equal(Constants.ENTITY_COLOR_UNKNOWN) + assert_that(node.self_modulate).is_equal(Constants.ENTITY_COLOR_UNKNOWN) renderer.queue_free() func test_npc_uses_relationship_color_friendly() -> void: var renderer: Node2D = _make_entity_renderer() var entities: Array = [_make_entity(2, "Npc", "Friendly")] renderer.update_entities(entities) - var node: ColorRect = renderer.entity_nodes[2] as ColorRect + var node: Sprite2D = renderer.entity_nodes[2] as Sprite2D if not _entity_uses_relationship(renderer): push_warning("TestColorShift: entity renderer not yet using relationship for color — awaiting #521") renderer.queue_free() return - assert_that(node.color).is_equal(Constants.ENTITY_COLOR_FRIENDLY) + assert_that(node.self_modulate).is_equal(Constants.ENTITY_COLOR_FRIENDLY) renderer.queue_free() func test_npc_uses_relationship_color_poi() -> void: var renderer: Node2D = _make_entity_renderer() var entities: Array = [_make_entity(2, "Npc", "PersonOfInterest")] renderer.update_entities(entities) - var node: ColorRect = renderer.entity_nodes[2] as ColorRect + var node: Sprite2D = renderer.entity_nodes[2] as Sprite2D if not _entity_uses_relationship(renderer): renderer.queue_free() return - assert_that(node.color).is_equal(Constants.ENTITY_COLOR_POI) + assert_that(node.self_modulate).is_equal(Constants.ENTITY_COLOR_POI) renderer.queue_free() func test_npc_uses_relationship_color_hostile() -> void: var renderer: Node2D = _make_entity_renderer() var entities: Array = [_make_entity(2, "Npc", "Hostile")] renderer.update_entities(entities) - var node: ColorRect = renderer.entity_nodes[2] as ColorRect + var node: Sprite2D = renderer.entity_nodes[2] as Sprite2D if not _entity_uses_relationship(renderer): renderer.queue_free() return - assert_that(node.color).is_equal(Constants.ENTITY_COLOR_HOSTILE) + assert_that(node.self_modulate).is_equal(Constants.ENTITY_COLOR_HOSTILE) renderer.queue_free() func test_player_color_ignores_relationship() -> void: @@ -179,8 +179,8 @@ func test_player_color_ignores_relationship() -> void: var renderer: Node2D = _make_entity_renderer() var entities: Array = [_make_entity(1, "Player", "Hostile")] renderer.update_entities(entities) - var node: ColorRect = renderer.entity_nodes[1] as ColorRect - assert_that(node.color).is_equal(Constants.ENTITY_COLOR_PLAYER) + var node: Sprite2D = renderer.entity_nodes[1] as Sprite2D + assert_that(node.self_modulate).is_equal(Constants.ENTITY_COLOR_PLAYER) renderer.queue_free() func test_object_color_ignores_relationship() -> void: @@ -188,8 +188,8 @@ func test_object_color_ignores_relationship() -> void: var renderer: Node2D = _make_entity_renderer() var entities: Array = [_make_entity(3, "Object", "Friendly")] renderer.update_entities(entities) - var node: ColorRect = renderer.entity_nodes[3] as ColorRect - assert_that(node.color).is_equal(Constants.ENTITY_COLOR_OBJECT) + var node: Sprite2D = renderer.entity_nodes[3] as Sprite2D + assert_that(node.self_modulate).is_equal(Constants.ENTITY_COLOR_OBJECT) renderer.queue_free() @@ -204,20 +204,20 @@ func test_color_shift_not_instant() -> void: var renderer: Node2D = _make_entity_renderer() var entities_before: Array = [_make_entity(2, "Npc", "Friendly")] renderer.update_entities(entities_before) - var node: ColorRect = renderer.entity_nodes[2] as ColorRect + var node: Sprite2D = renderer.entity_nodes[2] as Sprite2D if not _entity_uses_relationship(renderer): renderer.queue_free() return # Change relationship to Hostile var entities_after: Array = [_make_entity(2, "Npc", "Hostile")] renderer.update_entities(entities_after) - # Immediately after update, color should NOT yet be the target - var color_after_immediate: Color = node.color + # Immediately after update, self_modulate should NOT yet be the target + var color_after_immediate: Color = node.self_modulate if not _renderer_has_tween_support(renderer): push_warning("TestColorShift: tween on relationship change not implemented yet — awaiting #521") renderer.queue_free() return - # The color should NOT be exactly the target yet (tween in progress) + # The self_modulate should NOT be exactly the target yet (tween in progress) assert_that(color_after_immediate != Constants.ENTITY_COLOR_HOSTILE).is_true() renderer.queue_free() @@ -236,8 +236,8 @@ func test_color_shift_reaches_target() -> void: while elapsed < 1.5: renderer._process(1.0 / 60.0) elapsed += 1.0 / 60.0 - var node: ColorRect = renderer.entity_nodes[2] as ColorRect - assert_that(node.color.is_equal_approx(Constants.ENTITY_COLOR_HOSTILE)).is_true() + var node: Sprite2D = renderer.entity_nodes[2] as Sprite2D + assert_that(node.self_modulate.is_equal_approx(Constants.ENTITY_COLOR_HOSTILE)).is_true() renderer.queue_free() func test_color_shift_mid_transition_retrigger() -> void: @@ -263,8 +263,8 @@ func test_color_shift_mid_transition_retrigger() -> void: while elapsed < 1.0: renderer._process(1.0 / 60.0) elapsed += 1.0 / 60.0 - var node: ColorRect = renderer.entity_nodes[2] as ColorRect - assert_that(node.color.is_equal_approx(Constants.ENTITY_COLOR_HOSTILE)).is_true() + var node: Sprite2D = renderer.entity_nodes[2] as Sprite2D + assert_that(node.self_modulate.is_equal_approx(Constants.ENTITY_COLOR_HOSTILE)).is_true() renderer.queue_free() @@ -275,10 +275,10 @@ func test_color_shift_same_relationship_no_tween() -> void: return var entities: Array = [_make_entity(2, "Npc", "Unknown")] renderer.update_entities(entities) - var node: ColorRect = renderer.entity_nodes[2] as ColorRect - var color_first: Color = node.color + var node: Sprite2D = renderer.entity_nodes[2] as Sprite2D + var color_first: Color = node.self_modulate renderer.update_entities(entities) - var color_second: Color = node.color + var color_second: Color = node.self_modulate assert_that(color_first).is_equal(color_second) renderer.queue_free() @@ -354,10 +354,12 @@ func _entity_uses_relationship(renderer: Node2D) -> bool: renderer.update_entities([friendly, hostile]) if not renderer.entity_nodes.has(10) or not renderer.entity_nodes.has(11): return false - var f_node: ColorRect = renderer.entity_nodes[10] as ColorRect - var h_node: ColorRect = renderer.entity_nodes[11] as ColorRect - var f_color: Color = f_node.color - var h_color: Color = h_node.color + var f_node: Sprite2D = renderer.entity_nodes[10] as Sprite2D + var h_node: Sprite2D = renderer.entity_nodes[11] as Sprite2D + if not f_node or not h_node: + return false + var f_color: Color = f_node.self_modulate + var h_color: Color = h_node.self_modulate var uses_rel: bool = not f_color.is_equal_approx(h_color) renderer.update_entities([]) return uses_rel diff --git a/client/tests/test_sprite_integration.gd b/client/tests/test_sprite_integration.gd new file mode 100644 index 000000000..e9405c91a --- /dev/null +++ b/client/tests/test_sprite_integration.gd @@ -0,0 +1,195 @@ +## Sprint 16 #540: Sprite integration tests. +## Tests z-sorting with real sprites, 24x32 D-044 footprint within D-066 64x64 +## bounding box, sprite asset existence from #541, and fog shader independence. +## Spec refs: D-019, D-043, D-044, D-049, D-066, #540, #541. +class_name TestSpriteIntegration +extends GdUnitTestSuite + +var EntityRendererScript = load("res://scripts/rendering/entity_renderer.gd") + + +func before_test() -> void: + GameState.player_entity_id = 1 + GameState.player_position = Vector2.ZERO + GameState.visible_entities = [] + + +# -- Helpers ------------------------------------------------------------------- + +func _make_entity_renderer() -> Node2D: + var renderer = Node2D.new() + renderer.set_script(EntityRendererScript) + add_child(renderer) + return renderer + + +# -- Footprint constants: D-044 spec (S16-S01, S16-S02) ----------------------- + +func test_entity_footprint_matches_d044_spec() -> void: + # S16-S01: D-044 specifies 24x32 entity footprint within 32x32 visual tile. + # (64x64 source sprite scaled to 32px runtime at 2x retina per D-066). + assert_that(EntityRenderer.ENTITY_WIDTH).override_failure_message( + "D-044: ENTITY_WIDTH must be 24px" + ).is_equal(24) + assert_that(EntityRenderer.ENTITY_HEIGHT).override_failure_message( + "D-044: ENTITY_HEIGHT must be 32px" + ).is_equal(32) + + +func test_entity_footprint_within_d066_2x2_sim_tile_bounding_box() -> void: + # S16-S02: D-066 requires entity sprite footprint contained within 2x2 sim tile + # bounding box. At 32px/tile → 64x64px max. Entity must fit to keep interaction + # range (2 sim tiles) accurate with the tilted perspective. + var tile_2x: int = Constants.TILE_SIZE * 2 + assert_that(EntityRenderer.ENTITY_WIDTH <= tile_2x).override_failure_message( + "D-066: ENTITY_WIDTH %d must fit within 2x tile width %dpx" % [ + EntityRenderer.ENTITY_WIDTH, tile_2x] + ).is_true() + assert_that(EntityRenderer.ENTITY_HEIGHT <= tile_2x).override_failure_message( + "D-066: ENTITY_HEIGHT %d must fit within 2x tile height %dpx" % [ + EntityRenderer.ENTITY_HEIGHT, tile_2x] + ).is_true() + + +func test_entity_width_fits_within_single_tile() -> void: + # S16-S03: Entity width (24) < TILE_SIZE (32) → centered within tile. + # Ensures horizontal centering offset is positive and entity doesn't overflow. + assert_that(EntityRenderer.ENTITY_WIDTH < Constants.TILE_SIZE).override_failure_message( + "Entity width must be less than TILE_SIZE for centered layout" + ).is_true() + assert_that(EntityRenderer.ENTITY_OFFSET_X >= 0.0).override_failure_message( + "ENTITY_OFFSET_X must be non-negative for horizontal centering" + ).is_true() + + +# -- Pixel position (S16-S04) ------------------------------------------------- + +func test_entity_pixel_position_at_tile_3_7() -> void: + # S16-S04: Entity at tile (3.0, 7.0) → pixel position must be + # (3 * TILE_SIZE + ENTITY_OFFSET_X, 7 * TILE_SIZE + ENTITY_OFFSET_Y). + var renderer := _make_entity_renderer() + var entity := [{"entity_id": 10, "x": 3.0, "y": 7.0, "z": 0, + "kind": {"variant": "Npc", "data": null}}] + renderer.update_entities(entity) + var node = renderer.entity_nodes[10] + var expected_x := 3.0 * Constants.TILE_SIZE + EntityRenderer.ENTITY_OFFSET_X + var expected_y := 7.0 * Constants.TILE_SIZE + EntityRenderer.ENTITY_OFFSET_Y + assert_that(node.position.x).override_failure_message( + "Entity x must be tile_x * TILE_SIZE + ENTITY_OFFSET_X" + ).is_equal_approx(expected_x, 0.1) + assert_that(node.position.y).override_failure_message( + "Entity y must be tile_y * TILE_SIZE + ENTITY_OFFSET_Y" + ).is_equal_approx(expected_y, 0.1) + renderer.queue_free() + + +# -- Z-sort ordering: D-049 y-based (S16-S05, S16-S06) ----------------------- + +func test_z_sort_south_entity_has_higher_pixel_y() -> void: + # S16-S05: D-049 y-sort — entity at y=8 (south) must have higher pixel.y + # than entity at y=4 (north). Godot y-sort renders higher-y on top. + # With tilted sprites, south-facing entity must visually overlap northern. + var renderer := _make_entity_renderer() + var entities := [ + {"entity_id": 20, "x": 5.0, "y": 4.0, "z": 0, "kind": {"variant": "Npc", "data": null}}, + {"entity_id": 21, "x": 5.0, "y": 8.0, "z": 0, "kind": {"variant": "Npc", "data": null}}, + ] + renderer.update_entities(entities) + var north_node = renderer.entity_nodes[20] + var south_node = renderer.entity_nodes[21] + assert_that(south_node.position.y > north_node.position.y).override_failure_message( + "Entity at y=8 must have higher pixel.y than entity at y=4 for y-sort" + ).is_true() + renderer.queue_free() + + +func test_z_sort_y_position_difference_equals_tile_size() -> void: + # S16-S06: Two entities one tile apart in y → pixel y difference = TILE_SIZE. + # Verifies position calculation is consistent for adjacent tiles. + var renderer := _make_entity_renderer() + var entities := [ + {"entity_id": 30, "x": 5.0, "y": 3.0, "z": 0, "kind": {"variant": "Npc", "data": null}}, + {"entity_id": 31, "x": 5.0, "y": 4.0, "z": 0, "kind": {"variant": "Npc", "data": null}}, + ] + renderer.update_entities(entities) + var node3 = renderer.entity_nodes[30] + var node4 = renderer.entity_nodes[31] + var delta_y := node4.position.y - node3.position.y + assert_that(delta_y).override_failure_message( + "Adjacent tiles must differ by exactly TILE_SIZE (%dpx) in y" % Constants.TILE_SIZE + ).is_equal_approx(float(Constants.TILE_SIZE), 0.1) + renderer.queue_free() + + +func test_z_sort_same_y_different_x_no_y_difference() -> void: + # S16-S07: Two entities at same y but different x → same pixel.y. + # Horizontal position must not affect y-sort order. + var renderer := _make_entity_renderer() + var entities := [ + {"entity_id": 40, "x": 2.0, "y": 5.0, "z": 0, "kind": {"variant": "Npc", "data": null}}, + {"entity_id": 41, "x": 8.0, "y": 5.0, "z": 0, "kind": {"variant": "Npc", "data": null}}, + ] + renderer.update_entities(entities) + var left_node = renderer.entity_nodes[40] + var right_node = renderer.entity_nodes[41] + assert_that(left_node.position.y).override_failure_message( + "Entities at same y-tile must have same pixel.y regardless of x" + ).is_equal_approx(right_node.position.y, 0.1) + renderer.queue_free() + + +# -- Sprite assets from #541 (S16-S08, S16-S09) -------------------------------- + +func test_npc_sprite_assets_exist_for_all_cardinal_directions() -> void: + # S16-S08: #541 delivers 64px NPC sprites for all four cardinal directions. + # entity_renderer.gd must be able to load these paths. + for direction in ["north", "east", "south", "west"]: + var path := "res://assets/sprites/npc_generic_%s_64.png" % direction + assert_that(ResourceLoader.exists(path)).override_failure_message( + "NPC sprite missing: %s" % path + ).is_true() + + +func test_wall_sprite_assets_exist_for_all_cardinal_directions() -> void: + # S16-S09: #541 delivers 64px wall sprites for all four cardinal directions. + for direction in ["north", "east", "south", "west"]: + var path := "res://assets/sprites/wall_structural_%s_64.png" % direction + assert_that(ResourceLoader.exists(path)).override_failure_message( + "Wall sprite missing: %s" % path + ).is_true() + + +# -- Fog shader independence: D-019 (S16-S10, S16-S11) ----------------------- + +func test_fog_shader_script_and_gdshader_load_correctly() -> void: + # S16-S10: fog_shader.gd and fog.gdshader must remain intact after sprite + # changes. D-019: "fog vision cone math remains pure 2D" — unaffected by + # the art-direction tilt baked into sprites. + assert_that(ResourceLoader.exists("res://scripts/rendering/fog_shader.gd")).override_failure_message( + "fog_shader.gd must load correctly — must not be affected by sprite changes" + ).is_true() + assert_that(ResourceLoader.exists("res://shaders/fog.gdshader")).override_failure_message( + "fog.gdshader must exist — fog is screen-space and sprite-independent" + ).is_true() + + +func test_fog_update_runs_independently_of_entity_renderer_state() -> void: + # S16-S11: FogState.update_from_state() must succeed with no entity renderer + # active. D-019: fog driven by LOS mask (visible_positions), not sprites. + var fog = get_node_or_null("/root/FogState") + if fog == null: + push_warning("TestSpriteIntegration: FogState not available — fog independence test skipped") + return + # Provide visibility data but no entity renderer context + GameState.visible_positions = {Vector2i(5, 5): true, Vector2i(6, 5): true} + GameState.visibility_sectors = { + Vector2i(5, 5): "Forward", + Vector2i(6, 5): "Peripheral", + } + if fog.has_method("update_from_state"): + fog.update_from_state() + assert_that(fog.visibility_texture).override_failure_message( + "FogState visibility_texture must be populated independently of sprite state" + ).is_not_null() + GameState.visible_positions.clear() + GameState.visibility_sectors.clear() -- 2.54.0 From f67f13ac955fbae0ae81cd126ef152b4cc394fad Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 23 Feb 2026 21:01:48 +0100 Subject: [PATCH 3/5] chore(meta): update changelog Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc8d62e6c..2d9e81921 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Format based on [Keep a Changelog](https://keepachangelog.com/). ## [Unreleased] ### Added +- Entity renderer migrated from ColorRect placeholders to Sprite2D with D-019 angle sprites — self_modulate for D-033 tinting, 8→4 octant direction mapping, feet-anchored y-sort (#540) - 3D sprite render pipeline — Camera3D at D-019 angle (-72.5° from horizontal), three-point studio lighting rig, orthographic projection, resolution chain 1024→256→64 - Generic NPC capsule model (24×32px footprint per D-044) and structural wall model for pipeline validation - Test sprites: 8 runtime 64px sprites (NPC + wall × 4 directions) deployed to client/assets/sprites/ @@ -27,6 +28,7 @@ Format based on [Keep a Changelog](https://keepachangelog.com/). - SetFacing and TeleportToHub added to roundtrip test coverage ### Changed +- Zone_id extraction in game_state.gd optimized from O(N) tile scan to O(1) dictionary lookup — builds _tile_by_coord from member visible_tiles covering both test and live paths (#543) - Shared run_dialogue_pipeline() helper eliminates ~60 lines of duplication between Talk and DialogueResponse systems - DialogueCooldownTracker documented as per-player-global by design (NPC-scoped line IDs per D-035 prevent collision) - CONFRONTATION_LINES marked TODO for migration to D-028/D-035 content pipeline -- 2.54.0 From 56b8381c40c690518e53a7864d5821c2bec26f37 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 23 Feb 2026 21:06:29 +0100 Subject: [PATCH 4/5] fix(client): guard null texture and warn on unknown octant (#540 review) Add push_error on null texture at create time, keep previous texture on null at update time (entity stays visible mid-game). Add push_warning on unrecognised octant in _octant_to_direction fallback. Co-Authored-By: Claude Opus 4.6 --- client/scripts/rendering/entity_renderer.gd | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/client/scripts/rendering/entity_renderer.gd b/client/scripts/rendering/entity_renderer.gd index f32061438..91586f77f 100644 --- a/client/scripts/rendering/entity_renderer.gd +++ b/client/scripts/rendering/entity_renderer.gd @@ -109,7 +109,10 @@ func _create_entity_node(entity_id: int, entity_data: Dictionary) -> void: # Load sprite for current facing direction var direction := _entity_direction(entity_id, entity_data) _entity_facing[entity_id] = direction - entity_node.texture = _load_sprite_texture(direction) + var tex := _load_sprite_texture(direction) + if tex == null: + push_error("EntityRenderer: no texture for entity %d direction '%s' — entity will be invisible" % [entity_id, direction]) + entity_node.texture = tex # D-033: self_modulate for relationship tinting; modulate.a is reserved for D-015 dimming. entity_node.self_modulate = _color_for_kind(entity_data) @@ -153,7 +156,10 @@ func _update_entity_node(entity_id: int, entity_data: Dictionary) -> void: var new_dir := _entity_direction(entity_id, entity_data) if new_dir != _entity_facing.get(entity_id, ""): _entity_facing[entity_id] = new_dir - (node as Sprite2D).texture = _load_sprite_texture(new_dir) + var new_tex := _load_sprite_texture(new_dir) + if new_tex != null: + (node as Sprite2D).texture = new_tex + # null: keep previous texture rather than going invisible mid-game # #521: Detect relationship change → fade D-033 self_modulate (0.5s via _process) var new_rel: String = entity_data.get("relationship", "Unknown") @@ -218,7 +224,9 @@ static func _octant_to_direction(octant: String) -> String: "Northeast", "East": return "east" "Southeast", "South": return "south" "Southwest", "West": return "west" - _: return "south" + _: + push_warning("EntityRenderer: unrecognised octant '%s' — defaulting to south" % octant) + return "south" # Load the sprite texture for the given 4-direction key. -- 2.54.0 From 0ebd417a1f288e58527c0b07680515e8ae62aa9f Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 23 Feb 2026 21:06:35 +0100 Subject: [PATCH 5/5] test(client): add direction mapping tests for octant and entity facing (#540 review) 14 tests covering _octant_to_direction (all 8 octants + 2 fallbacks) and _entity_direction (NPC default south, player facing 3 cases). Closes review warning on zero test coverage for direction system. Co-Authored-By: Claude Opus 4.6 --- client/tests/test_sprite_integration.gd | 97 +++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/client/tests/test_sprite_integration.gd b/client/tests/test_sprite_integration.gd index e9405c91a..1a9e76e30 100644 --- a/client/tests/test_sprite_integration.gd +++ b/client/tests/test_sprite_integration.gd @@ -193,3 +193,100 @@ func test_fog_update_runs_independently_of_entity_renderer_state() -> void: ).is_not_null() GameState.visible_positions.clear() GameState.visibility_sectors.clear() + + +# -- Direction mapping: _octant_to_direction (S16-S12 through S16-S21) -------- + +func test_octant_north_maps_to_north() -> void: + # S16-S12: "North" → "north" + assert_that(EntityRenderer._octant_to_direction("North")).is_equal("north") + +func test_octant_northwest_maps_to_north() -> void: + # S16-S13: "Northwest" → "north" (grouped with North per mapping spec) + assert_that(EntityRenderer._octant_to_direction("Northwest")).is_equal("north") + +func test_octant_northeast_maps_to_east() -> void: + # S16-S14: "Northeast" → "east" + assert_that(EntityRenderer._octant_to_direction("Northeast")).is_equal("east") + +func test_octant_east_maps_to_east() -> void: + # S16-S15: "East" → "east" + assert_that(EntityRenderer._octant_to_direction("East")).is_equal("east") + +func test_octant_southeast_maps_to_south() -> void: + # S16-S16: "Southeast" → "south" + assert_that(EntityRenderer._octant_to_direction("Southeast")).is_equal("south") + +func test_octant_south_maps_to_south() -> void: + # S16-S17: "South" → "south" + assert_that(EntityRenderer._octant_to_direction("South")).is_equal("south") + +func test_octant_southwest_maps_to_west() -> void: + # S16-S18: "Southwest" → "west" + assert_that(EntityRenderer._octant_to_direction("Southwest")).is_equal("west") + +func test_octant_west_maps_to_west() -> void: + # S16-S19: "West" → "west" + assert_that(EntityRenderer._octant_to_direction("West")).is_equal("west") + +func test_octant_unknown_string_falls_back_to_south() -> void: + # S16-S20: Unknown string → "south" fallback (safe default — viewer-facing per D-019) + assert_that(EntityRenderer._octant_to_direction("Unknown")).is_equal("south") + assert_that(EntityRenderer._octant_to_direction("invalid")).is_equal("south") + +func test_octant_empty_string_falls_back_to_south() -> void: + # S16-S21: Empty string → "south" fallback + assert_that(EntityRenderer._octant_to_direction("")).is_equal("south") + + +# -- Direction mapping: _entity_direction (S16-S22 through S16-S25) ---------- + +func test_entity_direction_npc_always_south() -> void: + # S16-S22: NPC entity → always "south" regardless of any data field. + # NPCs have no facing in v1 entity format; south is viewer-facing (D-019 angle). + var renderer := _make_entity_renderer() + GameState.player_entity_id = 1 + # entity_id 99 is not the player + var dir := renderer._entity_direction(99, {"entity_id": 99, + "kind": {"variant": "Npc", "data": null}}) + assert_that(dir).override_failure_message( + "NPC entity must always return 'south'" + ).is_equal("south") + renderer.queue_free() + +func test_entity_direction_player_uses_player_facing() -> void: + # S16-S23: Player entity → uses GameState.player_facing via _octant_to_direction. + var renderer := _make_entity_renderer() + GameState.player_entity_id = 1 + GameState.player_facing = "North" + var dir := renderer._entity_direction(1, {"entity_id": 1, + "kind": {"variant": "Player", "data": null}}) + assert_that(dir).override_failure_message( + "Player entity with player_facing='North' must return 'north'" + ).is_equal("north") + renderer.queue_free() + +func test_entity_direction_player_facing_east() -> void: + # S16-S24: Player facing "East" → "east" + var renderer := _make_entity_renderer() + GameState.player_entity_id = 1 + GameState.player_facing = "East" + var dir := renderer._entity_direction(1, {"entity_id": 1, + "kind": {"variant": "Player", "data": null}}) + assert_that(dir).override_failure_message( + "Player entity with player_facing='East' must return 'east'" + ).is_equal("east") + renderer.queue_free() + +func test_entity_direction_player_facing_diagonal_uses_nearest_cardinal() -> void: + # S16-S25: Player facing "Northwest" → "north" (nearest cardinal mapping). + # Diagonal octants map to one of the four cardinal sprite sets. + var renderer := _make_entity_renderer() + GameState.player_entity_id = 1 + GameState.player_facing = "Northwest" + var dir := renderer._entity_direction(1, {"entity_id": 1, + "kind": {"variant": "Player", "data": null}}) + assert_that(dir).override_failure_message( + "Player entity with player_facing='Northwest' must return 'north'" + ).is_equal("north") + renderer.queue_free() -- 2.54.0