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

feat(client): Godot 4 project boilerplate (epic #277) #1

Closed
jpmschweitzer wants to merge 0 commits from client into main
pull from: client
merge into: :main
Conversation 3 Commits - Files Changed -
jpmschweitzer commented 2026-02-11 17:21:38 +01:00
Owner
Copy Link
Copy Source

Summary

  • Initialize complete Godot 4.6 client project as a pure renderer (D-020)
  • Project foundation: scenes, autoloads (SimBridge, GameState, InputMapper), rendering stubs, UI shell stubs, input system with semantic actions
  • gdUnit4 test framework installed with 2 passing smoke tests
  • make ci-client pipeline functional (lint → build → test)

Tickets closed

Epic #277 and stories #287–296 (all 10 child tickets)

Test plan

  • make ci-client passes green (lint, build, test)
  • godot4 --path client opens without errors
  • Main scene displays initialization message
  • gdUnit4 reports 2/2 tests passed

🤖 Generated with Claude Code

## Summary - Initialize complete Godot 4.6 client project as a pure renderer (D-020) - Project foundation: scenes, autoloads (SimBridge, GameState, InputMapper), rendering stubs, UI shell stubs, input system with semantic actions - gdUnit4 test framework installed with 2 passing smoke tests - `make ci-client` pipeline functional (lint → build → test) ## Tickets closed Epic #277 and stories #287–296 (all 10 child tickets) ## Test plan - [ ] `make ci-client` passes green (lint, build, test) - [ ] `godot4 --path client` opens without errors - [ ] Main scene displays initialization message - [ ] gdUnit4 reports 2/2 tests passed 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 4 commits 2026-02-11 17:21:39 +01:00
chore(client): add gdUnit4 test framework addon 61e0762a84 
Vendor gdUnit4 for Godot 4 client testing (D-030).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat(client): initialize Godot 4 project boilerplate (epic #277) 603c24e177 
Set up the complete Godot 4.6 client foundation as a pure renderer (D-020):
- project.godot with 2D rendering, autoloads, input actions
- Main scene: Game > World (TileMapLayer, Entities, FogOverlay) + Camera2D + UILayer
- Autoloads: SimBridge (connection state machine + test mode), GameState, InputMapper
- Rendering stubs: WorldRenderer, EntityRenderer, FogRenderer
- UI stubs: HUD (health/perception/time), Minimap, MonologueDisplay
- Input mapping: WASD/arrows, E (interact), Tab (perception), Esc (menu), Space (pause)
- gdUnit4 smoke tests: scene loads, autoloads registered (2/2 passing)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update Makefile and gitignore for Godot client CI b92b99f744 
- Implement lint-client, build-client, test-client Make targets
- Add GODOT variable for binary detection
- Gitignore .godot cache, export artifacts, test reports
- Clean target handles Godot artifacts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(db): mark client boilerplate tickets done (#277, #287-296) f2d202f1ad 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-11 17:41:58 +01:00
Author
Owner
Copy Link
Copy Source

Dual-Agent Review: origin/client -> main

Hoshe (Code Quality): REQUEST_CHANGES

Summary: Solid foundation but critical bugs and test gaps need fixing before merge.

# File Severity Issue
1 scripts/autoloads/input_mapper.gd:8-20 critical Double-check of pressed state may prevent input capture — event.pressed filter + is_action_pressed() is redundant
2 scripts/rendering/entity_renderer.gd:55-57 critical No bounds checking on pos[0]/pos[1] — crashes on malformed snapshot data
3 scripts/autoloads/game_state.gd:14-15 critical Same array access vulnerability — Vector2(pos[0], pos[1]) with no validation
4 tests/ critical Only 1 smoke test — missing D-030 Layer 1 fixture tests for snapshot parsing
5 scripts/autoloads/sim_bridge.gd:63 warning Time.get_ticks_msec() makes test snapshot non-deterministic (violates D-010 principle 4)
6 ui/monologue_display.gd:31-33 warning Tween not stored/cancelled — potential parallel tweens on same property
7 All rendering scripts warning No error logging for malformed data — silent failures make IPC debugging difficult
8 scripts/rendering/entity_renderer.gd:57 suggestion Magic number 32px tile size needs a shared constant

Tyre (Architecture): APPROVE (with suggestions)

Summary: Exactly the right amount of architecture for a first commit. The D-020 contract is correctly implemented: client does zero simulation logic.

# File Severity Issue
1 addons/gdUnit4/ warning 979 vendor files committed — use git submodule or strip addons/gdUnit4/test/ directory
2 scripts/main.gd suggestion MonologueDisplay not wired into main loop — add stub for D-016 data path
3 scenes/main.tscn suggestion Camera2D not tracking player position (D-015 requires it)
4 scripts/autoloads/input_mapper.gd suggestion Event-driven input won't support held movement — add frame-polled input for WASD or document as placeholder
5 scripts/autoloads/sim_bridge.gd suggestion Test tick should use incrementing counter, not wall clock
6 ui/minimap.tscn warning SubViewportContainer has no SubViewport child — will produce editor warnings

Required before merge:

  1. Fix input_mapper double-check bug
  2. Add defensive bounds checking to all array access in entity_renderer and game_state
  3. Add D-030 Layer 1 fixture tests for snapshot parsing
  4. Make test_mode snapshot deterministic
  5. Add error logging for malformed data handling
## Dual-Agent Review: origin/client -> main ### Hoshe (Code Quality): REQUEST_CHANGES **Summary:** Solid foundation but critical bugs and test gaps need fixing before merge. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | scripts/autoloads/input_mapper.gd:8-20 | critical | Double-check of pressed state may prevent input capture — `event.pressed` filter + `is_action_pressed()` is redundant | | 2 | scripts/rendering/entity_renderer.gd:55-57 | critical | No bounds checking on `pos[0]`/`pos[1]` — crashes on malformed snapshot data | | 3 | scripts/autoloads/game_state.gd:14-15 | critical | Same array access vulnerability — `Vector2(pos[0], pos[1])` with no validation | | 4 | tests/ | critical | Only 1 smoke test — missing D-030 Layer 1 fixture tests for snapshot parsing | | 5 | scripts/autoloads/sim_bridge.gd:63 | warning | `Time.get_ticks_msec()` makes test snapshot non-deterministic (violates D-010 principle 4) | | 6 | ui/monologue_display.gd:31-33 | warning | Tween not stored/cancelled — potential parallel tweens on same property | | 7 | All rendering scripts | warning | No error logging for malformed data — silent failures make IPC debugging difficult | | 8 | scripts/rendering/entity_renderer.gd:57 | suggestion | Magic number 32px tile size needs a shared constant | ### Tyre (Architecture): APPROVE (with suggestions) **Summary:** Exactly the right amount of architecture for a first commit. The D-020 contract is correctly implemented: client does zero simulation logic. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | addons/gdUnit4/ | warning | 979 vendor files committed — use git submodule or strip `addons/gdUnit4/test/` directory | | 2 | scripts/main.gd | suggestion | MonologueDisplay not wired into main loop — add stub for D-016 data path | | 3 | scenes/main.tscn | suggestion | Camera2D not tracking player position (D-015 requires it) | | 4 | scripts/autoloads/input_mapper.gd | suggestion | Event-driven input won't support held movement — add frame-polled input for WASD or document as placeholder | | 5 | scripts/autoloads/sim_bridge.gd | suggestion | Test tick should use incrementing counter, not wall clock | | 6 | ui/minimap.tscn | warning | SubViewportContainer has no SubViewport child — will produce editor warnings | ### Required before merge: 1. Fix input_mapper double-check bug 2. Add defensive bounds checking to all array access in entity_renderer and game_state 3. Add D-030 Layer 1 fixture tests for snapshot parsing 4. Make test_mode snapshot deterministic 5. Add error logging for malformed data handling
jpmschweitzer added 1 commit 2026-02-11 17:53:26 +01:00
fix(client): address PR review — input bug, bounds checks, fixture tests 3368231396 
- Fix input_mapper double-check bug (redundant InputEventKey + pressed filter)
- Add bounds checking for position arrays in entity_renderer and game_state
- Make test snapshot deterministic (incrementing counter, not wall clock)
- Fix tween overlap in monologue_display (cancel active tween before new one)
- Extract TILE_SIZE constant from magic number 32
- Add 5 D-030 Layer 1 fixture tests for snapshot parsing (7/7 total passing)

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

Dual-Agent Review (Round 2): origin/client -> main

Hoshe (Code Quality): APPROVE

All 8 previous critical/warning issues properly addressed in commit 3368231. Bounds validation added, input bug fixed, test snapshots deterministic, tween cancellation implemented, magic constant extracted, 7 fixture tests covering happy path + edge cases. No new critical issues.

# File Severity Issue
1 input_mapper.gd:35 suggestion Input timestamp still uses Time.get_ticks_msec() (acceptable -- server will assign tick numbers during IPC)
2 entity_renderer.gd suggestion Bounds checking code path not directly tested (deferred to D-030 Layer 2)

Tyre (Architecture): REQUEST_CHANGES

Architecture is 85% solid -- D-020 separation is correct, D-030 testability implemented. But 3 critical wiring gaps remain:

# File Severity Issue
1 ui/minimap.tscn critical SubViewportContainer has no SubViewport child -- nonfunctional structure, breaks D-015
2 scripts/main.gd critical Camera2D never tracks player_position -- character walks offscreen, breaks D-015
3 ui/monologue_display.gd critical MonologueDisplay exists but never called -- D-016 core perception system orphaned
4 scripts/autoloads/input_mapper.gd warning Event-driven input only fires once per press -- WASD movement requires held-key polling
5 addons/gdUnit4/ warning 979 vendor files committed directly -- use git submodule for maintenance
6 rendering/fog_renderer.gd suggestion D-011 stub is fine for boilerplate, but track implementation with CRITICAL ticket
7 autoloads/sim_bridge.gd suggestion test_mode hardcoded true -- parameterize before IPC work begins

Verdict: CHANGES REQUESTED

Fix the 3 critical wiring issues (camera tracking, SubViewport child, monologue wiring) before merge. Estimated 2-4 hours.

## Dual-Agent Review (Round 2): origin/client -> main ### Hoshe (Code Quality): APPROVE All 8 previous critical/warning issues properly addressed in commit 3368231. Bounds validation added, input bug fixed, test snapshots deterministic, tween cancellation implemented, magic constant extracted, 7 fixture tests covering happy path + edge cases. No new critical issues. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | input_mapper.gd:35 | suggestion | Input timestamp still uses Time.get_ticks_msec() (acceptable -- server will assign tick numbers during IPC) | | 2 | entity_renderer.gd | suggestion | Bounds checking code path not directly tested (deferred to D-030 Layer 2) | ### Tyre (Architecture): REQUEST_CHANGES Architecture is 85% solid -- D-020 separation is correct, D-030 testability implemented. But 3 critical wiring gaps remain: | # | File | Severity | Issue | |---|------|----------|-------| | 1 | ui/minimap.tscn | critical | SubViewportContainer has no SubViewport child -- nonfunctional structure, breaks D-015 | | 2 | scripts/main.gd | critical | Camera2D never tracks player_position -- character walks offscreen, breaks D-015 | | 3 | ui/monologue_display.gd | critical | MonologueDisplay exists but never called -- D-016 core perception system orphaned | | 4 | scripts/autoloads/input_mapper.gd | warning | Event-driven input only fires once per press -- WASD movement requires held-key polling | | 5 | addons/gdUnit4/ | warning | 979 vendor files committed directly -- use git submodule for maintenance | | 6 | rendering/fog_renderer.gd | suggestion | D-011 stub is fine for boilerplate, but track implementation with CRITICAL ticket | | 7 | autoloads/sim_bridge.gd | suggestion | test_mode hardcoded true -- parameterize before IPC work begins | ### Verdict: CHANGES REQUESTED Fix the 3 critical wiring issues (camera tracking, SubViewport child, monologue wiring) before merge. Estimated 2-4 hours.
jpmschweitzer added 1 commit 2026-02-11 18:24:14 +01:00
fix(client): address PR review round 2 — camera tracking, minimap, monologue wiring 57a67ecb0b 
- Camera2D now tracks player_position from GameState (D-015)
- Add SubViewport child to minimap SubViewportContainer (fixes editor warning)
- Wire MonologueDisplay into main loop for D-016 perception data path

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-11 18:27:07 +01:00
Author
Owner
Copy Link
Copy Source

Dual-Agent Review (Round 3): origin/client -> main

Hoshe (Code Quality): APPROVE

All 3 critical issues from round 2 resolved. SubViewport hierarchy correct, camera tracking implemented with tile-to-pixel scaling, monologue wired into perception data path.

# File Severity Issue
1 scripts/main.gd:25 warning Missing null check on camera before position assignment (other nodes have null checks)
2 scripts/main.gd:35 warning No type validation on snapshot.monologue before passing to show_monologue(text: String)
3 scripts/main.gd:24-35 suggestion Camera updates after world renderer -- one-frame lag likely imperceptible

Tyre (Architecture): APPROVE

All 3 critical fixes satisfy D-015 and D-016 requirements. Camera locked to character, SubViewport properly structured, monologue data path wired. D-020 client-server separation maintained. No new architectural concerns.

Verdict: APPROVED

Both reviewers approve. Merging.

## Dual-Agent Review (Round 3): origin/client -> main ### Hoshe (Code Quality): APPROVE All 3 critical issues from round 2 resolved. SubViewport hierarchy correct, camera tracking implemented with tile-to-pixel scaling, monologue wired into perception data path. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | scripts/main.gd:25 | warning | Missing null check on camera before position assignment (other nodes have null checks) | | 2 | scripts/main.gd:35 | warning | No type validation on snapshot.monologue before passing to show_monologue(text: String) | | 3 | scripts/main.gd:24-35 | suggestion | Camera updates after world renderer -- one-frame lag likely imperceptible | ### Tyre (Architecture): APPROVE All 3 critical fixes satisfy D-015 and D-016 requirements. Camera locked to character, SubViewport properly structured, monologue data path wired. D-020 client-server separation maintained. No new architectural concerns. ### Verdict: APPROVED Both reviewers approve. Merging.
jpmschweitzer closed this pull request 2026-02-11 18:27:27 +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#1