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

Check for already at desiredPosition returns false positives sometimes #28

Open
UXVirtual opened this issue Sep 21, 2017 · 2 comments
Open

Comments

@UXVirtual
Copy link
Collaborator

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.

@tizzle
Copy link
Owner

tizzle commented Nov 20, 2017

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
Copy link
Owner

tizzle commented Nov 20, 2017

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

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