Ensure health displays don't pile up transforms when off-screen

[peppy]

Closes #25050.

I noticed theres some other HUD elements with similar issues, but they are far less prominent. Will get to those another time.

Can be tested with:

diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs
index 7bad623d7f..dafc3921af 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs
@@ -7,6 +7,7 @@
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Testing;
+using osu.Framework.Utils;
 using osu.Game.Rulesets.Judgements;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Osu.Judgements;
@@ -58,6 +59,23 @@ public void SetUpSteps()
                 if (healthDisplay.IsNotNull())
                     healthDisplay.BarHeight.Value = val;
             });
+
+            AddStep("Move off screen", () =>
+            {
+                healthDisplay.X = -10000;
+            });
+            AddStep("Move on screen", () =>
+            {
+                healthDisplay.X = 0;
+            });
+
+            AddStep("do heaps of shit", () =>
+            {
+                for (int i = 0; i < 100000; i++)
+                {
+                    healthDisplay.Current.Value = RNG.NextDouble();
+                }
+            });
         }
 
         [Test]

[bdach]

The fix side looks okay, just a few minor concerns / side notes

[bdach]

Just to reiterate / explain - delegates with this captures cannot be correctly used inline with AddOnce(). Captures cause the compiler to generate code that allocates a new delegate every call, and that fails on the equality check framework-side.

If you didn't want (or couldn't for whatever reason) to use method groups, the only (and pretty cursed) alternative would be to write the delegate such that it doesn't require captures:

diff --git a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs
index 9c7c0684b3..04e0307dc6 100644
--- a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs
+++ b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs
@@ -142,18 +142,18 @@ protected override void LoadComplete()
 
             Current.BindValueChanged(v =>
             {
-                Scheduler.AddOnce(() =>
+                Scheduler.AddOnce(display =>
                 {
-                    if (v.NewValue >= GlowBarValue)
-                        finishMissDisplay();
+                    if (display.Current.Value >= display.GlowBarValue)
+                        display.finishMissDisplay();
 
-                    double time = v.NewValue > GlowBarValue ? 500 : 250;
+                    double time = display.Current.Value > display.GlowBarValue ? 500 : 250;
 
                     // TODO: this should probably use interpolation in update.
-                    this.TransformTo(nameof(HealthBarValue), v.NewValue, time, Easing.OutQuint);
-                    if (resetMissBarDelegate == null)
-                        this.TransformTo(nameof(GlowBarValue), v.NewValue, time, Easing.OutQuint);
-                });
+                    display.TransformTo(nameof(HealthBarValue), display.Current.Value, time, Easing.OutQuint);
+                    if (display.resetMissBarDelegate == null)
+                        display.TransformTo(nameof(GlowBarValue), display.Current.Value, time, Easing.OutQuint);
+                }, this);
             }, true);
 
             BarLength.BindValueChanged(l => Width = l.NewValue, true);

This makes the compiler figure out that it can allocate just one instance of this delegate and re-use it across different ArgonHealthDisplay instances, and makes AddOnce() "magically" work again.

One for an analyser to check in the future maybe. If we ever get to writing those.

tl;dr: when in doubt about AddOnce(), use method group

[peppy]

Yeah I don't think it's worth the proposed change. I may just remove the comment and add some safety comments framework side instead.

[bdach]

The patch above was in no way a serious proposal mind. It was mostly supposed to be illustrative as to why sometimes inline delegates work and sometimes they don't. I wouldn't personally want to get caught committing such ugly code.