godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.71k stars 21.12k forks source link

Keyboard Input hangs for modifier keys like Ctrl / Shift #6388

Closed timoschwarzer closed 7 years ago

timoschwarzer commented 8 years ago

Operating system or device - Godot version: 2.2 as of 77cb836, Linux

Issue description (what happened, and what was expected): Keyboard Input doesn't get released for modifier keys like Ctrl or Shift.

Steps to reproduce:

After hitting Shift once, Input.is_action_pressed("test") will always return true.

timoschwarzer commented 8 years ago

Update: Even if I don't have any Input assigned to Shift but for example A: When I press Shift and A at the same frame, A doesn't get released.

NiclasEriksen commented 8 years ago

I'm also able to replicate this, on Linux ee37c2f.

aaschmitz commented 8 years ago

The commit https://github.com/godotengine/godot/commit/5b96c3a5527c1b2989dbfbe625f1c763b8887334 added the issue.

To replicate it, use the latest Platformer example from Godot 2D Demos and:

Add a mapping for CONTROL key in project Input Mapping Action shoot.

Change the player.gd line 63 from:

if (Input.is_action_just_pressed("shoot")):

to:

if (Input.is_action_pressed("shoot")):

Running the project and pressing the SPACE key, one stream of bullets is show and stop when releasing the key. Pressing the CONTROL key, the stream of bullets not stop when releasing the key.

Faless commented 8 years ago

This bug is quite annoying, although it seems the solution is not easy at all. I managed to do a workaround like this:

diff --git a/core/input_map.cpp b/core/input_map.cpp
index 3a0f959..d752246 100644
--- a/core/input_map.cpp
+++ b/core/input_map.cpp
@@ -122,7 +122,7 @@ List<InputEvent>::Element *InputMap::_find_event(List<InputEvent> &p_list,const

                        case InputEvent::KEY: {

-                               same=(e.key.scancode==p_event.key.scancode && e.key.mod == p_event.key.mod);
+                               same=(e.key.scancode==p_event.key.scancode && (!p_event.is_pressed() or e.key.mod == p_event.key.mod));

                        } break;
                        case InputEvent::JOYSTICK_BUTTON: {

Is seems to work. But edge cases should be checked.

I still believe there should be a way to tell the engine not to treat Shift, Alt, Control or Meta as modifiers. This way, after setting the option, pressing the button "A" while pressing the key "Shift" will generate the action associated with "A" and not "shift+a"

aaschmitz commented 8 years ago

For me, the pull request #6584 fixed the problem on Linux (Ubuntu).

akien-mga commented 8 years ago

Nice, closing as fixed then.

27thLiz commented 8 years ago

@akien-mga I can still reproduce this.

Faless commented 8 years ago

The fix for the modifier is probably needed, but the problem here is in the new way actions are handled.

Godot now keeps an internal state for every action, and sets it as pressed or released when parsing events.

The problem is that to check if the action is released modifiers are counted (which shouldn't happen).

Let's look at this use case:

InputMap: "action_a" -> key "A", modifiers == none "action_shift" -> key "Shift", modifiers == none

Usage: User press: A -> action_a is pressed (keycode == A, modifiers == none) press Shift -> action_shift is pressed (keycode == Shift) release A (while still holding shift) -> action_a will stay pressed because the release event will be for shift+A and not A so the action will not be marked as released. (keycode == A, modifiers == shift)

Sslaxx commented 8 years ago

May mean #6612 is related.

Sslaxx commented 8 years ago

If this was also happening under Windows the platform:linux tag should be removed.

Faless commented 8 years ago

This is a better workaround that only apply when calling InputMap::event_is_action. In that case, if the event is a release, comparison will only be done using the scancode and not the modifier. So releasing the key should work in every case (InputDefault::parse_input uses InputMap::event_is_action internally to mark actions as pressed/released).

diff --git a/core/input_map.cpp b/core/input_map.cpp
index 09cb7ce..ad1403a 100644
--- a/core/input_map.cpp
+++ b/core/input_map.cpp
@@ -106,7 +106,7 @@ List<StringName> InputMap::get_actions() const {
    return actions;
 }

-List<InputEvent>::Element *InputMap::_find_event(List<InputEvent> &p_list,const InputEvent& p_event) const {
+List<InputEvent>::Element *InputMap::_find_event(List<InputEvent> &p_list,const InputEvent& p_event, bool p_mod_ignore=false) const {

    for (List<InputEvent>::Element *E=p_list.front();E;E=E->next()) {

@@ -122,7 +122,7 @@ List<InputEvent>::Element *InputMap::_find_event(List<InputEvent> &p_list,const

            case InputEvent::KEY: {

-               same=(e.key.scancode==p_event.key.scancode && e.key.mod == p_event.key.mod);
+               same=(e.key.scancode==p_event.key.scancode && (p_mod_ignore || e.key.mod == p_event.key.mod));

            } break;
            case InputEvent::JOYSTICK_BUTTON: {
@@ -229,7 +229,7 @@ bool InputMap::event_is_action(const InputEvent& p_event, const StringName& p_ac
        return p_event.action.action==E->get().id;
    }

-   return _find_event(E->get().inputs,p_event)!=NULL;
+   return _find_event(E->get().inputs,p_event,!p_event.is_pressed())!=NULL;
 }

 const Map<StringName, InputMap::Action>& InputMap::get_action_map() const {
diff --git a/core/input_map.h b/core/input_map.h
index 21c4795..a974f6f 100644
--- a/core/input_map.h
+++ b/core/input_map.h
@@ -46,7 +46,7 @@ private:
    mutable Map<StringName, Action> input_map;
    mutable Map<int,StringName> input_id_map;

-   List<InputEvent>::Element *_find_event(List<InputEvent> &p_list,const InputEvent& p_event) const;
+   List<InputEvent>::Element *_find_event(List<InputEvent> &p_list,const InputEvent& p_event, bool p_mod_ignore) const;

    Array _get_action_list(const StringName& p_action);
    Array _get_actions();
RebelliousX commented 8 years ago

Any update on this issue? It is marked as 2.2 while the PR as 3.0!! So which one?

Sslaxx commented 8 years ago

No 2.2 release is planned, just straight to 3. Maybe cherry-picked for 2.1.x?

akien-mga commented 8 years ago

It doesn't affect 2.1 as far as I know.

Faless commented 8 years ago

It doesn't affect 2.1 as far as I know.

Yes, as long as "is_action_just_pressed" ( 5b96c3a ) does not get cherry picked in 2.1 this bug only affects master branch.

Thebas commented 5 years ago

This issue is still happening on my environment. (Only happens with the Alt key). Master branch. Linux, Manjaro, 64bits. To replicate:

Input.is_action_pressed("shift_pair")
Input.is_action_pressed("alt_pair")

Press Alt, then Shift, release Alt, only then release Shift. Alt stays locked as pressed.

donohutcheon commented 5 years ago

I seem to be getting this on 3.1 stable official on Ubuntu 18.4. It is really funky! (read on)... It seems to occur when you have pressed multiple buttons at the same time and release them as Thebas explained above. In my tutorial game I use cursor keys left; up(jump); right; so when jumping and moving and releasing the engine goes into a whack state where is_action_pressed always return true for whatever action it is stuck on. So this is not specific / limited to alt and shift.

NOW The funky part is: After reproducing this issue (it only seems to happen when I'm not trying), I put the focus into this edit box on the web page in my Chrome browser and the cursor was still stuck in the left loop like it was in my game.

donohutcheon commented 5 years ago

Doesn't seem to be reproducible on Windows 7, but I'll keep trying. It could be an underlying Linux issue outside of Godot (I'm just speculating)