Add way to add/remove custom beatmap samples to setup screen#36190
Merged
peppy merged 4 commits intoppy:masterfrom Jan 16, 2026
Merged
Add way to add/remove custom beatmap samples to setup screen#36190peppy merged 4 commits intoppy:masterfrom
peppy merged 4 commits intoppy:masterfrom
Conversation
Among others, this features a scary-looking change wherein `WorkingBeatmap` now exposes a `RealmAccess` via `IStorageResourceProvider`. This is necessary to make `RealmBackedResourceStore` actually start firing callbacks when the edited beatmap's skin is modified by adding new samples to it.
This is invariably a test fix but also a valid change to make in isolation. In short, the problem is thus: Now that the beatmap skin is hooked up to realm to receive notifications about changed files (which is required for correctly handling custom samples), tests started failing, because of the following sequence of events - Saving a brand new `.osu` to a beatmap set causes `RealmBackedResourceStore` to fire subscription callbacks because a new file was added to the set - This fires `RealmBackedResourceStore.CacheInvalidated` - Which fires `EditorBeatmapSkin.BeatmapSkinChanged` - Which would previously fire `EditorBeatmap.SaveState()` and as such mark the beatmap dirty / modified. In this scenario this is gratuitous. There's no need to be raising save states here, a new `.osu` was added to the set that is in a consistent saved state and nothing actually changed in the beatmap. However it does not appear sane to attempt to circumvent this with conditional guards or something, because in cases where files are added/removed from the set, *there isn't really any reason to take save states anyway*. The change handler only deals with the `.osu`, any modifications to any of the other files cannot be undone anyway. Therefore, only keep the state save to the one change to beatmap skin that *can* actually be sanely undone which is changing combo colours.
d1f2e39 to
affe295
Compare
peppy
approved these changes
Jan 16, 2026
Comment on lines
+104
to
+105
| if (beatmapSkin is LegacyBeatmapSkin skin) | ||
| BeatmapSkin = new EditorBeatmapSkin(this, skin); |
Member
There was a problem hiding this comment.
The biggest issue I have with this whole change is this part here (the fact we're not using ISkin and always typing a LegacyBeatmapSkin for beatmaps).
It's not really the fault of this PR and it might even be resolved by a naming change. Just going to turn a blind eye here for now.
|
Goated feature, been wanting this for a while |
This was referenced Jan 19, 2026
bdach
added a commit
to bdach/osu
that referenced
this pull request
Jan 22, 2026
Missed in ppy#36190. Caught through CI failures like https://github.com/ppy/osu/actions/runs/21238110652/job/61110112412?pr=36404#step:5:21 https://github.com/ppy/osu/actions/runs/21216676973/job/61039650053#step:5:21 I'd hope this is enough and I don't have to dig further.
bdach
added a commit
to bdach/osu
that referenced
this pull request
Jan 22, 2026
Missed in ppy#36190. Caught through CI failures like https://github.com/ppy/osu/actions/runs/21238110652/job/61110112412?pr=36404#step:5:21 https://github.com/ppy/osu/actions/runs/21216676973/job/61039650053#step:5:21 I'd hope this is enough and I don't have to dig further.
This was referenced Jan 28, 2026
This was referenced Feb 9, 2026
This was referenced Mar 4, 2026
This was referenced Mar 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
2025-12-31.12-15-10.mov
Closes #35370.
I'm treating this as a RFC tier PR and not something that's necessarily fleshed out because this is treading a bunch of new ground with regards as to what the editor needs to handle and I'd like to see general opinions on where this has headed first.
Of note, the flow in
RealmBackedResourceStorethat was supposed to ensure that skin changed events fire when the skin's set of files changes was not correctly hooked up for beatmap skins and this PR attempts to hook it up.Commit messages attempt to provide more insight to changes that you will likely find otherwise strange.