godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.14k stars 93 forks source link

OptionButton: Change the value returned in the signals "item_focused" and "item_selected" #2328

Open ghsoares opened 3 years ago

ghsoares commented 3 years ago

Describe the project you are working on

A plugin that uses a OptionButton

Describe the problem or limitation you are having in your project

It seens that the OptionButton's "item_focused" and "item_selected" returns the index of the item, it's seens a bit wrong, as the desired value was actually it's ID, not it's index. This was driving me crazy as I was not understanding why the item's id was wrong.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Change the value returned from these signals from the item's index to the item's id, or at least add the id as a argument of the signal. This can make easier to get the selected or focused item's id right out of the box, without needing to storing a extra reference of the OptionButton, or passing it as the signal extra argument.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Change the signature of the signals from item_focused(index: int) and item_selected(index: int) to item_focused(item_id: int) and item_selected(item_id: int) or item_focused(index: int, item_id: int) and item_selected(index: int, item_id: int)

If this enhancement will not be used often, can it be worked around with a few lines of script?

Well, keeping the reference of the OptionButton to get the item's id is possible, but in a plugin's UI where the OptionButton is temporary, that is refreshed in some conditions (like the node selectiong changes and a new OptionButton is instantiated with the node's option) a extra variable to just access one function seens a bit strange to me.

Is there a reason why this should be core and not an add-on in the asset library?

One could create a new class derived from the OptionButton and override how the item is selected and focused, but it would be nice to have these features right out of the box.

zinnschlag commented 3 years ago

Can't you just add the OptionButton as an additional argument when connecting the signal? Example: https://godotengine.org/qa/64575/binding-arguments-to-built-in-signal. Or maybe just add the ID, so that you can avoid an additional lookup?

Seems a fairly simple workaround. Better than breaking existing code for potentially a lot of users.

fuzorsilverbolt commented 7 months ago

I second this feature request. I also got confused by the values being returned by OptionButton in that it was sending idx instead of the parameter ID. I think the idx should be hidden from the user and only the ID parameter for the option should be sent to the signal and should be a required field.