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

Make OptionButton::selected read-only #95378

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

raulsntos
Copy link
Member

This PR makes the selected property read- only, the select method can be used to set the value instead.

The select method is already exposed and does the same thing as the property's setter _select_int, which is now unused so it was removed.

Alternative

Using the select method as the setter for the selected property is technically possible, since both methods have the same signature, and would prevent breaking compatibility. Unfortunately, this means the select method becomes hidden and I'm not sure that's what we want.

This PR makes the `selected` property read- only, the `select` method can be used to set the value instead.

The `select` method is already exposed and does the same thing as the property's setter `_select_int`, which is now unused so it was removed.
@KoBeWi
Copy link
Member

KoBeWi commented Aug 12, 2024

This makes it impossible to change the property in the inspector or load it from scene. I don't think hiding select() is a big deal if the method will still be usable (so it won't break any code). Setting property does pretty much the same anyway.

@raulsntos
Copy link
Member Author

I'm fine with that, but keep in mind it will also remove the documentation for the select() method since it disappears from the class reference. I can also expose a new set_selected() method for the setter that just calls select() in its implementation. But I don't have a strong preference, so whatever the team prefers.

@KoBeWi
Copy link
Member

KoBeWi commented Aug 17, 2024

it will also remove the documentation for the select() method since it disappears from the class reference.

You can copy it to selected description.

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