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

Bug with adding InputEventWithModifiers to InputMap #51879

Closed
boruok opened this issue Aug 19, 2021 · 11 comments
Closed

Bug with adding InputEventWithModifiers to InputMap #51879

boruok opened this issue Aug 19, 2021 · 11 comments

Comments

@boruok
Copy link
Contributor

boruok commented Aug 19, 2021

Godot version

v3.3.3.rc2.official [f66ff33]

System information

Windows 10, GTS450

Issue description

Tried to modify InputMap with following script.

extends Node


func add_input_action(action: String, key: String, modifiers:=[]) -> void:
	var e := InputEventKey.new()
	e.alt = modifiers.has("alt")
	e.shift = modifiers.has("shift")
	e.control = modifiers.has("control")
	e.meta = modifiers.has("meta")
	e.command = modifiers.has("command")
	e.scancode = OS.find_scancode_from_string(key)
	InputMap.add_action(action)
	InputMap.action_add_event(action, e)


func _ready():
	add_input_action("select", "o", ["command"])
	add_input_action("deselect", "o", ["alt"])


func _input(event: InputEvent):
	if event.is_action_pressed("select"):
		print("select")
	if event.is_action_pressed("deselect"):
		print("deselect")
	if event is InputEventKey:
		print(OS.get_scancode_string(event.get_scancode_with_modifiers()))
  1. notice editor output will be:

Control+O
Alt+O

  1. try to replace "command" to "control" and if statemements will be triggered even without "control" or "alt" keys are being pressed

Steps to reproduce

  1. attach script to the node
  2. run project
  3. press ctrl+o
  4. press alt+o
  5. check the output

Minimal reproduction project

No response

@Calinou
Copy link
Member

Calinou commented Aug 19, 2021

cc @EricEzaM

@EricEzaM
Copy link
Contributor

EricEzaM commented Aug 19, 2021

  1. notice editor output will be:
    Control+O
    Alt+O

Yes, since you are on Windows, command and control are the same thing. If you were on Mac, then it would say "Command + O".

  1. try to replace "command" to "control" and if statemements will be triggered even without "control" or "alt" keys are being pressed

Yep, this is a known thing in 3.X. However, it was fixed in 4.0 in #44355, and backported to 3.X in #50874. I think it should be in the RC release It's in 3.4. What you need to do is use the second parameter to is_action_pressed and set it to true, like this: event.is_action_pressed("select", true). The second param is "exact match". With it, only exact matches of the key combos will be accepted. Previously, if you had modifier keys pressed, it didn't care and would return true anyway.

@kleonc
Copy link
Member

kleonc commented Aug 19, 2021

Happens because on Windows command and control refers to the same memory (union below):

godot/core/os/input_event.h

Lines 229 to 243 in 0452832

#ifdef APPLE_STYLE_KEYS
union {
bool command;
bool meta; //< windows/mac key
};
bool control;
#else
union {
bool command; //< windows/mac key
bool control;
};
bool meta; //< windows/mac key
#endif

not sure why it's like that but it needs to be either fixed or documented (if that's expected/correct behavior).


@boruok So in your example when you write:

e.control = modifiers.has("control")
e.meta = modifiers.has("meta")
e.command = modifiers.has("command")

the e.command = modifiers.has("command") line overwrites whatever was assigned in the e.control = modifiers.has("control") line.
So according to the linked source code something like this should work instead:

func add_input_action(action: String, key: String, modifiers:=[]) -> void:
	var e := InputEventKey.new()
	e.alt = modifiers.has("alt")
	e.shift = modifiers.has("shift")

	# check if APPLE_STYLE_KEYS is defined, not sure if that's a correct check
	if OS.has_feature("OSX") or OS.has_feature("iOS"):
		# command/meta refer to the same thing
		e.command = modifiers.has("command") or modifiers.has("meta")
		e.control = modifiers.has("control")
	else:
		# command/control refer to the same thing
		e.command = modifiers.has("command") or modifiers.has("control")
		e.meta = modifiers.has("meta")

	e.scancode = OS.find_scancode_from_string(key)
	InputMap.add_action(action)
	InputMap.action_add_event(action, e)
	prints(e.as_text(), e.control, modifiers.has("control"))

@EricEzaM
Copy link
Contributor

EricEzaM commented Aug 19, 2021

Additionally, you would be better of ditching strings and using the key mask enums, KEY_MASK_META, KEY_MASK_COMMAND, etc. This would internally handle these cases, and you wouldn't need the script @kleonc wrote above. See here under section KeyModifierMask.

@akien-mga
Copy link
Member

Yep, this is a known thing in 3.X. However, it was fixed in 4.0 in #44355, and backported to 3.X in #50874. I think it should be in the RC release.

It's not backported to the 3.3 branch and probably shouldn't be. So it's not included in 3.3.3 RC 2, but it's in 3.4 beta 4.

@EricEzaM
Copy link
Contributor

Ah, thanks for clarifying.

@boruok
Copy link
Contributor Author

boruok commented Aug 19, 2021

@EricEzaM thanks!

@boruok boruok closed this as completed Aug 19, 2021
@kleonc
Copy link
Member

kleonc commented Aug 19, 2021

See here under section KeyModifierMask.

You can link directly to that section like this.


Probably OS specific behavior for the properties of InputEventWithModifiers (that one is alias for the other) should be also documented in its docs page.

@EricEzaM
Copy link
Contributor

@kleonc Yeah i looked for it, but there isn't those handy link icons for copying a link to that element id... and I just didn't bother copying the id to do it (I assume what you did?).

@kleonc
Copy link
Member

kleonc commented Aug 19, 2021

@EricEzaM In the description of KEY_MASK_CMD there are few links to specific entires of that enum, I've just manually modified one of them.

@EricEzaM
Copy link
Contributor

Ha, nice one. It would be nice if the docs had little hyperlink icons for each section.

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

5 participants