Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,56 @@
AddAssert("Order didn't change", () => Carousel.PostFilterBeatmaps.Select(b => b.ID), () => Is.EqualTo(originalOrder));
}

/// <summary>
/// Replicates the #34826 scenario: the matched beatmap is present in the replace snapshot, but has been
/// deleted from realm by the time the replace is processed, see https://github.com/ppy/osu/issues/34826.
/// </summary>
[Test]
public void TestBeatmapSetReplacedWithDeletedCurrentBeatmap()
{
int targetSetIndex = 0;

Check warning

Code scanning / InspectCode

Assignment is not used Warning test

Value assigned is not used in any execution path

AddStep("select first difficulty", () =>
{
Carousel.CurrentBeatmap = baseTestBeatmap.Beatmaps[0];
BeatmapRequestedSelections.Clear();
});

AddStep("delete current beatmap from realm and replace set", () =>
{
targetSetIndex = BeatmapSets.IndexOf(baseTestBeatmap);
var detachedSet = BeatmapSets[targetSetIndex];
var selectedBeatmap = detachedSet.Beatmaps[0];

Realm.Write(r =>
{
var toDelete = r.Find<BeatmapInfo>(selectedBeatmap.ID);
if (toDelete != null)
r.Remove(toDelete);
});

// Trigger the Replace action with a beatmap that is not in realm.
var staleSet = new BeatmapSetInfo
{
ID = detachedSet.ID,
OnlineID = detachedSet.OnlineID,
DateAdded = detachedSet.DateAdded,
DateSubmitted = detachedSet.DateSubmitted,
Status = detachedSet.Status,
Hash = detachedSet.Hash,
Protected = detachedSet.Protected,
};

var staleBeatmap = createBeatmap(staleSet, selectedBeatmap);
staleSet.Beatmaps.Add(staleBeatmap);
BeatmapSets.ReplaceRange(targetSetIndex, 1, [staleSet]);
});

WaitForFiltering();

AddAssert("deleted match never requested for selection", () => BeatmapRequestedSelections, () => Is.Empty);
}

private void assertDidFilter(int count = 1) => AddAssert("did filter", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count + count));

private void assertDidNotFilter() => AddAssert("did not filter", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count));
Expand Down
24 changes: 15 additions & 9 deletions osu.Game/Screens/Select/BeatmapCarousel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -259,20 +259,26 @@
newSetBeatmaps.FirstOrDefault(b => b.OnlineID > 0 && b.OnlineID == beatmap.OnlineID) ??
newSetBeatmaps.FirstOrDefault(b => b.DifficultyName == beatmap.DifficultyName && b.Ruleset.Equals(beatmap.Ruleset));

// The matching beatmap may have been deleted or invalidated in some way since this event was fired.
// Let's make sure we have the most up-to-date realm state.
if (matchingNewBeatmap?.ID is Guid matchingID)
matchingNewBeatmap = realm.Run(r => r.FindWithRefresh<BeatmapInfo>(matchingID)?.Detach());

if (matchingNewBeatmap != null)
{
// TODO: should this exist in song select instead of here?
// we need to ensure the global beatmap is also updated alongside changes.
if (CurrentBeatmap != null && beatmap.Equals(CurrentBeatmap))
// we don't know in which group the matching new beatmap is, but that's fine - we can keep the previous one for now.
// we are about to modify `Items`, which - if required - will trigger a re-filter,
// which will pick a correct group - if one is present - via `HandleFilterCompleted()`.
RequestSelection(new GroupedBeatmap(CurrentGroupedBeatmap?.Group, matchingNewBeatmap));
{
// The matching beatmap may have been deleted or invalidated in some way since this event was fired.
// Let's make sure we have the most up-to-date realm state of the current beatmap.
var refreshedNewBeatmap = realm.Run(r => r.FindWithRefresh<BeatmapInfo>(matchingNewBeatmap.ID)?.Detach());

Check warning

Code scanning / InspectCode

Access to modified captured variable Warning

Captured variable is modified in the outer scope

if (refreshedNewBeatmap != null)
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
{
matchingNewBeatmap = refreshedNewBeatmap;

// we don't know in which group the matching new beatmap is, but that's fine - we can keep the previous one for now.
// we are about to modify `Items`, which - if required - will trigger a re-filter,
// which will pick a correct group - if one is present - via `HandleFilterCompleted()`.
RequestSelection(new GroupedBeatmap(CurrentGroupedBeatmap?.Group, matchingNewBeatmap));
}
}

Items.ReplaceRange(previousIndex, 1, [matchingNewBeatmap]);
newSetBeatmaps.Remove(matchingNewBeatmap);
Expand Down
Loading