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

Remove bindable from CircularProgress #6199

Merged
merged 1 commit into from Feb 27, 2024

Conversation

EVAST9919
Copy link
Contributor

There's zero to no reason to have it Bindable imo.
Performance impact is tiny, though in cases such as TestSceneVertexUploadPerformance it's quite noticeable (which is unrealistic, but still).

master pr
master pr

@frenzibyte
Copy link
Member

I would look more into whether we can change ValueChangedEvent to a struct rather than slowly turning away from using Bindable in UI components, but turns out it's been attempted before in #6114 and things aren't going good in there.

I think this particular instance makes more sense to not be a bindable though, since it's a continuously changing component, rather than being dependant on user manually changing it (i.e. most other UI components).

@peppy peppy merged commit 1d48749 into ppy:master Feb 27, 2024
21 checks passed
@EVAST9919 EVAST9919 deleted the circular-progress-no-bindable branch February 27, 2024 04:59
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.

None yet

3 participants