-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Support GearVR 3DOF controller #2545
Conversation
6916cc3
to
2e857c4
Compare
2e857c4
to
061854b
Compare
rebased, but will need changes when #2513 is merged |
7917bd0
to
8da9d87
Compare
rebased, onto latest master and also #2567 |
This contains #2567 |
i am not sure whether it will work without it. @mkeblx does it work with only the commits after "add gearvr controls", and if so are you able to say that for only Samsung Internet, or also Oculus Browser? |
60a9783
to
b0361ec
Compare
fixed conflict with @ngokevin hand-controls changes |
1b9015c
to
eb39e11
Compare
src/utils/tracked-controls.js
Outdated
@@ -48,7 +49,7 @@ module.exports.isControllerPresent = function (sceneEl, idPrefix, queryObject) { | |||
isPrefixMatch = (!idPrefix || idPrefix === '' || gamepad.id.indexOf(idPrefix) === 0); | |||
isPresent = isPrefixMatch; | |||
if (isPresent && queryObject.hand) { | |||
isPresent = gamepad.hand === queryObject.hand; | |||
isPresent = (gamepad.hand || DEFAULT_HANDEDNESS) === queryObject.hand; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this instead?
if (isPresent && queryObject.hand && gamepad.hand) {
isPresent = gamepad.hand === queryObject.hand
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if queryObject.hand === 'left' and no gamepad.hand, DEFAULT_HANDEDNESS should be used so isPresent becomes false, right? with yours, isPresent stays true
// buttonId | ||
// 0 - trackpad | ||
// 1 - triggeri | ||
mapping: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not this follow the convention established by #2513? https://github.com/aframevr/aframe/pull/2513/files#diff-ae2d3da90e49853ba9d7c7ea05862707R35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but you haven't merged it yet! from 5 days ago:
rebased, but will need changes when #2513 is merged
onGamepadConnected: function (evt) { | ||
// for now, don't disable controller update listening, due to | ||
// apparent issue with FF Nightly only sending one event and seeing one controller; | ||
// this.everGotGamepadEvent = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want to set this flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.checkIfControllerPresent(); | ||
}, | ||
|
||
onGamepadDisconnected: function (evt) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this method needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eb39e11
to
62b03eb
Compare
Added controller model aframevr/assets#20 (Thanks @840labs) @machenmusik Something like below should work if you want to modify in this PR. buttonMeshes.trigger = controllerObject3D.getObjectByName('Trigger');
buttonMeshes.trackpad = controllerObject3D.getObjectByName('Touchpad');
// Offset pivot point # can be removed
// controllerObject3D.position.set(0, 0, 0); Note: Pivot should be not needed, set to center of trackpad like pivot on Daydream controller location. |
@mkeblx updated to use your model (I hope). Does http://codepen.io/machenmusik/pen/QvLjbx still work and do you see the new model? |
@mkeblx niiice! one more to try -- we're trying to land #2513, this codepen brings this PR into line with that one, does it work as well? http://codepen.io/machenmusik/pen/Gmgmgw |
Yes that works. |
Thanks everybody! Controller collection complete. Cross platform achievement unlocked! |
This PR, which includes #2538 in its entirety (and is intended to be merged afterward), should add support for the new 3DOF Gear VR Controller as proposed at https://webvr.slack.com/?redir=%2Farchives%2FC0EEECMA5%2Fp1490839724160554
Note that at time of this writing, this PR has not yet been tested on a supporting browser with supported hardware. Once such testing has been completed, this comment will be edited appropriately.