Cleanup IManifoldManager and shape methods

RELEASE-NOTES.md

@@ -35,12 +35,14 @@ END TEMPLATE-->
 
 ### Breaking changes
 
-*None yet*
+* ChildCount and ComputeAABB for IPhysShape were moved to SharedPhysicsSystem.
+* Radius is now validated for physics shapes and negative values are no longer allowed.
 
 ### New features
 
 * Add a `OnlyRotation` property to MoveEvent where the EntityCoordinates remain the same.
 * ValueList now implements IList and not just IEnumerable.
+* SharedPhysicsSystem now exposes TestOverlap and the collision manifold methods.
 
 ### Bugfixes
 

Robust.Client/Debugging/DebugPhysicsSystem.cs

@@ -135,7 +135,7 @@ public override void HandlePreSolve(Contact contact, in Manifold oldManifold)
                 var state1 = new PointState[2];
                 var state2 = new PointState[2];
 
-                CollisionManager.GetPointStates(ref state1, ref state2, oldManifold, manifold);
+                SharedPhysicsSystem.GetPointStates(ref state1, ref state2, oldManifold, manifold);
 
                 Span<Vector2> points = stackalloc Vector2[2];
                 var transformA = _physics.GetPhysicsTransform(contact.EntityA);
@@ -317,9 +317,9 @@ private void DrawWorld(DrawingHandleWorld worldHandle, OverlayDrawArgs args)
 
                     foreach (var fixture in _entityManager.GetComponent<FixturesComponent>(physBody).Fixtures.Values)
                     {
-                        for (var i = 0; i < fixture.Shape.ChildCount; i++)
+                        for (var i = 0; i < _physicsSystem.GetChildCount(fixture.Shape); i++)
                         {
-                            var shapeBB = fixture.Shape.ComputeAABB(xform, i);
+                            var shapeBB = _physicsSystem.ComputeAABB(fixture.Shape, xform, i);
                             aabb = aabb?.Union(shapeBB) ?? shapeBB;
                         }
                     }

Robust.Client/GameObjects/EntitySystems/GridChunkBoundsDebugSystem.cs

@@ -9,6 +9,7 @@
 using Robust.Shared.Maths;
 using Robust.Shared.Physics;
 using Robust.Shared.Physics.Collision.Shapes;
+using Robust.Shared.Physics.Systems;
 using Robust.Shared.Utility;
 
 namespace Robust.Client.GameObjects
@@ -20,6 +21,7 @@ public sealed partial class GridChunkBoundsDebugSystem : EntitySystem
         [Dependency] private IOverlayManager _overlayManager = default!;
         [Dependency] private TransformSystem _transform = default!;
         [Dependency] private SharedMapSystem _map = default!;
+        [Dependency] private SharedPhysicsSystem _physicsSystem = default!;
 
         private GridChunkBoundsOverlay? _overlay;
 
@@ -40,7 +42,8 @@ public bool Enabled
                         _eyeManager,
                         _mapManager,
                         _transform,
-                        _map);
+                        _map,
+                        _physicsSystem);
 
                     _overlayManager.AddOverlay(_overlay);
                 }
@@ -62,18 +65,20 @@ internal sealed class GridChunkBoundsOverlay : Overlay
         private readonly IMapManager _mapManager;
         private readonly SharedTransformSystem _transformSystem;
         private readonly SharedMapSystem _mapSystem;
+        private readonly SharedPhysicsSystem _physicsSystem;
 
         public override OverlaySpace Space => OverlaySpace.WorldSpace;
 
         private List<Entity<MapGridComponent>> _grids = new();
 
-        public GridChunkBoundsOverlay(IEntityManager entManager, IEyeManager eyeManager, IMapManager mapManager, SharedTransformSystem transformSystem, SharedMapSystem mapSystem)
+        public GridChunkBoundsOverlay(IEntityManager entManager, IEyeManager eyeManager, IMapManager mapManager, SharedTransformSystem transformSystem, SharedMapSystem mapSystem, SharedPhysicsSystem physicsSystem)
         {
             _entityManager = entManager;
             _eyeManager = eyeManager;
             _mapManager = mapManager;
             _transformSystem = transformSystem;
             _mapSystem = mapSystem;
+            _physicsSystem = physicsSystem;
         }
 
         protected internal override void Draw(in OverlayDrawArgs args)
@@ -110,9 +115,9 @@ protected internal override void Draw(in OverlayDrawArgs args)
 
                         worldHandle.DrawPrimitives(DrawPrimitiveTopology.TriangleFan, verts, Color.Green.WithAlpha(0.2f));
 
-                        for (var i = 0; i < fixture.Shape.ChildCount; i++)
+                        for (var i = 0; i < _physicsSystem.GetChildCount(fixture.Shape); i++)
                         {
-                            var aabb = fixture.Shape.ComputeAABB(transform, i);
+                            var aabb = _physicsSystem.ComputeAABB(fixture.Shape, transform, i);
 
                             args.WorldHandle.DrawRect(aabb, Color.Red.WithAlpha(0.5f), false);
                         }

Robust.Client/Physics/PhysicsSystem.Predict.cs

@@ -164,7 +164,7 @@ internal void UpdateIsTouching(List<Contact> toUpdate)
                 continue;
             }
 
-            if (indexA >= fixtureA.Proxies.Length || indexB >= fixtureB.Proxies.Length)
+            if (indexA >= fixtureA.ProxyCount || indexB >= fixtureB.ProxyCount)
                 continue;
 
             var broadphaseA = xformA.Broadphase?.Uid;

Robust.Server.Testing/RobustServerSimulation.cs

@@ -256,7 +256,6 @@ public ISimulation InitializeInstance()
             container.Register<IPrototypeManager, ServerPrototypeManager>();
             container.Register<IComponentFactory, ComponentFactory>();
             container.Register<IEntitySystemManager, EntitySystemManager>();
-            container.Register<IManifoldManager, CollisionManager>();
             container.Register<INetManager, NetManager>();
             container.Register<IAuthManager, AuthManager>();
             container.Register<ITileDefinitionManager, TileDefinitionManager>();

Robust.Shared.IntegrationTests/Map/GridFixtures_Tests.cs

@@ -77,15 +77,15 @@ await server.WaitAssertion(() =>
 
             Assert.That(manager.FixtureCount, Is.EqualTo(1));
             // Also should only be a single tile.
-            var bounds = manager.Fixtures.First().Value.Shape.ComputeAABB(new Transform(Vector2.Zero, (float) Angle.Zero.Theta), 0);
+            var bounds = physSystem.ComputeAABB(manager.Fixtures.First().Value.Shape, new Transform(Vector2.Zero, (float) Angle.Zero.Theta), 0);
             // Poly probably has Box2D's radius added to it so won't be a unit square
             Assert.That(MathHelper.CloseToPercent(Box2.Area(bounds), 1.0f, 0.1f));
 
             // Now do 2 tiles (same chunk)
             mapSystem.SetTile(grid, new Vector2i(0, 1), new Tile(1));
 
             Assert.That(manager.FixtureCount, Is.EqualTo(1));
-            bounds = manager.Fixtures.First().Value.Shape.ComputeAABB(new Transform(Vector2.Zero, (float) Angle.Zero.Theta), 0);
+            bounds = physSystem.ComputeAABB(manager.Fixtures.First().Value.Shape, new Transform(Vector2.Zero, (float) Angle.Zero.Theta), 0);
 
             // Even if we add a new tile old fixture should stay the same if they don't connect.
             Assert.That(MathHelper.CloseToPercent(Box2.Area(bounds), 2.0f, 0.1f));

Robust.Shared.IntegrationTests/Physics/Broadphase_Test.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Numerics;
@@ -144,11 +145,14 @@ public void ReparentBroadphase()
         Assert.That(fixture.ProxyCount, Is.EqualTo(1));
         Assert.That(broadphase.StaticSundriesTree, Does.Not.Contain(ent));
 
-        Assert.That(broadphase.StaticTree.GetProxy(fixture.Proxies[0].ProxyId)!.Equals(fixture.Proxies[0]));
+        var proxy = fixture.Proxies[0];
+        Assert.That(broadphase.StaticTree.GetProxy(proxy.ProxyId)!.Equals(proxy));
 
         // Now check we go to the map's tree correctly.
         xformSys.SetCoordinates(ent, new EntityCoordinates(mapEnt, Vector2.One));
-        Assert.That(entManager.GetComponent<BroadphaseComponent>(mapEnt).StaticTree.GetProxy(fixture.Proxies[0].ProxyId)!.Equals(fixture.Proxies[0]));
+
+        proxy = fixture.Proxies[0];
+        Assert.That(entManager.GetComponent<BroadphaseComponent>(mapEnt).StaticTree.GetProxy(proxy.ProxyId)!.Equals(proxy));
         Assert.That(xform.Broadphase!.Value.Uid.Equals(mapEnt));
     }
 

Robust.Shared.IntegrationTests/Physics/FixtureShape_Test.cs

@@ -34,6 +34,12 @@ public void TestCirclePoint()
             Assert.That(_shapeManager.TestPoint(circle, transform, posC), Is.EqualTo(true));
         }
 
+        [Test]
+        public void TestCircleNegativeRadiusThrows()
+        {
+            Assert.Throws<ArgumentOutOfRangeException>(() => new PhysShapeCircle(-0.5f));
+        }
+
         [Test]
         public void TestEdgePoint()
         {

Robust.Shared.IntegrationTests/Physics/ManifoldManager_Test.cs

@@ -1,19 +1,21 @@
 using System.Numerics;
 using NUnit.Framework;
-using Robust.Shared.IoC;
+using Robust.Shared.GameObjects;
 using Robust.Shared.Maths;
 using Robust.Shared.Physics;
 using Robust.Shared.Physics.Collision;
 using Robust.Shared.Physics.Collision.Shapes;
+using Robust.Shared.Physics.Systems;
 using Robust.Shared.Utility;
+using Robust.UnitTesting.Server;
 
 namespace Robust.UnitTesting.Shared.Physics
 {
     [TestFixture]
-    [TestOf(typeof(IManifoldManager))]
-    internal sealed class ManifoldManager_Test : OurRobustUnitTest
+    [TestOf(typeof(SharedPhysicsSystem))]
+    internal sealed class ManifoldManager_Test
     {
-        private IManifoldManager _manifoldManager = default!;
+        private SharedPhysicsSystem _physics = default!;
 
         private PhysShapeCircle _circleA = default!;
         private PhysShapeCircle _circleB = default!;
@@ -24,7 +26,10 @@ internal sealed class ManifoldManager_Test : OurRobustUnitTest
         [OneTimeSetUp]
         public void Setup()
         {
-            _manifoldManager = new CollisionManager();
+            var sim = RobustServerSimulation
+                .NewSimulation()
+                .InitializeInstance();
+            _physics = sim.Resolve<IEntityManager>().System<SharedPhysicsSystem>();
             _circleA = new PhysShapeCircle(0.5f);
             _circleB = new PhysShapeCircle(0.5f);
             _polyA = new PolygonShape();
@@ -40,15 +45,15 @@ public void TestCircleCollision()
             var transformB = new Transform(Vector2.One, 0f);
 
             // No overlap
-            Assert.That(_manifoldManager.TestOverlap(_circleA, 0, _circleB, 0, transformA, transformB), Is.EqualTo(false));
+            Assert.That(_physics.TestOverlap(_circleA, 0, _circleB, 0, transformA, transformB), Is.EqualTo(false));
 
             // Overlap directly
             transformA = new Transform(transformB.Position, 0f);
-            Assert.That(_manifoldManager.TestOverlap(_circleA, 0, _circleB, 0, transformA, transformB), Is.EqualTo(true));
+            Assert.That(_physics.TestOverlap(_circleA, 0, _circleB, 0, transformA, transformB), Is.EqualTo(true));
 
             // Overlap on edge
             transformA.Position = transformB.Position + new Vector2(0.5f, 0.0f);
-            Assert.That(_manifoldManager.TestOverlap(_circleA, 0, _circleB, 0, transformA, transformB), Is.EqualTo(true));
+            Assert.That(_physics.TestOverlap(_circleA, 0, _circleB, 0, transformA, transformB), Is.EqualTo(true));
         }
 
         [Test]
@@ -58,18 +63,18 @@ public void TestPolyCollisions()
             var transformB = new Transform(Vector2.One, 0f);
 
             // No overlap
-            Assert.That(_manifoldManager.TestOverlap(_polyA, 0, _polyB, 0, transformA, transformB), Is.EqualTo(false));
+            Assert.That(_physics.TestOverlap(_polyA, 0, _polyB, 0, transformA, transformB), Is.EqualTo(false));
 
             // Overlap directly
             transformA = new Transform(transformB.Position, 0f);
-            Assert.That(_manifoldManager.TestOverlap(_polyA, 0, _polyB, 0, transformA, transformB), Is.EqualTo(true));
+            Assert.That(_physics.TestOverlap(_polyA, 0, _polyB, 0, transformA, transformB), Is.EqualTo(true));
 
             // Overlap on edge
             transformA.Position = transformB.Position + new Vector2(0.5f, 0.0f);
-            Assert.That(_manifoldManager.TestOverlap(_polyA, 0, _polyB, 0, transformA, transformB), Is.EqualTo(true));
+            Assert.That(_physics.TestOverlap(_polyA, 0, _polyB, 0, transformA, transformB), Is.EqualTo(true));
 
             transformA.Quaternion2D = transformA.Quaternion2D.Set(45f);
-            Assert.That(_manifoldManager.TestOverlap(_polyA, 0, _polyB, 0, transformA, transformB), Is.EqualTo(true));
+            Assert.That(_physics.TestOverlap(_polyA, 0, _polyB, 0, transformA, transformB), Is.EqualTo(true));
         }
 
         [Test]
@@ -98,7 +103,7 @@ public void TestPolyOnPolyManifolds()
                     }
                 )
             };
-            _manifoldManager.CollidePolygons(ref manifold, _polyA, transformA, _polyB, transformB);
+            _physics.CollidePolygons(ref manifold, _polyA, transformA, _polyB, transformB);
 
             for (var i = 0; i < manifold.PointCount; i++)
             {

Robust.Shared.IntegrationTests/Physics/PhysicsComponent_Test.cs

@@ -4,13 +4,15 @@
 using NUnit.Framework;
 using Robust.Shared.GameObjects;
 using Robust.Shared.IoC;
+using Robust.Shared.Log;
 using Robust.Shared.Map;
 using Robust.Shared.Maths;
 using Robust.Shared.Physics;
 using Robust.Shared.Physics.Collision.Shapes;
 using Robust.Shared.Physics.Components;
 using Robust.Shared.Physics.Dynamics;
 using Robust.Shared.Physics.Systems;
+using Robust.Shared.Utility;
 
 namespace Robust.UnitTesting.Shared.Physics
 {
@@ -56,5 +58,38 @@ await server.WaitAssertion(() =>
                 Assert.That(box.AngularVelocity, Is.Not.EqualTo(0f));
             });
         }
+
+        [Test]
+        public async Task TestShapeRadiusValidation()
+        {
+            var server = StartServer(new ServerIntegrationOptions
+            {
+                FailureLogLevel = LogLevel.Fatal,
+            });
+            await server.WaitIdleAsync();
+
+            var entManager = server.ResolveDependency<IEntityManager>();
+            var fixtureSystem = server.ResolveDependency<IEntitySystemManager>()
+                .GetEntitySystem<FixtureSystem>();
+            var physicsSystem = server.ResolveDependency<IEntitySystemManager>()
+                .GetEntitySystem<SharedPhysicsSystem>();
+
+            await server.WaitAssertion(() =>
+            {
+                entManager.System<SharedMapSystem>().CreateMap(out var mapId);
+                var uid = entManager.SpawnEntity(null, new MapCoordinates(Vector2.Zero, mapId));
+                var body = entManager.AddComponent<PhysicsComponent>(uid);
+                var shape = new PhysShapeCircle(0.5f);
+                var fixture = new Fixture(shape, 0, 0, false);
+                fixtureSystem.CreateFixture(uid, "fix1", fixture, body: body);
+                var fixtures = entManager.GetComponent<FixturesComponent>(uid);
+                var xform = entManager.GetComponent<TransformComponent>(uid);
+
+                Assert.Throws<DebugAssertException>(() =>
+                    physicsSystem.SetRadius((uid, fixtures, body, xform), "fix1", fixture, shape, -0.5f));
+
+                Assert.That(shape.Radius, Is.EqualTo(0.5f));
+            });
+        }
     }
 }

Robust.Shared.IntegrationTests/Physics/ShapeAABB_Test.cs

@@ -5,12 +5,16 @@
 using Robust.Shared.Maths;
 using Robust.Shared.Physics;
 using Robust.Shared.Physics.Collision.Shapes;
+using Robust.Shared.Physics.Systems;
 
 namespace Robust.UnitTesting.Shared.Physics
 {
     [TestFixture]
     internal sealed class ShapeAABB_Test : OurRobustUnitTest
     {
+        private sealed class TestPhysicsSystem : SharedPhysicsSystem;
+
+        private readonly TestPhysicsSystem _physics = new();
         private Transform _transform;
         private Transform _rotatedTransform;
 
@@ -26,7 +30,7 @@ public void Setup()
         public void TestCircleAABB()
         {
             var circle = new PhysShapeCircle(0.5f);
-            var aabb = circle.ComputeAABB(_transform, 0);
+            var aabb = _physics.ComputeAABB(circle, _transform, 0);
             Assert.That(aabb.Width, Is.EqualTo(1f));
             Assert.That(aabb, Is.EqualTo(new Box2(0.5f, 0.5f, 1.5f, 1.5f)));
         }
@@ -35,7 +39,7 @@ public void TestCircleAABB()
         public void TestRotatedCircleAABB()
         {
             var circle = new PhysShapeCircle(0.5f);
-            var aabb = circle.ComputeAABB(_rotatedTransform, 0);
+            var aabb = _physics.ComputeAABB(circle, _rotatedTransform, 0);
             Assert.That(aabb.Width, Is.EqualTo(1f));
             Assert.That(aabb, Is.EqualTo(new Box2(0.5f, 0.5f, 1.5f, 1.5f)));
         }
@@ -44,7 +48,7 @@ public void TestRotatedCircleAABB()
         public void TestEdgeAABB()
         {
             var edge = new EdgeShape(Vector2.Zero, Vector2.One);
-            var aabb = edge.ComputeAABB(_transform, 0);
+            var aabb = _physics.ComputeAABB(edge, _transform, 0);
             Assert.That(aabb.Width, Is.EqualTo(1.02f));
             Assert.That(aabb, Is.EqualTo(new Box2(0.99f, 0.99f, 2.01f, 2.01f)));
         }
@@ -53,7 +57,7 @@ public void TestEdgeAABB()
         public void TestRotatedEdgeAABB()
         {
             var edge = new EdgeShape(Vector2.Zero, Vector2.One);
-            var aabb = edge.ComputeAABB(_rotatedTransform, 0);
+            var aabb = _physics.ComputeAABB(edge, _rotatedTransform, 0);
             Assert.That(MathHelper.CloseToPercent(aabb.Width, 0.02f));
             Assert.That(aabb.EqualsApprox(new Box2(0.99f, 0.99f, 1.01f, 2.42f), 0.01f));
         }
@@ -64,7 +68,7 @@ public void TestPolyAABB()
             var polygon = new PolygonShape();
             // Radius is added to the AABB hence we'll just deduct it here for simplicity
             polygon.SetAsBox(0.49f, 0.49f);
-            var aabb = polygon.ComputeAABB(_transform, 0);
+            var aabb = _physics.ComputeAABB(polygon, _transform, 0);
             Assert.That(aabb.Width, Is.EqualTo(1f));
             Assert.That(aabb, Is.EqualTo(new Box2(0.5f, 0.5f, 1.5f, 1.5f)));
         }
@@ -75,7 +79,7 @@ public void TestRotatedPolyAABB()
             var polygon = new PolygonShape();
             // Radius is added to the AABB hence we'll just deduct it here for simplicity
             polygon.SetAsBox(0.49f, 0.49f);
-            var aabb = polygon.ComputeAABB(_rotatedTransform, 0);
+            var aabb = _physics.ComputeAABB(polygon, _rotatedTransform, 0);
             // I already had a rough idea of what the AABB should be, I just put these in so the test passes.
             Assert.That(aabb.Width, Is.EqualTo(1.40592933f));
             Assert.That(aabb, Is.EqualTo(new Box2(0.29703534f, 0.29703534f, 1.7029647f, 1.7029647f)));