GamePads.update() doesn't remove entries from current upon gamepad disconnect

[leweaver]

When a gamepad is disconnected, it should also be removed from the app.gamepads.current and app.gamepads.previous arrays.

It seems that the logic in the update method only iterates over the number of devices returned from Poll(), so doesn't get a chance to set excess entries to null:

engine/src/input/game-pads.js

Line 109 in d4b8e0a

for (i = 0; i < len; i++) {

[OmarShehata]

I recently pushed this code in a PR as a fix for something, and I guess I broke something else!

I wonder if just a simple fix where you loop over max(pads.length, this.current.length) and delete excess if you find any would suffice.

[ransico]

I think the bug existed prior to your recent submission as well - so I don't think your recent PR regressed this :)

Your suggestion sounds like it should do the trick though!

[OmarShehata]

I've been thinking about this, and I just realized that might not be good enough @ransico . If you connected gamepads A and B, then current would look like this:

current = [A,B]

So gamepads.wasPressed(1,0) would return true if controller B pressed the 0 button. If you disconnect controller A, and you remove it from current then suddenly it becomes:

current = [B]

Where gamepads.wasPressed(1,0) wouldn't refer to anything and gamepads.wasPressed(0,0) is actually now controller B. I can see this being annoying if you had Player 1 using controller A, and Player 2 using controller B, that if A gets disconnected temporarily then suddenly B would be controlling the other player.

What doesn't change is pad.index. So B.index would still be 1.

I guess the question is, should PlayCanvas be handling making the the gamepads persistent in the array (we could index it by pad.index and it would be current = [null,B] when disconnected, which would match the GamePad API), or should that be something the developer sorts out on their own?

[leweaver]

Sorry for the delayed response - I've been out of the office for the past 10 days!

No easy answers on this one - the gamepads API doesn't really support the notion of 'pinning' a physical controller to a particular player number (especially across disconnect/reconnect).

If the number and ordering of gamepads mattered strongly to a developer across disconnect/reconnect, I suspect they would need to detect a change in the number of connected gamepads and respond by pausing the app, presenting a dialog of some description allowing players to 'reclaim' their controlled avatar.

I don't have a strong opinion on whether you should introduce gaps in the array; while it does introduce consistency with the Gamepad API - that is always subject to change since it is not a ratified standard no may be an own-goal in the long run. Additionally, introducing gaps could be more error prone in apps, since developers will need to check for nulls (and not just take gamepad 0)