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

Gracefully handles when calling getGamepads() is not allowed... #5581

Closed
wants to merge 7 commits into from
+35 −17
Closed

Gracefully handles when calling getGamepads() is not allowed... #5581

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f7338f3
Gracefully handles when Document's Permission Policy does not allow c…
DougReeder Sep 28, 2024
07650ee
Improves warning message
DougReeder Sep 29, 2024
7a6a13d
tracked-controllers-webvr: separate error messages for permission and…
DougReeder Sep 30, 2024
9eaeb0f
tracked-controls: specific warning messages when gamepad access is bl…
DougReeder Sep 30, 2024
ab0065f
tracked-controls: updates wording of gamepad error messages
DougReeder Oct 1, 2024
6a6bb02
tracked-controls: only shows permissions warning once
DougReeder Oct 13, 2024
a3f8df8
tracked-controllers-webvr: when not in vr-mode, doesn't call getGamep…
DougReeder Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions src/systems/tracked-controls-webvr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,38 @@ module.exports.System = registerSystem('tracked-controls-webvr', {
* Update controller list.
*/
updateControllerList: function () {
var controllers = this.controllers;
var gamepad;
var gamepads;
var i;
var prevCount;
try {
var controllers = this.controllers;
var gamepad;
var gamepads;
var i;
var prevCount;

gamepads = navigator.getGamepads && navigator.getGamepads();
if (!gamepads) { return; }
gamepads = navigator.getGamepads && navigator.getGamepads();
if (!gamepads) { return; }

prevCount = controllers.length;
controllers.length = 0;
for (i = 0; i < gamepads.length; ++i) {
gamepad = gamepads[i];
if (gamepad && gamepad.pose) {
controllers.push(gamepad);
prevCount = controllers.length;
controllers.length = 0;
for (i = 0; i < gamepads.length; ++i) {
gamepad = gamepads[i];
if (gamepad && gamepad.pose) {
controllers.push(gamepad);
}
}
}

if (controllers.length !== prevCount) {
this.el.emit('controllersupdated', undefined, false);
if (controllers.length !== prevCount) {
this.el.emit('controllersupdated', undefined, false);
}
} catch (e) {
if (e.name === 'SecurityError') {
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 specific enough? We shouldn't check if we are in an iframe?

https://stackoverflow.com/questions/326069/how-to-identify-if-a-webpage-is-being-loaded-inside-an-iframe-or-directly-into-t

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No other code inside the try-catch can throw a SecurityError.

if (window.self === window.top) {
console.warn('The HTTP `Permissions-Policy` header must not block this origin in the `gamepad` directive, to allow A-Frame to list the gamepads.', e);
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this. Seems pretty rare. Can add later if necessary

} else {
console.warn('The iframe `allow` attribute must not block the origin of this A-Frame app, to allow A-Frame to list the gamepads.', e);
Copy link
Member

Choose a reason for hiding this comment

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

This message is still not clear. I wouldn't know what to do and I'm well versed in web dev. What about?

It looks like your A-Frame experience is running inside an iframe. In order to access the gamepads API you need to add allow="gamepad" to your iframe attributes

}
} else {
console.error('Can\'t update controller list:', e);
Copy link
Member

Choose a reason for hiding this comment

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

why is this error and above just warnings?

Copy link
Contributor Author

@DougReeder DougReeder Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If the A-frame app doesn't need the non-webxr gamepads on this device, failing to get the gamepads will not keep the app from working. If the a-frame app does need gamepads on this device, not getting the gamepads is a problem that has to be fixed. Since this code can't determine which, a warning is reasonable. (However, if the error isn't caught at all, A-Frame doesn't fully initialize.)

"Can't update controller list" would only occur if there was some unforeseen error and would likely be a programming error in A-Frame.

}
}
}
});
Loading