engine: rest surface emits located cast (NPCs + live monsters) — closes #1639#1662
Conversation
#1639 The rest/walk surface's stage.tokens projected only the party + present NPCs; resident live monsters (spawn_monster, kind=monster) surfaced ONLY in combat, so the walked world rendered empty of its cast — the Goblin Boss on his throne and the crypt goblins were invisible (adventure_walk RED: actor_missing). Additive, combat-inert, hp>0-gated projection in viewer/server.py _scene_stage (the sole builder of stage.tokens): live monsters at the party's current location are emitted after the party + NPCs, on their engine-seeded stage_cell (else the authored foes spawn bucket, then the shared anchor/party fallback). rest_role 'foe' (team 'foe') keeps them off the client's click-to-talk path. Pure viewer projection — engine stays sole writer, no new model field, snapshots unchanged. Combat mode still returns empty stage.tokens (no double-paint on the tactical board).
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
evaOS review status: closed or merged before reviewPR: #1662 - engine: rest surface emits located cast (NPCs + live monsters) — closes #1639 evaOS review stopped because the PR closed or merged before this queued head could be reviewed. Automation note: agents should wait for this comment to reach PR URL: #1662 Details: state=closed |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82da7a070b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # target. Placement: the engine-authoritative stage_cell when set (walk-in / seeded), else the | ||
| # `foes` spawn bucket, then the shared anchor/party fallback (all inside _emit). | ||
| for k, cid in enumerate(present_monsters): | ||
| _emit(cid, foe_cells, k, fb, "foe") |
There was a problem hiding this comment.
When a resident monster is emitted as a rest-stage token, the existing rest board treats every non-rest_role === "npc" token as selectable and then posts walk_to_cell for the selected id on the next empty-cell click; _resolve_walk_to forwards that id directly to engine.walk_to, which only rejects different locations, not non-party actors. In a room with a visible boss, clicking the boss and then a cell will therefore move the hostile and persist its stage_cell instead of just showing it as inert cast presence, so these foe tokens need to be non-selectable or the walk resolver must reject non-party ids.
Useful? React with 👍 / 👎.
| # cast — the boss on his throne, not just the party). Foes fall back to the `foes` spawn bucket | ||
| # the generators author (scene_grid.py) when they carry no seeded stage_cell; drop any that sit | ||
| # on a blocking prop/wall, same discipline as the party/npc buckets. | ||
| foe_cells = [c for c in _cells(spawns.get("foes")) if c not in blocked] |
There was a problem hiding this comment.
For monsters that do not yet have stage_cell, this paints them onto the generic spawns.foes bucket, but the engine paths and combat seeding do not treat that derived cell as occupied: rest_blocked_cells only reads character stage_cell plus per-id npc:* anchors and skips the generic party/foes/npcs lists, and start_combat(seed_from_stage=True) likewise ignores combatants without stage_cell. In the new unwalked-monster scenario, the rest board can show a boss standing on a cell that /move can route through and that the tactical fight will not seed from, so either avoid presenting unplaced foes as occupied cells or make the engine own/block the same cell.
AGENTS.md reference: AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
| # target. Placement: the engine-authoritative stage_cell when set (walk-in / seeded), else the | ||
| # `foes` spawn bucket, then the shared anchor/party fallback (all inside _emit). | ||
| for k, cid in enumerate(present_monsters): | ||
| _emit(cid, foe_cells, k, fb, "foe") |
There was a problem hiding this comment.
Because the new monster tokens are emitted after party and NPC tokens, any foe whose stage_cell overlaps an earlier actor, or whose overflow fallback lands on an occupied party/NPC cell, wins in the existing rest board's single-token at[x,y] map and hides the active PC or NPC from the grid. This can happen in rooms with more live monsters than authored foe/anchor slots or with stale overlapping stage cells, so the projection should de-conflict placements before emitting foe tokens instead of relying on the client to render stacks it cannot represent.
Useful? React with 👍 / 👎.
1 participant
Closes #1639.
Root cause
The rest/walk surface's
stage.tokens(built byviewer/server.py_scene_stage, the solebuilder of that block) projected only the party + present NPCs. Resident live monsters
(
spawn_monster,kind=monster) surfaced only in combat — so the walked world renderedempty of its cast: Keeper Maera was emitted (the #1363 NPC path), but the Goblin Boss in the
throne hall and the crypt goblins were invisible (
adventure_walkRED:actor_missing).The additive projection
Live monsters at the party's current location are now emitted after the party + NPCs:
_is_dead_or_downedpredicate the party/NPC branches use — a dead ordowned (0 HP, not-yet-
dead) foe never re-stands in the walked scene (combat handles the fallen).party.Character.stage_cellwhen set (walk-in / seeded),else the authored
foesspawn bucket (scene_grid.py), then the shared anchor/party fallback —all filtered off blocking props/walls. Never mints a cell (pure read).
rest_role: "foe"(team: "foe"via_combat_team) — distinct from"npc", so the client'sclick-to-talk path (
rest_role === "npc",screen-combat.jsx) never opens a parley on amonster. No client changes; the client renders tokens generically.
Combat-inertness (provably inert)
_scene_stagereturns{"mode":"combat","tokens":[]}before any rest projection when combat isactive — the monster addition lives entirely in the rest branch and cannot double-emit onto the
tactical board. Test
test_combat_mode_carries_no_monster_tokenspins this with a live monsterpresent + combat active. The surface gains no non-
stagetop-level key(
test_monster_present_surface_is_still_stage_only_new_key).Serializer discipline / sole-writer
Pure viewer projection — the engine stays sole writer, no new model field,
positionAuthoritystays
"derived", and_scene_stagenever mutates the snapshot(
test_projection_does_not_mutate_the_snapshot). Engine-side round-trip test proves a snapshotcarrying an NPC + a live monster (with
stage_cells) is byte-identical on load→save, and anunplaced monster omits
stage_cellfrom the dump (test_cast_presence_snapshot_roundtrip.py).Tests (red-first)
viewer/tests/test_scene_at_rest_stage.py— +10 tests (SceneAtRestLiveMonsterTests): livemonster emitted, uses
foesbucket unwalked, renders atstage_cellwhen set, dead excluded,downed(0 HP) excluded, other-location excluded, derived-hint/
rest_role, combat-inert,stage-only-key, no-mutation, deterministic. Verified RED (4 emission tests
KeyError: mon_boss)with the fix reverted.
servers/engine/tests/test_cast_presence_snapshot_roundtrip.py— +2 tests (byte-identity + omit-when-None).viewer/testssuite: 875 passed, 8 skipped (no regressions). Enginetest_living_stageLive curl evidence
Seeded
adventure_demo_v1(qa/seed_adventure_demo.py) into throwaway state dirs, served theviewer on scratch ports,
GET /combat-surface:Owner engine (:8766), owner state dir, :8866/:8972, and
qa/state/adv_live3(a live eval) were nottouched — scratch ports :8123/:8124 +
/tmpseed dirs only.