diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index ebee358730..2664c7a42c 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -138,7 +138,7 @@ namespace osu.Game.Tests.Visual.SongSelect { createSongSelect(); changeRuleset(2); - importForRuleset(0); + addRulesetImportStep(0); AddUntilStep("no selection", () => songSelect.Carousel.SelectedBeatmap == null); } @@ -147,8 +147,8 @@ namespace osu.Game.Tests.Visual.SongSelect { createSongSelect(); changeRuleset(2); - importForRuleset(2); - importForRuleset(1); + addRulesetImportStep(2); + addRulesetImportStep(1); AddUntilStep("has selection", () => songSelect.Carousel.SelectedBeatmap.RulesetID == 2); changeRuleset(1); @@ -210,7 +210,52 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("start not requested", () => !startRequested); } - private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); + [Test] + public void TestAddNewBeatmapWhileSelectingRandom() + { + const int test_count = 10; + int beatmapChangedCount = 0; + int debounceCount = 0; + createSongSelect(); + AddStep("Setup counters", () => + { + beatmapChangedCount = 0; + debounceCount = 0; + songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount++; + }); + AddRepeatStep($"Create beatmaps {test_count} times", () => + { + importForRuleset(0); + + Scheduler.AddDelayed(() => + { + // Wait for debounce + songSelect.Carousel.SelectNextRandom(); + ++debounceCount; + }, 400); + }, test_count); + + AddUntilStep("Debounce limit reached", () => debounceCount == test_count); + + // The selected beatmap should have changed an additional 2 times since both initially loading songselect and the first import also triggers selectionChanged + AddAssert($"Beatmap changed {test_count + 2} times", () => beatmapChangedCount == test_count + 2); + } + + [Test] + public void TestHideSetSelectsCorrectBeatmap() + { + int? previousID = null; + createSongSelect(); + addRulesetImportStep(0); + AddStep("Move to last difficulty", () => songSelect.Carousel.SelectBeatmap(songSelect.Carousel.BeatmapSets.First().Beatmaps.Last())); + AddStep("Store current ID", () => previousID = songSelect.Carousel.SelectedBeatmap.ID); + AddStep("Hide first beatmap", () => manager.Hide(songSelect.Carousel.SelectedBeatmapSet.Beatmaps.First())); + AddAssert("Selected beatmap has not changed", () => songSelect.Carousel.SelectedBeatmap.ID == previousID); + } + + private void addRulesetImportStep(int id) => AddStep($"import test map for ruleset {id}", () => importForRuleset(id)); + + private void importForRuleset(int id) => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray())); private static int importId; private int getImportId() => ++importId; diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 63ad3b6ab2..cf21c78c7f 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -152,9 +152,12 @@ namespace osu.Game.Screens.Select { Schedule(() => { + int? previouslySelectedID = null; CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); - bool hadSelection = existingSet?.State?.Value == CarouselItemState.Selected; + // If the selected beatmap is about to be removed, store its ID so it can be re-selected if required + if (existingSet?.State?.Value == CarouselItemState.Selected) + previouslySelectedID = selectedBeatmap?.Beatmap.ID; var newSet = createCarouselSet(beatmapSet); @@ -172,8 +175,8 @@ namespace osu.Game.Screens.Select applyActiveCriteria(false, false); //check if we can/need to maintain our current selection. - if (hadSelection) - select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID) ?? newSet); + if (previouslySelectedID != null) + select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == previouslySelectedID) ?? newSet); itemsCache.Invalidate(); Schedule(() => BeatmapSetsChanged?.Invoke()); diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 5390d24892..d0645dbab6 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -594,11 +594,17 @@ namespace osu.Game.Screens.Select { bindBindables(); + // If a selection was already obtained, do not attempt to update the selected beatmap. + if (Carousel.SelectedBeatmapSet != null) + return; + + // Attempt to select the current beatmap on the carousel, if it is valid to be selected. if (!Beatmap.IsDefault && Beatmap.Value.BeatmapSetInfo?.DeletePending == false && Beatmap.Value.BeatmapSetInfo?.Protected == false && Carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, false)) return; - if (Carousel.SelectedBeatmapSet == null && !Carousel.SelectNextRandom()) + // If the current active beatmap could not be selected, select a new random beatmap. + 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).