Add way to add/remove custom beatmap samples to setup screen #36190
+254
−25
Conversation
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
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.
bdach
force-pushed
the
add-custom-samples-via-setup
branch
from
d1f2e39 to
affe295
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.