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

Add observers to WebXRCamera when a rotation is performed #14660

Merged

Conversation

yuripourre
Copy link
Contributor

@yuripourre yuripourre commented Dec 31, 2023

Add observables onBeforeCameraTeleportRotation and onAfterCameraTeleportRotation to WebXRMotionControllerTeleportation.

npm run format:fix

@bjsplat
Copy link
Collaborator

bjsplat commented Dec 31, 2023

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Dec 31, 2023

@carolhmj carolhmj requested a review from RaananW January 2, 2024 11:57
Copy link
Member

@RaananW RaananW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the mistake of putting the first observable on the camera and not on the feature itself. There is no reason for it to be on the camera. Let's learn from (my) mistakes and move the observables to the feature class

@yuripourre
Copy link
Contributor Author

@RaananW Done!

@yuripourre yuripourre force-pushed the webxr-camera-rotation-observers branch from b672bc2 to de319bd Compare January 3, 2024 20:51
@yuripourre yuripourre force-pushed the webxr-camera-rotation-observers branch from de319bd to a574589 Compare January 3, 2024 20:54
@sebavan sebavan enabled auto-merge January 3, 2024 22:59
@sebavan sebavan merged commit a99db8f into BabylonJS:master Jan 3, 2024
10 checks passed
@yuripourre yuripourre deleted the webxr-camera-rotation-observers branch January 4, 2024 17:21
@RaananW
Copy link
Member

RaananW commented Jan 8, 2024

Thanks! this is great. I guess we can add a proxy from the teleportation to the camera's observable and mark it deprecated on the camera. But this is a very low priority.

@yuripourre
Copy link
Contributor Author

@RaananW that would be great. It crossed my mind to do that but I am focusing on other improvements currently, like adding a BVH file loader.

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