Bug 214210 - Cleanup GameController framework button binding with some constants
Summary: Cleanup GameController framework button binding with some constants
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-10 16:23 PDT by Brady Eidson
Modified: 2020-07-10 20:45 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.86 KB, patch)
2020-07-10 16:30 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (5.86 KB, patch)
2020-07-10 17:08 PDT, Brady Eidson
darin: review+
Details | Formatted Diff | Diff
PFL (6.34 KB, patch)
2020-07-10 18:54 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2020-07-10 16:23:52 PDT
Cleanup GameController framework button binding with some constants
Comment 1 Brady Eidson 2020-07-10 16:30:35 PDT
Created attachment 404022 [details]
Patch
Comment 2 Darin Adler 2020-07-10 16:36:29 PDT
Comment on attachment 404022 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404022&action=review

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:38
> +enum class GamepadButtons : uint8_t
> +{

WebKit coding style puts this on a single line.
Comment 3 Brady Eidson 2020-07-10 17:07:01 PDT
(In reply to Darin Adler from comment #2)
> Comment on attachment 404022 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=404022&action=review
> 
> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:38
> > +enum class GamepadButtons : uint8_t
> > +{
> 
> WebKit coding style puts this on a single line.

Yup, in my quick search I saw non-Webkit code's style and copied it. Whoops!
Comment 4 Brady Eidson 2020-07-10 17:07:13 PDT
Failing to apply because it relied on https://bugs.webkit.org/show_bug.cgi?id=212933 landing
Comment 5 Brady Eidson 2020-07-10 17:08:05 PDT
Created attachment 404027 [details]
Patch
Comment 6 Darin Adler 2020-07-10 17:55:09 PDT
Comment on attachment 404027 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404027&action=review

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:101
> +    m_buttonValues.resize(homeButton ? (size_t)GamepadButtons::CenterClusterCenter + 1 : (size_t)GamepadButtons::CenterClusterCenter);

Not thrilled with how this hard-codes which is the last gamepad button 40 lines after the enum. Maybe two named constants right after the enum class would make the relationship clearer? Something like this perhaps:

    static constexpr auto numGamepadButtonsWithoutHomeButton = static_cast<size_t>(GamepadButtons::CenterClusterCenter) + 1;
    static constexpr auto numGamepadButtonsWithHomeButton = static_cast<size_t>(GamepadButtons::CenterClusterCenter);
Comment 7 Brady Eidson 2020-07-10 18:54:07 PDT
Created attachment 404034 [details]
PFL
Comment 8 EWS 2020-07-10 20:44:34 PDT
Committed r264258: <https://trac.webkit.org/changeset/264258>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404034 [details].
Comment 9 Radar WebKit Bug Importer 2020-07-10 20:45:40 PDT
<rdar://problem/65380536>