Re: [w3c/gamepad] Add extended buttons to support trackpad (#191) (PR #196)

@nondebug commented on this pull request.



> @@ -820,8 +820,13 @@ <h3>
           <li>[=list/For each=] |rawInputIndex:long| of [=the range=] from 0 to
           |inputCount| − 1:
             <ol>
-              <li>If the the gamepad button at index |rawInputIndex|
-              [=represents a Standard Gamepad button=]:
+              <li>Let |button| be the button at index |rawInputIndex|.
+              </li>
+              <li>Let |type| be the result of determining if the |button| [=represents
+              a Standard Gamepad button=] or represents [=Additional gamepad buttons=]

Let's use singular in both cases and add a period at the end of the sentence:

```
a Standard Gamepad button=] or represents an [=additional gamepad button=].
```

I'm not sure if respec will link the definition correctly if we don't use "Additional gamepad buttons"; if not, we can add a data-lt attribute to the <dfn> tag below:

```
<dfn data-lt="additional gamepad button">Additional gamepad buttons</dfn>
```

> @@ -878,8 +883,15 @@ <h3>
           <li>Initialize |buttons| to be an empty [=list=].
           </li>
           <li>[=list/For each=] |buttonIndex:long| of [=the range=] from 0 to
-          |buttonsSize| − 1, [=list/append=] a [=new=] {{GamepadButton}} to
-          |buttons|.
+          |buttonsSize| − 1:
+            <ol>
+              <li>Determine the |type| of the button from the provided {{GamepadButtonType}}
+              array, defaulting to null if not specified.

```
`null`
```

> @@ -878,8 +883,15 @@ <h3>
           <li>Initialize |buttons| to be an empty [=list=].
           </li>
           <li>[=list/For each=] |buttonIndex:long| of [=the range=] from 0 to
-          |buttonsSize| − 1, [=list/append=] a [=new=] {{GamepadButton}} to
-          |buttons|.
+          |buttonsSize| − 1:
+            <ol>
+              <li>Determine the |type| of the button from the provided {{GamepadButtonType}}
+              array, defaulting to null if not specified.
+              </li>
+              <li>[=list/Append=] a [=new=] {{GamepadButton}} to |buttons|

This should reference the |type| variable we defined above:

```
<li>Let |button:GamepadButton| be a [=new=] {{GamepadButton}} with its {{GamepadButton/type}} attribute initialized to |type|.
</li>
<li>[=list/Append=] |button| to |buttons|.
</li>
```

> @@ -878,8 +883,15 @@ <h3>
           <li>Initialize |buttons| to be an empty [=list=].
           </li>
           <li>[=list/For each=] |buttonIndex:long| of [=the range=] from 0 to
-          |buttonsSize| − 1, [=list/append=] a [=new=] {{GamepadButton}} to
-          |buttons|.
+          |buttonsSize| − 1:
+            <ol>
+              <li>Determine the |type| of the button from the provided {{GamepadButtonType}}

> Determine the |type| of the button from the provided {{GamepadButtonType}} array

This should be phrased "Let |type| be..." so it's clear we're defining a variable. Also, it's unclear where the GamepadButtonType array came from or how it's being used. If it's provided then it should be passed into this algorithm and given a name, for instance: "To initialize buttons for a gamepad with an array |types| of GamepadButtonType values"

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/gamepad/pull/196#pullrequestreview-2050974002
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/gamepad/pull/196/review/2050974002@github.com>