Add funtion for switching base rsi and layer data at the same time

[korczoczek]

Bane of my existence. Setting the base rsi first logs an error, changing layer data first also logs an error. Gotta do it at the same time.

Added a parameter to SetBaseRsi stopping the error from being logged
Added an overload method with the previous behavior, so no breaking changes
Added a new function SetBaseRsiWithLayers receiving the sprite to be changed, the new RSI path and a dict of new layer data to be updated

Part of space-wizards/space-station-14/pull/43410

[moonheart08]

This seems like a bodge over a content-side error (having layers with invalid states), which is never supported and making this not log masks wider issues.

[korczoczek]

Well, not exactly. As the comment says, you're supposed to modify layer info in conjuction with changing the base rsi, but you have to do it one after another.

What's more, changing the layer data first logs errors both for missing layers in the current rsi and the previous states being missing in the new rsi (because of them still being cached i assume).

In practice, the code works even without the engine change, both the rsi and layers get modified in the same tick, but a big mean error shows up to tell you that the no longer defined layers don't exist.

And even in the case where a dev explicitly calls the function with logging disabled, but doesn't set up proper layer switching and the sprite ends up in an invalid state, then it will just show up as the error texture.

[moonheart08]

Well, not exactly. As the comment says, you're supposed to modify layer info in conjuction with changing the base rsi, but you have to do it one after another.

What's more, changing the layer data first logs errors both for missing layers in the current rsi and the previous states being missing in the new rsi (because of them still being cached i assume).

In practice, the code works even without the engine change, both the rsi and layers get modified in the same tick, but a big mean error shows up to tell you that the no longer defined layers don't exist.

And even in the case where a dev explicitly calls the function with logging disabled, but doesn't set up proper layer switching and the sprite ends up in an invalid state, then it will just show up as the error texture.

If you have a map of old layers to new layers, why not make a new method for doing that transform atomically (i.e. takes a dict of layers to modify or an action that is expected to do fixup)?

Then we never have layers in an invalid state in a way that can result in no warnings.