fix(physics): #307 — PreviousTeleport was always 0 on the live Position path
Shipped defect in route 2 (9966b531), found while reviewing route 4a.
`InboundPhysicsStateController.TryApplyPosition` built its AcceptedPhysicsTimestamps
via `Current(gate, teleportAdvanced: ...)`, omitting `previousTeleport`, which
defaulted to a literal 0. The only site that populated it was the deferred
initial-create path — which is why the continuation executor was correct and
every newer consumer was not.
Consequence in shipped code: route 2 feeds this into
`ValidAcceptedAuthority`, which requires Previous == Accepted for a
ForcePosition. Any local player whose TELEPORT_TS is nonzero — anyone who has
portalled or recalled this session — had the authority rejected and the force
correction SILENTLY DROPPED. The user's @pklite acceptance was genuine but
narrow: that character had not teleported, so the stamp was still 0.
Second latent consequence: with an accepted stamp >= 0x8000, wrap-safe
TeleportRegressed also fires against the 0 and rejects ordinary Apply positions,
not just ForcePosition.
The fix captures `previousTeleport = gate.TeleportTimestamp` BEFORE
`TryAcceptPositionEvent` mutates it, matching the shape the deferred path
already used. Ordering is the whole point: capturing after would make
Previous == Accepted unconditionally, so ValidAcceptedAuthority's check would
pass vacuously — the symptom would disappear while the semantics broke.
Also removes the footgun that allowed it. `Current`'s parameter is now
`ushort? previousTeleport = null` resolving to `gate.TeleportTimestamp`, so the
eleven non-Position channels — none of which can move TELEPORT_TS — get
"previous == current" by omission rather than a literal 0 that is
indistinguishable from a genuine "never teleported".
Consumer audit: only TryApplyPosition was defective. The two route-2 call sites
trace back to it; the RuntimeEntityObjectLifetime sites source from
TryAcceptDeferredPosition and were already correct.
Tests discrimination-verified by reverting the argument to 0: the stamp test
fails Expected 10 / Actual 0, and the classifier test fails Expected
SetPositionSimple / Actual RejectedAuthority — the shipped defect reproduced
exactly.
Gates: complete Release solution 10,935 passed / 4 skipped / 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
b633b10967
commit
19d9509497
3 changed files with 206 additions and 4 deletions
|
|
@ -763,6 +763,50 @@ it. Do #297 FIRST — #298 depends on it.
|
||||||
`data_7dXXXX` symbol) and
|
`data_7dXXXX` symbol) and
|
||||||
`tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs`.
|
`tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs`.
|
||||||
|
|
||||||
|
## C4 accepted-position authority — 2026-08-03
|
||||||
|
|
||||||
|
- **#307 — DONE (2026-08-03) — `AcceptedPhysicsTimestamps.PreviousTeleport` was
|
||||||
|
always 0 on the live Position path, silently dropping every local-player
|
||||||
|
ForcePosition correction after the character's first teleport.**
|
||||||
|
`InboundPhysicsStateController.TryApplyPosition` called its private
|
||||||
|
`Current(gate, teleportAdvanced: …)` helper without the `previousTeleport`
|
||||||
|
argument, which defaulted to a literal `0`. The only site that populated it
|
||||||
|
was the deferred initial-create path `TryAcceptDeferredPosition`, which is
|
||||||
|
why `RuntimeInitialCreateContinuationExecutor` was correct and every newer
|
||||||
|
consumer was not.
|
||||||
|
|
||||||
|
**Live blast radius.** Shipped in C4 route 2 (`9966b531`).
|
||||||
|
`LiveEntityNetworkUpdateController.cs` and
|
||||||
|
`RuntimeLiveEntitySessionController.cs` feed the value into
|
||||||
|
`RuntimeAcceptedPositionDriveController.TryExecuteAcceptedLocalPosition`,
|
||||||
|
where `RuntimeAuthoritativePositionRouteClassifier.ValidAcceptedAuthority`
|
||||||
|
requires `PreviousTeleportSequence == AcceptedTeleportSequence` for a
|
||||||
|
`ForcePosition` disposition — which is exactly what retail's FORCE_POSITION
|
||||||
|
branch guarantees (`SmartBox::HandleReceivedPosition` @0x00453FD0 fires only
|
||||||
|
when the packet's teleport stamp equals the live one, and never advances it).
|
||||||
|
With `Previous` pinned to 0, any player whose TELEPORT_TS had advanced — i.e.
|
||||||
|
anyone who had portalled or recalled that session — had the authority
|
||||||
|
rejected and the server's force correction dropped. Route 2's user
|
||||||
|
acceptance was genuine but narrow: the acceptance character had never
|
||||||
|
teleported, so the stamp was still 0. A second latent consequence: with
|
||||||
|
an accepted stamp ≥ 0x8000 the wrap-safe `TeleportRegressed` check would
|
||||||
|
also have fired against the 0, rejecting ordinary `Apply` positions too.
|
||||||
|
|
||||||
|
**Fix.** Capture `previousTeleport = gate.TeleportTimestamp` BEFORE
|
||||||
|
`TryAcceptPositionEvent` mutates it (the exact shape
|
||||||
|
`TryAcceptDeferredPosition` already used) and pass it through. The zero
|
||||||
|
default on `Current` is removed outright — the parameter is now `ushort?`
|
||||||
|
defaulting to the gate's own live stamp, so the channels that cannot move
|
||||||
|
TELEPORT_TS get "previous == current" by omission instead of a silent 0 that
|
||||||
|
is indistinguishable from a real "never teleported".
|
||||||
|
|
||||||
|
Regression tests:
|
||||||
|
`InboundPhysicsStateControllerTests.TryApplyPosition_ReportsThePreEventTeleportStamp`
|
||||||
|
and
|
||||||
|
`…LocalPlayerForcePositionAfterATeleport_ClassifiesAsAnAcceptedForceCorrection`
|
||||||
|
(the second drives the real classifier and fails with `RejectedAuthority`
|
||||||
|
against the pre-fix behaviour).
|
||||||
|
|
||||||
## C3c placement cutover — 2026-08-02
|
## C3c placement cutover — 2026-08-02
|
||||||
|
|
||||||
- **#276 — OPEN — SpawnPlacementSettler discards the settle's resolved
|
- **#276 — OPEN — SpawnPlacementSettler discards the settle's resolved
|
||||||
|
|
|
||||||
|
|
@ -624,8 +624,16 @@ public sealed class InboundPhysicsStateController
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// #307: TryAcceptPositionEvent MUTATES gate.TeleportTimestamp, so the
|
||||||
|
// pre-event value must be captured BEFORE the call - the exact shape
|
||||||
|
// TryAcceptDeferredPosition already used. Reading it afterwards would
|
||||||
|
// yield the accepted value and make Previous == Accepted vacuously
|
||||||
|
// true; omitting it (the original defect) left it 0, which made every
|
||||||
|
// downstream ValidAcceptedAuthority ForcePosition check fail for any
|
||||||
|
// entity whose TELEPORT_TS had ever advanced.
|
||||||
|
ushort previousTeleport = gate.TeleportTimestamp;
|
||||||
bool advancesTeleport = PhysicsTimestampGate.IsNewer(
|
bool advancesTeleport = PhysicsTimestampGate.IsNewer(
|
||||||
gate.TeleportTimestamp,
|
previousTeleport,
|
||||||
update.TeleportSequence);
|
update.TeleportSequence);
|
||||||
disposition = gate.TryAcceptPositionEvent(
|
disposition = gate.TryAcceptPositionEvent(
|
||||||
update.InstanceSequence,
|
update.InstanceSequence,
|
||||||
|
|
@ -636,7 +644,8 @@ public sealed class InboundPhysicsStateController
|
||||||
timestamps = Current(
|
timestamps = Current(
|
||||||
gate,
|
gate,
|
||||||
teleportAdvanced: disposition is PositionTimestampDisposition.Apply
|
teleportAdvanced: disposition is PositionTimestampDisposition.Apply
|
||||||
&& advancesTeleport);
|
&& advancesTeleport,
|
||||||
|
previousTeleport: previousTeleport);
|
||||||
accepted = ApplyAcceptedPosition(
|
accepted = ApplyAcceptedPosition(
|
||||||
old,
|
old,
|
||||||
update,
|
update,
|
||||||
|
|
@ -1113,17 +1122,28 @@ public sealed class InboundPhysicsStateController
|
||||||
0,
|
0,
|
||||||
spawn.InstanceSequence);
|
spawn.InstanceSequence);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// #307: <paramref name="previousTeleport"/> is the TELEPORT_TS value the
|
||||||
|
/// gate held BEFORE the event this call is stamping. Only the two Position
|
||||||
|
/// entry points can move that channel, so they pass their own pre-event
|
||||||
|
/// capture; every other channel leaves it untouched and therefore gets
|
||||||
|
/// "previous == current" by omission. The parameter deliberately has NO
|
||||||
|
/// zero default: a literal 0 is a legal, common TELEPORT_TS, so a silent
|
||||||
|
/// zero is indistinguishable from a genuine "never teleported" and reads
|
||||||
|
/// as a spurious teleport regression to
|
||||||
|
/// <c>RuntimeAuthoritativePositionRouteClassifier</c>.
|
||||||
|
/// </summary>
|
||||||
private static AcceptedPhysicsTimestamps Current(
|
private static AcceptedPhysicsTimestamps Current(
|
||||||
PhysicsTimestampGate gate,
|
PhysicsTimestampGate gate,
|
||||||
bool teleportAdvanced = false,
|
bool teleportAdvanced = false,
|
||||||
ushort previousTeleport = 0) => new(
|
ushort? previousTeleport = null) => new(
|
||||||
gate.InstanceTimestamp,
|
gate.InstanceTimestamp,
|
||||||
gate.ServerControlledMoveTimestamp,
|
gate.ServerControlledMoveTimestamp,
|
||||||
gate.TeleportTimestamp,
|
gate.TeleportTimestamp,
|
||||||
gate.ForcePositionTimestamp,
|
gate.ForcePositionTimestamp,
|
||||||
teleportAdvanced,
|
teleportAdvanced,
|
||||||
TeleportHookRequired: false,
|
TeleportHookRequired: false,
|
||||||
previousTeleport);
|
previousTeleport ?? gate.TeleportTimestamp);
|
||||||
|
|
||||||
private static WorldSession.EntitySpawn MergeUntimestampedCreate(
|
private static WorldSession.EntitySpawn MergeUntimestampedCreate(
|
||||||
WorldSession.EntitySpawn retained,
|
WorldSession.EntitySpawn retained,
|
||||||
|
|
|
||||||
|
|
@ -2,7 +2,9 @@ using System.Numerics;
|
||||||
using AcDream.Core.Net;
|
using AcDream.Core.Net;
|
||||||
using AcDream.Core.Net.Messages;
|
using AcDream.Core.Net.Messages;
|
||||||
using AcDream.Core.Physics;
|
using AcDream.Core.Physics;
|
||||||
|
using AcDream.Runtime;
|
||||||
using AcDream.Runtime.Entities;
|
using AcDream.Runtime.Entities;
|
||||||
|
using AcDream.Runtime.Physics;
|
||||||
|
|
||||||
namespace AcDream.Runtime.Tests.Entities;
|
namespace AcDream.Runtime.Tests.Entities;
|
||||||
|
|
||||||
|
|
@ -484,6 +486,142 @@ public sealed class InboundPhysicsStateControllerTests
|
||||||
Assert.Equal(liveVelocity, retained.Physics.Value.Velocity);
|
Assert.Equal(liveVelocity, retained.Physics.Value.Velocity);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// #307: the live Position path must report the TELEPORT_TS value the gate
|
||||||
|
/// held BEFORE the event, not 0. Every consumer of
|
||||||
|
/// <c>AcceptedPhysicsTimestamps.PreviousTeleport</c> compares it against
|
||||||
|
/// the accepted stamp; a hard 0 makes "this packet did not advance
|
||||||
|
/// TELEPORT_TS" indistinguishable from "this packet regressed it" for any
|
||||||
|
/// entity that has ever teleported.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void TryApplyPosition_ReportsThePreEventTeleportStamp()
|
||||||
|
{
|
||||||
|
var controller = new InboundPhysicsStateController();
|
||||||
|
WorldSession.EntitySpawn spawn = WithTimestamps(
|
||||||
|
Spawn(0x50000010u, 3, 10, 1, Position(0x0101FFFFu, 10f), 0x408u),
|
||||||
|
teleport: 10,
|
||||||
|
forcePosition: 0);
|
||||||
|
controller.AcceptCreate(spawn);
|
||||||
|
|
||||||
|
// An ordinary Position carrying the SAME TELEPORT_TS: previous and
|
||||||
|
// accepted must both be the live stamp, and nothing advanced.
|
||||||
|
Assert.True(controller.TryApplyPosition(
|
||||||
|
PositionUpdate(spawn.Guid, instance: 3, position: 11, teleport: 10),
|
||||||
|
isLocalPlayer: false,
|
||||||
|
forcePositionRotation: null,
|
||||||
|
currentLocalVelocity: null,
|
||||||
|
out PositionTimestampDisposition steady,
|
||||||
|
out _,
|
||||||
|
out AcceptedPhysicsTimestamps steadyStamps));
|
||||||
|
Assert.Equal(PositionTimestampDisposition.Apply, steady);
|
||||||
|
Assert.Equal((ushort)10, steadyStamps.PreviousTeleport);
|
||||||
|
Assert.Equal((ushort)10, steadyStamps.Teleport);
|
||||||
|
Assert.False(steadyStamps.TeleportAdvanced);
|
||||||
|
|
||||||
|
// A fresh TELEPORT_TS: previous is the PRE-event stamp, accepted is
|
||||||
|
// the new one. Capturing after the gate mutated would collapse both
|
||||||
|
// onto 11 and silently lose the advance.
|
||||||
|
Assert.True(controller.TryApplyPosition(
|
||||||
|
PositionUpdate(spawn.Guid, instance: 3, position: 12, teleport: 11),
|
||||||
|
isLocalPlayer: false,
|
||||||
|
forcePositionRotation: null,
|
||||||
|
currentLocalVelocity: null,
|
||||||
|
out PositionTimestampDisposition advanced,
|
||||||
|
out _,
|
||||||
|
out AcceptedPhysicsTimestamps advancedStamps));
|
||||||
|
Assert.Equal(PositionTimestampDisposition.Apply, advanced);
|
||||||
|
Assert.Equal((ushort)10, advancedStamps.PreviousTeleport);
|
||||||
|
Assert.Equal((ushort)11, advancedStamps.Teleport);
|
||||||
|
Assert.True(advancedStamps.TeleportAdvanced);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// #307, the shipped consequence: a local player who has portalled or
|
||||||
|
/// recalled this session holds a nonzero TELEPORT_TS. Retail's
|
||||||
|
/// FORCE_POSITION branch (<c>SmartBox::HandleReceivedPosition</c>
|
||||||
|
/// 0x00453FD0) fires only when the packet's teleport stamp EQUALS the live
|
||||||
|
/// one and never advances it, so the authority C4 route 2 builds from
|
||||||
|
/// these timestamps must satisfy
|
||||||
|
/// <c>PreviousTeleportSequence == AcceptedTeleportSequence</c>. With
|
||||||
|
/// PreviousTeleport pinned to 0 the classifier rejected the authority and
|
||||||
|
/// the force correction was silently dropped.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void LocalPlayerForcePositionAfterATeleport_ClassifiesAsAnAcceptedForceCorrection()
|
||||||
|
{
|
||||||
|
var controller = new InboundPhysicsStateController();
|
||||||
|
WorldSession.EntitySpawn spawn = WithTimestamps(
|
||||||
|
Spawn(0x50000011u, 3, 10, 1, Position(0x0101FFFFu, 10f), 0x408u),
|
||||||
|
teleport: 10,
|
||||||
|
forcePosition: 0);
|
||||||
|
controller.AcceptCreate(spawn);
|
||||||
|
|
||||||
|
Assert.True(controller.TryApplyPosition(
|
||||||
|
PositionUpdate(
|
||||||
|
spawn.Guid,
|
||||||
|
instance: 3,
|
||||||
|
position: 9,
|
||||||
|
teleport: 10,
|
||||||
|
forcePosition: 1),
|
||||||
|
isLocalPlayer: true,
|
||||||
|
forcePositionRotation: Quaternion.Identity,
|
||||||
|
currentLocalVelocity: Vector3.Zero,
|
||||||
|
out PositionTimestampDisposition disposition,
|
||||||
|
out WorldSession.EntitySpawn accepted,
|
||||||
|
out AcceptedPhysicsTimestamps timestamps));
|
||||||
|
Assert.Equal(PositionTimestampDisposition.ForcePosition, disposition);
|
||||||
|
|
||||||
|
// Built exactly as the route-2 drive builds it from these outputs.
|
||||||
|
var authority = new RuntimeAuthoritativePositionAuthority(
|
||||||
|
new RuntimeGenerationToken(7),
|
||||||
|
new RuntimeEntityKey(spawn.Guid, 1),
|
||||||
|
PositionAuthorityVersion: 4UL,
|
||||||
|
AcceptedPositionSequence: 9,
|
||||||
|
timestamps.PreviousTeleport,
|
||||||
|
timestamps.Teleport,
|
||||||
|
disposition);
|
||||||
|
RuntimeAuthoritativePositionRoute route =
|
||||||
|
RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition(
|
||||||
|
new RuntimeAcceptedPositionRouteRequest(
|
||||||
|
authority,
|
||||||
|
RuntimePositionEntityKind.LocalPlayer,
|
||||||
|
RuntimeAcceptedPositionSource.PositionEvent,
|
||||||
|
accepted.Position!.Value,
|
||||||
|
PlacementFrame: 0u,
|
||||||
|
PositionPackVelocity: Vector3.Zero,
|
||||||
|
CommittedCellId: 0x0101FFFFu,
|
||||||
|
HasContact: true,
|
||||||
|
PlayerDistance: 0f,
|
||||||
|
UsePositionFromServer: true,
|
||||||
|
HasAnimations: false,
|
||||||
|
default));
|
||||||
|
|
||||||
|
Assert.Equal(
|
||||||
|
RuntimeAuthoritativePositionDisposition.SetPositionSimple,
|
||||||
|
route.Disposition);
|
||||||
|
Assert.True(route.SendPositionImmediately);
|
||||||
|
Assert.Equal((ushort)10, timestamps.PreviousTeleport);
|
||||||
|
Assert.Equal((ushort)10, timestamps.Teleport);
|
||||||
|
}
|
||||||
|
|
||||||
|
private static WorldSession.EntityPositionUpdate PositionUpdate(
|
||||||
|
uint guid,
|
||||||
|
ushort instance,
|
||||||
|
ushort position,
|
||||||
|
ushort teleport,
|
||||||
|
ushort forcePosition = 0) =>
|
||||||
|
new(
|
||||||
|
guid,
|
||||||
|
Position(0x0101FFFFu, 20f),
|
||||||
|
null,
|
||||||
|
null,
|
||||||
|
true,
|
||||||
|
instance,
|
||||||
|
position,
|
||||||
|
teleport,
|
||||||
|
forcePosition);
|
||||||
|
|
||||||
private static WorldSession.EntitySpawn WithTimestamps(
|
private static WorldSession.EntitySpawn WithTimestamps(
|
||||||
WorldSession.EntitySpawn spawn,
|
WorldSession.EntitySpawn spawn,
|
||||||
ushort? movement = null,
|
ushort? movement = null,
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue