Skip to content

Fix stutter on returning to song select when updating large beatmap sets - #38514

Open
SevenSec114 wants to merge 6 commits into
ppy:masterfrom
SevenSec114:master
Open

Fix stutter on returning to song select when updating large beatmap sets#38514
SevenSec114 wants to merge 6 commits into
ppy:masterfrom
SevenSec114:master

Conversation

@SevenSec114

@SevenSec114 SevenSec114 commented Aug 4, 2026

Copy link
Copy Markdown

Addresses the stutter reported in #35129, whose regression introducer was bisected to the per-difficulty realm check added in #34914.

When a beatmap set is updated (e.g. returning from gameplay with a large set, or exiting the editor), the carousel's replace handling previously:

This is unnecessary for the items that are not the current selected beatmap since the crash is only by the RequestSelection given the invalid beatmap.

Comment thread osu.Game/Screens/Select/BeatmapCarousel.cs Fixed
@bdach

bdach commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator
  • Did you write this yourself? Was there "AI" assistance here?
  • Test failures will be relevant (it appears hiding difficulties no longer works - will let you figure out if this kills your optimisation or not).

Comment thread osu.Game/Screens/Select/BeatmapCarousel.cs Fixed
@SevenSec114

Copy link
Copy Markdown
Author

My bad for the test failure. I ran the tests I thought were affected and they passed, so I relied on the github CI for the rest.

Also this change is the actual performance fix:

-Items.ReplaceRange(previousIndex, 1, [matchingNewBeatmap]);
+Items.ReplaceRange(previousIndex, oldSetBeatmaps.Count, newBeatmaps);

but I forgot to flip the rest logic in HandleItemsChanged. Will fix.

Code is written by myself, I spent a lot to figure out the logic. But before committing I found something like comments in my own language may be hard to understand so I polished a lot.

@SevenSec114
SevenSec114 marked this pull request as draft August 4, 2026 16:19
@bdach

bdach commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Unsure why this PR is in draft, will probably not attempt review until it's out of draft.

@SevenSec114

Copy link
Copy Markdown
Author

I'm now still working on it to find if there is any other issues appeared due to the logic changing, because I don't want it happened one more time. Please give me more time🙏

{
// 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());
Comment thread osu.Game/Screens/Select/BeatmapCarousel.cs Fixed
@pull-request-size pull-request-size Bot added size/M and removed size/L labels Aug 5, 2026
[Test]
public void TestBeatmapSetReplacedWithDeletedCurrentBeatmap()
{
int targetSetIndex = 0;
@SevenSec114

Copy link
Copy Markdown
Author

I found myself over-engineered. But I'm not sure if this test should be removed later since the crash is caused by the assertion in osu.Game/Screens/Select/FooterButtonOptions.cs that can be solved in other place...

beatmap = realm.Run(r => r.Find<BeatmapInfo>(workingBeatmap.Value.BeatmapInfo.ID)!.ToLive(realm));

@SevenSec114
SevenSec114 marked this pull request as ready for review August 5, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editor: error popup when changing game mode of beatmap

3 participants