Forbid string literals for `NpcFactionSystem` methods (#38140)

* Add ForbidLiteral attribute to NpcFactionSystem methods

* Cleanup resulting warnings

Content.IntegrationTests/Tests/GameRules/NukeOpsTest.cs

@@ -16,6 +16,7 @@ using Content.Shared.FixedPoint;
 using Content.Shared.GameTicking;
 using Content.Shared.Hands.Components;
 using Content.Shared.Inventory;
+using Content.Shared.NPC.Prototypes;
 using Content.Shared.NPC.Systems;
 using Content.Shared.NukeOps;
 using Content.Shared.Pinpointer;
@@ -24,12 +25,16 @@ using Robust.Server.GameObjects;
 using Robust.Shared.GameObjects;
 using Robust.Shared.Map.Components;
 using Content.Shared._EE.Silicon.Components; // Goobstation
+using Robust.Shared.Prototypes;
 
 namespace Content.IntegrationTests.Tests.GameRules;
 
 [TestFixture]
 public sealed class NukeOpsTest
 {
+    private static readonly ProtoId<NpcFactionPrototype> SyndicateFaction = "Syndicate";
+    private static readonly ProtoId<NpcFactionPrototype> NanotrasenFaction = "NanoTrasen";
+
     /// <summary>
     /// Check that a nuke ops game mode can start without issue. I.e., that the nuke station and such all get loaded.
     /// </summary>
@@ -120,8 +125,8 @@ public sealed class NukeOpsTest
         Assert.That(entMan.HasComponent<NukeOperativeComponent>(player));
         Assert.That(roleSys.MindIsAntagonist(mind));
         Assert.That(roleSys.MindHasRole<NukeopsRoleComponent>(mind));
-        Assert.That(factionSys.IsMember(player, "Syndicate"), Is.True);
+        Assert.That(factionSys.IsMember(player, SyndicateFaction), Is.True);
-        Assert.That(factionSys.IsMember(player, "NanoTrasen"), Is.False);
+        Assert.That(factionSys.IsMember(player, NanotrasenFaction), Is.False);
         var roles = roleSys.MindGetAllRoleInfo(mind);
         var cmdRoles = roles.Where(x => x.Prototype == "NukeopsCommander");
         Assert.That(cmdRoles.Count(), Is.EqualTo(1));
@@ -131,8 +136,8 @@ public sealed class NukeOpsTest
         Assert.That(entMan.HasComponent<NukeOperativeComponent>(dummyEnts[1]));
         Assert.That(roleSys.MindIsAntagonist(dummyMind));
         Assert.That(roleSys.MindHasRole<NukeopsRoleComponent>(dummyMind));
-        Assert.That(factionSys.IsMember(dummyEnts[1], "Syndicate"), Is.True);
+        Assert.That(factionSys.IsMember(dummyEnts[1], SyndicateFaction), Is.True);
-        Assert.That(factionSys.IsMember(dummyEnts[1], "NanoTrasen"), Is.False);
+        Assert.That(factionSys.IsMember(dummyEnts[1], NanotrasenFaction), Is.False);
         roles = roleSys.MindGetAllRoleInfo(dummyMind);
         cmdRoles = roles.Where(x => x.Prototype == "NukeopsMedic");
         Assert.That(cmdRoles.Count(), Is.EqualTo(1));
@@ -147,8 +152,8 @@ public sealed class NukeOpsTest
             Assert.That(entMan.HasComponent<NukeOperativeComponent>(ent), Is.False);
             Assert.That(roleSys.MindIsAntagonist(mindCrew), Is.False);
             Assert.That(roleSys.MindHasRole<NukeopsRoleComponent>(mindCrew), Is.False);
-            Assert.That(factionSys.IsMember(ent, "Syndicate"), Is.False);
+            Assert.That(factionSys.IsMember(ent, SyndicateFaction), Is.False);
-            Assert.That(factionSys.IsMember(ent, "NanoTrasen"), Is.True);
+            Assert.That(factionSys.IsMember(ent, NanotrasenFaction), Is.True);
             var nukeroles = new List<string>() { "Nukeops", "NukeopsMedic", "NukeopsCommander" };
             Assert.That(roleSys.MindGetAllRoleInfo(mindCrew).Any(x => nukeroles.Contains(x.Prototype)), Is.False);
         }

Content.IntegrationTests/Tests/GameRules/TraitorRuleTest.cs

@@ -8,6 +8,7 @@ using Content.Server.Roles;
 using Content.Shared.GameTicking;
 using Content.Shared.GameTicking.Components;
 using Content.Shared.Mind;
+using Content.Shared.NPC.Prototypes;
 using Content.Shared.NPC.Systems;
 using Content.Shared.Objectives.Components;
 using Robust.Shared.GameObjects;
@@ -20,6 +21,8 @@ public sealed class TraitorRuleTest
 {
     private const string TraitorGameRuleProtoId = "Traitor";
     private const string TraitorAntagRoleName = "Traitor";
+    private static readonly ProtoId<NpcFactionPrototype> SyndicateFaction = "Syndicate";
+    private static readonly ProtoId<NpcFactionPrototype> NanotrasenFaction = "NanoTrasen";
 
     [Test]
     public async Task TestTraitorObjectives()
@@ -108,8 +111,8 @@ public sealed class TraitorRuleTest
         // Make sure the player is a traitor.
         var mind = mindSys.GetMind(player)!.Value;
         Assert.That(roleSys.MindIsAntagonist(mind));
-        Assert.That(factionSys.IsMember(player, "Syndicate"), Is.True);
+        Assert.That(factionSys.IsMember(player, SyndicateFaction), Is.True);
-        Assert.That(factionSys.IsMember(player, "NanoTrasen"), Is.False);
+        Assert.That(factionSys.IsMember(player, NanotrasenFaction), Is.False);
         Assert.That(traitorRule.TotalTraitors, Is.EqualTo(1));
         Assert.That(traitorRule.TraitorMinds[0], Is.EqualTo(mind));
 

Content.Server/Zombies/ZombieSystem.Transform.cs

@@ -40,6 +40,7 @@ using Content.Shared.Ghost.Roles.Components;
 using Content.Shared.Tag;
 using Robust.Shared.Player;
 using Robust.Shared.Prototypes;
+using Content.Shared.NPC.Prototypes;
 
 namespace Content.Server.Zombies;
 
@@ -68,6 +69,8 @@ public sealed partial class ZombieSystem
 
     private static readonly ProtoId<TagPrototype> InvalidForGlobalSpawnSpellTag = "InvalidForGlobalSpawnSpell";
     private static readonly ProtoId<TagPrototype> CannotSuicideTag = "CannotSuicide";
+    private static readonly ProtoId<NpcFactionPrototype> ZombieFaction = "Zombie";
+
     /// <summary>
     /// Handles an entity turning into a zombie when they die or go into crit
     /// </summary>
@@ -238,7 +241,7 @@ public sealed partial class ZombieSystem
         _mobState.ChangeMobState(target, MobState.Alive);
 
         _faction.ClearFactions(target, dirty: false);
-        _faction.AddFaction(target, "Zombie");
+        _faction.AddFaction(target, ZombieFaction);
         EnsureComp<NoFriendlyFireComponent>(target); // DeltaV - prevent shitters biting other zombies
 
         //gives it the funny "Zombie ___" name.

Content.Shared/NPC/Systems/NpcFactionSystem.cs

@@ -73,7 +73,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
     /// <summary>
     /// Returns whether an entity is a member of a faction.
     /// </summary>
-    public bool IsMember(Entity<NpcFactionMemberComponent?> ent, string faction)
+    public bool IsMember(Entity<NpcFactionMemberComponent?> ent, [ForbidLiteral] string faction)
     {
         if (!Resolve(ent, ref ent.Comp, false))
             return false;
@@ -85,7 +85,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
     /// Returns whether an entity is a member of any listed faction.
     /// If the list is empty this returns false.
     /// </summary>
-    public bool IsMemberOfAny(Entity<NpcFactionMemberComponent?> ent, IEnumerable<ProtoId<NpcFactionPrototype>> factions)
+    public bool IsMemberOfAny(Entity<NpcFactionMemberComponent?> ent, [ForbidLiteral] IEnumerable<ProtoId<NpcFactionPrototype>> factions)
     {
         if (!Resolve(ent, ref ent.Comp, false))
             return false;
@@ -102,7 +102,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
     /// <summary>
     /// Adds this entity to the particular faction.
     /// </summary>
-    public void AddFaction(Entity<NpcFactionMemberComponent?> ent, string faction, bool dirty = true)
+    public void AddFaction(Entity<NpcFactionMemberComponent?> ent, [ForbidLiteral] string faction, bool dirty = true)
     {
         if (!_proto.HasIndex<NpcFactionPrototype>(faction))
         {
@@ -121,7 +121,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
     /// <summary>
     /// Adds this entity to the particular faction.
     /// </summary>
-    public void AddFactions(Entity<NpcFactionMemberComponent?> ent, HashSet<ProtoId<NpcFactionPrototype>> factions, bool dirty = true)
+    public void AddFactions(Entity<NpcFactionMemberComponent?> ent, [ForbidLiteral] HashSet<ProtoId<NpcFactionPrototype>> factions, bool dirty = true)
     {
         ent.Comp ??= EnsureComp<NpcFactionMemberComponent>(ent);
 
@@ -143,7 +143,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
     /// <summary>
     /// Removes this entity from the particular faction.
     /// </summary>
-    public void RemoveFaction(Entity<NpcFactionMemberComponent?> ent, string faction, bool dirty = true)
+    public void RemoveFaction(Entity<NpcFactionMemberComponent?> ent, [ForbidLiteral] string faction, bool dirty = true)
     {
         if (!_proto.HasIndex<NpcFactionPrototype>(faction))
         {
@@ -202,7 +202,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
         return GetNearbyFactions(ent, range, ent.Comp.FriendlyFactions);
     }
 
-    private IEnumerable<EntityUid> GetNearbyFactions(EntityUid entity, float range, HashSet<ProtoId<NpcFactionPrototype>> factions)
+    private IEnumerable<EntityUid> GetNearbyFactions(EntityUid entity, float range, [ForbidLiteral] HashSet<ProtoId<NpcFactionPrototype>> factions)
     {
         var xform = Transform(entity);
         foreach (var ent in _lookup.GetEntitiesInRange<NpcFactionMemberComponent>(_xform.GetMapCoordinates((entity, xform)), range))
@@ -228,12 +228,12 @@ public sealed partial class NpcFactionSystem : EntitySystem
         return ent.Comp.Factions.Overlaps(other.Comp.Factions) || ent.Comp.FriendlyFactions.Overlaps(other.Comp.Factions);
     }
 
-    public bool IsFactionFriendly(string target, string with)
+    public bool IsFactionFriendly([ForbidLiteral] string target, [ForbidLiteral] string with)
     {
         return _factions[target].Friendly.Contains(with) && _factions[with].Friendly.Contains(target);
     }
 
-    public bool IsFactionFriendly(string target, Entity<NpcFactionMemberComponent?> with)
+    public bool IsFactionFriendly([ForbidLiteral] string target, Entity<NpcFactionMemberComponent?> with)
     {
         if (!Resolve(with, ref with.Comp, false))
             return false;
@@ -242,12 +242,12 @@ public sealed partial class NpcFactionSystem : EntitySystem
                with.Comp.FriendlyFactions.Contains(target);
     }
 
-    public bool IsFactionHostile(string target, string with)
+    public bool IsFactionHostile([ForbidLiteral] string target, [ForbidLiteral] string with)
     {
         return _factions[target].Hostile.Contains(with) && _factions[with].Hostile.Contains(target);
     }
 
-    public bool IsFactionHostile(string target, Entity<NpcFactionMemberComponent?> with)
+    public bool IsFactionHostile([ForbidLiteral] string target, Entity<NpcFactionMemberComponent?> with)
     {
         if (!Resolve(with, ref with.Comp, false))
             return false;
@@ -256,7 +256,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
                with.Comp.HostileFactions.Contains(target);
     }
 
-    public bool IsFactionNeutral(string target, string with)
+    public bool IsFactionNeutral([ForbidLiteral] string target, [ForbidLiteral] string with)
     {
         return !IsFactionFriendly(target, with) && !IsFactionHostile(target, with);
     }
@@ -264,7 +264,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
     /// <summary>
     /// Makes the source faction friendly to the target faction, 1-way.
     /// </summary>
-    public void MakeFriendly(string source, string target)
+    public void MakeFriendly([ForbidLiteral] string source, [ForbidLiteral] string target)
     {
         if (!_factions.TryGetValue(source, out var sourceFaction))
         {
@@ -286,7 +286,7 @@ public sealed partial class NpcFactionSystem : EntitySystem
     /// <summary>
     /// Makes the source faction hostile to the target faction, 1-way.
     /// </summary>
-    public void MakeHostile(string source, string target)
+    public void MakeHostile([ForbidLiteral] string source, [ForbidLiteral] string target)
     {
         if (!_factions.TryGetValue(source, out var sourceFaction))
         {