Validate prototype ids in c# fields (#18224)

Content.Client/Construction/UI/ConstructionMenuPresenter.cs

@@ -404,7 +404,7 @@ namespace Content.Client.Construction.UI
                 return;
             }
 
-            if (_selected == null || _selected.Mirror == String.Empty)
+            if (_selected == null || _selected.Mirror == null)
             {
                 return;
             }

Content.Client/Decals/DecalPlacementSystem.cs

@@ -197,7 +197,7 @@ public sealed class DecalPlacementSystem : EntitySystem
 
 public sealed class PlaceDecalActionEvent : WorldTargetActionEvent
 {
-    [DataField("decalId", customTypeSerializer:typeof(PrototypeIdSerializer<DecalPrototype>))]
+    [DataField("decalId", customTypeSerializer:typeof(PrototypeIdSerializer<DecalPrototype>), required:true)]
     public string DecalId = string.Empty;
 
     [DataField("color")]

Content.Client/Salvage/SalvageMagnetComponent.cs

@@ -1,5 +1,7 @@
 using Content.Shared.Salvage;
 using Robust.Shared.GameStates;
 
+namespace Content.Client.Salvage;
+
 [NetworkedComponent, RegisterComponent]
 public sealed class SalvageMagnetComponent : SharedSalvageMagnetComponent {}

Content.Server/Advertise/AdvertiseComponent.cs

@@ -24,7 +24,7 @@ namespace Content.Server.Advertise
         /// <summary>
         ///     The identifier for the advertisements pack prototype.
         /// </summary>
-        [DataField("pack", customTypeSerializer:typeof(PrototypeIdSerializer<AdvertisementsPackPrototype>))]
+        [DataField("pack", customTypeSerializer:typeof(PrototypeIdSerializer<AdvertisementsPackPrototype>), required: true)]
         public string PackPrototypeId { get; } = string.Empty;
 
         /// <summary>

Content.Server/Ame/Components/AmePartComponent.cs

@@ -1,5 +1,5 @@
+using Content.Shared.Tools;
 using Robust.Shared.Audio;
-using Robust.Shared.Prototypes;
 using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototype;
 
 namespace Content.Server.Ame.Components;
@@ -19,6 +19,6 @@ public sealed class AmePartComponent : Component
     /// <summary>
     /// The tool quality required to deploy the packaged AME shielding.
     /// </summary>
-    [DataField("qualityNeeded", customTypeSerializer: typeof(PrototypeIdSerializer<EntityPrototype>))]
+    [DataField("qualityNeeded", customTypeSerializer: typeof(PrototypeIdSerializer<ToolQualityPrototype>))]
     public string QualityNeeded = "Pulsing";
 }

Content.Server/GameTicking/Rules/Components/NukeOperativeSpawnerComponent.cs

@@ -1,3 +1,6 @@
+using Content.Shared.Roles;
+using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototype;
+
 namespace Content.Server.GameTicking.Rules.Components;
 
 /// <summary>
@@ -8,12 +11,12 @@ namespace Content.Server.GameTicking.Rules.Components;
 [RegisterComponent]
 public sealed class NukeOperativeSpawnerComponent : Component
 {
-    [DataField("name")]
+    [DataField("name", required:true)]
-    public string OperativeName = "";
+    public string OperativeName = default!;
 
-    [DataField("rolePrototype")]
+    [DataField("rolePrototype", customTypeSerializer:typeof(PrototypeIdSerializer<AntagPrototype>), required:true)]
-    public string OperativeRolePrototype = "";
+    public string OperativeRolePrototype = default!;
 
-    [DataField("startingGearPrototype")]
+    [DataField("startingGearPrototype", customTypeSerializer:typeof(PrototypeIdSerializer<StartingGearPrototype>), required:true)]
-    public string OperativeStartingGear = "";
+    public string OperativeStartingGear = default!;
 }

Content.Server/GameTicking/Rules/Components/NukeopsRuleComponent.cs

@@ -6,6 +6,7 @@ using Content.Shared.Roles;
 using Robust.Server.Player;
 using Robust.Shared.Audio;
 using Robust.Shared.Map;
+using Robust.Shared.Prototypes;
 using Robust.Shared.Serialization.TypeSerializers.Implementations;
 using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototype;
 using Robust.Shared.Serialization.TypeSerializers.Implementations.Custom.Prototype.Set;
@@ -43,19 +44,19 @@ public sealed class NukeopsRuleComponent : Component
     [DataField("spawnOutpost")]
     public bool SpawnOutpost = true;
 
-    [DataField("spawnPointProto", customTypeSerializer: typeof(PrototypeIdSerializer<StartingGearPrototype>))]
+    [DataField("spawnPointProto", customTypeSerializer: typeof(PrototypeIdSerializer<EntityPrototype>))]
     public string SpawnPointPrototype = "SpawnPointNukies";
 
-    [DataField("ghostSpawnPointProto", customTypeSerializer: typeof(PrototypeIdSerializer<StartingGearPrototype>))]
+    [DataField("ghostSpawnPointProto", customTypeSerializer: typeof(PrototypeIdSerializer<EntityPrototype>))]
     public string GhostSpawnPointProto = "SpawnPointGhostNukeOperative";
 
-    [DataField("commanderRoleProto", customTypeSerializer: typeof(PrototypeIdSerializer<StartingGearPrototype>))]
+    [DataField("commanderRoleProto", customTypeSerializer: typeof(PrototypeIdSerializer<AntagPrototype>))]
     public string CommanderRolePrototype = "NukeopsCommander";
 
-    [DataField("operativeRoleProto", customTypeSerializer: typeof(PrototypeIdSerializer<StartingGearPrototype>))]
+    [DataField("operativeRoleProto", customTypeSerializer: typeof(PrototypeIdSerializer<AntagPrototype>))]
     public string OperativeRoleProto = "Nukeops";
 
-    [DataField("medicRoleProto", customTypeSerializer: typeof(PrototypeIdSerializer<StartingGearPrototype>))]
+    [DataField("medicRoleProto", customTypeSerializer: typeof(PrototypeIdSerializer<AntagPrototype>))]
     public string MedicRoleProto = "NukeopsMedic";
 
     [DataField("commanderStartingGearProto", customTypeSerializer: typeof(PrototypeIdSerializer<StartingGearPrototype>))]

Content.Server/GameTicking/Rules/NukeopsRuleSystem.cs

@@ -574,6 +574,15 @@ public sealed class NukeopsRuleSystem : GameRuleSystem<NukeopsRuleComponent>
         // todo: this is kinda awful for multi-nukies
         foreach (var nukeops in EntityQuery<NukeopsRuleComponent>())
         {
+            if (nukeOpSpawner.OperativeName == null
+                || nukeOpSpawner.OperativeStartingGear == null
+                || nukeOpSpawner.OperativeRolePrototype == null)
+            {
+                // I have no idea what is going on with nuke ops code, but I'm pretty sure this shouldn't be possible.
+                Log.Error($"Invalid nuke op spawner: {ToPrettyString(spawner)}");
+                continue;
+            }
+
             SetupOperativeEntity(uid, nukeOpSpawner.OperativeName, nukeOpSpawner.OperativeStartingGear, profile, nukeops);
 
             nukeops.OperativeMindPendingData.Add(uid, nukeOpSpawner.OperativeRolePrototype);

Content.Server/Objectives/Requirements/NotRoleRequirement.cs

@@ -8,8 +8,8 @@ namespace Content.Server.Objectives.Requirements
     [DataDefinition]
     public sealed class NotRoleRequirement : IObjectiveRequirement
     {
-        [DataField("roleId", customTypeSerializer:typeof(PrototypeIdSerializer<JobPrototype>))]
+        [DataField("roleId", customTypeSerializer:typeof(PrototypeIdSerializer<JobPrototype>), required:true)]
-        private string roleId = "";
+        private string _roleId = default!;
 
         /// <summary>
         /// This requirement is met if the traitor is NOT the roleId, and fails if they are.
@@ -19,7 +19,7 @@ namespace Content.Server.Objectives.Requirements
             if (mind.CurrentJob == null) // no job no problems
                 return true;
 
-            return (mind.CurrentJob.Prototype.ID != roleId);
+            return (mind.CurrentJob.Prototype.ID != _roleId);
         }
     }
 }

Content.Shared/Body/Prototypes/BodyPrototype.cs

@@ -34,6 +34,10 @@ public sealed record BodyPrototypeSlot
     public readonly HashSet<string> Connections = new();
     public readonly Dictionary<string, string> Organs = new();
 
+    public BodyPrototypeSlot() : this(null, null, null)
+    {
+    }
+
     public BodyPrototypeSlot(string? part, HashSet<string>? connections, Dictionary<string, string>? organs)
     {
         Part = part;

Content.Shared/Cargo/CargoBountyData.cs

@@ -20,7 +20,8 @@ public readonly record struct CargoBountyData(int Id, string Bounty, TimeSpan En
     /// <summary>
     /// The prototype containing information about the bounty.
     /// </summary>
-    [DataField("bounty", customTypeSerializer: typeof(PrototypeIdSerializer<CargoBountyPrototype>)), ViewVariables(VVAccess.ReadWrite)]
+    [ViewVariables(VVAccess.ReadWrite)]
+    [DataField("bounty", customTypeSerializer: typeof(PrototypeIdSerializer<CargoBountyPrototype>), required:true)]
     public readonly string Bounty = Bounty;
 
     /// <summary>
@@ -28,4 +29,8 @@ public readonly record struct CargoBountyData(int Id, string Bounty, TimeSpan En
     /// </summary>
     [DataField("endTime", customTypeSerializer: typeof(TimeOffsetSerializer))]
     public readonly TimeSpan EndTime = EndTime;
+
+    public CargoBountyData() : this(default, string.Empty, default)
+    {
+    }
 }

Content.Shared/Chemistry/Components/Solution.cs

@@ -676,9 +676,9 @@ namespace Content.Shared.Chemistry.Components
         [DataDefinition]
         public readonly struct ReagentQuantity: IComparable<ReagentQuantity>
         {
-            [DataField("ReagentId", customTypeSerializer:typeof(PrototypeIdSerializer<ReagentPrototype>))]
+            [DataField("ReagentId", customTypeSerializer:typeof(PrototypeIdSerializer<ReagentPrototype>), required:true)]
             public readonly string ReagentId;
-            [DataField("Quantity")]
+            [DataField("Quantity", required:true)]
             public readonly FixedPoint2 Quantity;
 
             public ReagentQuantity(string reagentId, FixedPoint2 quantity)
@@ -687,6 +687,10 @@ namespace Content.Shared.Chemistry.Components
                 Quantity = quantity;
             }
 
+            public ReagentQuantity() : this(string.Empty, default)
+            {
+            }
+
             [ExcludeFromCodeCoverage]
             public override string ToString()
             {

Content.Shared/Clothing/EntitySystems/ClothingSystem.cs

@@ -14,6 +14,9 @@ public abstract class ClothingSystem : EntitySystem
     [Dependency] private readonly SharedHumanoidAppearanceSystem _humanoidSystem = default!;
     [Dependency] private readonly TagSystem _tagSystem = default!;
 
+    [ValidatePrototypeId<TagPrototype>]
+    private const string HairTag = "HidesHair";
+
     public override void Initialize()
     {
         base.Initialize();
@@ -27,14 +30,14 @@ public abstract class ClothingSystem : EntitySystem
     protected virtual void OnGotEquipped(EntityUid uid, ClothingComponent component, GotEquippedEvent args)
     {
         component.InSlot = args.Slot;
-        if (args.Slot == "head" && _tagSystem.HasTag(args.Equipment, "HidesHair"))
+        if (args.Slot == "head" && _tagSystem.HasTag(args.Equipment, HairTag))
             _humanoidSystem.SetLayerVisibility(args.Equipee, HumanoidVisualLayers.Hair, false);
     }
 
     protected virtual void OnGotUnequipped(EntityUid uid, ClothingComponent component, GotUnequippedEvent args)
     {
         component.InSlot = null;
-        if (args.Slot == "head" && _tagSystem.HasTag(args.Equipment, "HidesHair"))
+        if (args.Slot == "head" && _tagSystem.HasTag(args.Equipment, HairTag))
             _humanoidSystem.SetLayerVisibility(args.Equipee, HumanoidVisualLayers.Hair, true);
     }
 

Content.Shared/Construction/Prototypes/ConstructionPrototype.cs

@@ -31,7 +31,7 @@ public sealed class ConstructionPrototype : IPrototype
     /// <summary>
     ///     The <see cref="ConstructionGraphPrototype"/> this construction will be using.
     /// </summary>
-    [DataField("graph", customTypeSerializer:typeof(PrototypeIdSerializer<ConstructionGraphPrototype>))]
+    [DataField("graph", customTypeSerializer:typeof(PrototypeIdSerializer<ConstructionGraphPrototype>), required: true)]
     public string Graph = string.Empty;
 
     /// <summary>
@@ -85,7 +85,7 @@ public sealed class ConstructionPrototype : IPrototype
     ///     Construction to replace this construction with when the current one is 'flipped'
     /// </summary>
     [DataField("mirror", customTypeSerializer:typeof(PrototypeIdSerializer<ConstructionPrototype>))]
-    public string Mirror = string.Empty;
+    public string? Mirror;
 
     public IReadOnlyList<IConstructionCondition> Conditions => _conditions;
     public IReadOnlyList<SpriteSpecifier> Layers => _layers ?? new List<SpriteSpecifier>{Icon};

Content.Shared/Devour/Components/DevourerComponent.cs

@@ -14,9 +14,6 @@ namespace Content.Server.Devour.Components;
 [Access(typeof(SharedDevourSystem))]
 public sealed class DevourerComponent : Component
 {
-    [DataField("devourActionId", customTypeSerializer: typeof(PrototypeIdSerializer<EntityTargetActionPrototype>))]
-    public string DevourActionId = "Devour";
-
     [DataField("devourAction")]
     public EntityTargetAction? DevourAction;
 

Content.Shared/Roles/StartingGearPrototype.cs

@@ -14,13 +14,13 @@ namespace Content.Shared.Roles
         /// if empty, there is no skirt override - instead the uniform provided in equipment is added.
         /// </summary>
         [DataField("innerclothingskirt", customTypeSerializer:typeof(PrototypeIdSerializer<EntityPrototype>))]
-        private string _innerClothingSkirt = string.Empty;
+        private string? _innerClothingSkirt;
 
         [DataField("satchel", customTypeSerializer:typeof(PrototypeIdSerializer<EntityPrototype>))]
-        private string _satchel = string.Empty;
+        private string? _satchel;
 
         [DataField("duffelbag", customTypeSerializer:typeof(PrototypeIdSerializer<EntityPrototype>))]
-        private string _duffelbag = string.Empty;
+        private string? _duffelbag;
 
         public IReadOnlyDictionary<string, string> Inhand => _inHand;
         /// <summary>

Content.Shared/Salvage/Expeditions/Modifiers/SalvageDungeonMod.cs

@@ -23,6 +23,6 @@ public sealed class SalvageDungeonMod : IPrototype, IBiomeSpecificMod
     /// <summary>
     /// The config to use for spawning the dungeon.
     /// </summary>
-    [DataField("proto", customTypeSerializer: typeof(PrototypeIdSerializer<DungeonConfigPrototype>))]
+    [DataField("proto", customTypeSerializer: typeof(PrototypeIdSerializer<DungeonConfigPrototype>), required: true)]
     public string Proto = string.Empty;
 }

Content.Shared/Traits/TraitPrototype.cs

@@ -49,6 +49,6 @@ namespace Content.Shared.Traits
         ///     Gear that is given to the player, when they pick this trait.
         /// </summary>
         [DataField("traitGear", required: false, customTypeSerializer:typeof(PrototypeIdSerializer<EntityPrototype>))]
-        public string TraitGear = string.Empty;
+        public string? TraitGear;
     }
 }

Content.Shared/VendingMachines/VendingMachineComponent.cs

@@ -14,7 +14,7 @@ namespace Content.Shared.VendingMachines
         /// <summary>
         /// PrototypeID for the vending machine's inventory, see <see cref="VendingMachineInventoryPrototype"/>
         /// </summary>
-        [DataField("pack", customTypeSerializer: typeof(PrototypeIdSerializer<VendingMachineInventoryPrototype>))]
+        [DataField("pack", customTypeSerializer: typeof(PrototypeIdSerializer<VendingMachineInventoryPrototype>), required: true)]
         public string PackPrototypeId = string.Empty;
 
         /// <summary>

Content.YAMLLinter/Program.cs

@@ -7,6 +7,7 @@ using Robust.Shared.Prototypes;
 using Robust.Shared.Serialization.Markdown.Validation;
 using Robust.Shared.Timing;
 using Robust.Shared.Utility;
+using Robust.UnitTesting;
 
 namespace Content.YAMLLinter
 {
@@ -17,9 +18,11 @@ namespace Content.YAMLLinter
             var stopwatch = new Stopwatch();
             stopwatch.Start();
 
-            var errors = await RunValidation();
+            var (errors, fieldErrors) = await RunValidation();
 
-            if (errors.Count == 0)
+            var count = errors.Count + fieldErrors.Count;
+
+            if (count == 0)
             {
                 Console.WriteLine($"No errors found in {(int) stopwatch.Elapsed.TotalMilliseconds} ms.");
                 return 0;
@@ -33,80 +36,92 @@ namespace Content.YAMLLinter
                 }
             }
 
-            Console.WriteLine($"{errors.Count} errors found in {(int) stopwatch.Elapsed.TotalMilliseconds} ms.");
+            foreach (var error in fieldErrors)
+            {
+                Console.WriteLine(error);
+            }
+
+            Console.WriteLine($"{count} errors found in {(int) stopwatch.Elapsed.TotalMilliseconds} ms.");
             return -1;
         }
 
-        private static async Task<Dictionary<string, HashSet<ErrorNode>>> ValidateClient()
+        private static async Task<(Dictionary<string, HashSet<ErrorNode>> YamlErrors, List<string> FieldErrors)>
+            ValidateClient()
         {
             await using var pairTracker = await PoolManager.GetServerClient(new PoolSettings { DummyTicker = true, Disconnected = true });
             var client = pairTracker.Pair.Client;
-
+            var result = await ValidateInstance(client);
-            var cPrototypeManager = client.ResolveDependency<IPrototypeManager>();
-            var clientErrors = new Dictionary<string, HashSet<ErrorNode>>();
-
-            await client.WaitPost(() =>
-            {
-                clientErrors = cPrototypeManager.ValidateDirectory(new ResPath("/Prototypes"));
-            });
-
             await pairTracker.CleanReturnAsync();
-
+            return result;
-            return clientErrors;
         }
 
-        private static async Task<Dictionary<string, HashSet<ErrorNode>>> ValidateServer()
+        private static async Task<(Dictionary<string, HashSet<ErrorNode>> YamlErrors, List<string> FieldErrors)>
+            ValidateServer()
         {
-            await using var pairTracker = await PoolManager.GetServerClient(new PoolSettings { DummyTicker = true, Disconnected = true });
+            await using var pairTracker = await PoolManager.GetServerClient(new PoolSettings { DummyTicker = true, NoClient = true });
             var server = pairTracker.Pair.Server;
-
+            var result = await ValidateInstance(server);
-            var sPrototypeManager = server.ResolveDependency<IPrototypeManager>();
-            var serverErrors = new Dictionary<string, HashSet<ErrorNode>>();
-
-            await server.WaitPost(() =>
-            {
-                serverErrors = sPrototypeManager.ValidateDirectory(new ResPath("/Prototypes"));
-            });
-
             await pairTracker.CleanReturnAsync();
-
+            return result;
-            return serverErrors;
         }
 
-        public static async Task<Dictionary<string, HashSet<ErrorNode>>> RunValidation()
+        private static async Task<(Dictionary<string, HashSet<ErrorNode>>, List<string>)> ValidateInstance(
+            RobustIntegrationTest.IntegrationInstance instance)
         {
-            var allErrors = new Dictionary<string, HashSet<ErrorNode>>();
+            var protoMan = instance.ResolveDependency<IPrototypeManager>();
+            Dictionary<string, HashSet<ErrorNode>> yamlErrors = default!;
+            List<string> fieldErrors = default!;
+
+            await instance.WaitPost(() =>
+            {
+                yamlErrors = protoMan.ValidateDirectory(new ResPath("/Prototypes"), out var prototypes);
+                fieldErrors = protoMan.ValidateFields(prototypes);
+            });
+
+            return (yamlErrors, fieldErrors);
+        }
+
+        public static async Task<(Dictionary<string, HashSet<ErrorNode>> YamlErrors , List<string> FieldErrors)>
+            RunValidation()
+        {
+            var yamlErrors = new Dictionary<string, HashSet<ErrorNode>>();
 
             var serverErrors = await ValidateServer();
             var clientErrors = await ValidateClient();
 
-            foreach (var (key, val) in serverErrors)
+            foreach (var (key, val) in serverErrors.YamlErrors)
             {
                 // Include all server errors marked as always relevant
                 var newErrors = val.Where(n => n.AlwaysRelevant).ToHashSet();
 
                 // We include sometimes-relevant errors if they exist both for the client & server
-                if (clientErrors.TryGetValue(key, out var clientVal))
+                if (clientErrors.Item1.TryGetValue(key, out var clientVal))
                     newErrors.UnionWith(val.Intersect(clientVal));
 
                 if (newErrors.Count != 0)
-                    allErrors[key] = newErrors;
+                    yamlErrors[key] = newErrors;
             }
 
-            // Finally add any always-relevant client errors.
+            // Next add any always-relevant client errors.
-            foreach (var (key, val) in clientErrors)
+            foreach (var (key, val) in clientErrors.YamlErrors)
             {
                 var newErrors = val.Where(n => n.AlwaysRelevant).ToHashSet();
                 if (newErrors.Count == 0)
                     continue;
 
-                if (allErrors.TryGetValue(key, out var errors))
+                if (yamlErrors.TryGetValue(key, out var errors))
                     errors.UnionWith(val.Where(n => n.AlwaysRelevant));
                 else
-                    allErrors[key] = newErrors;
+                    yamlErrors[key] = newErrors;
             }
 
-            return allErrors;
+            // Finally, combine the prototype ID field errors.
+            var fieldErrors = serverErrors.FieldErrors
+                .Concat(clientErrors.FieldErrors)
+                .Distinct()
+                .ToList();
+
+            return (yamlErrors, fieldErrors);
         }
     }
 }

Resources/Prototypes/Entities/Markers/Spawners/ghost_roles.yml

@@ -66,7 +66,6 @@
     rules: You are a syndicate operative tasked with the destruction of the station. As an antagonist, do whatever is required to complete this task.
   - type: GhostRoleMobSpawner
     prototype: MobHumanNukeOp
-  - type: NukeOperativeSpawner
   - type: Sprite
     sprite: Markers/jobs.rsi
     layers: