Fix station deletion bug (#10348)

Content.Client/LateJoin/LateJoinGui.cs

@@ -64,6 +64,10 @@ namespace Content.Client.LateJoin
             _jobCategories.Clear();
 
             var gameTicker = EntitySystem.Get<ClientGameTicker>();
+
+            if (!gameTicker.DisallowedLateJoin && gameTicker.StationNames.Count == 0)
+                Logger.Warning("No stations exist, nothing to display in late-join GUI");
+
             foreach (var (id, name) in gameTicker.StationNames)
             {
                 var jobList = new BoxContainer

Content.IntegrationTests/Tests/EntityTest.cs

@@ -101,6 +101,7 @@ namespace Content.IntegrationTests.Tests
                 "DebugExceptionStartup",
                 "Map", // We aren't testing a map entity in this test
                 "MapGrid",
+                "StationData", // errors when removed mid-round
                 "Actor", // We aren't testing actor components, those need their player session set.
             };
 
@@ -195,6 +196,7 @@ namespace Content.IntegrationTests.Tests
                 "DebugExceptionStartup",
                 "Map", // We aren't testing a map entity in this test
                 "MapGrid",
+                "StationData", // errors when deleted mid-round
                 "Actor", // We aren't testing actor components, those need their player session set.
             };
 

Content.IntegrationTests/Tests/FollowerSystemTest.cs

@@ -43,22 +43,7 @@ public sealed class FollowerSystemTest
 
             followerSystem.StartFollowingEntity(follower, followed);
 
-            foreach (var ent in entMan.GetEntities().ToArray())
+            entMan.DeleteEntity(mapMan.GetMapEntityId(map));
-            {
-                // Let's skip entities that have been deleted, as we want to get their TransformComp for extra info.
-                if (entMan.Deleted(ent))
-                {
-                    logger.Info($"Skipping {entMan.ToPrettyString(ent)}...");
-                    continue;
-                }
-
-                // Log some information about the entity before we delete it.
-                var transform = entMan.GetComponent<TransformComponent>(ent);
-                logger.Info($"Deleting entity {entMan.ToPrettyString(ent)}... Parent: {entMan.ToPrettyString(transform.ParentUid)} | Children: {string.Join(", ", transform.Children.Select(c => entMan.ToPrettyString(c.Owner)))}");
-
-                // Actually delete the entity now.
-                entMan.DeleteEntity(ent);
-            }
         });
         await pairTracker.CleanReturnAsync();
     }

Content.Server/Singularity/EntitySystems/SingularitySystem.cs

@@ -1,5 +1,6 @@
 using Content.Server.Ghost.Components;
 using Content.Server.Singularity.Components;
+using Content.Server.Station.Components;
 using Content.Shared.Singularity;
 using Content.Shared.Singularity.Components;
 using JetBrains.Annotations;
@@ -130,6 +131,7 @@ namespace Content.Server.Singularity.EntitySystems
             return entity != component.Owner &&
                    !EntityManager.HasComponent<IMapGridComponent>(entity) &&
                    !EntityManager.HasComponent<GhostComponent>(entity) &&
+                   !EntityManager.HasComponent<StationDataComponent>(entity) && // these SHOULD be in null-space... but just in case. Also, maybe someone moves a singularity there.. 
                    (component.Level > 4 ||
                    !EntityManager.HasComponent<ContainmentFieldComponent>(entity) &&
                    !EntityManager.HasComponent<ContainmentFieldGeneratorComponent>(entity));

Content.Server/Station/Systems/StationSystem.cs

@@ -25,7 +25,6 @@ namespace Content.Server.Station.Systems;
 [PublicAPI]
 public sealed class StationSystem : EntitySystem
 {
-    [Dependency] private readonly IChatManager _chatManager = default!;
     [Dependency] private readonly IConfigurationManager _configurationManager = default!;
     [Dependency] private readonly ILogManager _logManager = default!;
     [Dependency] private readonly IMapManager _mapManager = default!;
@@ -59,8 +58,9 @@ public sealed class StationSystem : EntitySystem
         SubscribeLocalEvent<GameRunLevelChangedEvent>(OnRoundEnd);
         SubscribeLocalEvent<PreGameMapLoad>(OnPreGameMapLoad);
         SubscribeLocalEvent<PostGameMapLoad>(OnPostGameMapLoad);
-        SubscribeLocalEvent<StationDataComponent, ComponentAdd>(OnStationStartup);
+        SubscribeLocalEvent<StationDataComponent, ComponentAdd>(OnStationAdd);
         SubscribeLocalEvent<StationDataComponent, ComponentShutdown>(OnStationDeleted);
+        SubscribeLocalEvent<StationDataComponent, EntParentChangedMessage>(OnParentChanged);
 
         _configurationManager.OnValueChanged(CCVars.StationOffset, x => _randomStationOffset = x, true);
         _configurationManager.OnValueChanged(CCVars.MaxStationOffset, x => _maxRandomStationOffset = x, true);
@@ -85,7 +85,7 @@ public sealed class StationSystem : EntitySystem
 
     #region Event handlers
 
-    private void OnStationStartup(EntityUid uid, StationDataComponent component, ComponentAdd args)
+    private void OnStationAdd(EntityUid uid, StationDataComponent component, ComponentAdd args)
     {
         _stations.Add(uid);
 
@@ -94,11 +94,36 @@ public sealed class StationSystem : EntitySystem
 
     private void OnStationDeleted(EntityUid uid, StationDataComponent component, ComponentShutdown args)
     {
-        _stations.Remove(uid);
+        if (_stations.Contains(uid) && // Was not deleted via DeleteStation() 
+            _gameTicker.RunLevel == GameRunLevel.InRound) // And not due to a round restart
+        {
+            throw new InvalidOperationException($"Station entity {ToPrettyString(uid)} is getting deleted mid-round.");
+        }
 
+        _stations.Remove(uid);
         RaiseNetworkEvent(new StationsUpdatedEvent(_stations), Filter.Broadcast());
     }
 
+    /// <summary>
+    ///     If a station data entity is getting re-parented mid-round, this will log an error.
+    /// </summary>
+    /// <remarks>
+    ///     This doesn't really achieve anything, it just for debugging any future station data bugs.
+    /// </remarks>
+    private void OnParentChanged(EntityUid uid, StationDataComponent component, ref EntParentChangedMessage args)
+    {
+        if (_gameTicker.RunLevel != GameRunLevel.InRound ||
+            MetaData(uid).EntityLifeStage >= EntityLifeStage.MapInitialized ||
+            component.LifeStage <= ComponentLifeStage.Initializing)
+        {
+            return;
+        }
+
+        // Yeah this doesn't actually stop the parent change..... it just ineffectually yells about it.
+        // STOP RIGHT THERE CRIMINAL SCUM
+        _sawmill.Error($"Station entity {ToPrettyString(uid)} is getting reparented from {ToPrettyString(args.OldParent ?? EntityUid.Invalid)} to {ToPrettyString(args.Transform.ParentUid)}");
+    }
+
     private void OnPreGameMapLoad(PreGameMapLoad ev)
     {
         // this is only for maps loaded during round setup!
@@ -261,8 +286,12 @@ public sealed class StationSystem : EntitySystem
     /// <returns>The initialized station.</returns>
     public EntityUid InitializeNewStation(StationConfig? stationConfig, IEnumerable<EntityUid>? gridIds, string? name = null)
     {
-        //HACK: This needs to go in null-space but that crashes currently.
         var station = Spawn(null, new MapCoordinates(0, 0, _gameTicker.DefaultMap));
+
+        // TODO SERIALIZATION The station data needs to be saveable somehow, but when a map gets saved, this entity
+        // won't be included because its in null-space. Also, what happens to shuttles on other maps?
+        _transform.DetachParentToNull(Transform(station));
+
         var data = AddComp<StationDataComponent>(station);
         var metaData = MetaData(station);
         data.StationConfig = stationConfig;
@@ -374,6 +403,7 @@ public sealed class StationSystem : EntitySystem
         if (!Resolve(station, ref stationData))
             throw new ArgumentException("Tried to use a non-station entity as a station!", nameof(station));
 
+        // component shutdown will error if the station was not removed from _stations prior to deletion.
         _stations.Remove(station);
         Del(station);
     }