Reimplement missing gameplay test hotkeys from stable #28705

bdach · 2024-07-02T13:08:09Z

Namely:

Tab to toggle autoplay
Ctrl-P to toggle pause
F1 for quick exit at initial position
F2 for exit at current position

There are more that are currently intentionally unimplemented:

Ctrl-B would add bookmarks, but bookmarks are currently completely broken, so that has to get fixed first (Editor does not read properties ignored on BeatmapInfo correctly #20883)
F3 would change autoplay speed but I think that's weird and I'd rather have something more universal like a rate adjustment control period, regardless of whether autoplay is or is not active. (You could say this also lies within the purview of testing gameplay with selected mods, which is also one thing that people have been asking for.)

Contains some hacks to fix weird behaviours like rewinding to the start on enabling autoplay, or gameplay cursor hiding.

bdach · 2024-07-02T13:10:46Z

osu.Game/Input/Bindings/GlobalActionContainer.cs

@@ -442,6 +466,7 @@ public enum GlobalActionCategory
        Replay,
        SongSelect,
        AudioControl,
-        Overlays
+        Overlays,
+        EditorTestPlay,


These getting a separate section will likely look gratuitous but it bypasses several issues:

If they were to be implemented as non-rebindable, they wouldn't be listed anywhere, so there'd probably need to be text that explains that these exist

F1 and F2 are already used in the editor section (for compose/design tabs), and keybindings must be unique within a section

This also solves various input priority weirdness. As it stands, KeyBindingContainer takes precedence over plain OnKeyDown() which meant that other various bindings like leaderboard toggle were eating these hotkeys when implementing them via OnKeyDown().

bdach · 2024-07-02T13:11:18Z

osu.Game/Input/Bindings/GlobalActionContainer.cs

@@ -100,7 +104,6 @@ public static IEnumerable<GlobalAction> GetGlobalActionsFor(GlobalActionCategory
            new KeyBinding(new[] { InputKey.Control, InputKey.Shift, InputKey.F }, GlobalAction.ToggleFPSDisplay),
            new KeyBinding(new[] { InputKey.Control, InputKey.T }, GlobalAction.ToggleToolbar),
            new KeyBinding(new[] { InputKey.Control, InputKey.Shift, InputKey.S }, GlobalAction.ToggleSkinEditor),
-            new KeyBinding(new[] { InputKey.Control, InputKey.P }, GlobalAction.ToggleProfile),


This is moved/deprioritised to the overlay toggle section because in its previous place it was blocking the quick pause hotkey.

Checks out since it's an overlay toggle key binding in itself.

frenzibyte · 2024-07-03T03:31:02Z

osu!catch catcher no longer responds to input after toggling autoplay on then off:

CleanShot.2024-07-03.at.06.17.49.mp4
(this one is less severe) on any ruleset, holding a game key then turning on autoplay keeps the key pressed indefinitely, and the autoplay cannot hit any object with that key:

CleanShot.2024-07-03.at.06.22.53.mp4

frenzibyte

On a side note, it's pretty interesting to see that the ability of toggling autoplay in a Player screen works out of the box like that 😅 (unless there's an existing case of this that I did not know about). Requesting changes for the points mentioned above.

osu.Game/Screens/Edit/GameplayTest/EditorPlayer.cs

…uleset input manager

bdach · 2024-07-03T08:38:01Z

Issues should in theory be fixed although I wouldn't be surprised to see test failures.

… autoplay

bdach · 2024-07-03T12:38:35Z

osu.Game/Rulesets/UI/RulesetInputManager.cs

+                new MouseButtonInput([], state.Mouse.Buttons).Apply(state, handler);
+                new KeyboardKeyInput([], state.Keyboard.Keys).Apply(state, handler);
+                new TouchInput(Enum.GetValues<TouchSource>().Select(s => new Touch(s, Vector2.Zero)), false).Apply(state, handler);
+                new JoystickButtonInput([], state.Joystick.Buttons).Apply(state, handler);
+                new MidiKeyInput(new MidiState(), state.Midi).Apply(state, handler);
+                new TabletPenButtonInput([], state.Tablet.PenButtons).Apply(state, handler);
+                new TabletAuxiliaryButtonInput([], state.Tablet.AuxiliaryButtons).Apply(state, handler);


See https://discord.com/channels/188630481301012481/188630652340404224/1258027155208536115 for rationale for this atrocity.

I would like to think that this is a workaround for a bug caused by the way replay input handling / KeyBindingContainer.Trigger{Pressed,Released} works, and is fine to exist for now.

Replay input handler should have full control over the ruleset input manager's input state, but currently they only mutate the state of the KeyBindingContainer, and that's not good because the state of KeyBindingContainer will still be influenced by the input manager's input state, which the ReplayInputHandler does not touch.

When user switches from gameplay to autoplay while holding a key, KeyBindingContainer.TriggerReleased will get called on the action associated with the key, but nothing happens because KBC still sees the corresponding key in a pressed state and because simultaneous binding mode != all (see discord for more info).

Either the replay input handler should update the state of the input manager rather than the key binding container, or TriggerPressed/TriggerReleased should be disassociated with the state of the parenting input manager by some way. None of these methods seem that simple to work with, so I'm fine with performing a manual reset here for now.

frenzibyte · 2024-07-04T03:40:09Z

Requesting another review mainly for #28705 (comment).

peppy · 2024-07-04T04:26:13Z

In stable, these hotkeys are explained in a raw text string top-left of test play mode. I'd like to see something similar, but probably better implemented as a tool container in PlayerSettingsOverlay which also allows click/touch access to the various controls.

@bdach any thoughts on this part? I'm fine with getting this in as-is but also don't want the visibility of these keys to be forgotten.

bdach · 2024-07-04T04:36:50Z

Having these rebindable and show up in the keybinding panel was supposed to be my solution to the issue of discoverability of these, but I guess I don't see any reason not to try and add these back as actual on-screen buttons with a tooltip that shows the relevant key combination I guess.

peppy · 2024-07-04T04:44:19Z

Yeah I get that they are there, but that likely isn't going to be the first place people look to find hotkeys.

In stable we used to have a help overlay which revealed all the hotkeys that aren't necessarily visible otherwise, maybe that can be something we can explore in the future.

Going to get this in first, regardless.

bdach added 4 commits July 2, 2024 14:34

Implement autoplay toggle for editor test play

1e4db77

Contains some hacks to fix weird behaviours like rewinding to the start on enabling autoplay, or gameplay cursor hiding.

Implement quick pause toggle for editor test play

e28befb

Implement quick exit hotkeys for editor test play

d85c467

Add test coverage

3d61a21

bdach added the area:editor label Jul 2, 2024

bdach self-assigned this Jul 2, 2024

pull-request-size bot added the size/L label Jul 2, 2024

bdach commented Jul 2, 2024

View reviewed changes

frenzibyte self-requested a review July 3, 2024 03:12

frenzibyte requested changes Jul 3, 2024

View reviewed changes

osu.Game/Screens/Edit/GameplayTest/EditorPlayer.cs Show resolved Hide resolved

bdach added 4 commits July 3, 2024 09:33

Enable NRT in RulesetInputManager

29c3ff0

Add failing test coverage for replay detach

7f1d113

Clear pressed keys and last replay frame when detaching replay from r…

294aa09

…uleset input manager

Fix user-pressed keys remaining pressed whtn autoplay is turned on

84c7d34

bdach added 2 commits July 3, 2024 13:54

Merge branch 'master' into editor-test-play

e73faae

Use alternative method of releasing user-pressed keys when activating…

0ab13e4

… autoplay

bdach commented Jul 3, 2024

View reviewed changes

frenzibyte self-requested a review July 4, 2024 02:34

frenzibyte added 2 commits July 4, 2024 05:43

Remove unnecessary null-conditional operators

371ca4c

Update test to match new behaviour

e4f9071

frenzibyte approved these changes Jul 4, 2024

View reviewed changes

frenzibyte requested a review from a team July 4, 2024 03:39

peppy self-requested a review July 4, 2024 04:13

peppy approved these changes Jul 4, 2024

View reviewed changes

peppy merged commit 73d71d3 into ppy:master Jul 4, 2024
11 of 17 checks passed

bdach deleted the editor-test-play branch July 4, 2024 06:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement missing gameplay test hotkeys from stable #28705

Reimplement missing gameplay test hotkeys from stable #28705

bdach commented Jul 2, 2024

bdach Jul 2, 2024

bdach Jul 2, 2024

frenzibyte Jul 3, 2024

frenzibyte commented Jul 3, 2024

frenzibyte left a comment

bdach commented Jul 3, 2024

bdach Jul 3, 2024

frenzibyte Jul 4, 2024

frenzibyte commented Jul 4, 2024

peppy commented Jul 4, 2024

bdach commented Jul 4, 2024 •

edited

Loading

peppy commented Jul 4, 2024

Reimplement missing gameplay test hotkeys from stable #28705

Reimplement missing gameplay test hotkeys from stable #28705

Conversation

bdach commented Jul 2, 2024

bdach Jul 2, 2024

Choose a reason for hiding this comment

bdach Jul 2, 2024

Choose a reason for hiding this comment

frenzibyte Jul 3, 2024

Choose a reason for hiding this comment

frenzibyte commented Jul 3, 2024

frenzibyte left a comment

Choose a reason for hiding this comment

bdach commented Jul 3, 2024

bdach Jul 3, 2024

Choose a reason for hiding this comment

frenzibyte Jul 4, 2024

Choose a reason for hiding this comment

frenzibyte commented Jul 4, 2024

peppy commented Jul 4, 2024

bdach commented Jul 4, 2024 • edited Loading

peppy commented Jul 4, 2024

bdach commented Jul 4, 2024 •

edited

Loading