Merge pull request #24720 from bdach/overlapping-hit-circle-input

Block input to objects lying under already-hit hitcircles when classic note lock is active

osu.Game.Rulesets.Osu.Tests/TestSceneHitCircleLateFade.cs

@@ -142,6 +142,7 @@ private TestDrawableHitCircle createCircle(bool shouldHit = false)
 
             drawableHitCircle.Scale = new Vector2(2f);
 
+            LoadComponent(drawableHitCircle);
             foreach (var mod in SelectedMods.Value.OfType<IApplicableToDrawableHitObject>())
                 mod.ApplyToDrawableHitObject(drawableHitCircle);
 

osu.Game.Rulesets.Osu.Tests/TestSceneLegacyHitPolicy.cs

@@ -505,6 +505,56 @@ public void TestAutopilotReducesHittableRange()
             addClickActionAssert(0, ClickAction.Shake);
         }
 
+        [Test]
+        public void TestInputDoesNotFallThroughOverlappingSliders()
+        {
+            const double time_first_slider = 1000;
+            const double time_second_slider = 1250;
+            Vector2 positionFirstSlider = new Vector2(100, 50);
+            Vector2 positionSecondSlider = new Vector2(100, 80);
+            var midpoint = (positionFirstSlider + positionSecondSlider) / 2;
+
+            var hitObjects = new List<OsuHitObject>
+            {
+                new Slider
+                {
+                    StartTime = time_first_slider,
+                    Position = positionFirstSlider,
+                    Path = new SliderPath(PathType.Linear, new[]
+                    {
+                        Vector2.Zero,
+                        new Vector2(25, 0),
+                    })
+                },
+                new Slider
+                {
+                    StartTime = time_second_slider,
+                    Position = positionSecondSlider,
+                    Path = new SliderPath(PathType.Linear, new[]
+                    {
+                        Vector2.Zero,
+                        new Vector2(25, 0),
+                    })
+                }
+            };
+
+            performTest(hitObjects, new List<ReplayFrame>
+            {
+                new OsuReplayFrame { Time = time_first_slider, Position = midpoint, Actions = { OsuAction.RightButton } },
+                new OsuReplayFrame { Time = time_first_slider + 25, Position = midpoint, Actions = { OsuAction.LeftButton } },
+                new OsuReplayFrame { Time = time_first_slider + 50, Position = midpoint },
+            });
+
+            addJudgementAssert(hitObjects[0], HitResult.Ok);
+            addJudgementOffsetAssert("first slider head", () => ((Slider)hitObjects[0]).HeadCircle, 0);
+            addJudgementAssert(hitObjects[1], HitResult.Miss);
+            // the slider head of the first slider prevents the second slider's head from being hit, so the judgement offset should be very late.
+            // this is not strictly done by the hit policy implementation itself (see `OsuModClassic.blockInputToUnderlyingObjects()`),
+            // but we're testing this here anyways to just keep everything related to input handling and note lock in one place.
+            addJudgementOffsetAssert("second slider head", () => ((Slider)hitObjects[1]).HeadCircle, referenceHitWindows.WindowFor(HitResult.Meh));
+            addClickActionAssert(0, ClickAction.Hit);
+        }
+
         private void addJudgementAssert(OsuHitObject hitObject, HitResult result)
         {
             AddAssert($"({hitObject.GetType().ReadableName()} @ {hitObject.StartTime}) judgement is {result}",
@@ -523,6 +573,12 @@ private void addJudgementOffsetAssert(OsuHitObject hitObject, double offset)
                 () => judgementResults.Single(r => r.HitObject == hitObject).TimeOffset, () => Is.EqualTo(offset).Within(50));
         }
 
+        private void addJudgementOffsetAssert(string name, Func<OsuHitObject?> hitObject, double offset)
+        {
+            AddAssert($"{name} @ judged at {offset}",
+                () => judgementResults.Single(r => r.HitObject == hitObject()).TimeOffset, () => Is.EqualTo(offset).Within(50));
+        }
+
         private void addClickActionAssert(int inputIndex, ClickAction action)
             => AddAssert($"input #{inputIndex} resulted in {action}", () => testPolicy.ClickActions[inputIndex], () => Is.EqualTo(action));
 

osu.Game.Rulesets.Osu.Tests/TestSceneStartTimeOrderedHitPolicy.cs

@@ -336,6 +336,52 @@ public void TestHitSliderHeadBeforeHitCircle()
             addJudgementAssert(hitObjects[1], HitResult.IgnoreHit);
         }
 
+        [Test]
+        public void TestInputFallsThroughJudgedSliders()
+        {
+            const double time_first_slider = 1000;
+            const double time_second_slider = 1250;
+            Vector2 positionFirstSlider = new Vector2(100, 50);
+            Vector2 positionSecondSlider = new Vector2(100, 80);
+            var midpoint = (positionFirstSlider + positionSecondSlider) / 2;
+
+            var hitObjects = new List<OsuHitObject>
+            {
+                new TestSlider
+                {
+                    StartTime = time_first_slider,
+                    Position = positionFirstSlider,
+                    Path = new SliderPath(PathType.Linear, new[]
+                    {
+                        Vector2.Zero,
+                        new Vector2(25, 0),
+                    })
+                },
+                new TestSlider
+                {
+                    StartTime = time_second_slider,
+                    Position = positionSecondSlider,
+                    Path = new SliderPath(PathType.Linear, new[]
+                    {
+                        Vector2.Zero,
+                        new Vector2(25, 0),
+                    })
+                }
+            };
+
+            performTest(hitObjects, new List<ReplayFrame>
+            {
+                new OsuReplayFrame { Time = time_first_slider, Position = midpoint, Actions = { OsuAction.RightButton } },
+                new OsuReplayFrame { Time = time_first_slider + 25, Position = midpoint, Actions = { OsuAction.LeftButton } },
+                new OsuReplayFrame { Time = time_first_slider + 50, Position = midpoint },
+            });
+
+            addJudgementAssert("first slider head", () => ((Slider)hitObjects[0]).HeadCircle, HitResult.Great);
+            addJudgementOffsetAssert("first slider head", () => ((Slider)hitObjects[0]).HeadCircle, 0);
+            addJudgementAssert("second slider head", () => ((Slider)hitObjects[1]).HeadCircle, HitResult.Great);
+            addJudgementOffsetAssert("second slider head", () => ((Slider)hitObjects[1]).HeadCircle, -200);
+        }
+
         private void addJudgementAssert(OsuHitObject hitObject, HitResult result)
         {
             AddAssert($"({hitObject.GetType().ReadableName()} @ {hitObject.StartTime}) judgement is {result}",
@@ -354,6 +400,12 @@ private void addJudgementOffsetAssert(OsuHitObject hitObject, double offset)
                 () => Precision.AlmostEquals(judgementResults.Single(r => r.HitObject == hitObject).TimeOffset, offset, 100));
         }
 
+        private void addJudgementOffsetAssert(string name, Func<OsuHitObject> hitObject, double offset)
+        {
+            AddAssert($"{name} @ judged at {offset}",
+                () => judgementResults.Single(r => r.HitObject == hitObject()).TimeOffset, () => Is.EqualTo(offset).Within(50));
+        }
+
         private ScoreAccessibleReplayPlayer currentPlayer;
         private List<JudgementResult> judgementResults;
 

osu.Game.Rulesets.Osu/Mods/OsuModClassic.cs

@@ -74,6 +74,10 @@ public void ApplyToDrawableHitObject(DrawableHitObject obj)
                     head.TrackFollowCircle = !NoSliderHeadMovement.Value;
                     if (FadeHitCircleEarly.Value && !usingHiddenFading)
                         applyEarlyFading(head);
+
+                    if (ClassicNoteLock.Value)
+                        blockInputToUnderlyingObjects(head);
+
                     break;
 
                 case DrawableSliderTail tail:
@@ -83,10 +87,29 @@ public void ApplyToDrawableHitObject(DrawableHitObject obj)
                 case DrawableHitCircle circle:
                     if (FadeHitCircleEarly.Value && !usingHiddenFading)
                         applyEarlyFading(circle);
+
+                    if (ClassicNoteLock.Value)
+                        blockInputToUnderlyingObjects(circle);
+
                     break;
             }
         }
 
+        /// <summary>
+        /// On stable, hitcircles that have already been hit block input from reaching objects that may be underneath them.
+        /// The purpose of this method is to restore that behaviour.
+        /// In order to avoid introducing yet another confusing config option, this behaviour is roped into the general notion of "note lock".
+        /// </summary>
+        private static void blockInputToUnderlyingObjects(DrawableHitCircle circle)
+        {
+            var oldHitAction = circle.HitArea.Hit;
+            circle.HitArea.Hit = () =>
+            {
+                oldHitAction?.Invoke();
+                return true;
+            };
+        }
+
         private void applyEarlyFading(DrawableHitCircle circle)
         {
             circle.ApplyCustomUpdateState += (dho, state) =>

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs

@@ -261,7 +261,7 @@ public bool OnPressed(KeyBindingPressEvent<OsuAction> e)
                     case OsuAction.RightButton:
                         if (IsHovered && (Hit?.Invoke() ?? false))
                         {
-                            HitAction = e.Action;
+                            HitAction ??= e.Action;
                             return true;
                         }