Resuming gameplay by clicking orange cursor can cause a hit on the underlying circle

[kathelynn]

Describe the bug: 'Click the orange cursor' screen has weird behavior where it would register a hitcircle press. The hitcircle doesn't disappear when a hit is registered. Might be potentially registering two presses, but I haven't tested this very well yet. Works both on slider's hitcircle and normal hitcircles. Screenshots or videos showing encountered issue: https://drive.google.com/file/d/15YjV__Tz4P8694FNAE6I3crFbsq4gpCp/view?usp=sharing osu!lazer version: 2020.723.0

[peppy]

This is an interesting problem because we actually do want to register mouse down (in the case where the mouse was being held before pause was triggered).

[sawawas]

sorry... i thought after clicking the orange cursor it should start letting you click in game which is how you are able to resume sliders (which is how it should work if it's possible imo... maybe a toggle or something?)

apart from that maybe it can just not make you miss and it'll feel the same idk it's really annoying

[frenzibyte]

This incorrect behaviour stems from two distinct problems here:

1. The OsuResumeOverlay has no control over blocking the action from reaching the gameplay components, despite returning true on the event. This is because the overlay is wrapped in an input manager that is separate from the gameplay components, and when it handles the press event it receives, it only blocks it from the hierarchy that's under its local input manager (this one).
   • We can resolve this by scheduling the resume operation one frame forward so that the press event flows through the entire game scene and finishes before the input manager of the gameplay components is enabled and sees it.
2. When gameplay is resumed, a Sync() operation is performed on the input manager of the gameplay components. Sync() reads the state of the parent input manager and notices the click-to-resume action pressed, so it incorrectly applies the press to itself, causing the hit objects to receive the unwanted press event.
   • This behaviour was added since https://github.com/ppy/osu-framework/pull/1682.
   • This affects all input methods except for mouse. The method checks whether the parent has any mouse buttons that are released in the parent state, it does not check for presses like it does for other input methods.
   • To resolve this, I would propose changing behaviour of Sync() for all input methods to match mouse buttons (only apply a release on Sync(), never apply a press). And for the issue that this behaviour was introduced for, that should be handled on a more specific level (i.e. only sync presses on LoadComplete).

cc @ppy/team-client for opinions. I believe there is another way to resolve issue 1 but I was encountering issues with it that I couldn't understand in reasonable time, if scheduling the resume operation for just one frame is seen completely bad, then I can continue exploring through the other path (it involves framework changes to PassThroughInputManager as well, to say the least).

[peppy]

at face value both of the proposals make sense to me.

[bdach]

We can resolve this by scheduling the resume operation one frame forward so that the press event flows through the entire game scene and finishes before the input manager of the gameplay components is enabled and sees it.

I have bad feelings about this one. Hopefully they're unfounded, but relying on schedules for that sorta thing feels like asking for trouble later.

To resolve this, I would propose changing behaviour of Sync() for all input methods to match mouse buttons (only apply a release on Sync(), never apply a press). And for the issue that this behaviour was introduced for, that should be handled on a more specific level (i.e. only sync presses on LoadComplete).

I'm not sure I understand the implications of this one well enough just from that description. Maybe fine. But risky change.

Maybe you could make this settable on the concrete input manager instance somehow, via a flag or something.

[frenzibyte]

We can resolve this by scheduling the resume operation one frame forward so that the press event flows through the entire game scene and finishes before the input manager of the gameplay components is enabled and sees it.

I have bad feelings about this one. Hopefully they're unfounded, but relying on schedules for that sorta thing feels like asking for trouble later.

I will continue exploring through the other path I mentioned at the bottom of https://github.com/ppy/osu/issues/9658#issuecomment-2005767323 and report back if it spirals out of control, as a justification for resorting to a schedule.

To resolve this, I would propose changing behaviour of Sync() for all input methods to match mouse buttons (only apply a release on Sync(), never apply a press). And for the issue that this behaviour was introduced for, that should be handled on a more specific level (i.e. only sync presses on LoadComplete).

I'm not sure I understand the implications of this one well enough just from that description. Maybe fine. But risky change.

Maybe you could make this settable on the concrete input manager instance somehow, via a flag or something.

Hmm, I cannot say I like having that, especially since gameplay is almost the only real usage of PassThroughInputManager, but that's fine if it lowers down concern.