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 stutter on intensive storyboards when entering break time with 100% background dim #31506

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

peppy
Copy link
Member

@peppy peppy commented Jan 14, 2025

This avoids piled-up overhead when entering break time. It's not great, but it is what we need for now to avoid weirdness.

Addresses #31465.

peppy added 2 commits January 14, 2025 16:22
This avoids piled-up overhead when entering break time. It's not great,
but it is what we need for now to avoid weirdness.
@bdach bdach self-requested a review January 14, 2025 08:25
@bdach
Copy link
Collaborator

bdach commented Jan 14, 2025

I got this freak crash stack trace in testing this when messing with the storyboard toggle during player load:

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at osu.Game.Storyboards.Drawables.DrawableStoryboard.get_DrawScale() in /home/dachb/Documents/opensource/osu/osu.Game/Storyboards/Drawables/DrawableStoryboard.cs:line 38
   at osu.Framework.Graphics.Drawable.get_IsPresent()
   at osu.Framework.Graphics.Drawable.set_AlwaysPresent(Boolean value)
   at osu.Game.Screens.Play.DimmableStoryboard.<LoadComplete>b__11_0(ValueChangedEvent`1 show) in /home/dachb/Documents/opensource/osu/osu.Game/Screens/Play/DimmableStoryboard.cs:line 85
   at osu.Game.Screens.Play.DimmableStoryboard.LoadComplete() in /home/dachb/Documents/opensource/osu/osu.Game/Screens/Play/DimmableStoryboard.cs:line 72

Feels borderline out of scope, but maybe worth adding some safeties here and there to prevent this from being hit...

Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

see above

@peppy
Copy link
Member Author

peppy commented Jan 14, 2025

That's painful. I've applied band-aid..

Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

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

it's a stop-gap, but it'll do

@bdach bdach merged commit 614243f into ppy:master Jan 15, 2025
9 of 10 checks passed
@peppy peppy deleted the fix-storyboard-break-overhead branch January 17, 2025 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants