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

Specify live-ness of XRInputSources #608

Closed
toji opened this issue Apr 24, 2019 · 4 comments · Fixed by #624
Closed

Specify live-ness of XRInputSources #608

toji opened this issue Apr 24, 2019 · 4 comments · Fixed by #624
Labels
fixed by pending PR A PR that is in review will resolve this issue. potential breaking change Issues that may affect the core design structure.
Milestone

Comments

@toji
Copy link
Member

toji commented Apr 24, 2019

Just got done consulting with @bfgeek on the subject of whether or not XRInputSources and their child attributes (such as gamepad) should be "live" on not. That is, should an XRInputSource that you get from the getInputSources() call on one frame be updates with new values on the next frame (ie: live), or should you have call getInputSources() again which will return a new object representing the same input source but with new data. (ie: a snapshot). This is an issue that came up during Chrome implementation work and one that ended up biting the Gamepad API pretty hard a few years back, so it's in our interest to specify it now.

The general sentiment is that it's a bit weird to have live objects returned from a getter function, which is the way we currently have it. On the web platform getters typically should return snapshots, as that helps alleviate lifetime issues for the objects. If we want to represent the objects as live ones that update in-place, it's recommended that we have the accessor be an attribute rather than a method, since that helps imply that you'll be getting back the same objects with each access.

Given this, I think we need to clarify the behavior of XRInputSources in one of three ways:

  1. Change the getInputSources() method to a inputSources attribute and specify that the objects in the array are updated in place.
  2. Leave getInputSources() as a method and specify that it returns a snapshot of the input state which does not subsequently update.
  3. Leave getInputSources() as a method but specify that it returns a live array with big flashing red text in the spec so that nobody misses it, and also go to great pains to describe exactly how those returned values lifetime corresponds to the lifetime of the session object.

I have a preference for 1, but would be fine with 2 if the rest of the working group wanted it. I will accept 3 only if you have sufficient bribe money available for immediate deposit.

@Manishearth
Copy link
Contributor

Related: #593

I like having the objects be live (preference for option 1). However I'll note that in its current state we can't have the XRInputSource be live and also have a gamepad attribute, because Gamepads are not really live (AFAICT, see #593 and #593).

If we go with option 1 we should probably turn gamepad into a getGamepad() that does similar state fetching as navigator.getGamepads()

@toji
Copy link
Member Author

toji commented Apr 25, 2019

Fun fact: The Gamepad API spec doesn't really comment on liveness either way but some parts seem to imply that the gamepads returned are snapshots. HOWEVER no browser actually follows that and all of them expose the gamepads as live objects anyway. The one browser that did try to make them snapshots, Edge, eventually had to move to live objects anyway because of compatibility issues with the other browsers.

@Manishearth
Copy link
Contributor

Ah, thanks. I was told otherwise and a quick scan of the spec seemed to support it.

If Gamepads are live then I'm all for option 1

@cwilso cwilso added this to the June 2019 Working Draft milestone Apr 29, 2019
@toji toji added the potential breaking change Issues that may affect the core design structure. label Apr 29, 2019
@probot-label
Copy link

probot-label bot commented May 8, 2019

This issue is fixed by PR #624

@probot-label probot-label bot added the fixed by pending PR A PR that is in review will resolve this issue. label May 8, 2019
@toji toji closed this as completed in #624 May 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed by pending PR A PR that is in review will resolve this issue. potential breaking change Issues that may affect the core design structure.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants