From 17577a660654cd2e7b5f8a61405064ebed5aacc4 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 28 Nov 2023 04:40:28 +0300 Subject: [PATCH 1/8] Add visual test case for late miss in argon health display --- .../Gameplay/TestSceneArgonHealthDisplay.cs | 73 +++++++++++++++---- 1 file changed, 57 insertions(+), 16 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs index 6c364e41c7..3197de42d0 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs +++ b/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 - { - healthProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Miss }); - }, 5); + AddRepeatStep("apply miss judgement", applyMiss, 5); AddRepeatStep(@"decrease hp slightly", delegate { @@ -87,5 +89,44 @@ namespace osu.Game.Tests.Visual.Gameplay }); }, 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); + } + } + }); + } + + private void applyMiss() + { + healthProcessor.ApplyResult(new JudgementResult(new HitObject(), new Judgement()) { Type = HitResult.Miss }); + } } } From 5723715ea01b8790699ff2562303caa0d7e6b6ee Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Dec 2023 23:23:48 +0300 Subject: [PATCH 2/8] Fix `ArgonHealthDisplay` sometimes behaving weirdly on miss judgements --- .../Screens/Play/HUD/ArgonHealthDisplay.cs | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs index 3f6791d226..2bef6c312f 100644 --- a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs +++ b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs @@ -15,6 +15,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 +55,8 @@ namespace osu.Game.Screens.Play.HUD private ScheduledDelegate? resetMissBarDelegate; + private bool displayingMiss => resetMissBarDelegate != null; + private readonly List missBarVertices = new List(); private readonly List healthBarVertices = new List(); @@ -147,10 +150,13 @@ namespace osu.Game.Screens.Play.HUD }; } + private JudgementResult? pendingJudgementResult; + protected override void LoadComplete() { base.LoadComplete(); + HealthProcessor.NewJudgement += result => pendingJudgementResult = result; Current.BindValueChanged(_ => Scheduler.AddOnce(updateCurrent), true); // we're about to set `RelativeSizeAxes` depending on the value of `UseRelativeSize`. @@ -166,13 +172,22 @@ namespace osu.Game.Screens.Play.HUD private void updateCurrent() { - if (Current.Value >= GlowBarValue) finishMissDisplay(); + var result = pendingJudgementResult; + + if (Current.Value >= GlowBarValue) + finishMissDisplay(); double time = Current.Value > GlowBarValue ? 500 : 250; // TODO: this should probably use interpolation in update. this.TransformTo(nameof(HealthBarValue), Current.Value, time, Easing.OutQuint); - if (resetMissBarDelegate == null) this.TransformTo(nameof(GlowBarValue), Current.Value, time, Easing.OutQuint); + + if (result != null && !result.IsHit) + triggerMissDisplay(); + else if (!displayingMiss) + this.TransformTo(nameof(GlowBarValue), Current.Value, time, Easing.OutQuint); + + pendingJudgementResult = null; } protected override void Update() @@ -196,7 +211,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 +223,10 @@ namespace osu.Game.Screens.Play.HUD } } - protected override void Miss() + private void triggerMissDisplay() { - base.Miss(); - - if (resetMissBarDelegate != null) - { - resetMissBarDelegate.Cancel(); - resetMissBarDelegate = null; - } - else - { - // Reset any ongoing animation immediately, else things get weird. - this.TransformTo(nameof(GlowBarValue), HealthBarValue); - } + resetMissBarDelegate?.Cancel(); + resetMissBarDelegate = null; this.Delay(500).Schedule(() => { @@ -238,7 +243,7 @@ namespace osu.Game.Screens.Play.HUD private void finishMissDisplay() { - if (resetMissBarDelegate == null) + if (!displayingMiss) return; if (Current.Value > 0) From 4d82a555942d1b6e2e30a0c219a160feec598f81 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 4 Dec 2023 23:23:58 +0300 Subject: [PATCH 3/8] Remove method for being unused --- osu.Game/Screens/Play/HUD/HealthDisplay.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/HealthDisplay.cs b/osu.Game/Screens/Play/HUD/HealthDisplay.cs index fdbce15b40..13dc05229e 100644 --- a/osu.Game/Screens/Play/HUD/HealthDisplay.cs +++ b/osu.Game/Screens/Play/HUD/HealthDisplay.cs @@ -45,14 +45,6 @@ namespace osu.Game.Screens.Play.HUD { } - /// - /// Triggered when a resulted in the player losing health. - /// Calls to this method are debounced. - /// - 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) From 927cfe42570db2edb8e51cbcfd0a729e6b1cb6e5 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 5 Dec 2023 19:44:01 +0300 Subject: [PATCH 4/8] Fix health processor event leaks --- .../Screens/Play/HUD/ArgonHealthDisplay.cs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs index 2bef6c312f..e044db7bb2 100644 --- a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs +++ b/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; @@ -156,8 +157,8 @@ namespace osu.Game.Screens.Play.HUD { base.LoadComplete(); - HealthProcessor.NewJudgement += result => pendingJudgementResult = result; - 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, @@ -170,7 +171,12 @@ namespace osu.Game.Screens.Play.HUD BarHeight.BindValueChanged(_ => updatePath(), true); } - private void updateCurrent() + private void onNewJudgement(JudgementResult result) => pendingJudgementResult = result; + + private void onCurrentChanged(ValueChangedEvent valueChangedEvent) + => Scheduler.AddOnce(updateDisplay); + + private void updateDisplay() { var result = pendingJudgementResult; @@ -333,6 +339,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) From 9496cdf42bf9d024ca40a0622677e60a233693ed Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 5 Dec 2023 19:44:14 +0300 Subject: [PATCH 5/8] Add explanatory note for scheduling --- osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs index e044db7bb2..9993ca1ef6 100644 --- a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs +++ b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs @@ -174,6 +174,7 @@ namespace osu.Game.Screens.Play.HUD private void onNewJudgement(JudgementResult result) => pendingJudgementResult = result; private void onCurrentChanged(ValueChangedEvent 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() From 986f4fa407caf9e5906444b2fa4ec0cca5a4b68b Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 5 Dec 2023 21:55:25 +0300 Subject: [PATCH 6/8] Add test scenarios for same-frame judgements --- .../Gameplay/TestSceneArgonHealthDisplay.cs | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs index 3197de42d0..d863755a85 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs @@ -83,13 +83,18 @@ namespace osu.Game.Tests.Visual.Gameplay AddRepeatStep(@"increase hp with flash", delegate { healthProcessor.Health.Value += 0.1f; - healthProcessor.ApplyResult(new JudgementResult(new HitCircle(), new OsuJudgement()) - { - Type = HitResult.Perfect - }); + applyPerfectHit(); }, 3); } + private void applyPerfectHit() + { + healthProcessor.ApplyResult(new JudgementResult(new HitCircle(), new OsuJudgement()) + { + Type = HitResult.Perfect + }); + } + [Test] public void TestLateMissAfterConsequentMisses() { @@ -124,6 +129,29 @@ namespace osu.Game.Tests.Visual.Gameplay }); } + [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 }); From 20fd458fac9f85140328d22948fb33a53d2c2c78 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 5 Dec 2023 21:57:53 +0300 Subject: [PATCH 7/8] Perserve miss animation when followed by a hit at same frame --- .../Gameplay/TestSceneArgonHealthDisplay.cs | 3 +++ .../Screens/Play/HUD/ArgonHealthDisplay.cs | 21 ++++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs index d863755a85..59819d781f 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs @@ -134,6 +134,7 @@ namespace osu.Game.Tests.Visual.Gameplay { AddUntilStep("wait for health", () => healthDisplay.Current.Value == 1); AddStep("set half health", () => healthProcessor.Health.Value = 0.5f); + AddStep("apply miss and hit", () => { applyMiss(); @@ -141,7 +142,9 @@ namespace osu.Game.Tests.Visual.Gameplay applyPerfectHit(); applyPerfectHit(); }); + AddWaitStep("wait", 3); + AddStep("apply miss and cancel with hit", () => { applyMiss(); diff --git a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs index 9993ca1ef6..eaaf1c3c14 100644 --- a/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs +++ b/osu.Game/Screens/Play/HUD/ArgonHealthDisplay.cs @@ -151,7 +151,7 @@ namespace osu.Game.Screens.Play.HUD }; } - private JudgementResult? pendingJudgementResult; + private bool pendingMissAnimation; protected override void LoadComplete() { @@ -171,7 +171,7 @@ namespace osu.Game.Screens.Play.HUD BarHeight.BindValueChanged(_ => updatePath(), true); } - private void onNewJudgement(JudgementResult result) => pendingJudgementResult = result; + private void onNewJudgement(JudgementResult result) => pendingMissAnimation |= !result.IsHit; private void onCurrentChanged(ValueChangedEvent valueChangedEvent) // schedule display updates one frame later to ensure we know the judgement result causing this change (if there is one). @@ -179,22 +179,23 @@ namespace osu.Game.Screens.Play.HUD private void updateDisplay() { - var result = pendingJudgementResult; + double newHealth = Current.Value; - if (Current.Value >= GlowBarValue) + if (newHealth >= GlowBarValue) finishMissDisplay(); - double time = Current.Value > GlowBarValue ? 500 : 250; + 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 (result != null && !result.IsHit) + if (pendingMissAnimation && newHealth < GlowBarValue) triggerMissDisplay(); - else if (!displayingMiss) - this.TransformTo(nameof(GlowBarValue), Current.Value, time, Easing.OutQuint); - pendingJudgementResult = null; + pendingMissAnimation = false; + + if (!displayingMiss) + this.TransformTo(nameof(GlowBarValue), newHealth, time, Easing.OutQuint); } protected override void Update() From c55bfc03ee6f02f8234a80b8b0bf11022e763c34 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 5 Dec 2023 22:06:53 +0300 Subject: [PATCH 8/8] Move private method --- .../Gameplay/TestSceneArgonHealthDisplay.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs index 59819d781f..30fb4412f4 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneArgonHealthDisplay.cs @@ -87,14 +87,6 @@ namespace osu.Game.Tests.Visual.Gameplay }, 3); } - private void applyPerfectHit() - { - healthProcessor.ApplyResult(new JudgementResult(new HitCircle(), new OsuJudgement()) - { - Type = HitResult.Perfect - }); - } - [Test] public void TestLateMissAfterConsequentMisses() { @@ -159,5 +151,13 @@ namespace osu.Game.Tests.Visual.Gameplay { 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 + }); + } } }