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

Fix crash when adding mania notes right after changing timing point #28944

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Jul 19, 2024

Closes #28938.

This is related to reloading the composer on timing point changes in scrolling rulesets. The lack of unsubscription from this would cause blueprints to be created for disposed composers via the hitObjectAdded() flow.

The following line looks as if a sync load should be forced on a newly created placement blueprint:

placementBlueprintContainer.Child = CurrentPlacement = blueprint;

however, it is not the case if the parent (placementBlueprintContainer) is disposed, which it would be in this case. Therefore, the blueprint stays NotLoaded rather than Ready, therefore it never receives its DI dependencies, therefore it dies on an EditorBeatmap nullref.

Closes ppy#28938.

This is related to reloading the composer on timing point changes in
scrolling rulesets. The lack of unsubscription from this would cause
blueprints to be created for disposed composers via the
`hitObjectAdded()` flow.

The following line looks as if a sync load should be forced on a newly
created placement blueprint:

    https://github.com/ppy/osu/blob/da4d37c4aded5e10d0a65ff44a08a886e3897e19/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs#L364

however, it is not the case if the parent
(`placementBlueprintContainer`) is disposed, which it would be in this
case. Therefore, the blueprint stays `NotLoaded` rather than `Ready`,
therefore it never receives its DI dependencies, therefore it dies on
an `EditorBeatmap` nullref.
@bdach bdach added the next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! label Jul 19, 2024
@peppy peppy merged commit fb5a1ec into ppy:master Jul 19, 2024
7 of 11 checks passed
@ppy-sentryintegration
Copy link

Sentry issue: OSU-1F9B

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:editor next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! size/XS type:reliability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when adding mania notes right after changing timing point
2 participants