Fix OsuHoverContainer not updating visual hover state when enabled mid-hover (#4913)

Fix OsuHoverContainer not updating visual hover state when enabled mid-hover

Co-authored-by: Dean Herbert <pe@ppy.sh>

osu.Game.Tests/Visual/UserInterface/TestSceneOsuHoverContainer.cs

@@ -0,0 +1,194 @@
+// 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 NUnit.Framework;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Colour;
+using osu.Framework.Graphics.Shapes;
+using osu.Game.Graphics.Containers;
+using osuTK;
+using osuTK.Graphics;
+
+namespace osu.Game.Tests.Visual.UserInterface
+{
+    [TestFixture]
+    public class TestSceneOsuHoverContainer : ManualInputManagerTestScene
+    {
+        private OsuHoverTestContainer hoverContainer;
+        private Box colourContainer;
+
+        [SetUp]
+        public void SetUp() => Schedule(() =>
+        {
+            Child = hoverContainer = new OsuHoverTestContainer
+            {
+                Enabled = { Value = true },
+                Anchor = Anchor.Centre,
+                Origin = Anchor.Centre,
+                Size = new Vector2(100),
+                Child = colourContainer = new Box
+                {
+                    RelativeSizeAxes = Axes.Both,
+                },
+            };
+
+            doMoveOut();
+        });
+
+        [Description("Checks IsHovered property value on a container when it is hovered/unhovered.")]
+        [TestCase(true, TestName = "Enabled_Check_IsHovered")]
+        [TestCase(false, TestName = "Disabled_Check_IsHovered")]
+        public void TestIsHoveredHasProperValue(bool isEnabled)
+        {
+            setContainerEnabledTo(isEnabled);
+
+            checkNotHovered();
+
+            moveToText();
+            checkHovered();
+
+            moveOut();
+            checkNotHovered();
+
+            moveToText();
+            checkHovered();
+
+            moveOut();
+            checkNotHovered();
+        }
+
+        [Test]
+        [Description("Checks colour fading on an enabled container when it is hovered/unhovered.")]
+        public void TestTransitionWhileEnabled()
+        {
+            enableContainer();
+
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveToText();
+            waitUntilColourIs(OsuHoverTestContainer.HOVER_COLOUR);
+
+            moveOut();
+            waitUntilColourIs(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveToText();
+            waitUntilColourIs(OsuHoverTestContainer.HOVER_COLOUR);
+
+            moveOut();
+            waitUntilColourIs(OsuHoverTestContainer.IDLE_COLOUR);
+        }
+
+        [Test]
+        [Description("Checks colour fading on a disabled container when it is hovered/unhovered.")]
+        public void TestNoTransitionWhileDisabled()
+        {
+            disableContainer();
+
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveToText();
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveOut();
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveToText();
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveOut();
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+        }
+
+        [Test]
+        [Description("Checks that when a disabled & hovered container gets enabled, colour fading happens")]
+        public void TestBecomesEnabledTransition()
+        {
+            disableContainer();
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveToText();
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            enableContainer();
+            waitUntilColourIs(OsuHoverTestContainer.HOVER_COLOUR);
+        }
+
+        [Test]
+        [Description("Checks that when an enabled & hovered container gets disabled, colour fading happens")]
+        public void TestBecomesDisabledTransition()
+        {
+            enableContainer();
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveToText();
+            waitUntilColourIs(OsuHoverTestContainer.HOVER_COLOUR);
+
+            disableContainer();
+            waitUntilColourIs(OsuHoverTestContainer.IDLE_COLOUR);
+        }
+
+        [Test]
+        [Description("Checks that when a hovered container gets enabled and disabled multiple times, colour fading happens")]
+        public void TestDisabledChangesMultipleTimes()
+        {
+            enableContainer();
+            checkColour(OsuHoverTestContainer.IDLE_COLOUR);
+
+            moveToText();
+            waitUntilColourIs(OsuHoverTestContainer.HOVER_COLOUR);
+
+            disableContainer();
+            waitUntilColourIs(OsuHoverTestContainer.IDLE_COLOUR);
+
+            enableContainer();
+            waitUntilColourIs(OsuHoverTestContainer.HOVER_COLOUR);
+
+            disableContainer();
+            waitUntilColourIs(OsuHoverTestContainer.IDLE_COLOUR);
+        }
+
+        private void enableContainer() => setContainerEnabledTo(true);
+
+        private void disableContainer() => setContainerEnabledTo(false);
+
+        private void setContainerEnabledTo(bool newValue)
+        {
+            string word = newValue ? "Enable" : "Disable";
+            AddStep($"{word} container", () => hoverContainer.Enabled.Value = newValue);
+        }
+
+        private void moveToText() => AddStep("Move mouse to text", () => InputManager.MoveMouseTo(hoverContainer));
+
+        private void moveOut() => AddStep("Move out", doMoveOut);
+
+        private void checkHovered() => AddAssert("Check hovered", () => hoverContainer.IsHovered);
+
+        private void checkNotHovered() => AddAssert("Check not hovered", () => !hoverContainer.IsHovered);
+
+        private void checkColour(ColourInfo expectedColour)
+            => AddAssert($"Check colour to be '{expectedColour}'", () => currentColour.Equals(expectedColour));
+
+        private void waitUntilColourIs(ColourInfo expectedColour)
+            => AddUntilStep($"Wait until hover colour is {expectedColour}", () => currentColour.Equals(expectedColour));
+
+        private ColourInfo currentColour => colourContainer.DrawColourInfo.Colour;
+
+        /// <summary>
+        ///     Moves the cursor to top left corner of the screen
+        /// </summary>
+        private void doMoveOut()
+            => InputManager.MoveMouseTo(new Vector2(InputManager.ScreenSpaceDrawQuad.TopLeft.X, InputManager.ScreenSpaceDrawQuad.TopLeft.Y));
+
+        private sealed class OsuHoverTestContainer : OsuHoverContainer
+        {
+            public static readonly Color4 HOVER_COLOUR = Color4.Red;
+            public static readonly Color4 IDLE_COLOUR = Color4.Green;
+
+            public OsuHoverTestContainer()
+            {
+                HoverColour = HOVER_COLOUR;
+                IdleColour = IDLE_COLOUR;
+            }
+        }
+    }
+}

osu.Game/Graphics/Containers/OsuHoverContainer.cs

@@ -24,8 +24,13 @@ namespace osu.Game.Graphics.Containers
         {
             Enabled.ValueChanged += e =>
             {
-                if (!e.NewValue)
-                    unhover();
+                if (isHovered)
+                {
+                    if (e.NewValue)
+                        fadeIn();
+                    else
+                        fadeOut();
+                }
             };
         }
 
@@ -33,28 +38,28 @@ namespace osu.Game.Graphics.Containers
 
         protected override bool OnHover(HoverEvent e)
         {
+            if (isHovered)
+                return false;
+
+            isHovered = true;
+
             if (!Enabled.Value)
                 return false;
 
-            EffectTargets.ForEach(d => d.FadeColour(HoverColour, FADE_DURATION, Easing.OutQuint));
-            isHovered = true;
+            fadeIn();
 
             return base.OnHover(e);
         }
 
         protected override void OnHoverLost(HoverLostEvent e)
-        {
-            unhover();
-            base.OnHoverLost(e);
-        }
-
-        private void unhover()
         {
             if (!isHovered)
                 return;
 
             isHovered = false;
-            EffectTargets.ForEach(d => d.FadeColour(IdleColour, FADE_DURATION, Easing.OutQuint));
+            fadeOut();
+
+            base.OnHoverLost(e);
         }
 
         [BackgroundDependencyLoader]
@@ -69,5 +74,9 @@ namespace osu.Game.Graphics.Containers
             base.LoadComplete();
             EffectTargets.ForEach(d => d.FadeColour(IdleColour));
         }
+
+        private void fadeIn() => EffectTargets.ForEach(d => d.FadeColour(HoverColour, FADE_DURATION, Easing.OutQuint));
+
+        private void fadeOut() => EffectTargets.ForEach(d => d.FadeColour(IdleColour, FADE_DURATION, Easing.OutQuint));
     }
 }