Allow Vulps With Human Hair To Be Shaved Without Clyde Joining The Circus (#40171)

* Revert "Disable vulpkanin human hair (#40144)"

This reverts commit d02aa1a4e2.

* You can once again shave your pet Vulp

* I can see the ass, I'm safe

* Rectified docstring as I am a good person

* I am doing this instead of playing Silksong please help

* Fix forgetting to re-add shader overriding

Content.Client/DisplacementMap/DisplacementMapSystem.cs

@@ -1,3 +1,4 @@
+using System.Diagnostics.CodeAnalysis;
 using Content.Shared.DisplacementMap;
 using Robust.Client.GameObjects;
 using Robust.Client.Graphics;
@@ -10,6 +11,11 @@ public sealed class DisplacementMapSystem : EntitySystem
     [Dependency] private readonly ISerializationManager _serialization = default!;
     [Dependency] private readonly SpriteSystem _sprite = default!;
 
+    private static string? BuildDisplacementLayerKey(object key)
+    {
+        return key.ToString() is null ? null : $"{key}-displacement";
+    }
+
     /// <summary>
     /// Attempting to apply a displacement map to a specific layer of SpriteComponent
     /// </summary>
@@ -19,21 +25,22 @@ public sealed class DisplacementMapSystem : EntitySystem
     /// <param name="key">Unique layer key, which will determine which layer to apply displacement map to</param>
     /// <param name="displacementKey">The key of the new displacement map layer added by this function.</param>
     /// <returns></returns>
-    public bool TryAddDisplacement(DisplacementData data,
+    public bool TryAddDisplacement(
+        DisplacementData data,
         Entity<SpriteComponent> sprite,
         int index,
         object key,
-        out string displacementKey)
+        [NotNullWhen(true)] out string? displacementKey
+    )
     {
-        displacementKey = $"{key}-displacement";
+        displacementKey = BuildDisplacementLayerKey(key);
-
+        if (displacementKey is null)
-        if (key.ToString() is null)
             return false;
 
-        if (data.ShaderOverride != null)
+        EnsureDisplacementIsNotOnSprite(sprite, key);
-            sprite.Comp.LayerSetShader(index, data.ShaderOverride);
 
-        _sprite.RemoveLayer(sprite.AsNullable(), displacementKey, false);
+        if (data.ShaderOverride is not null)
+            sprite.Comp.LayerSetShader(index, data.ShaderOverride);
 
         //allows you not to write it every time in the YML
         foreach (var pair in data.SizeMaps)
@@ -70,7 +77,11 @@ public sealed class DisplacementMapSystem : EntitySystem
         }
 
         var displacementLayer = _serialization.CreateCopy(displacementDataLayer, notNullableOverride: true);
-        displacementLayer.CopyToShaderParameters!.LayerKey = key.ToString() ?? "this is impossible";
+
+        // This previously assigned a string reading "this is impossible" if key.ToString eval'd to false.
+        // However, for the sake of sanity, we've changed this to assert non-null - !.
+        // If this throws an error, we're not sorry. Nanotrasen thanks you for your service fixing this bug.
+        displacementLayer.CopyToShaderParameters!.LayerKey = key.ToString()!;
 
         _sprite.AddLayer(sprite.AsNullable(), displacementLayer, index);
         _sprite.LayerMapSet(sprite.AsNullable(), displacementKey, index);
@@ -78,14 +89,18 @@ public sealed class DisplacementMapSystem : EntitySystem
         return true;
     }
 
-    /// <inheritdoc cref="TryAddDisplacement"/>
+    /// <summary>
-    [Obsolete("Use the Entity<SpriteComponent> overload")]
+    /// Ensures that the displacement map associated with the given layer key is not in the Sprite's LayerMap.
-    public bool TryAddDisplacement(DisplacementData data,
+    /// </summary>
-        SpriteComponent sprite,
+    /// <param name="sprite">The sprite to remove the displacement layer from.</param>
-        int index,
+    /// <param name="key">The key of the layer that is referenced by the displacement layer we want to remove.</param>
-        object key,
+    /// <param name="logMissing">Whether to report an error if the displacement map isn't on the sprite.</param>
-        out string displacementKey)
+    public void EnsureDisplacementIsNotOnSprite(Entity<SpriteComponent> sprite, object key)
     {
-        return TryAddDisplacement(data, (sprite.Owner, sprite), index, key, out displacementKey);
+        var displacementLayerKey = BuildDisplacementLayerKey(key);
+        if (displacementLayerKey is null)
+            return;
+
+        _sprite.RemoveLayer(sprite.AsNullable(), displacementLayerKey, false);
     }
 }

Content.Client/Humanoid/HumanoidAppearanceSystem.cs

@@ -289,25 +289,26 @@ public sealed class HumanoidAppearanceSystem : SharedHumanoidAppearanceSystem
     private void RemoveMarking(Marking marking, Entity<SpriteComponent> spriteEnt)
     {
         if (!_markingManager.TryGetMarking(marking, out var prototype))
-        {
             return;
-        }
 
         foreach (var sprite in prototype.Sprites)
         {
             if (sprite is not SpriteSpecifier.Rsi rsi)
-            {
                 continue;
-            }
 
             var layerId = $"{marking.MarkingId}-{rsi.RsiState}";
             if (!_sprite.LayerMapTryGet(spriteEnt.AsNullable(), layerId, out var index, false))
-            {
                 continue;
-            }
 
             _sprite.LayerMapRemove(spriteEnt.AsNullable(), layerId);
             _sprite.RemoveLayer(spriteEnt.AsNullable(), index);
+
+            // If this marking is one that can be displaced, we need to remove the displacement as well; otherwise
+            // altering a marking at runtime can lead to the renderer falling over.
+            // The Vulps must be shaved.
+            // (https://github.com/space-wizards/space-station-14/issues/40135).
+            if (prototype.CanBeDisplaced)
+                _displacement.EnsureDisplacementIsNotOnSprite(spriteEnt, layerId);
         }
     }
 
@@ -346,9 +347,7 @@ public sealed class HumanoidAppearanceSystem : SharedHumanoidAppearanceSystem
         var sprite = entity.Comp2;
 
         if (!_sprite.LayerMapTryGet((entity.Owner, sprite), markingPrototype.BodyPart, out var targetLayer, false))
-        {
             return;
-        }
 
         visible &= !IsHidden(humanoid, markingPrototype.BodyPart);
         visible &= humanoid.BaseLayers.TryGetValue(markingPrototype.BodyPart, out var setting)
@@ -359,9 +358,7 @@ public sealed class HumanoidAppearanceSystem : SharedHumanoidAppearanceSystem
             var markingSprite = markingPrototype.Sprites[j];
 
             if (markingSprite is not SpriteSpecifier.Rsi rsi)
-            {
+                return;
-                continue;
-            }
 
             var layerId = $"{markingPrototype.ID}-{rsi.RsiState}";
 
@@ -375,28 +372,20 @@ public sealed class HumanoidAppearanceSystem : SharedHumanoidAppearanceSystem
             _sprite.LayerSetVisible((entity.Owner, sprite), layerId, visible);
 
             if (!visible || setting == null) // this is kinda implied
-            {
                 continue;
-            }
 
             // Okay so if the marking prototype is modified but we load old marking data this may no longer be valid
             // and we need to check the index is correct.
             // So if that happens just default to white?
             if (colors != null && j < colors.Count)
-            {
                 _sprite.LayerSetColor((entity.Owner, sprite), layerId, colors[j]);
-            }
             else
-            {
                 _sprite.LayerSetColor((entity.Owner, sprite), layerId, Color.White);
-            }
 
             if (humanoid.MarkingsDisplacement.TryGetValue(markingPrototype.BodyPart, out var displacementData) && markingPrototype.CanBeDisplaced)
-            {
                 _displacement.TryAddDisplacement(displacementData, (entity.Owner, sprite), targetLayer + j + 1, layerId, out _);
         }
     }
-    }
 
     public override void SetSkinColor(EntityUid uid, Color skinColor, bool sync = true, bool verify = true, HumanoidAppearanceComponent? humanoid = null)
     {

Resources/Prototypes/Species/vulpkanin.yml

@@ -41,7 +41,6 @@
   points:
     Hair:
       points: 1
-      onlyWhitelisted: true # TODO: Vulps are meant to use human hair, however something causes hair to break if affected by a displacement map and removed. Allow human hair again when #40135 is resolved.
       required: false
     FacialHair:
       points: 1