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

feat(simulation): sprint 9 gauntlet — test infrastructure and first 3 rooms #32

Closed
jpmschweitzer wants to merge 0 commits from server into main
pull from: server
merge into: :main
Conversation 2 Commits - Files Changed -
jpmschweitzer commented 2026-02-18 02:26:23 +01:00
Owner
Copy Link
Copy Source

Summary

  • Gauntlet test world (server/src/test_world/): 3 rooms (Inventory Warehouse, Occlusion Corridor, Pause Chamber) + Central Hub with hybrid Rust room definitions, WalkabilityMap + ECS entity spawning, and StableId ranges
  • Room constants module (constants.rs): 8 room constants, room_at() lookup, stable coordinate references for all Gauntlet tests
  • Room reset trigger (reset.rs): RoomResetTrigger component, RoomSnapshots resource, 10-tick debounce, "Reset" verb wired into input system
  • Layer 3 subprocess test (server/tests/layer3.rs): real subprocess integration — spawns server binary, connects TCP, exchanges snapshot, asserts protocol version (D-030)
  • Golden file comparison (tooling/test-client/src/golden.rs): recursive diff_json_values() with sorted keys, field-level diff output, 14 unit tests
  • Golden file test suite (server/tests/golden_suite.rs): 10-tick deterministic replay against committed golden file, UPDATE_GOLDEN=1 env var for regeneration
  • Content runtime validation (server/tests/content_loading.rs): boots full plugin stack with production content, ticks 10 times, asserts valid snapshot

Tickets: #482, #484, #485, #487, #488, #489, #490
449 tests passing, 0 failures.

Test plan

  • cargo nextest run — all 449 tests pass
  • cargo nextest run layer3 — subprocess test green
  • cargo nextest run golden_suite — golden file matches
  • cargo nextest run content_loading — runtime validation passes
  • Gauntlet rooms load without panic in test mode

🤖 Generated with Claude Code

## Summary - **Gauntlet test world** (`server/src/test_world/`): 3 rooms (Inventory Warehouse, Occlusion Corridor, Pause Chamber) + Central Hub with hybrid Rust room definitions, WalkabilityMap + ECS entity spawning, and StableId ranges - **Room constants module** (`constants.rs`): 8 room constants, `room_at()` lookup, stable coordinate references for all Gauntlet tests - **Room reset trigger** (`reset.rs`): `RoomResetTrigger` component, `RoomSnapshots` resource, 10-tick debounce, "Reset" verb wired into input system - **Layer 3 subprocess test** (`server/tests/layer3.rs`): real subprocess integration — spawns server binary, connects TCP, exchanges snapshot, asserts protocol version (D-030) - **Golden file comparison** (`tooling/test-client/src/golden.rs`): recursive `diff_json_values()` with sorted keys, field-level diff output, 14 unit tests - **Golden file test suite** (`server/tests/golden_suite.rs`): 10-tick deterministic replay against committed golden file, `UPDATE_GOLDEN=1` env var for regeneration - **Content runtime validation** (`server/tests/content_loading.rs`): boots full plugin stack with production content, ticks 10 times, asserts valid snapshot Tickets: #482, #484, #485, #487, #488, #489, #490 449 tests passing, 0 failures. ## Test plan - [ ] `cargo nextest run` — all 449 tests pass - [ ] `cargo nextest run layer3` — subprocess test green - [ ] `cargo nextest run golden_suite` — golden file matches - [ ] `cargo nextest run content_loading` — runtime validation passes - [ ] Gauntlet rooms load without panic in test mode 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 1 commit 2026-02-18 02:26:23 +01:00
feat(simulation): sprint 9 gauntlet — test infrastructure and first 3 rooms e66352e0ea 
Add Gauntlet test world with 3 rooms (Inventory Warehouse, Occlusion
Corridor, Pause Chamber) + Central Hub, room constants module, room
reset trigger mechanism, Layer 3 subprocess integration test, golden
file comparison engine and test suite, and content runtime validation.

Tickets: #482, #484, #485, #487, #488, #489, #490

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

Review: server -> main (type: code)

Hoshe (Code Quality): REQUEST_CHANGES

Well-structured test infrastructure with clean room builder patterns. StableId reservation and debounce logic are correct.

# File Severity Issue
1 reset.rs warning RoomMember component defined but never used — dead code
2 reset.rs warning Dual reset API (execute_reset/plan_reset) — production path uses only plan_reset; the other creates maintenance trap
3 mod.rs warning Naked .unwrap() on reset_plate in boot-path code — use .expect()
4 content_loading.rs warning TCP runtime test has no read timeout; can hang indefinitely
5 content_loading.rs suggestion Player not registered in EntityRegistry in runtime boot test
6 constants.rs suggestion room_at z-range assumption undocumented
7 mod.rs suggestion Entity count 24 hardcoded in assertion — derive from constants
8 mod.rs suggestion stable_id_ranges_match_spec doesn't verify reset plates at 49-51
9 reset.rs suggestion Debounce exact boundary (tick 9 rejected, tick 10 accepted) not tested

Tyre (Architecture): APPROVE

Architecturally consistent with D-010, D-030, D-031. StableId reservation pattern is "genuinely elegant." Reset mechanism uses Commands correctly in production path.

# File Severity Issue
1 lib.rs warning test_world unconditionally compiled into release binary — consider #[cfg(feature = "gauntlet")]
2 mod.rs warning setup_gauntlet bypasses startup system scheduler — add comment, track for future
3 reset.rs suggestion Dual reset paths — extract shared logic or cross-reference
4 constants.rs suggestion room_at fragile if corridors overlap room bounding boxes — document assumption
5 content_loading.rs suggestion Runtime TCP test belongs in own file, not with structural loading tests

Verdict: CHANGES REQUESTED

## Review: server -> main (type: code) ### Hoshe (Code Quality): REQUEST_CHANGES Well-structured test infrastructure with clean room builder patterns. StableId reservation and debounce logic are correct. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | reset.rs | warning | `RoomMember` component defined but never used — dead code | | 2 | reset.rs | warning | Dual reset API (`execute_reset`/`plan_reset`) — production path uses only `plan_reset`; the other creates maintenance trap | | 3 | mod.rs | warning | Naked `.unwrap()` on `reset_plate` in boot-path code — use `.expect()` | | 4 | content_loading.rs | warning | TCP runtime test has no read timeout; can hang indefinitely | | 5 | content_loading.rs | suggestion | Player not registered in EntityRegistry in runtime boot test | | 6 | constants.rs | suggestion | `room_at` z-range assumption undocumented | | 7 | mod.rs | suggestion | Entity count `24` hardcoded in assertion — derive from constants | | 8 | mod.rs | suggestion | `stable_id_ranges_match_spec` doesn't verify reset plates at 49-51 | | 9 | reset.rs | suggestion | Debounce exact boundary (tick 9 rejected, tick 10 accepted) not tested | ### Tyre (Architecture): APPROVE Architecturally consistent with D-010, D-030, D-031. StableId reservation pattern is "genuinely elegant." Reset mechanism uses Commands correctly in production path. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | lib.rs | warning | `test_world` unconditionally compiled into release binary — consider `#[cfg(feature = "gauntlet")]` | | 2 | mod.rs | warning | `setup_gauntlet` bypasses startup system scheduler — add comment, track for future | | 3 | reset.rs | suggestion | Dual reset paths — extract shared logic or cross-reference | | 4 | constants.rs | suggestion | `room_at` fragile if corridors overlap room bounding boxes — document assumption | | 5 | content_loading.rs | suggestion | Runtime TCP test belongs in own file, not with structural loading tests | ### Verdict: CHANGES REQUESTED
jpmschweitzer added 2 commits 2026-02-18 09:45:36 +01:00
refactor(simulation): address PR #32 review — 14 items from Hoshe + Tyre 09eb6d7582 
Hoshe (code quality):
- Remove dead RoomMember component from reset.rs
- Remove execute_reset (dual API trap); plan_reset is sole production path
- .unwrap() → .expect() on reset_plate in setup_gauntlet boot path
- Add 10s read timeout to TCP runtime test (prevents hangs)
- Register player in EntityRegistry in runtime boot test
- Document room_at z-range and corridor overlap assumptions
- Derive entity count from EXPECTED_ENTITY_COUNT constant (was hardcoded 24)
- Add reset plate (49-51) verification to stable_id_ranges_match_spec
- Add debounce exact boundary test (tick 9 rejected, tick 10 accepted)

Tyre (architecture):
- Gate test_world rooms/constants/setup behind "gauntlet" feature (default-on);
  reset module stays always-compiled (production dependency via input system)
- Document setup_gauntlet scheduler bypass for future tracking
- Extract runtime TCP test to content_runtime.rs (separate failure modes)

507 tests passing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
chore(meta): update changelog for PR #32 review fixes aadfe64cfb 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpmschweitzer commented 2026-02-18 10:18:49 +01:00
Author
Owner
Copy Link
Copy Source

Re-Review: server -> main (type: code) — Post-Fix

Hoshe (Code Quality): APPROVE

All 9 original items verified as FIXED. No new issues. Code is cleaner — simpler API surface, better test coverage, TCP test can no longer hang.

# Original Item Status
1 RoomMember dead code FIXED
2 Dual reset API FIXED — execute_reset removed
3 Naked .unwrap() FIXED — .expect() with messages
4 TCP test no timeout FIXED — 10s read timeout
5 Player not registered FIXED
6 room_at undocumented FIXED
7 Hardcoded entity count FIXED — derived from constants
8 Reset plates untested FIXED
9 Debounce boundary untested FIXED

Tyre (Architecture): APPROVE

All 5 original items verified as FIXED. Feature-gate approach well-structured. rooms_do_not_overlap test is a strong addition.

# Original Item Status
1 test_world in release binary FIXED — #[cfg(feature = "gauntlet")]
2 setup_gauntlet bypasses scheduler FIXED — documented + tracked
3 Dual reset paths FIXED — execute_reset removed
4 room_at corridor overlap FIXED — documented + test added
5 TCP test in wrong file FIXED — content_runtime.rs

Verdict: APPROVED

## Re-Review: server -> main (type: code) — Post-Fix ### Hoshe (Code Quality): APPROVE All 9 original items verified as FIXED. No new issues. Code is cleaner — simpler API surface, better test coverage, TCP test can no longer hang. | # | Original Item | Status | |---|--------------|--------| | 1 | `RoomMember` dead code | FIXED | | 2 | Dual reset API | FIXED — `execute_reset` removed | | 3 | Naked `.unwrap()` | FIXED — `.expect()` with messages | | 4 | TCP test no timeout | FIXED — 10s read timeout | | 5 | Player not registered | FIXED | | 6 | `room_at` undocumented | FIXED | | 7 | Hardcoded entity count | FIXED — derived from constants | | 8 | Reset plates untested | FIXED | | 9 | Debounce boundary untested | FIXED | ### Tyre (Architecture): APPROVE All 5 original items verified as FIXED. Feature-gate approach well-structured. `rooms_do_not_overlap` test is a strong addition. | # | Original Item | Status | |---|--------------|--------| | 1 | `test_world` in release binary | FIXED — `#[cfg(feature = "gauntlet")]` | | 2 | `setup_gauntlet` bypasses scheduler | FIXED — documented + tracked | | 3 | Dual reset paths | FIXED — `execute_reset` removed | | 4 | `room_at` corridor overlap | FIXED — documented + test added | | 5 | TCP test in wrong file | FIXED — `content_runtime.rs` | ### Verdict: APPROVED
jpmschweitzer closed this pull request 2026-02-18 10:24:34 +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#32