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

BaseButton.button_pressed doesn't work as documented #97206

Open
noidexe opened this issue Sep 19, 2024 · 1 comment
Open

BaseButton.button_pressed doesn't work as documented #97206

noidexe opened this issue Sep 19, 2024 · 1 comment

Comments

@noidexe
Copy link
Contributor

noidexe commented Sep 19, 2024

Tested versions

  • Reproducible in: v4.3.stable.official [77dcf97]

System information

Godot v4.3.stable - Windows 10.0.22631 - GLES3 (Compatibility) - AMD Radeon RX 6700S (Advanced Micro Devices, Inc.; 32.0.11037.4004) - AMD Ryzen 9 6900HS with Radeon Graphics (16 Threads)

Issue description

The documentation for BaseButton.button_pressed has a note that says:

Note: Setting button_pressed will result in toggled to be emitted. If you want to change the pressed state without emitting that signal, use set_pressed_no_signal().

To me this read as "Setting button_pressed will always result in toggled to be emitted".
However the actual behavior is that the toggled signal is only emitted when button_pressed is set to a different value (!button_pressed).

You can see in line 217 the function returns early if status.pressed == prev_pressed and _toggled(status.pressed) is never reached

void BaseButton::set_pressed(bool p_pressed) {
bool prev_pressed = status.pressed;
set_pressed_no_signal(p_pressed);
if (status.pressed == prev_pressed) {
return;
}
if (p_pressed) {
_unpress_group();
if (button_group.is_valid()) {
button_group->emit_signal(SceneStringName(pressed), this);
}
}
_toggled(status.pressed);
}

Users might think that setting button_pressed in code will force the emission of the toggled signal, even if they set it to the same value.

I'm not sure if the documentation or the implementation need to be changed. I assume it makes sense that "toggled" is only emitted when there is an actual toggle of the button_pressed boolean and at the same time changing the implementation is a breaking change.

Steps to reproduce

  • Create a button
  • Connect any callback to its "toggled" signal
  • In a script set the button's "button_pressed" property to the same value it already has

Expected: the callback gets called
Actual: the callback doesn't get called

Minimal reproduction project (MRP)

button_toggled_same_state_no_signal_mrp.zip

@akien-mga
Copy link
Member

s/Setting/Changing the value of/ basically and we're good :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants