diff --git a/Assets/_Project/Scripts/Client/Presentation/EnemyDangerTelegraphSystem.cs b/Assets/_Project/Scripts/Client/Presentation/EnemyDangerTelegraphSystem.cs index 95b987953..ac2d2ae71 100644 --- a/Assets/_Project/Scripts/Client/Presentation/EnemyDangerTelegraphSystem.cs +++ b/Assets/_Project/Scripts/Client/Presentation/EnemyDangerTelegraphSystem.cs @@ -118,7 +118,13 @@ namespace ProjectM.Client void UpdateEnemyDanger(float3 localPos) { if (_fxRoot == null || _dangerMat == null) return; - Unity.NetCode.NetworkTick serverTick = SystemAPI.TryGetSingleton(out var nt) ? nt.ServerTick : default; + // Enemies are ownerless INTERPOLATED ghosts, so their replicated AttackWindup arrives on the + // INTERPOLATION timeline. Timing the danger cone against the PREDICTED ServerTick makes the cue + // finish ~RTT/2 + interp-buffer ticks EARLY over a real connection — the dodge tell lies. Invisible on + // loopback, which is exactly why it survived (audit finding M1). Same idiom as ZoneTelegraphSystem. + Unity.NetCode.NetworkTick serverTick = default; + if (SystemAPI.TryGetSingleton(out var nt)) + serverTick = nt.InterpolationTick.IsValid ? nt.InterpolationTick : nt.ServerTick; _dangerSeen.Clear(); _enemySeen.Clear(); diff --git a/Assets/_Project/Scripts/Client/Presentation/PlayerAnimationDriveSystem.cs b/Assets/_Project/Scripts/Client/Presentation/PlayerAnimationDriveSystem.cs index e8e72130e..525fac765 100644 --- a/Assets/_Project/Scripts/Client/Presentation/PlayerAnimationDriveSystem.cs +++ b/Assets/_Project/Scripts/Client/Presentation/PlayerAnimationDriveSystem.cs @@ -83,7 +83,12 @@ static readonly FastAnimatorParameter k_ComboStep = new FastAnimatorParameter( if (dt < 1e-5f) dt = 1e-5f; // Current authoritative tick for the swing-window check (default = invalid -> IsAttacking stays false). - NetworkTick serverTick = SystemAPI.TryGetSingleton(out var nt) ? nt.ServerTick : default; + NetworkTick serverTick = default, interpTick = default; + if (SystemAPI.TryGetSingleton(out var nt)) + { + serverTick = nt.ServerTick; // predicted: local owner + interpTick = nt.InterpolationTick.IsValid ? nt.InterpolationTick : nt.ServerTick; // interpolated: remotes + } // Ability blob for the per-class special read (Cone -> slam anim); default(BlobAssetReference) if absent. var abilityBlob = SystemAPI.TryGetSingleton(out var adbSingleton) ? adbSingleton.Value : default; // 07-18 combat idle: swing-recency window (the same wrap-safe SwingActive predicate as the anim pulse, wider). 60Hz sim ticks. @@ -125,13 +130,18 @@ static readonly FastAnimatorParameter k_ComboStep = new FastAnimatorParameter( Dependency = localJob.ScheduleParallel(Dependency); // --- REMOTE players (position-delta). Single-threaded write to the shared prevPos cache. --- + // Remote teammates are INTERPOLATED ghosts: their replicated MeleeCombo / SocketCooldown arrive on the + // interpolation timeline, so their swing windows must be timed against InterpolationTick. Using the + // predicted ServerTick here desynced a teammate's swing animation from their damage by ~RTT/2 + + // interp buffer — invisible on loopback (audit finding M1). The LOCAL job above stays on ServerTick: + // the owning player IS predicted. var seen = new NativeParallelHashSet(16, Allocator.TempJob); var remoteJob = new RemoteDriveJob { moveX = k_MoveX, moveZ = k_MoveZ, speed = k_Speed, isDead = k_IsDead, isAttacking = k_IsAttacking, isFiring = k_IsFiring, isDashing = k_IsDashing, comboStep = k_ComboStep, isCone = k_IsCone, abilityDb = abilityBlob, - serverTick = serverTick, attackTicks = k_AttackAnimTicks, + serverTick = interpTick, attackTicks = k_AttackAnimTicks, dt = dt, strideScale = k_StrideScale, trudgeNaturalSpeed = math.max(0.5f, FeelConfig.TrudgeNaturalSpeed), runNaturalSpeed = math.max(0.5f, FeelConfig.RunNaturalSpeed), diff --git a/Assets/_Project/Scripts/Server/Combat/HealthApplyDamageSystem.cs b/Assets/_Project/Scripts/Server/Combat/HealthApplyDamageSystem.cs index 6cc67c37f..e763821b4 100644 --- a/Assets/_Project/Scripts/Server/Combat/HealthApplyDamageSystem.cs +++ b/Assets/_Project/Scripts/Server/Combat/HealthApplyDamageSystem.cs @@ -44,6 +44,7 @@ namespace ProjectM.Server foreach (var (health, dmg, entity) in SystemAPI.Query, DynamicBuffer>() + .WithAll() // predicted-group convention: only simulate what this tick simulates .WithEntityAccess()) { if (dmg.Length == 0) diff --git a/Assets/_Project/Scripts/Server/Combat/ProjectileDamageSystem.cs b/Assets/_Project/Scripts/Server/Combat/ProjectileDamageSystem.cs index 5f779b1ef..e28f97953 100644 --- a/Assets/_Project/Scripts/Server/Combat/ProjectileDamageSystem.cs +++ b/Assets/_Project/Scripts/Server/Combat/ProjectileDamageSystem.cs @@ -115,6 +115,7 @@ namespace ProjectM.Server foreach (var (xform, proj, owner, projectileEntity) in SystemAPI.Query, RefRW, RefRO>() + .WithAll() // predicted-group convention: only simulate what this tick simulates .WithEntityAccess()) { int projOwnerId = owner.ValueRO.NetworkId; diff --git a/Assets/_Project/Scripts/Server/Connection/StaleRpcReaperSystem.cs b/Assets/_Project/Scripts/Server/Connection/StaleRpcReaperSystem.cs new file mode 100644 index 000000000..7faf0e9ce --- /dev/null +++ b/Assets/_Project/Scripts/Server/Connection/StaleRpcReaperSystem.cs @@ -0,0 +1,58 @@ +using Unity.Burst; +using Unity.Collections; +using Unity.Entities; +using Unity.NetCode; + +namespace ProjectM.Server +{ + /// + /// Terminal drain for RPC requests no receiver consumed. Destroys any entity still carrying an unconsumed + /// after it has survived a couple of frames, so a missing receiver + /// degrades to a silent drop instead of a permanent entity leak. + /// + /// WHY (2026-08-06 audit, reproduced live): every RPC receiver in this project gates itself behind + /// RequireForUpdate<T> on a scene-baked singleton. In DevSandbox none of those singletons exist, + /// so eleven receivers never ran at all — one keypress on E/Q/G/T created a server request entity that + /// nothing ever visited. Netcode's own WarnAboutStaleRpcSystem logs "has not been consumed or + /// destroyed for 4 (MaxRpcAgeFrames) frames", then calls Consume() — it never destroys the entity, and it is + /// compiled out of player builds (#if UNITY_EDITOR && !NETCODE_NDEBUG). So the entities + /// accumulated forever, silently, and worst in a shipped build. + /// + /// ORDERING: OrderLast in the plain server , so every same-frame receiver + /// (including those inside the nested predicted group) has already had its chance. The Age gate is belt and + /// braces on top of that: Age increments per frame the request survives, so requiring >= 2 means the request + /// lived through a full frame unhandled before we reap it. Consumed requests are skipped — their owning + /// receiver is responsible for destroying them, and Consume() sets Age to ushort.MaxValue anyway. + /// + [BurstCompile] + [WorldSystemFilter(WorldSystemFilterFlags.ServerSimulation)] + [UpdateInGroup(typeof(SimulationSystemGroup), OrderLast = true)] + public partial struct StaleRpcReaperSystem : ISystem + { + /// Frames an unconsumed request may survive before it is reaped. Must exceed 1 so a receiver + /// that legitimately runs later in the same frame is never robbed of its request. + const ushort k_MaxAgeFrames = 2; + + [BurstCompile] + public void OnCreate(ref SystemState state) + { + var b = new EntityQueryBuilder(Allocator.Temp).WithAll(); + state.RequireForUpdate(state.GetEntityQuery(b)); + } + + [BurstCompile] + public void OnUpdate(ref SystemState state) + { + var ecb = new EntityCommandBuffer(Allocator.Temp); + foreach (var (recv, entity) in + SystemAPI.Query>().WithEntityAccess()) + { + if (recv.ValueRO.IsConsumed) continue; // a receiver owns it; not ours to destroy + if (recv.ValueRO.Age < k_MaxAgeFrames) continue; + ecb.DestroyEntity(entity); + } + ecb.Playback(state.EntityManager); + ecb.Dispose(); + } + } +} diff --git a/Assets/_Project/Scripts/Server/Connection/StaleRpcReaperSystem.cs.meta b/Assets/_Project/Scripts/Server/Connection/StaleRpcReaperSystem.cs.meta new file mode 100644 index 000000000..40556027f --- /dev/null +++ b/Assets/_Project/Scripts/Server/Connection/StaleRpcReaperSystem.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 1929228eaaa47774a884d97cbdf50b63 \ No newline at end of file diff --git a/Assets/_Project/Tests/EditMode/StaleRpcReaperSystemTests.cs b/Assets/_Project/Tests/EditMode/StaleRpcReaperSystemTests.cs new file mode 100644 index 000000000..0875ca994 --- /dev/null +++ b/Assets/_Project/Tests/EditMode/StaleRpcReaperSystemTests.cs @@ -0,0 +1,74 @@ +using NUnit.Framework; +using ProjectM.Server; +using Unity.Entities; +using Unity.NetCode; + +namespace ProjectM.Tests +{ + /// + /// Regression pin for the RPC-leak class the 2026-08-06 audit reproduced live: every RPC receiver in this + /// project gates itself behind RequireForUpdate on a scene-baked singleton, so in a scene missing that + /// singleton the receiver never runs and the request entity is never destroyed. Netcode's own stale-RPC + /// warning Consume()s but never destroys, and is compiled out of player builds — so the entities accumulated + /// forever, worst in a shipped build. + /// + /// is the terminal drain. These tests pin the two halves of its contract: + /// an aged UNCONSUMED request is destroyed, and a CONSUMED one is left alone for its owning receiver. + /// + public class StaleRpcReaperSystemTests + { + [Test] + public void Unconsumed_Request_Is_Destroyed_Once_It_Has_Aged() + { + var (world, group) = TestWorld.Make("ReaperAged", tick: 100, server: true); + using (world) + { + var em = world.EntityManager; + var e = em.CreateEntity(); + // Age 2 == "survived a full frame unhandled" (the reaper's k_MaxAgeFrames). + em.AddComponentData(e, new ReceiveRpcCommandRequest { SourceConnection = Entity.Null, Age = 2 }); + + group.Update(); + + Assert.IsFalse(em.Exists(e), + "an unconsumed RPC request that outlived its receiving frame must be reaped, not leaked forever."); + } + } + + [Test] + public void Fresh_Request_Survives_So_A_Late_Receiver_Still_Sees_It() + { + var (world, group) = TestWorld.Make("ReaperFresh", tick: 100, server: true); + using (world) + { + var em = world.EntityManager; + var e = em.CreateEntity(); + em.AddComponentData(e, new ReceiveRpcCommandRequest { SourceConnection = Entity.Null, Age = 0 }); + + group.Update(); + + Assert.IsTrue(em.Exists(e), + "a request that arrived this frame must NOT be reaped — a receiver ordered later still owns it."); + } + } + + [Test] + public void Consumed_Request_Is_Left_To_Its_Receiver() + { + var (world, group) = TestWorld.Make("ReaperConsumed", tick: 100, server: true); + using (world) + { + var em = world.EntityManager; + var e = em.CreateEntity(); + var recv = new ReceiveRpcCommandRequest { SourceConnection = Entity.Null, Age = 250 }; + recv.Consume(); + em.AddComponentData(e, recv); + + group.Update(); + + Assert.IsTrue(em.Exists(e), + "a CONSUMED request belongs to the receiver that consumed it; the reaper must not race it."); + } + } + } +} diff --git a/Assets/_Project/Tests/EditMode/StaleRpcReaperSystemTests.cs.meta b/Assets/_Project/Tests/EditMode/StaleRpcReaperSystemTests.cs.meta new file mode 100644 index 000000000..548f56f5d --- /dev/null +++ b/Assets/_Project/Tests/EditMode/StaleRpcReaperSystemTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 3ae4a37d0d1a2ba418335a1d899b4886 \ No newline at end of file