Make song select choose random when initial selection fails.

- Revert TestCasePlaySongSelect to master

osu.Game.Tests/Visual/TestCasePlaySongSelect.cs

@@ -11,7 +11,6 @@ using osu.Framework.Allocation;
 using osu.Framework.Extensions;
 using osu.Framework.MathUtils;
 using osu.Game.Beatmaps;
-using osu.Game.Configuration;
 using osu.Game.Database;
 using osu.Game.Rulesets;
 using osu.Game.Screens.Select;
@@ -56,22 +55,10 @@ namespace osu.Game.Tests.Visual
             public WorkingBeatmap CurrentBeatmap => Beatmap.Value;
             public WorkingBeatmap CurrentBeatmapDetailsBeatmap => BeatmapDetails.Beatmap;
             public new BeatmapCarousel Carousel => base.Carousel;
-
-            public void SetRuleset(RulesetInfo ruleset) => Ruleset.Value = ruleset;
-
-            public int? RulesetID => Ruleset.Value.ID;
-
-            protected override void Dispose(bool isDisposing)
-            {
-                base.Dispose(isDisposing);
-
-                // Necessary while running tests because gc is moody and uncollected object interferes with OnEntering test
-                Beatmap.ValueChanged -= WorkingBeatmapChanged;
-            }
         }
 
         [BackgroundDependencyLoader]
-        private void load(OsuGameBase game, OsuConfigManager config)
+        private void load(OsuGameBase game)
         {
             TestSongSelect songSelect = null;
 
@@ -90,7 +77,6 @@ namespace osu.Game.Tests.Visual
             {
                 if (deleteMaps)
                 {
-                    // TODO: check why this alone doesn't allow import test to run twice in the same session, probably because the delete op is not saved?
                     manager.Delete(manager.GetAllUsableBeatmapSets());
                     game.Beatmap.SetDefault();
                 }
@@ -102,8 +88,6 @@ namespace osu.Game.Tests.Visual
                 }
 
                 Add(songSelect = new TestSongSelect());
-
-                songSelect?.SetRuleset(rulesets.AvailableRulesets.First());
             });
 
             loadNewSongSelect(true);
@@ -118,36 +102,6 @@ namespace osu.Game.Tests.Visual
             {
                 for (int i = 0; i < 100; i += 10)
                     manager.Import(createTestBeatmapSet(i));
-
-                // also import a set which has a single non - osu ruleset beatmap
-                manager.Import(new BeatmapSetInfo
-                {
-                    OnlineBeatmapSetID = 1993,
-                    Hash = new MemoryStream(Encoding.UTF8.GetBytes(Guid.NewGuid().ToString())).ComputeMD5Hash(),
-                    Metadata = new BeatmapMetadata
-                    {
-                        OnlineBeatmapSetID = 1993,
-                        // Create random metadata, then we can check if sorting works based on these
-                        Artist = "MONACA " + RNG.Next(0, 9),
-                        Title = "Black Song " + RNG.Next(0, 9),
-                        AuthorString = "Some Guy " + RNG.Next(0, 9),
-                    },
-                    Beatmaps = new List<BeatmapInfo>
-                    {
-                        new BeatmapInfo
-                        {
-                            OnlineBeatmapID = 1994,
-                            Ruleset = rulesets.AvailableRulesets.ElementAt(3),
-                            RulesetID = 3,
-                            Path = "normal.fruits",
-                            Version = "Normal",
-                            BaseDifficulty = new BeatmapDifficulty
-                            {
-                                OverallDifficulty = 3.5f,
-                            }
-                        },
-                    }
-                });
             });
 
             AddWaitStep(3);
@@ -161,45 +115,6 @@ namespace osu.Game.Tests.Visual
             AddStep(@"Sort by Title", delegate { songSelect.FilterControl.Sort = SortMode.Title; });
             AddStep(@"Sort by Author", delegate { songSelect.FilterControl.Sort = SortMode.Author; });
             AddStep(@"Sort by Difficulty", delegate { songSelect.FilterControl.Sort = SortMode.Difficulty; });
-
-            // Test that song select sets a playable beatmap while entering
-            AddStep(@"Remove song select", () =>
-            {
-                Remove(songSelect);
-                songSelect.Dispose();
-                songSelect = null;
-            });
-            AddStep(@"Set non-osu beatmap", () => game.Beatmap.Value = manager.GetWorkingBeatmap(manager.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID != 0)));
-            AddAssert(@"Non-osu beatmap set", () => game.Beatmap.Value.BeatmapInfo.RulesetID != 0);
-            loadNewSongSelect();
-            AddWaitStep(3);
-            AddAssert(@"osu beatmap set", () => game.Beatmap.Value.BeatmapInfo.RulesetID == 0);
-
-            // Test that song select changes WorkingBeatmap to be playable in current ruleset when updated externally
-            AddStep(@"Try set non-osu beatmap", () =>
-            {
-                var testMap = manager.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID != 0);
-                songSelect.SetRuleset(rulesets.AvailableRulesets.First());
-                game.Beatmap.Value = manager.GetWorkingBeatmap(testMap);
-            });
-            AddAssert(@"Beatmap changed to osu", () => songSelect.RulesetID == 0 && game.Beatmap.Value.BeatmapInfo.RulesetID == 0);
-
-            // Test that song select updates WorkingBeatmap when ruleset conversion is disabled
-            AddStep(@"Disable beatmap conversion", () => config.Set(OsuSetting.ShowConvertedBeatmaps, false));
-            AddStep(@"Set osu beatmap taiko rs", () =>
-            {
-                game.Beatmap.Value = manager.GetWorkingBeatmap(manager.GetAllUsableBeatmapSets().First().Beatmaps.First(b => b.RulesetID == 0));
-                songSelect.SetRuleset(rulesets.AvailableRulesets.First(r => r.ID == 1));
-            });
-            AddAssert(@"taiko beatmap set", () => songSelect.RulesetID == 1);
-
-            // Test that song select changes the active ruleset when externally set beatmapset has no playable beatmaps
-            AddStep(@"Set fruits only beatmapset", () =>
-            {
-                songSelect.SetRuleset(rulesets.AvailableRulesets.First());
-                game.Beatmap.Value = manager.GetWorkingBeatmap(manager.QueryBeatmapSet(b => b.OnlineBeatmapSetID == 1993).Beatmaps.First());
-            });
-            AddAssert(@"Ruleset changed to fruits", () => songSelect.RulesetID == game.Beatmap.Value.BeatmapInfo.RulesetID);
         }
 
         private BeatmapSetInfo createTestBeatmapSet(int i)
@@ -221,8 +136,7 @@ namespace osu.Game.Tests.Visual
                     new BeatmapInfo
                     {
                         OnlineBeatmapID = 1234 + i,
-                        Ruleset = rulesets.AvailableRulesets.ElementAt(0),
+                        Ruleset = rulesets.AvailableRulesets.First(),
-                        RulesetID = 0,
                         Path = "normal.osu",
                         Version = "Normal",
                         BaseDifficulty = new BeatmapDifficulty
@@ -233,9 +147,8 @@ namespace osu.Game.Tests.Visual
                     new BeatmapInfo
                     {
                         OnlineBeatmapID = 1235 + i,
-                        Ruleset = rulesets.AvailableRulesets.First(r => r.ID != 0),
+                        Ruleset = rulesets.AvailableRulesets.First(),
-                        RulesetID = 1,
+                        Path = "hard.osu",
-                        Path = "hard.taiko",
                         Version = "Hard",
                         BaseDifficulty = new BeatmapDifficulty
                         {
@@ -245,9 +158,8 @@ namespace osu.Game.Tests.Visual
                     new BeatmapInfo
                     {
                         OnlineBeatmapID = 1236 + i,
-                        Ruleset = rulesets.AvailableRulesets.ElementAt(2),
+                        Ruleset = rulesets.AvailableRulesets.First(),
-                        RulesetID = 2,
+                        Path = "insane.osu",
-                        Path = "insane.fruits",
                         Version = "Insane",
                         BaseDifficulty = new BeatmapDifficulty
                         {

osu.Game/Screens/Select/BeatmapCarousel.cs

@@ -171,27 +171,41 @@ namespace osu.Game.Screens.Select
 
         /// <summary>
         /// Selects a given beatmap on the carousel.
+        ///
+        /// If skipFiltered is true, we will try to select another unfiltered beatmap in the same set. If the
+        /// entire set is filtered, no selection is made.
         /// </summary>
         /// <param name="beatmap">The beatmap to select.</param>
         /// <param name="skipFiltered">Whether to skip selecting filtered beatmaps.</param>
-        /// <returns>True if a selection was made, false if it was skipped.</returns>
+        /// <returns>True if a selection was made, False if it wasn't.</returns>
         public bool SelectBeatmap(BeatmapInfo beatmap, bool skipFiltered = false)
         {
             if (beatmap?.Hidden != false)
                 return false;
 
-            var group = beatmapSets.FirstOrDefault(s => s.BeatmapSet.OnlineBeatmapSetID == beatmap.BeatmapSet.OnlineBeatmapSetID);
+            foreach (CarouselBeatmapSet set in beatmapSets)
+            {
+                if (skipFiltered && set.Filtered)
+                    continue;
 
-            if (group == null || !skipFiltered && group.Filtered)
+                var item = set.Beatmaps.FirstOrDefault(p => p.Beatmap.Equals(beatmap));
-                return false;
 
-            var item = group.Beatmaps.FirstOrDefault(p => p.Beatmap.Equals(beatmap));
+                if (item == null)
+                    // The beatmap that needs to be selected doesn't exist in this set
+                    continue;
 
-            if (item == null || !skipFiltered && item.Filtered)
+                if (skipFiltered && item.Filtered)
-                return false;
+                    // The beatmap exists in this set but is filtered, so look for the first unfiltered map in the set
+                    item = set.Beatmaps.FirstOrDefault(b => !b.Filtered);
 
-            select(item);
+                if (item != null)
-            return true;
+                {
+                    select(item);
+                    return true;
+                }
+            }
+
+            return false;
         }
 
         /// <summary>

osu.Game/Screens/Select/SongSelect.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
 
 using System;
-using System.Linq;
 using System.Threading;
 using OpenTK;
 using OpenTK.Input;
@@ -10,14 +9,12 @@ using osu.Framework.Allocation;
 using osu.Framework.Audio;
 using osu.Framework.Audio.Sample;
 using osu.Framework.Audio.Track;
-using osu.Framework.Configuration;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Input;
 using osu.Framework.Screens;
 using osu.Framework.Threading;
 using osu.Game.Beatmaps;
-using osu.Game.Configuration;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
 using osu.Game.Overlays;
@@ -66,8 +63,6 @@ namespace osu.Game.Screens.Select
         private SampleChannel sampleChangeDifficulty;
         private SampleChannel sampleChangeBeatmap;
 
-        private Bindable<bool> rulesetConversionAllowed;
-
         private CancellationTokenSource initialAddSetsTask;
 
         private DependencyContainer dependencies;
@@ -184,7 +179,7 @@ namespace osu.Game.Screens.Select
         }
 
         [BackgroundDependencyLoader(permitNulls: true)]
-        private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuGame osu, OsuColour colours, OsuConfigManager config)
+        private void load(BeatmapManager beatmaps, AudioManager audio, DialogOverlay dialog, OsuGame osu, OsuColour colours)
         {
             dependencies.CacheAs(this);
 
@@ -199,8 +194,6 @@ namespace osu.Game.Screens.Select
             if (this.beatmaps == null)
                 this.beatmaps = beatmaps;
 
-            rulesetConversionAllowed = config.GetBindable<bool>(OsuSetting.ShowConvertedBeatmaps);
-
             if (osu != null)
                 Ruleset.BindTo(osu.Ruleset);
 
@@ -459,16 +452,14 @@ namespace osu.Game.Screens.Select
 
         private void carouselBeatmapsLoaded()
         {
-            if (!Beatmap.IsDefault && Beatmap.Value.BeatmapSetInfo?.DeletePending == false && Beatmap.Value.BeatmapSetInfo?.Protected == false)
+            if (!Beatmap.IsDefault && Beatmap.Value.BeatmapSetInfo?.DeletePending == false && Beatmap.Value.BeatmapSetInfo?.Protected == false && Carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, true))
+                return;
+
+            if (Carousel.SelectedBeatmapSet == null && !Carousel.SelectNextRandom())
             {
-                Carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo);
+                // in the case random selection failed, we want to trigger selectionChanged
-            }
+                // to show the dummy beatmap (we have nothing else to display).
-            else if (Carousel.SelectedBeatmapSet == null)
+                carouselSelectionChanged(null);
-            {
-                if (!Carousel.SelectNextRandom())
-                    // in the case random selection failed, we want to trigger selectionChanged
-                    // to show the dummy beatmap (we have nothing else to display).
-                    carouselSelectionChanged(null);
             }
         }