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

Take custom bank name length into account when collapsing sample point indicators #31352

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Dec 30, 2024

RFC. Would close #31312.

Not super happy with the performance overhead of this, but this is already a heuristic-based implementation to avoid every-frame .ChildrenOfType<>() calls or similar, so not super sure how to do better. The Array.Contains() check stands out in profiling, but without it the indicators can collapse too eagerly sometimes.

before after
1735556844 1735556860

…t indicators

Would close ppy#31312.

Not super happy with the performance overhead of this, but this is
already a heuristic-based implementation to avoid every-frame
`.ChildrenOfType<>()` calls or similar, so not super sure how to do
better. The `Array.Contains()` check stands out in profiling, but
without it the indicators can collapse *too* eagerly sometimes.
@smoogipoo smoogipoo merged commit ab11117 into ppy:master Dec 30, 2024
9 of 10 checks passed
@bdach bdach deleted the collapse-sample-point-indicators-better branch December 30, 2024 13:02
@rvneXe
Copy link

rvneXe commented Jan 9, 2025

Umm is the video below caused by this PR? Should I make a new issue?

2025-01-09.21-13-31.mp4

@bdach
Copy link
Collaborator Author

bdach commented Jan 9, 2025

i don't see anything wrong there

@rvneXe
Copy link

rvneXe commented Jan 9, 2025

i don't see anything wrong there

like the mechanism looking a little over-reacting. Collapsing even when the input name isn't that wide just rely on "3-characters long so I'm gonna collapse it"

@bdach
Copy link
Collaborator Author

bdach commented Jan 9, 2025

it's doing that because there are 3 indicators on the slider that are almost touching. the thing is collapsing because any longer name causes overlaps. that is intentional.

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.

Sample point indicators do not collapse when the custom name is overlapping
3 participants