Merge pull request #28881 from smoogipoo/fix-judgement-cut

Fix judgement animation getting cut early

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

@@ -5,19 +5,23 @@ using osu.Framework.Allocation;
 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 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             });
         }
 
+        public override void Apply(JudgementResult result, DrawableHitObject? judgedObject)
+        {
+            base.Apply(result, judgedObject);
+
+            if (judgedObject is not DrawableOsuHitObject osuObject)
+                return;
+
+            AccentColour = osuObject.AccentColour.Value;
+
+            switch (osuObject)
+            {
+                case DrawableSlider slider:
+                    screenSpacePosition = slider.TailCircle.ToScreenSpace(slider.TailCircle.OriginPosition);
+                    break;
+
+                default:
+                    screenSpacePosition = osuObject.ToScreenSpace(osuObject.OriginPosition);
+                    break;
+            }
+
+            Scale = new Vector2(osuObject.HitObject.Scale);
+        }
+
         protected override void PrepareForUse()
         {
             base.PrepareForUse();
 
             Lighting.ResetAnimation();
-            Lighting.SetColourFrom(JudgedObject, Result);
+            Lighting.SetColourFrom(this, Result);
-
+            Position = Parent!.ToLocalSpace(screenSpacePosition);
-            positionTransferred = false;
-        }
-
-        protected override void Update()
-        {
-            base.Update();
-
-            if (!positionTransferred && JudgedObject is DrawableOsuHitObject osuObject && JudgedObject.IsInUse)
-            {
-                switch (osuObject)
-                {
-                    case DrawableSlider slider:
-                        Position = slider.TailCircle.ToSpaceOfOtherDrawable(slider.TailCircle.OriginPosition, Parent!);
-                        break;
-
-                    default:
-                        Position = osuObject.ToSpaceOfOtherDrawable(osuObject.OriginPosition, Parent!);
-                        break;
-                }
-
-                positionTransferred = true;
-
-                Scale = new Vector2(osuObject.HitObject.Scale);
-            }
         }
 
         protected override void ApplyHitAnimations()

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 DrawableOsuJudgement? targetJudgement;
-        private JudgementResult targetResult;
+        private JudgementResult? targetResult;
 
         public SkinnableLighting()
             : base("lighting")
@@ -29,11 +27,11 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
         /// <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="targetJudgement">The <see cref="DrawableHitObject"/> that's been judged.</param>
-        /// <param name="targetResult">The <see cref="JudgementResult"/> that <paramref name="targetObject"/> was judged with.</param>
+        /// <param name="targetResult">The <see cref="JudgementResult"/> that <paramref name="targetJudgement"/> was judged with.</param>
-        public void SetColourFrom(DrawableHitObject targetObject, JudgementResult targetResult)
+        public void SetColourFrom(DrawableOsuJudgement targetJudgement, JudgementResult? targetResult)
         {
-            this.targetObject = targetObject;
+            this.targetJudgement = targetJudgement;
             this.targetResult = targetResult;
 
             updateColour();
@@ -41,10 +39,10 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
 
         private void updateColour()
         {
-            if (targetObject == null || targetResult == null)
+            if (targetJudgement == null || targetResult == null)
                 Colour = Color4.White;
             else
-                Colour = targetResult.IsHit ? targetObject.AccentColour.Value : Color4.Transparent;
+                Colour = targetResult.IsHit ? targetJudgement.AccentColour : Color4.Transparent;
         }
     }
 }

osu.Game.Tests/Visual/Gameplay/TestSceneJudgementContainer.cs

@@ -0,0 +1,98 @@
+// 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 NUnit.Framework;
+using osu.Framework.Graphics;
+using osu.Framework.Testing;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Rulesets.Judgements;
+using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.Osu.Judgements;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
+using osu.Game.Rulesets.Scoring;
+using osu.Game.Rulesets.UI;
+
+namespace osu.Game.Tests.Visual.Gameplay
+{
+    public partial class TestSceneJudgementContainer : OsuTestScene
+    {
+        private JudgementContainer<DrawableOsuJudgement> judgementContainer = null!;
+
+        [SetUpSteps]
+        public void SetUp()
+        {
+            AddStep("create judgement container", () => Child = judgementContainer = new JudgementContainer<DrawableOsuJudgement>
+            {
+                Anchor = Anchor.Centre,
+                Origin = Anchor.Centre
+            });
+        }
+
+        [Test]
+        public void TestJudgementFromSameHitObjectIsRemoved()
+        {
+            DrawableHitCircle drawableHitCircle1 = null!;
+            DrawableHitCircle drawableHitCircle2 = null!;
+
+            AddStep("create hit circles", () =>
+            {
+                Add(drawableHitCircle1 = new DrawableHitCircle(createHitCircle()));
+                Add(drawableHitCircle2 = new DrawableHitCircle(createHitCircle()));
+            });
+
+            int judgementCount = 0;
+
+            AddStep("judge the same hitobject twice via different drawables", () =>
+            {
+                addDrawableJudgement(drawableHitCircle1);
+                drawableHitCircle2.Apply(drawableHitCircle1.HitObject);
+                addDrawableJudgement(drawableHitCircle2);
+                judgementCount = judgementContainer.Count;
+            });
+
+            AddAssert("one judgement in container", () => judgementCount, () => Is.EqualTo(1));
+        }
+
+        [Test]
+        public void TestJudgementFromDifferentHitObjectIsNotRemoved()
+        {
+            DrawableHitCircle drawableHitCircle = null!;
+
+            AddStep("create hit circle", () => Add(drawableHitCircle = new DrawableHitCircle(createHitCircle())));
+
+            int judgementCount = 0;
+
+            AddStep("judge two hitobjects via the same drawable", () =>
+            {
+                addDrawableJudgement(drawableHitCircle);
+                drawableHitCircle.Apply(createHitCircle());
+                addDrawableJudgement(drawableHitCircle);
+                judgementCount = judgementContainer.Count;
+            });
+
+            AddAssert("two judgements in container", () => judgementCount, () => Is.EqualTo(2));
+        }
+
+        private void addDrawableJudgement(DrawableHitObject drawableHitObject)
+        {
+            var judgement = new DrawableOsuJudgement();
+
+            judgement.Apply(new JudgementResult(drawableHitObject.HitObject, new OsuJudgement())
+            {
+                Type = HitResult.Great,
+                TimeOffset = Time.Current
+            }, drawableHitObject);
+
+            judgementContainer.Add(judgement);
+        }
+
+        private HitCircle createHitCircle()
+        {
+            var circle = new HitCircle();
+            circle.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty());
+            return circle;
+        }
+    }
+}

osu.Game/Rulesets/Judgements/DrawableJudgement.cs

@@ -7,6 +7,7 @@ using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Pooling;
+using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.Scoring;
 using osu.Game.Skinning;
@@ -23,7 +24,7 @@ namespace osu.Game.Rulesets.Judgements
 
         public JudgementResult? Result { get; private set; }
 
-        public DrawableHitObject? JudgedObject { get; private set; }
+        public HitObject? JudgedHitObject { get; private set; }
 
         public override bool RemoveCompletedTransforms => false;
 
@@ -94,17 +95,17 @@ namespace osu.Game.Rulesets.Judgements
         /// </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;
         }
 
         protected override void FreeAfterUse()
         {
             base.FreeAfterUse();
 
-            JudgedObject = null;
+            JudgedHitObject = null;
         }
 
         protected override void PrepareForUse()

osu.Game/Rulesets/UI/JudgementContainer.cs

@@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.UI
 
             // remove any existing judgements for the judged object.
             // this can be the case when rewinding.
-            RemoveAll(c => c.JudgedObject == judgement.JudgedObject, false);
+            RemoveAll(c => c.JudgedHitObject == judgement.JudgedHitObject, false);
 
             base.Add(judgement);
         }