Add scope parameter to `Input.is_action_just_pressed()`

timothyqiu commented 2 months ago

Describe the project you are working on

The Godot Engine

Describe the problem or limitation you are having in your project

Misunderstandings about Input.is_action_just_pressed() happen from time to time. Users want to use Input.is_action_just_pressed() instead of event.is_action_pressed(). For example:

For 4.x, we made it clear in the documentation that Input.is_action_just_pressed should not be used when inside input event callbacks. But the problem itself is mostly caused by not reading documentations, so documenting it makes not much difference for solving the user's confusion :P

Describe the feature / enhancement and how it helps to overcome the problem or limitation

The cause of the problem is that we have three ways to check for pressed actions in _input():

event.is_action_pressed(): exactly "just" pressed (as it's checking the current event).
Input.is_action_just_pressed(): just pressed in current process frame.
Input.is_action_pressed() is currently held down.

Since we already made Input.is_action_just_pressed() to return different results based on whether it's called in _process() or _physics_process(), I think we can make another step forward, making it equivalent to event.is_action_pressed() in input event callbacks by default.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

In addition to the behavior change when called inside input event callbacks, we can also add a scope parameter to is_action_just_pressed() to better control what "just" means.

Users could also revert to the current behavior by changing the scope back to SCOPE_PROCESS.

enum Scope {
    # Depending on where this function is called
    # `_process()` / `_physics_process()` / input event callback
    SCOPE_AUTO,
    # Whether the given action is just pressed in the current process frame
    SCOPE_PROCESS,
    # Whether the given action is just pressed in the current physics frame
    SCOPE_PHYSICS,
    # Whether the given action is just pressed in the current input event callback
    # Equivalent to `event.is_action_pressed()`, always false if not called inside input event callbacks
    SCOPE_INPUT,
}

func is_action_just_pressed(action: StringName, exact_match: bool, scope: Scope = SCOPE_AUTO) -> bool

This also allows finer checks like checking if the action is pressed in current process frame when inside _physics_process(), making it more useful.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No. This modifies the default behavior of Input.is_action_just_pressed().

Is there a reason why this should be core and not an add-on in the asset library?

Modifying core behavior.

lawnjelly commented 2 months ago

Another option might be just to emit a warning (WARN_PRINT_ONCE maybe?) if the user calls Input.is_action_just_pressed() within _input(), and a recommendation to replace with event.is_action_pressed().

This would likely flag up in some existing projects, but it is already a potential bug in their project code, so should likely be changed in some way, either automatically, or via warnings.

With this proposal I'm slightly unsure about a user setting e.g. SCOPE_PHYSICS when _input() could potentially be called outside physics, especially on platforms with multithread input (e.g. Android). This would then become dependent on when the Engine::physics_frame got incremented, which could lead to later bugs. The same problem could occur for idles frames.

There do seem a lot of cases for Scope to be inappropriate when called from different places (_input, _physics_process, _process).

I'm also not 100% convinced on the actual use case for checking e.g. frame is_action_just_pressed() within physics (usually this is for something you would want to happen just once, and there may be multiple physics ticks per frame).

Calinou commented 2 months ago

Another option might be just to emit a warning (WARN_PRINT_ONCE maybe?) if the user calls Input.is_action_just_pressed() within _input(), and a recommendation to replace with event.is_action_pressed().

I think this makes sense, but I'm not sure how to detect that the method was called in _input() (unless we implement this as a GDScript warning and modify the parser for this, which feels hacky and won't work in C#).

timothyqiu commented 2 months ago

how to detect that the method was called in _input() (unless we implement this as a GDScript warning and modify the parser for this, which feels hacky and won't work in C#)

The way is_action_just_pressed() distinguishes whether it is called in _process() or _physics_process() is to check the global Engine.is_in_physics_process(). The same can be done for inputs: register the global current event before propagating the input event so that it can be checked in is_action_just_pressed().

lawnjelly commented 2 months ago

The same can be done for inputs: register the global current event before propagating the input event so that it can be checked in is_action_just_pressed().

As timothy says, I added PR for this already (I'm not sure there are any special considerations for C#, as that should be called from within the function, but if so, please comment on PR): https://github.com/godotengine/godot/pull/97575

godotengine / godot-proposals