Implement input action duration

[Xrayez]

Closes #21005.

A simple mechanism is added to fetch the time in seconds to tell how long an action has been pressed or released. A new timestamp field is added to calculate the difference between current and action toggle ticks using OS.get_ticks_usec.

It means that you can also tell for how long a particular action has not been pressed, some gd-pseudocode combining both cases:

# weapon.gd

const CHARGE_TIME = 1.0 # seconds
const RELOAD_TIME = 3.0 # seconds
var reloaded = true

func _physics_process(_delta):
    var duration = Input.get_action_duration("shoot")
    var power = duration / CHARGE_TIME

    if Input.is_action_pressed("shoot") and reloaded:
        if duration > CHARGE_TIME:
            # Maximum power reached, shoot now!
            shoot(power)
            reloaded = false
    else:
        if duration > RELOAD_TIME:
            # Ready to shoot again.
            reloaded = true

    if Input.is_action_just_released("shoot") and reloaded:
        # Shoot with the current power.
        shoot(power)

Note: if an action has not been pressed nor released during the game at all, the method will return 0.0 (can also consider returning -1.0 to indicate that particular case).

Potential limitations: #26828.

Test project

input-action-duration.zip

[groud]

I am kind of against adding specific things the action system. Getting how long an action has been pressed is far from being a common use case and can easily be worked around with a simple script registering the action states.

I'd rather keep the action system as simple as possible, and avoid adding more logic there unless there is no way around it.

[FeatherAntennae]

A lot of games make use of such a system. Be it for charging attacks, input/timing based puzzles, jump height, weapon overheating, cooldowns, etc.

Also, a lot of people keep asking about this on pretty much every major engines' support forums.

I agree the Input system should stay as simple as possible, but I personally think such a small addition to the Input system is useful and small enough to outweight the added complexity.

[Calinou]

The feature seems small and self-contained enough to make this acceptable to me. The diff is only 15 lines after all 🙂

[Calinou]

Please document the new method in the class reference 🙂

[Xrayez]

Added method documentation with some minimal code example with weapon charging mechanics.

[groud]

Indeed, it is simple and might be used, but it has nothing to do with input to action mapping. Actions are for mapping input, that's all. If we start adding functions to allow game logic there, we are going to end up with a big and complex API I don't want to see. The two problems I see are :

• There are plenty of way to handle timings in a game (input buffers, timers, etc...)
• what you propose can be done with two lines of code.

IMHO, everything related to timings should be kept out of the action mapping system.

[Xrayez]

IMHO, everything related to timings should be kept out of the action mapping system.

Just to clarify some existing logic, there are two methods which have to do with timings currently:

• is_action_just_pressed (pressed tick equals current tick)
• is_action_just_released (!pressed tick equals current tick)

Similarly:

• get_action_duration (current tick minus pressed/released tick)

I believe the first two would be used more often though. Yet you can also use the difference logic to query the same information. Compare:

Just pressed logic:

if is_action_just_pressed("jump"):
    jump()

Equivalent code with duration logic:

if Input.is_action_pressed("jump") and is_equal_approx(get_action_duration("jump"), 0.0):
    jump()

So if we remove is_action_just_pressed/released methods, we can still achieve the same logic with just a few lines of code on the scripting side with get_action_duration.

Furthermore, the duration logic can be used to tell whether the action's pressed state was changed (regardless of whether it was pressed or released):

if get_action_duration("shake") == 0.0
    shake()

So if anything, it only unifies all the possible use cases rather than adding something specific things to the input system.

[groud]

Yeah, but this makes things more complex than they need to be. The just_pressed logic is only computed for the current frame and does not involves time-related computations.

If you implement that, we are going to have people asking for how long an action went going unpressed (to implement cooldowns) too, or even a way to get all event's in a list with their respective timstamps, etc... That opens the door to adding a bunch of features that are, in my opinion, out of the scope of the action mapping system.

We have to draw a limit between what belongs to the action mapping system and what does not. For me, we should stop before adding any kind of time-related features. This will ensure the system is kept simple : it maps inputs to actions, that's all.

If we want to handle more complex scenarios, like input buffer, cooldowns, input event pre-shot (I did not find the right name, but I meant when an action is triggered even if pressed too soon. Like a jump registering even if you did not land on the platform yet), we should either move that to another singleton or to a plugin in the assets store.

[Xrayez]

If you implement that, we are going to have people asking for how long an action went going unpressed (to implement cooldowns) too

This PR actually covers both pressed and unpressed durations, but I get what you're talking about.

we should either move that to another singleton or to a plugin in the assets store.

I doubt such a singleton will be implemented in core as you expressed similar concerns about that too in godotengine/godot-proposals#104 (comment):

I see this proposal as adding yet another layer of complexity over a system that does already have too much: the raw events, the Input singleton storing the state, and the mapping of states to actions.

Exposing direct input states as in #35240 could actually make it much easier to implement plugin-based input time system in my opinion, possibly making this PR somewhat redundant. There's still a hurdle of storing a "database" of action timestamps via script for each and every action which are already available in the engine, it's just that they're not exposed.

Another way to think about this is adding methods which can retrieve internal timestamps to make this easier perhaps:

Input.get_action_physics_frame()
Input.get_action_idle_frame()

They don't involve time-related computations as you say, but alleviate the need to create an entire singleton for the only purpose of duplicating action timestamps with Engine.get_physics/idle_frame (available to scripting since 3.2: #35092).

Returning to the original purpose of this PR, I still think that the added method is useful for at least of 50% use cases. But I'm not sure what percentage of use cases is required to make this change eligible for merging, and really I'm not even sure if there's a way to know except for the amount of 👍 next to the proposals.

[groud]

I doubt such a singleton will be implemented in core as you expressed similar concerns about that too in godotengine/godot-proposals#104 (comment)

The main problem with that is finding a simple but flexible way to do it. Usually, such system is highly dependent on the kind of game you're building, thus I believe the assets store is likely the best way to have this. Nonetheless, if a plugin gets used a lot it could be moved to core afterwards too, that's also a solution we could use.

There's still a hurdle of storing a "database" of action timestamps via script for each and every action which are already available in the engine, it's just that they're not exposed.

The only thing you need to do that is being able to retrieve the list of actions, which is definitely the simplest way to allow you storing the whole action state.

Returning to the original purpose of this PR, I still think that the added method is useful for at least of 50% use cases. But I'm not sure what percentage of use cases is required to make this change eligible for merging, and really I'm not even sure if there's a way to know except for the amount of +1 next to the proposals.

I did not say it wasn't useful, I said it was, IMO, out of the scope of this API. The amount of a +1 is considered, but it has also to be accepted be core developers. This PR will probably need to be discussed in a PR meeting.

[Xrayez]

Updated the docs to denote that the method may not return accurate results if it's called during the idle frame due to possible FPS fluctuation so it's not recommended to be used for game logic specifically in those cases, yet this is perfectly fine for editor/visual/UI usages for instance.

OS.get_ticks_usec timestamps can be used, but it doesn't seem to be as robust compared to computing the duration based on physics and idle frames which also makes it easier/possible to compare against 0.0.

[FeatherAntennae]

Okay, this might be a bad idea, I'm still a begginer in C++, I'm not sure of all the implications on memory and performance, and core systems are intimidating, but if the idea is to keep the Input singleton as clean and as simple as possible, couldn't we make Action an actual class?

What if instead of calling Input.get_action_just_pressed("shoot") or anything like that, we were instead just having the singleton expose Actions as something the user can interact with?

The current Action struct could be an ActionState struct and an Action could be a Class containing an ActionState and the methods like pressed(), just_pressed(), etc.

The Action itself could contain the logic for timing and the Input singleton would only pass the ActionState to the appropriate Action or fetch an Action, exposing it and it's methods to the user.

This way if someone wants to add timing functionalities, a buffer, keep an history or anything else, they could extend Action and make a BufferAction or TimedAction and the Input singleton would stay clean and simple.

The base Action could have the same functionalities as what the current Input singleton exposes and to avoid breaking compatibility, we could keep the functions in the singleton, deprecate them and just replace their content by a call to the same method in the appropriate Action for the time being.

If this wouldn't cause too much problems, I could open a proper proposal for this and try to implement it myself.

[Xrayez]

What if instead of calling Input.get_action_just_pressed("shoot") or anything like that, we were instead just having the singleton expose Actions as something the user can interact with?

This somehow reminds me the changes introduced by #36368 where signals can operate as Callable objects if I understand correctly (signals are not only represented as strings). Similarly, actions can also be represented as resources which can be passed around.

Some Action's properties like just_pressed are time-dependent so if we want the Action resource to not depend on Engine singleton calls like get_physics_frame(), those need to be updated on each MainLoop iteration perhaps. I mean if I were to make Action extends Resource, I'd make sure it's purely data-driven. That's where @groud's reasoning makes sense.

With the changes I proposed in godotengine/godot-proposals#104 I went full-blown refactor in #35240 which exposes an entire InputState as a resource, which has an additional benefit of keeping not only the timing information connected to actions, but also the global timestamp for the entire input state (keys pressed, joy buttons pressed, mouse motion etc). Things like input buffers would be a matter of duplicating an entire InputState on demand and storing an array of those.

The question whether Input.get_action("jump").pressed would be more intuitive than Input.is_action_pressed("jump"), but having both API is also an option as you said. Some methods like Input.action_press() would make more sense as Input.get_action("jump").pressed = true.

The Input singleton could have methods which operated on a group of actions instead. For instance Input.get_actions_pressed() #15681, which would return a list of Action resources, each having necessary data to tell specific things about them. Sort of like get_tree.get_nodes_in_group() but for actions. In fact you could go further and assign specific controller ids to each type of actions perhaps.

There are many fruitful possibilities. Some would say that it adds complexity, but to me that's flexibility (which requires maintenance efforts by other core contributors unfortunately because there's only a handful of them).

[Xrayez]

Alright I made changed as suggested by @groud. Instead of relying on physics and idle frames, OS.get_ticks_usec timestamp is added to Action struct.

Some pros and cons I personally find with this:

Pros:

• Doesn't depend on FPS which can fluctuate on idle time (physics time should be fixed but as @groud said it may drop a few frames occasionally), so avoids inaccurate results with great fluctuations.
• Higher granularity/precision for when an action was physically pressed, not only frame-by-frame duration.

Cons:

• Since the timestamp may not represent the logical game time, it might be more difficult to do equality checks (even with is_equal_approx(get_action_duration("jump"), 0.0)).
• Recording and later replaying events might not be as accurate due to physical time vs physics frame time divergences, but this isn't critical for most use cases anyway I guess.

[Xrayez]

Rebased following the InputFilter renamings.

I'm not sure about the direction of this PR. I wanted to close this at some point given my findings about the hidden ideology regarding the development of Godot and how allegedly specific features like this does not belong in core, but I'll leave this PR up to community anyways.

To further decrease the chances of this being merged (😄 ) I wanted to say the following, please don't feel offended because that's just my experience:

@groud from the first moment I started contributing to Godot (like 3 years ago), one of the first things I received is your comments on how my features are too specific and outright shadowing the intrinsic value of a feature being proposed. While I understand that you want to ensure that the engine remains healthy in terms of preventing the software bloat, this attitude is not healthy towards people who do try to improve the engine. I realize this is just me with my emotional issues but I had to say that.

Cheers. ❤️

[groud]

@groud from the first moment I started contributing to Godot (like 3 years ago), one of the first things I received is your comments on how my features are too specific and outright shadowing the intrinsic value of a feature being proposed. While I understand that you want to ensure that the engine remains healthy in terms of preventing the software bloat, this attitude is not healthy towards people who do try to improve the engine. I realize this is just me with my emotional issues but I had to say that.

I am sorry for that, I am by no mean wanting to prevent people from contributing. With my limited time available, I tend to comment mostly on pull requests where I don't agree with the feature, which kind of biases the conversations. But anyway, as I did not take the time to do so I would like to thank you for your work on the engine, you contribute a lot to the engine. So thanks ! :)

Regarding the point here, if you want to limit the number of times this kind of things happen, I would advise you to come to the #godotengine-devel IRC channel to discuss new features first, and eventually open a proposal on the proposal repo. It's really hard to draw a line between what should be the engine responsibility and what should be the user's one, so before opening a PR, we have to discuss it together, between contributors. If you open the PR without taking the time to discuss it first, it has a significantly higher chance to be refused, and there is nothing we can really do about it as the project becomes bigger over time.

[Xrayez]

So far it seems like most my proposals are on the hard line of engine vs user responsibility, so that's where I'm torn oftentimes.

The discussion takes me quite a bunch of personal energy (not to mention the increased anxiety this process involves for me, especially at #godotengine-devel IRC). Besides, the work I do on the engine can usually be streamlined downstream without much issues, as I already have a fork which is specific to my project's requirements.

there is nothing we can really do about it as the project becomes bigger over time.

godotengine/godot-proposals#575.

80-90% of pull requests like this one can be prevented by having a more or less clear criteria of what goes into the engine vs plugin. Only the remaining 10-20% are worth discussing at #godotengine-devel.

Yeah there's a link at CONTRIBUTING.md document which points to this article though, but I guess the visibility of the decision diagram there asks for the better IMO (it's only an article, doesn't have much enforcement on the documentation side, so developers may ignore it).

Also, I'm totally fine if even 50% of pull requests will be closed, as long as I can advertise my skills to general public that way, hehe.

All I'm asking is a balance between the critique for the sake of it (so it doesn't become a habit), and having ability to see the other side of the coin. 😛

[Xrayez]

Closing this for now unless core devs express explicit interest in this and I'll reopen accordingly, I don't see the point of having this in Limbo, cheers. 😃

There's another PR which perhaps is more in alignment with the scope of the Input class: #35012.

[Xrayez]

Reopening because this can help implementing goostengine/goost#16, likely not in 3.2 but certainly in the future.

To resurrect the discussion and regurgitate my points a bit regarding the scope of the proposed changes: we already have time-dependent is_action_just_pressed and is_action_just_released, so makes sense to add something similar which actually covers both those methods: get_action_duration, see above discussion!