fix(motion): restore retail object manager order

Process animation completion at the retail process_hooks boundary, then run targeting, movement, PartArray completion, and PositionManager in the named UpdateObjectInternal order for local, remote, hidden, and position-less animated objects. Retire TS-42 with deterministic conformance coverage.\n\nCo-authored-by: OpenAI Codex <codex@openai.com>
This commit is contained in:
Erik 2026-07-19 21:40:14 +02:00
parent 0f996db747
commit c5ab99081c
17 changed files with 493 additions and 98 deletions

View file

@ -216,7 +216,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps.
| AP-117 | Outdoor particle `CLandCell::IsInView` state is reconstructed with the modern landscape renderer's per-cell frustum plus active doorway clip-plane/scissor-AABB tests; retail `LScape::landcell_check` uses `Render::get_clip_height` + `Render::block_check` on terrain-cell corner intervals | `src/AcDream.App/Rendering/TerrainModernRenderer.cs` (`CollectVisibleCells`) | The mandatory modern renderer batches terrain by landblock and has no retail `ViewIntervalType` product. Publishing cell visibility from the exact landscape draw slices preserves ownership/order and removes the former object-survivor dependency without adding a second view pipeline | At a terrain cell grazing a frustum or doorway boundary, the conservative AABB test may freeze or resume particles on a slightly different frame than retail; whole regions outside the active doorway slice are rejected, and authored distance, login/portal fail-closed behavior, and indoor PView cells remain exact | `LScape::landcell_check @ 0x005050A0`; `CLandCell::IsInView @ 0x00532CB0`; `CPhysicsObj::ShouldDrawParticles @ 0x0050FE60` |
| AP-118 | An AutoWield transaction begun in active combat preserves the ready mode implied by the requested weapon. After authoritative `WieldObject`, a mode that settled without a blocker transition clears immediately; local ACE's observed pre-wield transition plus `ready -> NonCombat`, or post-wield `NonCombat -> ready -> NonCombat`, causes one normal `ChangeCombatMode` request from the trailing notice. Explicit user combat input cancels settlement. Retail's client does not need this extra request against the retail server. | `src/AcDream.App/UI/AutoWieldController.cs`; production binding in `GameWindow.cs` | Local ACE queues a trailing NonCombat callback during primary-weapon replacement and rejects an earlier request while the shuffle is busy; responding to the authoritative notice that completes that exact sequence orders the ordinary request after it without suppressing any server state | A non-ACE server that emits a different intermediate sequence can retain the settlement until a later explicit combat request, replacement, or logout clears it; peace-mode equips send none | `CPlayerSystem::AutoWield @ 0x00560A60`; `ACCWeenieObject::ServerSaysMoveItem @ 0x0058DBB0`; ACE `Player_Inventory.TryShuffleStance` / `TryDequipObjectWithNetworking` |
## 4. Temporary stopgap (TS) — 33 active rows (TS-20 retired 2026-07-16 — the later named-retail audit disproved the proposed DrawingBSP polygon filter; TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded; TS-45 retired 2026-07-07 — hand-rolled `SphereCollision` replaced by the faithful CSphere family port, fixing the player-vs-monster crowd wedge; TS-3 retired 2026-07-07 — `frames_stationary_fall` accounting ported in the #182 verbatim UpdateObjectInternal rebuild, fixing the airborne falling-animation wedge; TS-41 retired 2026-07-07 — SERVERVEL synth-velocity remote body-drive replaced by the retail interp catch-up + unconditional MovementManager::UseTime, the remote-creature de-overlap #184; TS-44 narrowed same day — NPC UP unified onto the interp queue, gate retained for orientation)
## 4. Temporary stopgap (TS) — 32 active rows (TS-20 retired 2026-07-16 — the later named-retail audit disproved the proposed DrawingBSP polygon filter; TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded; TS-45 retired 2026-07-07 — hand-rolled `SphereCollision` replaced by the faithful CSphere family port, fixing the player-vs-monster crowd wedge; TS-3 retired 2026-07-07 — `frames_stationary_fall` accounting ported in the #182 verbatim UpdateObjectInternal rebuild, fixing the airborne falling-animation wedge; TS-41 retired 2026-07-07 — SERVERVEL synth-velocity remote body-drive replaced by the retail interp catch-up + unconditional MovementManager::UseTime, the remote-creature de-overlap #184; TS-42 retired 2026-07-19 — semantic animation completion now precedes the ordered Target/Movement/PartArray/Position tail; TS-44 narrowed 2026-07-07 — NPC UP unified onto the interp queue, gate retained for orientation)
| # | Divergence | Where (file:line) | Why it is safe / justified | Risk if assumption breaks | Retail oracle |
|---|---|---|---|---|---|
@ -249,7 +249,6 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps.
| TS-37 | RETIRED misattribution note (not a live divergence — kept here as the historical record R3-W3 closes): the S2a port had `contact_allows_move` (0x00528240) arm `StandingLongJump` as a side effect, explicitly flagged "PRE-EXISTING acdream side effect (not part of 0x00528240)". R3-W3 deletes that side effect; `ChargeJump` (0x005281c0) is now the ONLY arming site, matching retail exactly. No further action — recorded per the register's retire-in-same-commit rule | `src/AcDream.Core/Physics/MotionInterpreter.cs` (`contact_allows_move`, `ChargeJump`) | N/A — retired | N/A — retired | `CMotionInterp::charge_jump` 0x005281c0 @305448 |
| TS-38 | `MotionInterpreter.Initted` defaults to `true` in both constructors, not retail's `false` — retail's `CMotionInterp` is never observed pre-`enter_default_state` (every real construction path calls it before exposing the interpreter); acdream's constructors are used directly by ~40 pre-existing tests and both App call sites as complete, immediately-usable objects with no separate "enter default state" step | `src/AcDream.Core/Physics/MotionInterpreter.cs` (`Initted` property + both constructors) | Defaulting `true` is the C# equivalent of "the constructor already did what `enter_default_state` would have done to this flag" — `EnterDefaultState()` remains available, verbatim, for the REST of retail's reset semantics (state defaults, sentinel enqueue, `LeaveGround` tail) when a caller wants them | None observed: no code path needs `apply_current_movement`/`ReportExhaustion` to no-op before an explicit `EnterDefaultState()` call, since nothing constructs a `MotionInterpreter` and defers initialization today. If a future caller DOES need staged construction (build now, `EnterDefaultState()` later), it must explicitly set `Initted = false` first | `CMotionInterp::enter_default_state` 0x00528c80 @306124 sets `initted = 1`; retire if/when construction is staged through `EnterDefaultState()` uniformly |
| ~~TS-41~~ | **RETIRED 2026-07-07 (remote-creature de-overlap #184)** — the SERVERVEL synth-velocity body-drive (`Body.Velocity = ServerVelocity` / `get_state_velocity()` leg) is DELETED. Grounded NPC remotes now translate by the retail interp CATCH-UP (`RemoteMotionCombiner.ComputeOffset``InterpolationManager::adjust_offset` toward the MoveOrTeleport-queued server waypoint) and `MovementManager::UseTime` (`TickRemoteMoveTo`) runs UNCONDITIONALLY per tick — the retail `UpdateObjectInternal` shape (no wire-velocity leg-driver). The de-overlap sweep resolves the catch-up movement; the resolved position is written back into the SHADOW (AP-86) so it persists. Residual: the non-retail anim-cycle stale-stop heuristic (`ApplyServerControlledVelocityCycle(Zero)` on a >0.6 s velocity-staleness timer) is kept as ANIM-only and stays covered by **AP-80**; it no longer drives the body. | `src/AcDream.App/Rendering/GameWindow.cs` (`TickAnimations` grounded NPC branch) | — | — | `CPhysicsObj::UpdateObjectInternal` 0x005156b0 (`MovementManager::UseTime` @0x00515998, unconditional); `MoveOrTeleport` 0x00516330; `InterpolationManager::adjust_offset` 0x00555d30 |
| TS-42 | Per-tick DRAIN ORDER inverted vs retail: acdream's `TickAnimations` runs `HandleTargetting``Movement.UseTime` (the R5-V5 MovementManager relay, ex-loose `MoveTo.UseTime`) FIRST and the animation-completion drain (Sequencer.Advance → AnimDone hooks → `MotionTableManager.AnimationDone`/`UseTime``CMotionInterp.MotionDone` pops) LAST, so every motion-completion-gated decision (`BeginTurnToHeading`'s `motions_pending` wait) sees a queue that is one frame STALE — the unblock after a stop/swing lands one frame later than retail. Retail order (pinned from the named decomp this session): `UpdatePositionInternal` (CPartArray::Update + `process_hooks` @0x00512d3d — the drain) runs BEFORE `TargetManager::HandleTargetting` @0x00515989`MovementManager::UseTime` @0x00515998`CPartArray::HandleMovement` @0x005159a4 (zero-tick sweep) in `UpdateObjectInternal` | `src/AcDream.App/Rendering/GameWindow.cs` (`TickAnimations` per-entity phase order; the R2-Q4 comment already marks the placement "provisional until R6") | Bounded to exactly ONE frame (~16 ms) of extra latency per completion-gated event; every queue eventually drains identically (RemoteChaseEndToEndHarnessTests conformance) | Motion-completion-gated transitions (chase turn start, post-swing re-arm) systematically lag retail by one frame; under compound churn the lag can cost an extra retry cycle | `CPhysicsObj::UpdateObjectInternal` 0x005156b0 + `UpdatePositionInternal` 0x00512c30 (`process_hooks` @0x00512d3d); retire in R6 (retail UpdateObjectInternal order) |
| TS-44 | NPC UpdatePosition reconciliation (position + orientation) is SUPPRESSED while the entity is stuck (`PositionManager.GetStickyObjectId() != 0`). **#184 (2026-07-07) UNIFIED the NPC path onto the interp queue** — the retire-condition mechanism is now ported: the grounded UP routes through `CPhysicsObj::MoveOrTeleport` (Enqueue) and the per-tick catch-up SEEDS the `PositionManager::adjust_offset` chain where `StickyManager::adjust_offset` OVERWRITES it while armed (0x00555190 / 0x00555430 assigns m_fOrigin), exactly like the player-remote branch — so POSITION is now handled retail-faithfully by the compose-overwrite whether the gate is on or off. The `snapSuppressedByStick` gate is RETAINED for two residuals: (a) it suppresses the during-stick **Enqueue** so a stale server waypoint never enters the queue, and (b) it gates the **orientation** hard-snap (still outside the interp chain — retail's sticky owns facing). Bookkeeping (`LastServerPos/Time`, cell) still records; server truth reasserts on the first UP after unstick | `src/AcDream.App/Rendering/GameWindow.cs` (`OnLivePositionUpdated` NPC section, `snapSuppressedByStick` gate) | Retail's position mechanism (sticky-overwrites-interp) is now REACHABLE for NPCs (#184 gave them the interp-queue architecture); the gate remains only for orientation + during-stick Enqueue cleanliness | A stick that stays armed while ACE moves the monster far (shouldn't happen — sticks follow the target) keeps a stale orientation until unstick+next UP; bounded by the 1 s lease | `PositionManager::adjust_offset` 0x00555190; `CPhysicsObj::MoveOrTeleport` 0x00516330 (NPC UP routing, #184); retire the orientation residual in R6 |
| TS-46 | Player/remote collision spheres are passed as TWO SCALARS (radius, capsule-top height) and reconstructed by `SpherePath.InitPath` (foot center at `radius`, head center at `height radius`) — retail passes the Setup's SPHERE LIST verbatim (`CPhysicsObj::transition` 0x00512dc0 → `init_sphere(GetNumSphere, GetSphere, m_scale)`, ≤2 spheres, each origin AND radius × m_scale). With the corrected callers (0.48, 1.835 = Setup.Height) the reconstruction sits 5 mm off the dat: foot center 0.480 vs dat 0.475, head center 1.355 vs dat 1.350 (human Setup 0x02000001). **#184 Slice 3 (2026-07-07) NARROWED this: the remote de-overlap sweep now derives its scalars from the creature's OWN Setup (`GetSetupCylinder` = `setup.Radius`/`setup.Height` × ObjScale) — remotes NO LONGER use human dims regardless of Setup/scale.** RESIDUAL: (a) it is still the two-SCALAR reconstruction, not retail's ≤2-sphere LIST (lossy for creatures whose foot/head spheres differ), for both player and remotes; (b) the remote sweep's `stepUpHeight`/`stepDownHeight` stay a hardcoded 0.4 m, where retail derives them from `setup->step_up_height`/`step_down_height` (0x005180d0/0x005180f0, 0.04 m fallback, `radius×0.5` clamp) — an adjacent non-Setup divergence left for a later slice. (The pre-2026-07-06 value 1.2f put the head TOP at 1.2 m — the #137 window climb; fixed same day.) | `src/AcDream.Core/Physics/TransitionTypes.cs` (`InitPath`); `src/AcDream.App/Input/PlayerMovementController.cs` (player, human — correct as-is); `src/AcDream.App/Rendering/GameWindow.cs` (remote sweep now `GetSetupCylinder`-fed with a shapeless→human fallback) | The scalar API predates the Setup ingestion; 5 mm is below the visual/feel threshold; the remote scalars are now the creature's real (radius,height)×ObjScale, consistent with the shadow registration's entScale and the moveto/sticky radii | Marginal rε/r+ε grazes still flip on the 5 mm scalar offset; a creature whose head sphere is wider than its foot de-overlaps by the single (radius) approximation, not the true 2-sphere profile; the 0.4 m step heights are non-Setup for all movers | `CPhysicsObj::transition` 0x00512dc0; `SPHEREPATH::init_sphere` 0x0050c670 (≤2, ×m_scale); `set_description` 0x00514f40 (m_scale from wire ObjScale); retire by plumbing the full Setup sphere list into `InitPath` |
| ~~TS-45~~ | **RETIRED 2026-07-07** — the hand-rolled `SphereCollision` (forced `combinedR+1 cm` radial de-penetration + leaked `SetSlidingNormal` + always-Slid, head-sphere ignored) is REPLACED by the faithful `CSphere::intersects_sphere` family port (branch dispatcher 0x00537A80 + `step_sphere_up`/`slide_sphere`/`land_on_sphere`/`collide_with_point`/`step_sphere_down`), routing the grounded slide through the shared crease `SlideSphere` (0x00537440). Humanoid creatures collide via body Spheres, so this was the player-vs-monster crowd path; the radial de-penetration was the "can't wiggle free in a packed crowd" wedge. `SphereCollisionFamilyTests` (slide-around, block, ethereal) + `docs/research/2026-07-07-csphere-collision-family-pseudocode.md`. Residual `AP-84` (PerfectClip TOI dead in M1.5). | — | — | — | `CSphere::intersects_sphere` 0x00537A80 (pc:321678) |