Update all packages

[vincentfretin]

This closes #5

[vincentfretin]

I tested VR on Quest and AR on Chrome Android on this glitch https://glitch.com/edit/#!/vfretin-aframe-xr-starterkit no regression as far as I can tell, only bugfixes. :)

[vincentfretin]

Previously in VR with movement-controls right joystick didn't work to move the camera, now it works since https://github.com/n5ro/aframe-extras/pull/373 was merged but if I move position with left joystick afterwards it doesn't go in the correct direction. Funny I don't have this issue at all in my app and the code is the same as far as I can tell except my camera has id="camera" and yours id="head". There is another thing that bother me, it moves forward now when I press trigger when movement-controls is enabled, I'm not sure what changes introduced that.

[vincentfretin]

I removed your hack that reset the cameraRig rotation to y=0 when switching to movement-controls. My guess is that you added this hack because of the issue:

• https://github.com/n5ro/aframe-extras/issues/394

that I now fixed in the PR:

• https://github.com/n5ro/aframe-extras/pull/396

I used the build of this PR here.

[vincentfretin]

If I comment the main.js file, I don't have the issue anymore where the left joystick goes into the wrong direction, so there is something in main.js that does something on cameraRig or camera rotation.

[vincentfretin]

It's your xr-follow component that causes the issue, but I don't understand why. This component is set on the parent of the sword and watergun. I don't understand why it impacts cameraRig/camera heading when moving.

[AdaRoseCannon]

Oh that’s weird it shouldn’t change anything else. I’ll take a quick look.

[vincentfretin]

I found it. It's the following code:

const cameraObject = scene.camera;
const camera = scene.is('vr-mode') ? scene.renderer.xr.getCamera(cameraObject) : cameraObject;

scene.renderer.xr.getCamera is overriding the active camera scene.camera I think.

If I replace those line by just

const camera = scene.camera;

it seems to work properly. I see the sword and watergun, I can grab both.

[AdaRoseCannon]

That was to get it working in VR glad it is no longer needed but weird that it would have side effects

[vincentfretin]

Yes it's a bit weird, I don't fully understand it, but if the code is simpler, still work in VR and this fixes the movement issue, then I won't dig it further. :)

[vincentfretin]

Ok so there is only one issue remaining before we can merge. This is this issue:

• https://github.com/n5ro/aframe-extras/issues/392

[vincentfretin]

Yeah I really don't understand how the issues are related :D How it can impact the heading calculation here https://github.com/n5ro/aframe-extras/blob/5b1339d86f59e871931a7e94f51288f59364aa6a/src/controls/movement-controls.js#L191-L194

I just wanted to mention getCamera has no argument in threejs r144 https://github.com/mrdoob/three.js/blob/68daccedef9c9c325cc5f4c929fcaf05229aa1b3/src/renderers/webxr/WebXRManager.js#L560-L564 so the previous code was slightly wrong, in this threejs version anyway.

[vincentfretin]

FYI in threejs r144 we have this change https://github.com/mrdoob/three.js/commit/777f97a0425c806f3ef7fb670a74e91373e23068 that's probably why we don't need the previous fix now.

[vincentfretin]

Alright, I fixed the issue https://github.com/n5ro/aframe-extras/issues/392 with trigger moving forward, that was a regression when I merged the changes in touch-controls for Chrome Android Cardboard button. I currently use an aframe-extras build with https://github.com/n5ro/aframe-extras/pull/396 changes. Let's wait a few days if someone has any feedback on those changes. Once this is merged, you will be able to merge this PR too.

You can test the changes in my glitch https://glitch.com/edit/#!/vfretin-aframe-xr-starterkit

[AdaRoseCannon]

FYI in threejs r144 we have this change mrdoob/three.js@777f97a that's probably why we don't need the previous fix now.

Nice find!

[AdaRoseCannon]

Thank you for all the fixes, glad to see so many hacks removed

[vincentfretin]

I merged the aframe-extras PR, now using again a build from aframe-extras master. I updated to use an aframe master build with three r147 (r144 previously), there are no new warning or error. I tested in VR, grabbing the sword, climbed the ladder, and tested again AR mode on Chrome Android. You can merge now or wait for aframe 1.4.0 in a week or two as you wish.

[AdaRoseCannon]

Fantastic! Let’s merge:D

Thank you so much for all of your work in this!

[vincentfretin]

Cool, thanks. Please don't forget to update the glitch as well. I'm not sure what it your workflow here, you just open the glitch terminal and git pull and refresh I guess?

[AdaRoseCannon]

Updated