From 555ce7684b9d57183a603f7469dc23bd36354d83 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Tue, 20 Jun 2023 20:04:02 +0900
Subject: [PATCH] Adjust `GameplaySampleTriggerSource` to only switch samples
 when close enough to the next hit object

Closes #23963.

To simplify things, I've removed the optimisation of using
`AliveObject`s because it would break the way this whole lookup works.
---
 .../UI/GameplaySampleTriggerSource.cs         | 56 +++++++------------
 1 file changed, 19 insertions(+), 37 deletions(-)

diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs
index fbb7a20a5d..909b633a72 100644
--- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs
+++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs
@@ -7,7 +7,7 @@ using System.Linq;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Audio;
 using osu.Game.Rulesets.Objects;
-using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.Scoring;
 using osu.Game.Skinning;
 
 namespace osu.Game.Rulesets.UI
@@ -39,7 +39,7 @@ namespace osu.Game.Rulesets.UI
             };
         }
 
-        private HitObjectLifetimeEntry fallbackObject;
+        private HitObjectLifetimeEntry mostValidObject;
 
         /// <summary>
         /// Play the most appropriate hit sound for the current point in time.
@@ -67,56 +67,38 @@ namespace osu.Game.Rulesets.UI
 
         protected HitObject GetMostValidObject()
         {
-            // The most optimal lookup case we have is when an object is alive. There are usually very few alive objects so there's no drawbacks in attempting this lookup each time.
-            var drawableHitObject = hitObjectContainer.AliveObjects.FirstOrDefault(h => h.Result?.HasResult != true);
-
-            if (drawableHitObject != null)
-            {
-                // A hit object may have a more valid nested object.
-                drawableHitObject = getMostValidNestedDrawable(drawableHitObject);
-
-                return drawableHitObject.HitObject;
-            }
-
-            // In the case a next object isn't available in drawable form, we need to do a somewhat expensive traversal to get a valid sound to play.
-            // This lookup can be skipped if the last entry is still valid (in the future and not yet hit).
-            if (fallbackObject == null || fallbackObject.Result?.HasResult == true)
+            if (mostValidObject == null || isAlreadyHit(mostValidObject))
             {
                 // We need to use lifetime entries to find the next object (we can't just use `hitObjectContainer.Objects` due to pooling - it may even be empty).
                 // If required, we can make this lookup more efficient by adding support to get next-future-entry in LifetimeEntryManager.
-                fallbackObject = hitObjectContainer.Entries
-                                                   .Where(e => e.Result?.HasResult != true).MinBy(e => e.HitObject.StartTime);
-
-                if (fallbackObject != null)
-                    return getEarliestNestedObject(fallbackObject.HitObject);
+                var candidate = hitObjectContainer.Entries.Where(e => !isAlreadyHit(e)).MinBy(e => e.HitObject.StartTime);
 
                 // In the case there are no non-judged objects, the last hit object should be used instead.
-                fallbackObject ??= hitObjectContainer.Entries.LastOrDefault();
+                if (candidate == null)
+                    mostValidObject = hitObjectContainer.Entries.LastOrDefault();
+                else
+                {
+                    if (isCloseEnoughToCurrentTime(candidate))
+                        mostValidObject = candidate;
+                    else
+                        mostValidObject ??= hitObjectContainer.Entries.FirstOrDefault();
+                }
             }
 
-            if (fallbackObject == null)
+            if (mostValidObject == null)
                 return null;
 
-            bool fallbackHasResult = fallbackObject.Result?.HasResult == true;
-
             // If the fallback has been judged then we want the sample from the object itself.
-            if (fallbackHasResult)
-                return fallbackObject.HitObject;
+            if (isAlreadyHit(mostValidObject))
+                return mostValidObject.HitObject;
 
             // Else we want the earliest (including nested).
             // In cases of nested objects, they will always have earlier sample data than their parent object.
-            return getEarliestNestedObject(fallbackObject.HitObject);
+            return getEarliestNestedObject(mostValidObject.HitObject);
         }
 
-        private DrawableHitObject getMostValidNestedDrawable(DrawableHitObject o)
-        {
-            var nestedWithoutResult = o.NestedHitObjects.FirstOrDefault(n => n.Result?.HasResult != true);
-
-            if (nestedWithoutResult == null)
-                return o;
-
-            return getMostValidNestedDrawable(nestedWithoutResult);
-        }
+        private bool isAlreadyHit(HitObjectLifetimeEntry h) => h.Result?.HasResult == true;
+        private bool isCloseEnoughToCurrentTime(HitObjectLifetimeEntry h) => Time.Current > h.HitObject.StartTime - h.HitObject.HitWindows.WindowFor(HitResult.Miss) * 1.5;
 
         private HitObject getEarliestNestedObject(HitObject hitObject)
         {