godot icon indicating copy to clipboard operation
godot copied to clipboard

Add full customization of 3D navigation controls

Open RobProductions opened this issue 2 years ago • 9 comments

Implements this proposal: godotengine/godot-proposals#6406 which in turn satisfies godotengine/godot-proposals#3646 as well.

The goal is to allow users to freely customize which mouse buttons/keys navigate the 3D viewport in the editor. Previously, the spatial editor controls were limited to hardcoded button checks based on the navigation_scheme setting. With this change, navigation_scheme instead controls 3 new mouse button settings and has a new CUSTOM setting which allows users to adjust the mouse buttons however they want:

https://github.com/godotengine/godot/assets/11141862/283d48c0-98c8-4698-8ba5-0a7cdb2e7e13

In order to properly set up user settings (in case they have an old version of Godot with "Modo" or "Maya" selected), I figured it was best to inject the preset detection call into editor_node.cpp so that it gets checked when the editor opens. It also protects against users manually editing their editor_settings-4.tres file to remove mouse settings for example, as that would be confusing to view in the settings dialog. If we don't care about these cases/have a different way to approach it feel free to let me know and I can adjust it.

To tackle modifiers, the previous orbit, pan, and zoom settings were moved into Shortcuts instead because of the existing flexibility with that system to add multiple keys/any key desired:

image

I wanted to use just one modifier for each operation, but due to a bug (or intended behavior?) in InputEventKey modifiers, pressing a modifier with it's "main" key and then releasing the modifier will still count the button as being pressed. This Reddit post explains it best I think. Another benefit of 2 modifiers is being able to set 2 different "standard" keys like Z/X as opposed to being limited to CTRL, ALT, etc. Like the mouse settings, these will be set by the navigation_scheme preset and overriding them will swap the scheme to Custom.

With these new settings in place, the user retains all the same navigation options as before and can easily access them through the preset, but they also acquire a bunch of new ways to customize the controls to their liking:

https://github.com/godotengine/godot/assets/11141862/606d8937-c666-49d0-98b8-177bfa62c76f

I've also run the doc generation tool and filled out the descriptions for the new mouse settings, as well as updating the navigation_scheme description to clearly state the new behavior. It seems like there aren't any unit tests for the editor yet but I made sure to run them locally just in case. I've checked tablet navigation on my Wacom as well, and ensured the Emulate 3-Button Mouse setting still works when the shortcuts are set properly.

Due to code ordering complications, I made the assumption that there are less orbit modifiers than pan modifiers for any usage of the same mouse button, otherwise one of the modes could be skipped over. This is true for all of the presets previously available, so it will not cause any regression; however if the user adjusts orbit modifiers in such a way that they should be detected first, it might get skipped. In the interest of creating a smaller PR to review, I've left it at that for now hoping the ordering will be suitable for the majority of use cases. In the future I can look into creating another PR that checks the modes in a more correct order based on how many shortcut keys are set for each op, though it will require a few more helper functions.

Since this makes the navigation code more generic, it becomes a lot easier to add in more presets. If this is approved, I'd love to see/work on more PRs that add in Blender/Unreal presets, or add in the ability to use mouse buttons 4 and 5. For now though, I wanted to make sure this way of doing it was functionally complete and looks good to everyone. This is my first time submitting code to Godot so please let me know if there's anything else I missed or if I can clarify anything further :) thanks!

RobProductions avatar Nov 24 '23 23:11 RobProductions

You need to run --doctool to update the documentation with the new settings: https://docs.godotengine.org/en/stable/contributing/documentation/updating_the_class_reference.html#updating-class-reference-when-working-on-the-engine

YeldhamDev avatar Nov 26 '23 04:11 YeldhamDev

There was another style error. You can set up a git hook to check for those locally: https://docs.godotengine.org/en/stable/contributing/development/code_style_guidelines.html#using-clang-format-locally

YeldhamDev avatar Nov 26 '23 04:11 YeldhamDev

You need to run --doctool to update the documentation with the new settings

Actually I did but the doctool doesn't seem remove references to deleted members in other descriptions, oops. My bad for not catching it before though! And thanks for the info on the local styling tests, I'll check that out. Appreciate your patience with me setting this stuff up for the first time :)

RobProductions avatar Nov 26 '23 04:11 RobProductions

Rebased to handle merge conflict

RobProductions avatar Jan 30 '24 19:01 RobProductions

Done, thank you! Feel free to check out the updated code :)

RobProductions avatar Jan 31 '24 18:01 RobProductions

Looks good! Haven't tested and this isn't my area but reads a lot better 🙂

AThousandShips avatar Jan 31 '24 18:01 AThousandShips

This is very nice addition! But I'm not sure about control separation (mouse buttons in Editor Settings, modifier keys in Shortcuts), it might break UX consistency. Maybe better to move all navigation controls to Editor Settings (Marvelous Designer) or Shortcuts (Blender)? For example, Marvelous Designer's View Controls: KPpcyCSiSQ

I wanted to use just one modifier for each operation, but due to a bug (or intended behavior?) in InputEventKey modifiers, pressing a modifier with it's "main" key and then releasing the modifier will still count the button as being pressed.

If I understand it right, that way is used in Unity, Blender and Maya. Unreal works like Godot. But, yeah, if current behavior is preferred, it should be fixed.

Broken1334 avatar Mar 18 '24 08:03 Broken1334

This is very nice addition! But I'm not sure about control separation (mouse buttons in Editor Settings, modifier keys in Shortcuts), it might break UX consistency. Maybe better to move all navigation controls to Editor Settings (Marvelous Designer) or Shortcuts (Blender)?

Thanks! Yeah, that would be ideal but I don't know of an easy way to bring shortcut controls into the regular settings dialog or the other way around. The way the code is currently built, it'll automatically populate the shortcut menu with all the controls once they're registered so there's no quick way I can think of to insert a dropdown in between them. This would be a great addition for a future PR but would have to come in multiple steps, i.e. adding support for displaying shortcuts in the general settings tab in a specific order and then implementing that for these controls.

For now it's at least functional for those that need it and the presets should be easy enough to use for anyone that just needs the same behavior as the current build :)

If I understand it right, that way is used in Unity, Blender and Maya. Unreal works like Godot. But, yeah, if current behavior is preferred, it should be fixed.

It is a bit jarring to work with as it forces you to do more to work around it (hence the multiple shortcuts you see here) so I would like to see it addressed, though I don't know if it'll need to be an option to keep the original behavior for compatibility purposes. Either way, that's out of my scope for now but if it is fixed I can come back to this and simplify these controls.

I just rebased on latest master to handle the merge conflict and tested to make sure it's still working, let me know if there's anything else I should do here!

RobProductions avatar Mar 18 '24 16:03 RobProductions

Rebased! Any chance this can get a review before it gets too stale?

RobProductions avatar Jun 27 '24 19:06 RobProductions

@Calinou Gotcha, I definitely see how it's useful to preview the shortcuts alongside the mouse button though personally it's a little confusing seeing them in the "value" section of what's supposed to be a mouse button setting. Maybe in the future we can add a separate label generated by the different values to keep the mouse button selection clean?

Anyways, your idea sounds good for now and I think I've figured it out, take a look:

image

And when you update the preset or the shortcuts, the hint values for the mouse buttons are also updated automatically. It worked like you described because the enum is int based:

image

The only caveat was finding a good time to refresh the tree so there can be a small delay after changing the setting for it to update the hint (it relies on a notification), but it's not worse than the usual delays you see in the settings screen so should be functional for now! Finally, while I was at it I also fixed the code ordering issue that I explained in the original post in order to reduce work for later PRs. Now you can have any amount of shortcuts on orbit, pan, and zoom, and the action with the least amount of shortcuts is always checked first so nothing should ever get skipped.

Feel free to check out the updated code and let me know what you think :) Thanks!

RobProductions avatar Jul 01 '24 23:07 RobProductions

@AThousandShips Thanks for the review and guidance! I've updated the code with all your suggestions, let me know if this looks better now

RobProductions avatar Jul 02 '24 16:07 RobProductions

Thanks!

akien-mga avatar Aug 27 '24 22:08 akien-mga

Hello! I am testing this in 4.4dev2. I am using a pen tablet so I don't have a middle click and use Emulate 3 Button Mouse.

Would it be possible to keep the default for Emulate 3 Button Mouse to ALT for orbit, SHIFT for pan and CTRL for zoom? Otherwise, it can be very confusing when we open 4.4 for the first time: the view is rotating without us touching anything.

Let me know if I should let my feedback somewhere else.

Nodragem avatar Sep 12 '24 09:09 Nodragem

Hi @Nodragem , in order to maximize compatibility with these new customization features, I think its best if the Emulate 3 Button Mouse setting does exactly what it says in the docs:

If true, enables 3-button mouse emulation mode. This is useful on laptops when using a trackpad. When 3-button mouse emulation mode is enabled, the pan, zoom and orbit modifiers can always be used in the 3D editor viewport, even when not holding down any mouse button.

Otherwise people who want to customize orbit, pan, and zoom keys for tablet would be forced into ALT, SHIFT, and CTRL even if they have something different selected in their settings. Unfortunately the view rotating in your case is a side effect of the default navigation scheme being Godot which uses no keyboard press for orbit, only mouse input, so when 3 button mouse mode comes along and eliminates the need for mouse input, it will always rotate. To fix this you can change the preset to Modo or customize the Viewport Orbit Modifier in the Shortcuts tab so that a key is used.

The only way I could see the "default" improving without eliminating the customization is if we had a different setting track the "3 button mouse"-specific modifier keys separately from standard keys, but that seems a little excessive. In my opinion, if you're changing 3 button mouse from the default state of "off" it's hopefully fine to simply change the preset as well; to me that's preferable over losing 3 button mouse key customization.

Alternatively, I could add a Tablet preset with your requested hotkeys to sit alongside Godot and the others. Though it'll still require an extra setting switch in addition to 3 button mouse, it should make things more clear to Tablet users that they should be using it instead of Godot. Does that sound like a good solution?

RobProductions avatar Sep 12 '24 18:09 RobProductions

Default rotating by itself with 3 button emulation is really bad UX. Can Modo be made default?

Zireael07 avatar Sep 13 '24 08:09 Zireael07

Alternatively, I could add a Tablet preset with your requested hotkeys to sit alongside Godot and the others. Though it'll still require an extra setting switch in addition to 3 button mouse, it should make things more clear to Tablet users that they should be using it instead of Godot. Does that sound like a good solution?

I think it is also an issue with trackpad/laptop.

The default behaviour in 4.3 was to have ALT pressed to emulate the middle mouse click. Otherwise, the camera is rotating while you are using the trackpad, or hovering the pen over the tablet, or simply moving the mouse.

Maybe when selecting emulate 3 button mouse, a new option key to emulate middle mouse could be unfolded. This option would be set to the default ALT when in godot mode. And we can change the default to whichever key depending on the mode (modo, maya)? what do you think?

Edit: forget about my idea. Thinking of it, your idea of adding a Tablet preset is better. Just need to automatically activate it when the user click on Emulate 3 Button mouse. Might need to rename the preset to Laptop/Pen Tablet.

Nodragem avatar Sep 13 '24 09:09 Nodragem

I am on trackpad and you're right, in previous versions I needed to press ALT to emulate middle mouse and I didn't have the camera rotate iirc

Zireael07 avatar Sep 13 '24 10:09 Zireael07

Default rotating by itself with 3 button emulation is really bad UX. Can Modo be made default?

I don't mind going that route but I'm not sure if Godot users/maintainers will be okay with the Godot preset no longer being the default for non-trackpad users when you open the engine for the first time.

I am on trackpad and you're right, in previous versions I needed to press ALT to emulate middle mouse and I didn't have the camera rotate iirc

Yeah, that's because it was hardcoded before - when 3 Button Mouse was on, it made assumptions about the scheme and just manually checked for ALT, or something like that, while ignoring mouse input. To adhere to the doc original description, "even when not holding down any mouse button" it now checks for your custom keybind while ignoring mouse input, like before except now the default scheme of Godot uses "no keybind" for orbit. If we change it back to ALT then 3 button mouse users lose nav customization. imo keeping the customization is preferable, but maybe we can do something where it warns the users that no key is currently set to Orbit while this button is on, prompting them to set a key or change scheme to Tablet as suggested above.

Perhaps "Emulate 3 Button Mouse" is better renamed to "Simulate mouse input" because that's what it was always doing? Maybe it was named that before because it kind of seemed like it was emulating just the middle mouse, but in reality it made assumptions and forced you to use the "global orbit modifier" even if your "navigation scheme" ignored it.

Thinking of it, your idea of adding a Tablet preset is better. Just need to automatically activate it when the user click on Emulate 3 Button mouse. Might need to rename the preset to Laptop/Pen Tablet.

Okay, thanks for the input! However I am weary to automatically set preset when clicking 3 button mouse to on - it seems a little unintuitive for it to change based on that, especially if a 3 button mouse user wants Maya or Modo setting instead, we're now forcing them to go through redundant clicks. What if it worked the other way around? When you set preset to Tablet (or whatever name we land on) it also activates 3 button mouse, so it just requires one click as before but you simply change preset instead of that toggle. If we go this route, I could also tie in 3 button mouse to the preset detection so that changing it from off to on while using Godot, Maya, or Modo will swap preset to "custom" and make it more clear that you're doing something unexpected. Likewise, changing it to off while in Tablet will also make the preset switch to "custom". Does this still sound good?

RobProductions avatar Sep 13 '24 20:09 RobProductions