h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2025-02-20 09:03:00 +08:00
Code Issues Releases Wiki Activity

Actually fix realm selection retention regression

Browse Source
This commit is contained in:
Dean Herbert 2023-12-19 18:10:55 +09:00
parent 7e9c1b2acb
commit 8f5d21dc70
No known key found for this signature in database
2 changed files with 22 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs
View File

@ -464,8 +464,6 @@ namespace osu.Game.Tests.Visual.SongSelect
manager.Import(testBeatmapSetInfo);
}, 10);
AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh()));
AddUntilStep("has selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID, () => Is.EqualTo(originalOnlineSetID));
Task<Live<BeatmapSetInfo>?> updateTask = null!;
@ -478,8 +476,6 @@ namespace osu.Game.Tests.Visual.SongSelect
});
AddUntilStep("wait for update completion", () => updateTask.IsCompleted);
AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh()));
AddUntilStep("retained selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID, () => Is.EqualTo(originalOnlineSetID));
}

24
osu.Game/Screens/Select/BeatmapCarousel.cs
View File

@ -269,8 +269,28 @@ namespace osu.Game.Screens.Select
if (changes == null)
return;
foreach (int i in changes.InsertedIndices)
removeBeatmapSet(sender[i].ID);
var removeableSets = changes.InsertedIndices.Select(i => sender[i].ID).ToHashSet();
// This schedule is required to retain selection of beatmaps over an ImportAsUpdate operation.
// This is covered by TestPlaySongSelect.TestSelectionRetainedOnBeatmapUpdate.
//
// In short, we have specialised logic in `beatmapSetsChanged` (directly below) to infer that an
// update operation has occurred. For this to work, we need to confirm the `DeletePending` flag
// of the current selection.
//
// If we don't schedule the following code, it is possible for the `deleteBeatmapSetsChanged` handler
// to be invoked before the `beatmapSetsChanged` handler (realm call order seems non-deterministic)
// which will lead to the currently selected beatmap changing via `CarouselGroupEagerSelect`.
//
// We need a better path forward here. A few ideas:
// - Avoid the necessity of having realm subscriptions on deleted/hidden items, maybe by storing all guids in realm
// to a local list so we can better look them up on receiving `DeletedIndices`.
// - Add a new property on `BeatmapSetInfo` to link to the pre-update set, and use that to handle the update case.
Schedule(() =>
{
foreach (var set in removeableSets)
removeBeatmapSet(set);
});
}
private void beatmapSetsChanged(IRealmCollection<BeatmapSetInfo> sender, ChangeSet? changes)