Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix song select rewind logic not handling deleted beatmaps #23312

Merged
merged 5 commits into from
Jun 15, 2023
Merged

Fix song select rewind logic not handling deleted beatmaps #23312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0991c56
Add extra check in 'SelectPreviousRandom'
Renzo904 Apr 25, 2023
2360219
Merge branch 'master' into fix-rewind-bug
frenzibyte Jun 15, 2023
cabb4df
Merge branch 'master' into fix-rewind-bug
frenzibyte Jun 15, 2023
b9543f4
Add failing test case
frenzibyte Jun 15, 2023
39db17d
Use better method to avoid rewinding to deleted beatmaps
frenzibyte Jun 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,25 @@ public void TestRandom()
AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false));
}

[Test]
public void TestRewindToDeletedBeatmap()
{
loadBeatmaps();

var firstAdded = TestResources.CreateTestBeatmapSetInfo();

AddStep("add new set", () => carousel.UpdateBeatmapSet(firstAdded));
AddStep("select set", () => carousel.SelectBeatmap(firstAdded.Beatmaps.First()));

nextRandom();

AddStep("delete set", () => carousel.RemoveBeatmapSet(firstAdded));

prevRandom();

AddAssert("deleted set not selected", () => carousel.SelectedBeatmapSet?.Equals(firstAdded) == false);
}

/// <summary>
/// Test adding and removing beatmap sets
/// </summary>
Expand Down
14 changes: 10 additions & 4 deletions osu.Game/Screens/Select/BeatmapCarousel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ private void loadBeatmapSets(IEnumerable<BeatmapSetInfo> beatmapSets)

public Bindable<RandomSelectAlgorithm> RandomAlgorithm = new Bindable<RandomSelectAlgorithm>();
private readonly List<CarouselBeatmapSet> previouslyVisitedRandomSets = new List<CarouselBeatmapSet>();
private readonly Stack<CarouselBeatmap> randomSelectedBeatmaps = new Stack<CarouselBeatmap>();
private readonly List<CarouselBeatmap> randomSelectedBeatmaps = new List<CarouselBeatmap>();

private CarouselRoot root;

Expand Down Expand Up @@ -348,6 +348,11 @@ private void removeBeatmapSet(Guid beatmapSetID) => Schedule(() =>
if (!root.BeatmapSetsByID.TryGetValue(beatmapSetID, out var existingSet))
return;

foreach (var beatmap in existingSet.Beatmaps)
randomSelectedBeatmaps.Remove(beatmap);

previouslyVisitedRandomSets.Remove(existingSet);

root.RemoveItem(existingSet);
itemsCache.Invalidate();

Expand Down Expand Up @@ -501,7 +506,7 @@ public bool SelectNextRandom()

if (selectedBeatmap != null && selectedBeatmapSet != null)
{
randomSelectedBeatmaps.Push(selectedBeatmap);
randomSelectedBeatmaps.Add(selectedBeatmap);

// when performing a random, we want to add the current set to the previously visited list
// else the user may be "randomised" to the existing selection.
Expand Down Expand Up @@ -538,9 +543,10 @@ public void SelectPreviousRandom()
{
while (randomSelectedBeatmaps.Any())
{
var beatmap = randomSelectedBeatmaps.Pop();
var beatmap = randomSelectedBeatmaps[^1];
randomSelectedBeatmaps.Remove(beatmap);

if (!beatmap.Filtered.Value)
if (!beatmap.Filtered.Value && beatmap.BeatmapInfo.BeatmapSet?.DeletePending != true)
{
if (selectedBeatmapSet != null)
{
Expand Down