Netcode: fix interpolated-tick cues + add a terminal RPC reaper (audit M1/M4/M12)

M1 — two systems timed INTERPOLATED ghosts against the PREDICTED tick, the exact hazard CLAUDE.md documents and the one that is invisible on loopback: - EnemyDangerTelegraphSystem timed the red danger cone off nt.ServerTick, so over a real connection the dodge tell finished ~RTT/2 + interp buffer EARLY. The cue lied. - PlayerAnimationDriveSystem fed the same predicted tick to RemoteDriveJob ([WithDisabled(GhostOwnerIsLocal)] — i.e. interpolated teammates), so a teammate's swing animation desynced from their damage. Both now use the ZoneTelegraphSystem idiom. The LOCAL drive job keeps ServerTick: the owning player really is predicted. M12 — the RPC leak I reproduced live during the audit. Every receiver in this project gates on RequireForUpdate over a scene-baked singleton; in a scene without it the receiver never runs and the request entity is never destroyed. Netcode's WarnAboutStaleRpcSystem Consume()s but never destroys, and is compiled out of player builds — so these accumulated silently, and worst in a shipped build. New StaleRpcReaperSystem (server, OrderLast, no RequireForUpdate) destroys any unconsumed request that outlived its receiving frame. Consumed requests are left to their owner. Verified live: a planted unconsumed request is gone within a few frames. Three regression tests pin both halves of the contract. Also: HealthApplyDamageSystem and ProjectileDamageSystem now filter .WithAll<Simulate>(). They are ServerSimulation-only so it was not a bug, but the audit's "all predicted systems filter Simulate" reassurance was false until now — the rule is unconditional again. The five undisposed Allocator.Temp ECBs the audit flagged all lived in systems the purge deleted; none remain. 298/298 green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-07 13:17:47 -07:00
parent 9f61f7c6fe
commit 37b211a7f8
8 changed files with 157 additions and 3 deletions
@@ -118,7 +118,13 @@ namespace ProjectM.Client
        void UpdateEnemyDanger(float3 localPos)
        {
            if (_fxRoot == null || _dangerMat == null) return;
-            Unity.NetCode.NetworkTick serverTick = SystemAPI.TryGetSingleton<NetworkTime>(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<NetworkTime>(out var nt))
+                serverTick = nt.InterpolationTick.IsValid ? nt.InterpolationTick : nt.ServerTick;
            _dangerSeen.Clear();
            _enemySeen.Clear();

@@ -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<NetworkTime>(out var nt) ? nt.ServerTick : default;
+            NetworkTick serverTick = default, interpTick = default;
+            if (SystemAPI.TryGetSingleton<NetworkTime>(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<AbilityDatabase>(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<Entity>(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),
@@ -44,6 +44,7 @@ namespace ProjectM.Server

            foreach (var (health, dmg, entity) in
                     SystemAPI.Query<RefRW<Health>, DynamicBuffer<DamageEvent>>()
+                         .WithAll<Simulate>() // predicted-group convention: only simulate what this tick simulates
                         .WithEntityAccess())
            {
                if (dmg.Length == 0)
@@ -115,6 +115,7 @@ namespace ProjectM.Server

            foreach (var (xform, proj, owner, projectileEntity) in
                     SystemAPI.Query<RefRO<LocalTransform>, RefRW<Projectile>, RefRO<GhostOwner>>()
+                         .WithAll<Simulate>() // predicted-group convention: only simulate what this tick simulates
                         .WithEntityAccess())
            {
                int projOwnerId = owner.ValueRO.NetworkId;
@@ -0,0 +1,58 @@
+using Unity.Burst;
+using Unity.Collections;
+using Unity.Entities;
+using Unity.NetCode;
+
+namespace ProjectM.Server
+{
+    /// <summary>
+    /// Terminal drain for RPC requests no receiver consumed. Destroys any entity still carrying an unconsumed
+    /// <see cref="ReceiveRpcCommandRequest"/> 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
+    /// <c>RequireForUpdate&lt;T&gt;</c> 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 <c>WarnAboutStaleRpcSystem</c> 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 (<c>#if UNITY_EDITOR &amp;&amp; !NETCODE_NDEBUG</c>). So the entities
+    /// accumulated forever, silently, and worst in a shipped build.
+    ///
+    /// ORDERING: OrderLast in the plain server <see cref="SimulationSystemGroup"/>, 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.
+    /// </summary>
+    [BurstCompile]
+    [WorldSystemFilter(WorldSystemFilterFlags.ServerSimulation)]
+    [UpdateInGroup(typeof(SimulationSystemGroup), OrderLast = true)]
+    public partial struct StaleRpcReaperSystem : ISystem
+    {
+        /// <summary>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.</summary>
+        const ushort k_MaxAgeFrames = 2;
+
+        [BurstCompile]
+        public void OnCreate(ref SystemState state)
+        {
+            var b = new EntityQueryBuilder(Allocator.Temp).WithAll<ReceiveRpcCommandRequest>();
+            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<RefRO<ReceiveRpcCommandRequest>>().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();
+        }
+    }
+}
@@ -0,0 +1,2 @@
+fileFormatVersion: 2
+guid: 1929228eaaa47774a884d97cbdf50b63