Merge pull request #7712 from iiSaLMaN/fix-key-counter-visible-on-reshowing-hud

Fix key counter ignoring visibility setting on HUD visibility change

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

@@ -2,12 +2,16 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Framework.Testing;
 using osu.Game.Configuration;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Screens.Play;
+using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.Gameplay
 {
@@ -15,7 +19,9 @@ namespace osu.Game.Tests.Visual.Gameplay
     {
         private HUDOverlay hudOverlay;
 
-        private Drawable hideTarget => hudOverlay.KeyCounter; // best way of checking hideTargets without exposing.
+        // best way to check without exposing.
+        private Drawable hideTarget => hudOverlay.KeyCounter;
+        private FillFlowContainer<KeyCounter> keyCounterFlow => hudOverlay.KeyCounter.ChildrenOfType<FillFlowContainer<KeyCounter>>().First();
 
         [Resolved]
         private OsuConfigManager config { get; set; }
@@ -28,6 +34,7 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddAssert("showhud is set", () => hudOverlay.ShowHud.Value);
 
             AddAssert("hidetarget is visible", () => hideTarget.IsPresent);
+            AddAssert("key counter flow is visible", () => keyCounterFlow.IsPresent);
             AddAssert("pause button is visible", () => hudOverlay.HoldToQuit.IsPresent);
         }
 
@@ -50,6 +57,9 @@ namespace osu.Game.Tests.Visual.Gameplay
 
             AddUntilStep("hidetarget is hidden", () => !hideTarget.IsPresent);
             AddAssert("pause button is still visible", () => hudOverlay.HoldToQuit.IsPresent);
+
+            // Key counter flow container should not be affected by this, only the key counter display will be hidden as checked above.
+            AddAssert("key counter flow not affected", () => keyCounterFlow.IsPresent);
         }
 
         [Test]
@@ -68,12 +78,40 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddAssert("config unchanged", () => originalConfigValue == config.Get<bool>(OsuSetting.ShowInterface));
         }
 
+        [Test]
+        public void TestChangeHUDVisibilityOnHiddenKeyCounter()
+        {
+            bool keyCounterVisibleValue = false;
+
+            createNew();
+            AddStep("save keycounter visible value", () => keyCounterVisibleValue = config.Get<bool>(OsuSetting.KeyOverlay));
+
+            AddStep("set keycounter visible false", () =>
+            {
+                config.Set<bool>(OsuSetting.KeyOverlay, false);
+                hudOverlay.KeyCounter.AlwaysVisible.Value = false;
+            });
+
+            AddStep("set showhud false", () => hudOverlay.ShowHud.Value = false);
+            AddUntilStep("hidetarget is hidden", () => !hideTarget.IsPresent);
+            AddAssert("key counters hidden", () => !keyCounterFlow.IsPresent);
+
+            AddStep("set showhud true", () => hudOverlay.ShowHud.Value = true);
+            AddUntilStep("hidetarget is visible", () => hideTarget.IsPresent);
+            AddAssert("key counters still hidden", () => !keyCounterFlow.IsPresent);
+
+            AddStep("return value", () => config.Set<bool>(OsuSetting.KeyOverlay, keyCounterVisibleValue));
+        }
+
         private void createNew(Action<HUDOverlay> action = null)
         {
             AddStep("create overlay", () =>
             {
                 Child = hudOverlay = new HUDOverlay(null, null, null, Array.Empty<Mod>());
 
+                // Add any key just to display the key counter visually.
+                hudOverlay.KeyCounter.Add(new KeyCounterKeyboard(Key.Space));
+
                 action?.Invoke(hudOverlay);
             });
         }

osu.Game/Screens/Play/KeyCounterDisplay.cs

@@ -14,18 +14,32 @@ using osuTK.Graphics;
 
 namespace osu.Game.Screens.Play
 {
-    public class KeyCounterDisplay : FillFlowContainer<KeyCounter>
+    public class KeyCounterDisplay : Container<KeyCounter>
     {
         private const int duration = 100;
         private const double key_fade_time = 80;
 
-        public readonly Bindable<bool> Visible = new Bindable<bool>(true);
         private readonly Bindable<bool> configVisibility = new Bindable<bool>();
 
+        protected readonly FillFlowContainer<KeyCounter> KeyFlow;
+
+        protected override Container<KeyCounter> Content => KeyFlow;
+
+        /// <summary>
+        /// Whether the key counter should be visible regardless of the configuration value.
+        /// This is true by default, but can be changed.
+        /// </summary>
+        public readonly Bindable<bool> AlwaysVisible = new Bindable<bool>(true);
+
         public KeyCounterDisplay()
         {
-            Direction = FillDirection.Horizontal;
             AutoSizeAxes = Axes.Both;
+
+            InternalChild = KeyFlow = new FillFlowContainer<KeyCounter>
+            {
+                Direction = FillDirection.Horizontal,
+                AutoSizeAxes = Axes.Both,
+            };
         }
 
         public override void Add(KeyCounter key)
@@ -49,7 +63,7 @@ namespace osu.Game.Screens.Play
         {
             base.LoadComplete();
 
-            Visible.BindValueChanged(_ => updateVisibility());
+            AlwaysVisible.BindValueChanged(_ => updateVisibility());
             configVisibility.BindValueChanged(_ => updateVisibility(), true);
         }
 
@@ -100,7 +114,9 @@ namespace osu.Game.Screens.Play
             }
         }
 
-        private void updateVisibility() => this.FadeTo(Visible.Value || configVisibility.Value ? 1 : 0, duration);
+        private void updateVisibility() =>
+            // Isolate changing visibility of the key counters from fading this component.
+            KeyFlow.FadeTo(AlwaysVisible.Value || configVisibility.Value ? 1 : 0, duration);
 
         public override bool HandleNonPositionalInput => receptor == null;
         public override bool HandlePositionalInput => receptor == null;

osu.Game/Screens/Play/Player.cs

@@ -219,7 +219,7 @@ namespace osu.Game.Screens.Play
                         IsPaused = { BindTarget = GameplayClockContainer.IsPaused }
                     },
                     PlayerSettingsOverlay = { PlaybackSettings = { UserPlaybackRate = { BindTarget = GameplayClockContainer.UserPlaybackRate } } },
-                    KeyCounter = { Visible = { BindTarget = DrawableRuleset.HasReplayLoaded } },
+                    KeyCounter = { AlwaysVisible = { BindTarget = DrawableRuleset.HasReplayLoaded } },
                     RequestSeek = GameplayClockContainer.Seek,
                     Anchor = Anchor.Centre,
                     Origin = Anchor.Centre