Fix action state when multiple events are assigned

[KoBeWi]

Fixes #45628

This PR simply changes Action.pressed (internal struct) from bool to int and increments it when action is pressed and decrements when it's released. Input.is_action_pressed() and related actions return true when at least one even is pressed.

There is some weirdness when releasing actions with modifier keys. They can be released without being pressed. I added a small workaround, but the changes need some testing in general.

[Sauermann]

The concept looks good from a quick glance at the changes.
Do you think that this PR supersedes #47599?
This PR should probably revert #30890.

[KoBeWi]

Do you think that this PR supersedes #47599?

Yes. That PR has so many changes that I'm not even sure what it's trying to do.

This PR should probably revert #30890.

Done.

[Sauermann]

This PR reverts #30890, which originally solved issue #30888.
I have tested this PR and can confirm, that this PR fixes the implementation and resolves #30888. Implementation looks good.

I can't test if this PR solves issue #45628, because I don't have a PS4 controller.

[KoBeWi]

I can't test if this PR solves issue #45628, because I don't have a PS4 controller.

You can test with a keyboard, with 2 keys assigned to the same action (e.g. the default ui_accept).

[Sauermann]

I have tested this PR with 2 keys assigned to the same action.
So #45628 is a duplicate of #30888!?

[KoBeWi]

Seems so, although it's weird that it says that it worked in Godot 3 🤔
I'll test with a controller.

[KoBeWi]

Ok I pushed some changes to also fix action strength.

I can't exactly reproduce the original issue (i.e. "controls randomly not working"), so it needs to be tested by affected users.

[Sauermann]

I can't replicate #45628, so it is difficult for me to verify, if the new changes fix that issue.

[Sauermann]

This implementation changes the action.strength on every event, which might lead to alternating values, if two axis inputs are mapped to the same action and both axis are moved.

The comment #45628 (comment) suggests to use as strength the maximum strength of all related actions. Perhaps that would be a better solution?

[KoBeWi]

But then what happens on release? It should assign max strength of still pressed actions, but they are not tracked. This means that if you use analog and then press a key, your analog strength will be ignored until it's released.
Maybe it's fine, idk.

[Sauermann]

It should assign max strength of still pressed actions, but they are not tracked

Perhaps we could track it with the following, but I haven thought it through yet.

diff --git a/core/input/input_map.h b/core/input/input_map.h
index b4d5beacb3..b59e898730 100644
--- a/core/input/input_map.h
+++ b/core/input/input_map.h
@@ -48,10 +48,17 @@ public:
         */
        static int ALL_DEVICES;
 
+       struct EventWithStrength {
+               Ref<InputEvent> event;
+               int pressed = 0;
+               float strength = 0.0f;
+               float raw_strength = 0.0f;
+       };
+
        struct Action {
                int id;
                float deadzone;
-               List<Ref<InputEvent>> inputs;
+               List<EventWithStrength> inputs;
        };
 
 private:

[KoBeWi]

This can be addressed later if someone opens an issue. Let's see if the current behavior is ok for now.

[Sauermann]

Sounds good to me.

[romlok]

Stumbled upon this PR while investigating some related behaviour, so I tested this under Linux:

When idle, the (my) DS4's analogue sticks often send out a stream of events with very low values (|n| < 0.1), alternating between two ±0.01 values (eg. 0.06, 0.07 in quick succession).

So if a stick axis is mapped to an action, and that action is triggered through a different input than the stick, the action will tend to be "released" after a short amount of time.

---

I also noticed that, with this PR, if the action is triggered via the stick, it will not always release immediately when the stick is released. The associated input events will be fired, but Input.is_action_pressed will continue to be true for a while afterward. Seemingly until some amount of low-value axis events are fired?

---

Also, unrelated to stick axes (I disconnected the DS4 just to be sure!), it appears that the "inputs held" counter doesn't reset to zero when the window loses focus. If I hold down two keys to trigger an action, and move focus to another window, Input.is_action_pressed continues to return true. For a single held key the action is considered released when the window loses focus.

FWIW, this was my test project: is-action-pressed.zip

[Sauermann]

alternating between two ±0.01 values (eg. 0.06, 0.07 in quick succession).

Have you tried to set the deadzone to 0.1? That might help.

Input.is_action_pressed continues to return true

That issue is tracked in #36223.

[reduz]

Looks good

[reduz]

To me this was always the right approach to solve the original issue btw.

[romlok]

alternating between two ±0.01 values (eg. 0.06, 0.07 in quick succession).

Have you tried to set the deadzone to 0.1? That might help.

The deadzone specified in the input map is the default 0.5. AFAICT deadzone is used to determine pressed/not pressed, but doesn't actually filter out any events. Or is there another deadzone setting? 🤔

Input.is_action_pressed continues to return true

That issue is tracked in #36223.

I'm using Linux, but that issue appears to be specific to MacOS? 😕
The current behaviour for me in 4.1 is that actions are considered released when the window loses focus, no matter how many of that action's inputs are being pressed. This patch changes that behaviour for multiple simultaneous inputs.

It seems to be that with this patch the "held keys" is reduced by one on each focus loss, so if an action does get "stuck" in this way, one can release it by repeatedly focus-defocusing the window.

[KoBeWi]

It seems to be that with this patch the "held keys" is reduced by one on each focus loss, so if an action does get "stuck" in this way, one can release it by repeatedly focus-defocusing the window.

Fixed.

[akien-mga]

Thanks!

---

I actually merged this by mistake xD
I had the tab open because I wanted to do some testing and make sure I can both reproduce the original bug, and confirm the fix. I guess I can still do that post merge.

[jordanlis]

The deadzone specified in the input map is the default 0.5. AFAICT deadzone is used to determine pressed/not pressed, but doesn't actually filter out any events. Or is there another deadzone setting? 🤔

Haa ok, I understand now. So the deadzone doesn't apply if the button is not pressed by the user ? (like these random events) ? I don't really see why we don't just apply this deadzone to any events, it should fix the original issue of really small and random events, no ?

[KoBeWi]

The deadzone is always used. It determines whether action is pressed or not.