Change key overlay to use the ordering provided by rulesets #26589
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
osu!mania already goes out of its way to order things correctly. Arguably, osu!taiko just did it wrong.
Fixes the issue originally fixed via ppy#10553 in a slightly different way, allowing more flexibility.
bdach
reviewed
Jan 17, 2024
Comment on lines
74
to
76
| new KeyBinding(InputKey.MouseLeft, TaikoAction.LeftCentre), | ||
| new KeyBinding(InputKey.J, TaikoAction.RightCentre), | ||
| new KeyBinding(InputKey.MouseRight, TaikoAction.LeftRim), |
There was a problem hiding this comment.
Doing this in this way causes revert-to-default buttons to appear in the settings overlay in the taiko section:
Something like the following:
diff --git a/osu.Game.Rulesets.Taiko/TaikoRuleset.cs b/osu.Game.Rulesets.Taiko/TaikoRuleset.cs
index b80d8218f5..24b0ec5d57 100644
--- a/osu.Game.Rulesets.Taiko/TaikoRuleset.cs
+++ b/osu.Game.Rulesets.Taiko/TaikoRuleset.cs
@@ -69,11 +69,11 @@ public class TaikoRuleset : Ruleset, ILegacyRuleset
public override IEnumerable<KeyBinding> GetDefaultKeyBindings(int variant = 0) => new[]
{
+ new KeyBinding(InputKey.MouseRight, TaikoAction.LeftRim),
new KeyBinding(InputKey.D, TaikoAction.LeftRim),
- new KeyBinding(InputKey.F, TaikoAction.LeftCentre),
new KeyBinding(InputKey.MouseLeft, TaikoAction.LeftCentre),
+ new KeyBinding(InputKey.F, TaikoAction.LeftCentre),
new KeyBinding(InputKey.J, TaikoAction.RightCentre),
- new KeyBinding(InputKey.MouseRight, TaikoAction.LeftRim),
new KeyBinding(InputKey.K, TaikoAction.RightRim),
};
fixes that, without regressing #10545. Think it's worth it?
This would also fix the test failures, by the way.
There was a problem hiding this comment.
I had a feeling that would be the case. I last minute reordered them for visual appeal without retesting.
Applying the change sounds fine.
bdach
approved these changes
Jan 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
osu!mania already goes out of its way to order things correctly. Arguably, osu!taiko just did it wrong.
See commit which fixes the original reason for adding
Order(#10553).Addresses #25944.