fix(physics): AP-129 review fix - port CanMoveInto/IsAllowedIn, stop failing closed

Campaign P Slice P4 Opus review verdict: FIX-FIRST. RestrictionObjPrevalenceInspectionTests
(commit 3b5e0992) found 103,766 of 729,888 installed EnvCells (1,293 landblocks -
the whole housing estate) carry a baked RestrictionObj. The AP-71 gate's
unconditional fail-closed default (CanMoveInto unmodeled) would have locked
every apartment/cottage/villa interior for every player, including its own
owner - a live regression, not the "inert in dev content" the original
register row assumed.

Ports ACCWeenieObject::CanMoveInto (0x0058da40, pc:407982-408056) and
RestrictionDB::IsAllowedIn (0x005ae8f0, pc:444493-444516) verbatim into
ObjectInfo.CheckEntryRestrictions:
- owner_iid == 0 or == mover's own guid -> admit (open/owner)
- no RestrictionDB (retail _db == 0, i.e. never authored or not yet
  received) -> admit
- present RestrictionDB -> IsAllowedIn: open-to-public flag, OR mover
  shares the house's allegiance monarch, OR mover's own guid is a
  guest-table member
- unresolved restriction object -> fails CLOSED, exactly retail's own
  fallback when GetObjectA can't resolve it (pc:704-716)

Wire feed (Core.Net):
- CreateObject.cs: HouseOwner (WeenieHeaderFlag 0x02000000), HouseRestrictions
  (0x04000000), and Monarch (0x40) PWD-tail fields were parsed-and-skipped;
  now captured. Also fixes the HouseRestrictions PHashTable header
  misconception: the wire is ONE packed u32 (low 24 bits = entry count),
  not a separate count(u16)+numBuckets(u16) pair - verified against
  Chorizite's RestrictionDB.generated.cs. The old skip's byte-count
  happened to match for realistic guest-list sizes, but a future
  numBuckets value >255 would have corrupted the parse; now correct
  regardless.
- GameEvents.cs/GameEventWiring.cs: new House_UpdateRestrictions (0x0248)
  parser + wiring - retail's live guest-list refresh, whole-unit replace.
  No-ops if the house object hasn't arrived via CreateObject yet.
- ClientObject/WeenieData/ClientObjectTable: HouseOwnerId, MonarchId,
  Restrictions (new HouseRestrictionRecord) fields + merge-preserving
  Ingest + targeted UpdateHouseRestrictions.

Physics wiring:
- PhysicsEngine gains an Objects (ClientObjectTable?) property, mirroring
  the existing DataCache pattern - acdream's GetObjectA equivalent, used
  ONLY by the entry-restriction gate.
- RuntimeEntityObjectLifetime wires Physics.Engine.Objects = Objects in
  all three constructors, right alongside the table's own construction -
  the same canonical table every other subsystem borrows from, never a
  second one. This is the production fix: without it the gate still fails
  closed on every restricted cell (unresolvable object), so the wiring is
  load-bearing, not cosmetic.

Register: AP-129 narrowed (not retired) to the genuine remaining residual -
House_UpdateRestrictions' Sequence byte isn't used for staleness/reordering
rejection (low-probability, self-correcting), and outdoor CLandCell
restriction (a separate DAT structure) remains unported and unaffected by
this fix.

Tests: 15 new/updated in Ap71EntryRestrictionGateTests.cs (resolved-unowned
admits, owner admits, present-list-excluded blocks, present-list-included
admits, open-to-public admits, shared-allegiance-monarch admits, unresolved
blocks via null and via an empty table, plus two new end-to-end
PhysicsEngine.Objects-wired scenarios); 2 new CreateObject parser tests +
2 new GameEventWiring tests for the wire feed.

AcDream.Core.Tests: 4049 passed, 2 skipped, 0 failed.
AcDream.Core.Net.Tests: 761 passed, 0 skipped, 0 failed.
Complete solution suite: 9,961 total, 9,956 passed, 5 skipped, 0 failed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-07-30 11:36:11 +02:00
parent dc0468cc2b
commit 7a0f836af5
15 changed files with 704 additions and 81 deletions

View file

@ -116,7 +116,7 @@ accepted-divergence entries (#96, #49, #50).
---
## 3. Documented approximation (AP) — 92 active rows (AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-129 filed same slice for the narrower remaining gap — `CanMoveInto`'s owner/guest-list decode is unmodeled, so a genuinely restricted cell fails closed for everyone, not just intruders; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-127 filed same slice for the two minor unmodeled bonus properties; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55)
## 3. Documented approximation (AP) — 92 active rows (AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-127 filed same slice for the two minor unmodeled bonus properties; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55)
Wave-0 UI ledger repair (2026-07-10) retired stale AP-38, resolved the AP-84
collision, restored overwritten paperdoll rows as AP-92/AP-93, and registered
@ -181,7 +181,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps.
| AP-68 | acdream keeps the 128 nearest-to-CAMERA point lights live (`MaxGlobalLights=128`, `BuildPointLightSnapshot`) and selects per cell CAMERA-INDEPENDENTLY (by the cell's own bounds), so a building interior stays lit at any distance within a town; retail keeps only the 40 nearest-to-PLAYER static lights (`Render::max_static_lights=0x28`, distance-sorted replace-farthest `insert_light`) and re-bakes a cell when its live light set changes, so distant interiors are baked dark and "light up" only as the player approaches and their torches enter the live 40. INTENTIONAL — acdream's always-lit interiors are the preferred behavior (no 1999-era light-budget pop-in); user-confirmed 2026-06-20. | `src/AcDream.Core/Lighting/LightManager.cs` (`MaxGlobalLights=128`, `BuildPointLightSnapshot`, `SelectForObject`) | The retail pop-in is a fixed-function light-budget artifact, not an intended aesthetic; revert to retail by clamping the global set to 40 + distance-to-player sort if ever desired | Distant town interiors are lit in acdream where retail's are dark until approached — a deliberate, user-preferred divergence | `Render::max_static_lights` 0x28; `insert_light` 0x0054d1b0 (distance-sorted, replace-farthest); bake re-trigger `SetStaticLightingVertexColors` cache `burnedInStaticLights != num_static_lights` |
| AP-69 | acdream preserves one accepted active record across rebucketing and ports retail's 25-second leave-visibility destruction lifecycle. Spatially resident records cancel expiry; otherwise the ACE compatibility boundary uses holtburger's conservative 384-unit distance envelope and retains attached/container/wielder/parent-owned objects. Expiry uses the exact generation-safe active teardown, then retains only a cold `EntitySpawn` because ACE can keep the GUID in `KnownObjects` and omit CreateObject on revisit; explicit F747/new generation/session reset removes it. DIVERGENCE: retail can delete the complete object under its visibility protocol; the fallback does not yet derive visibility from retail/ACE ObjCell PVS (`SeenOutside` plus `VisibleCells`), and trade/container preview retention has no separate lifecycle flag. | `src/AcDream.App/World/LiveEntityRuntime.cs`; `src/AcDream.App/World/LiveEntityLivenessController.cs`; `src/AcDream.App/World/DormantLiveEntityStore.cs`; `LiveEntityHydrationController.OnPrune` | Prevents stale portal destinations from accumulating animation/effect/render owners while still allowing doors, signs, portals, and other ACE-known objects to rematerialize when the server does not resend them | Dormant data-only snapshots can grow with every unique ACE destination until F747 or session reset; a nonresident object outside 384 units that remains visible through an unusual long EnvCell PVS could expire early; a future preview-only object with no parent/container ownership could also expire. Replace the compatibility predicate when exact ObjCell PVS and preview lifetimes are available | `CPhysicsObj::prepare_to_leave_visibility` 0x00511F40; `CPhysicsObj::prepare_to_enter_world` 0x00511FA0; `CObjectMaint::AddObjectToBeDestroyed` 0x00508F70; `CObjectMaint::UseTime` 0x005089B0; ACE `KnownObjects`; `docs/research/2026-07-18-retail-object-liveness-and-mesh-reclamation-pseudocode.md` |
| ~~AP-71~~ | **RETIRED 2026-07-30 (Campaign P Slice P4) — the `check_entry_restrictions` gate is now ported at the head of the indoor branch of `Transition.FindEnvCollisions`.** `ObjectInfo.CheckEntryRestrictions` (`src/AcDream.Core/Physics/TransitionTypes.cs`) reproduces retail's exact order: NPCs/props bypass, a mover with `CanBypassMoveRestrictions` (new PWD-bitfield decode, `BF_ADMIN 0x100000` AND `BF_IMMUNE_CELL_RESTRICTIONS 0x400000`, `acclient.h:6452-6454`) bypasses, an ordinary cell (`RestrictionObj == 0`) is a no-op. `CellPhysics.RestrictionObj` is now wired from the DAT-baked `EnvCell.RestrictionObj` field (§4.3's old open question — RESOLVED via `references/ACE/Source/ACE.DatLoader/FileTypes/EnvCell.cs:32,66-67` and an independent reflection probe of `Chorizite.DatReaderWriter` 2.1.7's own `EnvCell.RestrictionObj` field: it is a plain per-cell DAT field gated by `EnvCellFlags.HasRestrictionObj (0x8)`, NOT a live wire override; the BN pseudo-C's "count for an array alloc" reading at the same `UnPack` offset was the mis-attributed field-name collision `feedback_bn_decomp_field_names` warned about). Wired in BOTH the dev/graph-fixture path (`PhysicsDataCache.CacheCellStruct`) and the production/prepared path (`CachePreparedCellStruct`) — the latter already receives a live parsed `envCell` for `Position`/`EnvironmentId`, so no bake-format change was needed. See AP-129 for the narrower remaining gap this leaves. | `src/AcDream.Core/Physics/TransitionTypes.cs` (`ObjectInfo.CheckEntryRestrictions`, `Transition.FindEnvCollisions`); `src/AcDream.Core/Physics/PhysicsDataCache.cs` (`CellPhysics.RestrictionObj`) | — | — | `CObjCell::check_entry_restrictions` pc:308873-308912 (0x0052b6d0); `CEnvCell::find_env_collisions` pc:309573-309597; `ACCWeenieObject::CanBypassMoveRestrictions` 0x0058c500; `ACCWeenieObject::CanMoveInto` 0x0058da40; `references/ACE/Source/ACE.Server/Physics/Common/ObjCell.cs:286-333` |
| AP-129 | **`check_entry_restrictions`'s `CanMoveInto` (house owner IID + guest/ban list) is unmodeled** — retail resolves the cell's `restriction_obj` to a live weenie and asks `CanMoveInto(mover)` (owner IID match, else a `RestrictionDB` guest/ban list check). acdream has no restriction-weenie resolution and no `RestrictionDB`/house-guest wire model (`HouseData 0x0225`, `HouseUpdateRestrictions 0x0248`, and the `House_*` guest-list opcode family are enumerated in `GameEventType.cs` but none are parsed into any acdream data structure). `ObjectInfo.CheckEntryRestrictions` therefore fails CLOSED (`Collided`) for ANY cell whose DAT-baked `RestrictionObj` is nonzero and whose mover cannot bypass — this is retail's own fallback when the restriction object can't be resolved, so it is not a fabricated behavior, but it means a legitimate house owner/guest would ALSO be blocked, not just intruders. | `src/AcDream.Core/Physics/TransitionTypes.cs` (`ObjectInfo.CheckEntryRestrictions`) | Zero access-restricted EnvCells exist in the ACE starter-area test content the project runs against day to day (this is DAT-authored, per-cell data — not synthesized), so the fail-closed default is inert for ordinary dungeon/outdoor testing; it only fires if a player-built house (or other content with `EnvCellFlags.HasRestrictionObj` set) is visited. Outdoor `CLandCell` restriction (`LandblockInfo.RestrictionTables`, a separate per-landblock packed hash table — `references/ACE/Source/ACE.Server/Physics/Common/Landblock.cs:508-520`, `ACE.DatLoader.FileTypes.LandblockInfo.cs:43`) is a distinct, unparsed DAT structure and is explicitly NOT ported by this row's gate (the gate only reads `CellPhysics.RestrictionObj`, the indoor/EnvCell field). | Any house-barrier EnvCell visited today denies entry to everyone including its owner, until `RestrictionDB`/`CanMoveInto` wire parsing lands; an outdoor restricted cell (if that content ever exists) is not gated at all. | `ACCWeenieObject::CanMoveInto` 0x0058da40; `references/ACE/Source/ACE.Server/Physics/Common/ObjCell.cs:301-325`; `references/ACE/Source/ACE.Server/Network/Structure/RestrictionDB.cs` |
| AP-129 | **NARROWED 2026-07-30 (P4 Opus review fix) — `CanMoveInto`/`IsAllowedIn` are now ported and fed; two narrow gaps remain.** `ObjectInfo.CheckEntryRestrictions` resolves the cell's `RestrictionObj` via `PhysicsEngine.Objects` (a `ClientObjectTable`, acdream's `GetObjectA` equivalent) and evaluates the real owner IID / `HouseRestrictionRecord` (open flag, allegiance monarch, guest table) fed from CreateObject's `HouseOwner`/`HouseRestrictions`/`Monarch` PWD-tail fields and live `House_UpdateRestrictions (0x0248)` refreshes — see `RestrictionObjPrevalenceInspectionTests` (103,766 of 729,888 installed EnvCells, 1,293 landblocks, carry a baked `RestrictionObj`; this is the whole housing estate, not a rare case, which is why the OLD unconditional-fail-closed row was upgraded to FIX-FIRST rather than shipped). Remaining gaps: (1) `House_UpdateRestrictions`'s `Sequence` byte is parsed but not used for staleness/reordering rejection — a lost-then-late UDP delivery could transiently apply an older restriction snapshot over a newer one (low-probability; the next full CreateObject or another update self-corrects). (2) Outdoor `CLandCell` restriction (`LandblockInfo.RestrictionTables`, a separate per-landblock packed hash table) remains entirely unported — unaffected by this fix, since the gate only reads the indoor/EnvCell `CellPhysics.RestrictionObj` field. `HouseData (0x0225)`/`HouseStatus (0x0226)` and the guest-management opcode family (`House_AddPermanentGuest`, `House_UpdateHAR`, etc.) remain unparsed but are NOT consulted by this entry gate (they carry rent/ownership-transfer UI data, not the owner-iid/guest-list pair `CanMoveInto` needs) — noted for future house-UI work, not a residual of this row. | `src/AcDream.Core/Physics/TransitionTypes.cs` (`ObjectInfo.CheckEntryRestrictions`); `src/AcDream.Core/Physics/PhysicsEngine.cs` (`Objects`); `src/AcDream.Core/Items/{ClientObject,ClientObjectTable,HouseRestrictions}.cs`; `src/AcDream.Core.Net/{Messages/CreateObject.cs,Messages/GameEvents.cs,GameEventWiring.cs}`; `src/AcDream.Runtime/Entities/RuntimeEntityObjectLifetime.cs` (production wiring) | A reordered `House_UpdateRestrictions` pair could transiently apply the older snapshot; self-corrects on the next update or CreateObject. An outdoor restricted cell (if that content ever exists) is not gated at all. | `ACCWeenieObject::CanMoveInto` 0x0058da40 (pc:407982-408056); `RestrictionDB::IsAllowedIn` 0x005ae8f0 (pc:444493-444516); `references/Chorizite.ACProtocol/Chorizite.ACProtocol/Types/RestrictionDB.generated.cs`; `references/ACE/Source/ACE.Server/Network/GameEvent/Events/GameEventHouseUpdateRestrictions.cs` |
| AP-72 | **Cursor art falls back to OS standard cursors when dat resolution fails** — retail always renders MediaDescCursor / EnumIDMap-resolved dat cursor art; acdream's `RetailCursorManager.Apply` falls back to Silk `StandardCursor` (IBeam/crosshair/not-allowed/…) when the EnumIDMap chain or RenderSurface decode fails, and `RetailCursorResolver`/`RetailCursorManager` permanently negative-cache the failed enum/surface id for the session. | `src/AcDream.App/Rendering/RetailCursorManager.cs:47` (`ApplyStandard`), `RetailCursorResolver.cs:47` (negative cache) | Fallback triggers only when the dat lacks the asset — nominal EoR dats always resolve the 0x27/0x28/0x29 chain; an OS cursor keeps the UI usable rather than showing nothing. | A dat-read or decode regression silently shows OS-native cursors instead of surfacing an error — masked failure class; check the `[D.2b]` cursor log lines before suspecting art. | `ClientUISystem::UpdateCursorState` 0x00564630 |
| AP-74 | **UseDone WeenieError text comes from a hardcoded subset map, not the portal String tables** — retail resolves the 0x01C7 UseDone error code through the client String tables into the canonical line ("You are not trained in healing!"); acdream's `WeenieErrorText.For` hardcodes the handful of codes the current use/heal flows produce (0x001D/0x04EB/0x04FC/0x04FE, texts phrased after the ACE enum names) with a generic code-carrying fallback. | `src/AcDream.Core.Net/Messages/WeenieErrorText.cs` | Every refusal is now visible; only unmapped wording deviates, and those lines retain the raw code. Retire by porting the String-table lookup (#202). | An unmapped WeenieError shows a generic line instead of retail's exact sentence | retail String-table error lookup; ACE `WeenieError.cs` values |
| AP-73 | **Character raises mutate optimistically, contrary to retail's server-authoritative flow** — after sending RaiseAttribute/RaiseVital/RaiseSkill/TrainSkill, `CharacterSheetProvider.ApplyLocalRaise` immediately bumps ranks and debits XP/credits. Named retail permits one request in flight, ghosts the clicked button, and waits for an authoritative quality-change element message before changing displayed state (**#199**). | `src/AcDream.App/UI/Layout/CharacterSheetProvider.cs` | ACE usually accepts client-affordable raises, so its later property echoes conceal the incorrect prediction; Wave 8 removes local mutation and owns one awaiting request | A rejected/reordered raise can display invented state until a later full refresh, and repeated clicks can create multiple speculative spends | `gmAttributeUI`/`gmSkillUI` raise and quality-change paths, pinned in `docs/research/2026-07-10-retail-panel-behavior-pseudocode.md` |