Merge pull request #25672 from frenzibyte/fix-argon-health-display-2

Fix `ArgonHealthDisplay` sometimes behaving weirdly on miss judgements (alternative)

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

@@ -24,6 +24,23 @@ namespace osu.Game.Tests.Visual.Gameplay
 
         private ArgonHealthDisplay healthDisplay = null!;
 
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            AddSliderStep("Height", 0, 64, 0, val =>
+            {
+                if (healthDisplay.IsNotNull())
+                    healthDisplay.BarHeight.Value = val;
+            });
+
+            AddSliderStep("Width", 0, 1f, 0.98f, val =>
+            {
+                if (healthDisplay.IsNotNull())
+                    healthDisplay.Width = val;
+            });
+        }
+
         [SetUpSteps]
         public void SetUpSteps()
         {
@@ -46,27 +63,12 @@ namespace osu.Game.Tests.Visual.Gameplay
                     },
                 };
             });
-
-            AddSliderStep("Height", 0, 64, 0, val =>
-            {
-                if (healthDisplay.IsNotNull())
-                    healthDisplay.BarHeight.Value = val;
-            });
-
-            AddSliderStep("Width", 0, 1f, 0.98f, val =>
-            {
-                if (healthDisplay.IsNotNull())
-                    healthDisplay.Width = val;
-            });
         }
 
         [Test]
         public void TestHealthDisplayIncrementing()
         {
-            AddRepeatStep("apply miss judgement", delegate
+            AddRepeatStep("apply miss judgement", applyMiss, 5);
-            {
-                healthProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Miss });
-            }, 5);
 
             AddRepeatStep(@"decrease hp slightly", delegate
             {
@@ -81,11 +83,81 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddRepeatStep(@"increase hp with flash", delegate
             {
                 healthProcessor.Health.Value += 0.1f;
+                applyPerfectHit();
+            }, 3);
+        }
+
+        [Test]
+        public void TestLateMissAfterConsequentMisses()
+        {
+            AddUntilStep("wait for health", () => healthDisplay.Current.Value == 1);
+            AddStep("apply sequence", () =>
+            {
+                for (int i = 0; i < 10; i++)
+                    applyMiss();
+
+                Scheduler.AddDelayed(applyMiss, 500 + 30);
+            });
+        }
+
+        [Test]
+        public void TestMissAlmostExactlyAfterLastMissAnimation()
+        {
+            AddUntilStep("wait for health", () => healthDisplay.Current.Value == 1);
+            AddStep("apply sequence", () =>
+            {
+                const double interval = 500 + 15;
+
+                for (int i = 0; i < 5; i++)
+                {
+                    if (i % 2 == 0)
+                        Scheduler.AddDelayed(applyMiss, i * interval);
+                    else
+                    {
+                        Scheduler.AddDelayed(applyMiss, i * interval);
+                        Scheduler.AddDelayed(applyMiss, i * interval);
+                    }
+                }
+            });
+        }
+
+        [Test]
+        public void TestMissThenHitAtSameUpdateFrame()
+        {
+            AddUntilStep("wait for health", () => healthDisplay.Current.Value == 1);
+            AddStep("set half health", () => healthProcessor.Health.Value = 0.5f);
+
+            AddStep("apply miss and hit", () =>
+            {
+                applyMiss();
+                applyMiss();
+                applyPerfectHit();
+                applyPerfectHit();
+            });
+
+            AddWaitStep("wait", 3);
+
+            AddStep("apply miss and cancel with hit", () =>
+            {
+                applyMiss();
+                applyPerfectHit();
+                applyPerfectHit();
+                applyPerfectHit();
+                applyPerfectHit();
+            });
+        }
+
+        private void applyMiss()
+        {
+            healthProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Miss });
+        }
+
+        private void applyPerfectHit()
+        {
             healthProcessor.ApplyResult(new JudgementResult(new HitCircle(), new OsuJudgement())
             {
                 Type = HitResult.Perfect
             });
-            }, 3);
         }
     }
 }

osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs

@@ -7,6 +7,7 @@ using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Extensions.Color4Extensions;
+using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Colour;
 using osu.Framework.Graphics.Containers;
@@ -15,6 +16,7 @@ using osu.Framework.Layout;
 using osu.Framework.Threading;
 using osu.Framework.Utils;
 using osu.Game.Configuration;
+using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Skinning;
@@ -54,6 +56,8 @@ namespace osu.Game.Screens.Play.HUD
 
         private ScheduledDelegate? resetMissBarDelegate;
 
+        private bool displayingMiss => resetMissBarDelegate != null;
+
         private readonly List<Vector2> missBarVertices = new List<Vector2>();
         private readonly List<Vector2> healthBarVertices = new List<Vector2>();
 
@@ -147,11 +151,14 @@ namespace osu.Game.Screens.Play.HUD
             };
         }
 
+        private bool pendingMissAnimation;
+
         protected override void LoadComplete()
         {
             base.LoadComplete();
 
-            Current.BindValueChanged(_ => Scheduler.AddOnce(updateCurrent), true);
+            HealthProcessor.NewJudgement += onNewJudgement;
+            Current.BindValueChanged(onCurrentChanged, true);
 
             // we're about to set `RelativeSizeAxes` depending on the value of `UseRelativeSize`.
             // setting `RelativeSizeAxes` internally transforms absolute sizing to relative and back to keep the size the same,
@@ -164,15 +171,31 @@ namespace osu.Game.Screens.Play.HUD
             BarHeight.BindValueChanged(_ => updatePath(), true);
         }
 
-        private void updateCurrent()
+        private void onNewJudgement(JudgementResult result) => pendingMissAnimation |= !result.IsHit;
-        {
-            if (Current.Value >= GlowBarValue) finishMissDisplay();
 
-            double time = Current.Value > GlowBarValue ? 500 : 250;
+        private void onCurrentChanged(ValueChangedEvent<double> valueChangedEvent)
+            // schedule display updates one frame later to ensure we know the judgement result causing this change (if there is one).
+            => Scheduler.AddOnce(updateDisplay);
+
+        private void updateDisplay()
+        {
+            double newHealth = Current.Value;
+
+            if (newHealth >= GlowBarValue)
+                finishMissDisplay();
+
+            double time = newHealth > GlowBarValue ? 500 : 250;
 
             // TODO: this should probably use interpolation in update.
-            this.TransformTo(nameof(HealthBarValue), Current.Value, time, Easing.OutQuint);
+            this.TransformTo(nameof(HealthBarValue), newHealth, time, Easing.OutQuint);
-            if (resetMissBarDelegate == null) this.TransformTo(nameof(GlowBarValue), Current.Value, time, Easing.OutQuint);
+
+            if (pendingMissAnimation && newHealth < GlowBarValue)
+                triggerMissDisplay();
+
+            pendingMissAnimation = false;
+
+            if (!displayingMiss)
+                this.TransformTo(nameof(GlowBarValue), newHealth, time, Easing.OutQuint);
         }
 
         protected override void Update()
@@ -196,7 +219,7 @@ namespace osu.Game.Screens.Play.HUD
             mainBar.TransformTo(nameof(BarPath.GlowColour), main_bar_glow_colour.Opacity(0.8f))
                    .TransformTo(nameof(BarPath.GlowColour), main_bar_glow_colour, 300, Easing.OutQuint);
 
-            if (resetMissBarDelegate == null)
+            if (!displayingMiss)
             {
                 glowBar.TransformTo(nameof(BarPath.BarColour), Colour4.White, 30, Easing.OutQuint)
                        .Then()
@@ -208,20 +231,10 @@ namespace osu.Game.Screens.Play.HUD
             }
         }
 
-        protected override void Miss()
+        private void triggerMissDisplay()
         {
-            base.Miss();
+            resetMissBarDelegate?.Cancel();
-
-            if (resetMissBarDelegate != null)
-            {
-                resetMissBarDelegate.Cancel();
             resetMissBarDelegate = null;
-            }
-            else
-            {
-                // Reset any ongoing animation immediately, else things get weird.
-                this.TransformTo(nameof(GlowBarValue), HealthBarValue);
-            }
 
             this.Delay(500).Schedule(() =>
             {
@@ -238,7 +251,7 @@ namespace osu.Game.Screens.Play.HUD
 
         private void finishMissDisplay()
         {
-            if (resetMissBarDelegate == null)
+            if (!displayingMiss)
                 return;
 
             if (Current.Value > 0)
@@ -328,6 +341,14 @@ namespace osu.Game.Screens.Play.HUD
             mainBar.Position = healthBarVertices[0];
         }
 
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            if (HealthProcessor.IsNotNull())
+                HealthProcessor.NewJudgement -= onNewJudgement;
+        }
+
         private partial class BackgroundPath : SmoothPath
         {
             protected override Color4 ColourAt(float position)

osu.Game/Screens/Play/HUD/HealthDisplay.cs

@@ -45,14 +45,6 @@ namespace osu.Game.Screens.Play.HUD
         {
         }
 
-        /// <summary>
-        /// Triggered when a <see cref="Judgement"/> resulted in the player losing health.
-        /// Calls to this method are debounced.
-        /// </summary>
-        protected virtual void Miss()
-        {
-        }
-
         [Resolved]
         private HUDOverlay? hudOverlay { get; set; }
 
@@ -122,8 +114,6 @@ namespace osu.Game.Screens.Play.HUD
         {
             if (judgement.IsHit && judgement.Type != HitResult.IgnoreHit)
                 Scheduler.AddOnce(Flash);
-            else if (judgement.Judgement.HealthIncreaseFor(judgement) < 0)
-                Scheduler.AddOnce(Miss);
         }
 
         protected override void Dispose(bool isDisposing)