PredictedQueueDel fix attempt (space-wizards#6688)

Author: metalgearsloth

RELEASE-NOTES.md

@@ -43,7 +43,7 @@ END TEMPLATE-->
 
 ### Bugfixes
 
-*None yet*
+* Fix PredictedQueueDel not rolling back properly on the client.
 
 ### Other
 

Robust.Client/GameObjects/ClientEntityManager.cs

@@ -31,6 +31,7 @@ public sealed partial class ClientEntityManager : EntityManager, IClientEntityMa
 
         private readonly Queue<EntityUid> _queuedPredictedDeletions = new();
         private readonly HashSet<EntityUid> _queuedPredictedDeletionsSet = new();
+        private readonly HashSet<EntityUid> _predictedDetachedEntities = new();
         private Histogram? _tickUpdateHistogram;
         private Histogram.Child? _entityNetHistogram;
 
@@ -61,6 +62,9 @@ public override void FlushEntities()
             // Server doesn't network deletions on client shutdown so we need to
             // manually clear these out or risk stale data getting used.
             PendingNetEntityStates.Clear();
+            _queuedPredictedDeletions.Clear();
+            _queuedPredictedDeletionsSet.Clear();
+            _predictedDetachedEntities.Clear();
             using var _ = _gameTiming.StartStateApplicationArea();
             base.FlushEntities();
         }
@@ -97,6 +101,14 @@ public override void QueueDeleteEntity(EntityUid? uid)
                 LogManager.RootSawmill.Error($"Predicting the queued deletion of a networked entity: {ToPrettyString(uid.Value)}. Trace: {Environment.StackTrace}");
         }
 
+        public override void DeleteEntity(EntityUid? uid)
+        {
+            if (uid != null)
+                ClearPredictedDeletion(uid.Value);
+
+            base.DeleteEntity(uid);
+        }
+
         /// <inheritdoc />
         public override void Dirty(EntityUid uid, IComponent component, MetaDataComponent? meta = null)
         {
@@ -237,6 +249,9 @@ internal override void ProcessQueueudDeletions()
             base.ProcessQueueudDeletions();
             while (_queuedPredictedDeletions.TryDequeue(out var uid))
             {
+                if (!_queuedPredictedDeletionsSet.Remove(uid))
+                    continue;
+
                 if (!MetaQuery.TryGetComponentInternal(uid, out var meta))
                     continue;
 
@@ -246,19 +261,14 @@ internal override void ProcessQueueudDeletions()
                 var xform = TransformQuery.GetComponentInternal(uid);
                 if (meta.NetEntity.IsClientSide())
                 {
+                    ClearPredictedDeletion(uid);
                     DeleteEntity(uid, meta, xform);
                 }
                 else
                 {
-                    _xforms.DetachEntity(uid, xform, meta, null);
-                    // base call bypasses IGameTiming.InPrediction check
-                    // This is pretty janky and there should be a way for the client to dirty an entity outside of prediction
-                    // TODO PREDICTION Is actually needed after the current predicted deletion fix?
-                    base.Dirty(uid, xform, meta);
+                    PredictedDetachNetworkedEntity(uid, xform, meta);
                 }
             }
-
-            _queuedPredictedDeletionsSet.Clear();
         }
 
         /// <inheritdoc />
@@ -354,16 +364,42 @@ public override void PredictedDeleteEntity(Entity<MetaDataComponent?, TransformC
 
             if (ent.Comp1.NetEntity.IsClientSide())
             {
+                ClearPredictedDeletion(ent.Owner);
                 DeleteEntity(ent, ent.Comp1, ent.Comp2);
             }
             else
             {
-                _xforms.DetachEntity(ent, ent.Comp2);
+                PredictedDetachNetworkedEntity(ent.Owner, ent.Comp2, ent.Comp1);
             }
         }
 
+        internal bool IsPredictedDetached(EntityUid uid)
+        {
+            return _predictedDetachedEntities.Contains(uid);
+        }
+
+        internal void ClearPredictedDeletion(EntityUid uid)
+        {
+            _predictedDetachedEntities.Remove(uid);
+            _queuedPredictedDeletionsSet.Remove(uid);
+        }
+
+        private void PredictedDetachNetworkedEntity(EntityUid uid, TransformComponent xform, MetaDataComponent meta)
+        {
+            if (!_predictedDetachedEntities.Add(uid))
+                return;
+
+            // base call bypasses IGameTiming.InPrediction check. Predicted queue deletes are processed after prediction,
+            // but reset still needs to see the detached entity as dirty and restore it from the last server state.
+            base.Dirty(uid, xform, meta);
+            _xforms.DetachEntity(uid, xform, meta, null);
+            meta.Flags |= MetaDataFlags.Detached;
+        }
+
         public override bool IsQueuedForDeletion(EntityUid uid)
-            => QueuedDeletionsSet.Contains(uid) || _queuedPredictedDeletions.Contains(uid);
+            => QueuedDeletionsSet.Contains(uid)
+               || _queuedPredictedDeletionsSet.Contains(uid)
+               || _predictedDetachedEntities.Contains(uid);
 
         /// <inheritdoc />
         public override void PredictedQueueDeleteEntity(Entity<MetaDataComponent?> ent)

Robust.Client/GameStates/ClientGameStateManager.cs

@@ -602,15 +602,20 @@ public void ResetPredictedEntities()
                     !_processor.TryGetLastServerStates(meta.NetEntity, out var last))
                 {
                     // Entity was probably deleted on the server so do nothing.
+                    _entities.ClearPredictedDeletion(entity);
                     continue;
                 }
 
                 countReset += 1;
+                var predictedDetached = _entities.IsPredictedDetached(entity);
 
                 try
                 {
                     _resettingPredictedEntities = true;
 
+                    if (predictedDetached)
+                        meta.Flags &= ~MetaDataFlags.Detached;
+
                     foreach (var (netId, comp) in meta.NetComponents)
                     {
                         if (!comp.NetSyncEnabled)
@@ -634,6 +639,7 @@ public void ResetPredictedEntities()
                             }
                         }
 
+                        // If the component wasn't modified, or if we have no state to roll back to.
                         if (comp.LastModifiedTick <= _timing.LastRealTick ||
                             !last.TryGetValue(netId, out var compState))
                         {
@@ -693,6 +699,7 @@ public void ResetPredictedEntities()
 
                 DebugTools.Assert(meta.EntityLastModifiedTick > _timing.LastRealTick);
                 meta.EntityLastModifiedTick = _timing.LastRealTick;
+                _entities.ClearPredictedDeletion(entity);
             }
 
             _entities.System<PhysicsSystem>().ResetContacts();

Robust.Client/Properties/AssemblyInfo.cs

@@ -7,6 +7,7 @@
 [assembly: InternalsVisibleTo("Robust.Benchmarks")]
 [assembly: InternalsVisibleTo("Robust.Client.Tests")]
 [assembly: InternalsVisibleTo("Robust.Client.IntegrationTests")]
+[assembly: InternalsVisibleTo("Robust.Shared.IntegrationTests")]
 
 #if NET5_0_OR_GREATER
 [module: SkipLocalsInit]