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

Replace Gamepad id from "Gear VR Controller" to Oculus Go/HTC HTC Vive Focus controllers #981

Merged

Conversation

daoshengmu
Copy link
Contributor

Per #812, we should use the same gamepad id, Oculus Go Controller, as other browsers when running in Oculus Go display.

@daoshengmu daoshengmu self-assigned this Mar 4, 2019
@ghost ghost added the in progress label Mar 4, 2019
@bluemarvin
Copy link
Contributor

Be aware this was done for compatibility reasons. So make sure changing it doesn't break some of existing immersive web sites.

@philip-lamb
Copy link
Contributor

We should also update the string exposed for the HTC Vive Focus controller. Per aframevr/aframe#3876 (https://github.com/aframevr/aframe/pull/3876/files#diff-0cf01d3ef4445863595ddf0ff798ecfbR11) it should be corrected to 'HTC Vive Focus Controller' at https://github.com/MozillaReality/FirefoxReality/blob/master/app/src/wavevr/cpp/DeviceDelegateWaveVR.cpp#L302 .

@bluemarvin
Copy link
Contributor

For reference: #441

@daoshengmu daoshengmu changed the title Replace Gamepad id from "Gear VR Controller" to "Oculus Go Controller". Replace Gamepad id from "Gear VR Controller" to Oculus Go/HTC HTC Vive Focus controllers Mar 4, 2019
@daoshengmu
Copy link
Contributor Author

daoshengmu commented Mar 4, 2019

I begin to worry about that will affect content that didn't use A-Frame or didn't follow its latest update. I guess we do need more opinions from @cvan .

@philip-lamb
Copy link
Contributor

philip-lamb commented Mar 5, 2019

I'll log an issue to consider a compatibility setting in the UI. If we continue incorrectly sending Gear VR as the identifier, we'll continue to diverge in webcompat from the Oculus browser. We can potentially sniff older AFrame versions before the Oculus Go controller and HTC Vive Focus controller strings were added.

@cvan
Copy link
Contributor

cvan commented Mar 5, 2019

If we continue incorrectly sending Gear VR as the identifier, we'll continue to diverge in webcompat from the Oculus browser.

I agree that we should report the same Gamepad#id as Oculus Browser does.

We can potentially sniff older AFrame versions before the Oculus Go controller and HTC Vive Focus controller strings were added.

Assuming we can't reach out to the site owners of the more popular A-Frame experiences (e.g., the ones in the Content Feed), ensuring backwards compatibility with older A-Frame versions I don't think is a reasonable expectation.

On a related note, see this comment I made about Gamepad#id in general here:

I've commented on w3c/gamepad#73 (comment). Will update as I do some research on the actual sites possibly impacted by Gamepad#id sniffing.

@cvan cvan added this to the v1.1.3 milestone Mar 5, 2019
@cvan
Copy link
Contributor

cvan commented Mar 5, 2019

So as to not break Soundboxing (and likely other WebVR experiences I'm not aware of), we should probably wait before merging this. I've emailed the Soundboxing staff asking them to upgrade to A-Frame version 0.9.0 and to use the oculus-go-controls component (currently they're using only the gearvr-controls component). I'll respond here when I hear a response. Thank you, @daoshengmu!

@MortimerGoro
Copy link
Contributor

MortimerGoro commented Mar 7, 2019

Let's wait to merge this until @cvan confirms that the content feed demos are ready

Copy link
Contributor

@cvan cvan left a comment

Choose a reason for hiding this comment

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

👍 nice work. this looks good to ship.

I haven't found any WebCompat issues in any of the following WebVR experiences I've tested. with your change, these all worked:

  • Delight-VR video player (and in fact, the controller mesh graphic is now correctly an Oculus Go controller instead of a Samsung Gear VR controller 👍)
  • WITHIN
  • PanoMoments (doesn't require a Gamepad)
  • Sketchfab (and in fact, the controller mesh graphic is now correctly an Oculus Go controller instead of a Samsung Gear VR controller 👍)
  • EFF's Spot the Surveillance (doesn't require a Gamepad)
  • Pristine
  • Jingle Smash
  • Vuppets
  • Hallovreen (doesn't require a Gamepad)
  • Soundboxing

@MortimerGoro
Copy link
Contributor

@cvan Have you also tested the controller change in HTC Vive focus?

@bluemarvin bluemarvin merged commit 9027410 into MozillaReality:master Mar 14, 2019
@ghost ghost removed the in progress label Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants