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

Attempt to bring the Gamepad API examples up to date #33724

Merged
merged 7 commits into from
Jul 24, 2024
Merged

Conversation

eevee
Copy link
Contributor

@eevee eevee commented May 23, 2024

This had some alarmingly ancient and misleading stuff in it. I touched this up very quickly in a jsbin textarea, but it wfm in current Firefox and current Chrome, and based on some bug tracker diving I think it should be fine going back some eight or more years.

It's unfortunate that the examples both link to github projects I have no control over, but one had already drifted out of sync, and I think making the page itself be more correct is more important.

Could probably stand further improvement; this is more a "stop the bleeding" patch after I realized I'd gotten a completely wrong impression about Chrome's behavior from this page.

Description

This code was based around very old engine differences that aren't even explained well in the prose. I am baffled by issue templates.

Motivation

I initially thought the paragraph about Chrome was suggesting that Gamepad's state only updated every so often unless explicitly nudged, or something, then later realized that didn't make any sense since there's no way to "nudge" it. In retrospect it looks like it was referring to how GamepadEvent.gamepad is (to this day) a snapshot in Chrome but a live object in Firefox, but this difference can be avoided entirely by just using navigator.getGamepads() when updating, instead of keeping the event's property around. Also the given code doesn't work in current Chrome anyway!

Then I fixed some other stuff like how connecting two gamepads would start two rAF loops in the final demo?? And got rid of checking whether Gamepad.button is a float or an object because it's been specced as an object for an entire decade and nothing else in the article even explains this. And also just some style nits like .forEach being the most 2013 thing I have ever seen.

Additional details

Please. Please there are so many fields. I have now spent longer writing this issue than I spent on the patch

- Gamepad.buttons has been specced as an object since February 2014.

- All of this stuff about Chrome seems to be to explain why you can't simply hold onto the same Gamepad object and keep checking it in Chrome?  But there's no need to poll to fix that; just read `navigator.getGamepads()`, which works in every browser.
@eevee eevee requested a review from a team as a code owner May 23, 2024 01:05
@eevee eevee requested review from sideshowbarker and removed request for a team May 23, 2024 01:05
@github-actions github-actions bot added Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed labels May 23, 2024
@sideshowbarker sideshowbarker removed their request for review May 23, 2024 01:33
Copy link
Contributor

github-actions bot commented May 23, 2024

Preview URLs

External URLs (2)

URL: /en-US/docs/Web/API/Gamepad_API/Using_the_Gamepad_API
Title: Using the Gamepad API

(comment last updated: 2024-07-24 12:31:43)

wbamberg and others added 3 commits May 22, 2024 21:18
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@bsmth bsmth self-requested a review June 12, 2024 15:18
@bsmth
Copy link
Member

bsmth commented Jul 11, 2024

Heya @eevee thanks for opening this one. I have one prose suggestion for you above, do you plan to come back to this? Thank you :)

@bsmth bsmth added the awaiting response Awaiting for author to address review/feedback label Jul 11, 2024
Co-authored-by: Brian Thomas Smith <brian@smith.berlin>
@eevee
Copy link
Contributor Author

eevee commented Jul 19, 2024

hey! yeah that seems reasonable. (i did think about trying to get the two versions of the demo back in sync, but the upstream one seems to have sprouted some extra features in the meantime that would distract from the MDN article, so.)

@bsmth bsmth removed the awaiting response Awaiting for author to address review/feedback label Jul 24, 2024
Copy link
Member

@bsmth bsmth left a comment

Choose a reason for hiding this comment

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

Super, thank you. I'll merge shortly 👍🏻

@bsmth bsmth merged commit b419a3e into mdn:main Jul 24, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants