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

Support GearVR 3DOF controller #2545

Merged
merged 4 commits into from
Apr 14, 2017
Merged

Conversation

machenmusik
Copy link
Contributor

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.

@machenmusik
Copy link
Contributor Author

rebased, but will need changes when #2513 is merged

@machenmusik
Copy link
Contributor Author

rebased, onto latest master and also #2567

@dmarcos
Copy link
Member

dmarcos commented Apr 11, 2017

This contains #2567
Can we remove it from this one?

@machenmusik
Copy link
Contributor Author

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?

@machenmusik
Copy link
Contributor Author

removed portions from closed #2567 -- @mkeblx does codepen still work?

@machenmusik
Copy link
Contributor Author

fixed conflict with @ngokevin hand-controls changes

@@ -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;
Copy link
Member

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
}

Copy link
Contributor Author

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: {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

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;
Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assuming that #2513 survives with that flag, that is one of the changes to bring in line with #2513

this.checkIfControllerPresent();
},

onGamepadDisconnected: function (evt) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this method needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assuming that #2513 survives with that flag, that is one of the changes to bring in line with #2513

@machenmusik
Copy link
Contributor Author

got confirmation that this PR still works after removing portions of closed #2567.
if #2513 is merged before this one, I will update to conform.
otherwise, should be good to go

@mkeblx
Copy link
Contributor

mkeblx commented Apr 13, 2017

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.

@machenmusik
Copy link
Contributor Author

@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
Copy link
Contributor

mkeblx commented Apr 14, 2017

Yep looks good!

com oculus vrshell-20170413-202106

(and the touched: undefined is only after exit out of Presenting; only accessible when presenting)

@machenmusik
Copy link
Contributor Author

@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

@mkeblx
Copy link
Contributor

mkeblx commented Apr 14, 2017

Yes that works.

@dmarcos
Copy link
Member

dmarcos commented Apr 14, 2017

Thanks everybody! Controller collection complete. Cross platform achievement unlocked!

@dmarcos dmarcos merged commit e73e3aa into aframevr:master Apr 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants