From e21583ff1b642e404bd604d4dd045fd7d23cc2ae Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Jun 2023 16:35:59 +0900 Subject: [PATCH] Refactor `InputCountController` to not require being added to foreign body via `Attach` I've made the flow match `ClicksPerSecondCalculator` as close as possible. Hopefully this reads better. --- .../Visual/Gameplay/TestSceneKeyCounter.cs | 7 +++- osu.Game/Rulesets/UI/RulesetInputManager.cs | 21 +++++----- .../Screens/Play/HUD/InputCountController.cs | 17 +++----- .../Screens/Play/HUD/KeyCounterDisplay.cs | 39 +++++-------------- osu.Game/Screens/Play/HUDOverlay.cs | 4 +- 5 files changed, 29 insertions(+), 59 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneKeyCounter.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneKeyCounter.cs index a2c227a76a..3df0f18558 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneKeyCounter.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneKeyCounter.cs @@ -48,13 +48,16 @@ namespace osu.Game.Tests.Visual.Gameplay } }; - controller.AddRange(new InputTrigger[] + var inputTriggers = new InputTrigger[] { new KeyCounterKeyboardTrigger(Key.X), new KeyCounterKeyboardTrigger(Key.X), new KeyCounterMouseTrigger(MouseButton.Left), new KeyCounterMouseTrigger(MouseButton.Right), - }); + }; + + AddRange(inputTriggers); + controller.AddRange(inputTriggers); AddStep("Add random", () => { diff --git a/osu.Game/Rulesets/UI/RulesetInputManager.cs b/osu.Game/Rulesets/UI/RulesetInputManager.cs index 08e180536f..9be210f4b2 100644 --- a/osu.Game/Rulesets/UI/RulesetInputManager.cs +++ b/osu.Game/Rulesets/UI/RulesetInputManager.cs @@ -162,25 +162,22 @@ namespace osu.Game.Rulesets.UI public void Attach(InputCountController inputCountController) { - KeyBindingContainer.Add(inputCountController); + var triggers = KeyBindingContainer.DefaultKeyBindings + .Select(b => b.GetAction()) + .Distinct() + .OrderBy(action => action) + .Select(action => new KeyCounterActionTrigger(action)) + .ToArray(); - inputCountController.AddRange(KeyBindingContainer.DefaultKeyBindings - .Select(b => b.GetAction()) - .Distinct() - .OrderBy(action => action) - .Select(action => new KeyCounterActionTrigger(action))); + KeyBindingContainer.AddRange(triggers); + inputCountController.AddRange(triggers); } #endregion #region Keys per second Counter Attachment - public void Attach(ClicksPerSecondCalculator calculator) - { - var listener = new ActionListener(calculator); - - KeyBindingContainer.Add(listener); - } + public void Attach(ClicksPerSecondCalculator calculator) => KeyBindingContainer.Add(new ActionListener(calculator)); private partial class ActionListener : Component, IKeyBindingHandler { diff --git a/osu.Game/Screens/Play/HUD/InputCountController.cs b/osu.Game/Screens/Play/HUD/InputCountController.cs index 47163eeb6d..4827f2315f 100644 --- a/osu.Game/Screens/Play/HUD/InputCountController.cs +++ b/osu.Game/Screens/Play/HUD/InputCountController.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Collections.Generic; using osu.Framework.Bindables; using osu.Framework.Extensions.IEnumerableExtensions; @@ -17,26 +16,20 @@ namespace osu.Game.Screens.Play.HUD { public readonly Bindable IsCounting = new BindableBool(true); - public event Action? OnNewTrigger; + private readonly BindableList triggers = new BindableList(); - private readonly Container triggers; + public IBindableList Triggers => triggers; - public IReadOnlyList Triggers => triggers; - - public InputCountController() - { - InternalChild = triggers = new Container(); - } + public void AddRange(IEnumerable triggers) => triggers.ForEach(Add); public void Add(InputTrigger trigger) { + // Note that these triggers are not added to the hierarchy here. It is presumed they are added externally at a + // more correct location (ie. inside a RulesetInputManager). triggers.Add(trigger); trigger.IsCounting.BindTo(IsCounting); - OnNewTrigger?.Invoke(trigger); } - public void AddRange(IEnumerable inputTriggers) => inputTriggers.ForEach(Add); - public override bool HandleNonPositionalInput => true; public override bool HandlePositionalInput => true; } diff --git a/osu.Game/Screens/Play/HUD/KeyCounterDisplay.cs b/osu.Game/Screens/Play/HUD/KeyCounterDisplay.cs index e222099c63..e7e866932e 100644 --- a/osu.Game/Screens/Play/HUD/KeyCounterDisplay.cs +++ b/osu.Game/Screens/Play/HUD/KeyCounterDisplay.cs @@ -1,11 +1,9 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Collections.Generic; +using System.Collections.Specialized; using osu.Framework.Allocation; using osu.Framework.Bindables; -using osu.Framework.Extensions.IEnumerableExtensions; -using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics.Containers; using osu.Game.Configuration; using osu.Game.Rulesets.UI; @@ -24,35 +22,17 @@ namespace osu.Game.Screens.Play.HUD /// public Bindable AlwaysVisible { get; } = new Bindable(true); - /// - /// The s contained in this . - /// - public IEnumerable Counters => KeyFlow; - protected abstract FillFlowContainer KeyFlow { get; } protected readonly Bindable ConfigVisibility = new Bindable(); + private readonly IBindableList triggers = new BindableList(); + [Resolved] private InputCountController controller { get; set; } = null!; protected abstract void UpdateVisibility(); - /// - /// Add a to this display. - /// - public void Add(InputTrigger trigger) - { - var keyCounter = CreateCounter(trigger); - - KeyFlow.Add(keyCounter); - } - - /// - /// Add a range of to this display. - /// - public void AddRange(IEnumerable triggers) => triggers.ForEach(Add); - protected abstract KeyCounter CreateCounter(InputTrigger trigger); [BackgroundDependencyLoader] @@ -68,19 +48,18 @@ namespace osu.Game.Screens.Play.HUD { base.LoadComplete(); - controller.OnNewTrigger += Add; - AddRange(controller.Triggers); + triggers.BindTo(controller.Triggers); + triggers.BindCollectionChanged(triggersChanged, true); AlwaysVisible.BindValueChanged(_ => UpdateVisibility()); ConfigVisibility.BindValueChanged(_ => UpdateVisibility(), true); } - protected override void Dispose(bool isDisposing) + private void triggersChanged(object? sender, NotifyCollectionChangedEventArgs e) { - base.Dispose(isDisposing); - - if (controller.IsNotNull()) - controller.OnNewTrigger -= Add; + KeyFlow.Clear(); + foreach (var trigger in controller.Triggers) + KeyFlow.Add(CreateCounter(trigger)); } public bool UsesFixedAnchor { get; set; } diff --git a/osu.Game/Screens/Play/HUDOverlay.cs b/osu.Game/Screens/Play/HUDOverlay.cs index ae9c6a7d87..278c7b9de2 100644 --- a/osu.Game/Screens/Play/HUDOverlay.cs +++ b/osu.Game/Screens/Play/HUDOverlay.cs @@ -111,9 +111,6 @@ namespace osu.Game.Screens.Play RelativeSizeAxes = Axes.Both; - // intentionally not added to hierarchy here as it will be attached via `BindDrawableRuleset()`. - InputCountController = new InputCountController(); - Children = new[] { CreateFailingLayer(), @@ -161,6 +158,7 @@ namespace osu.Game.Screens.Play Spacing = new Vector2(5) }, clicksPerSecondCalculator = new ClicksPerSecondCalculator(), + InputCountController = new InputCountController(), }; hideTargets = new List { mainComponents, rulesetComponents, topRightElements };