]> git.smokeofanarchy.ru Git - space-station-14.git/commitdiff
Rev Components are no longer leaked + Rev and Zombie icon visibility to ghosts is...
authornikthechampiongr <32041239+nikthechampiongr@users.noreply.github.com>
Thu, 1 Feb 2024 13:08:03 +0000 (15:08 +0200)
committerGitHub <noreply@github.com>
Thu, 1 Feb 2024 13:08:03 +0000 (00:08 +1100)
* Initial work on having the Rev icons not be visible to ghosts depending on a Cvar and a component.

This commit just makes it so that the revcomponent and headrev component
are not shared with clients that shouldn't know about them. This is due
to the concern that clients having access to those components, even if
no image was displayed could allow modified clients to have meta
knowledge of revs.

Currently this has the issue that if a player later
for example becomes a rev, none of the existing rev components get
networked to them. I am not sure there is currently an effecient
solution to this.

This is probably in an issue for a lot more stuff. I might just make it
so all the logic just moves to the client on whether to put the icon
again.

Also this commit adds the ShowRevIconsComponent to allow anyone with it to just
view rev icons.

* Rev components now get communicated to clients that didn't have them previously and the AntagIconSystem is now properly checking whether to give the icons.

We now dirty all the rev/headrev components when someone gets converted
or gets the ViewRevIcons component. The AntagIconSystem now checks
whether it should draw the icons mostly based on an event, this is still done
client side.

This is not a full proof solution to make it so clients can't know
someone is an antag when they shouldn't because:
1. There are other components that need similar treatment, to my
   knowledge not to for revs but for other antags. Maybe even the mind
   component. This could be addressed in future PRs.
2. We cannot ensure that clients forget about these components if the
   client gets deconverted for example. We can of course have code that
   does this, but it will necessarily need to be done on the client and
   if the client is modified then there is no way to ensure this.
   Of course at that point they should already know who their fellow
   revs are so this might not be an issue.

I now need to do the same thing for zombies in a future commit.
A similar system for nukies also needs to be looked at but I will not be
doing that in the PR this commit ends up in.

* Misc name changes and cleaning up the ZombieSystem

Changed some names around and decoupled the ZombieSystem from the
AntagStatusIconsystem. Now there is a cvar for ghost visibility for them
as well. The Zombie Component was not made SessionSpecific because:
1. Zombies are pretty visible anyways
2. The Component is needed to change the appearance of zombie players.

* Misc name changes and cleaning up the ZombieSystem

Changed some names around and decoupled the ZombieSystem from the
AntagStatusIconsystem. Now there is a cvar for ghost visibility for them
as well. The Zombie Component was not made SessionSpecific because:
1. Zombies are pretty visible anyways
2. The Component is needed to change the appearance of zombie players.

* Merged 2 if statements into 1 on the Zombiesystem.

* Cut down on code duplication in AntagStatusIconSystem

Now instead of having a seperate function for each component, there is 1 generic function. Functions for special cases
like the Rev/Headrev comp can have a separate function that does the special check and then calls the generic one.
This is done through the IAntagStatusIconComponent interface which provides a common interface to get the Icon.

* Removed some duplication from the SharedRevolutionarySystem with generics.

I have no idea why I didn't think of this sooner.

* Addressed Reviews I think

I think events get unsubbed automatically but I am probably missing something that I have not understood.
Either way this is a requested change.

* Replace war crimes with actual fixes for reviews

It was not clear to me what the reviews meant

* Addressed reviews by removing need for cvars.

Whether icons are visible to ghosts is now determined by a bool in IAntagStatusIcon which all antag components
with status icons should implement.

* Update Content.Shared/Revolutionary/SharedRevolutionarySystem.cs

---------

Co-authored-by: metalgearsloth <31366439+metalgearsloth@users.noreply.github.com>
12 files changed:
Content.Client/Antag/AntagStatusIconSystem.cs
Content.Client/Revolutionary/RevolutionarySystem.cs
Content.Client/Zombies/ZombieSystem.cs
Content.Shared/Antag/IAntagStatusIconComponent.cs [new file with mode: 0644]
Content.Shared/Revolutionary/Components/HeadRevolutionaryComponent.cs
Content.Shared/Revolutionary/Components/RevolutionaryComponent.cs
Content.Shared/Revolutionary/Components/ShowRevIconsComponent.cs [new file with mode: 0644]
Content.Shared/Revolutionary/SharedRevolutionarySystem.cs
Content.Shared/StatusIcon/Components/StatusIconComponent.cs
Content.Shared/Zombies/ShowZombieIconsComponent.cs [new file with mode: 0644]
Content.Shared/Zombies/ZombieComponent.cs
Resources/Prototypes/Entities/Mobs/Player/admin_ghost.yml

index bf3955b49ab1edfe3eb0cbb40f5a7b5432c5ae2a..5d87837893c4c509262382ff3b076504b988ccc1 100644 (file)
@@ -1,6 +1,8 @@
-using Content.Shared.Ghost;
+using Content.Shared.Antag;
+using Content.Shared.Revolutionary.Components;
 using Content.Shared.StatusIcon;
 using Content.Shared.StatusIcon.Components;
+using Content.Shared.Zombies;
 using Robust.Client.Player;
 using Robust.Shared.Prototypes;
 
@@ -9,24 +11,44 @@ namespace Content.Client.Antag;
 /// <summary>
 /// Used for assigning specified icons for antags.
 /// </summary>
-public abstract class AntagStatusIconSystem<T> : SharedStatusIconSystem
-    where T : IComponent
+public sealed class AntagStatusIconSystem : SharedStatusIconSystem
 {
     [Dependency] private readonly IPrototypeManager _prototype = default!;
     [Dependency] private readonly IPlayerManager _player = default!;
+    public override void Initialize()
+    {
+        base.Initialize();
+
+        SubscribeLocalEvent<RevolutionaryComponent, GetStatusIconsEvent>(GetRevIcon);
+        SubscribeLocalEvent<ZombieComponent, GetStatusIconsEvent>(GetIcon);
+        SubscribeLocalEvent<HeadRevolutionaryComponent, GetStatusIconsEvent>(GetIcon);
+    }
 
     /// <summary>
-    /// Will check if the local player has the same component as the one who called it and give the status icon.
+    /// Adds a Status Icon on an entity if the player is supposed to see it.
     /// </summary>
-    /// <param name="antagStatusIcon">The status icon that your antag uses</param>
-    /// <param name="args">The GetStatusIcon event.</param>
-    protected virtual void GetStatusIcon(string antagStatusIcon, ref GetStatusIconsEvent args)
+    private void GetIcon<T>(EntityUid uid, T comp, ref GetStatusIconsEvent ev) where T: IAntagStatusIconComponent
     {
-        var ent = _player.LocalPlayer?.ControlledEntity;
+        var ent = _player.LocalSession?.AttachedEntity;
+
+        var canEv = new CanDisplayStatusIconsEvent(ent);
+        RaiseLocalEvent(uid, ref canEv);
+
+        if (!canEv.Cancelled)
+            ev.StatusIcons.Add(_prototype.Index(comp.StatusIcon));
+    }
 
-        if (!HasComp<T>(ent) && !HasComp<GhostComponent>(ent))
+
+    /// <summary>
+    /// Adds the Rev Icon on an entity if the player is supposed to see it. This additional function is needed to deal
+    /// with a special case where if someone is a head rev we only want to display the headrev icon.
+    /// </summary>
+    private void GetRevIcon(EntityUid uid, RevolutionaryComponent comp, ref GetStatusIconsEvent ev)
+    {
+        if (HasComp<HeadRevolutionaryComponent>(uid))
             return;
 
-        args.StatusIcons.Add(_prototype.Index<StatusIconPrototype>(antagStatusIcon));
+        GetIcon(uid, comp, ref ev);
+
     }
 }
index 0818b14bc0473498e4899b51bca6849e00f46c2e..682c73f93e784adc3927127ce63ab54187053c05 100644 (file)
@@ -1,5 +1,6 @@
+using Content.Shared.Antag;
 using Content.Shared.Revolutionary.Components;
-using Content.Client.Antag;
+using Content.Shared.Ghost;
 using Content.Shared.StatusIcon.Components;
 
 namespace Content.Client.Revolutionary;
@@ -7,29 +8,37 @@ namespace Content.Client.Revolutionary;
 /// <summary>
 /// Used for the client to get status icons from other revs.
 /// </summary>
-public sealed class RevolutionarySystem : AntagStatusIconSystem<RevolutionaryComponent>
+public sealed class RevolutionarySystem : EntitySystem
 {
+
     public override void Initialize()
     {
         base.Initialize();
 
-        SubscribeLocalEvent<RevolutionaryComponent, GetStatusIconsEvent>(GetRevIcon);
-        SubscribeLocalEvent<HeadRevolutionaryComponent, GetStatusIconsEvent>(GetHeadRevIcon);
+        SubscribeLocalEvent<RevolutionaryComponent, CanDisplayStatusIconsEvent>(OnCanShowRevIcon);
+        SubscribeLocalEvent<HeadRevolutionaryComponent, CanDisplayStatusIconsEvent>(OnCanShowRevIcon);
     }
 
     /// <summary>
-    /// Checks if the person who triggers the GetStatusIcon event is also a Rev or a HeadRev.
+    /// Determine whether a client should display the rev icon.
     /// </summary>
-    private void GetRevIcon(EntityUid uid, RevolutionaryComponent comp, ref GetStatusIconsEvent args)
+    private void OnCanShowRevIcon<T>(EntityUid uid, T comp, ref CanDisplayStatusIconsEvent args) where T : IAntagStatusIconComponent
     {
-        if (!HasComp<HeadRevolutionaryComponent>(uid))
-        {
-            GetStatusIcon(comp.RevStatusIcon, ref args);
-        }
+        args.Cancelled = !CanDisplayIcon(args.User, comp.IconVisibleToGhost);
     }
 
-    private void GetHeadRevIcon(EntityUid uid, HeadRevolutionaryComponent comp, ref GetStatusIconsEvent args)
+    /// <summary>
+    /// The criteria that determine whether a client should see Rev/Head rev icons.
+    /// </summary>
+    private bool CanDisplayIcon(EntityUid? uid, bool visibleToGhost)
     {
-        GetStatusIcon(comp.HeadRevStatusIcon, ref args);
+        if (HasComp<HeadRevolutionaryComponent>(uid) || HasComp<RevolutionaryComponent>(uid))
+            return true;
+
+        if (visibleToGhost && HasComp<GhostComponent>(uid))
+            return true;
+
+        return HasComp<ShowRevIconsComponent>(uid);
     }
+
 }
index 6d0355f6f8df2ec7b25a0653cbc9730d209f28ce..bd89e978c70fa3cc03fa8b6d3bacfddac08b16dc 100644 (file)
@@ -1,5 +1,5 @@
 using System.Linq;
-using Content.Client.Antag;
+using Content.Shared.Ghost;
 using Content.Shared.Humanoid;
 using Content.Shared.StatusIcon.Components;
 using Content.Shared.Zombies;
@@ -7,15 +7,14 @@ using Robust.Client.GameObjects;
 
 namespace Content.Client.Zombies;
 
-public sealed class ZombieSystem : AntagStatusIconSystem<ZombieComponent>
+public sealed class ZombieSystem : EntitySystem
 {
-
     public override void Initialize()
     {
         base.Initialize();
 
         SubscribeLocalEvent<ZombieComponent, ComponentStartup>(OnStartup);
-        SubscribeLocalEvent<ZombieComponent, GetStatusIconsEvent>(OnGetStatusIcon);
+        SubscribeLocalEvent<ZombieComponent, CanDisplayStatusIconsEvent>(OnCanDisplayStatusIcons);
     }
 
     private void OnStartup(EntityUid uid, ZombieComponent component, ComponentStartup args)
@@ -32,8 +31,17 @@ public sealed class ZombieSystem : AntagStatusIconSystem<ZombieComponent>
         }
     }
 
-    private void OnGetStatusIcon(EntityUid uid, ZombieComponent component, ref GetStatusIconsEvent args)
+    /// <summary>
+    /// Determines whether a player should be able to see the StatusIcon for zombies.
+    /// </summary>
+    private void OnCanDisplayStatusIcons(EntityUid uid, ZombieComponent component, ref CanDisplayStatusIconsEvent args)
     {
-        GetStatusIcon(component.ZombieStatusIcon, ref args);
+        if (HasComp<ZombieComponent>(args.User) || HasComp<ShowZombieIconsComponent>(args.User))
+            return;
+
+        if (component.IconVisibleToGhost && HasComp<GhostComponent>(args.User))
+            return;
+
+        args.Cancelled = true;
     }
 }
diff --git a/Content.Shared/Antag/IAntagStatusIconComponent.cs b/Content.Shared/Antag/IAntagStatusIconComponent.cs
new file mode 100644 (file)
index 0000000..981937c
--- /dev/null
@@ -0,0 +1,12 @@
+using Content.Shared.StatusIcon;
+using Robust.Shared.Prototypes;
+
+namespace Content.Shared.Antag;
+
+public interface IAntagStatusIconComponent
+{
+    public ProtoId<StatusIconPrototype> StatusIcon { get; set; }
+
+    public bool IconVisibleToGhost { get; set; }
+}
+
index 48d7c23097540b3310701d2c1088c5dfa3b0656c..d2c8374fefee064cf0bf6202029c8d94e0a2ba71 100644 (file)
@@ -1,3 +1,4 @@
+using Content.Shared.Antag;
 using Robust.Shared.GameStates;
 using Content.Shared.StatusIcon;
 using Robust.Shared.Prototypes;
@@ -8,17 +9,22 @@ namespace Content.Shared.Revolutionary.Components;
 /// Component used for marking a Head Rev for conversion and winning/losing.
 /// </summary>
 [RegisterComponent, NetworkedComponent, Access(typeof(SharedRevolutionarySystem))]
-public sealed partial class HeadRevolutionaryComponent : Component
+public sealed partial class HeadRevolutionaryComponent : Component, IAntagStatusIconComponent
 {
     /// <summary>
     /// The status icon corresponding to the head revolutionary.
     /// </summary>
     [DataField, ViewVariables(VVAccess.ReadWrite)]
-    public ProtoId<StatusIconPrototype> HeadRevStatusIcon = "HeadRevolutionaryFaction";
+    public ProtoId<StatusIconPrototype> StatusIcon { get; set; } = "HeadRevolutionaryFaction";
 
     /// <summary>
     /// How long the stun will last after the user is converted.
     /// </summary>
     [DataField, ViewVariables(VVAccess.ReadWrite)]
     public TimeSpan StunTime = TimeSpan.FromSeconds(3);
+
+    public override bool SessionSpecific => true;
+
+    [DataField]
+    public bool IconVisibleToGhost { get; set; } = true;
 }
index e55c87786b264dd07090b9f15d5c87d59a632830..587c148dd75adbf96f7d6fe61ed80d991d53b032 100644 (file)
@@ -1,3 +1,4 @@
+using Content.Shared.Antag;
 using Robust.Shared.GameStates;
 using Content.Shared.StatusIcon;
 using Robust.Shared.Prototypes;
@@ -8,11 +9,16 @@ namespace Content.Shared.Revolutionary.Components;
 /// Used for marking regular revs as well as storing icon prototypes so you can see fellow revs.
 /// </summary>
 [RegisterComponent, NetworkedComponent, Access(typeof(SharedRevolutionarySystem))]
-public sealed partial class RevolutionaryComponent : Component
+public sealed partial class RevolutionaryComponent : Component, IAntagStatusIconComponent
 {
     /// <summary>
     /// The status icon prototype displayed for revolutionaries
     /// </summary>
     [DataField, ViewVariables(VVAccess.ReadWrite)]
-    public ProtoId<StatusIconPrototype> RevStatusIcon = "RevolutionaryFaction";
+    public ProtoId<StatusIconPrototype> StatusIcon { get; set; } = "RevolutionaryFaction";
+
+    public override bool SessionSpecific => true;
+
+    [DataField]
+    public bool IconVisibleToGhost { get; set; } = true;
 }
diff --git a/Content.Shared/Revolutionary/Components/ShowRevIconsComponent.cs b/Content.Shared/Revolutionary/Components/ShowRevIconsComponent.cs
new file mode 100644 (file)
index 0000000..11e20db
--- /dev/null
@@ -0,0 +1,11 @@
+using Robust.Shared.GameStates;
+
+namespace Content.Shared.Revolutionary.Components;
+
+/// <summary>
+/// Determines whether Someone can see rev/headrev icons on revs.
+/// </summary>
+[RegisterComponent, NetworkedComponent]
+public sealed partial class ShowRevIconsComponent: Component
+{
+}
index 1399b116e0f6b5c3f06a50d8c20d4e303c47b6d6..ddaf73fcc9f967e141eec0007f030d6b66a03075 100644 (file)
@@ -1,8 +1,11 @@
+using Content.Shared.Ghost;
 using Content.Shared.IdentityManagement;
 using Content.Shared.Mindshield.Components;
 using Content.Shared.Popups;
 using Content.Shared.Revolutionary.Components;
 using Content.Shared.Stunnable;
+using Robust.Shared.GameStates;
+using Robust.Shared.Player;
 
 namespace Content.Shared.Revolutionary;
 
@@ -14,7 +17,13 @@ public sealed class SharedRevolutionarySystem : EntitySystem
     public override void Initialize()
     {
         base.Initialize();
+
         SubscribeLocalEvent<MindShieldComponent, MapInitEvent>(MindShieldImplanted);
+        SubscribeLocalEvent<RevolutionaryComponent, ComponentGetStateAttemptEvent>(OnRevCompGetStateAttempt);
+        SubscribeLocalEvent<HeadRevolutionaryComponent, ComponentGetStateAttemptEvent>(OnRevCompGetStateAttempt);
+        SubscribeLocalEvent<RevolutionaryComponent, ComponentStartup>(DirtyRevComps);
+        SubscribeLocalEvent<HeadRevolutionaryComponent, ComponentStartup>(DirtyRevComps);
+        SubscribeLocalEvent<ShowRevIconsComponent, ComponentStartup>(DirtyRevComps);
     }
 
     /// <summary>
@@ -37,4 +46,64 @@ public sealed class SharedRevolutionarySystem : EntitySystem
             _popupSystem.PopupEntity(Loc.GetString("rev-break-control", ("name", name)), uid);
         }
     }
+
+    /// <summary>
+    /// Determines if a HeadRev component should be sent to the client.
+    /// </summary>
+    private void OnRevCompGetStateAttempt(EntityUid uid, HeadRevolutionaryComponent comp, ref ComponentGetStateAttemptEvent args)
+    {
+        args.Cancelled = !CanGetState(args.Player, comp.IconVisibleToGhost);
+    }
+
+    /// <summary>
+    /// Determines if a Rev component should be sent to the client.
+    /// </summary>
+    private void OnRevCompGetStateAttempt(EntityUid uid, RevolutionaryComponent comp, ref ComponentGetStateAttemptEvent args)
+    {
+        args.Cancelled = !CanGetState(args.Player, comp.IconVisibleToGhost);
+    }
+
+    /// <summary>
+    /// The criteria that determine whether a Rev/HeadRev component should be sent to a client.
+    /// </summary>
+    /// <param name="player"> The Player the component will be sent to.</param>
+    /// <param name="visibleToGhosts"> Whether the component permits the icon to be visible to observers. </param>
+    /// <returns></returns>
+    private bool CanGetState(ICommonSession? player, bool visibleToGhosts)
+    {
+        //Apparently this can be null in replays so I am just returning true.
+        if (player is null)
+            return true;
+
+        var uid = player.AttachedEntity;
+
+        if (HasComp<RevolutionaryComponent>(uid) || HasComp<HeadRevolutionaryComponent>(uid))
+            return true;
+
+        if (visibleToGhosts && HasComp<GhostComponent>(uid))
+            return true;
+
+        return HasComp<ShowRevIconsComponent>(uid);
+    }
+    /// <summary>
+    /// Dirties all the Rev components so they are sent to clients.
+    ///
+    /// We need to do this because if a rev component was not earlier sent to a client and for example the client
+    /// becomes a rev then we need to send all the components to it. To my knowledge there is no way to do this on a
+    /// per client basis so we are just dirtying all the components.
+    /// </summary>
+    private void DirtyRevComps<T>(EntityUid someUid, T someComp, ComponentStartup ev)
+    {
+        var revComps = AllEntityQuery<RevolutionaryComponent>();
+        while (revComps.MoveNext(out var uid, out var comp))
+        {
+            Dirty(uid, comp);
+        }
+
+        var headRevComps = AllEntityQuery<HeadRevolutionaryComponent>();
+        while (headRevComps.MoveNext(out var uid, out var comp))
+        {
+            Dirty(uid, comp);
+        }
+    }
 }
index 9efe9731c8b8ca9c54e7e60778760fc5303a6066..385f9760c659ff6a46deb5dd4236a51953964d57 100644 (file)
@@ -25,3 +25,15 @@ public sealed partial class StatusIconComponent : Component
 /// <param name="StatusIcons"></param>
 [ByRefEvent]
 public record struct GetStatusIconsEvent(List<StatusIconData> StatusIcons, bool InContainer);
+
+/// <summary>
+/// Event raised on the Client-side to determine whether to display a status icon on an entity.
+/// </summary>
+/// <param name="User">The player that will see the icons</param>
+[ByRefEvent]
+public record struct CanDisplayStatusIconsEvent(EntityUid? User = null)
+{
+    public EntityUid? User = User;
+
+    public bool Cancelled = false;
+}
diff --git a/Content.Shared/Zombies/ShowZombieIconsComponent.cs b/Content.Shared/Zombies/ShowZombieIconsComponent.cs
new file mode 100644 (file)
index 0000000..a2bc85c
--- /dev/null
@@ -0,0 +1,12 @@
+using Robust.Shared.GameStates;
+
+namespace Content.Shared.Zombies;
+
+/// <summary>
+/// Makes it so an entity can view ZombieAntagIcons.
+/// </summary>
+[RegisterComponent, NetworkedComponent]
+public sealed partial class ShowZombieIconsComponent: Component
+{
+
+}
index bb1f6bec5f1e6db0379f9e418892ac9404adb15c..5ae441b1327ad161403ef8dca6b3f3aaca021674 100644 (file)
@@ -1,3 +1,4 @@
+using Content.Shared.Antag;
 using Content.Shared.Chat.Prototypes;
 using Content.Shared.Chemistry.Reagent;
 using Content.Shared.Damage;
@@ -13,7 +14,7 @@ using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototy
 namespace Content.Shared.Zombies;
 
 [RegisterComponent, NetworkedComponent]
-public sealed partial class ZombieComponent : Component
+public sealed partial class ZombieComponent : Component, IAntagStatusIconComponent
 {
     /// <summary>
     /// The baseline infection chance you have if you are completely nude
@@ -93,8 +94,11 @@ public sealed partial class ZombieComponent : Component
     [DataField("nextTick", customTypeSerializer:typeof(TimeOffsetSerializer))]
     public TimeSpan NextTick;
 
-    [DataField("zombieStatusIcon", customTypeSerializer: typeof(PrototypeIdSerializer<StatusIconPrototype>))]
-    public string ZombieStatusIcon = "ZombieFaction";
+    [DataField("zombieStatusIcon")]
+    public ProtoId<StatusIconPrototype> StatusIcon { get; set; } = "ZombieFaction";
+
+    [DataField]
+    public bool IconVisibleToGhost { get; set; } = true;
 
     /// <summary>
     /// Healing each second
index d74f91109d08771ec923fd02e871e509b06210c6..ee4a860877bfbdc6270b3f0133bd5fb52c3af93a 100644 (file)
@@ -83,6 +83,8 @@
   - type: Stripping
   - type: SolutionScanner
   - type: IgnoreUIRange
+  - type: ShowRevIcons
+  - type: ShowZombieIcons
   - type: Inventory
     templateId: aghost
   - type: InventorySlots