Walk Navigation: Add local up/down movement with R/F #111682

Aaron Franke · 2023-08-30T04:35:29+02:00

Aaron Franke commented

2023-08-30 04:35:29 +02:00

This PR is a minor usability enhancement for the walk navigation mode. Currently walk navigation has local movement for forward/back/left/right, but only global movement for up/down. This PR adds local up/down movement, bound to the R and F keys.

Justification:

Why have local up/down movement: This is a common feature in many 3D editors (ex: Unity, Godot) and 6DoF games (ex: Space Engineers, Pioneer, Elite: Dangerous), and it provides feature parity with the forward/back/left/right movement which are local.
Why use R/F for this: R/F are a common choice in 6DoF games (ex: Elite: Dangerous). They are easy to remember because R is above F and therefore R must be up and F must be down. Also, these keys are otherwise unused in walk navigation, so binding them does not negatively affect anyone.
Why keep E/Q for global up/down movement: The simple answer is for preserving the existing behavior in Blender. Also, E/Q for global up/down is the same as Unreal. But even if it wasn't for compatibility or consistency with other development tools, I would argue that E/Q for global and R/F for local is the best choice because of parity with 6DoF games.
- A common choice for 6DoF games is to use Q/E for roll left/right (ex: Space Engineers, Pioneer). Since walk navigation is typically 5DoF, we don't have roll and so have the Q and E keys free, yet with 5DoF a global up is a sensible concept to allow moving along, so it makes sense to use E/Q for global up/down movement.
- If Blender ever has a full 6DoF mode, this means that Q/E can be used for roll left/right, while keeping R/F consistent with 5DoF walk navigation.

This PR also renames the global up/down enums internally (but the keybind names are still the same, and the values are still the same, so compatibility is preserved). This PR also fixes a bug where if you press both up/down at the same time down was taking priority (direction = 1; changed to direction += 1; to match the other code that uses +=).

Documentation: blender/blender-manual#104538

I have compiled and tested this PR to ensure it works correctly. I've attached a screenshot of what the control hints look like.

This PR is a minor usability enhancement for the walk navigation mode. Currently walk navigation has local movement for forward/back/left/right, but only global movement for up/down. This PR adds local up/down movement, bound to the R and F keys. Justification: * Why have local up/down movement: This is a common feature in many 3D editors (ex: Unity, Godot) and 6DoF games (ex: Space Engineers, Pioneer, Elite: Dangerous), and it provides feature parity with the forward/back/left/right movement which are local. * Why use R/F for this: R/F are a common choice in 6DoF games (ex: Elite: Dangerous). They are easy to remember because R is above F and therefore R must be up and F must be down. Also, these keys are otherwise unused in walk navigation, so binding them does not negatively affect anyone. * Why keep E/Q for global up/down movement: The simple answer is for preserving the existing behavior in Blender. Also, E/Q for global up/down is the same as Unreal. But even if it wasn't for compatibility or consistency with other development tools, I would argue that E/Q for global and R/F for local is the best choice because of parity with 6DoF games. * A common choice for 6DoF games is to use Q/E for roll left/right (ex: Space Engineers, Pioneer). Since walk navigation is typically 5DoF, we don't have roll and so have the Q and E keys free, yet with 5DoF a global up is a sensible concept to allow moving along, so it makes sense to use E/Q for global up/down movement. * If Blender ever has a full 6DoF mode, this means that Q/E can be used for roll left/right, while keeping R/F consistent with 5DoF walk navigation. This PR also renames the global up/down enums internally (but the keybind names are still the same, and the values are still the same, so compatibility is preserved). This PR also fixes a bug where if you press both up/down at the same time down was taking priority (`direction = 1;` changed to `direction += 1;` to match the other code that uses `+=`). Documentation: https://projects.blender.org/blender/blender-manual/pulls/104538 I have compiled and tested this PR to ensure it works correctly. I've attached a screenshot of what the control hints look like.

Screenshot 2023-08-29 at 9.12.10 PM.png

114 KiB

Aaron Franke referenced this pull request from blender/blender-manual

2023-08-30 05:06:56 +02:00

Walk Navigation: Add local up/down movement #104538

Aaron Franke changed title from ~~Walk Navigation: Add local up/down movement~~ to Walk Navigation: Add local up/down movement with R/F

2023-08-30 23:40:10 +02:00

Pratik Borhade reviewed 2023-09-18 13:55:52 +02:00

source/blender/editors/space_view3d/view3d_navigate_walk.cc Outdated

						
				@ -1208,2 +1230,3 @@

				        if (walk->navigation_mode == WALK_MODE_FREE) {

				          if (walk->navigation_mode == WALK_MODE_FREE) {

				          if ((walk->active_directions & WALK_BIT_GLOBAL_UP) || (walk->active_directions & WALK_BIT_GLOBAL_DOWN)) {

Pratik Borhade commented

2023-09-18 13:55:52 +02:00

Maybe use ELEM(walk->active_directions, WALK_BIT_LOCAL_UP, WALK_BIT_LOCAL_DOWN) ?

Maybe use `ELEM(walk->active_directions, WALK_BIT_LOCAL_UP, WALK_BIT_LOCAL_DOWN)` ?

Aaron Franke commented

2023-09-18 18:13:29 +02:00

I don't think these are equivalent, ELEM is for checking equality, this is bitwise.

For now I am following the existing code style.

I don't think these are equivalent, `ELEM` is for checking equality, this is bitwise. For now I am following the existing code style.

Campbell Barton commented

2023-09-26 14:22:39 +02:00

Can be (walk->active_directions & (WALK_BIT_GLOBAL_UP | WALK_BIT_GLOBAL_DOWN)).

Can be `(walk->active_directions & (WALK_BIT_GLOBAL_UP | WALK_BIT_GLOBAL_DOWN))`.

Aaron Franke commented

2023-09-28 05:47:59 +02:00

Good idea, might be faster that way and it's more readable. I have pushed a change to do this.

Pratik Borhade reviewed 2023-09-18 13:56:09 +02:00

Pratik Borhade left a comment

Hi, thanks for the PR. Change looks correct to me.

I'll tag @dfelinto for final review (since he is the author of walk/fly nav)

Hi, thanks for the PR. Change looks correct to me. I'll tag @dfelinto for final review (since he is the author of walk/fly nav)

Campbell Barton reviewed 2023-09-26 14:26:30 +02:00

Campbell Barton left a comment

This seems OK, having both global/local directions for all directions makes sense although personally I find it a bit obscure - on a user level. I'd be interested in a second opinion.

Aaron Franke commented

2023-09-28 05:42:29 +02:00

@ideasman42 This isn't both global/local for all directions, only up/down.

In 5DoF movement, the vertical axis is special because it's what the camera pivots around. The same cannot be said about the horizontal axes. So it makes sense to have global up/down, but less so global horizontal movement. But regardless, this PR is adding local up/down, which makes sense in all situations and is consistent with the existing local horizontal movement.

@ideasman42 This isn't both global/local for all directions, only up/down. In 5DoF movement, the vertical axis is special because it's what the camera pivots around. The same cannot be said about the horizontal axes. So it makes sense to have global up/down, but less so global horizontal movement. But regardless, this PR is adding local up/down, which makes sense in all situations and is consistent with the existing local horizontal movement.

Aaron Franke force-pushed walk-nav-local-up-rf from 3209540f3e to a5f94f5ff9

2023-09-28 05:46:49 +02:00

Compare

Aaron Franke force-pushed walk-nav-local-up-rf from a5f94f5ff9 to 33781473eb

2023-11-03 03:13:24 +01:00

Compare

Aaron Franke commented

2023-11-03 03:23:26 +01:00

I rebased and re-tested this pull request on the latest Blender main branch (now 4.1.0 alpha).

Aaron Franke requested review from Campbell Barton 2023-11-03 04:49:04 +01:00

Aaron Franke requested review from Dalai Felinto 2023-11-03 04:49:04 +01:00

Aaron Franke requested review from Pratik Borhade 2023-11-03 04:49:04 +01:00

Dalai Felinto commented

2023-11-03 10:16:08 +01:00

I think this feature is ok. I think it is harmless to have it and I could see its use. Let me look at the code quickly, but as far as the feature is concern, my late over-thought +1 :)

Since he is the author of walk/fly nav

For the records, Campbell is the author of fly navigation :) (I was the author of walk navigation though indeed).

I think this feature is ok. I think it is harmless to have it and I could see its use. Let me look at the code quickly, but as far as the feature is concern, my late over-thought +1 :) > Since he is the author of walk/fly nav For the records, Campbell is the author of fly navigation :) (I was the author of walk navigation though indeed).

👍 1

Dalai Felinto approved these changes 2023-11-03 10:22:13 +01:00

Dalai Felinto left a comment

The code seems fine. I will wait Campbell for a final review

Pratik Borhade refused to review 2023-11-03 10:53:42 +01:00

Campbell Barton referenced this issue from a commit

2023-11-05 07:28:29 +01:00

3D View: add local up/down movement to walk navigation

Campbell Barton manually merged commit c62009a6ac into main

2023-11-05 07:29:08 +01:00

Campbell Barton approved these changes 2023-11-05 07:29:23 +01:00

John Cox commented

2023-11-05 12:15:04 +01:00

First-time contributor

Hi, This seems to break Gravity option:

Enter Walk > Press g or TAB > view becomes stuck under the grid, keyboard input does nothing, mouse still works.

Hi, This seems to break Gravity option: Enter Walk > Press g or TAB > view becomes stuck under the grid, keyboard input does nothing, mouse still works.

Campbell Barton commented

2023-11-05 12:35:45 +01:00

Hi, This seems to break Gravity option:

Enter Walk > Press g or TAB > view becomes stuck under the grid, keyboard input does nothing, mouse still works.

Thanks for letting me know, this was an error in a following cleanup, corrected: 361bd68f21

> Hi, This seems to break Gravity option: > > Enter Walk > Press g or TAB > view becomes stuck under the grid, keyboard input does nothing, mouse still works. Thanks for letting me know, this was an error in a following cleanup, corrected: 361bd68f21c9dd65c205d602c47a4487739ed272

Aaron Franke deleted branch walk-nav-local-up-rf

2023-12-04 08:03:04 +01:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Walk Navigation: Add local up/down movement with R/F #111682