M-ALIGN: reciprocal-door arrival contract — land at the door you came through (#1541)#1550
Conversation
… through (#1541) cross_door arrival re-staging anchored the party at the destination's door_cells[0] regardless of which room they came from, so exiting the tavern (whose crypt-facing door is the crypt's SECOND door) dropped the hero beside the crypt's CAMP door (owner playtest #8: random placement). _seed_stage_cells_on_arrival now takes an optional source_id and anchors on the RECIPROCAL door — the destination's door_cells[j] whose connections[j] == source_id (the same positional convention cross_door resolves the forward hop by). Falls back to the lowest-sorted door (byte-identical pre-#1541 behavior) when no such door exists. Party lands on the door's walkable Chebyshev-1 ring (reachable excludes the threshold), NPCs untouched; engine stays sole writer, all _scene_stage blocked-cell guards in force. RED-first: tests/test_arrival_reciprocal_door.py — a crypt<->tavern round trip across the walkslice 3-room seed lands the hero within Chebyshev-1 of the reciprocal door on return (failed pre-fix at (5,1) near the camp door); plus a camp coherence round trip and a byte-identical fallback proof.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 53 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 (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
evaOS review status: completedPR: #1550 - M-ALIGN: reciprocal-door arrival contract — land at the door you came through (#1541) evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #1550 Review URL: #1550 (review) |
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #1550 - M-ALIGN: reciprocal-door arrival contract — land at the door you came through (#1541)
Head: c3a589465c0b69cddd3c2c8da9434928952d40bf into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 1/5 (~14 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
servers/engine/server.py |
modified | +32/-7 | Changed file | Low |
servers/engine/tests/test_arrival_reciprocal_door.py |
added | +134/-0 | Test coverage | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 1 (P0: 0, P1: 0, P2: 0, P3: 1).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- API compatibility: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: Unity editor or Play Mode smoke - WorldOS repo profile implies Unity runtime risk. Proof: Unity editor smoke; Play Mode log; scene/prefab screenshot or recording.
Proof status: missing - 1 required validation/proof recommendation(s) missing from PR metadata.
Profile validation hints: Prefer correctness, persistence, CI, release, and regression findings over style-only feedback.
Profile proof expectations: Look for Unity editor, play-mode, fixture, or focused smoke evidence when runtime behavior changes.
Related Context
Related issues/PRs: #1541, #8, #1540.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
Assets/**- Prioritize scene, prefab, save-state, asset-reference, and gameplay regressions. - Path instructions:
ProjectSettings/**- Treat build, platform, input, graphics, and release behavior changes as high risk. - Label suggestions: unity, gameplay, regression-hardening
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
| no_reciprocal = _seed_with("not-a-connection") # a source that maps to no door here | ||
| assert baseline == no_reciprocal # byte-identical fallback to door_cells[0] | ||
| # And the fallback anchor is the lowest-sorted door (3,0), not (10,8). | ||
| assert combat_grid.chebyshev_cells(tuple(baseline[1]), (3, 0)) <= 1 |
There was a problem hiding this comment.
P3: No adversarial test for connections/door_cells ordering drift
The reciprocal-door resolution (_seed_stage_cells_on_arrival lines 4803-4808) assumes the destination's connections[j] is positionally aligned with its door_cells[j] — the SAME convention cross_door's forward hop relies on. The new tests only exercise the walkslice seed, which authors connections and door_cells in matching order. There is no test asserting what happens when a destination's connections order DIVERGES from its door_cells order (e.g. door_cells=[A,B] but connections=[B,A]): in that case the party would land at the WRONG door, and because the fallback only triggers when no reciprocal match exists, the mismatch would silently misplace the party rather than fall back. Consider adding a focused unit test that authors a destination with deliberately mis-ordered connections vs door_cells and asserts the resulting placement, to pin the contract (or document that positional alignment is a hard authoring requirement). This is a proof gap, not a validated defect — every current seed follows the convention.
Category: API compatibility
Why this matters: The fix's correctness is entirely contingent on an authoring convention that is nowhere asserted by a test. A future seed that appends a connection before its matching door_cell would silently route arrivals to the wrong door, with the byte-identical fallback masking the regression.
1 participant
Fixes #1541.
Defect (code-verified)
cross_door()→travel_to→_seed_stage_cells_on_arrivalre-staged the arriving party at the destination'sdoor_cells[0]regardless of which room they crossed FROM. So leaving the tavern — whose crypt-facing door is the crypt's second door (13,4) — dropped the hero beside the crypt's camp door (6,0). Owner playtest #8: exiting the tavern placed him at random in the crypt; exiting the camp spawned him "outside a tree."Fix (additive, engine sole-writer invariant)
_seed_stage_cells_on_arrivalnow takes an optionalsource_id(the location just crossed FROM) and anchors the arrival on the reciprocal door — the destination'sdoor_cells[j]whoseconnections[j] == source_id, the same positional authoring conventioncross_dooruses to resolve the forward hop (door_cells[i] → connections[i]). The party lands on that door's walkable Chebyshev-1 ring (combat_grid.reachableexcludes the threshold cell and ranks nearest-first, spreading outward when crowded); NPCs keep their normal staging.Falls back to today's lowest-sorted
door_cells[0]anchor — byte-identical to pre-#1541 behavior — only when there is no door mapping back to the source (source_idisNone, unmapped, or the destination has fewer door cells than the source's connection index). All existing invariants respected: engine sole writer, additive-by-default,rest_blocked_cellsblocked-cell filtering stays in force.RED-first evidence (engine logic — no pixels)
New
tests/test_arrival_reciprocal_door.pydrives the real walkslice three-room seed (crypt hub ↔ camp + tavern) end-to-end.Before the fix, the tavern round-trip test failed exactly on the defect:
After the fix, the named acceptance command is green:
Full engine suite: 3526 passed.
Tests added:
test_tavern_round_trip_lands_at_the_reciprocal_door— the discriminating case (tavern-facing door is the crypt's non-first door).test_camp_round_trip_lands_at_the_reciprocal_door— camp coherence (reciprocal isdoor_cells[0]).test_fallback_to_first_door_is_byte_identical_when_no_reciprocal— proves the fallback path is unchanged.Note: the visual-journey lane (#1540) has this same round trip as a screenshot check; this fix is what flips it green. That worktree was not touched.