godot: Input.parse_input_event(input_event) crashes Godot on edge buttons

Godot version

4.0.2

System information

Windows 10, Windows 11

Issue description

Currently in my project there are 4 buttons in the main menu. They can be easily navigated with the ui input actions from a controller. However I want to trigger them programmatically as well for other reasons.

The above picture is my implementation of this. It works perfectly except for when a command is pressed to move in a direction that doesn’t have any more buttons.

So if you press “up” while at the first button the game crashes (image below).

Same for pressing “down” while on the last button. Otherwise it works fine.

Godot --verbose logs don’t log anything since it becomes non-responsive immediately. It seems to me it might be calling itself recursively somehow.

Steps to reproduce

In any scene, add a vbox container and at least 2 buttons.
Then add a script to the base node and this code:

	if Input.is_action_just_pressed("move_down") or\
	Input.is_action_just_pressed("move_up") or \
	Input.is_action_just_pressed("move_left") or \
	Input.is_action_just_pressed("move_right"):
		var input_event := InputEventAction.new()
		if Input.is_action_just_pressed("move_down"):
			input_event.action = "ui_down"
		elif Input.is_action_just_pressed("move_up"):
			input_event.action = "ui_up"
		elif Input.is_action_just_pressed("move_left"):
			input_event.action = "ui_left"
		elif Input.is_action_just_pressed("move_right"):
			input_event.action = "ui_right"
		input_event.pressed = true
		Input.parse_input_event(input_event)

Run the scene (make sure one of the buttons is already focused) and try navigating with the input actions and watch the game crash.

Minimal reproduction project

crash_button.zip

About this issue

Original URL
State: closed
Created a year ago
Comments: 27 (22 by maintainers)

Most upvoted comments

I can confirm this on 4.1.dev 64eeb04d2. However, I don’t get a crash but an infinite loop. Getting a backtrace from gdb during the infinite loop returns:

(gdb) bt
#0  0x0000000002fd5e40 in Control::_window_find_focus_neighbor (this=this@entry=0xc4086b0, p_dir=..., p_at=p_at@entry=0xc3e5d20, p_points=p_points@entry=0x7fffffffa3a0, p_min=p_min@entry=648, r_closest_dist=@0x7fffffffa36c: 10000000, r_closest=r_closest@entry=0x7fffffffa370) at scene/gui/control.cpp:2288
#1  0x0000000002fd5e30 in Control::_window_find_focus_neighbor (r_closest=<optimized out>, r_closest_dist=<optimized out>, p_min=<optimized out>, p_points=<optimized out>, p_at=0xc3e5d20, p_dir=..., this=<optimized out>) at scene/gui/control.cpp:2234
#2  Control::_window_find_focus_neighbor (this=this@entry=0xc4086b0, p_dir=..., p_at=p_at@entry=0xc403920, p_points=p_points@entry=0x7fffffffa3a0, p_min=p_min@entry=648, r_closest_dist=@0x7fffffffa36c: 10000000, r_closest=r_closest@entry=0x7fffffffa370) at scene/gui/control.cpp:2286
#3  0x0000000002fde7ab in Control::_window_find_focus_neighbor (r_closest=0x7fffffffa370, r_closest_dist=@0x7fffffffa36c: 10000000, p_min=648, p_points=0x7fffffffa3a0, p_at=0xc403920, p_dir=..., this=0xc4086b0) at scene/gui/control.cpp:2231
#4  Control::_get_focus_neighbor (this=this@entry=0xc4086b0, p_side=p_side@entry=SIDE_BOTTOM, p_count=p_count@entry=0) at scene/gui/control.cpp:2228
#5  0x0000000002ef6ab1 in Viewport::_gui_input_event (this=this@entry=0xc3749a0, p_event=...) at scene/main/viewport.cpp:2150
#6  0x0000000002f0a0c1 in Viewport::push_input (this=this@entry=0xc3749a0, p_event=..., p_local_coords=p_local_coords@entry=false) at scene/main/viewport.cpp:2862
#7  0x0000000002f2feed in Window::_window_input (this=0xc3749a0, p_ev=...) at scene/main/window.cpp:1385
#8  0x0000000002f474ca in call_with_variant_args_helper<Window, Ref<InputEvent> const&, 0ul> (r_error=..., p_args=<optimized out>, p_method=<optimized out>, p_instance=<optimized out>) at ./core/variant/binder_common.h:293
#9  call_with_variant_args<Window, Ref<InputEvent> const&> (r_error=..., p_argcount=<optimized out>, p_args=<optimized out>, p_method=<optimized out>, p_instance=<optimized out>) at ./core/variant/binder_common.h:407
#10 CallableCustomMethodPointer<Window, Ref<InputEvent> const&>::call (this=<optimized out>, p_arguments=<optimized out>, p_argcount=<optimized out>, r_return_value=..., r_call_error=...) at ./core/object/callable_method_pointer.h:104
#11 0x0000000000a90c4f in DisplayServerX11::_dispatch_input_event (this=<optimized out>, p_event=...) at platform/linuxbsd/x11/display_server_x11.cpp:3601
#12 0x0000000004b798c1 in Input::_parse_input_event_impl (this=this@entry=0x7d84750, p_event=..., p_is_emulated=p_is_emulated@entry=false) at core/input/input.cpp:690
#13 0x0000000004b7c4a5 in Input::flush_buffered_events (this=0x7d84750) at ./core/templates/list.h:115
#14 0x0000000000a94e81 in DisplayServerX11::process_events (this=0x8458310) at platform/linuxbsd/x11/display_server_x11.cpp:4661
#15 0x0000000000a70000 in OS_LinuxBSD::run (this=this@entry=0x7fffffffcac0) at platform/linuxbsd/os_linuxbsd.cpp:885
#16 0x0000000000a629da in main (argc=<optimized out>, argv=0x7fffffffd028) at platform/linuxbsd/godot_linuxbsd.cpp:73

PS: In the MRP, remember to use WASD instead of arrow keys to navigate the UI (otherwise, built-in actions are used).

Calinou on May 13, 2023

To clarify, my suggestion for the correct code in this case is:

if event.is_action_pressed(&"move_down") or\
	event.is_action_pressed(&"move_up") or \
	event.is_action_pressed(&"move_left") or \
	event.is_action_pressed(&"move_right"):
		var input_event := InputEventAction.new()
		if event.is_action_pressed(&"move_down"):
			input_event.action = &"ui_down"
		elif event.is_action_pressed(&"move_up"):
			input_event.action = &"ui_up"
		elif event.is_action_pressed(&"move_left"):
			input_event.action = &"ui_left"
		elif event.is_action_pressed(&"move_right"):
			input_event.action = &"ui_right"
		input_event.pressed = true
		Input.parse_input_event(input_event)

AThousandShips on May 14, 2023

The class reference should probably be amended to mention what’s been said on this issue 🙂

Calinou on May 14, 2023

Sorry, I’m not quite sure I understand, how would I go about fixing this then?

The previous action can be released manually.

func _unhandled_input(event):
	if Input.is_action_just_pressed("move_down") or\
	Input.is_action_just_pressed("move_up") or \
	Input.is_action_just_pressed("move_left") or \
	Input.is_action_just_pressed("move_right"):
		var input_event := InputEventAction.new()
		var old_action : StringName
		if Input.is_action_just_pressed("move_down"):
			input_event.action = "ui_down"
			old_action = &"move_down"
		elif Input.is_action_just_pressed("move_up"):
			input_event.action = "ui_up"
			old_action = &"move_up"
		elif Input.is_action_just_pressed("move_left"):
			input_event.action = "ui_left"
			old_action = &"move_left"
		elif Input.is_action_just_pressed("move_right"):
			input_event.action = "ui_right"
			old_action = &"move_right"
		input_event.pressed = true
		Input.action_release(old_action)
		Input.parse_input_event(input_event)

Rindbee on May 14, 2023

This is expected behavior as it is incorrect usage of the provided APIs. _unhandled_input() is called once per input event but the user code is not checking if the current event is the one that is relevant to test against so it is always emitting a new event. This means the code will run in a loop forever and maybe hit a non-recursive mutex lock depending on the engine’s version.

lmurray on May 14, 2023

State records are kept forever, why should that change?

AThousandShips on May 14, 2023

The reason the action_state isn’t cleared is because that’s how is_action_just_released is made possible, otherwise you couldn’t track a released event and the time it’s released, it shouldn’t really affect performance as it’s a hash map using StringName (especially with memory as HashMap never shrinks, i.e. never removes capacity)

Currently, action_state is a mapping of the states of all actions that have occurred. And it’s used in various is_*_pressed() functions. The results of these functions should vary with input events. This means that action_state can rebuild the mapping before a new event propagates.

And if it is treated as a mapping of the state of the corresponding actions during an input event (An event maps multiple actions‘ states), it will work fine and avoid such issues, because we can clear action_state when the current event switches to the next event.

Rindbee on May 14, 2023

It does happen if you release the event immediately after in any way.

The freeze happens immediately as soon as an event is parsed in a direction with no more ui elements, regardless of if it’s released or not.

Vennnot on May 13, 2023