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

No way to properly handle Looking Glass device existance or errors #22

Open
hybridherbst opened this issue Nov 14, 2022 · 7 comments
Open
Assignees
Labels
enhancement New feature or request

Comments

@hybridherbst
Copy link

Seems that here:

const client = new HoloPlayCore.Client(
(msg) => {
if (msg.devices.length < 1) {
console.error('No Looking Glass devices found!');
return;
}
if (msg.devices.length > 1) {
console.warn('More than one Looking Glass device found... using the first one');
}
this.calibration = msg.devices[0].calibration;
},
(err) => {
console.error('Error creating Looking Glass client:', err);
});

would be the right way to have callbacks for errors, ability to check if the device is actually there, if it responds (bridge is running), etc., all needed for proper error handling and showing the right state (I don't want to show a "View on Looking Glass" button if people don't have one).

Additionally, it seems that even with no device connected and the bridge not installed, config.calibration has always exactly one entry (some default one?), so that is also no use to check for actual devices.

Would be better if there was a way to

  • check if LG is available before actually patching the WebXR APIs from the browser
  • respond to messages such as "WebSocket connection to 'ws://localhost:11222/driver' failed:" and "Error creating Looking Glass client:" in a proper way
@alxdncn alxdncn added the enhancement New feature or request label Nov 14, 2022
@hybridherbst
Copy link
Author

Want to note that while this is labeled as enhancement it's pretty much a blocker for us for a tighter LG integration - due to not being able to detect / react to it we would have to always lead people to a specific LG-only page right now.

@alxdncn
Copy link

alxdncn commented Nov 16, 2022

Hi @hybridherbst that's fair - Bryan and I discussed the importance of getting this fixed so we're on the same page about how high-priority it is for you! I do think it's an enhancement rather than a bug fix, just a critical enhancement to unlock key user flows.

@BryanChrisBrown is on top of this and I'm sure will have updates for you in the near future - it's high priority for us!

@hybridherbst
Copy link
Author

Agree it's an enhancement/missing feature not a bug :)

@BryanChrisBrown
Copy link
Collaborator

I'm still working on getting this setup properly, it's a bit tricky since the three.js XR button (and likely other XR Buttons) reads the XR state as soon as the page loads, so wrapping the XR state in a promise is a bit tricky as three will default to XR being disabled, as the device isn't detected in time.

There is a work around that Cody Bennett used where you can wrap the polyfill in an if statement detecting the browser, which would allow it to work on some stand alone headsets if a temporary solution is of interest. here's their codesandbox, you'll want to look at line 150

I'll keep digging on getting out a proper solution for this.

@BryanChrisBrown
Copy link
Collaborator

Initial attempt at getting this working is here if you're interested in the progress.

https://github.com/Looking-Glass/looking-glass-webxr/tree/feature/device-detection

the challenge currently is making sure that three knows a new device is present, it seems that just loading the polyfill after the device is detected doesn't update the XRButton's state.

@BryanChrisBrown
Copy link
Collaborator

@hybridherbst given that your example is using a custom XR button could you try out the feature/device-detection branch when you get a chance? right now the default three.js XR button fails,

you'd need to replace the following line new LookingGlassWebXRPolyfill() with LookingGlassWebXRPolyfill.init({}) to get the new functionality

@BryanChrisBrown BryanChrisBrown self-assigned this Nov 18, 2022
@BryanChrisBrown
Copy link
Collaborator

so the new functionality in the feature/device-detection branch does work, provided that the enter VR button is delayed by ~400ms or more, since it needs to wait until after the Looking Glass is detected or not, in order to run. Wrapping it in a timeout function like below does work.

setTimeout(function () {
document.body.append(VRButton.createButton(renderer))
}, 400)

I'm not super happy with this solution at the moment as I'd like one that works without end user intervention, but it's also clear we're running up against an issue with a component that's outside our control here.

Perhaps adding our own "View on Looking Glass" button could work, but we'd have to think about how we do that in an engine agnostic way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants