Add CollisionPredictionTest (#5493)

Co-authored-by: metalgearsloth <[email protected]>
Co-authored-by: metalgearsloth <[email protected]>

RELEASE-NOTES.md

@@ -43,7 +43,7 @@ END TEMPLATE-->
 
 ### Bugfixes
 
-*None yet*
+* Fixed `RaisePredictiveEvent()` not properly re-raising events during prediction for event handlers that did not take an `EntitySessionEventArgs` argument.
 
 ### Other
 

Robust.Client/GameStates/ClientGameStateManager.cs

@@ -42,7 +42,7 @@ public sealed class ClientGameStateManager : IClientGameStateManager
         private uint _nextInputCmdSeq = 1;
         private readonly Queue<FullInputCmdMessage> _pendingInputs = new();
 
-        private readonly Queue<(uint sequence, GameTick sourceTick, EntityEventArgs msg, object sessionMsg)>
+        private readonly Queue<(uint sequence, GameTick sourceTick, object msg, object sessionMsg)>
             _pendingSystemMessages
                 = new();
 
@@ -504,9 +504,7 @@ public void PredictTicks(GameTick predictionTarget)
 
                 while (hasPendingMessage && pendingMessagesEnumerator.Current.sourceTick <= _timing.CurTick)
                 {
-                    var msg = pendingMessagesEnumerator.Current.msg;
-
-                    _entities.EventBus.RaiseEvent(EventSource.Local, msg);
+                    _entities.EventBus.RaiseEvent(EventSource.Local, pendingMessagesEnumerator.Current.msg);
                     _entities.EventBus.RaiseEvent(EventSource.Local, pendingMessagesEnumerator.Current.sessionMsg);
                     hasPendingMessage = pendingMessagesEnumerator.MoveNext();
                 }

Robust.Client/Physics/PhysicsSystem.Predict.cs

@@ -93,7 +93,7 @@ internal void ResetContacts()
         var maps = new HashSet<EntityUid>();
 
         var enumerator = AllEntityQuery<PredictedPhysicsComponent, PhysicsComponent, TransformComponent>();
-        while (enumerator.MoveNext(out var _, out var physics, out var xform))
+        while (enumerator.MoveNext(out _, out var physics, out var xform))
         {
             DebugTools.Assert(physics.Predict);
 

Robust.Shared/GameObjects/Systems/SharedTransformSystem.Component.cs

@@ -899,11 +899,11 @@ public void SetMapCoordinates(Entity<TransformComponent> entity, MapCoordinates
             _mapManager.TryFindGridAt(mapUid, coordinates.Position, out var targetGrid, out _))
         {
             var invWorldMatrix = GetInvWorldMatrix(targetGrid);
-            SetCoordinates(entity, new EntityCoordinates(targetGrid, Vector2.Transform(coordinates.Position, invWorldMatrix)));
+            SetCoordinates((entity.Owner, entity.Comp, MetaData(entity.Owner)), new EntityCoordinates(targetGrid, Vector2.Transform(coordinates.Position, invWorldMatrix)));
         }
         else
         {
-            SetCoordinates(entity, new EntityCoordinates(mapUid, coordinates.Position));
+            SetCoordinates((entity.Owner, entity.Comp, MetaData(entity.Owner)), new EntityCoordinates(mapUid, coordinates.Position));
         }
     }
 

Robust.Shared/Physics/Systems/SharedBroadphaseSystem.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Buffers;
 using System.Collections.Generic;
 using System.Numerics;
 using System.Threading.Tasks;
@@ -8,7 +7,6 @@
 using Robust.Shared.Configuration;
 using Robust.Shared.GameObjects;
 using Robust.Shared.IoC;
-using Robust.Shared.Log;
 using Robust.Shared.Map;
 using Robust.Shared.Map.Components;
 using Robust.Shared.Maths;
@@ -413,24 +411,34 @@ private void QueryBroadphase(IBroadPhase broadPhase, (List<FixtureProxy>, Fixtur
             }, aabb, true);
         }
 
+        [Obsolete("Use Entity<T> variant")]
         public void RegenerateContacts(EntityUid uid, PhysicsComponent body, FixturesComponent? fixtures = null, TransformComponent? xform = null)
         {
-            _physicsSystem.DestroyContacts(body);
-            if (!Resolve(uid, ref xform, ref fixtures))
+            RegenerateContacts((uid, body, fixtures, xform));
+        }
+
+        public void RegenerateContacts(Entity<PhysicsComponent?, FixturesComponent?, TransformComponent?> entity)
+        {
+            if (!Resolve(entity.Owner, ref entity.Comp1))
                 return;
 
-            if (xform.MapUid == null)
+            _physicsSystem.DestroyContacts(entity.Comp1);
+
+            if (!Resolve(entity.Owner, ref entity.Comp2 , ref entity.Comp3))
                 return;
 
-            if (!_xformQuery.TryGetComponent(xform.Broadphase?.Uid, out var broadphase))
+            if (entity.Comp3.MapUid == null)
+                return;
+
+            if (!_xformQuery.TryGetComponent(entity.Comp3.Broadphase?.Uid, out var broadphase))
                 return;
 
-            _physicsSystem.SetAwake((uid, body), true);
+            _physicsSystem.SetAwake(entity!, true);
 
             var matrix = _transform.GetWorldMatrix(broadphase);
-            foreach (var fixture in fixtures.Fixtures.Values)
+            foreach (var fixture in entity.Comp2.Fixtures.Values)
             {
-                TouchProxies(xform.MapUid.Value, matrix, fixture);
+                TouchProxies(entity.Comp3.MapUid.Value, matrix, fixture);
             }
         }
 

Robust.Shared/Physics/Systems/SharedPhysicsSystem.Queries.cs

@@ -202,27 +202,27 @@ public IEnumerable<Entity<PhysicsComponent>> GetCollidingEntities(MapId mapId, i
             return bodies;
         }
 
-        public HashSet<EntityUid> GetContactingEntities(EntityUid uid, PhysicsComponent? body = null, bool approximate = false)
+        public void GetContactingEntities(Entity<PhysicsComponent?> ent, HashSet<EntityUid> contacting, bool approximate = false)
         {
-            // HashSet to ensure that we only return each entity once, instead of once per colliding fixture.
-            var result = new HashSet<EntityUid>();
+            if (!Resolve(ent.Owner, ref ent.Comp))
+                return;
 
-            if (!Resolve(uid, ref body))
-                return result;
-
-            var node = body.Contacts.First;
+            var node = ent.Comp.Contacts.First;
 
             while (node != null)
             {
                 var contact = node.Value;
                 node = node.Next;
 
-                if (!approximate && !contact.IsTouching)
-                    continue;
-
-                result.Add(uid == contact.EntityA ? contact.EntityB : contact.EntityA);
+                if (approximate || contact.IsTouching)
+                    contacting.Add(ent.Owner == contact.EntityA ? contact.EntityB : contact.EntityA);
             }
+        }
 
+        public HashSet<EntityUid> GetContactingEntities(EntityUid uid, PhysicsComponent? body = null, bool approximate = false)
+        {
+            var result = new HashSet<EntityUid>();
+            GetContactingEntities((uid, body), result, approximate);
             return result;
         }
 

Robust.Shared/Physics/Systems/SharedPhysicsSystem.cs

@@ -327,6 +327,28 @@ protected void SimulateWorld(float deltaTime, bool prediction)
                     RaiseLocalEvent(ref updateMapBeforeSolve);
                 }
 
+                // TODO PHYSICS Fix Collision Mispredicts
+                // If a physics update induces a position update that brings fixtures into contact, the collision starts in the NEXT tick,
+                // as positions are updated after CollideContacts() gets called.
+                //
+                // If a player input induces a position update that brings fixtures into contact, the collision happens on the SAME tick,
+                // as inputs are handled before system updates.
+                //
+                // When applying a server's game state with new positions, the client won't know what caused the positions to update,
+                // and thus can't know whether the collision already occurred (i.e., whether its effects are already contained within the
+                // game state currently being applied), or whether it should start on the next tick and needs to predict the start of
+                // the collision.
+                //
+                // Currently the client assumes that any position updates happened due to physics steps. I.e., positions are reset, then
+                // contacts are reset via ResetContacts(), then positions are updated using the new game state. Alternatively, we could
+                // call ResetContacts() AFTER applying the server state, which would correspond to assuming that the collisions have
+                // already "started" in the state, and we don't want to re-raise the events.
+                //
+                // Currently there is no way to avoid mispredicts from happening. E.g., a simple collision-counter component will always
+                // either mispredict on physics induced position changes, or on player/input induced updates. The easiest way I can think
+                // of to fix this would be to always call `CollideContacts` again at the very end of a physics update.
+                // But that might be unnecessarily expensive for what are hopefully only infrequent mispredicts.
+
                 CollideContacts();
                 var enumerator = AllEntityQuery<PhysicsMapComponent>();