Skip to content

Commit a985c5e

Browse files
ShadowCommanderShadowCommander
authored
Fix ghosts getting spawned in nullspace (space-wizards#27617)
* Add tests for ghost spawn position * Make ghosts spawn immediately * Format mind system * Move ghost spawning to GhostSystem * Spawn ghost on grid or map This fixes the ghosts being attached the parent entity instead of the grid. * Move logging out of the ghost system * Make round start observer spawn using GhostSystem * Move GameTicker ghost spawning to GhostSystem Moved the more robust character name selection code over. Moved the TimeOfDeath code over. Added canReturn logic. * Add overrides and default for ghost spawn coordinates * Add warning log to ghost spawn fail * Clean up test * Dont spawn ghost on map delete * Minor changes to the role test * Fix role test failing to spawn ghost It was failing the map check due to using Nullspace * Fix ghost tests when running in parallel Not sure what happened, but it seems to be because they were running simultaneously and overwriting values. * Clean up ghost tests * Test that map deletion does not spawn ghosts * Spawn ghost on the next available map * Disallow spawning on deleted maps * Fix map deletion ghost test * Cleanup
1 parent 742a1a5 commit a985c5e

File tree

7 files changed

+261
-99
lines changed

7 files changed

+261
-99
lines changed

‎Content.IntegrationTests/Tests/Minds/GhostRoleTests.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#nullable enable
22
using System.Linq;
3+
using Content.IntegrationTests.Pair;
34
using Content.Server.Ghost.Roles;
45
using Content.Server.Ghost.Roles.Components;
56
using Content.Server.Players;
@@ -26,20 +27,23 @@ public sealed class GhostRoleTests
2627
";
2728

2829
/// <summary>
29-
/// This is a simple test that just checks if a player can take a ghost roll and then regain control of their
30+
/// This is a simple test that just checks if a player can take a ghost role and then regain control of their
3031
/// original entity without encountering errors.
3132
/// </summary>
3233
[Test]
3334
public async Task TakeRoleAndReturn()
3435
{
3536
await using var pair = await PoolManager.GetServerClient(new PoolSettings
3637
{
38+
Dirty = true,
3739
DummyTicker = false,
3840
Connected = true
3941
});
4042
var server = pair.Server;
4143
var client = pair.Client;
4244

45+
var mapData = await pair.CreateTestMap();
46+
4347
var entMan = server.ResolveDependency<IEntityManager>();
4448
var sPlayerMan = server.ResolveDependency<Robust.Server.Player.IPlayerManager>();
4549
var conHost = client.ResolveDependency<IConsoleHost>();
@@ -51,7 +55,7 @@ public async Task TakeRoleAndReturn()
5155
EntityUid originalMob = default;
5256
await server.WaitPost(() =>
5357
{
54-
originalMob = entMan.SpawnEntity(null, MapCoordinates.Nullspace);
58+
originalMob = entMan.SpawnEntity(null, mapData.GridCoords);
5559
mindSystem.TransferTo(originalMindId, originalMob, true);
5660
});
5761

@@ -69,12 +73,12 @@ await server.WaitPost(() =>
6973
Assert.That(entMan.HasComponent<GhostComponent>(ghost));
7074
Assert.That(ghost, Is.Not.EqualTo(originalMob));
7175
Assert.That(session.ContentData()?.Mind, Is.EqualTo(originalMindId));
72-
Assert.That(originalMind.OwnedEntity, Is.EqualTo(originalMob));
76+
Assert.That(originalMind.OwnedEntity, Is.EqualTo(originalMob), $"Original mob: {originalMob}, Ghost: {ghost}");
7377
Assert.That(originalMind.VisitingEntity, Is.EqualTo(ghost));
7478

7579
// Spawn ghost takeover entity.
7680
EntityUid ghostRole = default;
77-
await server.WaitPost(() => ghostRole = entMan.SpawnEntity("GhostRoleTestEntity", MapCoordinates.Nullspace));
81+
await server.WaitPost(() => ghostRole = entMan.SpawnEntity("GhostRoleTestEntity", mapData.GridCoords));
7882

7983
// Take the ghost role
8084
await server.WaitPost(() =>

‎Content.IntegrationTests/Tests/Minds/GhostTests.cs‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
using System.Numerics;
2+
using Content.IntegrationTests.Pair;
3+
using Content.Shared.Ghost;
4+
using Content.Shared.Mind;
5+
using Content.Shared.Players;
6+
using Robust.Server.GameObjects;
7+
using Robust.Shared.GameObjects;
8+
using Robust.Shared.Map;
9+
using Robust.Shared.Player;
10+
using Robust.UnitTesting;
11+
12+
namespace Content.IntegrationTests.Tests.Minds;
13+
14+
[TestFixture]
15+
public sealed class GhostTests
16+
{
17+
struct GhostTestData
18+
{
19+
public IEntityManager SEntMan;
20+
public Robust.Server.Player.IPlayerManager SPlayerMan;
21+
public Server.Mind.MindSystem SMindSys;
22+
public SharedTransformSystem STransformSys = default!;
23+
24+
public TestPair Pair = default!;
25+
26+
public TestMapData MapData => Pair.TestMap!;
27+
28+
public RobustIntegrationTest.ServerIntegrationInstance Server => Pair.Server;
29+
public RobustIntegrationTest.ClientIntegrationInstance Client => Pair.Client;
30+
31+
/// <summary>
32+
/// Initial player coordinates. Note that this does not necessarily correspond to the position of the
33+
/// <see cref="Player"/> entity.
34+
/// </summary>
35+
public NetCoordinates PlayerCoords = default!;
36+
37+
public NetEntity Player = default!;
38+
public EntityUid SPlayerEnt = default!;
39+
40+
public ICommonSession ClientSession = default!;
41+
public ICommonSession ServerSession = default!;
42+
43+
public GhostTestData()
44+
{
45+
}
46+
}
47+
48+
private async Task<GhostTestData> SetupData()
49+
{
50+
var data = new GhostTestData();
51+
52+
// Client is needed to create a session for the ghost system. Creating a dummy session was too difficult.
53+
data.Pair = await PoolManager.GetServerClient(new PoolSettings
54+
{
55+
DummyTicker = false,
56+
Connected = true,
57+
Dirty = true
58+
});
59+
60+
data.SEntMan = data.Pair.Server.ResolveDependency<IServerEntityManager>();
61+
data.SPlayerMan = data.Pair.Server.ResolveDependency<Robust.Server.Player.IPlayerManager>();
62+
data.SMindSys = data.SEntMan.System<Server.Mind.MindSystem>();
63+
data.STransformSys = data.SEntMan.System<SharedTransformSystem>();
64+
65+
// Setup map.
66+
await data.Pair.CreateTestMap();
67+
data.PlayerCoords = data.SEntMan.GetNetCoordinates(data.MapData.GridCoords.Offset(new Vector2(0.5f, 0.5f)).WithEntityId(data.MapData.MapUid, data.STransformSys, data.SEntMan));
68+
69+
if (data.Client.Session == null)
70+
Assert.Fail("No player");
71+
data.ClientSession = data.Client.Session!;
72+
data.ServerSession = data.SPlayerMan.GetSessionById(data.ClientSession.UserId);
73+
74+
Entity<MindComponent> mind = default!;
75+
await data.Pair.Server.WaitPost(() =>
76+
{
77+
data.Player = data.SEntMan.GetNetEntity(data.SEntMan.SpawnEntity(null, data.SEntMan.GetCoordinates(data.PlayerCoords)));
78+
mind = data.SMindSys.CreateMind(data.ServerSession.UserId, "DummyPlayerEntity");
79+
data.SPlayerEnt = data.SEntMan.GetEntity(data.Player);
80+
data.SMindSys.TransferTo(mind, data.SPlayerEnt, mind: mind.Comp);
81+
data.Server.PlayerMan.SetAttachedEntity(data.ServerSession, data.SPlayerEnt);
82+
});
83+
84+
await data.Pair.RunTicksSync(5);
85+
86+
Assert.Multiple(() =>
87+
{
88+
Assert.That(data.ServerSession.ContentData()?.Mind, Is.EqualTo(mind.Owner));
89+
Assert.That(data.ServerSession.AttachedEntity, Is.EqualTo(data.SPlayerEnt));
90+
Assert.That(data.ServerSession.AttachedEntity, Is.EqualTo(mind.Comp.CurrentEntity),
91+
"Player is not attached to the mind's current entity.");
92+
Assert.That(data.SEntMan.EntityExists(mind.Comp.OwnedEntity),
93+
"The mind's current entity does not exist");
94+
Assert.That(mind.Comp.VisitingEntity == null || data.SEntMan.EntityExists(mind.Comp.VisitingEntity),
95+
"The minds visited entity does not exist.");
96+
});
97+
98+
Assert.That(data.SPlayerEnt, Is.Not.EqualTo(null));
99+
100+
return data;
101+
}
102+
103+
/// <summary>
104+
/// Test that a ghost gets created when the player entity is deleted.
105+
/// 1. Delete mob
106+
/// 2. Assert is ghost
107+
/// </summary>
108+
[Test]
109+
public async Task TestGridGhostOnDelete()
110+
{
111+
var data = await SetupData();
112+
113+
var oldPosition = data.SEntMan.GetComponent<TransformComponent>(data.SPlayerEnt).Coordinates;
114+
115+
Assert.That(!data.SEntMan.HasComponent<GhostComponent>(data.SPlayerEnt), "Player was initially a ghost?");
116+
117+
// Delete entity
118+
await data.Server.WaitPost(() => data.SEntMan.DeleteEntity(data.SPlayerEnt));
119+
await data.Pair.RunTicksSync(5);
120+
121+
var ghost = data.ServerSession.AttachedEntity!.Value;
122+
Assert.That(data.SEntMan.HasComponent<GhostComponent>(ghost), "Player did not become a ghost");
123+
124+
// Ensure the position is the same
125+
var ghostPosition = data.SEntMan.GetComponent<TransformComponent>(ghost).Coordinates;
126+
Assert.That(ghostPosition, Is.EqualTo(oldPosition));
127+
128+
await data.Pair.CleanReturnAsync();
129+
}
130+
131+
/// <summary>
132+
/// Test that a ghost gets created when the player entity is queue deleted.
133+
/// 1. Delete mob
134+
/// 2. Assert is ghost
135+
/// </summary>
136+
[Test]
137+
public async Task TestGridGhostOnQueueDelete()
138+
{
139+
var data = await SetupData();
140+
141+
var oldPosition = data.SEntMan.GetComponent<TransformComponent>(data.SPlayerEnt).Coordinates;
142+
143+
Assert.That(!data.SEntMan.HasComponent<GhostComponent>(data.SPlayerEnt), "Player was initially a ghost?");
144+
145+
// Delete entity
146+
await data.Server.WaitPost(() => data.SEntMan.QueueDeleteEntity(data.SPlayerEnt));
147+
await data.Pair.RunTicksSync(5);
148+
149+
var ghost = data.ServerSession.AttachedEntity!.Value;
150+
Assert.That(data.SEntMan.HasComponent<GhostComponent>(ghost), "Player did not become a ghost");
151+
152+
// Ensure the position is the same
153+
var ghostPosition = data.SEntMan.GetComponent<TransformComponent>(ghost).Coordinates;
154+
Assert.That(ghostPosition, Is.EqualTo(oldPosition));
155+
156+
await data.Pair.CleanReturnAsync();
157+
}
158+
159+
}

‎Content.IntegrationTests/Tests/Minds/MindTests.EntityDeletion.cs‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
using System.Linq;
23
using Content.Server.GameTicking;
34
using Content.Shared.Ghost;
@@ -77,7 +78,7 @@ public async Task TestGhostOnDeleteMap()
7778
await using var pair = await SetupPair(dirty: true);
7879
var server = pair.Server;
7980
var testMap = await pair.CreateTestMap();
80-
var coordinates = testMap.GridCoords;
81+
var testMap2 = await pair.CreateTestMap();
8182

8283
var entMan = server.ResolveDependency<IServerEntityManager>();
8384
var mapManager = server.ResolveDependency<IMapManager>();
@@ -91,7 +92,7 @@ public async Task TestGhostOnDeleteMap()
9192
MindComponent mind = default!;
9293
await server.WaitAssertion(() =>
9394
{
94-
playerEnt = entMan.SpawnEntity(null, coordinates);
95+
playerEnt = entMan.SpawnEntity(null, testMap.GridCoords);
9596
mindId = player.ContentData()!.Mind!.Value;
9697
mind = entMan.GetComponent<MindComponent>(mindId);
9798
mindSystem.TransferTo(mindId, playerEnt);
@@ -100,14 +101,20 @@ await server.WaitAssertion(() =>
100101
});
101102

102103
await pair.RunTicksSync(5);
103-
await server.WaitPost(() => mapManager.DeleteMap(testMap.MapId));
104+
await server.WaitAssertion(() => mapManager.DeleteMap(testMap.MapId));
104105
await pair.RunTicksSync(5);
105106

106107
await server.WaitAssertion(() =>
107108
{
108109
#pragma warning disable NUnit2045 // Interdependent assertions.
109-
Assert.That(entMan.EntityExists(mind.CurrentEntity), Is.True);
110-
Assert.That(mind.CurrentEntity, Is.Not.EqualTo(playerEnt));
110+
// Spawn ghost on the second map
111+
var attachedEntity = player.AttachedEntity;
112+
Assert.That(entMan.EntityExists(attachedEntity), Is.True);
113+
Assert.That(attachedEntity, Is.Not.EqualTo(playerEnt));
114+
Assert.That(entMan.HasComponent<GhostComponent>(attachedEntity));
115+
var transform = entMan.GetComponent<TransformComponent>(attachedEntity.Value);
116+
Assert.That(transform.MapID, Is.Not.EqualTo(MapId.Nullspace));
117+
Assert.That(transform.MapID, Is.Not.EqualTo(testMap.MapId));
111118
#pragma warning restore NUnit2045
112119
});
113120

‎Content.Server/GameTicking/GameTicker.GamePreset.cs‎

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -274,35 +274,13 @@ public bool OnGhostAttempt(EntityUid mindId, bool canReturnGlobal, bool viaComma
274274
}
275275
}
276276

277-
var xformQuery = GetEntityQuery<TransformComponent>();
278-
var coords = _transform.GetMoverCoordinates(position, xformQuery);
279-
280-
var ghost = Spawn(ObserverPrototypeName, coords);
281-
282-
// Try setting the ghost entity name to either the character name or the player name.
283-
// If all else fails, it'll default to the default entity prototype name, "observer".
284-
// However, that should rarely happen.
285-
if (!string.IsNullOrWhiteSpace(mind.CharacterName))
286-
_metaData.SetEntityName(ghost, mind.CharacterName);
287-
else if (!string.IsNullOrWhiteSpace(mind.Session?.Name))
288-
_metaData.SetEntityName(ghost, mind.Session.Name);
289-
290-
var ghostComponent = Comp<GhostComponent>(ghost);
291-
292-
if (mind.TimeOfDeath.HasValue)
293-
{
294-
_ghost.SetTimeOfDeath(ghost, mind.TimeOfDeath!.Value, ghostComponent);
295-
}
277+
var ghost = _ghost.SpawnGhost((mindId, mind), position, canReturn);
278+
if (ghost == null)
279+
return false;
296280

297281
if (playerEntity != null)
298282
_adminLogger.Add(LogType.Mind, $"{EntityManager.ToPrettyString(playerEntity.Value):player} ghosted{(!canReturn ? " (non-returnable)" : "")}");
299283

300-
_ghost.SetCanReturnToBody(ghostComponent, canReturn);
301-
302-
if (canReturn)
303-
_mind.Visit(mindId, ghost, mind);
304-
else
305-
_mind.TransferTo(mindId, ghost, mind: mind);
306284
return true;
307285
}
308286

0 commit comments

Comments
 (0)