FirstPersonControls: Added .lookAt() #15650

Mugen87 · 2019-01-27T11:15:50Z

looeee · 2019-01-27T11:34:41Z

Wow, this is literally the exact thing that's been bugging me for the last half hour. Perfect timing 😀

Mugen87 · 2019-01-27T11:35:58Z

Wonderful! 👍

WestLangley · 2019-01-27T14:54:57Z

examples/js/controls/FirstPersonControls.js

 	this.verticalMax = Math.PI;

 	this.autoSpeedFactor = 0.0;
+	this.forward = new THREE.Vector3( 0, 0, - 1 );


What happens if the user changes this?

Then the forward vector is differently. This is necessary if the user passes a ordinary Object3D to the controls. It would be ( 0, 0, 1 ).

I believe FirstPersonControls is designed to control a camera, so this vector can be hardwired into your method.

if used to control other object, it becomes ThirdPersonControls :)

I believe FirstPersonControls is designed to control a camera

Why put a limit on what it can control, if it's as simple as exposing this.forward?

@looeee Absolutely!

@looeee @Mugen87 I think the proper question is why the hell forward needs to be different for camera vs other objects? Why can't it just be 0,0,-1 for everything?

That's more or less a WebGL/OpenGL convention. (0,0,1) for ordinary objects and (0,0,-1) for cameras, see https://stackoverflow.com/questions/33407209/why-does-the-camera-face-the-negative-end-of-the-z-axis-by-default?rq=1

@looeee I have hardwired the forward vector for now since the controls do not fully support arbitrary objects yet (too bad), see https://jsfiddle.net/q50o1r97/1/. If there is any interest in changing this, I suggest to do this with a separate PR. Same for renaming object to camera in control classes.

@Mugen87 OK, but I think it would be good to have clarity on this. Either we do support controlling arbitrary objects with controls, or we don't.

If we do, then it should be noted somewhere - even just as a comment in the files.

If we don't, then we can just rename object to camera.
I'm OK with either.

WestLangley · 2019-01-27T15:06:53Z

Sorry, I do not see how this can work. It is also not consistent with the three.js API.

Are you saying you can't call setOrientation() without calling camera.lookAt() first?

Is camera.lookAt() and setOrientation() supposed to be called only once prior to initializing the controls?

I hope that is the intention -- users should not be setting the camera behind the back of the controls.

The proper solution is to make setOrientation() private, and to set the camera orientation in the controls constructor.

Mugen87 · 2019-01-27T15:53:20Z

Are you saying you can't call setOrientation() without calling camera.lookAt() first?

No, it's just an example.

Is camera.lookAt() and setOrientation() supposed to be called only once prior to initializing the controls?

You can also call it after initializing the controls.

Sorry, I do not see how this can work. It is also not consistent with the three.js API.

It worked perfectly on my computer. I think the stackoverflow posts show that there is a demand for action. So your attitude seems a bit counterproductive to me.

WestLangley · 2019-01-27T17:23:11Z

In the pattern you are suggesting:

camera.lookAt( targetPosition );
controls.setOrientation( camera.quaternion );

you are setting the camera orientation, and then telling the controls to set the camera orientation. That is what bothers me...

Instead, you can inline setOrientation(), and set the camera orientation in the controls constructor. Doing so would be consistent with the OP's request in #15629.

If you prefer an exposed method, then adding the following, instead, makes sense to me:

controls.lookAt( targetPosition );

The second one may be more useful.

In either case, you can leverage your proposed code.

makc · 2019-01-27T19:06:45Z

@WestLangley they suggested the method rather than inline code because someone might want to change the camera direction after the controls were activated.

users should not be setting the camera behind the back of the controls

apparently users do not think so :)

WestLangley · 2019-01-27T20:26:13Z

Let the controls control the camera.

controls.lookAt( targetPosition );

looeee · 2019-01-28T05:03:51Z

I've had a chance to test this. It's an improvement IMO, but what I would prefer (and what I believe the other controls do) is to respect the camera's orientation and position at the time when the controls are instantiated.

If I do:

camera.lookAt( someObject.position );

const controls = new FirstPersonControls( camera );

The current behaviour is that the camera jumps to a new and unexpected orientation.
With this change, I can now do controls.setOrientation( camera.quaternion ) to fix this, which is an improvement. But I'd rather that just happens automatically, which again, I think is the case for other controls.

Mugen87 · 2019-01-28T11:30:52Z

FirstPersonControls.lookAt() sounds good! I have changed the PR and also implemented @looeee's suggestion. setOrientation() is now a private function and called in the ctor. This means it's now possible to respect the original orientation of the camera.

Mugen87 · 2019-01-28T11:39:32Z

I'll make a note in the migration guide about the changes related to FirstPersonControls when this gets merged.

Mugen87 · 2019-01-28T11:42:07Z

BTW: I have noticed that FirstPersonControls.constrainVertical has somewhat a confusing effect when using .lookAt(). It alters the value of phi and thus the result of .lookAt(). I mean it produces the expected result but users should aware about this fact when setting constrainVertical to true.

examples/webgl_nearestneighbour.html

examples/js/controls/FirstPersonControls.js

mrdoob · 2019-01-28T13:09:48Z

Thanks!

Mugen87 · 2019-01-28T13:22:12Z

I think we should update the stackoverflow posts, right?

WestLangley reviewed Jan 27, 2019

View reviewed changes

Mugen87 changed the title ~~FirstPersonControls: Added .setOrientation()~~ FirstPersonControls: Added .lookAt() Jan 28, 2019

Mugen87 force-pushed the dev26 branch from 4febda5 to ccc554c Compare January 28, 2019 11:01

WestLangley reviewed Jan 28, 2019

View reviewed changes

examples/webgl_nearestneighbour.html Outdated Show resolved Hide resolved

WestLangley reviewed Jan 28, 2019

View reviewed changes

examples/js/controls/FirstPersonControls.js Outdated Show resolved Hide resolved

Mugen87 force-pushed the dev26 branch from c828ccd to ba1b7b3 Compare January 28, 2019 12:28

FirstPersonControls: Added .lookAt()

6c337ef

Mugen87 force-pushed the dev26 branch from ba1b7b3 to 6c337ef Compare January 28, 2019 12:39

mrdoob added this to the r101 milestone Jan 28, 2019

mrdoob merged commit 2451173 into mrdoob:dev Jan 28, 2019

This was referenced Jan 28, 2019

How To make FirstPersonControls start by looking at an object. #3010

Closed

FirstPersonControls default lookAt issue #6551

Closed

Uh oh!

FirstPersonControls: Added .lookAt() #15650

FirstPersonControls: Added .lookAt() #15650

Uh oh!

Conversation

Mugen87 commented Jan 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

looeee commented Jan 27, 2019

Uh oh!

Mugen87 commented Jan 27, 2019

Uh oh!

WestLangley Jan 27, 2019

Choose a reason for hiding this comment

Uh oh!

Mugen87 Jan 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WestLangley Jan 27, 2019

Choose a reason for hiding this comment

Uh oh!

makc Jan 27, 2019

Choose a reason for hiding this comment

Uh oh!

looeee Jan 28, 2019

Choose a reason for hiding this comment

Uh oh!

Mugen87 Jan 28, 2019

Choose a reason for hiding this comment

Uh oh!

makc Jan 28, 2019

Choose a reason for hiding this comment

Uh oh!

Mugen87 Jan 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mugen87 Jan 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

looeee Jan 28, 2019

Choose a reason for hiding this comment

Uh oh!

WestLangley commented Jan 27, 2019

Uh oh!

Mugen87 commented Jan 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WestLangley commented Jan 27, 2019

Uh oh!

makc commented Jan 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WestLangley commented Jan 27, 2019

Uh oh!

looeee commented Jan 28, 2019

Uh oh!

Mugen87 commented Jan 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mugen87 commented Jan 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mugen87 commented Jan 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mrdoob commented Jan 28, 2019

Uh oh!

Mugen87 commented Jan 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Mugen87 commented Jan 27, 2019 •

edited

Loading

Mugen87 Jan 27, 2019 •

edited

Loading

Mugen87 Jan 28, 2019 •

edited

Loading

Mugen87 Jan 28, 2019 •

edited

Loading

Mugen87 commented Jan 27, 2019 •

edited

Loading

makc commented Jan 27, 2019 •

edited

Loading

Mugen87 commented Jan 28, 2019 •

edited

Loading

Mugen87 commented Jan 28, 2019 •

edited

Loading

Mugen87 commented Jan 28, 2019 •

edited

Loading