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

Continue displaying storyboard even if fully dimmed in specific circumstances #29967

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Sep 23, 2024

RFC.
Closes #9315.
Closes #29867.

Notably, this does nothing about #25075, but I'm not sure what to do with that one in the first place.

…mstances

Closes ppy#9315.
Closes ppy#29867.

Notably, this does nothing about
ppy#25075, but I'm not sure what to do
with that one in the first place.
/// </item>
/// </list>
/// </summary>
private readonly Lazy<bool> storyboardMustAlwaysBePresent;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Lazy usage here is primarily intended to prevent the recalculation of the heuristic conditions described, which can get pretty heavy on maps like https://osu.ppy.sh/beatmapsets/470977#osu/1010030 (measured at least 3ms spent on computing this in profiling per call). That said, it doesn't do that much, so it could be skipped if deemed offensive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for it to not be computed once in LoadComplete() instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Um... I guess you could, but it gets rather finicky, because if you do that you need to start worrying about call order - ShowDimContent is checked in UpdateVisuals() in UserDimContainer which is called in several places there, and one of them is... UserDimContainer.LoadComplete(). So one would have to be careful to compute this before that base call.

@peppy peppy self-requested a review September 27, 2024 07:24
@peppy peppy merged commit 19a96bc into ppy:master Sep 27, 2024
13 checks passed
@bdach bdach deleted the storyboard-full-dim branch September 27, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants