Add role prototype validation tests (#32801)

* Add role prototype validation test

* Rejig GetPrototypesWithComponent

* More tests n stuff

Content.IntegrationTests/Pair/TestPair.Helpers.cs

@@ -107,13 +107,41 @@ public sealed partial class TestPair
     /// <summary>
     /// Retrieve all entity prototypes that have some component.
     /// </summary>
-    public List<EntityPrototype> GetPrototypesWithComponent<T>(
+    public List<(EntityPrototype, T)> GetPrototypesWithComponent<T>(
         HashSet<string>? ignored = null,
         bool ignoreAbstract = true,
         bool ignoreTestPrototypes = true)
         where T : IComponent
     {
         var id = Server.ResolveDependency<IComponentFactory>().GetComponentName(typeof(T));
+        var list = new List<(EntityPrototype, T)>();
+        foreach (var proto in Server.ProtoMan.EnumeratePrototypes<EntityPrototype>())
+        {
+            if (ignored != null && ignored.Contains(proto.ID))
+                continue;
+
+            if (ignoreAbstract && proto.Abstract)
+                continue;
+
+            if (ignoreTestPrototypes && IsTestPrototype(proto))
+                continue;
+
+            if (proto.Components.TryGetComponent(id, out var cmp))
+                list.Add((proto, (T)cmp));
+        }
+
+        return list;
+    }
+
+    /// <summary>
+    /// Retrieve all entity prototypes that have some component.
+    /// </summary>
+    public List<EntityPrototype> GetPrototypesWithComponent(Type type,
+        HashSet<string>? ignored = null,
+        bool ignoreAbstract = true,
+        bool ignoreTestPrototypes = true)
+    {
+        var id = Server.ResolveDependency<IComponentFactory>().GetComponentName(type);
         var list = new List<EntityPrototype>();
         foreach (var proto in Server.ProtoMan.EnumeratePrototypes<EntityPrototype>())
         {
@@ -127,7 +155,7 @@ public sealed partial class TestPair
                 continue;
 
             if (proto.Components.ContainsKey(id))
-                list.Add(proto);
+                list.Add((proto));
         }
 
         return list;

Content.IntegrationTests/Tests/Minds/RoleTests.cs

@@ -0,0 +1,95 @@
+using System.Linq;
+using Content.Server.Roles;
+using Content.Shared.Roles;
+using Content.Shared.Roles.Jobs;
+using Robust.Shared.GameObjects;
+using Robust.Shared.Reflection;
+
+namespace Content.IntegrationTests.Tests.Minds;
+
+[TestFixture]
+public sealed class RoleTests
+{
+    /// <summary>
+    /// Check that any prototype with a <see cref="MindRoleComponent"/> is properly configured
+    /// </summary>
+    [Test]
+    public async Task ValidateRolePrototypes()
+    {
+        await using var pair = await PoolManager.GetServerClient();
+
+        var jobComp = pair.Server.ResolveDependency<IComponentFactory>().GetComponentName(typeof(JobRoleComponent));
+
+        Assert.Multiple(() =>
+        {
+            foreach (var (proto, comp) in pair.GetPrototypesWithComponent<MindRoleComponent>())
+            {
+                Assert.That(comp.AntagPrototype == null || comp.JobPrototype == null, $"Role {proto.ID} has both a job and antag prototype.");
+                Assert.That(!comp.ExclusiveAntag || comp.Antag, $"Role {proto.ID} is marked as an exclusive antag, despite not being an antag.");
+                Assert.That(comp.Antag || comp.AntagPrototype == null, $"Role {proto.ID} has an antag prototype, despite not being an antag.");
+
+                if (comp.JobPrototype != null)
+                    Assert.That(proto.Components.ContainsKey(jobComp), $"Role {proto.ID} is a job, despite not having a job prototype.");
+
+                // It is possible that this is meant to be supported? Though I would assume that it would be for
+                // admin / prototype uploads, and that pre-defined roles should still check this.
+                Assert.That(!comp.Antag || comp.AntagPrototype != null , $"Role {proto.ID} is an antag, despite not having a antag prototype.");
+            }
+        });
+
+        await pair.CleanReturnAsync();
+    }
+
+    /// <summary>
+    /// Check that any prototype with a <see cref="JobRoleComponent"/> also has a properly configured
+    /// <see cref="MindRoleComponent"/>
+    /// </summary>
+    [Test]
+    public async Task ValidateJobPrototypes()
+    {
+        await using var pair = await PoolManager.GetServerClient();
+
+        var mindCompId = pair.Server.ResolveDependency<IComponentFactory>().GetComponentName(typeof(MindRoleComponent));
+
+        Assert.Multiple(() =>
+        {
+            foreach (var (proto, comp) in pair.GetPrototypesWithComponent<JobRoleComponent>())
+            {
+                if (proto.Components.TryGetComponent(mindCompId, out var mindComp))
+                    Assert.That(((MindRoleComponent)mindComp).JobPrototype, Is.Not.Null);
+            }
+        });
+
+        await pair.CleanReturnAsync();
+    }
+
+    /// <summary>
+    /// Check that any prototype with a component that inherits from <see cref="BaseMindRoleComponent"/> also has a
+    /// <see cref="MindRoleComponent"/>
+    /// </summary>
+    [Test]
+    public async Task ValidateRolesHaveMindRoleComp()
+    {
+        await using var pair = await PoolManager.GetServerClient();
+
+        var refMan = pair.Server.ResolveDependency<IReflectionManager>();
+        var mindCompId = pair.Server.ResolveDependency<IComponentFactory>().GetComponentName(typeof(MindRoleComponent));
+
+        var compTypes = refMan.GetAllChildren(typeof(BaseMindRoleComponent))
+            .Append(typeof(RoleBriefingComponent))
+            .Where(x => !x.IsAbstract);
+
+        Assert.Multiple(() =>
+        {
+            foreach (var comp in compTypes)
+            {
+                foreach (var proto in pair.GetPrototypesWithComponent(comp))
+                {
+                    Assert.That(proto.Components.ContainsKey(mindCompId), $"Role {proto.ID} does not have a {nameof(MindRoleComponent)} despite having a {comp.Name}");
+                }
+            }
+        });
+
+        await pair.CleanReturnAsync();
+    }
+}

Content.IntegrationTests/Tests/Sprite/ItemSpriteTest.cs

@@ -40,7 +40,7 @@ public sealed class PrototypeSaveTest
 
         await pair.Client.WaitPost(() =>
         {
-            foreach (var proto in pair.GetPrototypesWithComponent<ItemComponent>(Ignored))
+            foreach (var (proto, _) in pair.GetPrototypesWithComponent<ItemComponent>(Ignored))
             {
                 var dummy = pair.Client.EntMan.Spawn(proto.ID);
                 pair.Client.EntMan.RunMapInit(dummy, pair.Client.MetaData(dummy));

Content.IntegrationTests/Tests/StorageTest.cs

@@ -94,14 +94,13 @@ namespace Content.IntegrationTests.Tests
 
             await Assert.MultipleAsync(async () =>
             {
-                foreach (var proto in pair.GetPrototypesWithComponent<StorageFillComponent>())
+                foreach (var (proto, fill) in pair.GetPrototypesWithComponent<StorageFillComponent>())
                 {
                     if (proto.HasComponent<EntityStorageComponent>(compFact))
                         continue;
 
                     StorageComponent? storage = null;
                     ItemComponent? item = null;
-                    StorageFillComponent fill = default!;
                     var size = 0;
                     await server.WaitAssertion(() =>
                     {
@@ -112,7 +111,6 @@ namespace Content.IntegrationTests.Tests
                         }
 
                         proto.TryGetComponent("Item", out item);
-                        fill = (StorageFillComponent) proto.Components[id].Component;
                         size = GetFillSize(fill, false, protoMan, itemSys);
                     });
 
@@ -179,7 +177,7 @@ namespace Content.IntegrationTests.Tests
 
             var itemSys = entMan.System<SharedItemSystem>();
 
-            foreach (var proto in pair.GetPrototypesWithComponent<StorageFillComponent>())
+            foreach (var (proto, fill) in pair.GetPrototypesWithComponent<StorageFillComponent>())
             {
                 if (proto.HasComponent<StorageComponent>(compFact))
                     continue;
@@ -192,7 +190,6 @@ namespace Content.IntegrationTests.Tests
                     if (entStorage == null)
                         return;
 
-                    var fill = (StorageFillComponent) proto.Components[id].Component;
                     var size = GetFillSize(fill, true, protoMan, itemSys);
                     Assert.That(size, Is.LessThanOrEqualTo(entStorage.Capacity),
                         $"{proto.ID} storage fill is too large.");

Content.Server/EntityEffects/EffectConditions/JobCondition.cs

@@ -26,9 +26,17 @@ public sealed partial class JobCondition : EntityEffectCondition
             if(!args.EntityManager.HasComponent<JobRoleComponent>(roleId))
                 continue;
 
-            if(!args.EntityManager.TryGetComponent<MindRoleComponent>(roleId, out var mindRole)
+            if (!args.EntityManager.TryGetComponent<MindRoleComponent>(roleId, out var mindRole))
-               || mindRole.JobPrototype is null)
+            {
+                Logger.Error($"Encountered job mind role entity {roleId} without a {nameof(MindRoleComponent)}");
                 continue;
+            }
+
+            if (mindRole.JobPrototype == null)
+            {
+                Logger.Error($"Encountered job mind role entity {roleId} without a {nameof(JobPrototype)}");
+                continue;
+            }
 
             if (Job.Contains(mindRole.JobPrototype.Value))
                 return true;

Content.Server/GameTicking/Rules/RevolutionaryRuleSystem.cs

@@ -155,8 +155,8 @@ public sealed class RevolutionaryRuleSystem : GameRuleSystem<RevolutionaryRuleCo
 
             if (_mind.TryGetMind(ev.User.Value, out var revMindId, out _))
             {
-                if (_role.MindHasRole<RevolutionaryRoleComponent>(revMindId, out _, out var role))
+                if (_role.MindHasRole<RevolutionaryRoleComponent>(revMindId, out var role))
-                    role.Value.Comp.ConvertedCount++;
+                    role.Value.Comp2.ConvertedCount++;
             }
         }
 

Content.Server/Ghost/Roles/GhostRoleSystem.cs

@@ -516,8 +516,8 @@ public sealed class GhostRoleSystem : EntitySystem
 
         _roleSystem.MindAddRole(newMind, "MindRoleGhostMarker");
 
-        if(_roleSystem.MindHasRole<GhostRoleMarkerRoleComponent>(newMind, out _, out var markerRole))
+        if(_roleSystem.MindHasRole<GhostRoleMarkerRoleComponent>(newMind!, out var markerRole))
-            markerRole.Value.Comp.Name = role.RoleName;
+            markerRole.Value.Comp2.Name = role.RoleName;
 
         _mindSystem.SetUserId(newMind, player.UserId);
         _mindSystem.TransferTo(newMind, mob);

Content.Shared/Roles/Jobs/SharedJobSystem.cs

@@ -103,7 +103,6 @@ public abstract class SharedJobSystem : EntitySystem
     public bool MindHasJobWithId(EntityUid? mindId, string prototypeId)
     {
 
-        MindRoleComponent? comp = null;
         if (mindId is null)
             return false;
 
@@ -112,9 +111,7 @@ public abstract class SharedJobSystem : EntitySystem
         if (role is null)
             return false;
 
-        comp = role.Value.Comp;
+        return role.Value.Comp1.JobPrototype == prototypeId;
-
-        return (comp.JobPrototype == prototypeId);
     }
 
     public bool MindTryGetJob(
@@ -124,7 +121,7 @@ public abstract class SharedJobSystem : EntitySystem
         prototype = null;
         MindTryGetJobId(mindId, out var protoId);
 
-        return (_prototypes.TryIndex<JobPrototype>(protoId, out prototype) || prototype is not null);
+        return _prototypes.TryIndex(protoId, out prototype) || prototype is not null;
     }
 
     public bool MindTryGetJobId(
@@ -137,9 +134,9 @@ public abstract class SharedJobSystem : EntitySystem
             return false;
 
         if (_roles.MindHasRole<JobRoleComponent>(mindId.Value, out var role))
-            job = role.Value.Comp.JobPrototype;
+            job = role.Value.Comp1.JobPrototype;
 
-        return (job is not null);
+        return job is not null;
     }
 
     /// <summary>

Content.Shared/Roles/SharedRoleSystem.cs

@@ -10,6 +10,7 @@ using Robust.Shared.Audio.Systems;
 using Robust.Shared.Configuration;
 using Robust.Shared.Map;
 using Robust.Shared.Prototypes;
+using Robust.Shared.Utility;
 
 namespace Content.Shared.Roles;
 
@@ -92,19 +93,18 @@ public abstract class SharedRoleSystem : EntitySystem
         bool silent = false,
         string? jobPrototype = null)
     {
+        if (!Resolve(mindId, ref mind))
+            return;
+
         // Can't have someone get paid for two jobs now, can we
-        if (MindHasRole<JobRoleComponent>(mindId, out var jobRole)
+        if (MindHasRole<JobRoleComponent>((mindId, mind), out var jobRole)
-            && jobRole.Value.Comp.JobPrototype != jobPrototype)
+            && jobRole.Value.Comp1.JobPrototype != jobPrototype)
-        {
-            Resolve(mindId, ref mind);
-            if (mind is not null)
         {
             _adminLogger.Add(LogType.Mind,
                 LogImpact.Low,
-                    $"Job Role of {ToPrettyString(mind.OwnedEntity)} changed from '{jobRole.Value.Comp.JobPrototype}' to '{jobPrototype}'");
+                $"Job Role of {ToPrettyString(mind.OwnedEntity)} changed from '{jobRole.Value.Comp1.JobPrototype}' to '{jobPrototype}'");
-            }
 
-            jobRole.Value.Comp.JobPrototype = jobPrototype;
+            jobRole.Value.Comp1.JobPrototype = jobPrototype;
         }
         else
             MindAddRoleDo(mindId, "MindRoleJob", mind, silent, jobPrototype);
@@ -146,11 +146,12 @@ public abstract class SharedRoleSystem : EntitySystem
         {
             mindRoleComp.JobPrototype = jobPrototype;
             EnsureComp<JobRoleComponent>(mindRoleId);
+            DebugTools.AssertNull(mindRoleComp.AntagPrototype);
+            DebugTools.Assert(!mindRoleComp.Antag);
+            DebugTools.Assert(!mindRoleComp.ExclusiveAntag);
         }
 
-        if (mindRoleComp.Antag || mindRoleComp.ExclusiveAntag)
+        antagonist |= mindRoleComp.Antag;
-            antagonist = true;
-
         mind.MindRoles.Add(mindRoleId);
 
         var mindEv = new MindRoleAddedEvent(silent);
@@ -182,51 +183,55 @@ public abstract class SharedRoleSystem : EntitySystem
     /// <summary>
     ///     Removes all instances of a specific role from this mind.
     /// </summary>
-    /// <param name="mindId">The mind to remove the role from.</param>
+    /// <param name="mind">The mind to remove the role from.</param>
     /// <typeparam name="T">The type of the role to remove.</typeparam>
-    /// <exception cref="ArgumentException">Thrown if the mind does not exist or does not have this role.</exception>
+    /// <returns>Returns false if the role did not exist. True if successful</returns>>
-    /// <returns>Returns False if there was something wrong with the mind or the removal. True if successful</returns>>
+    public bool MindRemoveRole<T>(Entity<MindComponent?> mind) where T : IComponent
-    public bool MindRemoveRole<T>(EntityUid mindId) where T : IComponent
     {
-        if (!TryComp<MindComponent>(mindId, out var mind) )
+        if (typeof(T) == typeof(MindRoleComponent))
-            throw new ArgumentException($"{mindId} does not exist or does not have mind component");
+            throw new InvalidOperationException();
+
+        if (!Resolve(mind.Owner, ref mind.Comp))
+            return false;
 
         var found = false;
         var antagonist = false;
         var delete = new List<EntityUid>();
-        foreach (var role in mind.MindRoles)
+        foreach (var role in mind.Comp.MindRoles)
         {
             if (!HasComp<T>(role))
                 continue;
 
-            var roleComp = Comp<MindRoleComponent>(role);
+            if (!TryComp(role, out MindRoleComponent? roleComp))
-            antagonist = roleComp.Antag;
+            {
-            _entityManager.DeleteEntity(role);
+                Log.Error($"Encountered mind role entity {ToPrettyString(role)} without a {nameof(MindRoleComponent)}");
-
+                continue;
-            delete.Add(role);
-            found = true;
-
             }
 
-        foreach (var role in delete)
+            antagonist |= roleComp.Antag | roleComp.ExclusiveAntag;
-        {
+            _entityManager.DeleteEntity(role);
-            mind.MindRoles.Remove(role);
+            delete.Add(role);
+            found = true;
         }
 
         if (!found)
+            return false;
+
+        foreach (var role in delete)
         {
-            throw new ArgumentException($"{mindId} does not have this role: {typeof(T)}");
+            mind.Comp.MindRoles.Remove(role);
         }
 
-        var message = new RoleRemovedEvent(mindId, mind, antagonist);
+        if (mind.Comp.OwnedEntity != null)
-
-        if (mind.OwnedEntity != null)
         {
-            RaiseLocalEvent(mind.OwnedEntity.Value, message, true);
+            var message = new RoleRemovedEvent(mind.Owner, mind.Comp, antagonist);
+            RaiseLocalEvent(mind.Comp.OwnedEntity.Value, message, true);
         }
+
         _adminLogger.Add(LogType.Mind,
             LogImpact.Low,
-            $"'Role {typeof(T).Name}' removed from mind of {ToPrettyString(mind.OwnedEntity)}");
+            $"All roles of type '{typeof(T).Name}' removed from mind of {ToPrettyString(mind.Comp.OwnedEntity)}");
+
         return true;
     }
 
@@ -238,16 +243,14 @@ public abstract class SharedRoleSystem : EntitySystem
     /// <returns>True if the role existed and was removed</returns>
     public bool MindTryRemoveRole<T>(EntityUid mindId) where T : IComponent
     {
-        if (!MindHasRole<T>(mindId))
-        {
-            Log.Warning($"Failed to remove role {typeof(T)} from {mindId} : mind does not have role ");
-            return false;
-        }
-
         if (typeof(T) == typeof(MindRoleComponent))
             return false;
 
-        return MindRemoveRole<T>(mindId);
+        if (MindRemoveRole<T>(mindId))
+            return true;
+
+        Log.Warning($"Failed to remove role {typeof(T)} from {ToPrettyString(mindId)} : mind does not have role ");
+        return false;
     }
 
     /// <summary>
@@ -259,30 +262,29 @@ public abstract class SharedRoleSystem : EntitySystem
     /// <param name="role">The Mind Role entity component</param>
     /// <param name="roleT">The Mind Role's entity component for T</param>
     /// <returns>True if the role is found</returns>
-    public bool MindHasRole<T>(EntityUid mindId,
+    public bool MindHasRole<T>(Entity<MindComponent?> mind,
-        [NotNullWhen(true)] out Entity<MindRoleComponent>? role,
+        [NotNullWhen(true)] out Entity<MindRoleComponent, T>? role) where T : IComponent
-        [NotNullWhen(true)] out Entity<T>? roleT) where T : IComponent
     {
         role = null;
-        roleT = null;
+        if (!Resolve(mind.Owner, ref mind.Comp))
-
-        if (!TryComp<MindComponent>(mindId, out var mind))
             return false;
 
-        var found = false;
+        foreach (var roleEnt in mind.Comp.MindRoles)
-
-        foreach (var roleEnt in mind.MindRoles)
         {
-            if (!HasComp<T>(roleEnt))
+            if (!TryComp(roleEnt, out T? tcomp))
                 continue;
 
-            role = (roleEnt,Comp<MindRoleComponent>(roleEnt));
+            if (!TryComp(roleEnt, out MindRoleComponent? roleComp))
-            roleT = (roleEnt,Comp<T>(roleEnt));
+            {
-            found = true;
+                Log.Error($"Encountered mind role entity {ToPrettyString(roleEnt)} without a {nameof(MindRoleComponent)}");
-            break;
+                continue;
             }
 
-        return found;
+            role = (roleEnt, roleComp, tcomp);
+            return true;
+        }
+
+        return false;
     }
 
     /// <summary>
@@ -317,7 +319,13 @@ public abstract class SharedRoleSystem : EntitySystem
             if (!HasComp(roleEnt, type))
                 continue;
 
-            role = (roleEnt,Comp<MindRoleComponent>(roleEnt));
+            if (!TryComp(roleEnt, out MindRoleComponent? roleComp))
+            {
+                Log.Error($"Encountered mind role entity {ToPrettyString(roleEnt)} without a {nameof(MindRoleComponent)}");
+                continue;
+            }
+
+            role = (roleEnt, roleComp);
             found = true;
             break;
         }
@@ -325,20 +333,6 @@ public abstract class SharedRoleSystem : EntitySystem
         return found;
     }
 
-    /// <summary>
-    /// Finds the first mind role of a specific type on a mind entity.
-    /// Outputs an entity component for the mind role's MindRoleComponent
-    /// </summary>
-    /// <param name="mindId">The mind entity</param>
-    /// <param name="role">The Mind Role entity component</param>
-    /// <typeparam name="T">The type of the role to find.</typeparam>
-    /// <returns>True if the role is found</returns>
-    public bool MindHasRole<T>(EntityUid mindId,
-        [NotNullWhen(true)] out Entity<MindRoleComponent>? role) where T : IComponent
-    {
-        return MindHasRole<T>(mindId, out role, out _);
-    }
-
     /// <summary>
     /// Finds the first mind role of a specific type on a mind entity.
     /// </summary>
@@ -347,7 +341,7 @@ public abstract class SharedRoleSystem : EntitySystem
     /// <returns>True if the role is found</returns>
     public bool MindHasRole<T>(EntityUid mindId) where T : IComponent
     {
-        return MindHasRole<T>(mindId, out _, out _);
+        return MindHasRole<T>(mindId, out _);
     }
 
     //TODO: Delete this later
@@ -374,28 +368,31 @@ public abstract class SharedRoleSystem : EntitySystem
     /// <summary>
     /// Reads all Roles of a mind Entity and returns their data as RoleInfo
     /// </summary>
-    /// <param name="mindId">The mind entity</param>
+    /// <param name="mind">The mind entity</param>
     /// <returns>RoleInfo list</returns>
-    public List<RoleInfo> MindGetAllRoleInfo(EntityUid mindId)
+    public List<RoleInfo> MindGetAllRoleInfo(Entity<MindComponent?> mind)
     {
         var roleInfo = new List<RoleInfo>();
 
-        if (!TryComp<MindComponent>(mindId, out var mind))
+        if (!Resolve(mind.Owner, ref mind.Comp))
             return roleInfo;
 
-        foreach (var role in mind.MindRoles)
+        foreach (var role in mind.Comp.MindRoles)
         {
             var valid = false;
             var name = "game-ticker-unknown-role";
             var prototype = "";
             string? playTimeTracker = null;
 
-            var comp = Comp<MindRoleComponent>(role);
+            if (!TryComp(role, out MindRoleComponent? comp))
-            if (comp.AntagPrototype is not null)
             {
-                prototype = comp.AntagPrototype;
+                Log.Error($"Encountered mind role entity {ToPrettyString(role)} without a {nameof(MindRoleComponent)}");
+                continue;
             }
 
+            if (comp.AntagPrototype is not null)
+                prototype = comp.AntagPrototype;
+
             if (comp.JobPrototype is not null && comp.AntagPrototype is null)
             {
                 prototype = comp.JobPrototype;
@@ -429,7 +426,7 @@ public abstract class SharedRoleSystem : EntitySystem
             }
 
             if (valid)
-                roleInfo.Add(new RoleInfo(name, comp.Antag || comp.ExclusiveAntag , playTimeTracker, prototype));
+                roleInfo.Add(new RoleInfo(name, comp.Antag, playTimeTracker, prototype));
         }
         return roleInfo;
     }
@@ -442,12 +439,9 @@ public abstract class SharedRoleSystem : EntitySystem
     public bool MindIsAntagonist(EntityUid? mindId)
     {
         if (mindId is null)
-        {
-            Log.Warning($"Antagonist status of mind entity {mindId} could not be determined - mind entity not found");
             return false;
-        }
 
-        return CheckAntagonistStatus(mindId.Value).Item1;
+        return CheckAntagonistStatus(mindId.Value).Antag;
     }
 
     /// <summary>
@@ -458,37 +452,28 @@ public abstract class SharedRoleSystem : EntitySystem
     public bool MindIsExclusiveAntagonist(EntityUid? mindId)
     {
         if (mindId is null)
-        {
-            Log.Warning($"Antagonist status of mind entity {mindId} could not be determined - mind entity not found");
             return false;
+
+        return CheckAntagonistStatus(mindId.Value).ExclusiveAntag;
     }
 
-        return CheckAntagonistStatus(mindId.Value).Item2;
+   public (bool Antag, bool ExclusiveAntag) CheckAntagonistStatus(Entity<MindComponent?> mind)
-    }
-
-   private (bool, bool) CheckAntagonistStatus(EntityUid mindId)
    {
-       if (!TryComp<MindComponent>(mindId, out var mind))
+       if (!Resolve(mind.Owner, ref mind.Comp))
-       {
-           Log.Warning($"Antagonist status of mind entity {mindId} could not be determined - mind component not found");
            return (false, false);
-       }
 
         var antagonist = false;
         var exclusiveAntag = false;
-        foreach (var role in mind.MindRoles)
+        foreach (var role in mind.Comp.MindRoles)
         {
             if (!TryComp<MindRoleComponent>(role, out var roleComp))
             {
-                //If this ever shows up outside of an integration test, then we need to look into this further.
+                Log.Error($"Mind Role Entity {ToPrettyString(role)} does not have a MindRoleComponent, despite being listed as a role belonging to {ToPrettyString(mind)}|");
-                Log.Warning($"Mind Role Entity {role} does not have MindRoleComponent!");
                 continue;
             }
 
-            if (roleComp.Antag || exclusiveAntag)
+            antagonist |= roleComp.Antag;
-                antagonist = true;
+            exclusiveAntag |= roleComp.ExclusiveAntag;
-            if (roleComp.ExclusiveAntag)
-                exclusiveAntag = true;
         }
 
         return (antagonist, exclusiveAntag);
@@ -504,6 +489,9 @@ public abstract class SharedRoleSystem : EntitySystem
             _audio.PlayGlobal(sound, mind.Session);
     }
 
+    // TODO ROLES Change to readonly.
+    // Passing around a reference to a prototype's hashset makes me uncomfortable because it might be accidentally
+    // mutated.
     public HashSet<JobRequirement>? GetJobRequirement(JobPrototype job)
     {
         if (_requirementOverride != null && _requirementOverride.Jobs.TryGetValue(job.ID, out var req))
@@ -512,6 +500,7 @@ public abstract class SharedRoleSystem : EntitySystem
         return job.Requirements;
     }
 
+    // TODO ROLES Change to readonly.
     public HashSet<JobRequirement>? GetJobRequirement(ProtoId<JobPrototype> job)
     {
         if (_requirementOverride != null && _requirementOverride.Jobs.TryGetValue(job, out var req))
@@ -520,6 +509,7 @@ public abstract class SharedRoleSystem : EntitySystem
         return _prototypes.Index(job).Requirements;
     }
 
+    // TODO ROLES Change to readonly.
     public HashSet<JobRequirement>? GetAntagRequirement(ProtoId<AntagPrototype> antag)
     {
         if (_requirementOverride != null && _requirementOverride.Antags.TryGetValue(antag, out var req))
@@ -528,6 +518,7 @@ public abstract class SharedRoleSystem : EntitySystem
         return _prototypes.Index(antag).Requirements;
     }
 
+    // TODO ROLES Change to readonly.
     public HashSet<JobRequirement>? GetAntagRequirement(AntagPrototype antag)
     {
         if (_requirementOverride != null && _requirementOverride.Antags.TryGetValue(antag.ID, out var req))