Update aframe-teleport to work with WebXR #73
Conversation
|
Tested without problems on Firefox Windows with Oculus Link running as an Oculus Quest. |
riccardogiorato
left a comment
riccardogiorato
left a comment
There was a problem hiding this comment.
Switch urls from dbradleyfl to the originals fernandojsg
| el.addEventListener(data.button + 'down', this.onButtonDown); | ||
| el.addEventListener(data.button + 'up', this.onButtonUp); | ||
| } | ||
|
|
There was a problem hiding this comment.
The fact that I did away with these lines mean the "startEvents" and "endEvents" properties don't do anything anymore. They might be un-necessary now with the new WebXR api, but I wasn't sure. Thought it was worth noting, and is part of the reason I didn't originally create this pull request.
There was a problem hiding this comment.
I don't think is a good idea to remove these events and replace them just with axismove as you are forcing the users to use the axis for teleporting without any chance to change that configuration.
There was a problem hiding this comment.
I added this back, but switched the default behavior to axismove as I believe that is the most common desired behavior (due to quest).
riccardogiorato
requested review from
dmarcos and
feiss
and removed request for
feiss
| el.addEventListener(data.button + 'down', this.onButtonDown); | ||
| el.addEventListener(data.button + 'up', this.onButtonUp); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think is a good idea to remove these events and replace them just with axismove as you are forcing the users to use the axis for teleporting without any chance to change that configuration.
index.js
Outdated
| el.addEventListener(data.button + 'up', this.onButtonUp); | ||
| } | ||
|
|
||
| el.addEventListener('axismove', this.handleAxis.bind(this)) |
package.json
Outdated
| "unpkg": "dist/aframe-teleport-controls.min.js", | ||
| "scripts": { | ||
| "dev": "budo index.js:dist/aframe-teleport-controls.min.js --port 7000 --live --open", | ||
| "dev": "budo index.js:dist/aframe-teleport-controls.min.js --port 7000 --live --open --ssl --cors --cert cert.crt --key key.key", |
There was a problem hiding this comment.
it will probably make sense to switch to webpack with the --https parameter so it doesn't need a certificate, as currently this will break on people as they won't have the installed certificate locally
.gitignore
Outdated
| gh-pages | ||
| node_modules/ | ||
| npm-debug.log | ||
| cert.crt |
There was a problem hiding this comment.
nit: I don't think you should be using local cert and adding them to ignore, but instead using a server that could run the https by itself as webpack or so. Not sure if budo has that option though
There was a problem hiding this comment.
FYI, budo generates a self-signed certificate if no -cert / --key are passed
There was a problem hiding this comment.
that's great to know thanks!
There was a problem hiding this comment.
Yep. Just discovered the project was running an old version of budo that required a cert and key. Updated that dep.
dbradleyfl
force-pushed
the
master
branch
2 times, most recently
from
d64886a to
b54de8d
Compare
|
Just rebased so this is ready to merge if you're ok with it @fernandojsg. Happy to make more changes if necessary. |
|
@dbradleyfl Thanks for sticking with it. Great effort 🏅 |
|
@dbradleyfl Hi! sorry for the delay. I was checking an old branch I had trying to migrate to WebXR and I found some other issues and bugs that the original version already had, (eg computing the normal of the plane where you will land is incorrect right now). So I'm working on fixing them all and update the code, hopefully in the next few weeks. I'll be back here if there is something missing ^_^ |
4 participants
Update aframe-teleport to work with WebXR