search

godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.43k stars 20.24k forks source link

Button group signal "pressed" not triggered #64604

Closed floppyhammer closed 2 years ago

floppyhammer commented 2 years ago

Godot version

v3.5.stable.official [991bb6ac7]

System information

Windows 11, GLES3

Issue description

Button group signal "pressed" cannot be triggered when any button in the group is pressed.

Steps to reproduce

Add some buttons to a button group, connect the "pressed" signal, click any button in the group.

Minimal reproduction project

[Uploading ButtonGroupSignal.zip…]()

floppyhammer commented 2 years ago

What's wrong with Github? I can't upload the test project even when I'm on another computer.

image
kleonc commented 2 years ago

I'd guess your buttons are not toggleable? Seems like ButtonGroup works only for toggleable buttons: https://github.com/godotengine/godot/blob/35cfaafda8073f700c9d2fe42a43d3d81eaaea67/doc/classes/ButtonGroup.xml#L6-L9

I guess this signal should be called something like toggled_on in the first place to avoid confusion... or just toggled and be also emitted for the button being toggled off. But this can't be changed in 3.x, it would break compatibility. However maybe ButtonGroup could be changed/improved in 4.0? :thinking:

floppyhammer commented 2 years ago

@kleonc You're right. Making all member buttons togglable does work. Is there any reason for all member buttons to be togglable? I don't see an obvious reason for this limitation.

kleonc commented 2 years ago

Is there any reason for all member buttons to be togglable?

I can be only guessing, seems like it was simply made specifically for radio buttons (only one button in a group can be pressed at once).

I don't see an obvious reason for this limitation.

Same, it's rather a design choice. I think potential changes are a subject for a proposal though. ButtonGroup seems to work as intended (whether we like it or not).

For the current state maybe the docs could/should be improved to be a little more clear about the required togglebility? :thinking:

KoBeWi commented 2 years ago

This is already documented.

Group of [Button]. All direct and indirect children buttons become radios. Only one allows being pressed.[member [BaseButton.toggle_mode] should be [code]true[/code].