aframe-orbit-controls-component icon indicating copy to clipboard operation
aframe-orbit-controls-component copied to clipboard

Check for already at desiredPosition returns false positives sometimes

Open UXVirtual opened this issue 6 years ago • 2 comments

The below snippet in the update() method causes camera rotations near the desiredPosition to incorrectly match, preventing rotateTo() from firing:

if (!this.desiredPosition.equals(rotateToVec3)) {
        this.desiredPosition.copy(rotateToVec3);
        this.rotateTo(this.desiredPosition);
}

removing the !this.desiredPosition.equals(rotateToVec3) check results in more consistent behavior, with the exception that the camera will automatically rotate to face the very top of the model on load if rotateTo is not set.

UXVirtual avatar Sep 21 '17 04:09 UXVirtual

I guess we could remove the default value of the rotateTo and set it to undefined. This way the if-clause ⬆️ only fires when a value is present, which should be the intended behaviour. I tried this out and all the examples still work as expected. What do you think?

Edit: I dove into the rotateTo functionality a little and feel there are multiple things not ideal: First: the check in the update function shouldn't compare the rotateToVec3 and desiredPosition vector. I feel a comparison between desiredPosition and dolly.position (the actual position of the camera dolly) is better suited. Second: The example is causing some troubles in this. Whenever one of the three buttons is clicked, the component updates it's data for the rotateTo props. If this data is not different to what is in place, the component does not enter the update lifecycle. This means when i clicked the second button, wait for the rotateTo to finish, then move the camera by hand and click the second button again, it won't fire, because the manual camera movement did not invalidate the rotateTo data and the component does not enter the update cycle, as no data has changed.

May this behaviour be what you where referring to with this issue?

tizzle avatar Nov 20 '17 13:11 tizzle

After talking to dmarcos from the aframe team i made a branch using an exposed function to change the rotation, as this gets rid of some of the side-effects of the setAttribute approach. Feel free to check: https://github.com/tizzle/aframe-orbit-controls-component/tree/setRotateTo-function

tizzle avatar Nov 20 '17 18:11 tizzle