From 8b5425498ca1b04d7b7c248e1d17f9875fa8ec17 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 30 Jul 2026 09:21:53 +0200 Subject: [PATCH] fix(physics): AD-25 - remote collision response through ported HandleAllCollisions Campaign P Slice P3 item 2. CPhysicsObj::handle_all_collisions (0x00514780, pc:282647) is one uniform function retail calls unconditionally after every SetPositionInternal, player or remote. The gate is shouldReflect = !(prevOnWalkable && nowOnWalkable && !sledding). RuntimeRemotePhysicsUpdater.Tick's post-resolve reflect was still the 2026-07-05 (#173) hand-inlined block, gated on resolveResult.CollisionNormalValid and using two ad-hoc branches that diverge from retail in exactly the cases the register row named: - non-sledding: old = "!prevOnWalkable && !nowOnWalkable" (reflects ONLY airborne-before-AND-after); retail reflects on every transition except grounded-before-AND-after. - sledding: old = "!(prevOnWalkable && nowOnWalkable)" (suppresses the bounce exactly when both grounded); retail's "!sledding" term forces shouldReflect = true unconditionally when sledding, the opposite polarity. Both gaps meant a remote's post-landing reflect never ran on a grounded-transition tick at all -- the "acdream lands clean and dead" half of #166's slope-landing composite. Replace the hand-inlined block with a direct call to PhysicsObjUpdate.HandleAllCollisions -- the same verbatim port the local player and every ordinary body already use via CommitSetPositionTransition -- passing the same prevContact/prevOnWalkable/nowOnWalkable values the old code already computed. Narrower swap per the research's explicit recommendation: does not fold in CommitSetPositionTransition's HitGround/LeaveGround dispatch, leaving the remote's bespoke landing-detection block (interp-queue-clear, animation-hook-specific logic) untouched. The call is now unconditional (matching retail's own unconditional call site) rather than gated behind CollisionNormalValid, since HandleAllCollisions already no-ops the reflect step internally when no normal was found but still runs the frames-stationary-fall bleed regardless. PhysicsObjUpdate.HandleAllCollisionsTests already exhaustively pins the retail formula in isolation; this change is a mechanical wiring swap to the already-tested function using values the removed block already computed. Full regression suites (Core.Tests 3991/2 skip, Runtime.Tests 425/0, App.Tests 3968/3 skip) pass unchanged -- no existing test pinned the old broken formula. Register: AD-25 retired (both the local-player and remote halves are now the ported HandleAllCollisions); #166's reattribution note updated to reflect the closure, leaving only TS-4 as the remaining blocker on that issue's downhill-jump-glide acceptance. Co-Authored-By: Claude Opus 5 --- docs/ISSUES.md | 31 ++++-- .../retail-divergence-register.md | 3 +- .../Physics/RuntimeRemotePhysicsUpdater.cs | 98 ++++++++----------- 3 files changed, 65 insertions(+), 67 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 8b01e77b..8bc0074f 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -4297,24 +4297,37 @@ now ports retail's confirmed 0.25f threshold); TS-4's removal was attempted per its own fixture-first requirement and reproduced the historical 2026-04-30 wedge, so **TS-4 stays deferred** (see its register row and the research doc §7 item 6 for the precise mechanism and the concrete next -step). Closure of #166 therefore pends: (a) TS-4 actually landing, and (b) +step). + +**AD-25 closed (2026-07-30, Campaign P Slice P3):** the remote dead- +reckoning post-resolve now calls the exact ported +`PhysicsObjUpdate.HandleAllCollisions` — the same function the local +player and every ordinary body already use — instead of its own +hand-inlined, narrower reflect gate (`RuntimeRemotePhysicsUpdater.cs`, +`Tick`). The old gate reflected only airborne-before-AND-after and +suppressed the sledding case backwards; the ported gate reflects on any +transition except grounded→grounded-and-not-sledding, matching retail's +`shouldReflect = !(prevOnWalkable && nowOnWalkable && !sledding)`. Both +halves of AD-25 (local player and remote) are now retired. + +Closure of #166 therefore pends only: (a) TS-4 actually landing, and (b) re-checking Campaign P's final visual matrix item 5 ("Downhill jump landing: sled glide + bounce") against a fresh capture — if the glide/ -bounce still visibly mismatches retail after AP-7 alone (with TS-4 still -deferred), that capture, not a guess, is what should drive any further -work here, and it should go through cdb against live retail before any -client-side Sledding-state mechanism is written. +bounce still visibly mismatches retail after AP-7 and AD-25 (with TS-4 +still deferred), that capture, not a guess, is what should drive any +further work here, and it should go through cdb against live retail +before any client-side Sledding-state mechanism is written. -**Where:** `PlayerMovementController.cs:874` (AD-25 suppression, remote -half only — local half already ported via #182), +**Where:** `src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs` +(remote reflect, AD-25 — DONE 2026-07-30), `src/AcDream.Core/Physics/PhysicsBody.cs` (`calc_friction`, AP-7 — DONE 2026-07-30), `src/AcDream.Core/Physics/BSPQuery.cs` (Path 6 steep branches, TS-4 — deferred, shortcut still in place). **Acceptance:** side-by-side downhill jump: acdream glides/bounces like retail; flat-ground landings unchanged; no micro-bounce death spiral -(the reason AD-25 exists) reintroduced. Blocked on TS-4's eventual landing -per the note above. +(the reason AD-25 existed) reintroduced. Blocked on TS-4's eventual +landing per the note above. ## #164 — UM action-replay dispatches drop the per-action Autonomous bit diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 91a8da42..d9b09107 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -62,7 +62,7 @@ accepted-divergence entries (#96, #49, #50). --- -## 2. Adaptation (AD) — 47 rows (AD-55 filed 2026-07-30 at Campaign P Slice P2, split out of the retired AP-7 row — the open cos(10°)-vs-0.99999536f Sledding slope-flatness constant; AD-53/AD-54 filed the same slice, split out of the retired TS-1 row — CliffSlide's three-source reference-normal fallback chain and the walkable-steepness reroute to CliffSlide; AD-52 filed 2026-07-29 at Campaign N slice N6 — the fragment-assembler 60 s partial TTL + completed-sequence ring; AD-51 filed 2026-07-29 at Campaign N slice N4 — the reclaimed-word pool for ACE's fresh-sequence cleartext RejectRetransmit; AD-50 filed 2026-07-29 at Campaign N slice N2 — the inbound-watermark ACE init; AD-49 stays reserved for Campaign N §5's blob-layer ordering deferral, filed when its slice lands; AD-47 and AD-48 filed 2026-07-29 at Campaign V slice V11 — the MSAA sample-position and present-pacing rows the campaign's risk register scheduled for the GL deletion; AD-11 retired 2026-07-23 — exact low-bit ItemUses predicate; AD-31 retired 2026-07-15 — the DAT-authored portal-space viewport replaces the black transit cover) +## 2. Adaptation (AD) — 46 rows (AD-25 retired 2026-07-30 at Campaign P Slice P3 — the remote dead-reckoning post-resolve now calls the exact ported `PhysicsObjUpdate.HandleAllCollisions` (the same function the local player and every ordinary body already use) instead of its own hand-inlined, narrower reflect gate; the row's own premise ("the remote DR sweep hasn't been rebuilt yet") no longer holds; AD-55 filed 2026-07-30 at Campaign P Slice P2, split out of the retired AP-7 row — the open cos(10°)-vs-0.99999536f Sledding slope-flatness constant; AD-53/AD-54 filed the same slice, split out of the retired TS-1 row — CliffSlide's three-source reference-normal fallback chain and the walkable-steepness reroute to CliffSlide; AD-52 filed 2026-07-29 at Campaign N slice N6 — the fragment-assembler 60 s partial TTL + completed-sequence ring; AD-51 filed 2026-07-29 at Campaign N slice N4 — the reclaimed-word pool for ACE's fresh-sequence cleartext RejectRetransmit; AD-50 filed 2026-07-29 at Campaign N slice N2 — the inbound-watermark ACE init; AD-49 stays reserved for Campaign N §5's blob-layer ordering deferral, filed when its slice lands; AD-47 and AD-48 filed 2026-07-29 at Campaign V slice V11 — the MSAA sample-position and present-pacing rows the campaign's risk register scheduled for the GL deletion; AD-11 retired 2026-07-23 — exact low-bit ItemUses predicate; AD-31 retired 2026-07-15 — the DAT-authored portal-space viewport replaces the black transit cover) | # | Divergence | Where (file:line) | Why it is safe / justified | Risk if assumption breaks | Retail oracle | |---|---|---|---|---|---| @@ -96,7 +96,6 @@ accepted-divergence entries (#96, #49, #50). | AD-22 | Async streamed mesh loading with bounded CPU replay residency, per-frame upload budgets, and point-of-use self-heal (`EnsureLoaded` re-request in the dispatcher's mesh-missing path, **#128**); retail loads synchronously — geometry is never absent | `src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs`; `src/AcDream.App/Rendering/Wb/MeshUploadCaches.cs`; `src/AcDream.App/Rendering/Wb/MeshUploadFrameBudget.cs` | Immutable preparation descriptors and the bounded CPU cache can re-stage an evicted mesh; dispatcher self-heal makes absence transient while upload budgets prevent a portal arrival from monopolizing a frame | A future consumer that neither retains an owner nor reaches the self-heal/replay path can remain invisible; under heavy admission pressure a valid mesh can pop in later than retail's synchronous path | retail synchronous content load; `docs/architecture/worldbuilder-inventory.md` portal-readiness and bounded-residency seams | | AD-23 | Live entities with `ServerGuid != 0` and null `ParentCellId` are culled (ClipSlotCull) while indoor clip routing is active; retail objects are always cell-resident (synchronous add-to-cell at creation) | `src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs:484` | Phase U.4 policy: parentless = unresolved indoors, equivalent to retail's not-in-any-visible-cell ⇒ not drawn, *given membership resolves promptly* | An entity whose membership lags (late CreateObject hydration, resolver hiccup) blinks invisible while the player is indoors, even in plain sight | retail per-cell object lists in PView traversal | | AD-24 | EnvCell shell geometry content-deduplicated and instanced; retail draws each CEnvCell's own structure directly | `src/AcDream.Core/Rendering/Wb/EnvCellGeometryIdentity.cs` | Phase A8 retained WB's 31× hash; the 2026-07-24 full-DAT gate proved a real collision (`0x00030175`/`0x01BC0105`), so App+Bake now share a namespaced FNV-1a tuple identity and the bake rejects any full-tuple collision | A future collision outside the installed full-DAT gate could still merge different shells at runtime; the stronger 59-bit payload makes this extremely unlikely, and every bake fails loudly rather than publishing it | retail `PView::DrawCells` → per-cell drawing_bsp (cited at the former renderer `:319`) | -| AD-25 | **REMOTE-DR sweep only** (the player half retired 2026-07-07 by the #182 verbatim rebuild): the remote dead-reckoning post-resolve still reflects velocity with the airborne-before-AND-after suppression; retail bounces unless grounded→grounded-and-not-sledding. The PLAYER path now runs the ported `handle_all_collisions` (`PhysicsObjUpdate`) with retail's `should_reflect` rule — the micro-bounce spiral it guarded is gone (contact is committed BEFORE the reflect and the small-velocity-zero is ungated) | `src/AcDream.App/Physics/RemotePhysicsUpdater.cs` (remote sweep post-resolve, #173 block) | The remote DR sweep hasn't been rebuilt yet (it has no fsf/SetPositionInternal chain); the old airborne-only suppression keeps remote landings from micro-bouncing on the remote landing-snap gate | Remote landing-reflection behavior (slope-landing momentum) won't reproduce; retire when the remote-DR sweep gets the same UpdateObjectInternal rebuild as the player | `handle_all_collisions` pc:282699-282715; ACE PhysicsObj.cs:2656-2721 | | AD-27 | PickUp fires on natural moveto completion via the `MoveToComplete` client-addition seam (retail's `CleanUpAndCallWeenie` contains no weenie call in this build and notifies nothing on arrival). The companion `MoveToCancelled` seam only withdraws the waiting pickup presentation/action. **Use retired 2026-07-25:** `ItemHolder::UseObject` sends `Event_UseEvent` before `CPlayerSystem::UsingItem`; acdream now does the same and leaves approach to ACE's authoritative MoveToChain. | `src/AcDream.App/Interaction/SelectionInteractionController.cs` (`OnNaturalMoveToComplete`/`OnMoveToCancelled`); `src/AcDream.App/Input/PlayerModeController.cs` (player MoveTo seam binding); `src/AcDream.Core/Physics/Motion/MoveToManager.cs` (`MoveToComplete`/`MoveToCancelled`) | ACE's server-side pickup chain may have timed out by the time our body arrives; the close-range deferred send hits ACE's WithinUseRadius fast-path. | If the server's chain has not timed out, pickup may execute twice or produce protocol noise on non-ACE servers | ACE CreateMoveToChain / WithinUseRadius; `MoveToManager::CleanUpAndCallWeenie` 00529650 §7e (no weenie call); `ItemHolder::UseObject` 0x00588A80 | | AD-28 | Chat transcript (`UiText`) and input (`UiChatInput`) are two separate widget classes placed inside their dat-authored container panels; retail's `ChatInterface` uses a single mode-flagged `UIElement_Text` (Type-12) that switches between read and edit mode | `src/AcDream.App/UI/Layout/ChatWindowController.cs:135` (transcript) + `:150` (input) | `UIElement_Text` is inside keystone.dll with no PDB/decomp; a two-widget split is functionally equivalent (read-only scroll, editable input) and is the structural adaptation required by our UiElement architecture | A future consumer expecting a single widget for both read/write (e.g. a plugin calling the chat API and getting one widget back) must be written to the two-widget contract | `UIElement_Text` (Type-12) @ keystone.dll; `gmMainChatUI::PostInit` @0x4ce130 | | AD-29 | `ClientObjectTable` fires global `ObjectAdded`/`ObjectUpdated`/`ObjectRemoved` events; consumers filter by guid on their end. Retail dispatches per-object via `NoticeRegistrar` observer dispatch — each UI cell observes only its specific object guid | `src/AcDream.Core/Items/ClientObjectTable.cs:48` (events); `src/AcDream.App/UI/Layout/ToolbarController.cs:115` (guid filter) | `NoticeRegistrar` is inside keystone.dll with no PDB/decomp; global broadcast + consumer-side filter is functionally equivalent for the current panel count and object volumes seen in practice | At high object counts (>1 000 objects), every `ObjectUpdated` wakes every subscribed consumer — O(n·m) notification cost instead of retail's O(1) per-observer dispatch; a consumer that forgets the guid filter processes all objects (a latent correctness bug) | `NoticeRegistrar` (keystone.dll, no PDB); retail per-object observer registration in `CObjectMaint` | diff --git a/src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs b/src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs index ba91fefd..9cf3d381 100644 --- a/src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs +++ b/src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs @@ -334,20 +334,25 @@ internal sealed class RuntimeRemotePhysicsUpdater // setup.Height) × ObjScale — the creature's own dat Setup // scaled by its wire ObjScale, the same source the local // player + moveto/sticky use, and consistent with the - // spawn-time shadow registration's entScale. Retail seeds - // the transition from the object's own Setup sphere list × - // m_scale (CPhysicsObj::transition 0x00512dc0 → init_sphere; - // ObjScale from set_description 0x00514f40). This narrows - // TS-46 (remotes no longer use human dims); the two-scalar - // API is still a lossy stand-in for retail's full (≤2) - // sphere list, and stepUp/stepDown stay 0.4 (retail derives - // those from the Setup too — an adjacent divergence left as-is). + // spawn-time shadow registration's entScale. TS-46 (2026-07-30) + // closed the remaining residual: sphereList/sphereScale below + // now carry the Setup's own verbatim sphere list (falling back + // to this deR/deH reconstruction only when the Setup has no + // sphere rows), and stepUpHeight/stepDownHeight are + // Setup-derived rather than a hardcoded 0.4f. // Fallback to the human capsule for a shapeless / unresolvable // Setup (GetSetupCylinder returns (0,0)); a zero radius would // degenerate the sweep. float deR = radius; float deH = height; if (deR < 0.05f) { deR = 0.48f; deH = 1.835f; } + // AD-25 (2026-07-30): retail handle_all_collisions (pc:282647) + // reads the mover's own transient_state CONTACT_TS/ + // ON_WALKABLE_TS bits BEFORE this SetPositionInternal-equivalent + // resolve — capture them here, matching TickHidden's identical + // pre-resolve capture below. + bool previousContact = rm.Body.InContact; + bool previousOnWalkable = rm.Body.OnWalkable; var resolveResult = _physics.Engine.ResolveWithTransition( preIntegratePos, postIntegratePos, rm.CellId, sphereRadius: deR, @@ -425,54 +430,35 @@ internal sealed class RuntimeRemotePhysicsUpdater // to actually-moving remotes — the perf risk the review flagged for // a packed town. (In-place shadow-move + cell-relink-on-change is a // further optimization if profiling still shows churn.) - // #173 (2026-07-05): retail CPhysicsObj::handle_all_collisions - // (pc:282699-282715) runs after EVERY SetPositionInternal — - // remote objects included; a VectorUpdate-launched jump arc - // is ordinary object physics in retail. acdream ported the - // velocity reflection for the LOCAL player only (L.3a, - // PlayerMovementController ~:940), so a remote jumping into - // a dungeon ceiling had its POSITION pinned by the sweep - // while its +Z velocity kept integrating — the char hovered - // at the roof until gravity burned the arc off, landing - // late (user report, 0x0007 dungeon). Mirror the local - // site exactly: - // v_new = v − (1 + elasticity)·dot(v, n)·n - // with the AD-25 suppression (bounce only when airborne - // before AND after — corridor slides and landings don't - // reflect; the landing snap below keeps its - // `Velocity.Z <= 0` gate intact). Inelastic movers - // (missiles, later) zero out instead. - if (resolveResult.CollisionNormalValid) - { - bool prevOnWalkable = rm.Body.OnWalkable; - bool nowOnWalkable = resolveResult.IsOnGround; - bool applyBounce = rm.Body.State.HasFlag( - AcDream.Core.Physics.PhysicsStateFlags.Sledding) - ? !(prevOnWalkable && nowOnWalkable) - : (!prevOnWalkable && !nowOnWalkable); - if (applyBounce) - { - if (rm.Body.State.HasFlag( - AcDream.Core.Physics.PhysicsStateFlags.Inelastic)) - { - rm.Body.Velocity = System.Numerics.Vector3.Zero; - } - else - { - var vRem = rm.Body.Velocity; - var nRem = resolveResult.CollisionNormal; - float dotVN = System.Numerics.Vector3.Dot(vRem, nRem); - if (dotVN < 0f) - { - rm.Body.Velocity = - vRem + nRem * (-(dotVN * (rm.Body.Elasticity + 1f))); - if (Environment.GetEnvironmentVariable("ACDREAM_DUMP_MOTION") == "1") - Console.WriteLine( - $"VU.bounce guid=0x{serverGuid:X8} n=({nRem.X:F2},{nRem.Y:F2},{nRem.Z:F2}) vZ {vRem.Z:F2}->{rm.Body.Velocity.Z:F2}"); - } - } - } - } + // AD-25 (2026-07-30): retail CPhysicsObj::handle_all_collisions + // (0x00514780, pc:282647) runs UNCONDITIONALLY after EVERY + // SetPositionInternal — remote objects included; a + // VectorUpdate-launched jump arc is ordinary object physics in + // retail. #173 (2026-07-05) first mirrored the local player's + // reflect math here by hand, but with a narrower gate than + // retail's: `shouldReflect = !(prevOnWalkable && nowOnWalkable + // && !sledding)` collapses to the two ad-hoc branches this + // block used to hand-roll, and got BOTH wrong — the sledding + // branch suppressed the bounce exactly when retail's + // `!sledding` term forces it UNCONDITIONALLY, and the + // non-sledding branch only reflected airborne→airborne where + // retail reflects on every transition except grounded→grounded. + // PhysicsObjUpdate.HandleAllCollisions is the same verbatim + // port the local player and every ordinary body already use + // (PhysicsObjUpdate.CommitSetPositionTransition); call it + // directly instead of re-deriving the gate. It already + // no-ops the reflect step when collisionNormalValid is false, + // but — unlike the old wrapper this replaces — still runs the + // fsf>1 unconditional velocity-zero "bleed" regardless of + // whether this tick found a collision normal, matching + // retail's own unconditional call site. + AcDream.Core.Physics.PhysicsObjUpdate.HandleAllCollisions( + rm.Body, + resolveResult.CollisionNormalValid, + resolveResult.CollisionNormal, + previousContact, + previousOnWalkable, + resolveResult.IsOnGround); // K-fix15 (2026-04-26): post-resolve landing // detection for airborne remotes. Mirrors