Final whole-branch code review (Opus) surfaced two Important post-merge follow-ups + a one-word inaccuracy in the handoff doc: - #59: tighten WorldPicker per-entity Setup.Radius (M1-deferred; the ServerGuid==0 invariant is load-bearing and worth documenting before L.2d's CBuildingObj port lands). - #60: port retail's obstruction_ethereal downstream path so combat-HUD contact reporting works for ethereal creatures (M2-combat). - handoff: corrected "Added a _entitiesByServerGuid reverse-lookup" to "Used the pre-existing _entitiesByServerGuid" — the dict has existed since Phase 6.6/6.7; slice 1c used it, didn't add it. Review verdict: branch ready to merge to main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2c9bdb512b
commit
48ce52c6ed
2 changed files with 55 additions and 3 deletions
|
|
@ -46,6 +46,58 @@ Copy this block when adding a new issue:
|
||||||
|
|
||||||
# Active issues
|
# Active issues
|
||||||
|
|
||||||
|
## #60 — `obstruction_ethereal` retail downstream path not ported (M2 combat-HUD impact)
|
||||||
|
|
||||||
|
**Status:** OPEN
|
||||||
|
**Severity:** LOW for M1 (no observable defect); MEDIUM for M2 (combat contact reporting on ethereal creatures will be wrong)
|
||||||
|
**Filed:** 2026-05-13 (final-review surfaced from B.4b)
|
||||||
|
**Component:** physics / `CollisionExemption.ShouldSkip` + downstream movement contact handling
|
||||||
|
|
||||||
|
**Description:** B.4b's L.2g slice 1b widened `CollisionExemption.ShouldSkip` to exempt
|
||||||
|
on `ETHEREAL_PS` alone (cite `src/AcDream.Core/Physics/CollisionExemption.cs:62-79`). Retail's
|
||||||
|
`acclient_2013_pseudo_c.txt:276782` requires both `ETHEREAL_PS && IGNORE_COLLISIONS_PS` to wrap
|
||||||
|
the entire `FindObjCollisions` body — ETHEREAL alone takes the deeper path at line 276795 which
|
||||||
|
sets `sphere_path.obstruction_ethereal = 1` and lets downstream movement allow passage WHILE
|
||||||
|
STILL REPORTING THE CONTACT. We do not port that downstream path; we just exempt entirely.
|
||||||
|
|
||||||
|
**M2 impact:** Combat HUD work that relies on physics-contact reporting for ethereal creatures
|
||||||
|
(ghosts, partially-phased monsters, spell projectiles with ETHEREAL set) will see no contact at
|
||||||
|
all instead of "soft contact with obstruction_ethereal=1". The user will not be able to target
|
||||||
|
or interact with such entities via the contact path.
|
||||||
|
|
||||||
|
**Acceptance:** Port the retail deeper path so `obstruction_ethereal=1` flows through movement +
|
||||||
|
collision-reporting layers. Tests should cover: ETHEREAL creature target → contact reported but
|
||||||
|
passage allowed; ETHEREAL+IGNORE_COLLISIONS target (door, retail-style) → full exempt.
|
||||||
|
|
||||||
|
**Estimated scope:** Moderate. Touches `CollisionExemption.cs`, transition/movement layer, and
|
||||||
|
sphere-path state propagation. Visible test through a spawned ethereal creature in ACE.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## #59 — `WorldPicker` 5m fixed-radius could over-pick at tight thresholds (M1-deferred polish)
|
||||||
|
|
||||||
|
**Status:** OPEN
|
||||||
|
**Severity:** LOW (cosmetic — picker grabs the right entity in Holtburg-tested scenarios)
|
||||||
|
**Filed:** 2026-05-13 (final-review surfaced from B.4b)
|
||||||
|
**Component:** selection / `AcDream.Core.Selection.WorldPicker.Pick`
|
||||||
|
|
||||||
|
**Description:** `WorldPicker.Pick` uses a hardcoded 5m sphere around every candidate's
|
||||||
|
`Position` regardless of the entity's actual size (`src/AcDream.Core/Selection/WorldPicker.cs:82`).
|
||||||
|
This matches `WorldEntity.DefaultAabbRadius` and is sufficient for M1 acceptance: in tight
|
||||||
|
doorways, every server-keyed candidate has correct sphere coverage and the closest-wins logic
|
||||||
|
plus `ServerGuid==0` skip filter the wrong picks. But the invariant "non-clickable geometry has
|
||||||
|
`ServerGuid==0`" is load-bearing — if L.2d ever ports `CBuildingObj` as a server-keyed entity,
|
||||||
|
the picker may mis-target buildings. Per-entity `Setup.Radius` would be tighter.
|
||||||
|
|
||||||
|
**Acceptance:** Either (a) tighten picker to read per-entity Setup.Radius / CylSphere bounds,
|
||||||
|
or (b) document the invariant in `WorldPicker.cs` and add a regression test asserting
|
||||||
|
`ServerGuid==0` entities never reach the per-candidate hit test.
|
||||||
|
|
||||||
|
**Estimated scope:** Quick (~1 hour) — wire `Setup.Radius` lookup into the picker and update
|
||||||
|
the 6 existing picker tests with realistic radii.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## #58 — Door swing animation: UpdateMotion not wired for non-creature entities
|
## #58 — Door swing animation: UpdateMotion not wired for non-creature entities
|
||||||
|
|
||||||
**Status:** OPEN
|
**Status:** OPEN
|
||||||
|
|
|
||||||
|
|
@ -249,9 +249,9 @@ detection working, the correct gate firing, and `CollisionExemption`
|
||||||
widened, the registry still held the stale closed state and the door
|
widened, the registry still held the stale closed state and the door
|
||||||
stayed solid.
|
stayed solid.
|
||||||
|
|
||||||
**Fix:** Added a `_entitiesByServerGuid` reverse-lookup dictionary to
|
**Fix:** Used the pre-existing `_entitiesByServerGuid` reverse-lookup
|
||||||
`GameWindow` (populated at entity registration in `OnLiveCreateObject`).
|
dictionary on `GameWindow` (populated at entity registration in
|
||||||
`OnLiveStateUpdated` now does:
|
`OnLiveCreateObject` since Phase 6.6/6.7). `OnLiveStateUpdated` now does:
|
||||||
|
|
||||||
```csharp
|
```csharp
|
||||||
if (_entitiesByServerGuid.TryGetValue(parsed.Guid, out var entity))
|
if (_entitiesByServerGuid.TryGetValue(parsed.Guid, out var entity))
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue