Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modifier+key action shows also key action as pressed #18793

Closed
AlexHolly opened this issue May 11, 2018 · 10 comments · Fixed by #44355
Closed

modifier+key action shows also key action as pressed #18793

AlexHolly opened this issue May 11, 2018 · 10 comments · Fixed by #44355

Comments

@AlexHolly
Copy link
Contributor

Godot Master
https://godotengine.org/qa/28199/how-to-use-shortcuts-with-modifier

I am looking for something like this I thought godot checks this internally. Try to press CONTROL+S.

Expected result CONTROL_S

func _process(event):
    if Input.is_action_just_pressed("CONTROL_S"): # KEY_CONTROL+KEY_S
        print("CONTROL_S")

    if Input.is_action_just_pressed("S"): # KEY_S
        print("S")

# Output: CONTROL_S and S

This works but I can't use it in my case

func _process(delta):
    if Input.is_action_just_released("CONTROL_S"): # KEY_CONTROL+KEY_S
        print("CONTROL_S")

    if Input.is_action_just_released("S") && !Input.is_action_just_released("CONTROL_S"): # KEY_S
        print("S")

# Output: CONTROL_S
@kubecz3k
Copy link
Contributor

kubecz3k commented Jun 3, 2018

How would you solve that? Go though Input action list and ignore other actions if there is 'direct match'?

@AlexHolly
Copy link
Contributor Author

If such a check is possible, that would probably solve it.

Someone one irc suggested to add a flag for these cases. If that behavior I described is intended you would have to activate the flag for the specific actions.

@vnen
Copy link
Member

vnen commented Jun 3, 2018

Someone one irc suggested to add a flag for these cases. If that behavior I described is intended you would have to activate the flag for the specific actions.

That's probably the best way. Many times you don't want a modifier key to shadow an action without modifier, e.g. pressing Space to jump should work even if you are holding Shift to run.

@AlexHolly
Copy link
Contributor Author

Yeah thats true.
I have no idea how the actions work internally in the example the shadow should only happen when an actual action exists for mod+space otherwise they are simply 2 separate actions mod=run and space=jump but when mod+space=superjump only run will activate and superjump will happen once space is also pressed.

If that is only possible with a flag thats fine too.

@vnen
Copy link
Member

vnen commented Jun 3, 2018

That's assuming Shift+Space replaces jump. If the action is used for something on UI, for instance, then this automatic detection will be a bad thing.

I believe there are use cases for both, so it should be a flag per-action.

@TokisanGames
Copy link
Contributor

If an InputMap is going to abstract the keys for me, then it should do the work of filtering and fire exclusively based on most specific matching.

My code should not know or care whether ui_focus_next or ui_focus_prev has more or less specific keys, whether they use modifiers or duplicate keys, or what order they should be in, as that defeats the whole purpose of the InputMap and abstracting the keys for me.

ex.

  • ui_focus_next -> S
  • ui_focus_prev -> Control+S
    Pressing control+s should result only in ui_focus_prev.

ex2.

  • shift -> run
  • space -> jump
  • shift+space -> super jump
    Pressing shift+space should result in run, then super jump. If the third mapping is removed, then it should result in run, then jump.

ptrojahn added a commit to ptrojahn/godot that referenced this issue Aug 11, 2019
When pressing Alt + Left, the item list already handled the event
and moved the selection to the left, unless the first item was
selected. Because of this, moving up the history didn't work
consistently. By using the gui_input signal to override the event
handling, we can catch these events and handle them before the
item list gets its fingers on them.
Also gui_input should be emitted in viewport.cpp before _gui_input
is called(Like explained in line 1516).

Related to godotengine#18793
Might be a better way to do godotengine#22198
@DoctorWhoof
Copy link

I just ran into this, and was about to file a bug.
In my case it's an App with a complicated UI, not a game, and there will be tons of such conflicts, i.e:

Z: Play C4 Note on piano
Ctrl+Z: Undo

But Ctrl+Z Plays the note AND performs undo.
Since there are two piano octaves available via keyboard... yeah, there will be a ton of conflicts. 😅

Filtering it myself would be a lot of work prone to bugs, since there's no central "UI Control" script and each part of the UI is a sub-scene and manages itself, not aware about where the conflicts may happen in other scenes. Allowing the Input system to perform this filtering as it's being suggested would be much, much welcome.

Maybe an "Exclusive" checkbox column in the Input Map editor would cover most of the cases, and stay simple enough? Checking it would only fire the action if that key, and only that key, is pressed.

Thanks!

@EricEzaM
Copy link
Contributor

More Information about this bug is in #44180.

I found where the problem code is. I will have to fix this very soon as it is required for #43663 to be merged

@MuffinInACup
Copy link

Reproducible in Godot 4.1
It can be worked around with an elif setup in the correct order but figured might as well report this

Sample project: https://drive.google.com/file/d/176GERr0Lba_PfpjPdpepBfQg24ZYtmJK/view?usp=sharing

Pressing ctrl+A in the project triggers the action for a standalone A press

@pineapplemachine
Copy link

pineapplemachine commented Jan 8, 2024

I've also just run into this. Not having a way to require that a modifier key not be pressed means that I will apparently need to write my own input controller code and cannot use the one integrated with the engine if I will allow players to configure these controls. This seems like a pretty serious piece of missing functionality, or a serious regression if this was working before.

When using the new optional argument for e.g. Input.is_action_pressed, this does actually work as expected. (#44355)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment