Add vibrationActuator to the main spec

[gabrielsanbrito]

Closes #173

The following tasks have been completed:

• Modified Web platform tests (link to pull request)

Implementation commitment:

• WebKit (https://bugs.webkit.org/show_bug.cgi?id=267523) - implemented and shipping
• Chromium (https://bugs.chromium.org/p/chromium/issues/detail?id=) - implemented and shipping.
• Gecko (https://bugzilla.mozilla.org/show_bug.cgi?id=1824217)

---

Preview | Diff

[gabrielsanbrito]

@nondebug, @marcoscaceres Just a few questions I had while editing:

• Should we remove the GamepadHapticActuatorType from the spec, as it has been marked as deprecated? If that is true, should we remove the construction step for the GamepadHapticActuator interface?

  • If we are keeping it, i have made minor changes to the definitions of "vibration" and "dual-rumble" actuator types to fix a CI error where Gamepad.hapticActuators was being mentioned in the main spec without being previously defined.

• I have deleted the "This specification defines an external now hidden algorithm [...]" sentence on the GamepadHapticActuator "Page visibility" subsection. Should we create a pull request to add the visibility changed steps to the HTML spec(https://html.spec.whatwg.org/multipage/interaction.html#page-visibility), like the Screen Orientation API has done?

[nondebug]

Should we remove the GamepadHapticActuatorType from the spec

Yes, let's remove it. It was never really part of the spec so it's not even officially deprecated.

Should we create a pull request to add the visibility changed steps to the HTML spec

Yes, let's do that.

[marcoscaceres]

Um, any idea why it's showing both files as being entirely changed? I think you might have changed the line endings or something? (i.e., this makes it not possible to review the changes)

Can you try sending it again or reach out to us if you need a hand? Are you doing this on windows machine maybe?

[marcoscaceres]

Yeah, @gabrielsanbrito, it seems you've accidentally changed the whitespace of the file:
https://github.com/w3c/gamepad/pull/190/files?diff=unified&w=1

Can you see about changing it back?

Maybe try:

git rebase --whitespace=fix gh-pages

[marcoscaceres]

A couple of nits and suggestions, but generally looking pretty solid now.

Outstanding work @gabrielsanbrito. Thank you so much for putting so much work into this.

@nondebug, do you want to give this one last look over?

[marcoscaceres]

File Gecko and WebKit bugs... WebKit already implements vibratorActuator, and so does Chrome. But it would be good to make sure we are all interoperable.

In any case, it meets criteria for inclusion in the spec.

[marcoscaceres]

@gabrielsanbrito, I merged in my suggestions and rebased.

The last remaining thing is just adding the DOMHighResTimestamp suggestion. Do you want me to do that?

[gabrielsanbrito]

@gabrielsanbrito, I merged in my suggestions and rebased.

The last remaining thing is just adding the DOMHighResTimestamp suggestion. Do you want me to do that?

Thanks for doing that, @marcoscaceres! I was looking up on how to do the DOMHighResTimestamp. I was thinking on something like this:

<li>Let {{GamepadHapticActuator/[[playingEffectPromise]]}} be [=a
new promise=].
<li>Let |playEffectTimestamp:DOMHighResTimestamp| be the [=current high resolution time=] given the |document|'s [=relevant global object=].
</li>
<li>Do the following steps [=in parallel=]:
  <ol>
    <li>[=Issue a haptic effect=] to the actuator with |type|, |params|, and the 
     |playEffectTimestamp|.
    </li>
[...]

Then, on the issue a haptic effect, I was thinking of adding:

To <dfn>issue a haptic effect</dfn> on an actuator, the [=user agent=] MUST send a command to the device to render an effect of |type:GamepadHapticEffectType| and try to make it use the provided |params:GamepadEffectParameters|. The [=user agent=] SHOULD use the provided |playEffectTimestamp:DOMHighResTimestamp| for more precise playback timing when |params|.{{startDelay}} is not `0.0`.

What do you think? Please feel free to add it directly if it is too far from ideal.

[gabrielsanbrito]

File Gecko and WebKit bugs... WebKit already implements vibratorActuator, and so does Chrome. But it would be good to make sure we are all interoperable.

In any case, it meets criteria for inclusion in the spec.

It looks like both WebKit (https://bugs.webkit.org/show_bug.cgi?id=267523) and Gecko (https://bugzilla.mozilla.org/show_bug.cgi?id=1824217) already have bugs filed for the vibrationActuator addition. I have just added a small update comment on the Gecko one to let them know that vibrationActuator is being added to the Gamepad API spec.

[marcoscaceres]

@gabrielsanbrito, the proposed text for the timestamp sounds great!

[marcoscaceres]

Ok, cool. I've updated the OP to link to the existing Gecko bug. Thanks also for commenting there @gabrielsanbrito.

[marcoscaceres]

I wonder if we should do a little "test fest" on our next call. Would be nice to just run through the algorithms manually and check for any introp issues.

[toji]

Change LGTM, thanks for the update! And, to reiterate, I agree with the decision to leave pulse() in the extensions doc, given the confusion around it and it's intended behavior.

I appreciate the explicit "completed"/"preempted" status of playEffect() in this regard, and would recommend that if pulse() is carried forward that it may be worth evaluating whether or not to transition to that model for consistency and clarity. (Since the enums will evaluate to be truthy and existing implementations have apparently only ever returned true this is maybe a reasonable backwards compat break.)

[gabrielsanbrito]

@marcoscaceres had to remove the "switch" class from the dl tag. It was messing the numbers of the ordered list. With the class set, I was having:
Item 1: Given the value of {{GamepadHapticEffectType}} |type:GamepadHapticEffectType|, switch on:
 "dual-rumble": If params does not describe a valid dual-rumble effect, return false.
Item 3: Return true.

Just removing the class made "Return true." become item 2 again - so I have removed it. I don't know if I am missing on something here or if there is something wrong with the "switch" class. Please let me know if this looks good for you. Now it is looking like this:
Item 1: Given the value of {{GamepadHapticEffectType}} |type:GamepadHapticEffectType|, switch on:
 "dual-rumble": If params does not describe a valid dual-rumble effect, return false.
Item 2: Return true.

[marcoscaceres]

Yeah, that's fine. And don't worry, we will eventually get rid of all the <dl>s in algorithms... they are annoying.

[marcoscaceres]

@gabrielsanbrito, just waiting on Matt. Spoke to him and he's hoping to do the final review soon. Expecting this to land hopefully early next week.

[nondebug]

lgtm

[gabrielsanbrito]

Thank you all for the valuable feedback!

@marcoscaceres may I merge the pull request?

[marcoscaceres]

Merged 🎉