Add support for per-ruleset skin layouts #22673
Conversation
Also adds per-ruleset storage for each container type.
peppy
force-pushed
the
skin-per-ruleset-layouts
branch
from
2e3bbd3 to
a01c309
Compare
osu.Game/Skinning/Skin.cs
Outdated
| // handle namespace changes... | ||
| // can be removed 2023-01-31 | ||
| jsonContent = jsonContent.Replace(@"osu.Game.Screens.Play.SongProgress", @"osu.Game.Screens.Play.HUD.DefaultSongProgress"); | ||
| jsonContent = jsonContent.Replace(@"osu.Game.Screens.Play.HUD.LegacyComboCounter", @"osu.Game.Skinning.LegacyComboCounter"); |
There was a problem hiding this comment.
Can we remove these while we're here? The deprecation date is up on these anyways.
There was a problem hiding this comment.
As you touched on in your other comment, this deprecation notice relies on the user having edited skins since the change. I've been holding off removing these because I'm scared it will cause breakage.
As you say, it's probably best we force migration at some point so I'll remove the deprecation notice here and update the comments.
osu.Game/Skinning/Skin.cs
Outdated
| { | ||
| } | ||
|
|
||
| // if that fails, attempt to deserialise using the old naked list. |
There was a problem hiding this comment.
Are we putting a deprecation date on this pathway, or is it going to have to live forever? Right now it's on the read side, so to get rid of it we'd have to automigrate users' skins somehow I imagine.
There was a problem hiding this comment.
I guess we'll do this one day. I'll note this in the inline comments.
Co-authored-by: Bartłomiej Dach <[email protected]>
Co-authored-by: Bartłomiej Dach <[email protected]>
- It is entirely derived from `SkinLayoutInfo.DrawableInfo`, which is the actual primary thing we want to serialise. - It will never get read out from any serialised files anyway (corollary of the previous point - it is a get-only property derived from another). - It is only used in tests. All of the three reasons above make serialising the property out to skin files nothing more than a waste of space.
|
This seems pretty alright, and it doesn't seem to break under some light testing. I did apply a few fixups: one is just a slight comment reword (4372447), while the second is slightly more major but I'm pretty sure it shouldn't be controversial by any means - upon inspecting exported skin files, I noticed that To speed things up some I'll queue this up for merge now. Let's see where this goes next 👍 |
This builds off @frenzibyte's original proposal (see #20694) and mostly implements things in a similar way.
Of note, each skin components container can now have both global and per-ruleset components defined. This is done by serialising a new
SkinLayoutInfotype rather than a nakedIEnumerable<SerialisableDrawableInfo>. Includes "migration" from previous skins via advancedtry-catchchecking.This completes a good portion of #22388, but does not yet add updated defaults to, for instance, correct non-matching layouts in legacy skins (mania / taiko).