Fix ancient 2022 ghost test failures (#46060)

* fix ancient 2022 ghost test failures

* remove my logs

* update the comment to be accurate

* remove braces

* better loggeewafsafasafsafsfasfasfasafs

* What does bro even do?

Content.IntegrationTests/Tests/Minds/MindTest.DeleteAllThenGhost.cs

@@ -1,54 +0,0 @@
-#nullable enable
-using Robust.Shared.Console;
-using Robust.Shared.GameObjects;
-using Robust.Shared.Map;
-
-namespace Content.IntegrationTests.Tests.Minds;
-
-[TestFixture]
-public sealed partial class MindTests
-{
-    [Test]
-    public async Task DeleteAllThenGhost()
-    {
-        var pair = Pair;
-
-        // Client is connected with a valid entity & mind
-        Assert.That(pair.Client.EntMan.EntityExists(pair.Client.AttachedEntity));
-        Assert.That(pair.Server.EntMan.EntityExists(pair.PlayerData?.Mind));
-
-        // Delete **everything**
-        var conHost = pair.Server.ResolveDependency<IConsoleHost>();
-        await pair.Server.WaitPost(() => conHost.ExecuteCommand("entities delete"));
-        await pair.RunTicksSync(5);
-
-        Assert.That(pair.Server.EntMan.EntityCount, Is.EqualTo(0));
-
-        foreach (var ent in pair.Client.EntMan.GetEntities())
-        {
-            Console.WriteLine(pair.Client.EntMan.ToPrettyString(ent));
-        }
-
-        Assert.That(pair.Client.EntMan.EntityCount, Is.EqualTo(0));
-
-        // Create a new map.
-        MapId mapId = default;
-        await pair.Server.WaitPost(() => pair.Server.System<SharedMapSystem>().CreateMap(out mapId));
-        await pair.RunTicksSync(5);
-
-        // Client is not attached to anything
-        Assert.That(pair.Client.AttachedEntity, Is.Null);
-        Assert.That(pair.PlayerData?.Mind, Is.Null);
-
-        // Attempt to ghost
-        var cConHost = pair.Client.ResolveDependency<IConsoleHost>();
-        await pair.Client.WaitPost(() => cConHost.ExecuteCommand("ghost"));
-        await pair.RunTicksSync(10);
-
-        // Client should be attached to a ghost placed on the new map.
-        Assert.That(pair.Client.EntMan.EntityExists(pair.Client.AttachedEntity));
-        Assert.That(pair.Server.EntMan.EntityExists(pair.PlayerData?.Mind));
-        var xform = pair.Client.Transform(pair.Client.AttachedEntity!.Value);
-        Assert.That(xform.MapID, Is.EqualTo(mapId));
-    }
-}

Content.IntegrationTests/Tests/SaveLoadSaveTest.cs

@@ -94,6 +94,10 @@ namespace Content.IntegrationTests.Tests
                 mapSystem.DeleteMap(mapId0);
                 mapSystem.DeleteMap(mapId1);
             });
+            foreach (var ent in SEntMan.GetEntities())
+            {
+                Console.WriteLine(SEntMan.ToPrettyString(ent));
+            }
             Assert.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the end of CreateSaveLoadSaveGrid");
         }
 
@@ -177,6 +181,10 @@ namespace Content.IntegrationTests.Tests
 
             testSystem.Enabled = false;
             await server.WaitPost(() => mapSys.DeleteMap(mapId));
+            foreach (var ent in SEntMan.GetEntities())
+            {
+                Console.WriteLine(SEntMan.ToPrettyString(ent));
+            }
             Assert.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the end of LoadSaveTicksSaveBagel");
         }
 
@@ -253,6 +261,10 @@ namespace Content.IntegrationTests.Tests
                 mapSys.DeleteMap(mapId1);
                 mapSys.DeleteMap(mapId2);
             });
+            foreach (var ent in SEntMan.GetEntities())
+            {
+                Console.WriteLine(SEntMan.ToPrettyString(ent));
+            }
             Assert.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the end of LoadTickLoadBagel");
         }
 

Content.Server/GameTicking/ServerGameTicker.Spawning.cs

@@ -432,10 +432,9 @@ namespace Content.Server.GameTicking
         {
             _possiblePositions.Clear();
             var spawnPointQuery = EntityQueryEnumerator<SpawnPointComponent, TransformComponent>();
-            while (spawnPointQuery.MoveNext(out var uid, out var point, out var transform))
+            while (spawnPointQuery.MoveNext(out var point, out var transform))
             {
                 if (point.SpawnType != SpawnPointType.Observer
-                   || TerminatingOrDeleted(uid)
                    || transform.MapUid == null
                    || TerminatingOrDeleted(transform.MapUid.Value))
                 {
@@ -451,7 +450,9 @@ namespace Content.Server.GameTicking
                 var query = EntityQueryEnumerator<MapGridComponent>();
                 while (query.MoveNext(out var uid, out _))
                 {
-                    _possiblePositions.Add(new EntityCoordinates(uid, Vector2.Zero));
+                    // Band-aid fix cause we aren't queueing observer re-attach.
+                    if (!TerminatingOrDeleted(uid))
+                        _possiblePositions.Add(new EntityCoordinates(uid, Vector2.Zero));
                 }
             }
 
@@ -463,10 +464,9 @@ namespace Content.Server.GameTicking
                 var spawn = Random.Pick(_possiblePositions);
                 var toMap = XForm.ToMapCoordinates(spawn);
 
-                if (Map.TryFindGridAt(toMap, out var gridUid, out _))
+                if (Map.TryFindGridAt(toMap, out var gridUid, out _) && !TerminatingOrDeleted(gridUid))
                 {
                     var gridXform = Transform(gridUid);
-
                     return new EntityCoordinates(gridUid, Vector2.Transform(toMap.Position, XForm.GetInvWorldMatrix(gridXform)));
                 }
 

Content.Server/Ghost/GhostSystem.cs

@@ -470,16 +470,8 @@ namespace Content.Server.Ghost
             if (spawnPosition?.IsValid(EntityManager) != true)
                 return false;
 
-            var mapUid = _transformSystem.GetMap(spawnPosition.Value);
-            var gridUid = spawnPosition?.EntityId;
-            // Test if the map is being deleted
-            if (mapUid == null || TerminatingOrDeleted(mapUid.Value))
-                return false;
-            // Test if the grid is being deleted
-            if (gridUid != null && TerminatingOrDeleted(gridUid.Value))
-                return false;
-
-            return true;
+            // Test if the parent is being deleted
+            return !TerminatingOrDeleted(spawnPosition.Value.EntityId);
         }
 
         public EntityUid? SpawnGhost(Entity<MindComponent?> mind, EntityCoordinates? spawnPosition = null,
@@ -489,19 +481,18 @@ namespace Content.Server.Ghost
                 return null;
 
             // Test if the map or grid is being deleted
-            if (!IsValidSpawnPosition(spawnPosition))
-                spawnPosition = null;
-
-            // If it's bad, look for a valid point to spawn
-            spawnPosition ??= _gameTicker.GetObserverSpawnPoint();
-
-            // Make sure the new point is valid too
-            if (!IsValidSpawnPosition(spawnPosition))
+            if (spawnPosition == null || !IsValidSpawnPosition(spawnPosition))
             {
-                Log.Warning($"No spawn valid ghost spawn position found for {mind.Comp.CharacterName}"
-                    + $" \"{ToPrettyString(mind)}\"");
-                _minds.TransferTo(mind.Owner, null, createGhost: false, mind: mind.Comp);
-                return null;
+                // If it's bad, look for a valid point to spawn
+                spawnPosition = _gameTicker.GetObserverSpawnPoint();
+
+                // Make sure the new point is valid too
+                if (!IsValidSpawnPosition(spawnPosition))
+                {
+                    Log.Error($"Fallback spawn position: {spawnPosition} provided for {mind.Comp.CharacterName} {ToPrettyString(mind)} was not valid.");
+                    _minds.TransferTo(mind.Owner, null, createGhost: false, mind: mind.Comp);
+                    return null;
+                }
             }
 
             var ghost = SpawnAtPosition(ServerGameTicker.ObserverPrototypeName, spawnPosition.Value);

Content.Server/Mind/MindSystem.cs

@@ -25,14 +25,7 @@ public sealed partial class MindSystem : SharedMindSystem
     [Dependency] private SharedTransformSystem _transform = default!;
     [Dependency] private PvsOverrideSystem _pvsOverride = default!;
 
-    public override void Initialize()
-    {
-        base.Initialize();
-
-        SubscribeLocalEvent<MindContainerComponent, EntityTerminatingEvent>(OnMindContainerTerminating);
-        SubscribeLocalEvent<MindComponent, ComponentShutdown>(OnMindShutdown);
-    }
-
+    [SubscribeLocalEvent]
     private void OnMindShutdown(EntityUid uid, MindComponent mind, ComponentShutdown args)
     {
         if (mind.UserId is {} user)
@@ -49,6 +42,8 @@ public sealed partial class MindSystem : SharedMindSystem
         mind.OwnedEntity = null;
     }
 
+    // TODO: This should not run on EntityTerminatingEvent, instead we should detach the mind and then queue transferring it.
+    [SubscribeLocalEvent]
     private void OnMindContainerTerminating(EntityUid uid, MindContainerComponent component, ref EntityTerminatingEvent args)
     {
         if (!TryGetMind(uid, out var mindId, out var mind, component))
@@ -57,8 +52,7 @@ public sealed partial class MindSystem : SharedMindSystem
         // If the player is currently visiting some other entity, simply attach to that entity.
         if (mind.VisitingEntity is {Valid: true} visiting
             && visiting != uid
-            && !Deleted(visiting)
-            && !Terminating(visiting))
+            && !TerminatingOrDeleted(visiting))
         {
             TransferTo(mindId, visiting, mind: mind);
             if (TryComp(visiting, out GhostComponent? ghostComp))
@@ -77,8 +71,7 @@ public sealed partial class MindSystem : SharedMindSystem
             // Log these to make sure they're not causing the GameTicker round restart bugs...
             Log.Debug($"Entity \"{ToPrettyString(uid)}\" for {mind.CharacterName} was deleted, spawned \"{ToPrettyString(ghost)}\".");
         else
-            // This should be an error, if it didn't cause tests to start erroring when they delete a player.
-            Log.Warning($"Entity \"{ToPrettyString(uid)}\" for {mind.CharacterName} was deleted, and no applicable spawn location is available.");
+            Log.Error($"Entity \"{ToPrettyString(uid)}\" for {mind.CharacterName} was deleted, and no applicable spawn location is available.");
     }
 
     public override bool TryGetMind(NetUserId user, [NotNullWhen(true)] out EntityUid? mindId, [NotNullWhen(true)] out MindComponent? mind)
@@ -181,7 +174,7 @@ public sealed partial class MindSystem : SharedMindSystem
         MindContainerComponent? component = null;
         var alreadyAttached = false;
 
-        if (entity != null)
+        if (entity != null && !TerminatingOrDeleted(entity))
         {
             component = EnsureComp<MindContainerComponent>(entity.Value);