From 1cb10c2a2243bde4f0fcc85a5d6de338a34f28bd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 7 May 2021 17:27:34 +0900 Subject: [PATCH 1/4] Remove unnecessary binding logic from `HUDOverlay` --- .../Visual/Gameplay/TestSceneHUDOverlay.cs | 2 +- .../TestSceneSkinEditorMultipleSkins.cs | 2 +- .../Gameplay/TestSceneSkinnableHUDOverlay.cs | 2 +- osu.Game/Screens/Play/HUD/HitErrorDisplay.cs | 6 +++--- osu.Game/Screens/Play/HUDOverlay.cs | 21 ++++--------------- osu.Game/Screens/Play/Player.cs | 2 +- 6 files changed, 11 insertions(+), 24 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs index 861d4a4f7f..b7e92a79a0 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs @@ -146,7 +146,7 @@ namespace osu.Game.Tests.Visual.Gameplay { AddStep("create overlay", () => { - hudOverlay = new HUDOverlay(scoreProcessor, null, Array.Empty()); + hudOverlay = new HUDOverlay(null, Array.Empty()); // Add any key just to display the key counter visually. hudOverlay.KeyCounter.Add(new KeyCounterKeyboard(Key.Space)); diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs index ac5599cc27..c7c93b8892 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinEditorMultipleSkins.cs @@ -37,7 +37,7 @@ namespace osu.Game.Tests.Visual.Gameplay var drawableRuleset = ruleset.CreateDrawableRulesetWith(beatmap); - var hudOverlay = new HUDOverlay(scoreProcessor, drawableRuleset, Array.Empty()) + var hudOverlay = new HUDOverlay(drawableRuleset, Array.Empty()) { Anchor = Anchor.Centre, Origin = Anchor.Centre, diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs index 7a960a09fc..c92e9dcfd5 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs @@ -79,7 +79,7 @@ namespace osu.Game.Tests.Visual.Gameplay { SetContents(() => { - hudOverlay = new HUDOverlay(scoreProcessor, null, Array.Empty()); + hudOverlay = new HUDOverlay(null, Array.Empty()); // Add any key just to display the key counter visually. hudOverlay.KeyCounter.Add(new KeyCounterKeyboard(Key.Space)); diff --git a/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs b/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs index 37d10a5320..998a9fb7e7 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs @@ -22,11 +22,11 @@ namespace osu.Game.Screens.Play.HUD private readonly HitWindows hitWindows; - private readonly ScoreProcessor processor; + [Resolved] + private ScoreProcessor processor { get; set; } - public HitErrorDisplay(ScoreProcessor processor, HitWindows hitWindows) + public HitErrorDisplay(HitWindows hitWindows) { - this.processor = processor; this.hitWindows = hitWindows; RelativeSizeAxes = Axes.Both; diff --git a/osu.Game/Screens/Play/HUDOverlay.cs b/osu.Game/Screens/Play/HUDOverlay.cs index fd1aea016b..22812b779f 100644 --- a/osu.Game/Screens/Play/HUDOverlay.cs +++ b/osu.Game/Screens/Play/HUDOverlay.cs @@ -14,7 +14,6 @@ using osu.Game.Input.Bindings; using osu.Game.Overlays; using osu.Game.Overlays.Notifications; using osu.Game.Rulesets.Mods; -using osu.Game.Rulesets.Scoring; using osu.Game.Rulesets.UI; using osu.Game.Screens.Play.HUD; using osuTK; @@ -39,14 +38,11 @@ namespace osu.Game.Screens.Play public readonly SkinnableHealthDisplay HealthDisplay; public readonly SongProgress Progress; public readonly ModDisplay ModDisplay; - public readonly HitErrorDisplay HitErrorDisplay; public readonly HoldForMenuButton HoldToQuit; public readonly PlayerSettingsOverlay PlayerSettingsOverlay; - public readonly FailingLayer FailingLayer; public Bindable ShowHealthbar = new Bindable(true); - private readonly ScoreProcessor scoreProcessor; private readonly DrawableRuleset drawableRuleset; private readonly IReadOnlyList mods; @@ -74,9 +70,8 @@ namespace osu.Game.Screens.Play private IEnumerable hideTargets => new Drawable[] { visibilityContainer, KeyCounter, topRightElements }; - public HUDOverlay(ScoreProcessor scoreProcessor, DrawableRuleset drawableRuleset, IReadOnlyList mods) + public HUDOverlay(DrawableRuleset drawableRuleset, IReadOnlyList mods) { - this.scoreProcessor = scoreProcessor; this.drawableRuleset = drawableRuleset; this.mods = mods; @@ -84,7 +79,7 @@ namespace osu.Game.Screens.Play Children = new Drawable[] { - FailingLayer = CreateFailingLayer(), + CreateFailingLayer(), visibilityContainer = new Container { RelativeSizeAxes = Axes.Both, @@ -104,7 +99,7 @@ namespace osu.Game.Screens.Play AccuracyCounter = CreateAccuracyCounter(), ScoreCounter = CreateScoreCounter(), CreateComboCounter(), - HitErrorDisplay = CreateHitErrorDisplayOverlay(), + CreateHitErrorDisplayOverlay(), } }, }, @@ -156,9 +151,6 @@ namespace osu.Game.Screens.Play [BackgroundDependencyLoader(true)] private void load(OsuConfigManager config, NotificationOverlay notificationOverlay) { - if (scoreProcessor != null) - BindScoreProcessor(scoreProcessor); - if (drawableRuleset != null) { BindDrawableRuleset(drawableRuleset); @@ -309,15 +301,10 @@ namespace osu.Game.Screens.Play AutoSizeAxes = Axes.Both, }; - protected virtual HitErrorDisplay CreateHitErrorDisplayOverlay() => new HitErrorDisplay(scoreProcessor, drawableRuleset?.FirstAvailableHitWindows); + protected virtual HitErrorDisplay CreateHitErrorDisplayOverlay() => new HitErrorDisplay(drawableRuleset?.FirstAvailableHitWindows); protected virtual PlayerSettingsOverlay CreatePlayerSettingsOverlay() => new PlayerSettingsOverlay(); - protected virtual void BindScoreProcessor(ScoreProcessor processor) - { - AccuracyCounter?.Current.BindTo(processor.Accuracy); - } - public bool OnPressed(GlobalAction action) { switch (action) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 1538899281..116cf3cc99 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -345,7 +345,7 @@ namespace osu.Game.Screens.Play // display the cursor above some HUD elements. DrawableRuleset.Cursor?.CreateProxy() ?? new Container(), DrawableRuleset.ResumeOverlay?.CreateProxy() ?? new Container(), - HUDOverlay = new HUDOverlay(ScoreProcessor, DrawableRuleset, Mods.Value) + HUDOverlay = new HUDOverlay(DrawableRuleset, Mods.Value) { HoldToQuit = { From a1aeac567710b95f27b07daede58b9a1ad56ab45 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 7 May 2021 18:11:59 +0900 Subject: [PATCH 2/4] Remove remaining cruft from `SkinnableAccuracyCounter` --- osu.Game/Screens/Play/HUD/SkinnableAccuracyCounter.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/SkinnableAccuracyCounter.cs b/osu.Game/Screens/Play/HUD/SkinnableAccuracyCounter.cs index 17c2493d2d..fcb8fca35d 100644 --- a/osu.Game/Screens/Play/HUD/SkinnableAccuracyCounter.cs +++ b/osu.Game/Screens/Play/HUD/SkinnableAccuracyCounter.cs @@ -1,15 +1,12 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using osu.Framework.Bindables; using osu.Game.Skinning; namespace osu.Game.Screens.Play.HUD { public class SkinnableAccuracyCounter : SkinnableDrawable { - public Bindable Current { get; } = new Bindable(); - public SkinnableAccuracyCounter() : base(new HUDSkinComponent(HUDSkinComponents.AccuracyCounter), _ => new DefaultAccuracyCounter()) { From 97e72849af46241de4acbe4e955c5e6a72db9434 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 May 2021 15:19:27 +0900 Subject: [PATCH 3/4] Fix regressed `HitErrorDisplay` behaviour (and localise binding to meter implementations) --- .../Visual/Gameplay/TestSceneHitErrorMeter.cs | 39 +++++++------------ osu.Game/Screens/Play/HUD/HitErrorDisplay.cs | 24 ------------ .../HUD/HitErrorMeters/BarHitErrorMeter.cs | 2 +- .../HUD/HitErrorMeters/ColourHitErrorMeter.cs | 2 +- .../Play/HUD/HitErrorMeters/HitErrorMeter.cs | 29 +++++++++++++- 5 files changed, 45 insertions(+), 51 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs index 1021ac3760..6cefd01aab 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHitErrorMeter.cs @@ -2,15 +2,16 @@ // See the LICENCE file in the repository root for full licence text. using NUnit.Framework; -using osu.Game.Rulesets.Judgements; -using osu.Framework.Utils; +using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Threading; +using osu.Framework.Utils; using osu.Game.Graphics.Sprites; using osu.Game.Rulesets.Catch.Scoring; +using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Mania.Scoring; -using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu.Objects; using osu.Game.Rulesets.Osu.Scoring; using osu.Game.Rulesets.Scoring; using osu.Game.Rulesets.Taiko.Scoring; @@ -20,14 +21,11 @@ namespace osu.Game.Tests.Visual.Gameplay { public class TestSceneHitErrorMeter : OsuTestScene { - private BarHitErrorMeter barMeter; - private BarHitErrorMeter barMeter2; - private BarHitErrorMeter barMeter3; - private ColourHitErrorMeter colourMeter; - private ColourHitErrorMeter colourMeter2; - private ColourHitErrorMeter colourMeter3; private HitWindows hitWindows; + [Cached] + private ScoreProcessor scoreProcessor = new ScoreProcessor(); + public TestSceneHitErrorMeter() { recreateDisplay(new OsuHitWindows(), 5); @@ -105,40 +103,40 @@ namespace osu.Game.Tests.Visual.Gameplay } }); - Add(barMeter = new BarHitErrorMeter(hitWindows, true) + Add(new BarHitErrorMeter(hitWindows, true) { Anchor = Anchor.CentreRight, Origin = Anchor.CentreRight, }); - Add(barMeter2 = new BarHitErrorMeter(hitWindows, false) + Add(new BarHitErrorMeter(hitWindows, false) { Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, }); - Add(barMeter3 = new BarHitErrorMeter(hitWindows, true) + Add(new BarHitErrorMeter(hitWindows, true) { Anchor = Anchor.BottomCentre, Origin = Anchor.CentreLeft, Rotation = 270, }); - Add(colourMeter = new ColourHitErrorMeter(hitWindows) + Add(new ColourHitErrorMeter(hitWindows) { Anchor = Anchor.CentreRight, Origin = Anchor.CentreRight, Margin = new MarginPadding { Right = 50 } }); - Add(colourMeter2 = new ColourHitErrorMeter(hitWindows) + Add(new ColourHitErrorMeter(hitWindows) { Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, Margin = new MarginPadding { Left = 50 } }); - Add(colourMeter3 = new ColourHitErrorMeter(hitWindows) + Add(new ColourHitErrorMeter(hitWindows) { Anchor = Anchor.BottomCentre, Origin = Anchor.CentreLeft, @@ -149,18 +147,11 @@ namespace osu.Game.Tests.Visual.Gameplay private void newJudgement(double offset = 0) { - var judgement = new JudgementResult(new HitObject(), new Judgement()) + scoreProcessor.ApplyResult(new JudgementResult(new HitCircle { HitWindows = hitWindows }, new Judgement()) { TimeOffset = offset == 0 ? RNG.Next(-150, 150) : offset, Type = HitResult.Perfect, - }; - - barMeter.OnNewJudgement(judgement); - barMeter2.OnNewJudgement(judgement); - barMeter3.OnNewJudgement(judgement); - colourMeter.OnNewJudgement(judgement); - colourMeter2.OnNewJudgement(judgement); - colourMeter3.OnNewJudgement(judgement); + }); } } } diff --git a/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs b/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs index 998a9fb7e7..a24d9c10cb 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorDisplay.cs @@ -7,7 +7,6 @@ using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Configuration; -using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Scoring; using osu.Game.Screens.Play.HUD.HitErrorMeters; @@ -22,17 +21,11 @@ namespace osu.Game.Screens.Play.HUD private readonly HitWindows hitWindows; - [Resolved] - private ScoreProcessor processor { get; set; } - public HitErrorDisplay(HitWindows hitWindows) { this.hitWindows = hitWindows; RelativeSizeAxes = Axes.Both; - - if (processor != null) - processor.NewJudgement += onNewJudgement; } [BackgroundDependencyLoader] @@ -47,15 +40,6 @@ namespace osu.Game.Screens.Play.HUD type.BindValueChanged(typeChanged, true); } - private void onNewJudgement(JudgementResult result) - { - if (result.HitObject.HitWindows.WindowFor(HitResult.Miss) == 0) - return; - - foreach (var c in Children) - c.OnNewJudgement(result); - } - private void typeChanged(ValueChangedEvent type) { Children.ForEach(c => c.FadeOut(fade_duration, Easing.OutQuint)); @@ -139,13 +123,5 @@ namespace osu.Game.Screens.Play.HUD Add(display); display.FadeInFromZero(fade_duration, Easing.OutQuint); } - - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - - if (processor != null) - processor.NewJudgement -= onNewJudgement; - } } } diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs index 3b24c8cc9e..c5a21ade03 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/BarHitErrorMeter.cs @@ -214,7 +214,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters private const int max_concurrent_judgements = 50; - public override void OnNewJudgement(JudgementResult judgement) + protected override void OnNewJudgement(JudgementResult judgement) { if (!judgement.IsHit) return; diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs index 657235bfd4..465439cf19 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs @@ -26,7 +26,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters InternalChild = judgementsFlow = new JudgementFlow(); } - public override void OnNewJudgement(JudgementResult judgement) => judgementsFlow.Push(GetColourForHitResult(HitWindows.ResultFor(judgement.TimeOffset))); + protected override void OnNewJudgement(JudgementResult judgement) => judgementsFlow.Push(GetColourForHitResult(HitWindows.ResultFor(judgement.TimeOffset))); private class JudgementFlow : FillFlowContainer { diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index b3edfdedec..8b53a9676d 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -14,6 +14,9 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { protected readonly HitWindows HitWindows; + [Resolved] + private ScoreProcessor processor { get; set; } + [Resolved] private OsuColour colours { get; set; } @@ -22,7 +25,23 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters HitWindows = hitWindows; } - public abstract void OnNewJudgement(JudgementResult judgement); + protected override void LoadComplete() + { + base.LoadComplete(); + + if (processor != null) + processor.NewJudgement += onNewJudgement; + } + + private void onNewJudgement(JudgementResult result) + { + if (result.HitObject.HitWindows?.WindowFor(HitResult.Miss) == 0) + return; + + OnNewJudgement(result); + } + + protected abstract void OnNewJudgement(JudgementResult judgement); protected Color4 GetColourForHitResult(HitResult result) { @@ -47,5 +66,13 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters return colours.BlueLight; } } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + if (processor != null) + processor.NewJudgement -= onNewJudgement; + } } } From fa872858b592278b7672c16fbd840825d8ac7a24 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 10 May 2021 16:40:06 +0900 Subject: [PATCH 4/4] Remove unnecessary check --- osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs index 8b53a9676d..37e9ea43c5 100644 --- a/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs +++ b/osu.Game/Screens/Play/HUD/HitErrorMeters/HitErrorMeter.cs @@ -29,8 +29,7 @@ namespace osu.Game.Screens.Play.HUD.HitErrorMeters { base.LoadComplete(); - if (processor != null) - processor.NewJudgement += onNewJudgement; + processor.NewJudgement += onNewJudgement; } private void onNewJudgement(JudgementResult result)