From 886333a2a91bb64f772cae4022d1026e30e00a5b Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 6 Aug 2026 09:35:22 +0200 Subject: [PATCH] refactor(physics): delete the redundant pre-sweep slope projection (AD-10 retired) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 0's measurement (previous commit) says the projection is redundant, so AD-10 retires by deletion rather than by narrowing. The measurement. With the sample forced to null at BOTH fork sites, from a clean build: * a remote running 30 ticks down a 31-degree walkable ramp produces a BIT-IDENTICAL trajectory, position for position; * on an 8.4-degree ramp the two differ by at most 2.8e-5 m in Z after 30 ticks (0.03 mm) and are identical in X and Y — float ordering noise from projecting twice against the same plane rather than once; * the whole AcDream.Runtime.Tests suite is unchanged. That is what redundancy looks like, and the arithmetic explains it. The boundary projection and Transition.AdjustOffset are the same operation (v -= N * dot(v, N)) against the same plane, and the composition is idempotent: a vector already on the plane has dot(v, N) == 0, so the sweep's own projection is a no-op on an already-projected offset and the full-strength projection on an unprojected one. Either alone produces the same offset. On terrain a THIRD mechanism, ValidateWalkable's push-out, re-seats the sphere on the plane every sub-step regardless. Deleted: * both RuntimeRemotePhysicsUpdater sample sites (the host and no-host fork branches carried the block verbatim — the AP-22 shape, a row naming one site where two exist); * the terrainNormal parameter and projection block on RemoteMotionCombiner.ComposeOffset; * the same block on ComputeOffset, which has no production callers but held a second copy of the divergence, so leaving it would have made the row's retirement false; * PhysicsEngine.SampleTerrainNormal, now callerless. Removing the parameter rather than passing null is deliberate: it is what makes a future one-site-only regression a compile error instead of a silent half-fix. Two tests went with it — ComputeOffset_RootMotionFallback_SlopedTerrainNormal_ProjectsZOntoSlope and its flat-ground twin. Both were weak on their own terms: they drove the production-dead ComputeOffset and computed their expected values by re-implementing the projection formula, so they could catch a wrong MULTIPLY but never a wrong PLANE — which is exactly what the divergence was. The surviving coverage is geometric and runs the production tick. Three claims in the old row did not survive contact with the code and are recorded in the retired row rather than quietly dropped: the justification (remotes do run the sweep); the description of ComposeOffset's guard as "interpolation-active" when the code reads `if (!interpolationOverwrote`; and the roof clause, stale since Bug B gated the sample on OnWalkable — a steep roof is OnWalkable == false, so the path never ran on #32's geometry. The retail anchor is corrected too: pc:272296-272346 truncated both the sliding-normal validity gate at the head and the entire safety push-out block at the tail. The whole function is 0x0050a370, pc:272271-272393. This does not fix #32 and does not partially fix it. #32's remote half was already closed at 204d0ae0. What deletion does improve is the case #32 never covered: a remote on a WALKABLE non-terrain surface — a bridge, a dock, a gentle roof, a ramp inside a building — where the terrain sample returned the plane of the ground far below and applied a wrong plane rather than none. That surface now gets the body's own committed contact plane, because that is the only projection left. The planning contract this work executed is committed alongside as docs/research/2026-08-06-ad10-contract.md. Release build 0 errors. Complete solution suite 11,196 passed / 4 skipped / 0 failed against the ef976c6d baseline of 11,195 / 4 / 0 — reconciled exactly as +3 new Runtime tests and -2 deleted Core tests. Visual gate outstanding: G1 (the ~5 Hz staircase on rolling terrain) is the veto criterion and runs first; then slope-descent smoothness, a walkable non-terrain surface, the #32 roof scenario, and flat ground. Co-Authored-By: Claude Opus 4.8 --- docs/ISSUES.md | 15 +- .../retail-divergence-register.md | 2 +- docs/research/2026-08-06-ad10-contract.md | 805 ++++++++++++++++++ src/AcDream.Core/Physics/PhysicsEngine.cs | 22 +- .../Physics/RemoteMotionCombiner.cs | 66 +- .../Physics/RuntimeRemotePhysicsUpdater.cs | 52 +- .../Physics/RemoteMotionCombinerTests.cs | 68 +- 7 files changed, 885 insertions(+), 145 deletions(-) create mode 100644 docs/research/2026-08-06-ad10-contract.md diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 338ee30e..345e0987 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -10387,6 +10387,13 @@ other pieces that are either incomplete or unverified for remotes: project against, so even a corrected `OnWalkable` would need a real contact-plane-derived slide, not the terrain-only approximation AD-10 already flags as a divergence. + **Superseded 2026-08-06:** AD-10 was RETIRED BY DELETION, and this + paragraph's premise turned out to be inverted. Remote bodies DO run the full + sweep (`ResolveWithTransition` -> `Transition.AdjustOffset`, retail + `CTransition::adjust_offset` 0x0050a370 per sub-step), so the roof slide was + already driven by a real contact-plane projection; the terrain sample was an + EXTRA, non-retail layer on top of it and is now gone. Measured redundant + before deletion — see the retired register row. The investigation stopped there per project policy and instrumented a probe (`ACDREAM_PROBE_REMOTE_LANDING`, then the `[remote-slide-*]` family in @@ -10537,11 +10544,15 @@ pass that recorded them.** Still open on this row: the two dependencies named above. **#173**'s remote collision-velocity reflect now genuinely runs on a steep contact (the old code passed `IsOnGround` as `nowOnWalkable`, which suppressed the reflect exactly -where retail forces it), but its visual gate is still unrun. **AD-10**'s +where retail forces it), but its visual gate is still unrun. ~~**AD-10**'s terrain-only slope projection still cannot see building geometry; it is now correctly gated OFF while the body is not on walkable ground, so a roof slide is driven by gravity plus the sweep's own plane projection rather than by that -approximation. The retail-strict `step_up_slide`/`cliff_slide` audit that this +approximation.~~ **Closed 2026-08-06:** AD-10 was retired by deletion — the +terrain-only projection no longer exists anywhere in the tree, so this +dependency is discharged rather than merely gated. The roof slide is driven by +gravity plus the sweep's own contact-plane projection, which is what retail +does. The retail-strict `step_up_slide`/`cliff_slide` audit that this row was originally filed for is unchanged. --- diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 18c2241f..ae7c13ef 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -119,7 +119,7 @@ readiness/requeue adaptation. See | AD-2 | Async readiness gates replace retail's synchronous destination cell load. **#229 refinement (2026-07-20):** login and F751 portal-space exit now share `WorldRevealReadinessBarrier`, so neither path can expose the normal viewport until the same render-publication, composite-texture, and collision domains converge. A hydratable indoor claim requires its owning Near-tier static/EnvCell mesh set, destination composites, and exact EnvCell physics (`IsSpawnCellReady`); an outdoor claim requires those render domains plus terrain/collision residency across the DERIVED reveal window. **#280 amendment (2026-08-05):** that outdoor window is no longer a hardcoded radius-1 neighbourhood. Retail has exactly ONE landscape square — `LScape::mid_radius`, assigned directly from the `Render.LandscapeDrawDistance` preference (`SmartBox::SetRegion` @0x004531F0; values `Render_LandscapeDrawDistance_Values` @0x007CA988 = {3,5,8,11,15,25}, default 8, byte-verified) — and that same square is simultaneously the loaded set, the drawn set, and the set `LScape::PreFetchCells` @0x00505660 blocks on, so retail structurally cannot stream farther than it gates. acdream now DERIVES the outdoor radius from the live streaming window (`QualitySettings.FarRadius`, read per evaluation from `StreamingController` so a mid-hold Settings change re-arms the gate the way `SmartBox::set_mid_radius` @0x00453180 does), and the render-completeness predicate is TIER-AWARE to match acdream's two-tier landscape: inside `NearRadius`, full Near publication (`IsNearTier && IsRenderReady`); out to `FarRadius`, terrain publication only (`IsRenderReady`, which a Far-tier landblock satisfies through an empty spawn-adapter registration installed after its terrain upload crossed the render-thread barrier). **#280 review correction (2026-08-06):** as originally written this row asserted that property of a `PublicationKind.Far` *publication* only, which was true but not exhaustive — a landblock also reaches Far tier by Near→Far DEMOTE, and the demote's `LandblockRetirementStage.MeshReferences` left `LandblockSpawnAdapter.WantsLoaded == false` on a landblock that stays loaded and drawn, with no path that re-publishes it. Both review lenses found the same defect: one demoted member anywhere in the far ring made the gate unsatisfiable for the life of the streaming window (permanent portal-space hang, no recovery short of relog), reachable by two consecutive recalls to the same landblock with walking in between, or by a mid-hold quality-preset drop. The two routes are now genuinely equivalent — `GpuWorldState.ReleaseLandblockMeshReferences` re-asserts the empty Far registration after retiring the Near layer — rather than the predicate being taught to tolerate two meanings of "ready". Composite-texture warmup stays `NearRadius`-scoped because it is entity-scoped and Far builds carry no entities, and (same correction) its TRIGGER is scoped the same way: gating warmup on the whole widened gate serialised every composite upload behind the last outer-ring landblock, which is a hold longer than the streaming work requires. The destination reservation opens at exactly the gate's radius, since retail has one square for both. Runtime's readiness invariant is correspondingly a SHAPE check (`indoor ⇒ 0`, `outdoor ⇒ ≥1`), never a re-encoded value — Runtime does not own the graphical host's streaming configuration. Hard-recenter generations and tier-aware completion application prevent stale overlapping loads/unloads or Far/Near jobs from opening or erasing the gate; mesh upload remains separate from balanced landblock ownership. Claims beyond NumCells still take the loud unhydratable-placement path. `RuntimeWorldTransitState` owns the shared reveal generation, accepted readiness, transit correlation, and exact generation/cell-scoped host-acknowledgement suffix. `WorldRevealCoordinator` is a graphical adapter holding only App resource receipts; normalized Runtime checkpoints observe ownership without defining another readiness path. **Slice E3 refinement (2026-07-24):** the same generation now publishes an immediate `WorldGenerationQuiescence` edge: old-world drawing/spatial queries, simulation/effect clocks, reconciliation, targeting, and 3-D audio stop while retained physical teardown advances through metered cursors and destination network/UI/streaming/readiness remain live. **Slice E4 refinement (2026-07-24):** accepted render/physics/static publication may span update frames through retained exact cursors, but reveal still consumes only the completed spatial/render-ready generation; building and EnvCell snapshots remain invisible until complete and the final spatial identity swap stays observer-atomic. **Slice E5 refinement (2026-07-24):** the reveal generation owns one exact destination reservation across every typed budget dimension. Stale completion cannot consume or clear its replacement, and hydratable incomplete content is never force-revealed; portal transit retains the DAT tunnel and centered retail wait cue until readiness converges. The hold→materialize→regain-control lifecycle remains owned by `TeleportAnimSequencer`. **C4 route 3 refinement (2026-08-04):** retail places the local player IMMEDIATELY on the accepted destination Position (`SmartBox::TeleportPlayer` @0x00453910) and blocks SIMULATION on DAT prefetch (`CellManager::blocking_for_cells`; `SmartBox::UseTime` @0x00455410 runs only `CheckPrefetchStatus`) behind the portal viewport; acdream defers the PLACEMENT itself to this reveal-ready Place edge, executed by the canonical `RuntimeAcceptedPositionDriveController` portal arm (`TryExecuteAcceptedPortalArrival`). Two load-bearing notes from that route: (1) every accepted local Apply — including the portal destination Position itself — still writes the raw wire pose onto the local player's `WorldEntity` via the ordinary generic-remote-render-pose path while portal space covers the viewport (`LiveEntityNetworkUpdateController.cs`, `OwnsSteadyState` false for the local player's null route); the committed Place receipt's presentation suffix overwrites it with the resolved pose — tolerated, not suppressed, since suppressing it would be an unowned behaviour change on the ordinary local Apply path (AP-131/#275 territory). (2) The constraint-leash re-arm on a committed portal placement anchors at the RESOLVED post-placement body position (`PlayerMovementController.CommitCanonicalTeleportFrame` → `RearmConstraintLeashAtCurrentPosition`), where retail's `ConstrainTo` @0x0045418A anchors at the received WIRE destination; the two differ by at most the placement adjustment (ring search/floor snap) and the anchor is write-only downstream, so the delta is not user-observable — switching to the wire-destination anchor is a deliberately deferred decision, not adopted here. **B4 round-3 review refinement (2026-08-05):** the wait cue's trigger predicate (`LocalPlayerTeleportController.Tick`'s `placementReady = dataReady && TryAdvancePortalCommit(sequence)`, gating the cue at `haveDestination && !placementReady`) now covers a SECOND, distinct cause beyond the original streaming/DAT-readiness gate this row described: `TryAdvancePortalCommit` returning false while a DeferredCell park is outstanding or a fresh placement attempt has not yet succeeded (B1's `TryConsumePortalCommit` gate). The cue's five-second trigger and centered-tunnel behavior are unchanged (that trigger is an acdream divergence in its own right — AP-150, filed 2026-08-06); only the SET of conditions that can hold it open grew from "world data not ready" to "world data not ready OR canonical placement not yet committed" — a slow-publishing destination-landblock collision generation now presents identically to a slow asset stream, which is the correct retail-faithful degradation (both are `blocking_for_cells` causes retail itself does not distinguish), but is worth naming here since a future debugging session seeing the cue must not assume streaming is the only possible cause. | `src/AcDream.Runtime/World/RuntimeWorldTransitState.cs`; `src/AcDream.App/Streaming/WorldRevealCoordinator.cs`; `src/AcDream.App/Streaming/WorldGenerationQuiescence.cs`; `src/AcDream.App/Streaming/WorldRevealReadinessBarrier.cs`; `src/AcDream.App/Streaming/StreamingOriginRecenterCoordinator.cs`; `src/AcDream.App/Streaming/LandblockPresentationPipeline.cs`; `src/AcDream.App/Streaming/StreamingController.cs`; `src/AcDream.App/Rendering/PortalTunnelPresentation.cs`; `src/AcDream.App/UI/PortalWaitNoticeController.cs`; `src/AcDream.App/Streaming/GpuWorldState.cs` (`IsRenderReady`); `src/AcDream.App/Rendering/Wb/LandblockSpawnAdapter.cs`; `src/AcDream.Core/Physics/PhysicsEngine.cs` (`IsSpawnCellReady`, `IsNeighborhoodTerrainResident`) | This is the asynchronous equivalent of retail leaving `SmartBox::position_update_complete` false while `CellManager::blocking_for_cells` is set: neither initial login nor portal arrival may reveal or continue simulating an old/partial collision world, a terrain-only Far shell, or a published-but-not-drawable GPU landblock. Indoor does not require a terrain heightmap, only the owning render landblock and exact EnvCell. | Gate opens early → grey/untextured first login or portal reveal, free-fall, wrong-cell rooting, missing scenery, or a still-active old generation; predicate never satisfies (streamer/DAT/upload failure) → login remains behind the world render gate, while portal transit remains in the authored tunnel and presents the centered wait cue after five seconds — that five-second arming is acdream's own and is NOT retail's trigger; see AP-150. | `SmartBox::UseTime` 0x00455410; `gmSmartBoxUI::UseTime` 0x004D6E30; `gmSmartBoxUI::EndTeleportAnimation` 0x004D65A0; `LScape::PreFetchCells` 0x00505660; `LScape::SetMidRadius` 0x00504C00; `SmartBox::set_mid_radius` 0x00453180; `Render_LandscapeDrawDistance_Values` 0x007CA988 | | AD-5 | Outdoor `point_in_cell` is an identity compare against the global XY-column cell from `LandDefs.AdjustToOutside` (no per-cell containment test) | `src/AcDream.Core/Physics/CellTransit.cs:865` | Landcells are disjoint 24 m columns — identity-compare against the column under the sphere centre is exactly equivalent to retail's per-candidate test | If block-origin/lcoord math is wrong at a landblock seam, the compare silently never matches — outdoor membership freezes at boundaries (the pre-#106 symptom) | `find_cell_list` pick pc:308788-308825; `CLandCell::point_in_cell` (get_block_offset pc:308804) | | ~~AD-6~~ | **RETIRED 2026-07-31 (placement/streaming Slice 3B).** Cell/cache/topology/building/static-shadow publication plus every retained non-suspended owner touching or withdrawn from the prefix is one Runtime-owned collision generation. Retained includes dynamics and adjacent-root statics; only target-root statics are superseded by the authored replacement. App and Headless build one shared off-side `CollisionWorldState` through one-work-unit preparation/capture/seal cursors. Admission captures the active root in O(1); a stable landblock/owner slot suffix materializes non-target leaves incrementally, so resident-world size cannot become a synchronous clone spike. Reusable per-prefix owner slots and one Runtime-scoped versioned journal replace event-time exact-copy fanout: repeated live mutations coalesce by owner, every draft reconciles only that owner's latest exact state one owner per seal call, discovered relevant owners receive scoped exact updates, and visited unrelated owners receive only a cheap coalesced dirty notification before metered replay. Once topology sealing finishes, observed owners temporarily write through exactly until same-call activation; the finite pre-seal queue therefore drains even under continuous multi-owner movement. New drafts start at their captured journal suffix; old slots are superseded rather than reused behind live cursors and compact through the same meter. Unrelated churn therefore never restarts or starves target capture/sealing. Deterministically ordered concurrent preparations receive committed—not merely sealed—peer deltas and rebase one cache, graph, landblock, or owner leaf per seal step; cancellation therefore cannot leak unpublished topology. Demotion/withdrawal cancels a matching queued or active rebase, suppresses the prefix in unfinished source scans, and retires one owner/cache/graph/outdoor leaf per seal call. The complete previous generation remains queryable until one zero-managed-byte volatile root transfer in the same update-thread call as final reconciliation; that preserves PhysicsDataCache, CellGraph, PhysicsEngine, and ShadowObjectRegistry facade identity, revokes staging, and requires no quiet frame. A stale admission or staging failure disposes only that private generation and cannot withdraw the active world or invalidate a newer admission. Authored same-ID target statics, live-current-cell changes, owner departure/reuse, newly relevant seam-crossing statics, and teardown remain coherent across drafts; empty per-prefix owner containers are reclaimed without invalidating captured seal cursors. The commit clears repaired withdrawal markers before its single notification/readiness acknowledgement, so no optional hydration callback can omit reflood and no observer sees mixed old/new cells. | `src/AcDream.Runtime/Physics/RuntimePhysicsState.cs` (`PrepareCollisionGeneration`, `AdvanceCollisionGenerationPreparation`, `AdvanceCollisionGenerationSeal`, `CommitCollisionGeneration`); `src/AcDream.Core/Physics/CollisionWorldState.cs`; `PhysicsDataCache.cs`; `PhysicsEngine.cs`; `ShadowObjectRegistry.cs`; `src/AcDream.App/Streaming/LandblockPhysicsPublisher.cs`; `src/AcDream.Headless/Hosting/HeadlessSessionWorldProjection.cs`; `tests/AcDream.Runtime.Tests/Physics/RuntimePhysicsStateTests.cs`; `tests/AcDream.App.Tests/Streaming/LandblockPhysicsPublisherTests.cs`; `tests/AcDream.Headless.Tests/HeadlessSessionHostTests.cs` | — | — | `CObjCell::init_objects` → `CPhysicsObj::recalc_cross_cells`, 0x0052b420 / 0x00515a30; `CPhysicsObj::SetPositionInternal` shadow replacement tail 0x00515330 | -| AD-10 | Remote slope projection relocated to the queue-empty/head-reached combiner boundary; retail projects inside `CTransition::adjust_offset` during the sweep | **2026-08-04: file:line corrected** — the mechanism now lives in `src/AcDream.Core/Physics/RemoteMotionCombiner.cs` (`ComposeOffset` ~:65-72 for the interpolation-active boundary projection, the queue-empty fallback ~:163-168); the row's meaning is unchanged, the class was renamed/moved from the stale `PositionManager.cs:47` citation (see the class's own doc comment: "Renamed R5 (was PositionManager)") | Remote bodies don't run a full local transition sweep; boundary projection removes the ~5 Hz Z staircase on slopes, no-op on flat ground | The single-point terrain-normal sample can differ from the sweep's contact plane (cell boundaries, props underfoot) — remote Z drift / stair-stepping; it also cannot see building/EnvCell geometry at all (terrain-only sample), so a remote landing on a house roof gets no slope response from this path regardless of `OnWalkable` — a contributing factor in the 2026-08-04 Bug B roof-plant observation (see `docs/ISSUES.md` #32) | `CTransition::adjust_offset` pc:272296-272346 | +| ~~AD-10~~ | **RETIRED 2026-08-06 by deletion.** The row's justification ("remote bodies don't run a full local transition sweep") was false at HEAD: `RuntimeRemotePhysicsUpdater.Tick` calls `PhysicsEngine.ResolveWithTransition` with the remote's own body, and that sweep runs acdream's verbatim port of `CTransition::adjust_offset` once per sub-step. So this was never a relocation of a missing mechanism — it was an EXTRA pre-sweep projection layered on top of the faithful one, against a surface retail never uses (`SampleTerrainNormal(x, y)`, an XY-only landblock lookup blind to the body's Z, its cell, buildings, EnvCells and statics). Measured before deleting: with the projection forced null at both fork sites, the production trajectory of a remote running 30 ticks down a 31-degree ramp is BIT-IDENTICAL, a 8.4-degree ramp differs by at most 2.8e-5 m in Z, and the whole `AcDream.Runtime.Tests` suite is unchanged. Deleted: both `RuntimeRemotePhysicsUpdater` sample sites, the `terrainNormal` parameter and projection block on `RemoteMotionCombiner.ComposeOffset` AND on the production-dead `ComputeOffset`, and the now-callerless `PhysicsEngine.SampleTerrainNormal`. Removing the parameter is what makes an AP-22-shaped one-site-only regression a compile error. Two claims in the old row were also stale/backwards and did not survive: it described `ComposeOffset`'s guard as "interpolation-active" when the code is `if (!interpolationOverwrote ...)`, and its second cited site (`ComputeOffset` ~:163-168) had zero production callers. The roof clause was stale too — since Bug B (`204d0ae0`) the sample was gated on `OnWalkable`, and a steep roof is `OnWalkable == false`, so the path did not run on #32's geometry at all. | `src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs`; `src/AcDream.Core/Physics/RemoteMotionCombiner.cs`; `src/AcDream.Core/Physics/PhysicsEngine.cs` (deletion); `tests/AcDream.Runtime.Tests/Physics/RuntimeRemoteSlopeProjectionTests.cs`; `tests/AcDream.Runtime.Tests/Physics/RemoteRampHarness.cs` | — | — | `CTransition::adjust_offset` 0x0050a370, pc:272271-272393 (the old anchor pc:272296-272346 truncated both the sliding-normal validity gate at the head and the entire safety push-out block at the tail); per-step call from `CTransition::find_transitional_position` 0x0050bdf0 | | ~~AD-11~~ | **RETIRED 2026-07-23** — the matching binary disproved the old nonzero interpretation: `ItemUses::IsUseable` executes `not bitfield; and eax,1`, so absent/reset zero is usable and only `USEABLE_NO` disables use. Toolbar, item policy, and world interaction now share that exact Core predicate. | `src/AcDream.Core/Items/ClientObject.cs` (`ItemUseability.IsUseable`); `src/AcDream.Core/Items/ItemInteractionPolicy.cs`; `src/AcDream.App/Interaction/WorldSelectionQuery.cs` | — | — | `ItemUses::IsUseable @ 0x004FCCC0`; matching v11.4186 instructions recorded in `docs/research/2026-07-23-retail-item-use-and-autowear-pseudocode.md` | | AD-12 | SecondaryAttributeTable coefficients hardcoded (Health=End×0.5, Stam=End×1.0, Mana=Self×1.0) instead of dat-read; unknown attributes contribute 0 | `src/AcDream.Core/Player/LocalPlayerState.cs:279` | Coefficients never vary across retail dat versions; re-confirmed by ACE AttributeFormula.cs + holtburger; dat port can replace later | A customized portal.dat with modified vital formulas silently yields wrong max-vitals; a missing attribute snapshot underestimates max | SecondaryAttributeTable portal.dat 0x0E0..0x0E2; `CreatureVital::GetMaxValue` 0x0058F2DD | | AD-13 | 1-second dedup window for identical system chat messages (retail has none) | `src/AcDream.Core/Chat/ChatLog.cs:29` | ACE dual-sends the same system text (0xF7E0 + 0x02EB) for back-compat; without dedup every line doubled (Phase J compromise) | Two genuinely distinct but textually identical system messages within 1 s collapse to one line where retail shows both | ACE dual-send 0xF7E0 + 0x02EB | diff --git a/docs/research/2026-08-06-ad10-contract.md b/docs/research/2026-08-06-ad10-contract.md new file mode 100644 index 00000000..719db6f2 --- /dev/null +++ b/docs/research/2026-08-06-ad10-contract.md @@ -0,0 +1,805 @@ +# AD-10 contract — remote slope projection at the combiner boundary + +**Date:** 2026-08-06 +**Worktree:** `.claude/worktrees/peaceful-visvesvaraya-e0a196`, +branch `claude/acdream-physics-divergence-5aa784`, HEAD `ef976c6d` +**Register row:** AD-10 (`docs/architecture/retail-divergence-register.md:122`) +**Status of this document:** planning contract. No production or test code was +written; no commit was made. + +--- + +## 0. Verdict up front + +**AD-10 cannot be asserted retirable today, and it must not be force-retired. +But its retirement is *decidable*, and deciding it costs one offline +measurement against a harness that already exists.** + +Three separate things were conflated in the row and are now separated: + +| | Claim | Status at HEAD | +|---|---|---| +| **A** | Retail's in-sweep contact-plane projection is missing from acdream | **False.** `Transition.AdjustOffset` (`src/AcDream.Core/Physics/TransitionTypes.cs:5180`) is a faithful port of `CTransition::adjust_offset` and runs per sub-step inside the sweep (`:1486`), and remotes *do* run that sweep (`src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs:399`). | +| **B** | The remote path has an *extra*, non-retail projection before the sweep | **True.** `RemoteMotionCombiner.ComposeOffset` `:65-73`, fed from `PhysicsEngine.SampleTerrainNormal` at `RuntimeRemotePhysicsUpdater.cs:278-282` and `:321-325`. | +| **C** | That extra projection samples the wrong surface | **True and unambiguous.** `SampleTerrainNormal(x, y)` (`PhysicsEngine.cs:1019`) is a pure XY→landblock terrain lookup. It ignores the body's Z, its cell, buildings, EnvCells, statics, and other objects. | + +So AD-10 is **not a relocation of a missing mechanism**. It is an **additional +pre-sweep projection layered on top of the faithful one**, against a surface +retail never uses. + +That reframing gives two candidate ships, and the contract sequences them so +the cheap measurement runs first: + +- **Stage 0 (measurement, no production change).** Determine whether the + pre-sweep projection is redundant with the sweep's own `AdjustOffset`. If it + is, **AD-10 retires by deletion** — the cleanest outcome available. +- **Stage 1 (the fallback ship, if Stage 0 says the projection is + load-bearing).** Change the *sample source*, not the mechanism: + `SampleTerrainNormal(x, y)` → the body's own committed + `ContactPlane.Normal`. That is 2 statements. It **retires AD-10's entire + risk column** — the terrain/sweep disagreement, the cell-boundary sample, + the props-underfoot sample, and the building/EnvCell blindness all vanish, + because the projection surface becomes *the same plane the sweep uses*. What + survives is only the row's first clause: the projection still happens at the + combiner boundary rather than inside the sweep. AD-10 is **narrowed**, not + retired. + +**Do not skip Stage 0 to get to Stage 1.** Stage 0 is the only thing that can +justify a deletion, and deletion is strictly better than narrowing. + +**Do not skip Stage 1 to get to deletion.** The historical record (§2.4) shows +the sweep was already present and already seeding a contact plane on the day +the pre-sweep projection was added to fix a real, observed staircase. I could +not establish from static reading why the sweep's projection was insufficient +then. Assume it was for a reason until Stage 0 says otherwise. + +--- + +## 1. Retail's mechanism, pinned + +### 1.1 The function + +`CTransition::adjust_offset` @ **0x0050a370**, pseudo-C +`docs/research/named-retail/acclient_2013_pseudo_c.txt:272271-272400`. + +> **Citation correction.** The register row cites `pc:272296-272346`. That range +> lands *inside* the function but truncates both ends: it omits the +> sliding-normal validity gate at the head and the entire post-projection +> safety push-out block plus the sliding-normal-only tail at 272355-272400. +> Cite **272271-272400** (the whole function) or the address **0x0050a370**. + +### 1.2 What it does, verified by disassembly + +Binary Ninja's pseudo-C renders every x87 comparison here as the +`fnstsw`/`test ah, imm8` mush. All four comparisons were re-read from the +PDB-paired binary. + +Binary: `C:\Users\erikn\Downloads\acclient.exe`. +`py tools/pdb-extract/check_exe_pdb.py` → `=== MATCH: this exe pairs with our +acclient.pdb ===` (GUID `{9e847e2f-777c-4bd9-886c-22256bb87f32}`, age 1). +Disassembled with capstone 5.0.7, image base 0x400000. + +``` +adjust_offset(this, out, offset): + v = *offset + keepSlide = 0 + if (collision_info.sliding_normal_valid) { + # 0050a3bc fcomp [0x795344] ; 0050a3c4 test ah,1 ; 0050a3c7 jne 0x50a3d1 + # C0 alone == "less than" + if (dot(v, sliding_normal) < 0) keepSlide = 1 + else collision_info.sliding_normal_valid = 0 + } + if (collision_info.contact_plane_valid) { + cAngle = dot(v, contact_plane.N) + if (keepSlide) { + cross = sliding_normal x contact_plane.N + if (normalize_check_small(cross) == 0) v = cross * dot(cross, v) + else v = 0 + } + else { + # 0050a4fa fcomp [0x795344] ; 0050a502 test ah,0x41 ; 0050a505 jne 0x50a515 + # 0x41 == C0|C3 == "less than or equal"; jne taken -> 0x50a515 (SUBTRACT) + if (cAngle <= 0) v -= contact_plane.N * cAngle # 0x0050a515 + else Plane::snap_to_plane(&contact_plane, &v) # 0x0050a50e + } + # post-projection safety push-out + if (!contact_plane_is_water && contact_plane_cell_id != 0) { + LandDefs::get_block_offset(&blockOff, sphere_path.check_pos.objcell_id, + contact_plane_cell_id) + dist = dot(global_sphere.center - blockOff, contact_plane.N) + contact_plane.d + # 0050a5cf fcompp ; 0050a5d3 test ah,5 ; 0050a5d6 jp -> skip + # test ah,5 / jp is the canonical "jump if >=" idiom + if (dist < global_sphere.radius - 0.0002) { + zDist = (global_sphere.radius - dist) / contact_plane.N.z + # 0050a5eb fcompp ; 0050a5ed test ah,0x41 ; 0050a5f0 jne -> skip + if (global_sphere.radius > |zDist|) + SPHEREPATH::add_offset_to_check_pos(&sphere_path, (0, 0, zDist)) + } + } + } + else if (keepSlide) { + v -= sliding_normal * dot(v, sliding_normal) + } + *out = v +``` + +Float constants read from the binary: +`0x795344 = 0.0f` (bytes `00000000`); +`0x7c6878 = 0.00019999999494757503f` (bytes `17b75139`). + +`Plane::snap_to_plane` @ **0x00509c50** (pc:271852): +``` +if (|N.z| > 0.0002) # 00509c5f test ah,5 ; 00509c62 jnp + v.z = -(v.x*N.x + v.y*N.y) / N.z # v.x, v.y UNTOUCHED +``` + +### 1.3 The three answers the row asks for + +**What does retail project?** The **per-sub-step movement offset**, not the +whole frame delta. `calc_num_steps` splits the transition into steps of one +sphere radius; `adjust_offset` runs once per step +(`find_transitional_position` @ 0x0050bdf0, call at pc:273695 / `0x0050bf66`). + +**Against what surface?** `collision_info.contact_plane` — *whatever surface +the previous sub-step's collision actually found*. Its producers are: + +| Producer | Address | Surface class | +|---|---|---| +| `CTransition::init_contact_plane` | 0x0050e850 | initial seed | +| `BSPTREE::step_sphere_down` | 0x0053a210 | **BSP polygons — buildings, EnvCells, dungeon geometry, statics** | +| `BSPTREE::find_collisions` | 0x0053a440 | same | +| `CSphere::step_sphere_down` | 0x00536d20 | **other objects** | +| `CCylSphere::step_sphere_down` | 0x0053a9b0 | **other objects** | +| `CSphere::intersects_sphere` / `CCylSphere::intersects_sphere` | 0x00537a80 / 0x0053b440 | other objects | +| `CTransition::validate_transition` | 0x0050aa70 | last-known restore | + +Retail's projection surface is therefore the **general contact plane from any +collidable geometry**. It sees buildings and EnvCells natively. This is the +direct, primary-source confirmation of the gap AD-10's risk column names. + +**At what point in the sweep?** At the **head of each sub-step, before that +step's `transitional_insert`**, consuming the plane the *previous* step +established. acdream's port preserves that ordering verbatim +(`TransitionTypes.cs:1483-1486` — `AdjustOffset` first, state cleared after, +`:1523-1527`). + +### 1.4 Two unregistered divergences found inside acdream's port of this function + +Both are in `Transition.AdjustOffset` +(`src/AcDream.Core/Physics/TransitionTypes.cs:5180-5320`). Neither has a +divergence-register row (grepped: no row mentions `snap_to_plane`, +`SnapToPlane`, `naturalResting`, or `away-plane`). Both are **out of scope for +this change** — but both are in the same function AD-10 points at, and one of +them is directly about slope descent, so they are recorded here rather than +absorbed. + +**(a) The `snap_to_plane` branch is substituted, not ported.** +`TransitionTypes.cs:5252-5258`: +```csharp +else +{ + // Moving away from contact plane: snap to plane surface. + result -= ci.ContactPlane.Normal * collisionAngle; + branch = "away-plane"; +} +``` +This makes the `if` and the `else` **byte-identical** — both do +`result -= N * collisionAngle`. Retail's `else` calls `snap_to_plane`, which +adjusts **only Z** and leaves XY alone. + +For a slope of angle θ and a horizontal step of length `d`: + +| Direction | `dot(v,N)` | Retail result | acdream result | +|---|---|---|---| +| **Uphill** | `< 0` | `d·cosθ` along the plane (XY shrinks by cos θ) | identical | +| **Downhill** | `> 0` | XY preserved at `d`, Z drops `d·tanθ` (speed along the plane `d/cosθ`) | XY shrinks to `d·cos²θ`, speed along the plane `d·cosθ` | + +So acdream descends slopes **slower than retail by a factor of cos²θ in XY**: +13% slow at 30°, 29% at 45°. Uphill is correct. This is a *plausible* +contributor to the open **#269 slope-slide feel residual** (Campaign P), which +CLAUDE.md records as still needing a live cdb A/B — worth handing to whoever +picks #269 up, but **do not fold it into AD-10**: it changes local-player +movement feel and needs its own visual gate. + +**(b) The safety push-out threshold is deliberately altered.** +`TransitionTypes.cs:5285-5309` replaces retail's `radius` with +`naturalRestingDist = radius * ContactPlane.Normal.Z` in both the trigger +comparison and the `zDist` numerator. The code comment argues the case at +length and says "ACE and the published pseudocode have the original +threshold". The disassembly at 0x0050a5c4-0x0050a5ff confirms retail uses the +bare `radius` in both places. Whether or not the correction is right, **an +intentional deviation from a byte-confirmed retail constant with no register +row is exactly what the register exists to catch.** + +**Action:** file both as register rows (or as one row with two clauses) in a +separate commit. Neither blocks AD-10. + +--- + +## 2. What remote bodies actually run at HEAD + +Established by symbol, not from inherited documentation. + +### 2.1 The tick + +`RuntimeRemotePhysicsUpdater.Tick`, +`src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs`: + +1. `:142` — `bool bodyOnWalkableAtTickStart = rm.Body.OnWalkable;` +2. `:143-146` — root motion scaled by `objectScale` **only while OnWalkable** + (retail `UpdatePositionInternal` @0x00512CA1). Zero otherwise. +3. `:278-282` / `:321-325` — **the AD-10 sample**: + `bodyOnWalkableAtTickStart ? _physics.Engine.SampleTerrainNormal(Body.X, Body.Y) : null` +4. `:283-292` / `:326-335` — `rm.Position.ComposeOffset(..., terrainNormalNpc, inContact: rm.Body.InContact)` +5. `:293` / — `npcHost.PositionManager.AdjustOffset` (Sticky → Constraint) +6. `:301` / `:336` — `ApplyPositionManagerDelta` → `body.Position += Transform(delta.Origin, orientation)` (`:1087-1098`) +7. `:338-339` — `calc_acceleration()`, `UpdatePhysicsInternal(dt)` +8. `:371-451` — **`_physics.Engine.ResolveWithTransition(preIntegratePos, postIntegratePos, …, body: rm.Body, …)`** — the full sweep +9. `:453+` — commit position/cell, then `CommitSetPositionTransition` + +Steps 3-4 are the divergence. Step 8 is the retail mechanism. + +The `if (rm.Host is { } npcHost) / else` fork at `:265`/`:303` duplicates the +sample and the `ComposeOffset` call **verbatim**. There are therefore **two +identical production sites**, not one — any edit must touch both. (This is the +AP-22 shape: a row naming one site where more exist.) + +### 2.2 The sweep is real and the contact plane is real + +- `PhysicsEngine.ResolveWithTransition` (`:1888`) → `FindTransitionalPosition` + (`:2094`) → the step loop (`TransitionTypes.cs:1472`) → `AdjustOffset` + per step (`:1486`). +- The result is **accumulated from the projected offsets** + (`sp.AddOffsetToCheckPos(sp.GlobalOffset)`, `TransitionTypes.cs:1530`) — it + is *not* clamped to `targetPos`. So the sweep genuinely produces a + slope-following Z from a purely horizontal input offset. +- The transition is **seeded** with the body's committed contact plane + (`PhysicsEngine.cs:1984-1997`, register row IA-1), gated on retail's + `check_contact` predicate (`dot(velocity, N) <= 0.0002`). Remotes pass + `body: rm.Body`, so they get the seed. +- The plane is **written back** to the body after every successful resolve + (`PhysicsEngine.cs:2102-2145`), including `body.GroundNormal`. So + `rm.Body.ContactPlane.Normal` at tick start is the real plane the previous + tick's sweep found — terrain, BSP, or object. + +**This is the fact that makes Stage 1 a two-statement change.** The correct +projection surface is already sitting on the body. + +### 2.3 The third `ComposeOffset` call site is not part of AD-10 + +`TickHidden` at `:936-944` calls `ComposeOffset` with the `terrainNormal` +parameter **omitted** (defaults to `null`). It never projects. Leave it alone; +do not "make it consistent." + +### 2.4 History — why the caution in §0 is not ceremonial + +- `9e4772a8` (**2026-05-05**) `fix(motion): project anim root motion onto + terrain plane (slope staircase)` added the mechanism. The commit body + documents a real, measured, user-visible ~5 Hz Z staircase and reasons it + out correctly from the queue-empty fallback returning a Z=0 body-local + delta. +- `93cbabbc` (**2026-04-21**) `fix(physics): full retail per-frame chain for + remote motion + persist ContactPlane across frames` — the sweep **and** the + cross-frame contact-plane persistence were already present two weeks + earlier. + +So the sweep's own `AdjustOffset` was live and plane-seeded on the day the +staircase was observed, and it did not remove it. **I could not establish from +static reading why.** Candidate explanations that have all landed *since*, any +of which could have changed the answer: + +- `204d0ae0` (2026-08-04, Bug B / #32): removed a per-tick forge of + `Contact | OnWalkable` and a per-tick `Body.Velocity = Vector3.Zero`, and + made the tick run the full `CommitSetPositionTransition` sequence instead of + consuming only `Position/CellId/IsOnGround`. +- `2d611b2b` (2026-07-30, #265): replaced the `isOnGround`-driven contact seed + with retail's `check_contact` predicate. +- The 2026-07-30 `body.GroundNormal = ci.ContactPlane.Normal` sync + (`PhysicsEngine.cs:2116-2127`), whose own comment says *"nothing wrote it + from a live resolve before now — `calc_friction` always saw the + `Vector3.UnitZ` default, i.e. every slope behaved like flat ground."* + +That last one is a documented instance of a slope-related value being +silently flat for months. It is precisely the reason Stage 0 must be a +**measurement**, not an argument. + +--- + +## 3. Relationship to #32 (Bug B roof-plant) + +Read `docs/ISSUES.md:10267-10480` in full. + +**Fixing AD-10 does not fix #32, and does not partially fix it. The two are now +disjoint.** Evidence: + +1. #32's remote half is **already closed** — fixed `204d0ae0`, user-passed + 2026-08-04 ("it lands and slides correctly now"). +2. The actual root cause was four forced writes in the remote tick, none of + them AD-10's. The issue text states it directly: *"acdream's classifier was + correct and was being overruled."* +3. The AD-10 projection is now gated on `bodyOnWalkableAtTickStart` + (`:278`, `:321`). A steep roof produces `OnWalkable == false` (contact-plane + `Normal.Z` 0.6097 against `FloorZ` 0.6642, measured live in the #32 + capture), so **on the exact geometry #32 is about, the AD-10 path does not + run at all.** + +**Therefore the AD-10 row's own risk column is now stale on this point.** It +reads: *"a remote landing on a house roof gets no slope response from this +path **regardless of `OnWalkable`**."* After Bug B's fix the clause is +inverted — the path is gated *off* by `OnWalkable`, and the slide comes from +gravity plus the sweep, exactly as retail does it. The same staleness is in +#32's own AD-10 paragraph. + +**What AD-10 *does* still break is the case #32 never covered: a remote on a +WALKABLE surface that is not terrain.** A flat or gentle roof, a bridge, a +dock, a dungeon floor, a ramp inside a building. There `OnWalkable == true`, +the gate opens, and `SampleTerrainNormal` returns the plane of the *ground far +below* — an unrelated surface. That is worse than no projection: it applies a +wrong plane rather than none. **This, not the roof-plant, is the live symptom +AD-10 should be judged on.** + +**Do not promise #32 anything.** Its remaining open items (LeaveGround chatter +bound, the `!Ok` airborne latch, `contact_allows_move`, and local-player +edge-slide) are untouched by this work. + +--- + +## 4. The exact change + +### Stage 0 — measurement (no production change; may be discarded) + +Build the fixture in §7.1 and answer one question: **with the pre-sweep +projection disabled, does the sweep alone track the surface Z?** + +- **If yes** → delete the projection: remove the `terrainNormal` parameter's + two production feeds (`RuntimeRemotePhysicsUpdater.cs:278-282`, `:321-325`), + the projection block in `RemoteMotionCombiner.ComposeOffset:65-73`, and the + now-dead `terrainNormal` parameter. **AD-10 retires**; delete row 122. +- **If no** → record the measured failure mode in the closeout doc (it is a + real finding about the remote sweep either way) and ship Stage 1. + +### Stage 1 — narrowing (the fallback ship) + +**Symbols touched — the complete list.** + +| File | Site | Change | +|---|---|---| +| `src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs` | `:278-282` (host branch) | `SampleTerrainNormal(Body.X, Body.Y)` → `rm.Body.ContactPlaneValid ? rm.Body.ContactPlane.Normal : (Vector3?)null` | +| `src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs` | `:321-325` (no-host branch) | identical | +| `src/AcDream.Core/Physics/RemoteMotionCombiner.cs` | `:48`, `:65-73`, `:91-104` | rename the parameter (`terrainNormal` → `contactPlaneNormal`) and rewrite the doc comment, which currently asserts a terrain sample | +| `docs/architecture/retail-divergence-register.md:122` | AD-10 | rewrite: risk column collapses to the boundary-placement clause; correct the three stale claims in §10 | + +**Keep unchanged, deliberately:** + +- The `bodyOnWalkableAtTickStart` gate. It is what keeps a *wall* normal out of + the projection (`OnWalkable` ⟺ `N.z >= FloorZ`), and it is the Bug B fix. +- The `Normal.Z > 0.01f` guard inside `ComposeOffset`. Redundant under the + `OnWalkable` gate but harmless and defensive. +- The `!interpolationOverwrote` guard. The projection must stay confined to the + queue-empty/head-reached case; an interpolation catch-up is already a 3D + vector toward a server-reported Z and must not be re-projected. +- `TickHidden` (`:936`). +- `RemoteMotionCombiner.ComputeOffset` — see §10.3. Do not modify it as part of + this change; if it is to be deleted, that is its own commit. + +**Why this is strictly safer than it looks.** + +- On outdoor terrain the two sources agree by construction: the committed + plane on flat/rolling ground *is* the terrain triangle plane + (`PhysicsEngine.cs:2113` writes `ci.ContactPlane`, whose terrain producer is + the same `SampleTerrainWalkable` triangle `SampleTerrainNormal` reads). The + staircase-removing behaviour the mechanism exists for is preserved + identically. +- The one-tick lag (committed plane from the previous sweep vs. a + current-XY sample) is **more** retail-faithful, not less: retail's + `adjust_offset` reads the plane the *previous* sub-step established. +- It removes a whole class of failure rather than trading one for another — + there is no scenario where a single-point terrain sample is right and the + body's own committed contact plane is wrong. + +--- + +## 5. Blast radius — both hosts + +### 5.1 The graphical host + +`AcDream.App.Physics.RemotePhysicsUpdater` +(`src/AcDream.App/Physics/RemotePhysicsUpdater.cs:17`) is a **thin adapter**: +it constructs a `RuntimeRemotePhysicsUpdater` at `:46` and forwards +(`:220-249`), supplying DAT shape dimensions and presentation callbacks. It +contains **no duplicated projection**. Per-frame drive is +`AcDream.App.Rendering.LiveEntityAnimationScheduler:25`. + +### 5.2 The headless host — the finding that inverts the C5b lesson + +**`AcDream.Headless` never runs this code path at all.** + +Evidence, exhaustive: + +``` +$ grep -rn "new RuntimeRemotePhysicsUpdater" --include=*.cs src/ tests/ +src/AcDream.App/Physics/RemotePhysicsUpdater.cs:46 <- only production site +tests/AcDream.Runtime.Tests/Physics/RuntimePhysicsStateTests.cs:2884,2938,3017 +tests/AcDream.Runtime.Tests/Physics/RuntimeRemoteSteepContactSlideTests.cs:406 +tests/AcDream.Runtime.Tests/Physics/RuntimeSetPositionStateTests.cs:414 +``` + +`src/AcDream.Headless/` contains no reference to `RemoteMotion`, +`RemotePhysicsUpdater`, or `OrdinaryPhysicsUpdater`. The class is `internal` to +`AcDream.Runtime` and reaches production only through `InternalsVisibleTo` into +`AcDream.App`. + +The C5b lesson was "a survey over one host's call graph missed +`AcDream.Headless` entirely." Here the reasoning that would produce the same +mistake runs the other way: `RemoteMotionCombiner` *is* in `AcDream.Core`, and +`RuntimeRemotePhysicsUpdater` *is* in `AcDream.Runtime`, so the reflex +conclusion "therefore headless runs it" is available and **wrong**. Assembly +placement is not evidence of reachability; the instantiation census is. + +Consequences for this contract: + +1. **No headless gate is required or meaningful for AD-10.** Do not design one; + a passing headless run would be vacuous evidence. +2. Headless *does* publish terrain, cell surfaces, buildings and static objects + into the engine (`HeadlessSessionWorldProjection.cs:430-467` → + `LandblockPhysicsContentBuilder.PublishStaticCollision`), so if remote DR is + ever given to headless, Stage 1's change works there unmodified — the + committed contact plane is available on both hosts. Stage 0's terrain-only + assumption would *not* have been. +3. **#330's relevance is narrow.** Headless registering no live-entity + collision means a headless body could never receive a contact plane from + `CSphere`/`CCylSphere::step_sphere_down` (standing on another creature). + That is a #330 consequence, not an AD-10 one, and it is unreachable today + because headless does not run remote DR at all. + +### 5.3 Adjacent gap observed, not claimed as a defect + +Headless bots appear to have **no remote dead-reckoning whatsoever** — remote +entities would move only at `UpdatePosition` cadence. Whether that matters +depends on what headless bots are for, which this contract does not decide. +**Recommendation: file it as an issue with the §5.2 evidence, tagged as +adjacent to #330, and let the headless owner judge severity.** Do not fold it +into AD-10. + +--- + +## 6. Gate design + +This is remote-movement **feel**; the acceptance test is the user's eyes, and +it batches into the next connected session. Two clients: acdream observing an +acdream-driven `+Acdream`, or acdream observing a retail-driven character. + +**Absence of a symptom is not the criterion.** Each row below names a +*positive* observable. + +| # | Observable | Positive criterion | Why it is here | +|---|---|---|---| +| **G1** | **The ~5 Hz Z staircase** — a remote running across open rolling terrain (Holtburg fields, a hillside) | The remote's feet track the ground **continuously**. Watch specifically *between* server updates, at ~200 ms spacing. Any stepped/ratcheting Z is an immediate fail. | This is the artifact the current mechanism exists to remove. **A regression here is worse than the divergence** and is the single thing that vetoes the change. | +| **G2** | **Slope descent smoothness** — the remote running *downhill* on a moderate grade | The descent reads as a continuous glide with the feet planted; no floating above the surface, no periodic sink-and-pop. Compare uphill on the same slope — they should feel symmetric. | Downhill is the branch §1.4(a) shows acdream already handles differently from retail, and the branch the queue-empty fallback dominates. | +| **G3** | **The surface the divergence is actually about** — a remote walking on a **walkable non-terrain surface**: a bridge, a dock, a gently-pitched roof, a raised platform, a ramp or sloped floor inside a dungeon or building | The remote's feet stay **on that surface** while it moves, including across its slope. It must not sink toward, or drift with, the terrain below. | This is the case §3 identifies as AD-10's live symptom. It is the reason the change exists. | +| **G4** | **Roof / steep-face behaviour** — repeat #32's own scenario: a remote jumps onto a house roof | Unchanged from the 2026-08-04 user-passed behaviour: it lands and slides down. | Guards the closed half of #32. The `OnWalkable` gate should make this a literal no-op; confirm rather than assume. | +| **G5** | **Flat ground** — a remote walking on level terrain, and a remote standing still | Unchanged. Nothing new appears — no drift, no jitter, no Z creep while stationary. | The projection is a no-op on flat ground by construction; a change here means something else moved. | + +**Sequencing:** run G1 before anything else. If G1 fails, stop and revert — +nothing below it matters. + +**Instrumentation:** `ACDREAM_PROBE_RESOLVE=1` gives one `[resolve]` line per +`ResolveWithTransition` with the contact-plane status and the responsible +entity guid, which is enough to correlate a visual observation to a specific +remote. `ACDREAM_PROBE_CELL=1` is low-volume and useful for G3. Both are +runtime-toggleable from the DebugPanel under `ACDREAM_DEVTOOLS=1`. + +**Build:** Release. Debug FPS produces false-regression alarms +(`feedback_debug_vs_release_perf`), and G1 is a cadence observation. + +--- + +## 7. Proof obligations and test plan + +**Standing rule for every test below: it does not count until the named +sabotage has been applied and observed to redden it.** This campaign has +shipped five green tests that covered nothing; the AP-22 review disproved a +coverage claim by sabotage. Assume no discrimination until demonstrated. + +**No source-text pins. No test may re-encode the constant under test** — in +particular, no test may compute its expected Z by re-implementing +`v -= N·dot(v,N)`. Expected values come from the *geometry* (the surface the +body is standing on), so a wrong-plane projection produces a wrong answer +rather than a self-consistent one. + +### 7.0 The existing harness + +`tests/AcDream.Runtime.Tests/Physics/RuntimeRemoteSteepContactSlideTests.cs` +already drives the **production** `RuntimeRemotePhysicsUpdater.Tick` over a +synthetic landblock, with `Harness.OnRamp(gradient)` / +`Harness.Airborne(gradient, height)` and a `Tick(count, dt)` loop +(`:304-460`). `PhysicsEngine.AddLandblock` there accepts terrain, a +`CellSurface[]` and a `PortalPlane[]`. **Build on this harness.** It is the +only existing fixture that produces a *real geometric* contact plane for a +remote rather than a stubbed one, and it already models the fixture-validation +pattern (`SteepTerrainProducesANonWalkableContactPlane`, `:55-64`). + +### 7.1 T1 — Stage 0's decisive measurement: does the sweep alone track Z? + +**Layer:** `AcDream.Runtime.Tests`, the §7.0 harness. +**Fixture:** `Harness.OnRamp(WalkableGradient)` with a non-empty root-motion +frame driving the body along the slope, interpolation queue empty (so the +fallback path runs). +**Body:** tick N frames with the AD-10 sample forced to `null`, and assert the +body's Z stays within a tight band of the terrain surface Z at its own XY +(`TerrainSurface.SampleZ`), monotonically, with no per-tick Z plateau longer +than one tick. +**Sabotage that must redden it:** flatten the ramp gradient to 0 and invert the +assertion — the test must not pass on flat ground for the wrong reason. And: +short-circuit `Transition.AdjustOffset` to `return offset;` — T1 must go red, +proving it is measuring the sweep's projection and nothing else. + +This test *is* Stage 0. Its result selects the ship. + +### 7.2 T2 — the discriminating test: wrong plane vs. right plane + +**This is the load-bearing one.** It must fail on HEAD and pass after Stage 1. + +**Requirement (functional, not prescriptive):** a fixture in which the body +rests on a **walkable surface whose plane differs from the terrain plane at the +same XY**, with the difference established by the *sweep* (i.e. +`Body.ContactPlane.Normal != SampleTerrainNormal(x, y)` at tick start). + +Two candidate constructions, in preference order: + +- **(a) Terrain crest.** Extend `Harness.Ramp` to a two-gradient heightmap (a + ridge). Walk the body across the crest. At the tick after the crossing, the + current-XY sample and the committed plane are from *different triangles*. + Cheap, certainly buildable with the existing harness, and it exercises the + row's "cell boundaries" risk directly. **Mandatory.** +- **(b) Off-terrain walkable surface.** A sloped static/building collision + surface above flat terrain, so the terrain sample is `(0,0,1)` while the + committed plane is the platform. This is the case that matters most (§3) but + needs a collision payload the current harness does not build — note that + `CellSurface` is **not** consulted by `TransitionTypes`/`BSPQuery` (the + former `HasCellSurface` path was deleted in C5a, + `PhysicsEngine.cs:1806`), so it must go through the flat-collision/static + publication path, not `AddLandblock`'s `cells` argument. **Required, with a + documented fallback:** if the fixture cannot be built in reasonable time, say + so explicitly in the closeout, ship on (a), and record (b) as an untested + axis rather than silently dropping it. + +**Assertion:** the body's Z tracks the surface it is *standing on*, derived +from that surface's own geometry — never from a re-implementation of the +projection formula. + +**Fixture validation, mandatory and first:** before any motion assertion, +assert `Body.ContactPlane.Normal` is the expected surface normal **and** that +`SampleTerrainNormal` at the same XY differs from it. A fixture where the two +agree cannot discriminate, and a green test on such a fixture is worthless. + +**Sabotage that must redden it:** revert the sample source to +`SampleTerrainNormal`. T2 must go red. If it does not, the fixture does not +discriminate and the test is void. + +### 7.3 T3 — the production entry point is covered at all + +There is currently **no test of the production mechanism** (§10.6). + +Assert that `RemoteMotionCombiner.ComposeOffset`, on the `!interpolationOverwrote` +path with a sloped normal, produces a body-local delta whose *world* rotation +has the expected Z sign and magnitude; and that on the `interpolationOverwrote` +path it produces **no** projection. + +**Sabotage:** invert the `!interpolationOverwrote` guard. T3 must go red. +(Today, inverting that guard reddens nothing.) + +### 7.4 T4 — the `OnWalkable` gate still closes on a steep face + +Reuse `Harness.OnRamp(SteepGradient)`. Assert no projection is applied while +`Body.OnWalkable` is false, whichever normal source is wired. + +**Sabotage:** remove the `bodyOnWalkableAtTickStart` ternary at `:278`/`:321`. +T4 must go red. This is the Bug B regression guard. + +### 7.5 T5 — both fork branches + +`RuntimeRemotePhysicsUpdater` has **two** identical sites (`:278` host branch, +`:321` no-host branch). At least one test must exercise the **no-host** +(pre-`PositionManager`-binding) branch. + +**Sabotage:** apply the fix to only *one* branch. T5 must go red. + +This is the AP-22 lesson made a test: a row that names one site where several +exist produces a fix that lands on one site. + +### 7.6 Suite obligations + +- Full `AcDream.Runtime.Tests`, `AcDream.Core.Tests`, `AcDream.App.Tests` + green. +- Complete solution suite green. +- No headless gate (§5.2) — and say so in the closeout, so its absence reads as + a decision rather than an omission. + +--- + +## 8. Verification hygiene — mandatory + +**This worktree's incremental build has twice served a stale DLL containing +deleted code, including under `--no-incremental` and `-t:Rebuild`.** One +reviewer had to delete all 44 `bin`/`obj` directories to get a truthful result. + +Any **verdict-deciding** test result — T1's Stage-0 answer, and every sabotage +observation in §7 — must come from a genuinely clean build: + +```powershell +Get-ChildItem -Recurse -Directory -Include bin,obj | Remove-Item -Recurse -Force +dotnet build -c Release +``` + +A sabotage that "did not redden the test" is **not** evidence until it has been +re-run from a clean tree. The failure mode this guards against is exactly the +one that makes a false coverage claim look verified. + +--- + +## 9. Traps, size, split + +### Traps + +1. **Do not touch `Transition.AdjustOffset`.** Its two unregistered divergences + (§1.4) are real, one of them plausibly feeds #269, and both change + *local-player* movement feel. Fixing them inside an AD-10 commit puts a + local-player regression behind a remote-movement gate. Separate commits, + separate gates. +2. **Do not "unify" `TickHidden`.** It deliberately does not project (§2.3). +3. **Do not fix only one fork branch** (§7.5). +4. **Do not use `Body.GroundNormal` as the source.** It aliases + `ContactPlane.Normal` on the success path but has a documented + stale-retention branch (`PhysicsEngine.cs:2137-2145`). Read + `ContactPlaneValid` + `ContactPlane.Normal` and treat invalid as `null`. +5. **Do not remove the `!interpolationOverwrote` guard.** Re-projecting an + interpolation catch-up would fight the server's own Z. +6. **Do not promise #32 anything** (§3). +7. **Do not design a headless gate** (§5.2). +8. **Do not let the double projection go unexamined if Stage 1 ships.** After + Stage 1, the offset is projected once at the boundary and again per-step + inside the sweep, both against the same plane. That composition is + *idempotent* (a vector already on the plane has `dot(v,N)==0`) — but say so + explicitly in the closeout, with the arithmetic, rather than leaving it as + an unexamined assumption. It is the reason Stage 0 exists. +9. **`SampleTerrainNormal` is XY-only and Z-blind.** It will happily return a + normal for a body inside a dungeon, on a tower, or under a bridge, provided + the landblock is resident. There is no "no terrain here" case to rely on. + +### Size + +- **Stage 0:** ~1 test + ~40 lines of harness extension. Half a session, + including the clean-build discipline. +- **Stage 1 (if needed):** ~2 production statements, ~1 parameter rename, ~4 + doc comments, 1 register-row rewrite, 4-5 tests. One session. +- **The §1.4 register rows:** ~30 minutes, separate commit, no code. +- **The §5.3 headless issue:** ~15 minutes, separate commit, no code. + +### Split call + +**Do not split across agents.** Total production surface is a handful of +statements; the expensive part is the fixture work in T2, which is a single +coupled piece of reasoning about one harness. Splitting it would reproduce the +"coupled plan slices given to parallel agents" failure +(`feedback_dont_parallelize_coupled_plan_slices`). + +**Commit sequence:** + +1. `test(physics): measure whether the remote sweep alone tracks surface Z (AD-10 Stage 0)` — T1 only, no production change. +2. Either `refactor(physics): delete the redundant pre-sweep slope projection (AD-10 retired)` **or** `fix(physics): project remote root motion onto the body's committed contact plane (AD-10 narrowed)` — with T2-T5 and the register edit **in the same commit** (same-commit row discipline). +3. `docs(register): file the AdjustOffset snap_to_plane and safety-threshold divergences` — §1.4, separate. +4. `docs: file the headless remote dead-reckoning gap` — §5.3, separate. + +Commits 1-2 are user-gated on §6 before anything downstream builds on them. + +--- + +## 10. Claims found false or stale at HEAD + +Numbered, each with its evidence. + +**10.1 — AD-10's justification column is false.** +The row says *"Remote bodies don't run a full local transition sweep."* They +do: `RuntimeRemotePhysicsUpdater.cs:399` calls +`_physics.Engine.ResolveWithTransition(...)` with the remote's own body, +Setup-derived sphere list, step heights and mover flags, and +`TickHidden:982` does the same. This is the premise the entire "cannot move it +into the sweep" reasoning rests on, and it is the one that reframes the row +from *relocation* to *addition*. + +**10.2 — The row describes its one live site backwards.** +It cites *"`ComposeOffset` ~:65-72 **for the interpolation-active** boundary +projection."* The guard at `RemoteMotionCombiner.cs:65` is +`if (!interpolationOverwrote && ...)` — the projection runs **only when +interpolation did *not* overwrite**, i.e. the queue-empty/head-reached case. +The row has the two mutually exclusive cases swapped. + +**10.3 — The row's other cited site is dead code.** +*"the queue-empty fallback ~:163-168"* is inside +`RemoteMotionCombiner.ComputeOffset` (`:105-170`). `ComputeOffset` has **zero +production callers** — `grep -rn "ComputeOffset" --include=*.cs src/` returns +only its own definition (`:105`) and its internal call to `ComposeOffset` +(`:143`, which passes `terrainNormal: null`). Its only callers are in +`tests/AcDream.Core.Tests/Physics/`. So the row cites one live site described +backwards and one correctly-described site that cannot execute in production. + +**10.4 — The row's risk column is stale on the roof clause.** +It reads *"a remote landing on a house roof gets no slope response from this +path **regardless of `OnWalkable`**."* Since Bug B (`204d0ae0`, 2026-08-04) the +sample is gated on `bodyOnWalkableAtTickStart` +(`RuntimeRemotePhysicsUpdater.cs:278`, `:321`), and a steep roof is +`OnWalkable == false` by measurement (contact-plane `Normal.Z` 0.6097 vs. +`FloorZ` 0.6642, from #32's live capture). The path is now gated *off* on +exactly that geometry. The same stale clause is repeated verbatim in +`docs/ISSUES.md` #32's AD-10 paragraph. + +**10.5 — The row's retail anchor range truncates the function.** +`pc:272296-272346` omits the sliding-normal validity gate at the head +(272276-272296) and the entire post-projection safety push-out block plus the +sliding-normal-only tail (272355-272400). Both omitted regions are part of what +"retail projects inside `adjust_offset`" means. Correct anchor: +`CTransition::adjust_offset` **0x0050a370**, pc:272271-272400. + +**10.6 — The mechanism's only test tests a method production never calls.** +`RemoteMotionCombinerTests.ComputeOffset_RootMotionFallback_SlopedTerrainNormal_ProjectsZOntoSlope` +(`tests/AcDream.Core.Tests/Physics/RemoteMotionCombinerTests.cs:198-225`) is +labelled *"Lock-the-fix for the 'remote running on a slope shows ~5 Hz Z +staircase' bug"*. It exercises `ComputeOffset`, which is dead in production +(10.3). It also hard-codes the expected result by re-implementing the +projection formula in a comment and asserting the arithmetic — it cannot detect +a wrong *plane*, only a wrong *multiply*. **The production path +(`ComposeOffset` with a non-null normal) has no test at all.** This is the +sixth green-test-covering-nothing in this campaign. + +**10.7 — Two divergences exist in acdream's `CTransition::adjust_offset` port +with no register row.** +(a) The `collisionAngle > 0` branch substitutes `v -= N·dot(v,N)` for retail's +`Plane::snap_to_plane`, making the two arms of the `if/else` identical and +shortening downhill XY travel by `cos²θ` relative to retail +(`TransitionTypes.cs:5252-5258` vs. 0x0050a50e → 0x00509c50). +(b) The safety push-out substitutes `radius * N.z` for retail's bare `radius` +in both the trigger and the numerator (`TransitionTypes.cs:5285-5309` vs. +0x0050a5c4-0x0050a5ff), knowingly and with a written rationale, but with no +row. Grep confirms: no register row mentions `snap_to_plane`, `SnapToPlane`, +`naturalResting`, or `away-plane`. + +**10.8 — The reflex blast-radius inference is wrong here, in the opposite +direction from C5b.** +`RemoteMotionCombiner` is in `AcDream.Core` and `RuntimeRemotePhysicsUpdater` +is in `AcDream.Runtime`, so "headless runs it too" is the available +conclusion. It is false: the only production instantiation of +`RuntimeRemotePhysicsUpdater` is `src/AcDream.App/Physics/RemotePhysicsUpdater.cs:46`, +and `src/AcDream.Headless/` never names it. Assembly placement is not +reachability. + +--- + +## 11. What I could not establish + +Flagged rather than guessed. + +1. **Why the sweep's own `AdjustOffset` did not remove the 2026-05-05 + staircase**, given that both the sweep and cross-frame contact-plane + persistence landed on 2026-04-21 (`93cbabbc`), two weeks earlier. Three + subsequent changes (§2.4) could each have altered the answer. **This is + exactly what Stage 0 measures**; do not proceed on either an assumption of + redundancy or an assumption of necessity. +2. **Whether a body in a dungeon EnvCell gets a non-null `SampleTerrainNormal` + in practice.** The function is XY-only and Z-blind, so it *will* return a + normal whenever a landblock covering that XY is resident; whether pure + dungeon landblocks stream terrain into `PhysicsEngine._landblocks` at all + was not verified. This affects how bad the indoor case currently is, not + whether the fix is correct. Determinable with `ACDREAM_PROBE_RESOLVE=1` in a + dungeon. +3. **Whether §7.2(b)'s off-terrain fixture can be built against the current + flat-collision publication API** in reasonable time. `CellSurface` is + confirmed *not* to be the route (`TransitionTypes`/`BSPQuery` do not consult + it; the `HasCellSurface` path was deleted in C5a). The static/building + publication path was not traced end to end. §7.2 carries an explicit + fallback for this. +4. **Whether §1.4(a) is a live contributor to #269.** The arithmetic is + confirmed and the direction (downhill-only, XY-shortening) is suggestive, + but #269's own note says the friction and jump chains were byte-verified + identical and it needs a live cdb A/B. Handed over as a lead, not a + diagnosis. diff --git a/src/AcDream.Core/Physics/PhysicsEngine.cs b/src/AcDream.Core/Physics/PhysicsEngine.cs index 2f5c47df..74f33484 100644 --- a/src/AcDream.Core/Physics/PhysicsEngine.cs +++ b/src/AcDream.Core/Physics/PhysicsEngine.cs @@ -1007,20 +1007,14 @@ public sealed class PhysicsEngine return null; } - /// - /// Public surface for callers that only need the local terrain plane - /// normal at a world-space XY (e.g., the grounded-remote tick path - /// projecting anim root motion onto the slope to avoid the staircase - /// between server position updates). Returns null when no registered - /// landblock covers the point. Mirrors the plane component of - /// without exposing the internal - /// TerrainWalkableSample shape. - /// - public Vector3? SampleTerrainNormal(float worldX, float worldY) - { - var sample = SampleTerrainWalkable(worldX, worldY); - return sample?.Plane.Normal; - } + // AD-10 (retired 2026-08-06): SampleTerrainNormal(worldX, worldY) lived + // here. Its only caller was the remote tick's pre-sweep slope projection, + // which was itself an extra copy of retail's in-sweep + // CTransition::adjust_offset (0x0050a370). The lookup was XY-only and + // Z-blind, so it answered with terrain even for a body on a bridge, in a + // dungeon or on a roof. Nothing needs a bare terrain normal now; callers + // that need a contact surface read the body's own committed ContactPlane, + // which the resolve below publishes. /// /// Sample the outdoor terrain walkable triangle at the given world-space diff --git a/src/AcDream.Core/Physics/RemoteMotionCombiner.cs b/src/AcDream.Core/Physics/RemoteMotionCombiner.cs index 28a4bc70..d80cd29b 100644 --- a/src/AcDream.Core/Physics/RemoteMotionCombiner.cs +++ b/src/AcDream.Core/Physics/RemoteMotionCombiner.cs @@ -36,6 +36,20 @@ public sealed class RemoteMotionCombiner /// active, replaces the PartArray frame via /// Position::subtract2; otherwise the authored root frame remains. /// + /// + /// AD-10, retired 2026-08-06. This method used to accept a + /// terrainNormal and project the composed world-space root motion + /// onto it whenever interpolation did not overwrite. That was an EXTRA + /// projection: retail projects the per-sub-step offset onto + /// collision_info.contact_plane INSIDE the sweep + /// (CTransition::adjust_offset 0x0050a370, + /// pc:272271-272393), acdream ports that verbatim in + /// Transition.AdjustOffset, and remote bodies do run that sweep. + /// The extra copy also sampled the wrong surface — a single-point + /// XY-only terrain lookup, blind to buildings, EnvCells and statics — so + /// on a walkable NON-terrain surface it applied the plane of the ground + /// far below. Removing it left the measured trajectory unchanged. + /// /// true when interpolation replaced the root frame. public bool ComposeOffset( double dt, @@ -45,7 +59,6 @@ public sealed class RemoteMotionCombiner InterpolationManager interp, float maxSpeed, MotionDeltaFrame output, - Vector3? terrainNormal = null, bool inContact = true) { ArgumentNullException.ThrowIfNull(rootMotionLocalFrame); @@ -62,16 +75,6 @@ public sealed class RemoteMotionCombiner output, inContact); - if (!interpolationOverwrote - && terrainNormal.HasValue - && terrainNormal.Value.Z > 0.01f) - { - Vector3 rootMotionWorld = Vector3.Transform(output.Origin, ori); - Vector3 normal = terrainNormal.Value; - rootMotionWorld -= normal * Vector3.Dot(rootMotionWorld, normal); - output.Origin = MoveToMath.GlobalToLocalVec(ori, rootMotionWorld); - } - return interpolationOverwrote; } @@ -88,28 +91,13 @@ public sealed class RemoteMotionCombiner /// Body orientation; used to rotate root motion from body-local to world. /// The remote's InterpolationManager (for AdjustOffset call). /// From MotionInterpreter.GetMaxSpeed() — passed to AdjustOffset for the catch-up clamp. - /// - /// Optional local terrain plane normal at the body's current XY. When - /// supplied AND the queue-empty / head-reached fallback path runs, the - /// world-space anim root motion is projected onto the plane so XY motion - /// produces a corresponding Z change on slopes. Without this, the - /// fallback advances XY at the locomotion cycle's pace but leaves Z at - /// the last UP's reported Z — visible as a ~5 Hz staircase on slopes - /// (the rate of server UpdatePositions). Mirrors retail's - /// CTransition::adjust_offset contact-plane projection - /// (named-retail acclient_2013_pseudo_c.txt:272296-272346) for grounded - /// motion, applied here at the queue-empty boundary instead of inside - /// the sweep. Pass null on flat ground / when no terrain sample - /// is available — projection is a no-op when normal == +Z. - /// public Vector3 ComputeOffset( double dt, Vector3 currentBodyPosition, Vector3 rootMotionLocalDelta, Quaternion ori, InterpolationManager interp, - float maxSpeed, - Vector3? terrainNormal = null) + float maxSpeed) { // Retail-faithful per-frame combiner. Mirrors // CPhysicsObj::UpdatePositionInternal (acclient @ 0x00512c30) + @@ -147,25 +135,9 @@ public sealed class RemoteMotionCombiner root, interp, maxSpeed, - output, - terrainNormal: null); - Vector3 rootMotionWorld = Vector3.Transform(output.Origin, ori); - - // Slope projection (queue-empty fallback only). Locomotion cycles - // bake Z=0 in body-local, so without projection the body's Z stays - // at the last UP's reported value while XY advances at the running - // pace — visible ~5 Hz staircase between UPs on hills. Projecting - // the world-space anim motion onto the local terrain plane gives - // it a Z component proportional to slope × forward speed, so the - // body follows the terrain mesh smoothly. No-op on flat ground - // (normal ≈ +Z, dot ≈ 0) so it can't regress the M2 flat-ground - // verification. - if (terrainNormal.HasValue && terrainNormal.Value.Z > 0.01f) - { - Vector3 N = terrainNormal.Value; - float into = Vector3.Dot(rootMotionWorld, N); - rootMotionWorld -= N * into; - } - return rootMotionWorld; + output); + // AD-10 (retired 2026-08-06): a second copy of the deleted terrain + // projection used to run here. See ComposeOffset's summary. + return Vector3.Transform(output.Origin, ori); } } diff --git a/src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs b/src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs index b294db3a..394924ab 100644 --- a/src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs +++ b/src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs @@ -269,17 +269,20 @@ internal sealed class RuntimeRemotePhysicsUpdater pmDelta.Origin = scaledRootMotionLocalOrigin; pmDelta.Orientation = rootMotionLocalFrame.Orientation; float maxSpeedNpc = rm.Motion.GetAdjustedMaxSpeed(); - // AD-10 terrain-only slope projection. Bug B (2026-08-04): - // gated on the committed ON_WALKABLE transient, the same fact - // retail root-frame scaling reads (0x00512CA1), instead of the - // client Airborne bool. A body resting on a NON-walkable steep - // contact must not have its root motion projected onto a - // terrain plane it is not standing on. - System.Numerics.Vector3? terrainNormalNpc = bodyOnWalkableAtTickStart - ? _physics.Engine.SampleTerrainNormal( - rm.Body.Position.X, - rm.Body.Position.Y) - : null; + // AD-10 (retired 2026-08-06): a terrain-only slope projection + // used to run here, ahead of the sweep. It was an EXTRA copy of + // retail's own per-sub-step projection + // (CTransition::adjust_offset 0x0050a370, pc:272271-272393, + // ported verbatim in Transition.AdjustOffset and reached by the + // ResolveWithTransition call below), taken against a + // single-point SampleTerrainNormal(x, y) lookup blind to the + // body's Z, its cell, buildings, EnvCells and statics. Retail + // has no such pre-sweep step. Measured redundant 2026-08-06: + // with it removed the production trajectory down a 31-degree + // ramp is bit-identical and the whole Runtime suite is + // unchanged. Both fork branches carried this block verbatim + // (the AP-22 shape); removing the parameter from ComposeOffset + // makes a one-site-only regression fail to compile. rm.Position.ComposeOffset( dt, rm.Body.Position, @@ -288,7 +291,6 @@ internal sealed class RuntimeRemotePhysicsUpdater rm.Interp, maxSpeedNpc, pmDelta, - terrainNormalNpc, inContact: rm.Body.InContact); npcHost.PositionManager.AdjustOffset(pmDelta, dt); // #167 (Campaign P P5): push the read side of TS-35's @@ -312,17 +314,20 @@ internal sealed class RuntimeRemotePhysicsUpdater pmDelta.Origin = scaledRootMotionLocalOrigin; pmDelta.Orientation = rootMotionLocalFrame.Orientation; float maxSpeedNpc = rm.Motion.GetAdjustedMaxSpeed(); - // AD-10 terrain-only slope projection. Bug B (2026-08-04): - // gated on the committed ON_WALKABLE transient, the same fact - // retail root-frame scaling reads (0x00512CA1), instead of the - // client Airborne bool. A body resting on a NON-walkable steep - // contact must not have its root motion projected onto a - // terrain plane it is not standing on. - System.Numerics.Vector3? terrainNormalNpc = bodyOnWalkableAtTickStart - ? _physics.Engine.SampleTerrainNormal( - rm.Body.Position.X, - rm.Body.Position.Y) - : null; + // AD-10 (retired 2026-08-06): a terrain-only slope projection + // used to run here, ahead of the sweep. It was an EXTRA copy of + // retail's own per-sub-step projection + // (CTransition::adjust_offset 0x0050a370, pc:272271-272393, + // ported verbatim in Transition.AdjustOffset and reached by the + // ResolveWithTransition call below), taken against a + // single-point SampleTerrainNormal(x, y) lookup blind to the + // body's Z, its cell, buildings, EnvCells and statics. Retail + // has no such pre-sweep step. Measured redundant 2026-08-06: + // with it removed the production trajectory down a 31-degree + // ramp is bit-identical and the whole Runtime suite is + // unchanged. Both fork branches carried this block verbatim + // (the AP-22 shape); removing the parameter from ComposeOffset + // makes a one-site-only regression fail to compile. rm.Position.ComposeOffset( dt, rm.Body.Position, @@ -331,7 +336,6 @@ internal sealed class RuntimeRemotePhysicsUpdater rm.Interp, maxSpeedNpc, pmDelta, - terrainNormalNpc, inContact: rm.Body.InContact); ApplyPositionManagerDelta(rm.Body, pmDelta); } diff --git a/tests/AcDream.Core.Tests/Physics/RemoteMotionCombinerTests.cs b/tests/AcDream.Core.Tests/Physics/RemoteMotionCombinerTests.cs index 3fb90ad1..a3b9c60b 100644 --- a/tests/AcDream.Core.Tests/Physics/RemoteMotionCombinerTests.cs +++ b/tests/AcDream.Core.Tests/Physics/RemoteMotionCombinerTests.cs @@ -186,65 +186,19 @@ public sealed class RemoteMotionCombinerTests } // ========================================================================= - // Test 7: slope projection — anim root motion gains Z proportional to slope - // - // Lock-the-fix for the "remote running on a slope shows ~5 Hz Z staircase" - // bug: the queue-empty fallback was returning a flat (Z=0) world motion - // because animation cycles bake Z=0 in body-local. Projecting onto the - // local terrain plane gives the motion a Z component matching slope angle - // × forward speed. + // AD-10 retired 2026-08-06. Two tests lived here: + // ComputeOffset_RootMotionFallback_SlopedTerrainNormal_ProjectsZOntoSlope + // ComputeOffset_RootMotionFallback_FlatTerrainNormal_NoZChange + // They exercised the pre-sweep terrain projection that has been deleted, so + // they are gone with it. Both were also weak on their own terms: they drove + // ComputeOffset, which has no production callers, and computed their + // expected values by re-implementing the projection formula in a comment — + // so they could detect a wrong MULTIPLY but never a wrong PLANE, which is + // what the divergence actually was. The surviving coverage of the same + // behaviour is geometric and runs the production tick: + // AcDream.Runtime.Tests.Physics.RuntimeRemoteSlopeProjectionTests. // ========================================================================= - [Fact] - public void ComputeOffset_RootMotionFallback_SlopedTerrainNormal_ProjectsZOntoSlope() - { - var pm = Make(); - var interp = EmptyInterp(); // queue empty → fallback path runs - - // Slope tilted 30° eastward (+X is downhill). Plane normal points - // up-and-east-of-vertical: (sin 30°, 0, cos 30°) = (0.5, 0, 0.866). - Vector3 N = Vector3.Normalize(new Vector3(0.5f, 0f, MathF.Sqrt(3f) / 2f)); - - // Body running due east at 4 m/s, dt = 1s → rootMotionWorld initially - // (4, 0, 0). After projection onto the plane: - // into = dot((4,0,0), (0.5,0,0.866)) = 2.0 - // result = (4,0,0) - (0.5,0,0.866) * 2.0 = (3.0, 0, -1.732) - // i.e. body moves east AND descends ~1.73m for the second. - Vector3 offset = pm.ComputeOffset( - dt: 1.0, - currentBodyPosition: Vector3.Zero, - rootMotionLocalDelta: new Vector3(4f, 0f, 0f), - ori: Quaternion.Identity, - interp: interp, - maxSpeed: 0f, - terrainNormal: N); - - Assert.Equal( 3.000f, offset.X, precision: 3); - Assert.Equal( 0.000f, offset.Y, precision: 3); - Assert.Equal(-1.732f, offset.Z, precision: 3); - } - - [Fact] - public void ComputeOffset_RootMotionFallback_FlatTerrainNormal_NoZChange() - { - var pm = Make(); - var interp = EmptyInterp(); - - // Flat ground: normal = +Z. Projection should be a no-op. - Vector3 offset = pm.ComputeOffset( - dt: 0.1, - currentBodyPosition: Vector3.Zero, - rootMotionLocalDelta: new Vector3(0f, 0.4f, 0f), - ori: Quaternion.Identity, - interp: interp, - maxSpeed: 0f, - terrainNormal: Vector3.UnitZ); - - Assert.Equal(0f, offset.X, precision: 4); - Assert.Equal(0.4f, offset.Y, precision: 4); - Assert.Equal(0f, offset.Z, precision: 4); - } - [Fact] public void ComputeOffset_QueueHeadReached_WithLiteralZeroRootMotion_DoesNotOvershoot() {