diff --git a/docs/ISSUES.md b/docs/ISSUES.md index e6aa2833..93d139df 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -763,6 +763,50 @@ it. Do #297 FIRST — #298 depends on it. `data_7dXXXX` symbol) and `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 - **#276 — OPEN — SpawnPlacementSettler discards the settle's resolved diff --git a/src/AcDream.Runtime/Entities/InboundPhysicsStateController.cs b/src/AcDream.Runtime/Entities/InboundPhysicsStateController.cs index f00d59d8..6b729935 100644 --- a/src/AcDream.Runtime/Entities/InboundPhysicsStateController.cs +++ b/src/AcDream.Runtime/Entities/InboundPhysicsStateController.cs @@ -624,8 +624,16 @@ public sealed class InboundPhysicsStateController 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( - gate.TeleportTimestamp, + previousTeleport, update.TeleportSequence); disposition = gate.TryAcceptPositionEvent( update.InstanceSequence, @@ -636,7 +644,8 @@ public sealed class InboundPhysicsStateController timestamps = Current( gate, teleportAdvanced: disposition is PositionTimestampDisposition.Apply - && advancesTeleport); + && advancesTeleport, + previousTeleport: previousTeleport); accepted = ApplyAcceptedPosition( old, update, @@ -1113,17 +1122,28 @@ public sealed class InboundPhysicsStateController 0, spawn.InstanceSequence); + /// + /// #307: 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 + /// RuntimeAuthoritativePositionRouteClassifier. + /// private static AcceptedPhysicsTimestamps Current( PhysicsTimestampGate gate, bool teleportAdvanced = false, - ushort previousTeleport = 0) => new( + ushort? previousTeleport = null) => new( gate.InstanceTimestamp, gate.ServerControlledMoveTimestamp, gate.TeleportTimestamp, gate.ForcePositionTimestamp, teleportAdvanced, TeleportHookRequired: false, - previousTeleport); + previousTeleport ?? gate.TeleportTimestamp); private static WorldSession.EntitySpawn MergeUntimestampedCreate( WorldSession.EntitySpawn retained, diff --git a/tests/AcDream.Runtime.Tests/Entities/InboundPhysicsStateControllerTests.cs b/tests/AcDream.Runtime.Tests/Entities/InboundPhysicsStateControllerTests.cs index 4edd2dbf..9665917b 100644 --- a/tests/AcDream.Runtime.Tests/Entities/InboundPhysicsStateControllerTests.cs +++ b/tests/AcDream.Runtime.Tests/Entities/InboundPhysicsStateControllerTests.cs @@ -2,7 +2,9 @@ using System.Numerics; using AcDream.Core.Net; using AcDream.Core.Net.Messages; using AcDream.Core.Physics; +using AcDream.Runtime; using AcDream.Runtime.Entities; +using AcDream.Runtime.Physics; namespace AcDream.Runtime.Tests.Entities; @@ -484,6 +486,142 @@ public sealed class InboundPhysicsStateControllerTests Assert.Equal(liveVelocity, retained.Physics.Value.Velocity); } + /// + /// #307: the live Position path must report the TELEPORT_TS value the gate + /// held BEFORE the event, not 0. Every consumer of + /// AcceptedPhysicsTimestamps.PreviousTeleport 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. + /// + [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); + } + + /// + /// #307, the shipped consequence: a local player who has portalled or + /// recalled this session holds a nonzero TELEPORT_TS. Retail's + /// FORCE_POSITION branch (SmartBox::HandleReceivedPosition + /// 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 + /// PreviousTeleportSequence == AcceptedTeleportSequence. With + /// PreviousTeleport pinned to 0 the classifier rejected the authority and + /// the force correction was silently dropped. + /// + [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( WorldSession.EntitySpawn spawn, ushort? movement = null,