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 various improvements to OptionButton #57330

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

aekobear
Copy link
Contributor

Fixed several bugs from #6558

  • OptionButton can now be set to no selection by calling optionButton.select(-1)
  • The Selected property in the editor can now be set to -1 for no selection
    • The Selected property can now be restored to default (which is -1)
  • When a selected item is deleted, the selection is set to no selection (-1)
    • This also fixes the scenarios where all items are deleted and where items are added after being deleted

I also found an undocumented bug introduced here: #54647

The PopupMenu had been edited to use IDs starting at 0 in some places, but in other places (and in OptionButton) it was expected to use IDs starting at 1. This caused a bug where when adding OptionButton items from the inspector, both of the first two items would have ID = 1

Rather than reverting, I fully converted PopupMenu and OptionButton to use 0-based IDs. This matches the documentation, which claims that IDs are set by index (which starts with 0). It also improves some dissonance where "0" could be passed in as a valid object ID but was also being used to represent "no selection" when calling get_selected_id(). Now, -1 represent no selection in both the index and the ID flows.

@aekobear aekobear requested review from a team as code owners January 27, 2022 23:41
@aekobear aekobear closed this Jan 27, 2022
@aekobear aekobear reopened this Jan 27, 2022
@aekobear aekobear changed the title Fix various OptionButton selection bugs Fix various OptionButton improvements Jan 27, 2022
@Calinou Calinou added this to the 4.0 milestone Jan 27, 2022
@aekobear aekobear changed the title Fix various OptionButton improvements Make various improvements to OptionButton Jan 27, 2022
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think it would be worth mentioning the new behavior in the select docs too:

Passing `[code]-1[/code]` as index deselects any currently selected item.

- Allow OptionButton selection to be set to -1 to signify no selection, both via API and in the editor.
- Reset OptionButton selection to -1 when the selected item has been removed.
- Fully convert PopupMenu to a zero-based ID system, which improves an inconsistency in generated IDs when making new items in the editor.
@aekobear
Copy link
Contributor Author

Looks good to me. I think it would be worth mentioning the new behavior in the select docs too:

Passing `[code]-1[/code]` as index deselects any currently selected item.

Good point- updated

@akien-mga akien-mga merged commit ffa566c into godotengine:master Jan 28, 2022
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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.

3 participants