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.

Sign up for GitHub

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

Jump to bottom

accommodate both names for Vive controllers: old 'OpenVR Gamepad' and… #2370

Merged
merged 2 commits into from
Feb 9, 2017
Merged

accommodate both names for Vive controllers: old 'OpenVR Gamepad' and… #2370

merged 2 commits into from
Feb 9, 2017

Conversation

machenmusik
Copy link
Contributor

… new 'OpenVR Controller'

Note that due to recent browser-specific changes, Vive controllers may be returned
by the Gamepad API with id values of either "OpenVR Gamepad" or "OpenVR Controller",
so using idPrefix for Vive / OpenVR controllers is recommended.

@machenmusik
Copy link
Contributor Author

machenmusik commented Feb 9, 2017 •
edited
Loading

separating from #2191 so they can be discussed and merged independently

This is now necessary to accommodate both old and new Vive controller change in gamepad id string.

@machenmusik machenmusik mentioned this pull request Feb 9, 2017
Merged
ngokevin
ngokevin reviewed Feb 9, 2017
View reviewed changes
src/components/tracked-controls.js Outdated
@@ -14,6 +14,7 @@ module.exports.Component = registerComponent('tracked-controls', {
schema: {
controller: {default: 0},
id: {default: 'Match none by default!'},
idPrefix: {default: undefined},
Copy link
Member

Choose a reason for hiding this comment

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

Should be a string.

ngokevin
ngokevin reviewed Feb 9, 2017
View reviewed changes
docs/components/tracked-controls.md Outdated
| Property | Description | Default Value |
|-------------|-------------------------------------------------------------- --|------------------------|
| controller | Index of the controller in array returned by the Gamepad API. | 0 |
| id | Selects the controller from the Gamepad API using exact match. | Match none by default! |
Copy link
Member

Choose a reason for hiding this comment

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

Can just put '' for both default values

@ngokevin
Copy link
Member

ngokevin commented Feb 9, 2017

This looks good to me.

@ngokevin ngokevin merged commit c28ceeb into aframevr:master Feb 9, 2017
@ngokevin ngokevin mentioned this pull request Feb 9, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ngokevin ngokevin ngokevin left review comments

Assignees
No one assigned
Labels
needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@machenmusik @ngokevin