changes SpriteComponent to use object as LayerKey #6025
Conversation
PJB3005
added
the
S: Requires Content PR
| @@ -60,7 +60,7 @@ public sealed partial class PrototypeCopyToShaderParameters | |||
| /// <summary> | |||
| /// The map key of the layer that will have its shader modified. | |||
| /// </summary> | |||
| [DataField(required: true)] public string LayerKey; | |||
| [DataField(required: true)] public object? LayerKey; | |||
There was a problem hiding this comment.
Why are you making this nullable?
There was a problem hiding this comment.
It's required to compile. Ever assigning null here should be impossible, so I am pretty certain it would be fine to set as non-nullable (had it like that initially, actually, as I find setting types which don't actually expect to ever have null as nullable just... confusing, and possibly error-prone) - but the build process gets angry with me if I try that (looks like the serialisation generator really does not like it?) `:D
There is of course always a possibility I am missing larger implications here, which a developer more seasoned with the codebase would catch. Fully agree that this shouldn't be nullable, just haven't found a way to not have it so.
I suspect that was not an issue before, because string natively handles null as an empty string in most cases IIRC.
The layer/displacement system uses objects as keys throughout, except in this one place, which is causing issues when one tries to apply a displacement to a layer using a HumanoidVisualLayers-object as a key, which - as far as I can tell - is the entire reason the system got switched from String to Object in the first place.
Goes together with sister PR on the non-engine side of things:
Requires space-wizards/space-station-14#38351