Merge pull request #7623 from peppy/fix-ss-click-crash

Fix crash due to misordered selection events

osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs

@@ -14,6 +14,7 @@ using osu.Framework.Extensions;
 using osu.Framework.Utils;
 using osu.Framework.Platform;
 using osu.Framework.Screens;
+using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Configuration;
 using osu.Game.Overlays;
@@ -25,6 +26,7 @@ using osu.Game.Rulesets.Taiko;
 using osu.Game.Screens.Select;
 using osu.Game.Screens.Select.Carousel;
 using osu.Game.Screens.Select.Filter;
+using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.SongSelect
 {
@@ -95,6 +97,123 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddAssert("filter count is 1", () => songSelect.FilterCount == 1);
         }
 
+        [Test]
+        public void TestChangeBeatmapBeforeEnter()
+        {
+            addRulesetImportStep(0);
+
+            createSongSelect();
+
+            AddUntilStep("wait for initial selection", () => !Beatmap.IsDefault);
+
+            WorkingBeatmap selected = null;
+
+            AddStep("store selected beatmap", () => selected = Beatmap.Value);
+
+            AddStep("select next and enter", () =>
+            {
+                InputManager.PressKey(Key.Down);
+                InputManager.ReleaseKey(Key.Down);
+                InputManager.PressKey(Key.Enter);
+                InputManager.ReleaseKey(Key.Enter);
+            });
+
+            AddAssert("ensure selection changed", () => selected != Beatmap.Value);
+
+            AddUntilStep("wait for return to song select", () => songSelect.IsCurrentScreen());
+            AddUntilStep("bindable lease returned", () => !Beatmap.Disabled);
+        }
+
+        [Test]
+        public void TestChangeBeatmapAfterEnter()
+        {
+            addRulesetImportStep(0);
+
+            createSongSelect();
+
+            AddUntilStep("wait for initial selection", () => !Beatmap.IsDefault);
+
+            WorkingBeatmap selected = null;
+
+            AddStep("store selected beatmap", () => selected = Beatmap.Value);
+
+            AddStep("select next and enter", () =>
+            {
+                InputManager.PressKey(Key.Enter);
+                InputManager.ReleaseKey(Key.Enter);
+                InputManager.PressKey(Key.Down);
+                InputManager.ReleaseKey(Key.Down);
+            });
+
+            AddAssert("ensure selection didn't change", () => selected == Beatmap.Value);
+
+            AddUntilStep("wait for return to song select", () => songSelect.IsCurrentScreen());
+            AddUntilStep("bindable lease returned", () => !Beatmap.Disabled);
+        }
+
+        [Test]
+        public void TestChangeBeatmapViaMouseBeforeEnter()
+        {
+            addRulesetImportStep(0);
+
+            createSongSelect();
+
+            AddUntilStep("wait for initial selection", () => !Beatmap.IsDefault);
+
+            WorkingBeatmap selected = null;
+
+            AddStep("store selected beatmap", () => selected = Beatmap.Value);
+
+            AddStep("select next and enter", () =>
+            {
+                InputManager.MoveMouseTo(songSelect.Carousel.ChildrenOfType<DrawableCarouselBeatmap>()
+                                                   .First(b => ((CarouselBeatmap)b.Item).Beatmap != songSelect.Carousel.SelectedBeatmap));
+
+                InputManager.PressButton(MouseButton.Left);
+                InputManager.ReleaseButton(MouseButton.Left);
+
+                InputManager.PressKey(Key.Enter);
+                InputManager.ReleaseKey(Key.Enter);
+            });
+
+            AddAssert("ensure selection changed", () => selected != Beatmap.Value);
+
+            AddUntilStep("wait for return to song select", () => songSelect.IsCurrentScreen());
+            AddUntilStep("bindable lease returned", () => !Beatmap.Disabled);
+        }
+
+        [Test]
+        public void TestChangeBeatmapViaMouseAfterEnter()
+        {
+            addRulesetImportStep(0);
+
+            createSongSelect();
+
+            AddUntilStep("wait for initial selection", () => !Beatmap.IsDefault);
+
+            WorkingBeatmap selected = null;
+
+            AddStep("store selected beatmap", () => selected = Beatmap.Value);
+
+            AddStep("select next and enter", () =>
+            {
+                InputManager.MoveMouseTo(songSelect.Carousel.ChildrenOfType<DrawableCarouselBeatmap>()
+                                                   .First(b => ((CarouselBeatmap)b.Item).Beatmap != songSelect.Carousel.SelectedBeatmap));
+
+                InputManager.PressButton(MouseButton.Left);
+
+                InputManager.PressKey(Key.Enter);
+                InputManager.ReleaseKey(Key.Enter);
+
+                InputManager.ReleaseButton(MouseButton.Left);
+            });
+
+            AddAssert("ensure selection didn't change", () => selected == Beatmap.Value);
+
+            AddUntilStep("wait for return to song select", () => songSelect.IsCurrentScreen());
+            AddUntilStep("bindable lease returned", () => !Beatmap.Disabled);
+        }
+
         [Test]
         public void TestNoFilterOnSimpleResume()
         {

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -56,6 +56,9 @@ namespace osu.Game.Screens.Select
         public override bool HandleNonPositionalInput => AllowSelection;
         public override bool HandlePositionalInput => AllowSelection;
 
+        public override bool PropagatePositionalInputSubTree => AllowSelection;
+        public override bool PropagateNonPositionalInputSubTree => AllowSelection;
+
         /// <summary>
         /// Whether carousel items have completed asynchronously loaded.
         /// </summary>
@@ -449,8 +452,6 @@ namespace osu.Game.Screens.Select
             return true;
         }
 
-        protected override bool ReceivePositionalInputAtSubTree(Vector2 screenSpacePos) => ReceivePositionalInputAt(screenSpacePos);
-
         protected override void Update()
         {
             base.Update();

osu.Game/Tests/Visual/ManualInputManagerTestScene.cs

@@ -8,6 +8,7 @@ using osu.Framework.Testing.Input;
 using osu.Game.Graphics.Cursor;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Graphics.UserInterface;
+using osu.Game.Input.Bindings;
 using osuTK;
 using osuTK.Graphics;
 
@@ -30,7 +31,11 @@ namespace osu.Game.Tests.Visual
                 InputManager = new ManualInputManager
                 {
                     UseParentInput = true,
-                    Child = content = new MenuCursorContainer { RelativeSizeAxes = Axes.Both },
+                    Child = new GlobalActionContainer(null)
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                        Child = content = new MenuCursorContainer { RelativeSizeAxes = Axes.Both }
+                    },
                 },
                 new Container
                 {