Clean up a ton of bugs with suspicion.

Content.Client/GameObjects/Components/Suspicion/SuspicionRoleComponent.cs

@@ -6,14 +6,11 @@ using Content.Shared.GameObjects.Components.Suspicion;
 using Robust.Client.GameObjects;
 using Robust.Client.Interfaces.Graphics.ClientEye;
 using Robust.Client.Interfaces.Graphics.Overlays;
-using Robust.Client.Interfaces.Input;
 using Robust.Client.Interfaces.ResourceManagement;
 using Robust.Shared.GameObjects;
 using Robust.Shared.Interfaces.GameObjects;
-using Robust.Shared.Interfaces.Network;
 using Robust.Shared.IoC;
-using Robust.Shared.Players;
+using Robust.Shared.ViewVariables;
-using Robust.Shared.Prototypes;
 
 namespace Content.Client.GameObjects.Components.Suspicion
 {
@@ -28,6 +25,7 @@ namespace Content.Client.GameObjects.Components.Suspicion
         private SuspicionGui? _gui;
         private string? _role;
         private bool? _antagonist;
+        private bool _overlayActive;
 
         public string? Role
         {
@@ -67,37 +65,8 @@ namespace Content.Client.GameObjects.Components.Suspicion
             }
         }
 
-        public HashSet<EntityUid> Allies { get; } = new();
+        [ViewVariables]
-
+        public List<(string name, EntityUid uid)> Allies { get; } = new();
-        private bool AddAlly(EntityUid ally)
-        {
-            if (!Allies.Add(ally))
-            {
-                return false;
-            }
-
-            if (!_overlayManager.TryGetOverlay<TraitorOverlay>(nameof(TraitorOverlay), out var overlay))
-            {
-                return false;
-            }
-
-            return overlay.AddAlly(ally);
-        }
-
-        private bool RemoveAlly(EntityUid ally)
-        {
-            if (!Allies.Remove(ally))
-            {
-                return false;
-            }
-
-            if (!_overlayManager.TryGetOverlay<TraitorOverlay>(nameof(TraitorOverlay), out var overlay))
-            {
-                return false;
-            }
-
-            return overlay.RemoveAlly(ally);
-        }
 
         private void AddTraitorOverlay()
         {
@@ -106,12 +75,18 @@ namespace Content.Client.GameObjects.Components.Suspicion
                 return;
             }
 
-            var overlay = new TraitorOverlay(Owner, Owner.EntityManager, _resourceCache, _eyeManager);
+            _overlayActive = true;
+            var overlay = new TraitorOverlay(Owner.EntityManager, _resourceCache, _eyeManager);
             _overlayManager.AddOverlay(overlay);
         }
 
         private void RemoveTraitorOverlay()
         {
+            if (!_overlayActive)
+            {
+                return;
+            }
+
             _overlayManager.RemoveOverlay(nameof(TraitorOverlay));
         }
 
@@ -126,6 +101,8 @@ namespace Content.Client.GameObjects.Components.Suspicion
 
             Role = state.Role;
             Antagonist = state.Antagonist;
+            Allies.Clear();
+            Allies.AddRange(state.Allies);
         }
 
         public override void HandleMessage(ComponentMessage message, IComponent? component)
@@ -160,36 +137,6 @@ namespace Content.Client.GameObjects.Components.Suspicion
             }
         }
 
-        public override void HandleNetworkMessage(ComponentMessage message, INetChannel netChannel, ICommonSession? session = null)
-        {
-            base.HandleNetworkMessage(message, netChannel, session);
-
-            switch (message)
-            {
-                case SuspicionAlliesMessage msg:
-                {
-                    Allies.Clear();
-
-                    foreach (var uid in msg.Allies)
-                    {
-                        AddAlly(uid);
-                    }
-
-                    break;
-                }
-                case SuspicionAllyAddedMessage msg:
-                {
-                    AddAlly(msg.Ally);
-                    break;
-                }
-                case SuspicionAllyRemovedMessage msg:
-                {
-                    RemoveAlly(msg.Ally);
-                    break;
-                }
-            }
-        }
-
         public override void OnRemove()
         {
             base.OnRemove();

Content.Client/GameObjects/Components/Suspicion/TraitorOverlay.cs

@@ -1,14 +1,14 @@
-using System.Collections.Generic;
+using Content.Shared.GameObjects.EntitySystems;
-using Content.Shared.GameObjects.EntitySystems;
 using Robust.Client.Graphics;
 using Robust.Client.Graphics.Drawing;
 using Robust.Client.Graphics.Overlays;
 using Robust.Client.Interfaces.Graphics.ClientEye;
 using Robust.Client.Interfaces.ResourceManagement;
+using Robust.Client.Player;
 using Robust.Client.ResourceManagement;
-using Robust.Shared.GameObjects;
 using Robust.Shared.GameObjects.Components;
 using Robust.Shared.Interfaces.GameObjects;
+using Robust.Shared.IoC;
 using Robust.Shared.Localization;
 using Robust.Shared.Maths;
 
@@ -18,37 +18,25 @@ namespace Content.Client.GameObjects.Components.Suspicion
     {
         private readonly IEntityManager _entityManager;
         private readonly IEyeManager _eyeManager;
+        private readonly IPlayerManager _playerManager;
 
         public override OverlaySpace Space => OverlaySpace.ScreenSpace;
         private readonly Font _font;
 
-        private readonly IEntity _user;
-        private readonly HashSet<EntityUid> _allies = new();
         private readonly string _traitorText = Loc.GetString("Traitor");
 
         public TraitorOverlay(
-            IEntity user,
             IEntityManager entityManager,
             IResourceCache resourceCache,
             IEyeManager eyeManager)
             : base(nameof(TraitorOverlay))
         {
+            _playerManager = IoCManager.Resolve<IPlayerManager>();
+
             _entityManager = entityManager;
             _eyeManager = eyeManager;
 
             _font = new VectorFont(resourceCache.GetResource<FontResource>("/Fonts/NotoSans/NotoSans-Regular.ttf"), 10);
-
-            _user = user;
-        }
-
-        public bool AddAlly(EntityUid ally)
-        {
-            return _allies.Add(ally);
-        }
-
-        public bool RemoveAlly(EntityUid ally)
-        {
-            return _allies.Remove(ally);
         }
 
         protected override void Draw(DrawingHandleBase handle, OverlaySpace currentSpace)
@@ -65,7 +53,13 @@ namespace Content.Client.GameObjects.Components.Suspicion
         {
             var viewport = _eyeManager.GetWorldViewport();
 
-            foreach (var uid in _allies)
+            var ent = _playerManager.LocalPlayer?.ControlledEntity;
+            if (ent == null || ent.TryGetComponent(out SuspicionRoleComponent sus) != true)
+            {
+                return;
+            }
+
+            foreach (var (_, uid) in sus.Allies)
             {
                 // Otherwise the entity can not exist yet
                 if (!_entityManager.TryGetEntity(uid, out var ally))
@@ -78,8 +72,8 @@ namespace Content.Client.GameObjects.Components.Suspicion
                     return;
                 }
 
-                if (!ExamineSystemShared.InRangeUnOccluded(_user.Transform.MapPosition, ally.Transform.MapPosition, 15,
+                if (!ExamineSystemShared.InRangeUnOccluded(ent.Transform.MapPosition, ally.Transform.MapPosition, 15,
-                    entity => entity == _user || entity == ally))
+                    entity => entity == ent || entity == ally))
                 {
                     return;
                 }

Content.Client/UserInterface/Suspicion/SuspicionGui.cs

@@ -58,14 +58,11 @@ namespace Content.Client.UserInterface.Suspicion
                 return;
             }
 
-            var allies = string.Join(", ",
+            var allies = string.Join(", ", role.Allies.Select(tuple => tuple.name));
-                role.Allies.Select(uid => _entityManager.GetEntity(uid).Name));
             var message = role.Allies.Count switch
             {
                 0 => Loc.GetString("You have no allies"),
-                1 => Loc.GetString("Your ally is {0}", allies),
+                var n => Loc.GetPluralString("Your ally is {0}", "Your allies are {0}", n, allies),
-                var n when n > 2 => Loc.GetString("Your allies are {0}", allies),
-                _ => throw new ArgumentException($"Invalid number of allies: {role.Allies.Count}")
             };
 
             role.Owner.PopupMessage(message);

Content.Server/GameObjects/Components/Suspicion/SuspicionRoleComponent.cs

@@ -1,4 +1,5 @@
 #nullable enable
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using Content.Server.GameObjects.Components.Mobs;
@@ -6,12 +7,10 @@ using Content.Server.GameObjects.EntitySystems;
 using Content.Server.Mobs;
 using Content.Server.Mobs.Roles;
 using Content.Server.Mobs.Roles.Suspicion;
-using Content.Shared.GameObjects.Components.Damage;
 using Content.Shared.GameObjects.Components.Mobs.State;
 using Content.Shared.GameObjects.Components.Suspicion;
 using Content.Shared.GameObjects.EntitySystems;
 using Robust.Server.GameObjects;
-using Robust.Server.Interfaces.GameObjects;
 using Robust.Shared.GameObjects;
 using Robust.Shared.GameObjects.Systems;
 using Robust.Shared.Interfaces.GameObjects;
@@ -25,6 +24,7 @@ namespace Content.Server.GameObjects.Components.Suspicion
     public class SuspicionRoleComponent : SharedSuspicionRoleComponent, IExamine
     {
         private Role? _role;
+        [ViewVariables]
         private readonly HashSet<SuspicionRoleComponent> _allies = new();
 
         [ViewVariables]
@@ -67,16 +67,12 @@ namespace Content.Server.GameObjects.Components.Suspicion
 
         public bool IsInnocent()
         {
-            return Owner.TryGetComponent(out MindComponent? mind) &&
+            return !IsTraitor();
-                   mind.HasMind &&
-                   mind.Mind!.HasRole<SuspicionInnocentRole>();
         }
 
         public bool IsTraitor()
         {
-            return Owner.TryGetComponent(out MindComponent? mind) &&
+            return Role?.Antagonist ?? false;
-                   mind.HasMind &&
-                   mind.Mind!.HasRole<SuspicionTraitorRole>();
         }
 
         public void SyncRoles()
@@ -98,36 +94,13 @@ namespace Content.Server.GameObjects.Components.Suspicion
             }
 
             _allies.Add(ally);
-
-            if (KnowsAllies && Owner.TryGetComponent(out IActorComponent? actor))
-            {
-                var channel = actor.playerSession.ConnectedClient;
-                DebugTools.AssertNotNull(channel);
-
-                var message = new SuspicionAllyAddedMessage(ally.Owner.Uid);
-
-                SendNetworkMessage(message, channel);
-            }
         }
 
         public bool RemoveAlly(SuspicionRoleComponent ally)
         {
-            if (ally == this)
-            {
-                return false;
-            }
-
             if (_allies.Remove(ally))
             {
-                if (KnowsAllies && Owner.TryGetComponent(out IActorComponent? actor))
+                Dirty();
-                {
-                    var channel = actor.playerSession.ConnectedClient;
-                    DebugTools.AssertNotNull(channel);
-
-                    var message = new SuspicionAllyRemovedMessage(ally.Owner.Uid);
-
-                    SendNetworkMessage(message, channel);
-                }
 
                 return true;
             }
@@ -139,46 +112,16 @@ namespace Content.Server.GameObjects.Components.Suspicion
         {
             _allies.Clear();
 
-            foreach (var ally in allies)
+            _allies.UnionWith(allies.Where(a => a != this));
-            {
-                if (ally == this)
-                {
-                    continue;
-                }
 
-                _allies.Add(ally);
+            Dirty();
-            }
-
-            if (!KnowsAllies ||
-                !Owner.TryGetComponent(out IActorComponent? actor))
-            {
-                return;
-            }
-
-            var channel = actor.playerSession.ConnectedClient;
-            DebugTools.AssertNotNull(channel);
-
-            var message = new SuspicionAlliesMessage(_allies.Select(role => role.Owner.Uid));
-
-            SendNetworkMessage(message, channel);
         }
 
         public void ClearAllies()
         {
             _allies.Clear();
 
-            if (!KnowsAllies ||
+            Dirty();
-                !Owner.TryGetComponent(out IActorComponent? actor))
-            {
-                return;
-            }
-
-            var channel = actor.playerSession.ConnectedClient;
-            DebugTools.AssertNotNull(channel);
-
-            var message = new SuspicionAlliesClearedMessage();
-
-            SendNetworkMessage(message, channel);
         }
 
         void IExamine.Examine(FormattedMessage message, bool inDetailsRange)
@@ -199,39 +142,28 @@ namespace Content.Server.GameObjects.Components.Suspicion
             message.AddMarkup(tooltip);
         }
 
-        public override void OnRemove()
-        {
-            Role = null;
-            base.OnRemove();
-        }
-
         public override ComponentState GetComponentState()
         {
             return Role == null
-                ? new SuspicionRoleComponentState(null, null)
+                ? new SuspicionRoleComponentState(null, null, Array.Empty<(string, EntityUid)>())
-                : new SuspicionRoleComponentState(Role?.Name, Role?.Antagonist);
+                : new SuspicionRoleComponentState(Role?.Name, Role?.Antagonist,
+                    _allies.Select(a => (a.Role!.Mind.CharacterName, a.Owner.Uid)).ToArray());
         }
 
         public override void HandleMessage(ComponentMessage message, IComponent? component)
         {
             base.HandleMessage(message, component);
 
-            if (message is not RoleMessage msg ||
-                msg.Role is not SuspicionRole role)
-            {
-                return;
-            }
-
             switch (message)
             {
-                case PlayerAttachedMsg _:
+                case PlayerAttachedMsg:
-                case PlayerDetachedMsg _:
+                case PlayerDetachedMsg:
                     SyncRoles();
                     break;
-                case RoleAddedMessage _:
+                case RoleAddedMessage {Role: SuspicionRole role}:
                     Role = role;
                     break;
-                case RoleRemovedMessage _:
+                case RoleRemovedMessage {Role: SuspicionRole}:
                     Role = null;
                     break;
             }

Content.Shared/GameObjects/Components/Suspicion/SharedSuspicionRoleComponent.cs

@@ -18,58 +18,13 @@ namespace Content.Shared.GameObjects.Components.Suspicion
     {
         public readonly string? Role;
         public readonly bool? Antagonist;
+        public readonly (string name, EntityUid)[] Allies;
 
-        public SuspicionRoleComponentState(string? role, bool? antagonist) : base(ContentNetIDs.SUSPICION_ROLE)
+        public SuspicionRoleComponentState(string? role, bool? antagonist, (string name, EntityUid)[] allies) : base(ContentNetIDs.SUSPICION_ROLE)
         {
             Role = role;
             Antagonist = antagonist;
-        }
-    }
-
-    [Serializable, NetSerializable]
-    public class SuspicionAlliesMessage : ComponentMessage
-    {
-        public readonly HashSet<EntityUid> Allies;
-
-        public SuspicionAlliesMessage(HashSet<EntityUid> allies)
-        {
-            Directed = true;
             Allies = allies;
         }
-
-        public SuspicionAlliesMessage(IEnumerable<EntityUid> allies) : this(allies.ToHashSet()) { }
-    }
-
-    [Serializable, NetSerializable]
-    public class SuspicionAllyAddedMessage : ComponentMessage
-    {
-        public readonly EntityUid Ally;
-
-        public SuspicionAllyAddedMessage(EntityUid ally)
-        {
-            Directed = true;
-            Ally = ally;
-        }
-    }
-
-    [Serializable, NetSerializable]
-    public class SuspicionAllyRemovedMessage : ComponentMessage
-    {
-        public readonly EntityUid Ally;
-
-        public SuspicionAllyRemovedMessage(EntityUid ally)
-        {
-            Directed = true;
-            Ally = ally;
-        }
-    }
-
-    [Serializable, NetSerializable]
-    public class SuspicionAlliesClearedMessage : ComponentMessage
-    {
-        public SuspicionAlliesClearedMessage()
-        {
-            Directed = true;
-        }
     }
 }