Merge branch 'master' into fix-keybinding-defaults

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

@@ -296,6 +296,44 @@ namespace osu.Game.Rulesets.Osu.Tests
             addJudgementAssert(hitObjects[1], HitResult.Great);
         }
 
+        [Test]
+        public void TestHitSliderHeadBeforeHitCircle()
+        {
+            const double time_circle = 1000;
+            const double time_slider = 1200;
+            Vector2 positionCircle = Vector2.Zero;
+            Vector2 positionSlider = new Vector2(80);
+
+            var hitObjects = new List<OsuHitObject>
+            {
+                new TestHitCircle
+                {
+                    StartTime = time_circle,
+                    Position = positionCircle
+                },
+                new TestSlider
+                {
+                    StartTime = time_slider,
+                    Position = positionSlider,
+                    Path = new SliderPath(PathType.Linear, new[]
+                    {
+                        Vector2.Zero,
+                        new Vector2(25, 0),
+                    })
+                }
+            };
+
+            performTest(hitObjects, new List<ReplayFrame>
+            {
+                new OsuReplayFrame { Time = time_circle - 100, Position = positionSlider, Actions = { OsuAction.LeftButton } },
+                new OsuReplayFrame { Time = time_circle, Position = positionCircle, Actions = { OsuAction.RightButton } },
+                new OsuReplayFrame { Time = time_slider, Position = positionSlider, Actions = { OsuAction.LeftButton } },
+            });
+
+            addJudgementAssert(hitObjects[0], HitResult.Great);
+            addJudgementAssert(hitObjects[1], HitResult.Great);
+        }
+
         private void addJudgementAssert(OsuHitObject hitObject, HitResult result)
         {
             AddAssert($"({hitObject.GetType().ReadableName()} @ {hitObject.StartTime}) judgement is {result}",
@@ -371,6 +409,9 @@ namespace osu.Game.Rulesets.Osu.Tests
                 {
                     HeadCircle.HitWindows = new TestHitWindows();
                     TailCircle.HitWindows = new TestHitWindows();
+
+                    HeadCircle.HitWindows.SetDifficulty(0);
+                    TailCircle.HitWindows.SetDifficulty(0);
                 };
             }
         }

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

@@ -125,7 +125,11 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
                     return new DrawableSliderTail(slider, tail);
 
                 case SliderHeadCircle head:
-                    return new DrawableSliderHead(slider, head) { OnShake = Shake };
+                    return new DrawableSliderHead(slider, head)
+                    {
+                        OnShake = Shake,
+                        CheckHittable = (d, t) => CheckHittable?.Invoke(d, t) ?? true
+                    };
 
                 case SliderTick tick:
                     return new DrawableSliderTick(tick) { Position = tick.Position - slider.Position };

osu.Game.Rulesets.Osu/UI/OrderedHitPolicy.cs

@@ -1,16 +1,17 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
+using System.Collections.Generic;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Drawables;
-using osu.Game.Rulesets.Osu.Objects;
 using osu.Game.Rulesets.Osu.Objects.Drawables;
 using osu.Game.Rulesets.UI;
 
 namespace osu.Game.Rulesets.Osu.UI
 {
     /// <summary>
-    /// Ensures that <see cref="HitObject"/>s are hit in-order.
+    /// Ensures that <see cref="HitObject"/>s are hit in-order. Affectionately known as "note lock".
     /// If a <see cref="HitObject"/> is hit out of order:
     /// <list type="number">
     /// <item><description>The hit is blocked if it occurred earlier than the previous <see cref="HitObject"/>'s start time.</description></item>
@@ -36,13 +37,9 @@ namespace osu.Game.Rulesets.Osu.UI
         {
             DrawableHitObject blockingObject = null;
 
-            // Find the last hitobject which blocks future hits.
+            foreach (var obj in enumerateHitObjectsUpTo(hitObject.HitObject.StartTime))
-            foreach (var obj in hitObjectContainer.AliveObjects)
             {
-                if (obj == hitObject)
+                if (hitObjectCanBlockFutureHits(obj))
-                    break;
-
-                if (drawableCanBlockFutureHits(obj))
                     blockingObject = obj;
             }
 
@@ -54,74 +51,56 @@ namespace osu.Game.Rulesets.Osu.UI
             // 1. The last blocking hitobject has been judged.
             // 2. The current time is after the last hitobject's start time.
             // Hits at exactly the same time as the blocking hitobject are allowed for maps that contain simultaneous hitobjects (e.g. /b/372245).
-            if (blockingObject.Judged || time >= blockingObject.HitObject.StartTime)
+            return blockingObject.Judged || time >= blockingObject.HitObject.StartTime;
-                return true;
-
-            return false;
         }
 
         /// <summary>
         /// Handles a <see cref="HitObject"/> being hit to potentially miss all earlier <see cref="HitObject"/>s.
         /// </summary>
         /// <param name="hitObject">The <see cref="HitObject"/> that was hit.</param>
-        public void HandleHit(HitObject hitObject)
+        public void HandleHit(DrawableHitObject hitObject)
         {
             // Hitobjects which themselves don't block future hitobjects don't cause misses (e.g. slider ticks, spinners).
             if (!hitObjectCanBlockFutureHits(hitObject))
                 return;
 
-            double maximumTime = hitObject.StartTime;
+            if (!IsHittable(hitObject, hitObject.HitObject.StartTime + hitObject.Result.TimeOffset))
+                throw new InvalidOperationException($"A {hitObject} was hit before it become hittable!");
 
-            // Iterate through and apply miss results to all top-level and nested hitobjects which block future hits.
+            foreach (var obj in enumerateHitObjectsUpTo(hitObject.HitObject.StartTime))
-            foreach (var obj in hitObjectContainer.AliveObjects)
             {
-                if (obj.Judged || obj.HitObject.StartTime >= maximumTime)
+                if (obj.Judged)
                     continue;
 
-                if (hitObjectCanBlockFutureHits(obj.HitObject))
+                if (hitObjectCanBlockFutureHits(obj))
-                    applyMiss(obj);
+                    ((DrawableOsuHitObject)obj).MissForcefully();
-
-                foreach (var nested in obj.NestedHitObjects)
-                {
-                    if (nested.Judged || nested.HitObject.StartTime >= maximumTime)
-                        continue;
-
-                    if (hitObjectCanBlockFutureHits(nested.HitObject))
-                        applyMiss(nested);
-                }
             }
-
-            static void applyMiss(DrawableHitObject obj) => ((DrawableOsuHitObject)obj).MissForcefully();
-        }
-
-        /// <summary>
-        /// Whether a <see cref="DrawableHitObject"/> blocks hits on future <see cref="DrawableHitObject"/>s until its start time is reached.
-        /// </summary>
-        /// <remarks>
-        /// This will ONLY match on top-most <see cref="DrawableHitObject"/>s.
-        /// </remarks>
-        /// <param name="hitObject">The <see cref="DrawableHitObject"/> to test.</param>
-        private static bool drawableCanBlockFutureHits(DrawableHitObject hitObject)
-        {
-            // Special considerations for slider tails aren't required since only top-most drawable hitobjects are being iterated over.
-            return hitObject is DrawableHitCircle || hitObject is DrawableSlider;
         }
 
         /// <summary>
         /// Whether a <see cref="HitObject"/> blocks hits on future <see cref="HitObject"/>s until its start time is reached.
         /// </summary>
-        /// <remarks>
-        /// This is more rigorous and may not match on top-most <see cref="HitObject"/>s as <see cref="drawableCanBlockFutureHits"/> does.
-        /// </remarks>
         /// <param name="hitObject">The <see cref="HitObject"/> to test.</param>
-        private static bool hitObjectCanBlockFutureHits(HitObject hitObject)
+        private static bool hitObjectCanBlockFutureHits(DrawableHitObject hitObject)
-        {
+            => hitObject is DrawableHitCircle;
-            // Unlike the above we will receive slider tails, but they do not block future hits.
-            if (hitObject is SliderTailCircle)
-                return false;
 
-            // All other hitcircles continue to block future hits.
+        private IEnumerable<DrawableHitObject> enumerateHitObjectsUpTo(double targetTime)
-            return hitObject is HitCircle;
+        {
+            foreach (var obj in hitObjectContainer.AliveObjects)
+            {
+                if (obj.HitObject.StartTime >= targetTime)
+                    yield break;
+
+                yield return obj;
+
+                foreach (var nestedObj in obj.NestedHitObjects)
+                {
+                    if (nestedObj.HitObject.StartTime >= targetTime)
+                        break;
+
+                    yield return nestedObj;
+                }
+            }
         }
     }
 }

osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs

@@ -86,7 +86,7 @@ namespace osu.Game.Rulesets.Osu.UI
         private void onNewResult(DrawableHitObject judgedObject, JudgementResult result)
         {
             // Hitobjects that block future hits should miss previous hitobjects if they're hit out-of-order.
-            hitPolicy.HandleHit(result.HitObject);
+            hitPolicy.HandleHit(judgedObject);
 
             if (!judgedObject.DisplayResult || !DisplayJudgements.Value)
                 return;