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

PointerLockControls: Add .pointerSpeed. #23516

Merged
merged 5 commits into from
Feb 18, 2022
Merged

Conversation

ChrisCrossCrash
Copy link
Contributor

Related issue: #23513

Description

This PR adds a new mouseSpeed property to PointerLockControls. Changing this property changes the mouse sensitivity for controlling the camera look direction. The PR also adds a new optional options parameter to the constructor for setting mouseSpeed when a new PointerLockControls instance is made.

`mouseSpeed` controls the mouse sensitivity for `PointerLockControls`.
This change also adds a new optional `options` parameter to the
constructor of `PointerLockControls`.
@WestLangley
Copy link
Collaborator

The PR also adds a new optional options parameter to the constructor

We do not have that option in the the other controls, so I'd omit it.

This PR adds a new mouseSpeed property

As implemented in this PR, the units of mouseSpeed are radians per pixel. Don't forget to update the docs.

//

Or, maybe better, implement it like so:

_euler.y -= movementX * 0.002 * mouseSpeed;

In that case, mouseSpeed would be unit-less, and have a default value of 1.

I'd defer to @mrdoob for the choice of parameterization.

@mrdoob
Copy link
Owner

mrdoob commented Feb 18, 2022

The PR also adds a new optional options parameter to the constructor

We do not have that option in the the other controls, so I'd omit it.

Agreed.

Or, maybe better, implement it like so:

_euler.y -= movementX * 0.002 * mouseSpeed;

In that case, mouseSpeed would be unit-less, and have a default value of 1.

Agreed too.

@mrdoob mrdoob added this to the r138 milestone Feb 18, 2022
@mrdoob
Copy link
Owner

mrdoob commented Feb 18, 2022

I think I can make these changes directly here... 🤔

@mrdoob
Copy link
Owner

mrdoob commented Feb 18, 2022

One more issue... Should it be pointerSpeed instead of mouseSpeed?

@WestLangley
Copy link
Collaborator

Should it be pointerSpeed instead of mouseSpeed?

Maybe trackingSpeed? That is the term macOS uses for the mouse sensitivity parameter.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 18, 2022

I vote for pointerSpeed. Fits best to PointerLockControls.

@ChrisCrossCrash
Copy link
Contributor Author

These are all great ideas! I've added two more commits.

The first renames mouseSpeed to pointerSpeed. I tend to agree with @Mugen87 that it's a nice fit because it goes well with PointerLockControls. I can change it if you prefer.

The second commit adds documentation for the pointerSpeed property.

Please let me know if there's anything else you would like me to change, or feel free to make your own changes.

@mrdoob mrdoob merged commit 2a5f972 into mrdoob:dev Feb 18, 2022
@mrdoob
Copy link
Owner

mrdoob commented Feb 18, 2022

Thanks!

@Mugen87 Mugen87 changed the title Add mouseSpeed property to PointerLockControls PointerLockControls: Add .pointerSpeed . Feb 18, 2022
@Mugen87 Mugen87 changed the title PointerLockControls: Add .pointerSpeed . PointerLockControls: Add .pointerSpeed. Feb 18, 2022
donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
* Add `mouseSpeed` property to `PointerLockControls`

`mouseSpeed` controls the mouse sensitivity for `PointerLockControls`.
This change also adds a new optional `options` parameter to the
constructor of `PointerLockControls`.

* Update PointerLockControls.js

* Update PointerLockControls.js

* Rename `mouseSpeed` to `pointerSpeed`

* Add `pointerSpeed` property documentation

Co-authored-by: mrdoob <info@mrdoob.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants