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

set position bugged after 1.37.5 #345

Closed
VegarRingdalAibel opened this issue Jan 26, 2023 · 4 comments
Closed

set position bugged after 1.37.5 #345

VegarRingdalAibel opened this issue Jan 26, 2023 · 4 comments

Comments

@VegarRingdalAibel
Copy link

VegarRingdalAibel commented Jan 26, 2023

Describe the bug

Hi

Tried to create sample showing setPosition is broken after 1.37.5

To Reproduce

See animated gifs

Live example

https://codesandbox.io/s/musing-mccarthy-zqneho?file=/src/index.js

You can switch between version here
image

Expected behavior

To use correct position and target, not using the offset its doing now after 1.37.5

Screenshots or Video

Bugged behavior

Bugged: I save 4 target/positions, but when I move camera, it stays in same position and not the one I saved

chrome_aciXKCEYKi

Expected behavior

chrome_vHM5CGk2jY

Device

Desktop

OS

Windows

Browser

Chrome

@yomotsu
Copy link
Owner

yomotsu commented Jan 27, 2023

Thanks for the code. It was helpful.

Actually both setTarget() and setPosition() are alias of setLookAt() and you are calling the same function at the same time eventually.
see:

/**
* setLookAt without target, keep gazing at the current target
* @param positionX
* @param positionY
* @param positionZ
* @param enableTransition
* @category Methods
*/
setPosition( positionX: number, positionY: number, positionZ: number, enableTransition: boolean = false ): Promise<void> {
return this.setLookAt(
positionX, positionY, positionZ,
this._targetEnd.x, this._targetEnd.y, this._targetEnd.z,
enableTransition,
);
}
/**
* setLookAt without position, Stay still at the position.
* @param targetX
* @param targetY
* @param targetZ
* @param enableTransition
* @category Methods
*/
setTarget( targetX: number, targetY: number, targetZ: number, enableTransition: boolean = false ): Promise<void> {
const pos = this.getPosition( _v3A );
const promise = this.setLookAt(
pos.x, pos.y, pos.z,
targetX, targetY, targetZ,
enableTransition,
);
// see https://github.com/yomotsu/camera-controls/issues/335
this._sphericalEnd.phi = clamp( this.polarAngle, this.minPolarAngle, this.maxPolarAngle );
return promise;
}

Thus, you are overwriting the first one with the second one. Therefore, setPosition() is not working in your code.
How about using setLookAt() instead?

@VegarRingdalAibel
Copy link
Author

setPosition worked perfectly combined with setTarget in 1.37.5 so I though it was a bug. 😄
Ill give the setLookAt a try, from the code sample above it looks to solve my issue

@yomotsu
Copy link
Owner

yomotsu commented Jan 27, 2023

I though it was a bug

TBH...I thought so...
But according to the code, the current version is more expected behavior.

@VegarRingdalAibel
Copy link
Author

thx

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

No branches or pull requests

2 participants