GUI: Inconsistent order of SpinBox value change and control pressing / item selection signals

[dalexeev]

Godot version

v4.1.1.stable.official [bd6af8e]

System information

Kubuntu 23.04

Issue description

There is an inconsistency in the order of losing focus / applying a field value (LineEdit, TextEdit, SpinBox) and receiving focus / pressing a control (Button, Tree, ItemList).

In the case of Button, the order of signals is expected: first the field value is changed, then the button is pressed.

In the case of Tree and ItemList, the signal order is reversed, which is inconvenient and leads to bugs in applying changes to the current item instead of the previous one (see below). You can use deferred signals as a workaround, but in my opinion this is a hack that could cause other bugs.

_on_spin_box_value_changed 10
_on_button_pressed

_on_tree_cell_selected
_on_spin_box_value_changed 20
_on_tree_cell_selected_deferred

_on_item_list_item_selected 1
_on_spin_box_value_changed 30
_on_item_list_item_selected_deferred 1

See also:

• FPS value in AnimatedSprite (2D) applied to wrong animation when switching to another #26979
• Apply sprite frames editor FPS value correctly upon _animation_select #28729
• Unexpected behavior on SpriteFrames animation FPS config #79445
• SpriteFrames Editor: Fix FPS applied to two animations when switching animation #79692

Steps to reproduce

Change the value of the SpinBox field (without pressing Enter) and try clicking on the Button, on the Tree item and on the ItemList item.

Minimal reproduction project

focus_order_bug.zip

[dalexeev]

Looks like this is a SpinBox issue (in the case of the Button it works correctly, since default action mode is on releasing the button). Are there any reasons why the SpinBox uses the deferred signal?

_on_spin_box_line_edit_focus_exited
_on_button_focus_entered
_on_spin_box_value_changed 10
_on_button_pressed

_on_spin_box_line_edit_focus_exited
_on_tree_focus_entered
_on_tree_cell_selected
_on_spin_box_value_changed 20
_on_tree_cell_selected_deferred

_on_spin_box_line_edit_focus_exited
_on_item_list_focus_entered
_on_item_list_item_selected 1
_on_spin_box_value_changed 30
_on_item_list_item_selected_deferred 1

godot/scene/gui/spin_box.cpp

Lines 387 to 390 in 0c2144d

	line_edit->connect("text_submitted", callable_mp(this, &SpinBox::_text_submitted), CONNECT_DEFERRED);
	line_edit->connect("focus_entered", callable_mp(this, &SpinBox::_line_edit_focus_enter), CONNECT_DEFERRED);
	line_edit->connect("focus_exited", callable_mp(this, &SpinBox::_line_edit_focus_exit), CONNECT_DEFERRED);
	line_edit->connect("gui_input", callable_mp(this, &SpinBox::_line_edit_input));

[Rindbee]

I quite agree.

• If it just delays the timing of showing/refreshing the UI, it has no side effects.
• If this delay affects the timing of data changes, it will cause data synchronization problems.

If both the data and the UI are updated in the receiving method, we can just defer the UI update call in the receiving method.

[stoofin]

I just ran into this. A SpinBox that's part of a Dialog will process losing focus after its ancestor Dialog's 'canceled' signal if you close that Dialog with the X button (in my case, even if that 'canceled' signal handler was itself deferred). So trying to reset the SpinBox's value in the 'canceled' handler won't work as it will get overwritten by its text. A dirty workaround I've found is to do something weird like spinbox->get_line_edit()->set_text("null"); spinbox->apply(); to get _update_text to run early and have the value change stick, but I don't like it.