Fix judgement animation getting cut early

[smoogipoo]

Fixes #28848
Fixes #26009
Fixes #11980

I've previously reported this (#26009) but now that it's been reported by another user, I believe it's pretty important to fix, as it's very jarring.

The code should make it immediately obvious what the issue is - DrawableHitObjects are getting re-used, and when they're re-judged it kills the existing DrawableJudgement which could still be in the process of fading out.

It can be reproed on basically any Expert+ map.

[bdach]

#11980 also relevant I suppose

[smoogipoo]

Oh cool, that test is perfect. I hope you don't mind that I've nabbed it and made some adjustments.

[bdach]

So I'm making a heavy assumption that judgements, in general, only refer to the DHO to pull their properties in once ever.

I'm slightly uncomfortable on letting this by in its current form because of precisely this concern, so what is your thoughts on something like this?:

diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuJudgement.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuJudgement.cs
index 0630ecfbb5..8b3fcb23cd 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuJudgement.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableOsuJudgement.cs
@@ -5,19 +5,23 @@
 using osu.Framework.Graphics;
 using osu.Game.Configuration;
 using osu.Game.Rulesets.Judgements;
+using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Scoring;
 using osuTK;
+using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Osu.Objects.Drawables
 {
     public partial class DrawableOsuJudgement : DrawableJudgement
     {
+        internal Color4 AccentColour { get; private set; }
+
         internal SkinnableLighting Lighting { get; private set; } = null!;
 
         [Resolved]
         private OsuConfigManager config { get; set; } = null!;
 
-        private bool positionTransferred;
+        private Vector2 screenSpacePosition;
 
         [BackgroundDependencyLoader]
         private void load()
@@ -32,37 +36,36 @@ private void load()
             });
         }
 
-        protected override void PrepareForUse()
+        public override void Apply(JudgementResult result, DrawableHitObject? judgedObject)
         {
-            base.PrepareForUse();
+            base.Apply(result, judgedObject);
 
-            Lighting.ResetAnimation();
-            Lighting.SetColourFrom(JudgedObject, Result);
+            if (judgedObject is not DrawableOsuHitObject osuObject)
+                return;
 
-            positionTransferred = false;
-        }
+            AccentColour = osuObject.AccentColour.Value;
 
-        protected override void Update()
-        {
-            base.Update();
-
-            if (!positionTransferred && JudgedObject is DrawableOsuHitObject osuObject && JudgedObject.IsInUse)
+            switch (osuObject)
             {
-                switch (osuObject)
-                {
-                    case DrawableSlider slider:
-                        Position = slider.TailCircle.ToSpaceOfOtherDrawable(slider.TailCircle.OriginPosition, Parent!);
-                        break;
+                case DrawableSlider slider:
+                    screenSpacePosition = slider.TailCircle.ToScreenSpace(slider.TailCircle.OriginPosition);
+                    break;
 
-                    default:
-                        Position = osuObject.ToSpaceOfOtherDrawable(osuObject.OriginPosition, Parent!);
-                        break;
-                }
+                default:
+                    screenSpacePosition = osuObject.ToScreenSpace(osuObject.OriginPosition);
+                    break;
+            }
 
-                positionTransferred = true;
+            Scale = new Vector2(osuObject.HitObject.Scale);
+        }
 
-                Scale = new Vector2(osuObject.HitObject.Scale);
-            }
+        protected override void PrepareForUse()
+        {
+            base.PrepareForUse();
+
+            Lighting.ResetAnimation();
+            Lighting.SetColourFrom(this, Result);
+            Position = Parent!.ToLocalSpace(screenSpacePosition);
         }
 
         protected override void ApplyHitAnimations()
diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SkinnableLighting.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SkinnableLighting.cs
index b39b9c4c54..aa6be2cd85 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/SkinnableLighting.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SkinnableLighting.cs
@@ -1,8 +1,6 @@
 // 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.
 
-#nullable disable
-
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Skinning;
@@ -12,8 +10,8 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
 {
     internal partial class SkinnableLighting : SkinnableSprite
     {
-        private DrawableHitObject targetObject;
-        private JudgementResult targetResult;
+        private DrawableOsuJudgement? targetObject;
+        private JudgementResult? targetResult;
 
         public SkinnableLighting()
             : base("lighting")
@@ -29,11 +27,11 @@ protected override void SkinChanged(ISkinSource skin)
         /// <summary>
         /// Updates the lighting colour from a given hitobject and result.
         /// </summary>
-        /// <param name="targetObject">The <see cref="DrawableHitObject"/> that's been judged.</param>
-        /// <param name="targetResult">The <see cref="JudgementResult"/> that <paramref name="targetObject"/> was judged with.</param>
-        public void SetColourFrom(DrawableHitObject targetObject, JudgementResult targetResult)
+        /// <param name="targetJudgement">The <see cref="DrawableHitObject"/> that's been judged.</param>
+        /// <param name="targetResult">The <see cref="JudgementResult"/> that <paramref name="targetJudgement"/> was judged with.</param>
+        public void SetColourFrom(DrawableOsuJudgement targetJudgement, JudgementResult? targetResult)
         {
-            this.targetObject = targetObject;
+            this.targetObject = targetJudgement;
             this.targetResult = targetResult;
 
             updateColour();
@@ -44,7 +42,7 @@ private void updateColour()
             if (targetObject == null || targetResult == null)
                 Colour = Color4.White;
             else
-                Colour = targetResult.IsHit ? targetObject.AccentColour.Value : Color4.Transparent;
+                Colour = targetResult.IsHit ? targetObject.AccentColour : Color4.Transparent;
         }
     }
 }
diff --git a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs
index 189be44033..bdeadfd201 100644
--- a/osu.Game/Rulesets/Judgements/DrawableJudgement.cs
+++ b/osu.Game/Rulesets/Judgements/DrawableJudgement.cs
@@ -24,8 +24,6 @@ public partial class DrawableJudgement : PoolableDrawable
 
         public JudgementResult? Result { get; private set; }
 
-        public DrawableHitObject? JudgedObject { get; private set; }
-
         public HitObject? JudgedHitObject { get; private set; }
 
         public override bool RemoveCompletedTransforms => false;
@@ -97,10 +95,9 @@ protected virtual void ApplyMissAnimations()
         /// </summary>
         /// <param name="result">The applicable judgement.</param>
         /// <param name="judgedObject">The drawable object.</param>
-        public void Apply(JudgementResult result, DrawableHitObject? judgedObject)
+        public virtual void Apply(JudgementResult result, DrawableHitObject? judgedObject)
         {
             Result = result;
-            JudgedObject = judgedObject;
             JudgedHitObject = judgedObject?.HitObject;
         }
 
@@ -108,7 +105,6 @@ protected override void FreeAfterUse()
         {
             base.FreeAfterUse();
 
-            JudgedObject = null;
             JudgedHitObject = null;
         }
 

Only lightly tested, but appears to work, and kills that concern in the crib if you will.

[smoogipoo]

Looks good to me.