Merge pull request #8370 from peppy/fix-carousel-root-selects-from-nothing

Fix selection not occurring when switching away from empty ruleset

osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs

@@ -278,6 +278,7 @@ namespace osu.Game.Tests.Visual.Background
 
         private void setupUserSettings()
         {
+            AddUntilStep("Song select is current", () => songSelect.IsCurrentScreen());
             AddUntilStep("Song select has selection", () => songSelect.Carousel?.SelectedBeatmap != null);
             AddStep("Set default user settings", () =>
             {

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

@@ -227,6 +227,34 @@ namespace osu.Game.Tests.Visual.SongSelect
             waitForSelection(set_count);
         }
 
+        [Test]
+        public void TestSelectionEnteringFromEmptyRuleset()
+        {
+            var sets = new List<BeatmapSetInfo>();
+
+            AddStep("Create beatmaps for taiko only", () =>
+            {
+                sets.Clear();
+
+                var rulesetBeatmapSet = createTestBeatmapSet(1);
+                var taikoRuleset = rulesets.AvailableRulesets.ElementAt(1);
+                rulesetBeatmapSet.Beatmaps.ForEach(b =>
+                {
+                    b.Ruleset = taikoRuleset;
+                    b.RulesetID = 1;
+                });
+
+                sets.Add(rulesetBeatmapSet);
+            });
+
+            loadBeatmaps(sets, () => new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(0) });
+
+            AddStep("Set non-empty mode filter", () =>
+                carousel.Filter(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(1) }, false));
+
+            AddAssert("Something is selected", () => carousel.SelectedBeatmap != null);
+        }
+
         /// <summary>
         /// Test sorting
         /// </summary>
@@ -399,20 +427,25 @@ namespace osu.Game.Tests.Visual.SongSelect
             AddStep("filter to ruleset 0", () =>
                 carousel.Filter(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(0) }, false));
             AddStep("select filtered map skipping filtered", () => carousel.SelectBeatmap(testMixed.Beatmaps[1], false));
-            AddAssert("unfiltered beatmap not selected", () => carousel.SelectedBeatmap == null);
+            AddAssert("unfiltered beatmap not selected", () => carousel.SelectedBeatmap.RulesetID == 0);
 
             AddStep("remove mixed set", () =>
             {
                 carousel.RemoveBeatmapSet(testMixed);
                 testMixed = null;
             });
-            var testSingle = createTestBeatmapSet(set_count + 2);
-            testSingle.Beatmaps.ForEach(b =>
+            BeatmapSetInfo testSingle = null;
+            AddStep("add single ruleset beatmapset", () =>
             {
-                b.Ruleset = rulesets.AvailableRulesets.ElementAt(1);
-                b.RulesetID = b.Ruleset.ID ?? 1;
+                testSingle = createTestBeatmapSet(set_count + 2);
+                testSingle.Beatmaps.ForEach(b =>
+                {
+                    b.Ruleset = rulesets.AvailableRulesets.ElementAt(1);
+                    b.RulesetID = b.Ruleset.ID ?? 1;
+                });
+
+                carousel.UpdateBeatmapSet(testSingle);
             });
-            AddStep("add single ruleset beatmapset", () => carousel.UpdateBeatmapSet(testSingle));
             AddStep("select filtered map skipping filtered", () => carousel.SelectBeatmap(testSingle.Beatmaps[0], false));
             checkNoSelection();
             AddStep("remove single ruleset set", () => carousel.RemoveBeatmapSet(testSingle));
@@ -546,7 +579,7 @@ namespace osu.Game.Tests.Visual.SongSelect
             checkVisibleItemCount(true, 15);
         }
 
-        private void loadBeatmaps(List<BeatmapSetInfo> beatmapSets = null)
+        private void loadBeatmaps(List<BeatmapSetInfo> beatmapSets = null, Func<FilterCriteria> initialCriteria = null)
         {
             createCarousel();
 
@@ -561,7 +594,7 @@ namespace osu.Game.Tests.Visual.SongSelect
             bool changed = false;
             AddStep($"Load {(beatmapSets.Count > 0 ? beatmapSets.Count.ToString() : "some")} beatmaps", () =>
             {
-                carousel.Filter(new FilterCriteria());
+                carousel.Filter(initialCriteria?.Invoke() ?? new FilterCriteria());
                 carousel.BeatmapSetsChanged = () => changed = true;
                 carousel.BeatmapSets = beatmapSets;
             });

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -750,13 +750,17 @@ namespace osu.Game.Screens.Select
 
             public CarouselRoot(BeatmapCarousel carousel)
             {
+                // root should always remain selected. if not, PerformSelection will not be called.
+                State.Value = CarouselItemState.Selected;
+                State.ValueChanged += state => State.Value = CarouselItemState.Selected;
+
                 this.carousel = carousel;
             }
 
             protected override void PerformSelection()
             {
                 if (LastSelected == null || LastSelected.Filtered.Value)
-                    carousel.SelectNextRandom();
+                    carousel?.SelectNextRandom();
                 else
                     base.PerformSelection();
             }

osu.Game/Screens/Select/SongSelect.cs

@@ -150,6 +150,7 @@ namespace osu.Game.Screens.Select
                                         },
                                         Child = Carousel = new BeatmapCarousel
                                         {
+                                            AllowSelection = false, // delay any selection until our bindables are ready to make a good choice.
                                             Anchor = Anchor.CentreRight,
                                             Origin = Anchor.CentreRight,
                                             RelativeSizeAxes = Axes.Both,
@@ -655,6 +656,8 @@ namespace osu.Game.Screens.Select
         {
             bindBindables();
 
+            Carousel.AllowSelection = true;
+
             // If a selection was already obtained, do not attempt to update the selected beatmap.
             if (Carousel.SelectedBeatmapSet != null)
                 return;