From affa9507a1549e0d7393cd244ebc5b1b5195c1b7 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Thu, 16 Feb 2023 17:34:38 +0900
Subject: [PATCH] Fix `GameplaySampleTriggerSource` not considering nested
 objects when determining the best sample to play

---
 .../UI/GameplaySampleTriggerSource.cs         | 67 ++++++++++++++-----
 1 file changed, 51 insertions(+), 16 deletions(-)

diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs
index d2244df3b8..de16cc05c7 100644
--- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs
+++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs
@@ -7,6 +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.Skinning;
 
 namespace osu.Game.Rulesets.UI
@@ -68,27 +69,61 @@ 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 hitObject = hitObjectContainer.AliveObjects.FirstOrDefault(h => h.Result?.HasResult != true)?.HitObject;
+            var drawableHitObject = hitObjectContainer.AliveObjects.FirstOrDefault(h => h.Result?.HasResult != true);
 
-            // 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.
-            if (hitObject == null)
+            if (drawableHitObject != null)
             {
-                // 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)
-                {
-                    // 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);
+                // A hit object may have a more valid nested object.
+                drawableHitObject = getMostValidNestedDrawable(drawableHitObject);
 
-                    // In the case there are no unjudged objects, the last hit object should be used instead.
-                    fallbackObject ??= hitObjectContainer.Entries.LastOrDefault();
-                }
-
-                hitObject = fallbackObject?.HitObject;
+                return drawableHitObject.HitObject;
             }
 
-            return 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)
+            {
+                // 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 fallbackObject.HitObject;
+
+                // In the case there are no unjudged objects, the last hit object should be used instead.
+                fallbackObject ??= hitObjectContainer.Entries.LastOrDefault();
+            }
+
+            if (fallbackObject == 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;
+
+            // 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);
+        }
+
+        private DrawableHitObject getMostValidNestedDrawable(DrawableHitObject o)
+        {
+            var nestedWithoutResult = o.NestedHitObjects.FirstOrDefault(n => n.Result?.HasResult != true);
+
+            if (nestedWithoutResult == null)
+                return o;
+
+            return getMostValidNestedDrawable(nestedWithoutResult);
+        }
+
+        private HitObject getEarliestNestedObject(HitObject hitObject)
+        {
+            var nested = hitObject.NestedHitObjects.FirstOrDefault();
+
+            return nested != null ? getEarliestNestedObject(nested) : hitObject;
         }
 
         private SkinnableSound getNextSample()