Merge pull request #17386 from peppy/skin-editor-eat-input

Fix keyboard controls not working to adjust skin settings

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

@@ -1,14 +1,19 @@
 // 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.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
+using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Testing;
 using osu.Game.Overlays;
+using osu.Game.Overlays.Settings;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Osu;
+using osu.Game.Screens.Play.HUD.HitErrorMeters;
 using osu.Game.Skinning.Editor;
+using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.Gameplay
 {
@@ -29,7 +34,7 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddStep("reload skin editor", () =>
             {
                 skinEditor?.Expire();
-                Player.ScaleTo(0.8f);
+                Player.ScaleTo(0.4f);
                 LoadComponentAsync(skinEditor = new SkinEditor(Player), Add);
             });
         }
@@ -40,6 +45,36 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddToggleStep("toggle editor visibility", visible => skinEditor.ToggleVisibility());
         }
 
+        [Test]
+        public void TestEditComponent()
+        {
+            BarHitErrorMeter hitErrorMeter = null;
+
+            AddStep("select bar hit error blueprint", () =>
+            {
+                var blueprint = skinEditor.ChildrenOfType<SkinBlueprint>().First(b => b.Item is BarHitErrorMeter);
+
+                hitErrorMeter = (BarHitErrorMeter)blueprint.Item;
+                skinEditor.SelectedComponents.Clear();
+                skinEditor.SelectedComponents.Add(blueprint.Item);
+            });
+
+            AddAssert("value is default", () => hitErrorMeter.JudgementLineThickness.IsDefault);
+
+            AddStep("hover first slider", () =>
+            {
+                InputManager.MoveMouseTo(
+                    skinEditor.ChildrenOfType<SkinSettingsToolbox>().First()
+                              .ChildrenOfType<SettingsSlider<float>>().First()
+                              .ChildrenOfType<SliderBar<float>>().First()
+                );
+            });
+
+            AddStep("adjust slider via keyboard", () => InputManager.Key(Key.Left));
+
+            AddAssert("value is less than default", () => hitErrorMeter.JudgementLineThickness.Value < hitErrorMeter.JudgementLineThickness.Default);
+        }
+
         protected override Ruleset CreatePlayerRuleset() => new OsuRuleset();
     }
 }

osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs

@@ -7,6 +7,7 @@ using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Extensions;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Screens;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
@@ -14,6 +15,7 @@ using osu.Game.Graphics.UserInterface;
 using osu.Game.Online.Leaderboards;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Mods;
+using osu.Game.Overlays.Settings;
 using osu.Game.Overlays.Toolbar;
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu.Mods;
@@ -21,10 +23,12 @@ using osu.Game.Scoring;
 using osu.Game.Screens.Menu;
 using osu.Game.Screens.OnlinePlay.Lounge;
 using osu.Game.Screens.Play;
+using osu.Game.Screens.Play.HUD.HitErrorMeters;
 using osu.Game.Screens.Ranking;
 using osu.Game.Screens.Select;
 using osu.Game.Screens.Select.Leaderboards;
 using osu.Game.Screens.Select.Options;
+using osu.Game.Skinning.Editor;
 using osu.Game.Tests.Beatmaps.IO;
 using osuTK;
 using osuTK.Input;
@@ -66,6 +70,73 @@ namespace osu.Game.Tests.Visual.Navigation
             AddAssert("Overlay was shown", () => songSelect.ModSelectOverlay.State.Value == Visibility.Visible);
         }
 
+        [Test]
+        public void TestEditComponentDuringGameplay()
+        {
+            Screens.Select.SongSelect songSelect = null;
+            PushAndConfirm(() => songSelect = new TestPlaySongSelect());
+            AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded);
+
+            AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely());
+
+            AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault);
+
+            SkinEditor skinEditor = null;
+
+            AddStep("open skin editor", () =>
+            {
+                InputManager.PressKey(Key.ControlLeft);
+                InputManager.PressKey(Key.ShiftLeft);
+                InputManager.Key(Key.S);
+                InputManager.ReleaseKey(Key.ControlLeft);
+                InputManager.ReleaseKey(Key.ShiftLeft);
+            });
+
+            AddUntilStep("get skin editor", () => (skinEditor = Game.ChildrenOfType<SkinEditor>().FirstOrDefault()) != null);
+
+            AddStep("Click gameplay scene button", () =>
+            {
+                skinEditor.ChildrenOfType<SkinEditorSceneLibrary.SceneButton>().First(b => b.Text == "Gameplay").TriggerClick();
+            });
+
+            AddUntilStep("wait for player", () =>
+            {
+                // dismiss any notifications that may appear (ie. muted notification).
+                clickMouseInCentre();
+                return Game.ScreenStack.CurrentScreen is Player;
+            });
+
+            BarHitErrorMeter hitErrorMeter = null;
+
+            AddUntilStep("select bar hit error blueprint", () =>
+            {
+                var blueprint = skinEditor.ChildrenOfType<SkinBlueprint>().FirstOrDefault(b => b.Item is BarHitErrorMeter);
+
+                if (blueprint == null)
+                    return false;
+
+                hitErrorMeter = (BarHitErrorMeter)blueprint.Item;
+                skinEditor.SelectedComponents.Clear();
+                skinEditor.SelectedComponents.Add(blueprint.Item);
+                return true;
+            });
+
+            AddAssert("value is default", () => hitErrorMeter.JudgementLineThickness.IsDefault);
+
+            AddStep("hover first slider", () =>
+            {
+                InputManager.MoveMouseTo(
+                    skinEditor.ChildrenOfType<SkinSettingsToolbox>().First()
+                              .ChildrenOfType<SettingsSlider<float>>().First()
+                              .ChildrenOfType<SliderBar<float>>().First()
+                );
+            });
+
+            AddStep("adjust slider via keyboard", () => InputManager.Key(Key.Left));
+
+            AddAssert("value is less than default", () => hitErrorMeter.JudgementLineThickness.Value < hitErrorMeter.JudgementLineThickness.Default);
+        }
+
         [Test]
         public void TestRetryCountIncrements()
         {
@@ -120,7 +191,8 @@ namespace osu.Game.Tests.Visual.Navigation
 
             AddStep("press back button", () => Game.ChildrenOfType<BackButton>().First().Action());
 
-            AddStep("show local scores", () => Game.ChildrenOfType<BeatmapDetailAreaTabControl>().First().Current.Value = new BeatmapDetailAreaLeaderboardTabItem<BeatmapLeaderboardScope>(BeatmapLeaderboardScope.Local));
+            AddStep("show local scores",
+                () => Game.ChildrenOfType<BeatmapDetailAreaTabControl>().First().Current.Value = new BeatmapDetailAreaLeaderboardTabItem<BeatmapLeaderboardScope>(BeatmapLeaderboardScope.Local));
 
             AddUntilStep("wait for score displayed", () => (scorePanel = Game.ChildrenOfType<LeaderboardScore>().FirstOrDefault(s => s.Score.Equals(score))) != null);
 
@@ -152,7 +224,8 @@ namespace osu.Game.Tests.Visual.Navigation
 
             AddStep("press back button", () => Game.ChildrenOfType<BackButton>().First().Action());
 
-            AddStep("show local scores", () => Game.ChildrenOfType<BeatmapDetailAreaTabControl>().First().Current.Value = new BeatmapDetailAreaLeaderboardTabItem<BeatmapLeaderboardScope>(BeatmapLeaderboardScope.Local));
+            AddStep("show local scores",
+                () => Game.ChildrenOfType<BeatmapDetailAreaTabControl>().First().Current.Value = new BeatmapDetailAreaLeaderboardTabItem<BeatmapLeaderboardScope>(BeatmapLeaderboardScope.Local));
 
             AddUntilStep("wait for score displayed", () => (scorePanel = Game.ChildrenOfType<LeaderboardScore>().FirstOrDefault(s => s.Score.Equals(score))) != null);
 

osu.Game/OsuGame.cs

@@ -1046,6 +1046,10 @@ namespace osu.Game
 
             switch (e.Action)
             {
+                case GlobalAction.ToggleSkinEditor:
+                    skinEditor.ToggleVisibility();
+                    return true;
+
                 case GlobalAction.ResetInputSettings:
                     Host.ResetInputHandlers();
                     frameworkConfig.GetBindable<ConfineMouseMode>(FrameworkSetting.ConfineMouseMode).SetDefault();

osu.Game/Overlays/Settings/Sections/SkinSection.cs

@@ -64,7 +64,7 @@ namespace osu.Game.Overlays.Settings.Sections
                 new SettingsButton
                 {
                     Text = SkinSettingsStrings.SkinLayoutEditor,
-                    Action = () => skinEditor?.Toggle(),
+                    Action = () => skinEditor?.ToggleVisibility(),
                 },
                 new ExportSkinButton(),
             };

osu.Game/Skinning/Editor/SkinEditorOverlay.cs

@@ -19,15 +19,15 @@ namespace osu.Game.Skinning.Editor
     /// A container which handles loading a skin editor on user request for a specified target.
     /// This also handles the scaling / positioning adjustment of the target.
     /// </summary>
-    public class SkinEditorOverlay : CompositeDrawable, IKeyBindingHandler<GlobalAction>
+    public class SkinEditorOverlay : OverlayContainer, IKeyBindingHandler<GlobalAction>
     {
         private readonly ScalingContainer scalingContainer;
 
+        protected override bool BlockNonPositionalInput => true;
+
         [CanBeNull]
         private SkinEditor skinEditor;
 
-        public const float VISIBLE_TARGET_SCALE = 0.8f;
-
         [Resolved(canBeNull: true)]
         private OsuGame game { get; set; }
 
@@ -49,33 +49,13 @@ namespace osu.Game.Skinning.Editor
 
                     Hide();
                     return true;
-
-                case GlobalAction.ToggleSkinEditor:
-                    Toggle();
-                    return true;
             }
 
             return false;
         }
 
-        public void Toggle()
+        protected override void PopIn()
         {
-            if (skinEditor == null)
-                Show();
-            else
-                skinEditor.ToggleVisibility();
-        }
-
-        public override void Hide()
-        {
-            // base call intentionally omitted.
-            skinEditor?.Hide();
-        }
-
-        public override void Show()
-        {
-            // base call intentionally omitted as we have custom behaviour.
-
             if (skinEditor != null)
             {
                 skinEditor.Show();
@@ -83,17 +63,11 @@ namespace osu.Game.Skinning.Editor
             }
 
             var editor = new SkinEditor();
+
             editor.State.BindValueChanged(visibility => updateComponentVisibility());
 
             skinEditor = editor;
 
-            // Schedule ensures that if `Show` is called before this overlay is loaded,
-            // it will not throw (LoadComponentAsync requires the load target to be in a loaded state).
-            Schedule(() =>
-            {
-                if (editor != skinEditor)
-                    return;
-
             LoadComponentAsync(editor, _ =>
             {
                 if (editor != skinEditor)
@@ -103,9 +77,10 @@ namespace osu.Game.Skinning.Editor
 
                 SetTarget(lastTargetScreen);
             });
-            });
         }
 
+        protected override void PopOut() => skinEditor?.Hide();
+
         private void updateComponentVisibility()
         {
             Debug.Assert(skinEditor != null);

osu.Game/Skinning/Editor/SkinEditorSceneLibrary.cs

@@ -104,7 +104,7 @@ namespace osu.Game.Skinning.Editor
             };
         }
 
-        private class SceneButton : OsuButton
+        public class SceneButton : OsuButton
         {
             public SceneButton()
             {