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

Action with modifiers can be released even without being pressed #92228

Open
MikkBenelis opened this issue May 21, 2024 · 18 comments
Open

Action with modifiers can be released even without being pressed #92228

MikkBenelis opened this issue May 21, 2024 · 18 comments

Comments

@MikkBenelis
Copy link

Tested versions

Reproducible and found it in v4.2.2.stable.official [15073af]

System information

Godot v4.2.2.stable - Windows 10.0.22631

Issue description

If you have an action with the Ctrl and/or Shift modifiers it can be released even without being triggered. Please, check steps to reproduce, it is quite straightforward. The expected behavior: an action can't be released until it is triggered, something like keeping a boolean flag somewhere, enabling it when action triggered and emitting the released event taking into account this flag.

Steps to reproduce

  1. Create a new action (Project Settings -> Input Map) with the key combination of Ctrl+Shift+F12
  2. Handle this action in _input, _process or similar functions as:
func _input(event : InputEvent) -> void:
	
	if Input.is_action_just_pressed("test_action"):
		print("is_action_just_pressed")
	
	if Input.is_action_just_released("test_action"):
		print("is_action_just_released")
	
	if event.is_action_pressed("test_action"):
		print("is_action_pressed")
	
	if event.is_action_released("test_action"):
		print("is_action_released")
  1. Press and release just the F12 key, an action will not be pressed, but it'll be released
  2. Press and release the entire combination of Ctrl+Shift+F12, an action will be pressed and released

Output window result (both 3 and 4 steps):
image

Minimal reproduction project (MRP)

Godot.zip

@AThousandShips
Copy link
Member

Try using Input.is_action_just_pressed("test_action", true), see the documentation

@MikkBenelis
Copy link
Author

How haven't I noticed it?... It solves the issue, but partially.
Actions still can be released without being pressed in some cases.

For simplicity, I used the same input map and this code:

func _input(_event):
	if Input.is_action_just_pressed("test_action", true): print("pressed")
	if Input.is_action_just_released("test_action", true): print("released")

Then I pressed and released the Ctrl+F12+Shift combination 5 times.
And here is the result that I've got (an action was released 5 times, but never pressed):
image

@AThousandShips
Copy link
Member

Try using the event, you shouldn't use Input in the _input method, see the documentation page I linked:

Note: During input handling (e.g. Node._input), use InputEvent.is_action_pressed instead to query the action state of the current event.

@MikkBenelis
Copy link
Author

There is no accessible method in event (InputEvent) with the _just_ part.
Tried to use this code:

func _input(event : InputEvent) -> void:
	if not event is InputEventKey: return
	if event.is_action_pressed("test_action", true): print("pressed")
	if event.is_action_released("test_action", true): print("released")

In this case the result is the same and it adds one more probably expected issue:
the pressed event is emitted multiple times if I hold these buttons for a while.

@AThousandShips
Copy link
Member

AThousandShips commented May 23, 2024

Again, see the documentation, you need to use allow_echo, it's all explained there, you need to use event.is_action_pressed("test_action", false, true)

@MikkBenelis
Copy link
Author

Well, both pressed and released are writen once with this flag, but there is still an original issue: an action can be released without being pressed (number of pressed is not equal to number of released). Input map is Ctrl+Shift+F12, but I'm pressing it in a slightly different order as Ctrl+F12+Shift and releasing it: as a result action is not pressed, but released.

The first attempt here was to press this combination properly, it resulted in both pressed and released. But all the next 5 attempts with a different keys order resulted just the released message.
image

Here is the code:

func _input(event : InputEvent) -> void:
	if not event is InputEventKey: return
	if event.is_action_pressed("test_action", false, true): print("pressed")
	if event.is_action_released("test_action", true): print("released")

@AThousandShips
Copy link
Member

AThousandShips commented May 23, 2024

Okay does it work if you have event.is_action_released("test_action", false, true)? It's the same syntax

Did you read the documentation?

@MikkBenelis
Copy link
Author

MikkBenelis commented May 23, 2024

Yes, I read it, but there is no such overload for this function: Too many arguments for "is_action_released()" call. Expected at most 2 but received 3. It is not an echo, in this case it is something different since I just pressed the same combination multiple times without holding it...

@AThousandShips
Copy link
Member

Oh yes sorry that's the case yes, true it should have just the true, that might be a bug indeed, but would have to test the specifics myself

@AThousandShips
Copy link
Member

So you released all parts of the combination before pressing again, and only Ctrl+Shift+F12 matches for pressed?

@MikkBenelis
Copy link
Author

Yes, for the pressed message to be printed the F12 key needs to be pressed last while holding both Ctrl and Shift keys. For the released message the F12 key needs to be released first while holding both Ctrl and Shift keys. But it leads to the situation where I can press F12 first, then both Ctrl and Shift and then release the F12 key (while still holding modifiers): pressed event will not be emitted, just released.

@AThousandShips
Copy link
Member

That sounds like it has a problem with registering the modifier keys, but unsure if it's necessarily a bug, I'd suspect that the main key without modifiers is the main part of the event, does it work that way in other software when pressed? The released part might be a bug in that case just specifically

Can you try with just _process or similar to test if Input.is_action_just_released? It might work differently in that sense, and it might just be a lack of equivalence between the events.

I will do some testing on this tomorrow, but I suspect the part where pressed isn't fired when the main (real, i.e. non modifier) isn't pressed last might not be a bug but just a matter of design (to be documented), if you can check other software that'd be a good point of reference

@MikkBenelis
Copy link
Author

MikkBenelis commented May 23, 2024

With the _process and static Input class calls it works identical.
Also I found a quite similar issue/behavior while tested it.
Below are formalized steps to reproduce both scenarios.

Scenario 1:

  1. press the F12+Ctrl+Shift combination;
  2. release the F12 key first;
  3. release all the rest keys.

Result: action is released, but never pressed

Scenario 2:

  1. press the Ctrl+Shift+F12 combination;
  2. release one of the modifier keys first;
  3. release all the rest keys.

Result: action is pressed, but never released

image

Here is the code:

func _process(_delta : float) -> void:
	if Input.is_action_just_pressed("test_action", true): print("pressed")
	if Input.is_action_just_released("test_action", true): print("released")

Not sure how does it work with other software, but it can cause some actions to "stick" and never release just if a user pressed it in a different order... Pressing the main key last for event to trigger is perfectly fine, but the fact that event can be only pressed or released but not both followed one after another looks more like a bug to me.

@AThousandShips
Copy link
Member

With some other software I've confirmed that at least there doing things in the "wrong" order doesn't count, so I think that part is by design, will test the released part tomorrow but that'd be the part I'd consider a bug, the pressed part isn't a bug IMO

@MikkBenelis
Copy link
Author

MikkBenelis commented May 23, 2024

Actually, it is much easier to reproduce these bugs (I still think that they are parts of a single bug):

  1. Prepare the same code I sent in a previous message and setup an action with the Ctrl+Z key combination
  2. Press Ctrl+Z, release the Ctrl key, then release the Z key ---> action will be pressed, but not released
  3. Press Z+Ctrl, release the Z key, then release the Ctrl key ---> action will be released, but not pressed

I think, it is quite easy to face this bug when playing a game with these simple two-key combinations and don't understand why it was not found earlier... Could it be something new? Also I'm wondering if it is keyboard specific, but have just one wireless keyboard to test.

@AThousandShips
Copy link
Member

AThousandShips commented May 24, 2024

I can confirm this but I'm not sure exactly what to treat it as, the release with inexact will occur when the active key (i.e. non-modifier) is released, the exact only when the full combination is released

The action itself is not considered pressed, with Input.is_action_pressed, if not in the right combination

So I'd say that the possible bug to discuss here is if the release trigger should be fired on these cases, and when releasing in a different order

I think this might be something to document rather than fix

Edit: At least on Windows the reason for this is that when pressing Ctrl+S+Shift the events that are dispatched are a Ctrl event, a S event with Ctrl modifier, and a Shift event, which I'd say is to be expected, this simply doesn't trigger an action to be marked as pressed

Now the question is if the release check should take into consideration if the event was ever pressed, but that's something that might be considered compatibility breaking and would require further evaluation

Related:

@MikkBenelis
Copy link
Author

Both behaviors I described are unexpected since you may have an action which will be pressed and never released and the other way around - an action which will be released without it beeing ever pressed. Think about this example: a player has a throwable item in his inventory, when an action is pressed - he'll take this item to hands, when the action is released - he'll throw it. But now he can take an item to hands and even after releasing both key it'll still be in his hands or the second option - he can try to throw an item without taking it to hands.

P.S.: Maybe we can delete all of our messages starting from the second one up to my message with easier steps to reproduce, so it'll not confuse anyone else who'll read this discussion and try to follow it.

@AThousandShips
Copy link
Member

I'd say that the pressed part is fully as expected and desired, a key combination, i.e. a shortcut, shouldn't fire for these cases IMO. Ctrl+S shouldn't turn into Ctrl+Shift+S when pressing Shift after the other two keys, that's dangerous and unexpected

And the release part is something to investigate and decide on, but I suspect it's something that's safest to document and clarify, it risks breaking compatibility in unforeseen ways

Further, I'd say that using modifier keys for actions in a game is not something that's necessarily the best solution, most of the time when developing games you have modifier keys modify your actions after the fact and you use other means to detect them

The thing I think could be changed, if it's considered safe, would be to add some form of detection for release events where modifier keys being released updates the actions related to them, but that's pretty complicated and might be fragile and undesired, so that's something we'll have to wait for further input on.

I think the first step might be to document some of the details of this to help people resolve potential pitfalls, and work on a future solution, and discuss the potential arguments about it

Maybe we can delete all of our messages starting from the second one up to my message with easier steps to reproduce, so it'll not confuse anyone else who'll read this discussion and try to follow it.

I can hide them as resolved but otherwise they should stay IMO

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

No branches or pull requests

2 participants