From 1e1f7748fed5226b200fbb31160a1ae886ee50e6 Mon Sep 17 00:00:00 2001 From: Princess Cheeseballs <66055347+Princess-Cheeseballs@users.noreply.github.com> Date: Fri, 18 Sep 2026 04:12:59 +0000 Subject: [PATCH] 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? --- .../Minds/MindTest.DeleteAllThenGhost.cs | 54 ------------------- .../Tests/SaveLoadSaveTest.cs | 12 +++++ .../GameTicking/ServerGameTicker.Spawning.cs | 10 ++-- Content.Server/Ghost/GhostSystem.cs | 35 +++++------- Content.Server/Mind/MindSystem.cs | 19 +++---- 5 files changed, 36 insertions(+), 94 deletions(-) delete mode 100644 Content.IntegrationTests/Tests/Minds/MindTest.DeleteAllThenGhost.cs diff --git a/Content.IntegrationTests/Tests/Minds/MindTest.DeleteAllThenGhost.cs b/Content.IntegrationTests/Tests/Minds/MindTest.DeleteAllThenGhost.cs deleted file mode 100644 index 5ebced5bd6..0000000000 --- a/Content.IntegrationTests/Tests/Minds/MindTest.DeleteAllThenGhost.cs +++ /dev/null @@ -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(); - 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().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(); - 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)); - } -} diff --git a/Content.IntegrationTests/Tests/SaveLoadSaveTest.cs b/Content.IntegrationTests/Tests/SaveLoadSaveTest.cs index 7dfc2ffc1e..2506a0e5ba 100644 --- a/Content.IntegrationTests/Tests/SaveLoadSaveTest.cs +++ b/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"); } diff --git a/Content.Server/GameTicking/ServerGameTicker.Spawning.cs b/Content.Server/GameTicking/ServerGameTicker.Spawning.cs index 67d79be7a1..b60e90f7e4 100644 --- a/Content.Server/GameTicking/ServerGameTicker.Spawning.cs +++ b/Content.Server/GameTicking/ServerGameTicker.Spawning.cs @@ -432,10 +432,9 @@ namespace Content.Server.GameTicking { _possiblePositions.Clear(); var spawnPointQuery = EntityQueryEnumerator(); - 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(); 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))); } diff --git a/Content.Server/Ghost/GhostSystem.cs b/Content.Server/Ghost/GhostSystem.cs index 3c9d94c827..a507059d04 100644 --- a/Content.Server/Ghost/GhostSystem.cs +++ b/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 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); diff --git a/Content.Server/Mind/MindSystem.cs b/Content.Server/Mind/MindSystem.cs index 826cbe9b2a..fec212ac02 100644 --- a/Content.Server/Mind/MindSystem.cs +++ b/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(OnMindContainerTerminating); - SubscribeLocalEvent(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(entity.Value);