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

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

Open
steamknight opened this issue Apr 11, 2024 · 5 comments

Comments

@steamknight
Copy link

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

@mournguard
Copy link

mournguard commented Apr 17, 2024

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

@steamknight
Copy link
Author

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);

@mournguard
Copy link

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.

@steamknight
Copy link
Author

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.

@mournguard
Copy link

mournguard commented Apr 27, 2024

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.

@Calinou Calinou added the bug label Jun 29, 2024
njamster added a commit to njamster/habituary that referenced this issue Jul 4, 2024
Requires a workaround since button shortcuts currently don't triger on
mouse events (see godotengine/godot#90516),
but I wanted to keep the visual feedback when triggering the button.
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

4 participants