ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.53k stars 2.31k forks source link

Gameplay cursor hides when playing "Autopilot" on touchscreen #16117

Open frenzibyte opened 2 years ago

frenzibyte commented 2 years ago

While touch input is blocked and mouse input is programmatically applied from OsuModAutopilot, the MenuCursorContainer remains unaware about that and keeps gameplay cursor hidden as if there's positional touch input consumed:

frenzibyte commented 2 years ago

Given that the MenuCursorContainer is responsible for showing/hiding the cursors and it's on a completely higher level from gameplay, the only way I can see to solve this issue is to make the "hide cursor on invalid input" logic toggleable per-IProvideCursor and disable it on DrawableOsuRuleset when user cursor movement is disabled (i.e. autopilot mod):

diff --git a/osu.Game.Rulesets.Osu/UI/DrawableOsuRuleset.cs b/osu.Game.Rulesets.Osu/UI/DrawableOsuRuleset.cs
index df3f7c64e4..0c4ad9f0a9 100644
--- a/osu.Game.Rulesets.Osu/UI/DrawableOsuRuleset.cs
+++ b/osu.Game.Rulesets.Osu/UI/DrawableOsuRuleset.cs
@@ -6,6 +6,7 @@
 using System.Linq;
 using osu.Framework.Input;
 using osu.Game.Beatmaps;
+using osu.Game.Graphics.Cursor;
 using osu.Game.Input.Handlers;
 using osu.Game.Replays;
 using osu.Game.Rulesets.Mods;
@@ -20,8 +21,10 @@

 namespace osu.Game.Rulesets.Osu.UI
 {
-    public class DrawableOsuRuleset : DrawableRuleset<OsuHitObject>
+    public class DrawableOsuRuleset : DrawableRuleset<OsuHitObject>, IProvideCursor
     {
+        public new OsuInputManager KeyBindingInputManager => (OsuInputManager)base.KeyBindingInputManager;
+
         protected new OsuRulesetConfigManager Config => (OsuRulesetConfigManager)base.Config;

         public new OsuPlayfield Playfield => (OsuPlayfield)base.Playfield;
@@ -57,5 +60,7 @@ public override double GameplayStartTime
                 return 0;
             }
         }
+
+        bool IProvideCursor.HideCursorOnInvalidInput => KeyBindingInputManager.AllowUserCursorMovement;
     }
 }
diff --git a/osu.Game/Graphics/Cursor/IProvideCursor.cs b/osu.Game/Graphics/Cursor/IProvideCursor.cs
index 3a920ba976..7230db62f7 100644
--- a/osu.Game/Graphics/Cursor/IProvideCursor.cs
+++ b/osu.Game/Graphics/Cursor/IProvideCursor.cs
@@ -22,5 +22,10 @@ public interface IProvideCursor : IDrawable
         /// This value is checked every frame and may be used to control whether multiple cursors are displayed (e.g
. watching replays).
         /// </summary>
         bool ProvidingUserCursor { get; }
+
+        /// <summary>
+        /// Whether <see cref="Cursor"/> should be hidden when input not valid for displaying a cursor is detected.
(e.g. touch input)
+        /// </summary>
+        bool HideCursorOnInvalidInput => true;
     }
 }
diff --git a/osu.Game/Graphics/Cursor/MenuCursorContainer.cs b/osu.Game/Graphics/Cursor/MenuCursorContainer.cs
index 4c7f7957e9..4d21694245 100644
--- a/osu.Game/Graphics/Cursor/MenuCursorContainer.cs
+++ b/osu.Game/Graphics/Cursor/MenuCursorContainer.cs
@@ -48,10 +48,7 @@ protected override void Update()
         {
             base.Update();

-            var lastMouseSource = inputManager.CurrentState.Mouse.LastSource;
-            bool hasValidInput = lastMouseSource != null && !(lastMouseSource is ISourcedFromTouch);
-
-            if (!hasValidInput || !CanShowCursor)
+            if (!CanShowCursor)
             {
                 currentTarget?.Cursor?.Hide();
                 currentTarget = null;
@@ -69,13 +66,19 @@ protected override void Update()
                 }
             }

-            if (currentTarget == newTarget)
-                return;
+            if (currentTarget != newTarget)
+            {
+                currentTarget?.Cursor?.Hide();
+                currentTarget = newTarget;
+            }

-            currentTarget?.Cursor?.Hide();
-            newTarget.Cursor?.Show();
+            var lastMouseSource = inputManager.CurrentState.Mouse.LastSource;
+            bool hasValidInput = lastMouseSource != null && !(lastMouseSource is ISourcedFromTouch);

-            currentTarget = newTarget;
+            if (hasValidInput || !currentTarget.HideCursorOnInvalidInput)
+                currentTarget?.Cursor?.Show();
+            else
+                currentTarget?.Cursor?.Hide();
         }
     }
 }

I'm holding from PR'ing this as I'm a bit unsure if this is a valid approach towards the issue, need feedback on this.

bdach commented 2 years ago

the notion of touch input being "invalid mouse input" is severely messing with my head in the diff above. additionally the MenuCursorContainer logic is really back and forth, and is different from the previous code in at least 1 place (the else branch in the final check should also reset currentTarget to null).

frenzibyte commented 2 years ago

the notion of touch input being "invalid mouse input" is severely messing with my head in the diff above.

That's the same name used to denote "input that should hide mouse" in the MenuCursorContainer logic, initially thought about being explicit and naming it HideCursorOnTouchInput instead.

As for the changes to the MenuCursorContainer logic, I agree with that, but I'm not really sure how to improve it further.