refactor(physics): delete the redundant pre-sweep slope projection (AD-10 retired)
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 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
fe6ee877d1
commit
886333a2a9
7 changed files with 885 additions and 145 deletions
|
|
@ -1007,20 +1007,14 @@ public sealed class PhysicsEngine
|
|||
return null;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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
|
||||
/// <see cref="SampleTerrainWalkable"/> without exposing the internal
|
||||
/// <c>TerrainWalkableSample</c> shape.
|
||||
/// </summary>
|
||||
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.
|
||||
|
||||
/// <summary>
|
||||
/// Sample the outdoor terrain walkable triangle at the given world-space
|
||||
|
|
|
|||
|
|
@ -36,6 +36,20 @@ public sealed class RemoteMotionCombiner
|
|||
/// active, replaces the PartArray frame via
|
||||
/// <c>Position::subtract2</c>; otherwise the authored root frame remains.
|
||||
/// </summary>
|
||||
///
|
||||
/// <para><b>AD-10, retired 2026-08-06.</b> This method used to accept a
|
||||
/// <c>terrainNormal</c> 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
|
||||
/// <c>collision_info.contact_plane</c> INSIDE the sweep
|
||||
/// (<c>CTransition::adjust_offset</c> <c>0x0050a370</c>,
|
||||
/// pc:272271-272393), acdream ports that verbatim in
|
||||
/// <c>Transition.AdjustOffset</c>, 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.</para>
|
||||
/// </summary>
|
||||
/// <returns><c>true</c> when interpolation replaced the root frame.</returns>
|
||||
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
|
|||
/// <param name="ori">Body orientation; used to rotate root motion from body-local to world.</param>
|
||||
/// <param name="interp">The remote's InterpolationManager (for AdjustOffset call).</param>
|
||||
/// <param name="maxSpeed">From <c>MotionInterpreter.GetMaxSpeed()</c> — passed to AdjustOffset for the catch-up clamp.</param>
|
||||
/// <param name="terrainNormal">
|
||||
/// 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
|
||||
/// <c>CTransition::adjust_offset</c> 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 <c>null</c> on flat ground / when no terrain sample
|
||||
/// is available — projection is a no-op when normal == +Z.
|
||||
/// </param>
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue