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

Add 3D translation sensitivity to Editor Settings #81714

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

CreepGin
Copy link
Contributor

@CreepGin CreepGin commented Sep 15, 2023

This for 3D Navigation. Currently in Godot, you cannot adjust Panning sensitivity. So for people who uses non-default Orbit sensitivity, the default panning speed will most likely feel off. This PR adds a Translation Sensitivity option that fixes this issue.

The max value increase also help addressing #75855 (for trackpad users). Original code change comes from @geminax from that thread. All credit should go to him; I just tested and prepared the PR.

I also want to add a bit more context as to why this is important to me. I have 15+ years of muscle memory from Maya and Unity. I tried to get into Godot in the past but the 3D navigation always felt off on my setup. I know Godot has been improving on this end as well. There are now more options I can tweak in this regard. And I think this Translation Sensitivity setting is the last missing piece I need.

@CreepGin CreepGin requested a review from a team as a code owner September 15, 2023 22:01

real_t pan_speed = 1 / 150.0;
real_t pan_speed = 4 / 150.0 * translation_sensitivity;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed 1 to 4 since default translation_sensitivity is 0.25

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies in advance for the belated review - this PR should be mostly ready to merge, but I wanted to flag this. This seems to be fairly arbitrary, if I understand correctly 0.25 was chosen as the default simply because that's what orbit_sensitivity uses?

orbit_sensitivity seems to be in "degrees per pixel", so it's actually a meaningful unit.

Here translation_sensitivity seems to be in 1/37.5-th pixels per second (I assume), which seems pretty arbitrary.

Maybe we can make this setting actually be the full pixels per second value? Or maybe with a factor 100 because editing small decimals precisely is tricky.

So:

Suggested change
real_t pan_speed = 4 / 150.0 * translation_sensitivity;
real_t pan_speed = translation_sensitivity / 100.0;

And change the default value to 0.667?

WDYT @Calinou?

Copy link
Member

@Calinou Calinou Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can make this setting actually be the full pixels per second value?

There is no time unit when panning, so it can't be "pixels per second" but would be something like "units per pixel" instead.

I would prefer we make the default value 1.0 instead as these units are often difficult for users to understand. We could do the same for orbit and freelook sensitivity, but we'll need to multiply the old value by 4 when migrating editor settings to ensure the sensitivity doesn't suddenly decrease after upgrading.

Many games also use arbitrary values for mouse sensitivity (3.0 is the default in most id Tech and Source games), so there is no real standard here.

The only measurement that is somewhat standard is measuring how much distance your mouse physically needs to travel to perform a 360-degree turn, but this can't be done in software.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR to change the default translation sensitivity to 1.0, and thus make the formula translation_sensitivity / 150.0.

@CreepGin
Copy link
Contributor Author

CreepGin commented Sep 15, 2023

Squashed everything and added missing entry to doc/classes/EditorSettings.xml

consider reporting if you notice unrelated files being updated

So after running --doctool, I get 4 other xml files also being updated (should be unrelated to this PR). Not sure if I should report here, or elsewhere, or ignore.

@CreepGin CreepGin force-pushed the navigation_feel branch 2 times, most recently from 3aa8055 to 3388057 Compare September 16, 2023 01:21
@AThousandShips
Copy link
Member

Are you building a double precision build? Then that's the reason, not a bug in that case

@CreepGin
Copy link
Contributor Author

CreepGin commented Sep 16, 2023

Are you building a double precision build? Then that's the reason, not a bug in that case

Thanks for the info! Yes, you are right. After building and using the x86_32 version, no more extraneous xml updates were generated for me.

So I think the doc is making it slightly confusing then. On the Updating class reference when working on the engine doc page, it specifically gives an example of running --doctool with the x86_64 version.
image

If I'm not mistaken there, I can also try to contribute to the doc repo (adding a note about the possible doc generation difference between x86_32 and x86_64).

@AThousandShips
Copy link
Member

Meant the --precision=double option, it often changes the docs by using for example PackedFloat64Array over PackedFloat32Array, and changing some default values

@Calinou
Copy link
Member

Calinou commented Sep 16, 2023

If I'm not mistaken there, I can also try to contribute to the doc repo (adding a note about the possible doc generation difference between x86_32 and x86_64).

Documentation output should be identical between 32-bit and 64-bit builds. (This isn't to be confused with the floating-point precision build-time option, which is entirely independent of the architecture's bitness.)

@CreepGin
Copy link
Contributor Author

Documentation output should be identical between 32-bit and 64-bit builds

On my end, running .\bin\godot.windows.editor.x86_64.exe --doctool generates 4 more updated xml files:

CreepGin@2e23ca2

Running the x86_32 version does not produce any extra updates.

@AThousandShips
Copy link
Member

AThousandShips commented Sep 16, 2023

Those are built with a different version I'm 99% sure, the redundant_for_variable_type setting was removed two days ago, and environment_blend_mode was added two days ago, have you built both versions from the same branch?

@CreepGin
Copy link
Contributor Author

have you built both versions from the same branch?

Yes

@AThousandShips
Copy link
Member

AThousandShips commented Sep 16, 2023

Just to be sure build both again and see, because all these changes were from PRs merged in the last two days, so it'd be a fantastic coincidence

In fact they are all changes from:
8ecc0c4
76fad10
3806d96

@CreepGin
Copy link
Contributor Author

Okay finally got it. It's definitely my silly mistake. I must've mixed up the exe names (with/without ".dev") for some of the runs. Sorry for the trouble! 😓

@AThousandShips
Copy link
Member

Happy it got worked out! It's an extremely easy mistake that I've done myself a few times

@YuriSizov YuriSizov modified the milestones: 4.2, 4.x Nov 9, 2023
@SephReed
Copy link

SephReed commented Nov 3, 2024

Hey! Who forgot about this? It's almost done and kind of a big deal for anybody working from a lap top with experience in other 3d software. It's not necessary by any means, but it's pretty bad feeling to not be able to navigate comfortably.

@CreepGin CreepGin requested a review from a team as a code owner November 3, 2024 12:08
@CreepGin
Copy link
Contributor Author

CreepGin commented Nov 3, 2024

I think it was ignored for a long time. Let me know if I can do anything more. I just pushed to the PR to resolve a recent conflict.

@vistuleB
Copy link

vistuleB commented Nov 8, 2024

I'm waiting for this. When will this be released?

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good to merge but I left a comment on the formula used for the pan speed.

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Nov 29, 2024
@AThousandShips AThousandShips changed the title Add Translation Sensitivity to Editor Settings Add 3D Translation Sensitivity to Editor Settings Dec 3, 2024
@veryyynice
Copy link

is it a matter of waiting for next release?

@akien-mga akien-mga changed the title Add 3D Translation Sensitivity to Editor Settings Add 3D translation sensitivity to Editor Settings Dec 5, 2024
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

I updated the PR to change the default translation sensitivity to 1.0, and thus make the formula translation_sensitivity / 150.0.

@Repiteo Repiteo merged commit fb4d401 into godotengine:master Dec 9, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 9, 2024

Thanks! Congratulations on your first merged contribution! 🎉

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

Successfully merging this pull request may close these issues.

9 participants