Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add collection transfer logic to beatmap import-as-update flow #19431

Merged
merged 6 commits into from Jul 28, 2022

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Jul 28, 2022

  • If an updated difficulty is in a collection, its hash will be removed and replaced by the updated one (assuming difficulty name matches.
  • If all difficulties in an updated beatmap set were in a collection, all difficulties post-update will be added to the same collection. This handles the case where a user has added a beatmap set to a collection (which is possible from the set header context menu).
  • Depends on Move beatmap collections to realm #19430

Closes #19346

And to answer a question asked elsewhere "why was it necessary to move collections to realm to make this work?", the only answer I have is "i didn't want to pass a CollectionManager in to BeatmapImporter". Also because the deeper I got in beatmap collection code, the more I felt it required a refactor pass.

@peppy peppy added the realm deals with local realm database label Jul 28, 2022
@smoogipoo
Copy link
Contributor

To confirm, the editor will at some point be updated to use the ImportAsUpdate flow or something? Because beatmaps should probably be preserved in the collections they're in after being edited.

@peppy
Copy link
Sponsor Member Author

peppy commented Jul 28, 2022

Yeah, that sounds like a good direction (at very least having BeatmapManager.Save() call the transfer function).


importAfterUpdate.PerformRead(updated =>
{
updated.Realm.Refresh();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to feel about this being done inside this nested context as opposed to realm.Run(r => r.Refresh()); like all the other methods (on master).

But at the end of the day this is still being run on the same context as the former, so I'll let it slide.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured this was safer because as you say, the context might be different otherwise (this ensures it definitely isn't).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
realm deals with local realm database size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating beatmaps removes them from collections
2 participants