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

feat(simulation): add Sound Lab, Decay Observatory, Shift Change gauntlet rooms (#505) #46

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

Summary

Three new gauntlet test rooms for Sprint 13, exercising D-018, D-041, and D-031.

  • Sound Lab (StableId 66-68): Three NPCs at 2, 6, 12 tile distances from observer. Tests verify SoundEvent::audible_at() for all three SoundRange variants — positive and negative assertions for each boundary.
  • Decay Observatory (StableId 69): NPC observed in LOS then broken. Tests confirm knowledge confidence decays KnowsDetails → KnowsOf → Suspects (floor enforced) and Stale state triggers at threshold.
  • Shift Change (StableId 70-71): Two NPCs with DailyRoutine components. Test confirms check_phase_transition issues PathRequest only to the NPC not already at its target location at the Morning→Afternoon boundary.
  • Sprint 13 reset plates (StableId 72-74): Wired per existing pattern.
  • constants.rs updated with room definitions, StableId ranges, EXPECTED_ENTITY_COUNT, and overlap/order tests.
  • mod.rs updated with room carving, entity spawning, reset plates, RoomSnapshots, and stable_id_ranges_match_spec assertions.

All 555 server unit tests pass. Pre-existing content_loading and golden_suite failures are unrelated to this branch.

Test plan

  • sound_lab_close_range_in_snapshot — Close emitter (dist=2) audible with SoundRange::Close
  • sound_lab_medium_range_indicator_present — Medium emitter (dist=6) audible with Medium, NOT Close
  • sound_lab_long_range_insert_only — Long emitter (dist=12) audible with Long, NOT Medium
  • decay_observatory_confidence_degrades_after_los_loss — KnowsDetails→KnowsOf→Suspects, floor verified
  • decay_observatory_stale_after_threshold — Stale state after stale_after exceeded
  • shift_change_npc_transitions_routine_at_phase_boundary — PathRequest fired only for displaced NPC
  • gauntlet_setup_creates_expected_entities — entity count matches EXPECTED_ENTITY_COUNT (75)
  • stable_id_ranges_match_spec — all StableIds 66-74 present in registry

🤖 Generated with Claude Code

## Summary Three new gauntlet test rooms for Sprint 13, exercising D-018, D-041, and D-031. - **Sound Lab** (StableId 66-68): Three NPCs at 2, 6, 12 tile distances from observer. Tests verify `SoundEvent::audible_at()` for all three `SoundRange` variants — positive and negative assertions for each boundary. - **Decay Observatory** (StableId 69): NPC observed in LOS then broken. Tests confirm knowledge confidence decays `KnowsDetails → KnowsOf → Suspects` (floor enforced) and `Stale` state triggers at threshold. - **Shift Change** (StableId 70-71): Two NPCs with `DailyRoutine` components. Test confirms `check_phase_transition` issues `PathRequest` only to the NPC not already at its target location at the Morning→Afternoon boundary. - **Sprint 13 reset plates** (StableId 72-74): Wired per existing pattern. - `constants.rs` updated with room definitions, StableId ranges, `EXPECTED_ENTITY_COUNT`, and overlap/order tests. - `mod.rs` updated with room carving, entity spawning, reset plates, `RoomSnapshots`, and `stable_id_ranges_match_spec` assertions. All 555 server unit tests pass. Pre-existing `content_loading` and `golden_suite` failures are unrelated to this branch. ## Test plan - [x] `sound_lab_close_range_in_snapshot` — Close emitter (dist=2) audible with `SoundRange::Close` - [x] `sound_lab_medium_range_indicator_present` — Medium emitter (dist=6) audible with Medium, NOT Close - [x] `sound_lab_long_range_insert_only` — Long emitter (dist=12) audible with Long, NOT Medium - [x] `decay_observatory_confidence_degrades_after_los_loss` — KnowsDetails→KnowsOf→Suspects, floor verified - [x] `decay_observatory_stale_after_threshold` — Stale state after stale_after exceeded - [x] `shift_change_npc_transitions_routine_at_phase_boundary` — PathRequest fired only for displaced NPC - [x] `gauntlet_setup_creates_expected_entities` — entity count matches EXPECTED_ENTITY_COUNT (75) - [x] `stable_id_ranges_match_spec` — all StableIds 66-74 present in registry 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 2 commits 2026-02-19 18:10:17 +01:00
feat(simulation): add Sound Lab, Decay Observatory, Shift Change gauntlet rooms (#505) c2de509c01 
Three new gauntlet test rooms for Sprint 13, each targeting a specific
system under test. StableIds are additive-only (66–74).

Sound Lab (66-68): Three NPCs at calibrated tile distances (2, 6, 12
tiles) verify D-018 three-range sound model. Tests confirm Close ≤3,
Medium ≤8, Long ≤20 tile thresholds via SoundEvent::audible_at().

Decay Observatory (69): Single NPC observed in LOS then broken. Tests
confirm D-041 confidence decay: KnowsDetails → KnowsOf → Suspects
(floor), and Stale state when stale_after threshold is exceeded.

Shift Change (70-71): Two NPCs with DailyRoutine components. Test
confirms D-031 phase-boundary check_phase_transition issues PathRequest
only to NPCs not already at their target location.

Sprint 13 reset plates (72-74) wired per existing pattern.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(simulation): #523 add zone_id to ObserverSnapshot (D-077, protocol v11) 51abc3e3c1 
Server-tracked zone_id on VisibleTile for D-073 zone crossfade and
D-059 deep fog temperature tint. ZoneMap resource backed by BTreeMap,
observer enrichment in snapshot assembly. Backwards-compatible: v10
payloads deserialize with zone_id: None.

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

Updated — Sprint 13 Server Complete

Commits:

  • c2de509 feat(simulation): add Sound Lab, Decay Observatory, Shift Change gauntlet rooms (#505)
  • 51abc3e feat(simulation): #523 add zone_id to ObserverSnapshot (D-077, protocol v11)

Summary:

  • #505: Three new gauntlet rooms — Sound Lab (D-018 three-range sound model), Decay Observatory (D-041 knowledge graph decay), Shift Change (D-031 NPC routine transitions). 6 deterministic tests added.
  • #523: Resolve OQ-09 with D-077 — server-tracked zone_id: Option<u16> on VisibleTile, protocol bumped to v11. ZoneMap resource (BTreeMap), observer enrichment, v10→v11 backwards compat. Enables D-073 zone crossfade and D-059 deep fog temperature tint on client.

Test results: 594/596 pass (2 pre-existing content_loading count mismatches). cargo clippy clean.
Files: 26 changed, +948 / -9

## Updated — Sprint 13 Server Complete **Commits:** - `c2de509` feat(simulation): add Sound Lab, Decay Observatory, Shift Change gauntlet rooms (#505) - `51abc3e` feat(simulation): #523 add zone_id to ObserverSnapshot (D-077, protocol v11) **Summary:** - **#505:** Three new gauntlet rooms — Sound Lab (D-018 three-range sound model), Decay Observatory (D-041 knowledge graph decay), Shift Change (D-031 NPC routine transitions). 6 deterministic tests added. - **#523:** Resolve OQ-09 with D-077 — server-tracked `zone_id: Option<u16>` on `VisibleTile`, protocol bumped to v11. ZoneMap resource (BTreeMap), observer enrichment, v10→v11 backwards compat. Enables D-073 zone crossfade and D-059 deep fog temperature tint on client. **Test results:** 594/596 pass (2 pre-existing content_loading count mismatches). `cargo clippy` clean. **Files:** 26 changed, +948 / -9
jpmschweitzer commented 2026-02-19 18:18:50 +01:00
Author
Owner
Copy Link
Copy Source

Review: server -> main (PR #46)

Hoshe (Code Quality): REQUEST_CHANGES

Well-structured PR with clean room implementations and consistent patterns. Issues with test completeness and documentation accuracy.

# File Severity Issue
1 shift_change.rs warning Verify the full #[cfg(test)] block with shift_change_npc_transitions_routine_at_phase_boundary test is present on branch
2 decay_observatory.rs:1-14 warning Doc comment claims "Direct -> KnowsDetails -> KnowsOf -> Suspects -> Stale" but test starts at KnowsDetails, not Direct. Misleading
3 zone.rs suggestion No test for full pipeline: ZoneMap.set_rect -> compute_observer_snapshot -> zone_id in VisibleTile
4 decay_observatory.rs suggestion TARGET = StableId(1) doesn't match spawned StableId 69 -- valid for unit tests but integration coverage gap
5 serialization.rs suggestion No cross-version test at Godot client layer (existing gap)

Tyre (Architecture): REQUEST_CHANGES

Architecture is sound -- ZoneMap follows D-010 principle 4, optional-resource pattern is clean. Two issues need fixing.

# File Severity Issue
1 types.rs warning zone_id on VisibleTile needs #[serde(default)] alongside skip_serializing_if. Every prior field uses both. Without it, v10-style VisibleTile (no zone_id key) may fail to deserialize into v11 struct
2 observer/mod.rs warning Zone enrichment path has zero direct test coverage. Option<Res<ZoneMap>> fallback never executes in any test. Need tests for: tile inside zone, tile outside zone, no ZoneMap resource
3 zone.rs suggestion Add comment noting production population path is TBD (currently Gauntlet-only)
4 mod.rs:16-33 suggestion StableId range header comment not updated for Sprint 13 (66-74)
5 decay_observatory.rs suggestion Tests KG directly, not through observer pipeline -- won't catch snapshot rendering regressions

Verdict: CHANGES REQUESTED

Key fixes before merge:

  1. #[serde(default)] on VisibleTile.zone_id -- backwards-compat gap
  2. Zone enrichment test coverage -- new observer pipeline code untested
  3. Decay Observatory doc comment -- claims sequence the test doesn't cover
## Review: server -> main (PR #46) ### Hoshe (Code Quality): REQUEST_CHANGES Well-structured PR with clean room implementations and consistent patterns. Issues with test completeness and documentation accuracy. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `shift_change.rs` | warning | Verify the full `#[cfg(test)]` block with `shift_change_npc_transitions_routine_at_phase_boundary` test is present on branch | | 2 | `decay_observatory.rs:1-14` | warning | Doc comment claims "Direct -> KnowsDetails -> KnowsOf -> Suspects -> Stale" but test starts at KnowsDetails, not Direct. Misleading | | 3 | `zone.rs` | suggestion | No test for full pipeline: ZoneMap.set_rect -> compute_observer_snapshot -> zone_id in VisibleTile | | 4 | `decay_observatory.rs` | suggestion | `TARGET = StableId(1)` doesn't match spawned StableId 69 -- valid for unit tests but integration coverage gap | | 5 | `serialization.rs` | suggestion | No cross-version test at Godot client layer (existing gap) | ### Tyre (Architecture): REQUEST_CHANGES Architecture is sound -- ZoneMap follows D-010 principle 4, optional-resource pattern is clean. Two issues need fixing. | # | File | Severity | Issue | |---|------|----------|-------| | 1 | `types.rs` | warning | `zone_id` on `VisibleTile` needs `#[serde(default)]` alongside `skip_serializing_if`. Every prior field uses both. Without it, v10-style VisibleTile (no zone_id key) may fail to deserialize into v11 struct | | 2 | `observer/mod.rs` | warning | Zone enrichment path has zero direct test coverage. `Option<Res<ZoneMap>>` fallback never executes in any test. Need tests for: tile inside zone, tile outside zone, no ZoneMap resource | | 3 | `zone.rs` | suggestion | Add comment noting production population path is TBD (currently Gauntlet-only) | | 4 | `mod.rs:16-33` | suggestion | StableId range header comment not updated for Sprint 13 (66-74) | | 5 | `decay_observatory.rs` | suggestion | Tests KG directly, not through observer pipeline -- won't catch snapshot rendering regressions | ### Verdict: CHANGES REQUESTED **Key fixes before merge:** 1. `#[serde(default)]` on `VisibleTile.zone_id` -- backwards-compat gap 2. Zone enrichment test coverage -- new observer pipeline code untested 3. Decay Observatory doc comment -- claims sequence the test doesn't cover
jpmschweitzer added 1 commit 2026-02-19 18:23:48 +01:00
fix(simulation): address PR #46 review — zone tests, doc accuracy, TBD comment dd1dd0feec 
Add 3 zone enrichment tests to observer pipeline (tile inside zone,
tile outside zone, absent ZoneMap resource). Fix misleading Decay
Observatory doc comment that claimed Direct start when test starts at
KnowsDetails. Add production population TBD note on ZoneMap.

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

Review Response — dd1dd0f

Fixed (3 items)

  1. Decay Observatory doc comment (decay_observatory.rs:3-5) — Corrected to accurately describe the test sequence: observe (Direct) → LOS break (KnowsDetails) → decay KnowsDetails → KnowsOf → Suspects (floor). Previously misleadingly implied the test started at Direct.

  2. Zone enrichment test coverage (observer/tests.rs) — Added 3 tests covering the full Option<Res<ZoneMap>> path:

    • zone_map_enriches_visible_tiles_with_zone_id — tile inside zone gets Some(42)
    • zone_map_tiles_outside_zone_get_none — tile outside any zone gets None
    • no_zone_map_resource_tiles_have_no_zone_id — absent ZoneMap → all tiles None

  3. Production population TBD comment (zone.rs) — Added per Tyre suggestion.

False positives (4 items)

  • Tyre #1 (#[serde(default)] on zone_id): Already present at types.rs:218 — #[serde(default, skip_serializing_if = "Option::is_none")]. Both attributes are there.
  • Hoshe #1 (shift_change test): Present on branch at shift_change.rs:135 with full #[cfg(test)] block.
  • Tyre #4 (StableId range header): Already updated at mod.rs:34-37 with Sprint 13 ranges (66-74).
  • Hoshe #4 / Tyre #5 (unit test vs pipeline test): Intentional — unit tests validate subsystem logic directly. Integration coverage through the observer pipeline is a separate concern and not a gap introduced by this PR.

Test results

All 559 server tests pass (57 observer tests, including 3 new zone enrichment tests). cargo clippy clean.

## Review Response — dd1dd0f ### Fixed (3 items) 1. **Decay Observatory doc comment** (`decay_observatory.rs:3-5`) — Corrected to accurately describe the test sequence: observe (Direct) → LOS break (KnowsDetails) → decay KnowsDetails → KnowsOf → Suspects (floor). Previously misleadingly implied the test started at Direct. 2. **Zone enrichment test coverage** (`observer/tests.rs`) — Added 3 tests covering the full `Option<Res<ZoneMap>>` path: - `zone_map_enriches_visible_tiles_with_zone_id` — tile inside zone gets `Some(42)` - `zone_map_tiles_outside_zone_get_none` — tile outside any zone gets `None` - `no_zone_map_resource_tiles_have_no_zone_id` — absent ZoneMap → all tiles `None` 3. **Production population TBD comment** (`zone.rs`) — Added per Tyre suggestion. ### False positives (4 items) - **Tyre #1** (`#[serde(default)]` on zone_id): Already present at `types.rs:218` — `#[serde(default, skip_serializing_if = "Option::is_none")]`. Both attributes are there. - **Hoshe #1** (shift_change test): Present on branch at `shift_change.rs:135` with full `#[cfg(test)]` block. - **Tyre #4** (StableId range header): Already updated at `mod.rs:34-37` with Sprint 13 ranges (66-74). - **Hoshe #4 / Tyre #5** (unit test vs pipeline test): Intentional — unit tests validate subsystem logic directly. Integration coverage through the observer pipeline is a separate concern and not a gap introduced by this PR. ### Test results All 559 server tests pass (57 observer tests, including 3 new zone enrichment tests). `cargo clippy` clean.
jpmschweitzer closed this pull request 2026-02-19 18:36:56 +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#46