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

Can't assign mouse buttons or mouse wheel to editor shortcuts #38326

Open
EricEzaM opened this issue Apr 29, 2020 · 12 comments
Open

Can't assign mouse buttons or mouse wheel to editor shortcuts #38326

EricEzaM opened this issue Apr 29, 2020 · 12 comments

Comments

@EricEzaM
Copy link
Contributor

EricEzaM commented Apr 29, 2020

Godot version:
All versions including v4.0.dev.custom_build.1d45a269f
OS/device including version:
W10 1903
Issue description:
Can't assign mouse button inputs to editor shortcuts
Steps to reproduce:
Editor Settings > Shortcuts > Try to set any mouse button, can't
Minimal reproduction project:
None

Some related issues:
#6366
#26999

The editor settings only allows key inputs, as shown below, where the event is cast to InputEventKey.

void EditorSettingsDialog::_wait_for_key(const Ref<InputEvent> &p_event) {
Ref<InputEventKey> k = p_event;
if (k.is_valid() && k->is_pressed() && k->get_keycode() != 0) {
last_wait_for_key = k;
const String str = keycode_get_string(k->get_keycode_with_modifiers());
press_a_key_label->set_text(str);
press_a_key->set_input_as_handled();
}
}

I think being able to assign BUTTON_XBUTTON1 and BUTTON_XBUTTON2 would be helpful, especially for back/forward editor navigation. In order to not let people mess up the general usage of the editor, I think it may be a case of only allowing the above 2 mouse button input types to be assigned... i.e. not allowing rebinding of MB1, MB2, scroll, etc (unless they are modified...? e.g. with shift, alt, ctrl).

Making this change would be relatively simple (I think - haven't looked into it too deep), but it would require changing a decent amount of existing code within the file above, since last_wait_for_key would need it's type altered.

Happy to work on a PR if the general consensus is that this is worthwhile.

@Calinou
Copy link
Member

Calinou commented Apr 29, 2020

See also #36910, where I had to hardcode the mouse buttons due to this limitation (in addition to not being able to assign multiple shortcuts to the same action).

This would also be helpful to implement #19772.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 29, 2020

Duplicate of #6648 basically, but it's more detailed.

@EricEzaM
Copy link
Contributor Author

EricEzaM commented Apr 30, 2020

Yeah so I think the issue with the solutions mentioned so far is that they limit flexibility. If the button is hard-coded then people who use those for Push To Talk wouldn't be happy, as some have already expressed in those issues.

This would allow for more opions, as those people could even rebind the functionality to ctrl/alt/shit + Mouse Button X in order to have both functions available to them.

I will start to implement this change and keep this issue up to date if I run into any hurdles... if not, expect a PR soon :D

@Calinou Calinou changed the title Can't assign mouse buttons to editor shortcuts Can't assign mouse buttons or mouse wheel to editor shortcuts Apr 30, 2020
@EricEzaM
Copy link
Contributor Author

EricEzaM commented Apr 30, 2020

@Calinou @KoBeWi So I pretty much have this done (shortcuts working - tested after assigning):

godot windows tools 64_XOADxDieJF

Only one issue, as below. The shortcuts use as_text() on the InputEvent to get the display information. Fine for keyboard... not so fine for mouse, as you can see.
godot windows tools 64_hpMSfAhq7D

I have thought about solutions and have considered:

  1. Adding another virtual method to InputEvent to get the text (not preferred)
  2. Changing as_text() to as_text(bool p_readable) or something similar. Where p_readable=false is what it gives now, and p_readable = true would give the same thing given in the first image above. The exact wording of the parameter could be anything of course.

As far as I can tell as_text() is only used in shortcut.cpp, in get_as_text() so it would not have wide reaching changes, but it would need changes to each of the InputEvent[Type] classes which inherit from InputEvent.

Keen to hear thoughts.

@Calinou
Copy link
Member

Calinou commented Apr 30, 2020

@EricEzaM I think it would be better to make as_text() return an human-readable string for all input event types. If we still need a debugging-friendly conversion, it could be done as an implicit _to_string() conversion (like array/dictionary types).

@EricEzaM
Copy link
Contributor Author

EricEzaM commented May 1, 2020

Bit of an update...

Using keys only for shortcuts is quite deeply ingrained in the engine code... Most checks for shortcuts happen on _unhandled_key_input(...) which clearly is not helpful when it comes to implementing non-key shortcuts. Additionally, some checks occur under _unhandled_input(...), which would be fine if the viewport didnt mark all mouse button events as handled after it was sent to _gui_input(...). Meaning that to get this working without changing the viewport input event handling methods, all methods where shortcuts are checked (in buttons, etc) need to be changed to _input(...).

So basically everywhere where shortcuts are read (so far only seems to be <5 classes) key input needs to be changed to check for mouse button input also. PR to come soon I hope.

@EricEzaM
Copy link
Contributor Author

EricEzaM commented Jan 3, 2021

FYI reduz is against this change currently due to fundamental differences bwteen how KB and Mouse input is handled.

Or at least he was several months ago when this was discussed on IRC.

@me2beats
Copy link

me2beats commented Jan 3, 2021

I think this is a useful feature.
Without going into implementation details, I can assume that for the end user, keyboard keys and mouse buttons are all buttons.
And assigning actions to mouse buttons is a fairly common practice.

If it is difficult to implement within the current editor's shortcut system architecture, then work-rounds could be provided, for example, ability to create a custom/additional shortcut system.

btw, what about something similar to InputMap?
That is, all shortcuts assignments could be stored in EditorSetting, etc.

@MagdielM
Copy link

MagdielM commented Feb 16, 2021

I'd find this particularly useful to rebind some rather irritating bindings that I can't change as of now. At least for my mouse, tilting the mouse wheel is bound to panning the graph editor, which happens often on accident when I try to click the mouse wheel in to pan by dragging. Often, this ends up happening:

2021-02-16.12-15-11.mp4

@Calinou
Copy link
Member

Calinou commented Feb 16, 2021

@MagdielM If this happens to you, it might be worth using dedicated software to disable the side mouse wheel buttons. This way, you won't experience this issue anymore in any software.

@Dok11
Copy link

Dok11 commented Sep 15, 2024

I would like to set shortcut for next/prev editor tabs via Ctrl+Shift+MouseWheelUp/Down. Does this issue relative for this?

@Calinou
Copy link
Member

Calinou commented Sep 16, 2024

I would like to set shortcut for next/prev editor tabs via Ctrl+Shift+MouseWheelUp/Down. Does this issue relative for this?

Yes, this issue is about any editor shortcut that can be configured. These shortcuts can be configured:

image

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

Successfully merging a pull request may close this issue.

6 participants