Add skin cycling with shortcuts for next and previous skin #36387
Conversation
| new KeyBinding(new[] { InputKey.Control, InputKey.Alt, InputKey.R }, GlobalAction.ResetInputSettings), | ||
|
|
||
| new KeyBinding(new[] { InputKey.Control, InputKey.Shift, InputKey.R }, GlobalAction.RandomSkin), | ||
| new KeyBinding(new[] { InputKey.Control, InputKey.Shift, InputKey.Down }, GlobalAction.NextSkin), |
There was a problem hiding this comment.
I don't agree with these as default bindings. Open to better proposals.
There was a problem hiding this comment.
The only alternative I can only think of is ctrl+shift+n / ctrl+shift+p for next/previous - fits with the current one for random skin, but then changing skins would use three ctrl-shift-letters. Maybe we want to keep those for something else
| /// "Press {0} to go to the next skin!" | ||
| /// </summary> | ||
| public static LocalisableString NextSkinShortcut(LocalisableString keybind) => new TranslatableString(getKey(@"next_skin_shortcut"), @"Press {0} to go to the next skin!", keybind); | ||
|
|
||
| /// <summary> | ||
| /// "Press {0} to go to the previous skin!" | ||
| /// </summary> | ||
| public static LocalisableString PreviousSkinShortcut(LocalisableString keybind) => new TranslatableString(getKey(@"previous_skin_shortcut"), @"Press {0} to go to the previous skin!", keybind); | ||
|
|
There was a problem hiding this comment.
I don't think these should be individual menu tips. If anything there should be one explaining all the skin traversal hotkeys.
Also we're probably adding favourite skins soon. I dunno how that will sit with this feature. Maybe the traversal should only be over favourited skins rather than all?
There was a problem hiding this comment.
I think it would be easiest to have a settings checkbox for only filtering through favorites, should be trivial to implement whenever that lands.
I changed RandomSkinShortcut to SkinChangeShortcuts and merged the tooltips into one with this text (barring the old shortcuts):
osu.Game/Skinning/SkinManager.cs
Outdated
| if (CurrentSkinInfo.Disabled) | ||
| return; | ||
|
|
||
| // Required local for iOS. Will cause runtime crash if inlined. |
There was a problem hiding this comment.
Where are you seeing this????????
|
Ok, this should address everything, only thing left is to find better shortcuts. |
|
my best proposal if we do want default binds are probably E/T, as in left and right on keyboard from the "random skin" binding. |
2 participants
This adds shortcuts for 'next skin' and 'previous skin', letting players switch skins more consistently than the 'random skin' option.
To implement this I had to I move the skin dropdown list generation (default skins + 'random skin' + user unprotected skins) into a separate function
GetDropdownItems()to avoid duplication. The behavior stays the same from my testing with importing/deleting skins.I added
CycleSkins(int step), then two wrappers for it (SelectNextSkinandSelectPreviousSkin), which are called by the global shortcuts.There was also a private SkinInfo object called random_skin_info that I removed in favor of comparisons with the public
SkinInfo.RANDOM_SKINID (there was only one comparison to the object).However, whenever the skin is changed for any reason, this tooltip shows up showing the random skin shortcut. I'll leave it, since cramming the two new tooltips into it would be ugly.
