Merge pull request #11679 from smoogipoo/hit-policy-refactor

Refactor osu! ruleset hit policies for extensibility

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

@@ -25,7 +25,7 @@ using osuTK;
 
 namespace osu.Game.Rulesets.Osu.Tests
 {
-    public class TestSceneOutOfOrderHits : RateAdjustedBeatmapTestScene
+    public class TestSceneStartTimeOrderedHitPolicy : RateAdjustedBeatmapTestScene
     {
         private const double early_miss_window = 1000; // time after -1000 to -500 is considered a miss
         private const double late_miss_window = 500; // time after +500 is considered a miss
@@ -169,7 +169,7 @@ namespace osu.Game.Rulesets.Osu.Tests
             addJudgementAssert(hitObjects[0], HitResult.Great);
             addJudgementAssert(hitObjects[1], HitResult.Great);
             addJudgementOffsetAssert(hitObjects[0], -200); // time_first_circle - 200
-            addJudgementOffsetAssert(hitObjects[0], -200); // time_second_circle - first_circle_time - 100
+            addJudgementOffsetAssert(hitObjects[1], -200); // time_second_circle - first_circle_time - 100
         }
 
         /// <summary>

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

@@ -0,0 +1,31 @@
+// 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 osu.Game.Rulesets.Objects;
+using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.UI;
+
+namespace osu.Game.Rulesets.Osu.UI
+{
+    public interface IHitPolicy
+    {
+        /// <summary>
+        /// The <see cref="IHitObjectContainer"/> containing the <see cref="DrawableHitObject"/>s which this <see cref="IHitPolicy"/> applies to.
+        /// </summary>
+        IHitObjectContainer HitObjectContainer { set; }
+
+        /// <summary>
+        /// Determines whether a <see cref="DrawableHitObject"/> can be hit at a point in time.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="DrawableHitObject"/> to check.</param>
+        /// <param name="time">The time to check.</param>
+        /// <returns>Whether <paramref name="hitObject"/> can be hit at the given <paramref name="time"/>.</returns>
+        bool IsHittable(DrawableHitObject hitObject, double time);
+
+        /// <summary>
+        /// Handles a <see cref="HitObject"/> being hit.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="HitObject"/> that was hit.</param>
+        void HandleHit(DrawableHitObject hitObject);
+    }
+}

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

@@ -31,7 +31,7 @@ namespace osu.Game.Rulesets.Osu.UI
         private readonly ProxyContainer spinnerProxies;
         private readonly JudgementContainer<DrawableOsuJudgement> judgementLayer;
         private readonly FollowPointRenderer followPoints;
-        private readonly OrderedHitPolicy hitPolicy;
+        private readonly StartTimeOrderedHitPolicy hitPolicy;
 
         public static readonly Vector2 BASE_SIZE = new Vector2(512, 384);
 
@@ -54,7 +54,7 @@ namespace osu.Game.Rulesets.Osu.UI
                 approachCircles = new ProxyContainer { RelativeSizeAxes = Axes.Both },
             };
 
-            hitPolicy = new OrderedHitPolicy(HitObjectContainer);
+            hitPolicy = new StartTimeOrderedHitPolicy { HitObjectContainer = HitObjectContainer };
 
             var hitWindows = new OsuHitWindows();
 

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

@@ -11,28 +11,17 @@ using osu.Game.Rulesets.UI;
 namespace osu.Game.Rulesets.Osu.UI
 {
     /// <summary>
-    /// Ensures that <see cref="HitObject"/>s are hit in-order. Affectionately known as "note lock".
+    /// Ensures that <see cref="HitObject"/>s are hit in-order of their start times. 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>
     /// <item><description>The hit causes all previous <see cref="HitObject"/>s to missed otherwise.</description></item>
     /// </list>
     /// </summary>
-    public class OrderedHitPolicy
+    public class StartTimeOrderedHitPolicy : IHitPolicy
     {
-        private readonly HitObjectContainer hitObjectContainer;
+        public IHitObjectContainer HitObjectContainer { get; set; }
 
-        public OrderedHitPolicy(HitObjectContainer hitObjectContainer)
-        {
-            this.hitObjectContainer = hitObjectContainer;
-        }
-
-        /// <summary>
-        /// Determines whether a <see cref="DrawableHitObject"/> can be hit at a point in time.
-        /// </summary>
-        /// <param name="hitObject">The <see cref="DrawableHitObject"/> to check.</param>
-        /// <param name="time">The time to check.</param>
-        /// <returns>Whether <paramref name="hitObject"/> can be hit at the given <paramref name="time"/>.</returns>
         public bool IsHittable(DrawableHitObject hitObject, double time)
         {
             DrawableHitObject blockingObject = null;
@@ -54,10 +43,6 @@ namespace osu.Game.Rulesets.Osu.UI
             return blockingObject.Judged || time >= blockingObject.HitObject.StartTime;
         }
 
-        /// <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(DrawableHitObject hitObject)
         {
             // Hitobjects which themselves don't block future hitobjects don't cause misses (e.g. slider ticks, spinners).
@@ -67,6 +52,7 @@ namespace osu.Game.Rulesets.Osu.UI
             if (!IsHittable(hitObject, hitObject.HitObject.StartTime + hitObject.Result.TimeOffset))
                 throw new InvalidOperationException($"A {hitObject} was hit before it became hittable!");
 
+            // Miss all hitobjects prior to the hit one.
             foreach (var obj in enumerateHitObjectsUpTo(hitObject.HitObject.StartTime))
             {
                 if (obj.Judged)
@@ -86,7 +72,7 @@ namespace osu.Game.Rulesets.Osu.UI
 
         private IEnumerable<DrawableHitObject> enumerateHitObjectsUpTo(double targetTime)
         {
-            foreach (var obj in hitObjectContainer.AliveObjects)
+            foreach (var obj in HitObjectContainer.AliveObjects)
             {
                 if (obj.HitObject.StartTime >= targetTime)
                     yield break;

osu.Game/Rulesets/UI/HitObjectContainer.cs

@@ -17,19 +17,10 @@ using osu.Game.Rulesets.Objects.Drawables;
 
 namespace osu.Game.Rulesets.UI
 {
-    public class HitObjectContainer : LifetimeManagementContainer
+    public class HitObjectContainer : LifetimeManagementContainer, IHitObjectContainer
     {
-        /// <summary>
-        /// All currently in-use <see cref="DrawableHitObject"/>s.
-        /// </summary>
         public IEnumerable<DrawableHitObject> Objects => InternalChildren.Cast<DrawableHitObject>().OrderBy(h => h.HitObject.StartTime);
 
-        /// <summary>
-        /// All currently in-use <see cref="DrawableHitObject"/>s that are alive.
-        /// </summary>
-        /// <remarks>
-        /// If this <see cref="HitObjectContainer"/> uses pooled objects, this is equivalent to <see cref="Objects"/>.
-        /// </remarks>
         public IEnumerable<DrawableHitObject> AliveObjects => AliveInternalChildren.Cast<DrawableHitObject>().OrderBy(h => h.HitObject.StartTime);
 
         /// <summary>

osu.Game/Rulesets/UI/IHitObjectContainer.cs

@@ -0,0 +1,24 @@
+// 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.Collections.Generic;
+using osu.Game.Rulesets.Objects.Drawables;
+
+namespace osu.Game.Rulesets.UI
+{
+    public interface IHitObjectContainer
+    {
+        /// <summary>
+        /// All currently in-use <see cref="DrawableHitObject"/>s.
+        /// </summary>
+        IEnumerable<DrawableHitObject> Objects { get; }
+
+        /// <summary>
+        /// All currently in-use <see cref="DrawableHitObject"/>s that are alive.
+        /// </summary>
+        /// <remarks>
+        /// If this <see cref="IHitObjectContainer"/> uses pooled objects, this is equivalent to <see cref="Objects"/>.
+        /// </remarks>
+        IEnumerable<DrawableHitObject> AliveObjects { get; }
+    }
+}