godot icon indicating copy to clipboard operation
godot copied to clipboard

Button Shortcut does not trigger if set to InputEventAction and the action is a Joypad Axis or anything Mouse related.

Open steamknight opened this issue 2 years ago • 5 comments

Tested versions

Godot Engine v4.2.1.stable.official.b09f793f5

System information

Godot v4.2.1.stable - Windows 10.0.22631 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3080 (NVIDIA; 31.0.15.5123) - Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz (16 Threads)

Issue description

When a button shortcut is set to InputEventAction and that action is bound to a joypad axis, it does not trigger. However, if bound to a key or button, it does.

I believe it happens because the code below filters out InputEventJoypadMotion among others.

void Viewport::_push_unhandled_input_internal(const Ref<InputEvent> &p_event) {
	// Shortcut Input.
	if (Object::cast_to<InputEventKey>(*p_event) != nullptr || Object::cast_to<InputEventShortcut>(*p_event) != nullptr || Object::cast_to<InputEventJoypadButton>(*p_event) != nullptr) {
		ERR_FAIL_COND(!is_inside_tree());
		get_tree()->_call_input_pause(shortcut_input_group, SceneTree::CALL_INPUT_TYPE_SHORTCUT_INPUT, p_event, this);
	}

Steps to reproduce

The MRP has one button that, when pressed, turns the background blue.

The action is bound to four inputs:

  • The Space key on the keyboard -- OK
  • Joypad Button 0 (Button A on an Xbox Controller) -- OK
  • Joypad Axis 1 (Left Stick Up, Joystick 0 Up) -- Reproduces bug
  • Right Mouse Button -- Reproduces bug

To reproduce the bug:

  1. Download and run the minimal reproduction project
  2. Press the Left Stick Up, notice nothing happens
  3. Press the Right mouse button, notice nothing happens
  4. Press the Space bar or Joypad Button 0 to see the background turn blue.

Minimal reproduction project (MRP)

ShortcutBug.zip

steamknight avatar Apr 11 '24 05:04 steamknight

Yup as mentioned in another issue I made it seems #62899 partially solved this but the other event types should also be included.

Alternatively it should be made impossible to set them at all but that would be super opinionated for no real reason, especially when the fix is super simple.

Should definitely add the "Good First Issue" label as the fix is basically as simple as #66750

mournguard avatar Apr 17 '24 04:04 mournguard

As you said, there's no reason to make it opinionated on what should or shouldn't be allowed in a shortcut. If I don't want a particular input event type to be a shortcut, I can simply not set it. :)

Would a potential fix be to remove the if-statement and always call

get_tree()->_call_input_pause(shortcut_input_group, SceneTree::CALL_INPUT_TYPE_SHORTCUT_INPUT, p_event, this);

steamknight avatar Apr 17 '24 05:04 steamknight

Since p_event is InputEvent and that's also what Shortcut has... yeah actually. This is so simple I just don't have a dev setup and I wouldn't want to just PR something from the browser.

mournguard avatar Apr 19 '24 05:04 mournguard

I set up the dev environment to give it a look.

Removing the code that checks for various InputEvent types seems to address this bug with regards to joypads, but there is still some weirdness with the mouse events.

As stated above, the example project has a joypad axis and the RMB as bindings to an InputEventAction. When the check was gone, I was getting the joypad axis but not the RMB.

The scene is set up like this:

- Control
  - ColorRect
  - Button

Both the Control and ColorRect had the mouse_filter property set to STOP, and once changed to PASS, the shortcut was working with the RMB.

However, this doesn't make sense to me since the button (which had the shortcut) is a child of the Control and sibling of the ColorRect, so the mouse filter shouldn't have applied to it as the button is in front of all of them. Perhaps my understanding of how mouse_filter is flawed, and that's the expected behavior.

steamknight avatar Apr 22 '24 00:04 steamknight

I think in that case it's more about your usage of the shortcut at all - the "shortcut" on buttons is like a independent way of globally (From the Viewport) catching input and correctly responding. The UI mouse events and filters are another thing. Setting a shortcut to RMB doesn't say "You need to press RMB on the button", it says "When RMB is sent, the button should react".

If you look at the source, this happens on "unhandled_input" in the viewport ; Sounds to me like your issue was that the button itself was indeed capturing the RMB, which means that event was handled, didn't bubble up to the viewport, and thus the shortcut wasn't executed.

If you want to change which mouse button activates the button control's own events, like pressed - use its button_mask. If you want to confirm what I'm saying, go back to your setup with mouse_filter to stop. RMB on the button would capture the event and not trigger the shortcut, but pressing RMB anywhere else in an empty space that has no control node capturing inputs probably will.

mournguard avatar Apr 27 '24 01:04 mournguard