From 6a45d36457d1b8a18ae38f50c77484644650f26e Mon Sep 17 00:00:00 2001 From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com> Date: Mon, 7 Aug 2023 15:29:10 +1200 Subject: [PATCH] Fix mind test issues (#18793) --- Content.IntegrationTests/PoolManager.cs | 30 ++++++++++--- .../Tests/Minds/GhostRoleTests.cs | 43 +++++++++++++++---- .../Tests/Minds/MindTests.EntityDeletion.cs | 2 +- .../Tests/Minds/MindTests.Helpers.cs | 5 ++- .../Tests/Minds/MindTests.cs | 21 +++++---- Content.Server/Mind/MindSystem.cs | 17 +++++--- 6 files changed, 85 insertions(+), 33 deletions(-) diff --git a/Content.IntegrationTests/PoolManager.cs b/Content.IntegrationTests/PoolManager.cs index 15e02b34db..64f94e136b 100644 --- a/Content.IntegrationTests/PoolManager.cs +++ b/Content.IntegrationTests/PoolManager.cs @@ -12,6 +12,7 @@ using Content.IntegrationTests.Tests.Destructible; using Content.IntegrationTests.Tests.DeviceNetwork; using Content.IntegrationTests.Tests.Interaction.Click; using Content.Server.GameTicking; +using Content.Server.Mind.Components; using Content.Shared.CCVar; using Content.Shared.GameTicking; using Robust.Client; @@ -298,7 +299,7 @@ public static partial class PoolManager // Newly created pairs should always be in a valid state. await RunTicksSync(pair, 5); await SyncTicks(pair, targetDelta: 1); - ValidateFastRecycle(pair, poolSettings); + ValidatePair(pair, poolSettings); } else { @@ -326,7 +327,7 @@ public static partial class PoolManager await RunTicksSync(pair, 5); await SyncTicks(pair, targetDelta: 1); - ValidateFastRecycle(pair, poolSettings); + ValidatePair(pair, poolSettings); } else { @@ -365,14 +366,15 @@ public static partial class PoolManager }; } - private static void ValidateFastRecycle(Pair pair, PoolSettings settings) + private static void ValidatePair(Pair pair, PoolSettings settings) { var cfg = pair.Server.ResolveDependency(); Assert.That(cfg.GetCVar(CCVars.AdminLogsEnabled), Is.EqualTo(settings.AdminLogsEnabled)); Assert.That(cfg.GetCVar(CCVars.GameLobbyEnabled), Is.EqualTo(settings.InLobby)); Assert.That(cfg.GetCVar(CCVars.GameDummyTicker), Is.EqualTo(settings.UseDummyTicker)); - var ticker = pair.Server.ResolveDependency().System(); + var entMan = pair.Server.ResolveDependency(); + var ticker = entMan.System(); Assert.That(ticker.DummyTicker, Is.EqualTo(settings.UseDummyTicker)); var expectPreRound = settings.InLobby | settings.DummyTicker; @@ -390,17 +392,33 @@ public static partial class PoolManager var cPlayer = pair.Client.ResolveDependency(); var sPlayer = pair.Server.ResolveDependency(); Assert.That(sPlayer.Sessions.Count(), Is.EqualTo(1)); - Assert.That(cPlayer.LocalPlayer?.Session.UserId, Is.EqualTo(sPlayer.Sessions.Single().UserId)); + var session = sPlayer.Sessions.Single(); + Assert.That(cPlayer.LocalPlayer?.Session.UserId, Is.EqualTo(session.UserId)); if (ticker.DummyTicker) return; - var status = ticker.PlayerGameStatuses[sPlayer.Sessions.Single().UserId]; + var status = ticker.PlayerGameStatuses[session.UserId]; var expected = settings.InLobby ? PlayerGameStatus.NotReadyToPlay : PlayerGameStatus.JoinedGame; Assert.That(status, Is.EqualTo(expected)); + + if (settings.InLobby) + { + Assert.Null(session.AttachedEntity); + return; + } + + Assert.NotNull(session.AttachedEntity); + Assert.That(entMan.EntityExists(session.AttachedEntity)); + Assert.That(entMan.HasComponent(session.AttachedEntity)); + var mindCont = entMan.GetComponent(session.AttachedEntity!.Value); + Assert.NotNull(mindCont.Mind); + Assert.Null(mindCont.Mind?.VisitingEntity); + Assert.That(mindCont.Mind!.OwnedEntity, Is.EqualTo(session.AttachedEntity!.Value)); + Assert.That(mindCont.Mind.UserId, Is.EqualTo(session.UserId)); } private static Pair? GrabOptimalPair(PoolSettings poolSettings) diff --git a/Content.IntegrationTests/Tests/Minds/GhostRoleTests.cs b/Content.IntegrationTests/Tests/Minds/GhostRoleTests.cs index 5d702ceef5..7fb56ba1aa 100644 --- a/Content.IntegrationTests/Tests/Minds/GhostRoleTests.cs +++ b/Content.IntegrationTests/Tests/Minds/GhostRoleTests.cs @@ -1,5 +1,6 @@ #nullable enable using System.Linq; +using Content.Server.Ghost.Components; using Content.Server.Ghost.Roles; using Content.Server.Ghost.Roles.Components; using Content.Server.Mind; @@ -43,23 +44,31 @@ public sealed class GhostRoleTests var conHost = client.ResolveDependency(); var mindSystem = entMan.System(); var session = sPlayerMan.ServerSessions.Single(); + var originalMind = session.ContentData()!.Mind!; // Spawn player entity & attach EntityUid originalMob = default; await server.WaitPost(() => { originalMob = entMan.SpawnEntity(null, MapCoordinates.Nullspace); - mindSystem.TransferTo(session.ContentData()!.Mind!, originalMob, true); + mindSystem.TransferTo(originalMind, originalMob, true); }); // Check player got attached. await PoolManager.RunTicksSync(pairTracker.Pair, 10); Assert.That(session.AttachedEntity, Is.EqualTo(originalMob)); + Assert.That(originalMind.OwnedEntity, Is.EqualTo(originalMob)); + Assert.Null(originalMind.VisitingEntity); // Use the ghost command conHost.ExecuteCommand("ghost"); await PoolManager.RunTicksSync(pairTracker.Pair, 10); - Assert.That(session.AttachedEntity, Is.Not.EqualTo(originalMob)); + var ghost = session.AttachedEntity; + Assert.That(entMan.HasComponent(ghost)); + Assert.That(ghost, Is.Not.EqualTo(originalMob)); + Assert.That(session.ContentData()?.Mind, Is.EqualTo(originalMind)); + Assert.That(originalMind.OwnedEntity, Is.EqualTo(originalMob)); + Assert.That(originalMind.VisitingEntity, Is.EqualTo(ghost)); // Spawn ghost takeover entity. EntityUid ghostRole = default; @@ -74,21 +83,39 @@ public sealed class GhostRoleTests // Check player got attached to ghost role. await PoolManager.RunTicksSync(pairTracker.Pair, 10); + var newMind = session.ContentData()!.Mind!; + Assert.That(newMind, Is.Not.EqualTo(originalMind)); Assert.That(session.AttachedEntity, Is.EqualTo(ghostRole)); + Assert.That(newMind.OwnedEntity, Is.EqualTo(ghostRole)); + Assert.Null(newMind.VisitingEntity); + + // Original mind should be unaffected, but the ghost will have deleted itself. + Assert.That(originalMind.OwnedEntity, Is.EqualTo(originalMob)); + Assert.Null(originalMind.VisitingEntity); + Assert.That(entMan.Deleted(ghost)); // Ghost again. conHost.ExecuteCommand("ghost"); await PoolManager.RunTicksSync(pairTracker.Pair, 10); - Assert.That(session.AttachedEntity, Is.Not.EqualTo(originalMob)); - Assert.That(session.AttachedEntity, Is.Not.EqualTo(ghostRole)); + var otherGhost = session.AttachedEntity; + Assert.That(entMan.HasComponent(otherGhost)); + Assert.That(otherGhost, Is.Not.EqualTo(originalMob)); + Assert.That(otherGhost, Is.Not.EqualTo(ghostRole)); + Assert.That(session.ContentData()?.Mind, Is.EqualTo(newMind)); + Assert.That(newMind.OwnedEntity, Is.EqualTo(ghostRole)); + Assert.That(newMind.VisitingEntity, Is.EqualTo(session.AttachedEntity)); // Next, control the original entity again: - await server.WaitPost(() => - { - mindSystem.TransferTo(session.ContentData()!.Mind!, originalMob, true); - }); + await server.WaitPost(() => mindSystem.SetUserId(originalMind, session.UserId)); await PoolManager.RunTicksSync(pairTracker.Pair, 10); Assert.That(session.AttachedEntity, Is.EqualTo(originalMob)); + Assert.That(originalMind.OwnedEntity, Is.EqualTo(originalMob)); + Assert.Null(originalMind.VisitingEntity); + + // the ghost-role mind is unaffected, though the ghost will have deleted itself + Assert.That(newMind.OwnedEntity, Is.EqualTo(ghostRole)); + Assert.Null(newMind.VisitingEntity); + Assert.That(entMan.Deleted(otherGhost)); await pairTracker.CleanReturnAsync(); } diff --git a/Content.IntegrationTests/Tests/Minds/MindTests.EntityDeletion.cs b/Content.IntegrationTests/Tests/Minds/MindTests.EntityDeletion.cs index c0ff4f197a..63bc52bbe8 100644 --- a/Content.IntegrationTests/Tests/Minds/MindTests.EntityDeletion.cs +++ b/Content.IntegrationTests/Tests/Minds/MindTests.EntityDeletion.cs @@ -118,7 +118,7 @@ public sealed partial class MindTests public async Task TestGhostOnDelete() { // Client is needed to spawn session - await using var pairTracker = await SetupPair(); + await using var pairTracker = await SetupPair(dirty: true); var server = pairTracker.Pair.Server; var entMan = server.ResolveDependency(); diff --git a/Content.IntegrationTests/Tests/Minds/MindTests.Helpers.cs b/Content.IntegrationTests/Tests/Minds/MindTests.Helpers.cs index cff9cd82cf..152715d471 100644 --- a/Content.IntegrationTests/Tests/Minds/MindTests.Helpers.cs +++ b/Content.IntegrationTests/Tests/Minds/MindTests.Helpers.cs @@ -23,12 +23,13 @@ public sealed partial class MindTests /// the player's mind's current entity, likely because some previous test directly changed the players attached /// entity. /// - private static async Task SetupPair() + private static async Task SetupPair(bool dirty = false) { var pairTracker = await PoolManager.GetServerClient(new PoolSettings { DummyTicker = false, - Connected = true + Connected = true, + Dirty = dirty }); var pair = pairTracker.Pair; diff --git a/Content.IntegrationTests/Tests/Minds/MindTests.cs b/Content.IntegrationTests/Tests/Minds/MindTests.cs index 2c970ac045..4899f22366 100644 --- a/Content.IntegrationTests/Tests/Minds/MindTests.cs +++ b/Content.IntegrationTests/Tests/Minds/MindTests.cs @@ -2,6 +2,7 @@ using System.Linq; using Content.Server.Ghost; using Content.Server.Ghost.Roles; +using Content.Server.Ghost.Roles.Components; using Content.Server.Mind; using Content.Server.Mind.Commands; using Content.Server.Mind.Components; @@ -403,7 +404,8 @@ public sealed partial class MindTests await using var pairTracker = await PoolManager.GetServerClient(new PoolSettings { DummyTicker = false, - Connected = true + Connected = true, + Dirty = true }); var server = pairTracker.Pair.Server; @@ -412,7 +414,7 @@ public sealed partial class MindTests var serverConsole = server.ResolveDependency(); //EntityUid entity = default!; - EntityUid mouse = default!; + EntityUid ghostRole = default!; EntityUid ghost = default!; Mind mind = default!; var player = playerMan.ServerSessions.Single(); @@ -436,36 +438,37 @@ public sealed partial class MindTests Assert.That(mind.OwnedEntity, Is.Not.Null); - mouse = entMan.SpawnEntity("MobMouse", new MapCoordinates()); + ghostRole = entMan.SpawnEntity("GhostRoleTestEntity", MapCoordinates.Nullspace); }); - await PoolManager.RunTicksSync(pairTracker.Pair, 120); + await PoolManager.RunTicksSync(pairTracker.Pair, 20); await server.WaitAssertion(() => { serverConsole.ExecuteCommand(player, "aghost"); }); - await PoolManager.RunTicksSync(pairTracker.Pair, 120); + await PoolManager.RunTicksSync(pairTracker.Pair, 20); await server.WaitAssertion(() => { - entMan.EntitySysManager.GetEntitySystem().Takeover(player, 0); + var id = entMan.GetComponent(ghostRole).Identifier; + entMan.EntitySysManager.GetEntitySystem().Takeover(player, id); }); - await PoolManager.RunTicksSync(pairTracker.Pair, 120); + await PoolManager.RunTicksSync(pairTracker.Pair, 20); await server.WaitAssertion(() => { var data = player.ContentData()!; - Assert.That(data.Mind!.OwnedEntity, Is.EqualTo(mouse)); + Assert.That(data.Mind!.OwnedEntity, Is.EqualTo(ghostRole)); serverConsole.ExecuteCommand(player, "aghost"); Assert.That(player.AttachedEntity, Is.Not.Null); ghost = player.AttachedEntity!.Value; }); - await PoolManager.RunTicksSync(pairTracker.Pair, 60); + await PoolManager.RunTicksSync(pairTracker.Pair, 20); await server.WaitAssertion(() => { diff --git a/Content.Server/Mind/MindSystem.cs b/Content.Server/Mind/MindSystem.cs index 99ec003c9c..6220637ea6 100644 --- a/Content.Server/Mind/MindSystem.cs +++ b/Content.Server/Mind/MindSystem.cs @@ -31,6 +31,7 @@ public sealed class MindSystem : EntitySystem [Dependency] private readonly GhostSystem _ghostSystem = default!; [Dependency] private readonly IAdminLogManager _adminLogger = default!; [Dependency] private readonly IPlayerManager _playerManager = default!; + [Dependency] private readonly ActorSystem _actor = default!; // This is dictionary is required to track the minds of disconnected players that may have had their entity deleted. private readonly Dictionary _userMinds = new(); @@ -584,11 +585,10 @@ public sealed class MindSystem : EntitySystem } /// - /// Sets the Mind's UserId, Session, and updates the player's PlayerData. - /// This should have no direct effect on the entity that any mind is connected to, but it may change a player's attached entity. + /// Sets the Mind's UserId, Session, and updates the player's PlayerData. This should have no direct effect on the + /// entity that any mind is connected to, except as a side effect of the fact that it may change a player's + /// attached entity. E.g., ghosts get deleted. /// - /// - /// public void SetUserId(Mind mind, NetUserId? userId) { if (mind.UserId == userId) @@ -602,7 +602,7 @@ public sealed class MindSystem : EntitySystem if (mind.Session != null) { - mind.Session.AttachToEntity(null); + _actor.Attach(null, mind.Session); mind.Session = null; } @@ -629,8 +629,11 @@ public sealed class MindSystem : EntitySystem mind.UserId = userId; mind.OriginalOwnerUserId ??= userId; - _playerManager.TryGetSessionById(userId.Value, out var ret); - mind.Session = ret; + if (_playerManager.TryGetSessionById(userId.Value, out var ret)) + { + mind.Session = ret; + _actor.Attach(mind.CurrentEntity, mind.Session); + } // session may be null, but user data may still exist for disconnected players. if (_playerManager.GetPlayerData(userId.Value).ContentData() is { } data)