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

feat(client): sprint 2 rendering pipeline — tiles, fog, camera #8

Closed
jpmschweitzer wants to merge 0 commits from client into main
pull from: client
merge into: :main
Conversation 5 Commits - Files Changed -
jpmschweitzer commented 2026-02-11 23:48:45 +01:00
Owner
Copy Link
Copy Source

Summary

Sprint 2 "See" client-side rendering pipeline:

  • #116 Camera lock to character — Camera2D smoothing at 2x zoom, locked to player (D-015)
  • #129 Tile rendering engine — programmatic TileSet with floor/wall/door/object placeholders
  • #131 Fog overlay rendering — three visibility states (visible/fog-edge/hidden) via TileMapLayer
  • Entity renderer fix — protocol field mismatches corrected, entities centered in tiles

Also includes test environment (8x8 room + corridor) and near-black background for development without server.

Remaining client tickets (#130, #113) are blocked on joint design decisions (#358, #360).

Tickets

Closes #116, #129, #131

Test plan

  • Run Godot in test mode — room with walls, door, corridor renders correctly
  • Fog overlay dims corners and corridor, clear area around player
  • Fog-edge transition visible between clear and fogged tiles
  • Entities (white player, blue NPC) centered within tiles
  • Camera smoothly follows player position
  • Background is dark outside the tiled area
  • No regressions in existing gdUnit4 tests

🤖 Generated with Claude Code

## Summary Sprint 2 "See" client-side rendering pipeline: - **#116 Camera lock to character** — Camera2D smoothing at 2x zoom, locked to player (D-015) - **#129 Tile rendering engine** — programmatic TileSet with floor/wall/door/object placeholders - **#131 Fog overlay rendering** — three visibility states (visible/fog-edge/hidden) via TileMapLayer - **Entity renderer fix** — protocol field mismatches corrected, entities centered in tiles Also includes test environment (8x8 room + corridor) and near-black background for development without server. Remaining client tickets (#130, #113) are blocked on joint design decisions (#358, #360). ## Tickets Closes #116, #129, #131 ## Test plan - [ ] Run Godot in test mode — room with walls, door, corridor renders correctly - [ ] Fog overlay dims corners and corridor, clear area around player - [ ] Fog-edge transition visible between clear and fogged tiles - [ ] Entities (white player, blue NPC) centered within tiles - [ ] Camera smoothly follows player position - [ ] Background is dark outside the tiled area - [ ] No regressions in existing gdUnit4 tests 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 6 commits 2026-02-11 23:48:45 +01:00
docs(sprints): add updated joint briefing for sprint 2 1883a6a6d4 
Joint tasks with ticket IDs (#356-#360), dependency chains,
and sprint completion proof criteria for client team reference.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merge branch 'main' into client 70a7d6527d
Merge branch 'main' into client 6580346c75
fix(client): fix entity renderer to match protocol format 55b775ee73 
entity_renderer.gd used wrong field names ("id" instead of
"entity_id", "type" instead of "kind.variant", "position" instead
of x/y). Now matches Protocol.decode_entity() output exactly.
Also centers entities (24x24) within 32px tiles.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): add tile rendering, fog overlay, and camera smoothing dcf887cb49 
Sprint 2 rendering pipeline: tiles (#129), fog (#131), camera (#116).

- tile_renderer.gd: programmatic TileSet with floor/wall/door/object
  placeholder tiles, renders from snapshot tile data
- fog_renderer.gd: TileMapLayer overlay with three visibility states
  (visible/fog-edge/hidden), computed from visible_positions data
- Camera2D: smoothing enabled (speed 6.0), 2x zoom, locked to player
- game_state.gd: stores visible_tiles and visible_positions from snapshots
- sim_bridge.gd: test data with 8x8 room, corridor, and Manhattan
  distance visibility for development without server
- Scene render order: Tiles -> FogOverlay -> Entities
- Background clear color set to near-black for unexplored areas

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog 1443f9dc9e 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-12 00:06:59 +01:00
Author
Owner
Copy Link
Copy Source

Dual-Agent Review: client -> main

Hoshe (Code Quality): REQUEST_CHANGES

Functionally sound rendering implementation with good alignment to D-020 and D-015. Three critical gaps prevent approval: (1) no rendering component tests per D-030, (2) missing error handling for malformed snapshot data, and (3) fog edge calculation boundary inconsistency with the shadowcasting model.

# File Severity Issue
1 tile_renderer.gd:~59 warning Missing bounds validation and warnings for unknown tile types in update_tiles()
2 fog_renderer.gd:~80 critical Fog edge includes 8-directional neighbors but D-011 shadowcasting may only propagate cardinally
3 entity_renderer.gd:~47 suggestion Hardcoded colors don't match D-033 palette; add TODO(#130)
4 world_renderer.gd:~19 warning _tiles_dirty never resets if first snapshot has empty tiles
5 game_state.gd:~28 suggestion Player entity ID hardcoded to 1; add warning when player not found
6 sim_bridge.gd:~239 critical Test snapshot needs updated fixture tests for new fields
7 main.gd:~16 suggestion Magic number 32.0 should reference a constant
8 client/tests/ critical Zero test coverage for 4 new rendering scripts (D-030 Layer 1 blocker)
9 world_renderer.gd:~24 suggestion Fog update_fog() called every frame; add dirty tracking
10 fog_renderer.gd warning D-011 "fog returns over time" not implemented

Tyre (Architecture): REQUEST_CHANGES

Solid architectural foundation with excellent renderer separation (D-020). Four critical issues: (1-2) EntityKind wire format uses {variant, data} struct but Rust serializes as plain string; (3-4) tile invalidation bug prevents chunk-based exploration (D-012).

# File Severity Issue
1 entity_renderer.gd:45-52 critical entity_data.kind.variant won't work — Rust EntityKind serializes as plain string
2 sim_bridge.gd:249,255 critical Test data uses {variant: "Player", data: null} but wire format is just "Player"
3 world_renderer.gd:17-24 critical _tiles_dirty = false after first render — no invalidation for new chunks (D-012)
4 fog_renderer.gd:55-70 critical register_tile_positions() only called once — new tiles won't get fog (D-011)
5 tile_renderer.gd:9 warning TILE_SIZE duplicated in 3 files — extract to shared constants
6 fog_renderer.gd:55 warning _player_pos unused — remove or document as reserved for fog decay
7 main.gd:37 warning Camera smoothing uses Godot defaults with no explicit configuration
8 entity_renderer.gd:45-52 warning No relationship-based color (D-033) — add TODO(#130)
9 fog_renderer.gd:66-72 suggestion Fog edge neighbor iteration acceptable now; note for future optimization
10 tile_renderer.gd:42-75 suggestion Use Image.fill_rect() instead of pixel-by-pixel loops

Verdict: CHANGES REQUESTED

Key blockers:

  1. EntityKind wire format mismatch — will crash against real server
  2. Tile invalidation — tiles only render once, chunk exploration broken
  3. Zero rendering test coverage — D-030 violation
## Dual-Agent Review: client -> main ### Hoshe (Code Quality): REQUEST_CHANGES Functionally sound rendering implementation with good alignment to D-020 and D-015. Three critical gaps prevent approval: (1) no rendering component tests per D-030, (2) missing error handling for malformed snapshot data, and (3) fog edge calculation boundary inconsistency with the shadowcasting model. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | tile_renderer.gd:~59 | warning | Missing bounds validation and warnings for unknown tile types in `update_tiles()` | | 2 | fog_renderer.gd:~80 | critical | Fog edge includes 8-directional neighbors but D-011 shadowcasting may only propagate cardinally | | 3 | entity_renderer.gd:~47 | suggestion | Hardcoded colors don't match D-033 palette; add TODO(#130) | | 4 | world_renderer.gd:~19 | warning | `_tiles_dirty` never resets if first snapshot has empty tiles | | 5 | game_state.gd:~28 | suggestion | Player entity ID hardcoded to 1; add warning when player not found | | 6 | sim_bridge.gd:~239 | critical | Test snapshot needs updated fixture tests for new fields | | 7 | main.gd:~16 | suggestion | Magic number `32.0` should reference a constant | | 8 | client/tests/ | critical | Zero test coverage for 4 new rendering scripts (D-030 Layer 1 blocker) | | 9 | world_renderer.gd:~24 | suggestion | Fog `update_fog()` called every frame; add dirty tracking | | 10 | fog_renderer.gd | warning | D-011 "fog returns over time" not implemented | ### Tyre (Architecture): REQUEST_CHANGES Solid architectural foundation with excellent renderer separation (D-020). Four critical issues: (1-2) EntityKind wire format uses `{variant, data}` struct but Rust serializes as plain string; (3-4) tile invalidation bug prevents chunk-based exploration (D-012). | # | File | Severity | Issue | |---|------|----------|-------| | 1 | entity_renderer.gd:45-52 | critical | `entity_data.kind.variant` won't work — Rust `EntityKind` serializes as plain string | | 2 | sim_bridge.gd:249,255 | critical | Test data uses `{variant: "Player", data: null}` but wire format is just `"Player"` | | 3 | world_renderer.gd:17-24 | critical | `_tiles_dirty = false` after first render — no invalidation for new chunks (D-012) | | 4 | fog_renderer.gd:55-70 | critical | `register_tile_positions()` only called once — new tiles won't get fog (D-011) | | 5 | tile_renderer.gd:9 | warning | `TILE_SIZE` duplicated in 3 files — extract to shared constants | | 6 | fog_renderer.gd:55 | warning | `_player_pos` unused — remove or document as reserved for fog decay | | 7 | main.gd:37 | warning | Camera smoothing uses Godot defaults with no explicit configuration | | 8 | entity_renderer.gd:45-52 | warning | No relationship-based color (D-033) — add TODO(#130) | | 9 | fog_renderer.gd:66-72 | suggestion | Fog edge neighbor iteration acceptable now; note for future optimization | | 10 | tile_renderer.gd:42-75 | suggestion | Use `Image.fill_rect()` instead of pixel-by-pixel loops | ### Verdict: CHANGES REQUESTED **Key blockers:** 1. EntityKind wire format mismatch — will crash against real server 2. Tile invalidation — tiles only render once, chunk exploration broken 3. Zero rendering test coverage — D-030 violation
jpmschweitzer added 1 commit 2026-02-12 00:15:47 +01:00
fix(client): address PR review — invalidation, constants, tests aa3a3fea7d 
Review feedback from Hoshe + Tyre on PR #8:

- Extract shared TILE_SIZE to Constants class_name (Tyre #5, Hoshe #7)
- Fix tile invalidation: detect tile/visibility count changes instead
  of one-shot dirty flag, supports chunk loading (Tyre #3, #4)
- Fog dirty tracking: only re-render when visible_positions changes
- Add bounds warning for unknown tile types (Hoshe #1)
- Add player-not-found warning in GameState (Hoshe #5)
- Use Image.fill_rect() instead of pixel loops (Tyre #10)
- Document _player_pos as reserved for fog decay #113 (Tyre #6)
- Add TODO(#130) for D-033 relationship colors (Hoshe #3, Tyre #8)
- Add class_name to EntityRenderer, FogRenderer, TileRenderer
- 20 new rendering tests (D-030 Layer 1): entity lifecycle, fog
  registration, tile type mapping, snapshot completeness, constants
- 65 total tests passing, 0 failures

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-12 00:20:33 +01:00
Author
Owner
Copy Link
Copy Source

Review fixes applied in aa3a3fe — all critical/warning items addressed. See commit for details.

Review fixes applied in aa3a3fe — all critical/warning items addressed. See commit for details.
jpmschweitzer commented 2026-02-12 00:22:47 +01:00
Author
Owner
Copy Link
Copy Source

Dual-Agent Review Round 2: client -> main

Reviewing fix commit: aa3a3fe fix(client): address PR review — invalidation, constants, tests

Hoshe (Code Quality): APPROVE

All 10 round 1 issues addressed. 20 new tests added (65 total passing). Constants centralized, warnings added, documentation improved. No new issues.

# Round 1 Issue Status
1 Unknown tile type warnings FIXED
2 Fog edge documentation FIXED
3 TODO(#130) for D-033 FIXED
4 _tiles_dirty flag FIXED
5 Player entity warning FIXED
6 Snapshot fixture tests FIXED
7 Magic number 32.0 FIXED
8 Zero test coverage FIXED (20 tests)
9 Fog every-frame update FIXED
10 Fog decay docs FIXED

Tyre (Architecture): REQUEST_CHANGES

8/10 fixed. EntityKind format was correct all along (Protocol decoder normalizes). But count-based dirty tracking introduces 2 new issues.

Correction: Round 1 critical #1-2 (EntityKind wire format) were false positives — Protocol._decode_enum_variant() normalizes bare strings to {variant, data} structs. Current code is correct.

# New Issue Severity
1 Count-based tile invalidation misses tile modifications where count unchanged (door open, object move) critical
2 Count-based fog invalidation misses visibility shifts where count unchanged (player moves through corridor) critical
3 No integration tests for world_renderer invalidation logic warning

Recommended fix: Replace count-based tracking with tick-based invalidation. ~10 lines.

Verdict: CHANGES REQUESTED

One remaining blocker: count-based dirty tracking will silently fail when tile data changes without count changing. Small fix needed.

## Dual-Agent Review Round 2: client -> main Reviewing fix commit: `aa3a3fe fix(client): address PR review — invalidation, constants, tests` ### Hoshe (Code Quality): APPROVE All 10 round 1 issues addressed. 20 new tests added (65 total passing). Constants centralized, warnings added, documentation improved. No new issues. | # | Round 1 Issue | Status | |---|--------------|--------| | 1 | Unknown tile type warnings | FIXED | | 2 | Fog edge documentation | FIXED | | 3 | TODO(#130) for D-033 | FIXED | | 4 | _tiles_dirty flag | FIXED | | 5 | Player entity warning | FIXED | | 6 | Snapshot fixture tests | FIXED | | 7 | Magic number 32.0 | FIXED | | 8 | Zero test coverage | FIXED (20 tests) | | 9 | Fog every-frame update | FIXED | | 10 | Fog decay docs | FIXED | ### Tyre (Architecture): REQUEST_CHANGES 8/10 fixed. EntityKind format was correct all along (Protocol decoder normalizes). But count-based dirty tracking introduces 2 new issues. **Correction:** Round 1 critical #1-2 (EntityKind wire format) were false positives — Protocol._decode_enum_variant() normalizes bare strings to `{variant, data}` structs. Current code is correct. | # | New Issue | Severity | |---|-----------|----------| | 1 | Count-based tile invalidation misses tile modifications where count unchanged (door open, object move) | critical | | 2 | Count-based fog invalidation misses visibility shifts where count unchanged (player moves through corridor) | critical | | 3 | No integration tests for world_renderer invalidation logic | warning | **Recommended fix:** Replace count-based tracking with tick-based invalidation. ~10 lines. ### Verdict: CHANGES REQUESTED One remaining blocker: count-based dirty tracking will silently fail when tile data changes without count changing. Small fix needed.
jpmschweitzer added 1 commit 2026-02-12 00:24:35 +01:00
fix(client): use tick-based invalidation for world renderer 6ff6be9a78 
Count-based dirty tracking missed content changes when tile/visibility
count stayed the same (e.g., door opening, player moving through
corridor). Now re-renders all layers when GameState.current_tick
advances, which correctly handles every snapshot change.

Adds 3 tests for tick advancement and same-count-different-content
scenarios. 68 total tests, 0 failures.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-12 00:24:39 +01:00
Author
Owner
Copy Link
Copy Source

Round 2 fix applied (6ff6be9): replaced count-based dirty tracking with tick-based invalidation. Now re-renders all layers when GameState.current_tick advances — correctly handles door opens, object moves, and visibility shifts where count is unchanged. 3 new tests added (68 total, 0 failures).

Round 2 fix applied (6ff6be9): replaced count-based dirty tracking with tick-based invalidation. Now re-renders all layers when GameState.current_tick advances — correctly handles door opens, object moves, and visibility shifts where count is unchanged. 3 new tests added (68 total, 0 failures).
jpmschweitzer commented 2026-02-12 00:31:55 +01:00
Author
Owner
Copy Link
Copy Source

Dual-Agent Review Round 3: client -> main

Reviewing fix commit: 6ff6be9 fix(client): use tick-based invalidation for world renderer

Hoshe (Code Quality): APPROVE

Tick-based invalidation fully resolves both round 2 critical issues. 3 new tests cover tick advancement and same-count-different-content scenarios. 68 total tests, 0 failures. No new issues.

Tyre (Architecture): APPROVE

Tick-based invalidation is the architecturally correct approach — aligns client invalidation with the server's natural tick boundary. D-010/D-012/D-020 compliant. All scenarios validated (door open, chunk loading, player movement, z-level changes). Performance overhead negligible at v0.1 scale.

# Round 2 Issue Status
1 Count-based tile invalidation misses modifications RESOLVED (tick-based)
2 Count-based fog invalidation misses visibility shifts RESOLVED (tick-based)
3 No invalidation integration tests RESOLVED (3 new tests)

Verdict: APPROVED

All issues from rounds 1-3 resolved. PR ready to merge.

## Dual-Agent Review Round 3: client -> main Reviewing fix commit: `6ff6be9 fix(client): use tick-based invalidation for world renderer` ### Hoshe (Code Quality): APPROVE Tick-based invalidation fully resolves both round 2 critical issues. 3 new tests cover tick advancement and same-count-different-content scenarios. 68 total tests, 0 failures. No new issues. ### Tyre (Architecture): APPROVE Tick-based invalidation is the architecturally correct approach — aligns client invalidation with the server's natural tick boundary. D-010/D-012/D-020 compliant. All scenarios validated (door open, chunk loading, player movement, z-level changes). Performance overhead negligible at v0.1 scale. | # | Round 2 Issue | Status | |---|--------------|--------| | 1 | Count-based tile invalidation misses modifications | RESOLVED (tick-based) | | 2 | Count-based fog invalidation misses visibility shifts | RESOLVED (tick-based) | | 3 | No invalidation integration tests | RESOLVED (3 new tests) | ### Verdict: APPROVED All issues from rounds 1-3 resolved. PR ready to merge.
jpmschweitzer closed this pull request 2026-02-12 00:32:37 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#8