ppy / osu

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

Double sized yellow cursor #26477

Closed wawdili closed 9 months ago

wawdili commented 9 months ago

Type

Game behaviour

Bug description

At the moment of canceling the pause, the size of the yellow cursor is larger than usual, about twice the size, and clicking on this double sized yellow cursor can still continue the game.

Screenshots or videos

image

https://github.com/ppy/osu/assets/87159802/e1e59e71-e41a-46bd-809c-5d0b99b158b4

Version

2023.1231.0

Logs

log

bdach commented 9 months ago

I don't understand what this is trying to say.

It appears that you were trying to upload a video of this but failed. Please try again.

bdach commented 9 months ago

it appears very large, I believe this started happening near the time depth mod was added

I would like a clear and reproducible description of the issue from someone please.

Joehuu commented 9 months ago

When the resume cursor appears, it is initially scaled: https://github.com/ppy/osu/blob/da29faffd0bcf80ae361a22683c5b705b5b45037/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs#L114-L118

The resume cursor also handles input, so in that 1 second, it has an unintended larger area to click on: Kapture 2024-01-11 at 11 57 17

peppy commented 9 months ago

Probably fine?

bdach commented 9 months ago

If it's what @Joehuu describes then yes I'd say it's a non-issue. But I'd actually like to hear that from @wawdili rather than other people passing by.

wawdili commented 9 months ago

If it's what @Joehuu describes then yes I'd say it's a non-issue. But I'd actually like to hear that from @wawdili rather than other people passing by.

I think that by clicking like in the video, I can get a larger position offset after pausing, which is equivalent to the cursor teleporting to a farther position. However, if someone does this like me and logs in to play, will it affect the game balance?

For example, doing this in places where always almost hit. This is reflected on some beatmaps (such as having only one note placed far away).

Kanzen Shouri*Esper Girl [AF's Insane] 未标题-1 It looks simpler

bdach commented 9 months ago

Okay so what you're saying that this is a gameplay fairness issue.

I'm not sure this is that important but I guess we can remove the scaling transition? I'd rather do that than have a "fake" hitbox that doesn't change scale.

wawdili commented 9 months ago

Okay so what you're saying that this is a gameplay fairness issue.

I'm not sure this is that important but I guess we can remove the scaling transition? I'd rather do that than have a "fake" hitbox that doesn't change scale.

I think that's okay. And I think we can make the yellow cursor flicker to remind players of its position. (Like Kiai time?)

frenzibyte commented 9 months ago

Okay so what you're saying that this is a gameplay fairness issue.

I'm not sure this is that important but I guess we can remove the scaling transition? I'd rather do that than have a "fake" hitbox that doesn't change scale.

Making the input handling area not change on scale looks to be easy, especially since OsuCursor already does something like this with its scale transition on press/release. Diff:

diff --git a/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs b/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs
index 18351c20ce..eaf2c3493c 100644
--- a/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs
+++ b/osu.Game.Rulesets.Osu/UI/Cursor/OsuCursor.cs
@@ -32,7 +32,7 @@ public partial class OsuCursor : SkinReloadableDrawable
         private SkinnableDrawable cursorSprite;
         private Container cursorScaleContainer = null!;

-        private Drawable expandTarget => (cursorSprite.Drawable as OsuCursorSprite)?.ExpandTarget ?? cursorSprite;
+        protected Drawable ExpandTarget => (cursorSprite.Drawable as OsuCursorSprite)?.ExpandTarget ?? cursorSprite;

         public IBindable<float> CursorScale => cursorScale;

@@ -106,10 +106,10 @@ public void Expand()
         {
             if (!cursorExpand) return;

-            expandTarget.ScaleTo(released_scale).ScaleTo(pressed_scale, 400, Easing.OutElasticHalf);
+            ExpandTarget.ScaleTo(released_scale).ScaleTo(pressed_scale, 400, Easing.OutElasticHalf);
         }

-        public void Contract() => expandTarget.ScaleTo(released_scale, 400, Easing.OutQuad);
+        public void Contract() => ExpandTarget.ScaleTo(released_scale, 400, Easing.OutQuad);

         /// <summary>
         /// Get the scale applicable to the ActiveCursor based on a beatmap's circle size.
diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs
index f5e83f46f2..e75812d6e0 100644
--- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs
+++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs
@@ -114,7 +114,7 @@ public void OnReleased(KeyBindingReleaseEvent<OsuAction> e)
             public void Appear() => Schedule(() =>
             {
                 updateColour();
-                this.ScaleTo(4).Then().ScaleTo(1, 1000, Easing.OutQuint);
+                ExpandTarget.ScaleTo(4).Then().ScaleTo(1, 1000, Easing.OutQuint);
             });

             private void updateColour()

master:

https://github.com/ppy/osu/assets/22781491/60911126-e6c9-4094-9d76-c7465372b6d3

diff:

https://github.com/ppy/osu/assets/22781491/087bb2dc-3e46-4b2c-870f-672ac6149c5d

If it sounds good, I can open a PR for that (plus/minus some details to make code slightly more digestible).

peppy commented 9 months ago

Seems fine to apply that change. The animation does help for visibility of where the cursor is.