From 17eaf7bb5c2d1041738ccf780c96e1b1214a2e21 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 13 Jun 2022 16:35:46 +0900 Subject: [PATCH 1/6] Add failing test coverage showing hit meters don't update when not visible --- .../Visual/Gameplay/TestSceneHitErrorMeter.cs | 30 +++++++++++++++++++ .../HUD/HitErrorMeters/ColourHitErrorMeter.cs | 12 ++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs index 7febb54010..cbf9760e21 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs @@ -142,6 +142,36 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("no circle added", () => !this.ChildrenOfType().Any()); } + [Test] + public void TestProcessingWhileHidden() + { + AddStep("OD 1", () => recreateDisplay(new OsuHitWindows(), 1)); + + AddStep("hide displays", () => + { + foreach (var hitErrorMeter in this.ChildrenOfType()) + hitErrorMeter.Hide(); + }); + + AddRepeatStep("hit", () => newJudgement(), ColourHitErrorMeter.MAX_DISPLAYED_JUDGEMENTS * 2); + + AddAssert("bars added", () => this.ChildrenOfType().Any()); + AddAssert("circle added", () => this.ChildrenOfType().Any()); + + AddUntilStep("wait for bars to disappear", () => !this.ChildrenOfType().Any()); + AddUntilStep("ensure max circles not exceeded", () => + { + return this.ChildrenOfType() + .All(m => m.ChildrenOfType().Count() <= ColourHitErrorMeter.MAX_DISPLAYED_JUDGEMENTS); + }); + + AddStep("show displays", () => + { + foreach (var hitErrorMeter in this.ChildrenOfType()) + hitErrorMeter.Show(); + }); + } + [Test] public void TestClear() { diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs index 5012be7249..9948b968d1 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs @@ -15,7 +15,11 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { public class ColourHitErrorMeter : HitErrorMeter { + internal const int MAX_DISPLAYED_JUDGEMENTS = 20; + private const int animation_duration = 200; + private const int drawable_judgement_size = 8; + private const int spacing = 2; private readonly JudgementFlow judgementsFlow; @@ -37,16 +41,12 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters private class JudgementFlow : FillFlowContainer { - private const int max_available_judgements = 20; - private const int drawable_judgement_size = 8; - private const int spacing = 2; - public override IEnumerable FlowingChildren => base.FlowingChildren.Reverse(); public JudgementFlow() { AutoSizeAxes = Axes.X; - Height = max_available_judgements * (drawable_judgement_size + spacing) - spacing; + Height = MAX_DISPLAYED_JUDGEMENTS * (drawable_judgement_size + spacing) - spacing; Spacing = new Vector2(0, spacing); Direction = FillDirection.Vertical; LayoutDuration = animation_duration; @@ -57,7 +57,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { Add(new HitErrorCircle(colour, drawable_judgement_size)); - if (Children.Count > max_available_judgements) + if (Children.Count > MAX_DISPLAYED_JUDGEMENTS) Children.FirstOrDefault(c => !c.IsRemoved)?.Remove(); } } From 86163d2225157af9c874af5b2dd59208572f8349 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 13 Jun 2022 16:37:26 +0900 Subject: [PATCH 2/6] Fix hit error meters not updating visual state when hidden It is an expectation of users that when the HUD is shown after a period of being hidden, it will visually reflect the state based on recent judgements. To achieve this, I've added `AlwaysPresent` and moved the transform application to the meter level, rather than at a child level. If this is seen as a bad direction, `AlwaysPresent` can be applied to the drawable children and the transforms can be moved back. Also of note, `ColourHitErrorMeter` is pretty weird. The flow class could potentially be removed and reduce `AlwaysPresent` usage by one. Can do that refactor as part of this PR if preferred. Closes #18624. --- .../HUD/HitErrorMeters/BarHitErrorMeter.cs | 35 ++++++++++--------- .../HUD/HitErrorMeters/ColourHitErrorMeter.cs | 34 +++++++----------- .../Play/HUD/HitErrorMeters/HitErrorMeter.cs | 2 ++ 3 files changed, 33 insertions(+), 38 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs index dca50c07ad..79fd22eb0e 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs @@ -400,6 +400,9 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { const int arrow_move_duration = 800; + const int judgement_fade_in_duration = 100; + const int judgement_fade_out_duration = 5000; + if (!judgement.IsHit || judgement.HitObject.HitWindows?.WindowFor(HitResult.Miss) == 0) return; @@ -420,12 +423,26 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters } } - judgementsContainer.Add(new JudgementLine + var judgementLine = new JudgementLine { JudgementLineThickness = { BindTarget = JudgementLineThickness }, Y = getRelativeJudgementPosition(judgement.TimeOffset), Colour = GetColourForHitResult(judgement.Type), - }); + Alpha = 0, + Width = 0, + }; + + judgementsContainer.Add(judgementLine); + + // Importantly, transforms should be applied in this method rather than constructed drawables + // to ensure that they are applied even when the `HitErrorMeter` is hidden (see `AlwaysPresent` usage). + judgementLine + .FadeTo(0.6f, judgement_fade_in_duration, Easing.OutQuint) + .ResizeWidthTo(1, judgement_fade_in_duration, Easing.OutQuint) + .Then() + .FadeOut(judgement_fade_out_duration) + .ResizeWidthTo(0, judgement_fade_out_duration, Easing.InQuint) + .Expire(); arrow.MoveToY( getRelativeJudgementPosition(floatingAverage = floatingAverage * 0.9 + judgement.TimeOffset * 0.1) @@ -456,23 +473,9 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters protected override void LoadComplete() { - const int judgement_fade_in_duration = 100; - const int judgement_fade_out_duration = 5000; - base.LoadComplete(); - Alpha = 0; - Width = 0; - JudgementLineThickness.BindValueChanged(thickness => Height = thickness.NewValue, true); - - this - .FadeTo(0.6f, judgement_fade_in_duration, Easing.OutQuint) - .ResizeWidthTo(1, judgement_fade_in_duration, Easing.OutQuint) - .Then() - .FadeOut(judgement_fade_out_duration) - .ResizeWidthTo(0, judgement_fade_out_duration, Easing.InQuint) - .Expire(); } } diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs index 9948b968d1..285a2f8251 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs @@ -51,47 +51,37 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters Direction = FillDirection.Vertical; LayoutDuration = animation_duration; LayoutEasing = Easing.OutQuint; + AlwaysPresent = true; } public void Push(Color4 colour) { - Add(new HitErrorCircle(colour, drawable_judgement_size)); + var hitErrorCircle = new HitErrorCircle(colour); + + Add(hitErrorCircle); + + hitErrorCircle.FadeInFromZero(animation_duration, Easing.OutQuint); + hitErrorCircle.MoveToY(-drawable_judgement_size); + hitErrorCircle.MoveToY(0, animation_duration, Easing.OutQuint); if (Children.Count > MAX_DISPLAYED_JUDGEMENTS) Children.FirstOrDefault(c => !c.IsRemoved)?.Remove(); } } - internal class HitErrorCircle : Container + internal class HitErrorCircle : Circle { public bool IsRemoved { get; private set; } - private readonly Circle circle; - - public HitErrorCircle(Color4 colour, int size) + public HitErrorCircle(Color4 colour) { - Size = new Vector2(size); - Child = circle = new Circle - { - RelativeSizeAxes = Axes.Both, - Alpha = 0, - Colour = colour - }; - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - circle.FadeInFromZero(animation_duration, Easing.OutQuint); - circle.MoveToY(-DrawSize.Y); - circle.MoveToY(0, animation_duration, Easing.OutQuint); + Colour = colour; + Size = new Vector2(drawable_judgement_size); } public void Remove() { IsRemoved = true; - this.FadeOut(animation_duration, Easing.OutQuint).Expire(); } } diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index 1f08cb8aa7..c4e738de28 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -37,6 +37,8 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { base.LoadComplete(); + AlwaysPresent = true; + if (gameplayClockContainer != null) gameplayClockContainer.OnSeek += Clear; From bd9ea9bd6fa8f1717e601ef5b7c24076714875e2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 13 Jun 2022 23:54:43 +0900 Subject: [PATCH 3/6] Revert most unnecessary changes Turns out `AlwaysPresent` at top level is actually enough. This reverts commit 86163d2225157af9c874af5b2dd59208572f8349. --- .../HUD/HitErrorMeters/BarHitErrorMeter.cs | 35 +++++++++---------- .../HUD/HitErrorMeters/ColourHitErrorMeter.cs | 34 +++++++++++------- .../Play/HUD/HitErrorMeters/HitErrorMeter.cs | 5 +-- 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs index 79fd22eb0e..dca50c07ad 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs @@ -400,9 +400,6 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { const int arrow_move_duration = 800; - const int judgement_fade_in_duration = 100; - const int judgement_fade_out_duration = 5000; - if (!judgement.IsHit || judgement.HitObject.HitWindows?.WindowFor(HitResult.Miss) == 0) return; @@ -423,26 +420,12 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters } } - var judgementLine = new JudgementLine + judgementsContainer.Add(new JudgementLine { JudgementLineThickness = { BindTarget = JudgementLineThickness }, Y = getRelativeJudgementPosition(judgement.TimeOffset), Colour = GetColourForHitResult(judgement.Type), - Alpha = 0, - Width = 0, - }; - - judgementsContainer.Add(judgementLine); - - // Importantly, transforms should be applied in this method rather than constructed drawables - // to ensure that they are applied even when the `HitErrorMeter` is hidden (see `AlwaysPresent` usage). - judgementLine - .FadeTo(0.6f, judgement_fade_in_duration, Easing.OutQuint) - .ResizeWidthTo(1, judgement_fade_in_duration, Easing.OutQuint) - .Then() - .FadeOut(judgement_fade_out_duration) - .ResizeWidthTo(0, judgement_fade_out_duration, Easing.InQuint) - .Expire(); + }); arrow.MoveToY( getRelativeJudgementPosition(floatingAverage = floatingAverage * 0.9 + judgement.TimeOffset * 0.1) @@ -473,9 +456,23 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters protected override void LoadComplete() { + const int judgement_fade_in_duration = 100; + const int judgement_fade_out_duration = 5000; + base.LoadComplete(); + Alpha = 0; + Width = 0; + JudgementLineThickness.BindValueChanged(thickness => Height = thickness.NewValue, true); + + this + .FadeTo(0.6f, judgement_fade_in_duration, Easing.OutQuint) + .ResizeWidthTo(1, judgement_fade_in_duration, Easing.OutQuint) + .Then() + .FadeOut(judgement_fade_out_duration) + .ResizeWidthTo(0, judgement_fade_out_duration, Easing.InQuint) + .Expire(); } } diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs index 285a2f8251..9948b968d1 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs @@ -51,37 +51,47 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters Direction = FillDirection.Vertical; LayoutDuration = animation_duration; LayoutEasing = Easing.OutQuint; - AlwaysPresent = true; } public void Push(Color4 colour) { - var hitErrorCircle = new HitErrorCircle(colour); - - Add(hitErrorCircle); - - hitErrorCircle.FadeInFromZero(animation_duration, Easing.OutQuint); - hitErrorCircle.MoveToY(-drawable_judgement_size); - hitErrorCircle.MoveToY(0, animation_duration, Easing.OutQuint); + Add(new HitErrorCircle(colour, drawable_judgement_size)); if (Children.Count > MAX_DISPLAYED_JUDGEMENTS) Children.FirstOrDefault(c => !c.IsRemoved)?.Remove(); } } - internal class HitErrorCircle : Circle + internal class HitErrorCircle : Container { public bool IsRemoved { get; private set; } - public HitErrorCircle(Color4 colour) + private readonly Circle circle; + + public HitErrorCircle(Color4 colour, int size) { - Colour = colour; - Size = new Vector2(drawable_judgement_size); + Size = new Vector2(size); + Child = circle = new Circle + { + RelativeSizeAxes = Axes.Both, + Alpha = 0, + Colour = colour + }; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + circle.FadeInFromZero(animation_duration, Easing.OutQuint); + circle.MoveToY(-DrawSize.Y); + circle.MoveToY(0, animation_duration, Easing.OutQuint); } public void Remove() { IsRemoved = true; + this.FadeOut(animation_duration, Easing.OutQuint).Expire(); } } diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index c4e738de28..e7aaee9fa1 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -31,14 +31,15 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters private void load(DrawableRuleset drawableRuleset) { HitWindows = drawableRuleset?.FirstAvailableHitWindows ?? HitWindows.Empty; + + // This is to allow the visual state to be correct after HUD comes visible after being hidden. + AlwaysPresent = true; } protected override void LoadComplete() { base.LoadComplete(); - AlwaysPresent = true; - if (gameplayClockContainer != null) gameplayClockContainer.OnSeek += Clear; From a20e43c2ae9f7fad40736eb8658a2245731b782d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 14 Jun 2022 18:22:23 +0900 Subject: [PATCH 4/6] Ensure containers which are being used to hide HUD elements still update for now I don't think this is necessarily a final solution (as this means all HUD elements are adding overhead even when not visible), but this will make the implementations much easier for the time being. I've checked and can't notice any perceivable overhead in profiling so we should be fine for now. --- osu.Game/Screens/Play/HUDOverlay.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/HUDOverlay.cs b/osu.Game/Screens/Play/HUDOverlay.cs index f6087e0958..6419532221 100644 --- a/osu.Game/Screens/Play/HUDOverlay.cs +++ b/osu.Game/Screens/Play/HUDOverlay.cs @@ -86,7 +86,10 @@ namespace osu.Game.Screens.Play Children = new Drawable[] { CreateFailingLayer(), - mainComponents = new MainComponentsContainer(), + mainComponents = new MainComponentsContainer + { + AlwaysPresent = true, + }, topRightElements = new FillFlowContainer { Anchor = Anchor.TopRight, @@ -108,6 +111,7 @@ namespace osu.Game.Screens.Play Margin = new MarginPadding(10), Spacing = new Vector2(10), AutoSizeAxes = Axes.Both, + AlwaysPresent = true, LayoutDuration = FADE_DURATION / 2, LayoutEasing = FADE_EASING, Direction = FillDirection.Vertical, From 0147a8ecee356c4454dda943ad6f9c5f5bc0bcf2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 14 Jun 2022 18:35:33 +0900 Subject: [PATCH 5/6] Add test coverage of HUD components still getting updated when hidden --- .../Visual/Gameplay/TestSceneHUDOverlay.cs | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs index 83c557ee51..949f0f667b 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs @@ -14,6 +14,7 @@ using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Scoring; using osu.Game.Screens.Play; +using osu.Game.Screens.Play.HUD.HitErrorMeters; using osu.Game.Skinning; using osu.Game.Tests.Gameplay; using osuTK.Input; @@ -145,6 +146,26 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("key counters still hidden", () => !keyCounterFlow.IsPresent); } + [Test] + public void TestHiddenHUDDoesntBlockComponentUpdates() + { + int updateCount = 0; + + AddStep("set hud to never show", () => localConfig.SetValue(OsuSetting.HUDVisibilityMode, HUDVisibilityMode.Never)); + + createNew(); + + AddUntilStep("wait for hud load", () => hudOverlay.IsLoaded); + AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType().Single().Alpha == 0); + + AddStep("bind on update", () => + { + hudOverlay.ChildrenOfType().First().OnUpdate += _ => updateCount++; + }); + + AddUntilStep("wait for updates", () => updateCount > 0); + } + [Test] public void TestHiddenHUDDoesntBlockSkinnableComponentsLoad() { @@ -153,7 +174,7 @@ namespace osu.Game.Tests.Visual.Gameplay createNew(); AddUntilStep("wait for hud load", () => hudOverlay.IsLoaded); - AddUntilStep("wait for components to be hidden", () => !hudOverlay.ChildrenOfType().Single().IsPresent); + AddUntilStep("wait for components to be hidden", () => hudOverlay.ChildrenOfType().Single().Alpha == 0); AddStep("reload components", () => hudOverlay.ChildrenOfType().Single().Reload()); AddUntilStep("skinnable components loaded", () => hudOverlay.ChildrenOfType().Single().ComponentsLoaded); From 6be42094581b72719011148b8018463ebe4057b8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 15 Jun 2022 01:04:43 +0900 Subject: [PATCH 6/6] Fix `AlwaysPresent` specification in wrong container --- osu.Game/Screens/Play/HUDOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/HUDOverlay.cs b/osu.Game/Screens/Play/HUDOverlay.cs index 6419532221..89329bfc65 100644 --- a/osu.Game/Screens/Play/HUDOverlay.cs +++ b/osu.Game/Screens/Play/HUDOverlay.cs @@ -94,6 +94,7 @@ namespace osu.Game.Screens.Play { Anchor = Anchor.TopRight, Origin = Anchor.TopRight, + AlwaysPresent = true, Margin = new MarginPadding(10), Spacing = new Vector2(10), AutoSizeAxes = Axes.Both, @@ -111,7 +112,6 @@ namespace osu.Game.Screens.Play Margin = new MarginPadding(10), Spacing = new Vector2(10), AutoSizeAxes = Axes.Both, - AlwaysPresent = true, LayoutDuration = FADE_DURATION / 2, LayoutEasing = FADE_EASING, Direction = FillDirection.Vertical,