Physics shutdown and other fixing (#4542)

* Did some fixes for simulation world releases

and physics destruction shutdown

* Fixed microbe emission system not running at all

added a world release bug suppression I saw crashing once

* Fixed shutting down world simulation with disabled bodies

* Fixed engulfing system not clearing endosome references correctly, and fixed scale

getting for engulfed microbes

* Turns out cilia animation did work already

adjusted just some TODO comments instead

src/engine/common_systems/FadeOutActionSystem.cs

@@ -133,7 +133,10 @@ private void DisableParticleEmission(Entity entity)
                 var particles = spatial.GraphicalInstance as Particles;
 
                 if (particles == null)
-                    throw new NullReferenceException("Graphical instance casted as particles is null");
+                {
+                    throw new NullReferenceException(
+                        $"Graphical instance ({spatial.GraphicalInstance}) casted as particles is null");
+                }
 
                 particles.Emitting = false;
 

src/engine/common_systems/PhysicsBodyCreationSystem.cs

@@ -18,23 +18,41 @@
     public sealed class PhysicsBodyCreationSystem : AEntitySetSystem<float>
     {
         private readonly IWorldSimulationWithPhysics worldSimulationWithPhysics;
-        private readonly OnBodyDeleted? deleteCallback;
 
         private readonly List<NativePhysicsBody> createdBodies = new();
 
-        public PhysicsBodyCreationSystem(IWorldSimulationWithPhysics worldSimulationWithPhysics,
-            OnBodyDeleted? deleteCallback, World world, IParallelRunner runner) : base(world, runner)
+        public PhysicsBodyCreationSystem(IWorldSimulationWithPhysics worldSimulationWithPhysics, World world,
+            IParallelRunner runner) : base(world, runner)
         {
             this.worldSimulationWithPhysics = worldSimulationWithPhysics;
-            this.deleteCallback = deleteCallback;
         }
 
-        public delegate void OnBodyDeleted(NativePhysicsBody body);
+        /// <summary>
+        ///   Destroys a body collision body immediately. This is needed to be called by the world to ensure that
+        ///   physics bodies of destroyed entities are immediately destroyed
+        /// </summary>
+        public void OnEntityDestroyed(in Entity entity)
+        {
+            if (!entity.Has<Physics>())
+                return;
+
+            ref var physics = ref entity.Get<Physics>();
+
+            if (physics.Body != null)
+            {
+                if (!createdBodies.Remove(physics.Body))
+                    GD.PrintErr("Body creation system told about a destroyed physics body it didn't create");
+
+                worldSimulationWithPhysics.DestroyBody(physics.Body);
+
+                physics.Body = null;
+            }
+        }
 
         protected override void PreUpdate(float delta)
         {
-            // TODO: would it be better to have the world take care of destroying physics bodies when the entity
-            // destruction is triggered?
+            // Immediate body destruction is handled by the world, but we still do this to detect if a physics
+            // component gets removed while things are running
             foreach (var createdBody in createdBodies)
             {
                 createdBody.Marked = false;
@@ -136,9 +154,6 @@ private bool DestroyBodyIfNotMarked(NativePhysicsBody body)
             if (body.Marked)
                 return false;
 
-            // Notify external things about the deleted body
-            deleteCallback?.Invoke(body);
-
             // TODO: ensure this works fine if the body is currently in disabled state
             worldSimulationWithPhysics.DestroyBody(body);
 

src/engine/common_systems/PhysicsBodyDisablingSystem.cs

@@ -3,6 +3,7 @@
     using System;
     using System.Collections.Generic;
     using Components;
+    using DefaultEcs;
     using DefaultEcs.System;
     using Godot;
     using World = DefaultEcs.World;
@@ -22,6 +23,23 @@ public PhysicsBodyDisablingSystem(PhysicalWorld physicalWorld, World world) : ba
             this.physicalWorld = physicalWorld;
         }
 
+        /// <summary>
+        ///   Forgets about a disabled body on entity. Needs to be called before
+        ///   <see cref="PhysicsBodyCreationSystem.OnEntityDestroyed"/> so that this can see the body to be destroyed
+        /// </summary>
+        public void OnEntityDestroyed(in Entity entity)
+        {
+            if (!entity.Has<Physics>())
+                return;
+
+            ref var physics = ref entity.Get<Physics>();
+
+            if (physics.Body != null)
+            {
+                disabledBodies.Remove(physics.Body);
+            }
+        }
+
         // TODO: figure out where this would need to be called
         /// <summary>
         ///   Needs to be called when a body is deleted so that state tracking for body disabling can remove it
@@ -85,11 +103,9 @@ private void Dispose(bool disposing)
         {
             if (disposing)
             {
-                // TODO: would this be needed?
-                foreach (var disabledBody in disabledBodies)
-                {
-                    physicalWorld.DestroyBody(disabledBody);
-                }
+                disabledBodies.Clear();
+
+                // The bodies are destroyed by the creation system / the world. Also see OnEntityDestroyed
             }
         }
     }

src/engine/common_systems/PhysicsCollisionManagementSystem.cs

@@ -27,6 +27,25 @@ public PhysicsCollisionManagementSystem(PhysicalWorld physicalWorld, World world
             this.physicalWorld = physicalWorld;
         }
 
+        /// <summary>
+        ///   Stops collision recording for a removed entity
+        /// </summary>
+        public void OnEntityDestroyed(in Entity entity)
+        {
+            if (!entity.Has<CollisionManagement>())
+                return;
+
+            ref var collisionManagement = ref entity.Get<CollisionManagement>();
+
+            if (collisionManagement.ActiveCollisions != null)
+            {
+                ref var physics = ref entity.Get<Physics>();
+
+                if (physics.Body != null)
+                    physicalWorld.BodyStopCollisionRecording(physics.Body);
+            }
+        }
+
         protected override void Update(float delta, in Entity entity)
         {
             ref var physics = ref entity.Get<Physics>();

src/engine/common_systems/SoundEffectSystem.cs

@@ -52,6 +52,44 @@ public void ReportPlayerPosition(Vector3 position)
             playerPosition = position;
         }
 
+        public void FreeNodeResources()
+        {
+            // Free all used player nodes
+            foreach (var player in freePositionalPlayers)
+            {
+                player.QueueFree();
+            }
+
+            freePositionalPlayers.Clear();
+
+            foreach (var player in usedPositionalPlayers)
+            {
+                player.Player.QueueFree();
+            }
+
+            usedPositionalPlayers.Clear();
+
+            foreach (var player in free2DPlayers)
+            {
+                player.QueueFree();
+            }
+
+            free2DPlayers.Clear();
+
+            foreach (var player in used2DPlayers)
+            {
+                player.Player.QueueFree();
+            }
+
+            used2DPlayers.Clear();
+        }
+
+        public override void Dispose()
+        {
+            Dispose(true);
+            base.Dispose();
+        }
+
         protected override void PreUpdate(float delta)
         {
             base.PreUpdate(delta);
@@ -481,6 +519,15 @@ private void ExpireOldAudioCacheEntries(float delta)
             soundCacheEntriesToClear.Clear();
         }
 
+        private void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                // Free cached sounds immediately
+                soundCache.Clear();
+            }
+        }
+
         /// <summary>
         ///   This and derived classes include extra info that is needed on a currently playing audio player for this
         ///   system

src/engine/common_systems/SpatialAttachSystem.cs

@@ -23,6 +23,16 @@ public SpatialAttachSystem(Node godotWorldRoot, World world) : base(world, null)
             this.godotWorldRoot = godotWorldRoot;
         }
 
+        public void FreeNodeResources()
+        {
+            foreach (var entry in attachedSpatialInstances)
+            {
+                entry.Key.QueueFree();
+            }
+
+            attachedSpatialInstances.Clear();
+        }
+
         protected override void PreUpdate(float state)
         {
             // Unmark all

src/engine/physics/NativePhysicsBody.cs

@@ -33,7 +33,7 @@ public class NativePhysicsBody : IDisposable, IEquatable<NativePhysicsBody>
 
     /// <summary>
     ///   Storage variable for collision recording, when this is active the pin handle is used to pin down this
-    ///   piece of memory to ensure the native code size can directly write here with pointers
+    ///   piece of memory to ensure the native code side can directly write here with pointers
     /// </summary>
     private PhysicsCollision[]? activeCollisions;
 
@@ -60,7 +60,8 @@ internal NativePhysicsBody(IntPtr nativeInstance)
 
     /// <summary>
     ///   Active collisions for this body. Only updated if started through
-    ///   <see cref="PhysicalWorld.BodyStartCollisionRecording"/>
+    ///   <see cref="PhysicalWorld.BodyStartCollisionRecording"/>. For entities prefer
+    ///   <see cref="Components.CollisionManagementHelpers.GetActiveCollisions"/>
     /// </summary>
     public PhysicsCollision[]? ActiveCollisions => activeCollisions;
 
@@ -69,6 +70,8 @@ internal NativePhysicsBody(IntPtr nativeInstance)
     /// </summary>
     public bool MicrobeControlEnabled { get; set; }
 
+    public bool IsDisposed => disposed;
+
     public static bool operator ==(NativePhysicsBody? left, NativePhysicsBody? right)
     {
         return Equals(left, right);

src/general/base_stage/WorldSimulation.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using DefaultEcs;
 using DefaultEcs.Command;
 using Godot;
@@ -30,11 +31,6 @@ public abstract class WorldSimulation : IWorldSimulation
 
     protected float accumulatedLogicTime;
 
-    /// <summary>
-    ///   True when multithreaded system updates are running
-    /// </summary>
-    protected bool runningMultithreaded;
-
     // TODO: implement saving
     // ReSharper disable once CollectionNeverQueried.Local
     private readonly List<Entity> entitiesToNotSave = new();
@@ -172,10 +168,10 @@ public virtual bool ProcessLogic(float delta)
     public Entity CreateEmptyEntity()
     {
         // Ensure thread unsafe operation doesn't happen
-        if (runningMultithreaded)
+        if (Processing)
         {
             throw new InvalidOperationException(
-                "Can't use thread unsafe create entity at this time, use deferred create");
+                "Can't use entity create at this time, use deferred create");
         }
 
         return entities.CreateEntity();
@@ -202,13 +198,12 @@ public void DestroyAllEntities(Entity? skip = null)
     {
         ProcessDestroyQueue();
 
-        foreach (var entity in entities)
+        // If destroy all is used a lot then this temporary memory use (ToList) here should be solved
+        foreach (var entity in entities.ToList())
         {
             if (entity == skip)
                 continue;
 
-            // TODO: check that this destroy while looping entities doesn't cause an issue
-
             PerformEntityDestroy(entity);
         }
 
@@ -330,6 +325,14 @@ public void SetLogicMaxUpdateRate(float logicFPS)
         minimumTimeBetweenLogicUpdates = 1 / logicFPS;
     }
 
+    public virtual void FreeNodeResources()
+    {
+    }
+
+    /// <summary>
+    ///   Note that often when this is disposed, the Nodes are already disposed so this has to skip releasing them.
+    ///   If that is not the case it is required to call <see cref="FreeNodeResources"/> before calling Dispose.
+    /// </summary>
     public void Dispose()
     {
         Dispose(true);
@@ -383,10 +386,26 @@ protected void PerformEntityDestroy(Entity entity)
     {
         entitiesToNotSave.Remove(entity);
 
+        OnEntityDestroyed(entity);
+
         // Destroy the entity from the ECS system
         entity.Dispose();
     }
 
+    /// <summary>
+    ///   Called when an entity is being destroyed (but before it is). Used by derived worlds to for example delete
+    ///   physics data.
+    /// </summary>
+    /// <remarks>
+    ///   <para>
+    ///     Note that when the entire world is disposed this is not called for each entity.
+    ///   </para>
+    /// </remarks>
+    /// <param name="entity">The entity that is being destroyed</param>
+    protected virtual void OnEntityDestroyed(in Entity entity)
+    {
+    }
+
     protected void ThrowIfNotInitialized()
     {
         if (!Initialized)
@@ -395,7 +414,10 @@ protected void ThrowIfNotInitialized()
 
     protected virtual void Dispose(bool disposing)
     {
-        DestroyAllEntities();
+        // TODO: decide if destroying all entities on world destroy is needed. It seems that disposing systems can
+        // release their allocated resources so this all can just be let to go for memory to be garbage collected later
+        // for faster world destroy
+        // DestroyAllEntities();
 
         if (disposing)
         {