`Input.is_action_just_pressed` dropping inputs at low framerates

[vpellen]

Godot version

v4.0.rc2.official [d2699dc]

Issue description

I chased something down a rabbit hole again.

So my understanding of Input.is_action_just_pressed was that it returned true if there was an input event that came in at the beginning of a given frame. This, generally, is true. I was concerned about doubling up in heavy load cases with multiple physics frames, but that turned out to be unwarranted: It returns true for the first _physics_process of the frame, but false for all others, and it returns true in that frame's _process. All good, no problems here, grats to whoever wrote that code. Things got weird when I decided to clamp the FPS and physics ticks to low values.

So here's the best way I can describe the behaviour: Input._is_action_just_pressed seems to return true only when the action is actually being held down. Inversely, Input._is_action_just_released only returns true when an action is not being held.

I realize that may sound blistering obvious, but essentially, here's a sequence of events that can happen:

Input processing begins
An event is found stating an action is pressed
An event is found stating that same action is released
the frame starts
Input.is_action_just_pressed is called
The action was pressed prior to the current frame
However, the action is not currently being pressed
Thus, Input.is_action_just_pressed returns false

Essentially, actions being released clears the criteria for is_just_pressed. Curiously, the inverse is also true: If you release-and-press-again between frames, it you won't get a call to Input.is_action_just_released.

So here's what I think is happening without actually trying to dredge through the source:

The impression I'm getting is that Input.is_action_just_pressed and Input.is_action_just_released is checking for "current" values, and them comparing those values to what they were the last time _process or _physics_process was called. This works in most cases, but it can break down if the internal state of an action toggles an even number of times between frames. Admittedly, this is probably not that critical an issue, but it's possible that in certain situations it could lead to dropped inputs if people are doing just checks on action presses that are faster than the current framerate.

Edit: Upon further investigation, I don't think pressed and released are based solely on the state of the button last frame, but there still does seem to be a constraint where just-pressed and just-released aren't triggering unless the action is also up/down.

Again, I don't know what the source looks like, but I feel like ideally you'd have pressed and released be independent boolean sets that are cleared at the end of each frame and then set based on whatever input events were just received. You'd occasionally get delayed presses, and sometimes you'd have pressed and released both returning true on the same frame, but you'd never have dropped inputs. It might also provide a path to dealing with the infamous mouse wheel events.. but I get ahead of myself.

If I've learned only one thing from this, it's that I really should be putting my input checks in _unhandled_input instead of _physics_process.

Steps to reproduce

The clearest way I've found to observe this in action is with aggressive use of print statements and the FPS limiter and physics ticks set to 1. That way you can easily press and release an action in the same frame, within the frame.

MRP

input_pressed.zip

So this is a crude tool that monitors the input of ui_accept (bound to space/enter by default I believe) and prints the output at a framerate of 1. If you don't want to bother downloading, the relevant script is here:

extends Node

func _init():
	Engine.max_fps = 1

func _input(event):
	var frame := Engine.get_frames_drawn()
	if event.is_action_pressed("ui_accept"):
		print("EVENT: \"ui_accept\" pressed")
	if event.is_action_released("ui_accept"):
		print("EVENT: \"ui_accept\" released")

var input_cache : bool = false

func _process(delta):
	var frame := Engine.get_frames_drawn()
	var pressed := Input.is_action_pressed("ui_accept")
	var just_pressed := Input.is_action_just_pressed("ui_accept")
	var just_released := Input.is_action_just_released("ui_accept")
	print("--- Frame %s ---" % frame)
	print("Last frame pressed: %s" % input_cache)
	print("Input.is_action_pressed(\"ui_accept\") == %s" % pressed)
	print("Input.is_action_just_pressed(\"ui_accept\") == %s" % just_pressed)
	print("Input.is_action_just_released(\"ui_accept\") == %s" % just_released)
	print("\n")
	input_cache = pressed

[lawnjelly]

Yes, I noticed this today when working on input with low frame / tick rates.

I'm not sure it is strictly speaking a bug in a sense, because what causes it is if the input is pressed and released within the physics tick / frame. At the time of inspection it is not pressed, therefore doesn't register as just pressed. But it does register as just released, because the state is released.

If it reported as "just pressed" some users may think it is actually pressed at the time.

EDIT: Actually thinking about it, although it is not a "bug", it makes handling input more difficult. I'll see if there is a way to fix this in a sensible way. Missing clicks / key presses because they happened too "fast" is not good for keyboard etc. Although I don't know whether this could cause extra spurious hits with touchscreen.

Maybe having both options available would be good. 🤔

[AThousandShips]

I'd say that if we "fix" this it should be optional, a setting that makes up-events triggered on the same frame as the same down-event can be pushed on to the next frame, or having a special flag on an event saying it's on though just for this single frame, I'm not sure how safe it would be and what issues it could cause in turn, so might be best as a default off setting?

[AThousandShips]

Also to address: What happens if we have an action that was pressed, gets unpressed, and then pressed again, during a single frame

I'm not sure how much should be done to deal with input issues in situations that are arguably not reliable, like, should the engine be expected to be reliable in a low framerate situation? What are the use cases, etc.

[lawnjelly]

I think the key problem is that Input.is_action_just_pressed() (or some equivalent function, or mode of this) should always report true if the action WAS pressed in the last tick or frame (not whether it is currently pressed). This is so that users can have e.g. jump buttons, drop bomb etc. The problem currently is that if you press one of these buttons fast enough, it won't register (it will cause is_action_just_released(), but few people check for that).

I had noticed before that input was very unreliable on Android, this may be why.

Note that this problem was masked at high tick rates, and becomes particularly bad at low tick rates (and frame rates).

[vpellen]

I feel like there's some overthinking happening regarding "desired behaviour". If we make the reasonable assumption that "just" means "between now and the last frame":

Input.is_action_just_pressed() should return true if the action went from inactive to active during the last frame interval
Input.is_action_just_released() should return true if the action went from active to inactive during the last frame interval
Input.is_action_pressed() should return true if the action is currently active - i.e. if the last event was an activation

The important thing is to realize that the above events are not mutually exclusive. If, in the space between frames, you somehow manage to press a key, release it, and then press it again (such as if there's lag between frames), then all three methods should return true.

There may be concerns with people assuming that an action can't be pressed and released during the same frame, but that is a provably false assumption, and I don't feel Godot should allow bugs to persist in the name of protecting developers from the negative consequences of their own poor engineering choices.