Merge pull request #23309 from peppy/key-counters-fix

Fix key counters counting during break time

osu.Game.Tests/Visual/Gameplay/TestSceneHUDOverlay.cs

@@ -45,7 +45,7 @@ namespace osu.Game.Tests.Visual.Gameplay
 
         // best way to check without exposing.
         private Drawable hideTarget => hudOverlay.KeyCounter;
-        private Drawable keyCounterFlow => hudOverlay.KeyCounter.ChildrenOfType<FillFlowContainer<DefaultKeyCounter>>().Single();
+        private Drawable keyCounterFlow => hudOverlay.KeyCounter.ChildrenOfType<FillFlowContainer<KeyCounter>>().Single();
 
         [BackgroundDependencyLoader]
         private void load()

osu.Game.Tests/Visual/Gameplay/TestSceneKeyCounter.cs

@@ -19,21 +19,21 @@ namespace osu.Game.Tests.Visual.Gameplay
     {
         public TestSceneKeyCounter()
         {
-            KeyCounterDisplay kc = new DefaultKeyCounterDisplay
+            KeyCounterDisplay defaultDisplay = new DefaultKeyCounterDisplay
             {
                 Origin = Anchor.Centre,
                 Anchor = Anchor.Centre,
                 Position = new Vector2(0, 72.7f)
             };
 
-            KeyCounterDisplay argonKc = new ArgonKeyCounterDisplay
+            KeyCounterDisplay argonDisplay = new ArgonKeyCounterDisplay
             {
                 Origin = Anchor.Centre,
                 Anchor = Anchor.Centre,
                 Position = new Vector2(0, -72.7f)
             };
 
-            kc.AddRange(new InputTrigger[]
+            defaultDisplay.AddRange(new InputTrigger[]
             {
                 new KeyCounterKeyboardTrigger(Key.X),
                 new KeyCounterKeyboardTrigger(Key.X),
@@ -41,7 +41,7 @@ namespace osu.Game.Tests.Visual.Gameplay
                 new KeyCounterMouseTrigger(MouseButton.Right),
             });
 
-            argonKc.AddRange(new InputTrigger[]
+            argonDisplay.AddRange(new InputTrigger[]
             {
                 new KeyCounterKeyboardTrigger(Key.X),
                 new KeyCounterKeyboardTrigger(Key.X),
@@ -49,32 +49,33 @@ namespace osu.Game.Tests.Visual.Gameplay
                 new KeyCounterMouseTrigger(MouseButton.Right),
             });
 
-            var testCounter = (DefaultKeyCounter)kc.Counters.First();
+            var testCounter = (DefaultKeyCounter)defaultDisplay.Counters.First();
 
             AddStep("Add random", () =>
             {
                 Key key = (Key)((int)Key.A + RNG.Next(26));
-                kc.Add(new KeyCounterKeyboardTrigger(key));
+                defaultDisplay.Add(new KeyCounterKeyboardTrigger(key));
-                argonKc.Add(new KeyCounterKeyboardTrigger(key));
+                argonDisplay.Add(new KeyCounterKeyboardTrigger(key));
             });
 
-            Key testKey = ((KeyCounterKeyboardTrigger)kc.Counters.First().Trigger).Key;
+            Key testKey = ((KeyCounterKeyboardTrigger)defaultDisplay.Counters.First().Trigger).Key;
-
-            void addPressKeyStep()
-            {
-                AddStep($"Press {testKey} key", () => InputManager.Key(testKey));
-            }
 
             addPressKeyStep();
             AddAssert($"Check {testKey} counter after keypress", () => testCounter.CountPresses.Value == 1);
             addPressKeyStep();
             AddAssert($"Check {testKey} counter after keypress", () => testCounter.CountPresses.Value == 2);
-            AddStep("Disable counting", () => testCounter.IsCounting.Value = false);
+            AddStep("Disable counting", () =>
+            {
+                argonDisplay.IsCounting.Value = false;
+                defaultDisplay.IsCounting.Value = false;
+            });
             addPressKeyStep();
             AddAssert($"Check {testKey} count has not changed", () => testCounter.CountPresses.Value == 2);
 
-            Add(kc);
+            Add(defaultDisplay);
-            Add(argonKc);
+            Add(argonDisplay);
+
+            void addPressKeyStep() => AddStep($"Press {testKey} key", () => InputManager.Key(testKey));
         }
     }
 }

osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableHUDOverlay.cs

@@ -44,7 +44,7 @@ namespace osu.Game.Tests.Visual.Gameplay
 
         // best way to check without exposing.
         private Drawable hideTarget => hudOverlay.KeyCounter;
-        private Drawable keyCounterFlow => hudOverlay.KeyCounter.ChildrenOfType<FillFlowContainer<DefaultKeyCounter>>().Single();
+        private Drawable keyCounterFlow => hudOverlay.KeyCounter.ChildrenOfType<FillFlowContainer<KeyCounter>>().Single();
 
         [Test]
         public void TestComboCounterIncrementing()

osu.Game/Screens/Play/ArgonKeyCounterDisplay.cs

@@ -1,7 +1,6 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using System.Collections.Generic;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Screens.Play.HUD;
@@ -13,13 +12,11 @@ namespace osu.Game.Screens.Play
     {
         private const int duration = 100;
 
-        private readonly FillFlowContainer<ArgonKeyCounter> keyFlow;
+        protected override FillFlowContainer<KeyCounter> KeyFlow { get; }
-
-        public override IEnumerable<KeyCounter> Counters => keyFlow;
 
         public ArgonKeyCounterDisplay()
         {
-            InternalChild = keyFlow = new FillFlowContainer<ArgonKeyCounter>
+            InternalChild = KeyFlow = new FillFlowContainer<KeyCounter>
             {
                 Direction = FillDirection.Horizontal,
                 AutoSizeAxes = Axes.Both,
@@ -32,13 +29,12 @@ namespace osu.Game.Screens.Play
         {
             base.Update();
 
-            Size = keyFlow.Size;
+            Size = KeyFlow.Size;
         }
 
-        public override void Add(InputTrigger trigger) =>
+        protected override KeyCounter CreateCounter(InputTrigger trigger) => new ArgonKeyCounter(trigger);
-            keyFlow.Add(new ArgonKeyCounter(trigger));
 
         protected override void UpdateVisibility()
-            => keyFlow.FadeTo(AlwaysVisible.Value || ConfigVisibility.Value ? 1 : 0, duration);
+            => KeyFlow.FadeTo(AlwaysVisible.Value || ConfigVisibility.Value ? 1 : 0, duration);
     }
 }

osu.Game/Screens/Play/HUD/DefaultKeyCounterDisplay.cs

@@ -1,7 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using System.Collections.Generic;
+using System.Linq;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osuTK.Graphics;
@@ -13,13 +13,11 @@ namespace osu.Game.Screens.Play.HUD
         private const int duration = 100;
         private const double key_fade_time = 80;
 
-        private readonly FillFlowContainer<DefaultKeyCounter> keyFlow;
+        protected override FillFlowContainer<KeyCounter> KeyFlow { get; }
-
-        public override IEnumerable<KeyCounter> Counters => keyFlow;
 
         public DefaultKeyCounterDisplay()
         {
-            InternalChild = keyFlow = new FillFlowContainer<DefaultKeyCounter>
+            InternalChild = KeyFlow = new FillFlowContainer<KeyCounter>
             {
                 Direction = FillDirection.Horizontal,
                 AutoSizeAxes = Axes.Both,
@@ -33,20 +31,19 @@ namespace osu.Game.Screens.Play.HUD
 
             // Don't use autosize as it will shrink to zero when KeyFlow is hidden.
             // In turn this can cause the display to be masked off screen and never become visible again.
-            Size = keyFlow.Size;
+            Size = KeyFlow.Size;
         }
 
-        public override void Add(InputTrigger trigger) =>
+        protected override KeyCounter CreateCounter(InputTrigger trigger) => new DefaultKeyCounter(trigger)
-            keyFlow.Add(new DefaultKeyCounter(trigger)
         {
             FadeTime = key_fade_time,
             KeyDownTextColor = KeyDownTextColor,
             KeyUpTextColor = KeyUpTextColor,
-            });
+        };
 
         protected override void UpdateVisibility() =>
             // Isolate changing visibility of the key counters from fading this component.
-            keyFlow.FadeTo(AlwaysVisible.Value || ConfigVisibility.Value ? 1 : 0, duration);
+            KeyFlow.FadeTo(AlwaysVisible.Value || ConfigVisibility.Value ? 1 : 0, duration);
 
         private Color4 keyDownTextColor = Color4.DarkGray;
 
@@ -58,7 +55,7 @@ namespace osu.Game.Screens.Play.HUD
                 if (value != keyDownTextColor)
                 {
                     keyDownTextColor = value;
-                    foreach (var child in keyFlow)
+                    foreach (var child in KeyFlow.Cast<DefaultKeyCounter>())
                         child.KeyDownTextColor = value;
                 }
             }
@@ -74,7 +71,7 @@ namespace osu.Game.Screens.Play.HUD
                 if (value != keyUpTextColor)
                 {
                     keyUpTextColor = value;
-                    foreach (var child in keyFlow)
+                    foreach (var child in KeyFlow.Cast<DefaultKeyCounter>())
                         child.KeyUpTextColor = value;
                 }
             }

osu.Game/Screens/Play/HUD/KeyCounterDisplay.cs

@@ -29,7 +29,9 @@ namespace osu.Game.Screens.Play.HUD
         /// <summary>
         /// The <see cref="KeyCounter"/>s contained in this <see cref="KeyCounterDisplay"/>.
         /// </summary>
-        public abstract IEnumerable<KeyCounter> Counters { get; }
+        public IEnumerable<KeyCounter> Counters => KeyFlow;
+
+        protected abstract FillFlowContainer<KeyCounter> KeyFlow { get; }
 
         /// <summary>
         /// Whether the actions reported by all <see cref="InputTrigger"/>s within this <see cref="KeyCounterDisplay"/> should be counted.
@@ -53,13 +55,22 @@ namespace osu.Game.Screens.Play.HUD
         /// <summary>
         /// Add a <see cref="InputTrigger"/> to this display.
         /// </summary>
-        public abstract void Add(InputTrigger trigger);
+        public void Add(InputTrigger trigger)
+        {
+            var keyCounter = CreateCounter(trigger);
+
+            KeyFlow.Add(keyCounter);
+
+            IsCounting.BindTo(keyCounter.IsCounting);
+        }
 
         /// <summary>
         /// Add a range of <see cref="InputTrigger"/> to this display.
         /// </summary>
         public void AddRange(IEnumerable<InputTrigger> triggers) => triggers.ForEach(Add);
 
+        protected abstract KeyCounter CreateCounter(InputTrigger trigger);
+
         [BackgroundDependencyLoader]
         private void load(OsuConfigManager config)
         {