Allow more localisation in skin editor

[SchiavoAnto]

This PR allows for more strings in the skin editor to be localised.

As a note, I wanted to localise the dropdown below as well, but the strings it uses come from Description attributes which expect a constant, so they cannot be passed a LocalisableString (or its value), so not sure how to proceed with that.

[Joehuu]

As a note, I wanted to localise the dropdown below as well, but the strings it uses come from Description attributes which expect a constant, so they cannot be passed a LocalisableString (or its value), so not sure how to proceed with that.

If you use VSCode and similarly other IDEs, you can use the "quick fix" feature where the raw string is underlined with three dots:

Code_I8UCLueMpr.mp4

I saw the documenting how to localise discussion. Might get the time to document this for you and others.

[SchiavoAnto]

Oh thank you, I knew to use that method to add new strings, but I added them manually for this PR because adding translations to SkinSelectionHandler would have created another file, while I thought it would be better to use SkinEditorStrings instead.

I will add them now and commit.

[Joehuu]

Oh thank you, I knew to use that method to add new strings, but I added them manually for this PR because adding translations to SkinSelectionHandler would have created another file, while I thought it would be better to use SkinEditorStrings instead.

What you did is right (putting the strings in SkinEditorStrings). You can first use that method to auto generate the line (with CommonStrings quick-fix), then moving it to where applicable.

[SchiavoAnto]

I accidentally committed, but it looks like it doesn't work?

[LocalisableDescription(typeof(SkinEditorStrings), nameof(SkinEditorStrings.HUD))]

I used this on all of the enum entries but it shows as the entry's name.

[Joehuu]

You would have to change these lines:

osu/osu.Game/Skinning/GlobalSkinnableContainerLookup.cs

Lines 34 to 36 in 3a8ed88

	if (Ruleset == null) return Lookup.GetDescription();
	return $"{Lookup.GetDescription()} (\"{Ruleset.Name}\" only)";

to use GetLocalisableDescription().

[SchiavoAnto]

Yes, I figured, thank you.

I think this is now fine, the only other thing that would be left to do is to localise the (\"{Ruleset.Name}\" only) text but not sure how it could be done while respecting language-specific rules (the "only" going before or after the ruleset).

[Joehuu]

This will always fallback to English. Unsure how to restructure this to not use ToString().

[smoogipoo]

I will harken back to my ruleset skinning configuration PR which added the target to the ISkinComponentLookup interface:

https://github.com/smoogipoo/osu/blob/ruleset-component-skinnin-redux/osu.Game/Skinning/ISkinComponentLookup.cs

https://github.com/smoogipoo/osu/blob/70edf15f21b3cc08668122a32ea6f49151528003/osu.Game/Overlays/SkinEditor/SkinEditor.cs#L753-L770

I still believe this is a valid path.

Otherwise, expose a Name property. There were other reasons why I moved Target into the base interface in my PR, but that would be enough for this one particular use in the interim.

[SchiavoAnto]

I just saw this, I don't really know how this would be implemented as I don't fully understand the code flow.

I was planning to open another PR to localise the gameplay overlay (during autoplay and replays), but I will wait for this in case there's the need for GetLocalisableDescription.

[bdach]

I think both the special dropdown path from here as well as adding a name property sound fine, but maybe for simplicity let's just revert this specific change for now so that this doesn't get blocked on it?

[SchiavoAnto]

I'm fine with reverting the dropdown localisation for now. I think of reverting fc1ebfd and 9cd7f2b, as without the former the dropdown entries look confusing (#30231 (comment))

[bdach]

Sure.

[SchiavoAnto]

Ok, reverted.