Add item sprite test (#21599)

Content.IntegrationTests/Pair/TestPair.Helpers.cs

@@ -1,7 +1,9 @@
 #nullable enable
+using System.Collections.Generic;
 using System.Linq;
 using Robust.Shared.GameObjects;
 using Robust.Shared.Map;
+using Robust.Shared.Prototypes;
 using Robust.UnitTesting;
 
 namespace Content.IntegrationTests.Pair;
@@ -95,4 +97,33 @@ public sealed partial class TestPair
         await Client.ExecuteCommand(cmd);
         await RunTicksSync(numTicks);
     }
+
+    /// <summary>
+    /// Retrieve all entity prototypes that have some component.
+    /// </summary>
+    public List<EntityPrototype> 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>();
+        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.ContainsKey(id))
+                list.Add(proto);
+        }
+
+        return list;
+    }
 }

Content.IntegrationTests/PoolManager.cs

@@ -425,25 +425,6 @@ we are just going to end this here to save a lot of time. This is the exception
         Assert.That(passed);
     }
 
-    /// <summary>
-    ///     Helper method that retrieves all entity prototypes that have some component.
-    /// </summary>
-    public static List<EntityPrototype> GetPrototypesWithComponent<T>(RobustIntegrationTest.IntegrationInstance instance) where T : IComponent
-    {
-        var protoMan = instance.ResolveDependency<IPrototypeManager>();
-        var compFact = instance.ResolveDependency<IComponentFactory>();
-
-        var id = compFact.GetComponentName(typeof(T));
-        var list = new List<EntityPrototype>();
-        foreach (var ent in protoMan.EnumeratePrototypes<EntityPrototype>())
-        {
-            if (ent.Components.ContainsKey(id))
-                list.Add(ent);
-        }
-
-        return list;
-    }
-
     /// <summary>
     /// Initialize the pool manager.
     /// </summary>

Content.IntegrationTests/Tests/Sprite/ItemSpriteTest.cs

@@ -0,0 +1,64 @@
+#nullable enable
+using System.Collections.Generic;
+using Content.Shared.Item;
+using Robust.Client.GameObjects;
+using Robust.Shared.GameObjects;
+using Robust.Shared.Prototypes;
+
+namespace Content.IntegrationTests.Tests.Sprite;
+
+/// <summary>
+/// This test checks that all items have a visible sprite. The general rationale is that all items can be picked up
+/// by players, thus they need to be visible and have a sprite that can be rendered on screen and in their hands GUI.
+/// This has nothing to do with in-hand sprites.
+/// </summary>
+/// <remarks>
+/// If a prototype fails this test, its probably either because it:
+/// - Should be marked abstract
+/// - inherits from BaseItem despite not being an item
+/// - Shouldn't have an item component
+/// - Is missing the required sprite information.
+/// If none of the abveo are true, it might need to be added to the list of ignored components, see
+/// <see cref="_ignored"/>
+/// </remarks>
+[TestFixture]
+public sealed class PrototypeSaveTest
+{
+    private static HashSet<string> _ignored = new()
+    {
+        // The only prototypes that should get ignored are those that REQUIRE setup to get a sprite. At that point it is
+        // the responsibility of the spawner to ensure that a valid sprite is set.
+        "HandVirtualItem"
+    };
+
+    [Test]
+    public async Task AllItemsHaveSpritesTest()
+    {
+        var settings = new PoolSettings() {Connected = true}; // client needs to be in-game
+        await using var pair = await PoolManager.GetServerClient(settings);
+        List<EntityPrototype> badPrototypes = new();
+
+        await pair.Client.WaitPost(() =>
+        {
+            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));
+                var spriteComponent = pair.Client.EntMan.GetComponentOrNull<SpriteComponent>(dummy);
+                if (spriteComponent?.Icon == null)
+                    badPrototypes.Add(proto);
+                pair.Client.EntMan.DeleteEntity(dummy);
+            }
+        });
+
+        Assert.Multiple(() =>
+        {
+            foreach (var proto in badPrototypes)
+            {
+                Assert.Fail($"Item prototype has no sprite: {proto.ID}. It should probably either be marked as abstract, not be an item, or have a valid sprite");
+            }
+        });
+
+        await pair.CleanReturnAsync();
+    }
+}

Content.IntegrationTests/Tests/StorageTest.cs

@@ -94,7 +94,7 @@ namespace Content.IntegrationTests.Tests
 
             Assert.Multiple(() =>
             {
-                foreach (var proto in PoolManager.GetPrototypesWithComponent<StorageFillComponent>(server))
+                foreach (var proto in pair.GetPrototypesWithComponent<StorageFillComponent>())
                 {
                     if (proto.HasComponent<EntityStorageComponent>(compFact))
                         continue;
@@ -174,7 +174,7 @@ namespace Content.IntegrationTests.Tests
 
             Assert.Multiple(() =>
             {
-                foreach (var proto in PoolManager.GetPrototypesWithComponent<StorageFillComponent>(server))
+                foreach (var proto in pair.GetPrototypesWithComponent<StorageFillComponent>())
                 {
                     if (proto.HasComponent<StorageComponent>(compFact))
                         continue;