MagicBeanDev/Hookshot

Archived

Watch

Fork 0

feat(input): log SDL controller GUID and active mapping #2678

Closed

github-sync wants to merge 2 commits from github/pr/2678 into main

github-sync commented

2026-11-12 06:22:37 +00:00

Owner

@mooki000 opened this pull request on GitHub · 2026-11-12 05:53 UTC · original
Merged on GitHub in 3abfa0a0ee

What this is for: make controller-mapping reports such as #2546 answerable from hookshot_log.txt, without changing the player's working bindings. What is measured: Linux game-free checks of the real SDL2/SDL3 adapter and connection-log call, with GCC 16.2.1 and Clang 23.1.3: 64 checks pass with each compiler; the same fixture fails against the original connection log. Both changed host C files also pass Windows-target syntax checks without warnings or errors. Not measured: a real controller on Windows, a game run, or a fix for the reporter's physical button mapping.

Refs #2546 and #2504.

Problem

The new attachment in #2546 identifies Core (Plus) Wired Controller through SDL3 and one connected SDL controller (lines 171, 216, 221), but does not record its GUID or active mapping. The reporter has since confirmed a basic PowerA wired Switch-licensed pad with no mode switch. A device name alone cannot distinguish the mapping routes involved (reporter's comment).

There is a concrete reason to collect the actual mapping rather than guess: SDL associates 20d6:a711 with both the PowerA Wired Controller Plus and its GameCube-style variant (SDL device list). Its GameCube-classified HIDAPI route can account for the exact reported permutation. This PR does not assume that ID/backend for the reporter and does not install a mapping workaround.

What changed

host/hos_sdl.c: optional diagnostic exports for SDL2 and SDL3. At a successful connection, obtain the borrowed joystick's hardware GUID and the active gamepad mapping, then release the mapping string through the same loaded library's SDL_free.
host/hos_sdl.h and host/pad_sdl.c: call one connection-log helper at the existing successful-connect site. Keep the controller name, gyroscope suffix and Steam Input suffix.
tests/test_sdl_pad_diagnostics.py: compile the actual loader, adapter and connection call with fake library exports. Cover a known mapping, no returned mapping, zero GUID, missing optional functions, missing SDL2 allocator, SDL3 missing joystick accessor, correct allocation/free ownership, and bounded output through the production-sized log sink. --source compares the same assertions with another checkout's source.
docs/development.md: describe the fields, bounds, status meanings and privacy qualification.

Bindings, input polling, prompt selection, controller enumeration, rumble and mapping-file loading are unchanged. There is no renderer API change and no new setting. Diagnostic queries run on connection, not every controller poll.

What the statuses mean

guid=<32 hex digits>: SDL's hardware GUID; a zero result is unknown, unavailable accessors are unavailable.
mapping=available: SDL returned an active mapping string.
mapping=none: no string was returned; this is not proof that the device is unmapped.
mapping=unavailable: the query or required allocator export is absent.

Neither success nor the mapping name identifies where the mapping originated. The SDL3 API and SDL2 API return a mapping or NULL, not a provenance flag.

Names are limited to 128 bytes. Mapping text is capped at 1024 bytes total, with a truncation marker; it follows the connection line in GUID-labelled parts of at most 512 bytes. That keeps the captured mapping intact inside hos_log's existing 1024-byte per-message buffer rather than enlarging the global logger.

Privacy

The GUID and the mapping describe a device model's button layout; no serial, path, user or account call is made. A mapping the player supplied himself, by environment or file, is logged as he wrote it, within the length limits.

SDL constructs these hardware GUIDs from bus/vendor/product/version, manufacturer/product names and driver bytes, with serial information stored separately (SDL3 GUID construction, HIDAPI serial separation). The active mapping serializer writes the GUID, supplied mapping name/body and platform metadata (SDL3, SDL2). Custom mapping text can therefore contain whatever the player put in it; clipping is not privacy redaction.

How it was tested

Arch Linux, kernel 7.2.15-1-omarchy, GCC 16.2.1 20260810, Clang 23.1.3, Python 3.12. No GPU, window, controller or performance measurement was used. Initial base edaef2a99; final commit e0864483 on 87991bc8b. The final rebase adds only an upstream team-chat script/test change; all five changed file blobs are identical to the tested pre-rebase version.

Check	GCC	Clang
Final SDL2/SDL3 diagnostic fixture	`PASS: 64 checks, 0 failures`	Same
Same fixture against original main source	`FAIL: 60 checks, 31 failures`	Same
Same fixture against the earlier single-line implementation	`FAIL: 60 checks, 4 failures`	Same

The failures are executable assertions, not missing-symbol compile failures. The original source still emits only the old connection message; the earlier implementation loses the tail and truncation note through the real-sized logger. Dependent chunk-byte assertions are entered only when both chunks exist, hence the different reached-check totals on the failing versions.

Example fixture output (authored device data, not the reporter's controller):

hid: controller "Test controller" connected (SDL), with a gyroscope, Steam Input's; guid=03000000341200007856000001000000; mapping=available; mapping-name="Test mapping"
hid: guid=03000000341200007856000001000000; mapping-string[1/1]="03000000341200007856000001000000,Test mapping,a:b1,b:b2,x:b0,y:b3,platform:Windows,"
hid: controller "Test controller" connected (SDL); guid=unknown; mapping=none; mapping-name="?"
hid: controller "Test controller" connected (SDL); guid=unavailable; mapping=unavailable; mapping-name="?"

Reproduction:

python -I -B tests/test_sdl_pad_diagnostics.py --cc gcc
python -I -B tests/test_sdl_pad_diagnostics.py --cc clang
python -I -B tests/test_sdl_pad_diagnostics.py --cc gcc --source /path/to/unmodified/main

Both changed host C files, hos_sdl.c and pad_sdl.c, pass Clang's x86_64-w64-mingw32 syntax-only check using llvm-mingw headers, -Wall -Wextra, with zero warnings/errors. This is a syntax check, not a Windows link or runtime test. Existing Steam-pad checks pass with both compilers, and the SDL library regression fixture passes.

One full suite ran before the final log-chunk correction, on intermediate aba3eb1a:

470 passed, 1 failed, 20 skipped in 157 s (linux, 32 at once)

The sole failure was nx2/test_texture_phi_dominance.py: clang++ compiling unchanged emit_spirv_image.cpp was terminated by SIGKILL. Its cause was not established; it was not a failed shader assertion, and no baseline/OOM explanation is claimed. The failed result is retained, not rerun away. Load average was 16.98 at entry and 37.45 at exit. The final chunk correction was then tested with both compilers and both Windows syntax passes, not another full suite; final focused load was 23.94.

The final branch passes the mechanical PR checks on current main: author/committer identity, assistance trailers, conventional title, test included, and no game data/build output. No core dumps were found at the recorded check.

Not tested

A real controller on Windows, including the reporter's PowerA pad.
Actual SDL2/SDL3 DLL calls, a Windows executable link, or game startup/play.
Physical hotplug or user-supplied mapping overrides in a game.
A green full-suite result on the final head; the retained suite interruption above remains outstanding.

Risks and overlaps

The new exports are optional and cannot make controller support fail to load. The mapping getter is not called if its matching allocator is unavailable. The GUID ABI mirrors SDL2/SDL3's 16-byte byte-array struct returned by value; the borrowed joystick is not closed.

The overlap scan found 44 open PRs sharing a changed file, mostly documentation, and no new textual conflicts. Shared-code hunks were read for #1543, #1600, #1872, #2252, #2459 and #2566. The scan preceded the small logger-boundary correction; the final branch's current-main merge check also passes.

#2252 needs semantic integration even though the text merges: it introduces non-SDL pad_sw2 handles and guards the existing SDL getters. If both changes land, the new diagnostic helper must likewise avoid passing those handles to SDL. That path is absent from this PR's main base; no speculative compatibility shim is included here.

This is diagnostics for #2546, not a controller-map fix. The next useful reporter evidence is one startup log from a build containing the change, with the pad connected.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]

> [@mooki000](https://github.com/mooki000) opened this pull request on GitHub · 2026-11-12 05:53 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2678) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/3abfa0a0ee07d9c48361b61bc8424f6c5fb7bc1f **What this is for:** make controller-mapping reports such as #2546 answerable from `hookshot_log.txt`, without changing the player's working bindings. **What is measured:** Linux game-free checks of the real SDL2/SDL3 adapter and connection-log call, with GCC 16.2.1 and Clang 23.1.3: 64 checks pass with each compiler; the same fixture fails against the original connection log. Both changed host C files also pass Windows-target syntax checks without warnings or errors. **Not measured:** a real controller on Windows, a game run, or a fix for the reporter's physical button mapping. Refs #2546 and #2504. ## Problem The new attachment in #2546 identifies `Core (Plus) Wired Controller` through SDL3 and one connected SDL controller (lines 171, 216, 221), but does not record its GUID or active mapping. The reporter has since confirmed a basic PowerA wired Switch-licensed pad with no mode switch. A device name alone cannot distinguish the mapping routes involved ([reporter's comment](https://git.magicbean.dev/MagicBeanDev/Hookshot/issues/2546)). There is a concrete reason to collect the actual mapping rather than guess: SDL associates `20d6:a711` with both the PowerA Wired Controller Plus and its GameCube-style variant ([SDL device list](https://github.com/libsdl-org/SDL/blob/f00dc919ca35b9756316e86c4777392de442f9b2/src/joystick/controller_list.h#L608-L614)). Its GameCube-classified HIDAPI route can account for the exact reported permutation. This PR does **not** assume that ID/backend for the reporter and does not install a mapping workaround. ## What changed - `host/hos_sdl.c`: optional diagnostic exports for SDL2 and SDL3. At a successful connection, obtain the borrowed joystick's hardware GUID and the active gamepad mapping, then release the mapping string through the same loaded library's `SDL_free`. - `host/hos_sdl.h` and `host/pad_sdl.c`: call one connection-log helper at the existing successful-connect site. Keep the controller name, gyroscope suffix and Steam Input suffix. - `tests/test_sdl_pad_diagnostics.py`: compile the actual loader, adapter and connection call with fake library exports. Cover a known mapping, no returned mapping, zero GUID, missing optional functions, missing SDL2 allocator, SDL3 missing joystick accessor, correct allocation/free ownership, and bounded output through the production-sized log sink. `--source` compares the same assertions with another checkout's source. - `docs/development.md`: describe the fields, bounds, status meanings and privacy qualification. Bindings, input polling, prompt selection, controller enumeration, rumble and mapping-file loading are unchanged. There is no renderer API change and no new setting. Diagnostic queries run on connection, not every controller poll. ### What the statuses mean - `guid=<32 hex digits>`: SDL's hardware GUID; a zero result is `unknown`, unavailable accessors are `unavailable`. - `mapping=available`: SDL returned an active mapping string. - `mapping=none`: no string was returned; this is **not** proof that the device is unmapped. - `mapping=unavailable`: the query or required allocator export is absent. Neither success nor the mapping name identifies where the mapping originated. The [SDL3 API](https://wiki.libsdl.org/SDL3/SDL_GetGamepadMapping) and [SDL2 API](https://wiki.libsdl.org/SDL2/SDL_GameControllerMapping) return a mapping or NULL, not a provenance flag. Names are limited to 128 bytes. Mapping text is capped at 1024 bytes total, with a truncation marker; it follows the connection line in GUID-labelled parts of at most 512 bytes. That keeps the captured mapping intact inside `hos_log`'s existing 1024-byte per-message buffer rather than enlarging the global logger. ### Privacy The GUID and the mapping describe a device model's button layout; no serial, path, user or account call is made. A mapping the player supplied himself, by environment or file, is logged as he wrote it, within the length limits. SDL constructs these hardware GUIDs from bus/vendor/product/version, manufacturer/product names and driver bytes, with serial information stored separately ([SDL3 GUID construction](https://github.com/libsdl-org/SDL/blob/f5e5f6588921eed3d7d048ce43d9eb1ff0da0ffc/src/joystick/SDL_joystick.c#L2693-L2734), [HIDAPI serial separation](https://github.com/libsdl-org/SDL/blob/f5e5f6588921eed3d7d048ce43d9eb1ff0da0ffc/src/joystick/hidapi/SDL_hidapijoystick.c#L855-L916)). The active mapping serializer writes the GUID, supplied mapping name/body and platform metadata ([SDL3](https://github.com/libsdl-org/SDL/blob/f5e5f6588921eed3d7d048ce43d9eb1ff0da0ffc/src/joystick/SDL_gamepad.c#L2182-L2231), [SDL2](https://github.com/libsdl-org/SDL/blob/5d249570393f7a37e037abf22cd6012a4cc56a71/src/joystick/SDL_gamecontroller.c#L1720-L1768)). Custom mapping text can therefore contain whatever the player put in it; clipping is not privacy redaction. ## How it was tested Arch Linux, kernel `7.2.15-1-omarchy`, GCC `16.2.1 20260810`, Clang `23.1.3`, Python 3.12. No GPU, window, controller or performance measurement was used. Initial base `edaef2a99`; final commit `e0864483` on `87991bc8b`. The final rebase adds only an upstream team-chat script/test change; all five changed file blobs are identical to the tested pre-rebase version. | Check | GCC | Clang | |---|---|---| | Final SDL2/SDL3 diagnostic fixture | `PASS: 64 checks, 0 failures` | Same | | Same fixture against original main source | `FAIL: 60 checks, 31 failures` | Same | | Same fixture against the earlier single-line implementation | `FAIL: 60 checks, 4 failures` | Same | The failures are executable assertions, not missing-symbol compile failures. The original source still emits only the old connection message; the earlier implementation loses the tail and truncation note through the real-sized logger. Dependent chunk-byte assertions are entered only when both chunks exist, hence the different reached-check totals on the failing versions. Example fixture output (authored device data, not the reporter's controller): ```text hid: controller "Test controller" connected (SDL), with a gyroscope, Steam Input's; guid=03000000341200007856000001000000; mapping=available; mapping-name="Test mapping" hid: guid=03000000341200007856000001000000; mapping-string[1/1]="03000000341200007856000001000000,Test mapping,a:b1,b:b2,x:b0,y:b3,platform:Windows," hid: controller "Test controller" connected (SDL); guid=unknown; mapping=none; mapping-name="?" hid: controller "Test controller" connected (SDL); guid=unavailable; mapping=unavailable; mapping-name="?" ``` Reproduction: ```sh python -I -B tests/test_sdl_pad_diagnostics.py --cc gcc python -I -B tests/test_sdl_pad_diagnostics.py --cc clang python -I -B tests/test_sdl_pad_diagnostics.py --cc gcc --source /path/to/unmodified/main ``` Both changed host C files, `hos_sdl.c` and `pad_sdl.c`, pass Clang's `x86_64-w64-mingw32` syntax-only check using llvm-mingw headers, `-Wall -Wextra`, with **zero warnings/errors**. This is a syntax check, not a Windows link or runtime test. Existing Steam-pad checks pass with both compilers, and the SDL library regression fixture passes. One full suite ran before the final log-chunk correction, on intermediate `aba3eb1a`: ```text 470 passed, 1 failed, 20 skipped in 157 s (linux, 32 at once) ``` The sole failure was `nx2/test_texture_phi_dominance.py`: `clang++` compiling unchanged `emit_spirv_image.cpp` was terminated by `SIGKILL`. Its cause was not established; it was not a failed shader assertion, and no baseline/OOM explanation is claimed. The failed result is retained, not rerun away. Load average was 16.98 at entry and 37.45 at exit. The final chunk correction was then tested with both compilers and both Windows syntax passes, **not another full suite**; final focused load was 23.94. The final branch passes the mechanical PR checks on current main: author/committer identity, assistance trailers, conventional title, test included, and no game data/build output. No core dumps were found at the recorded check. ## Not tested - A real controller on Windows, including the reporter's PowerA pad. - Actual SDL2/SDL3 DLL calls, a Windows executable link, or game startup/play. - Physical hotplug or user-supplied mapping overrides in a game. - A green full-suite result on the final head; the retained suite interruption above remains outstanding. ## Risks and overlaps The new exports are optional and cannot make controller support fail to load. The mapping getter is not called if its matching allocator is unavailable. The GUID ABI mirrors SDL2/SDL3's 16-byte byte-array struct returned by value; the borrowed joystick is not closed. The overlap scan found 44 open PRs sharing a changed file, mostly documentation, and no new textual conflicts. Shared-code hunks were read for #1543, #1600, #1872, #2252, #2459 and #2566. The scan preceded the small logger-boundary correction; the final branch's current-main merge check also passes. **#2252 needs semantic integration even though the text merges:** it introduces non-SDL `pad_sw2` handles and guards the existing SDL getters. If both changes land, the new diagnostic helper must likewise avoid passing those handles to SDL. That path is absent from this PR's main base; no speculative compatibility shim is included here. This is diagnostics for #2546, not a controller-map fix. The next useful reporter evidence is one startup log from a build containing the change, with the pad connected. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]>

github-sync added 2 commits

2026-11-12 06:22:37 +00:00

feat(input): log SDL controller GUID and active mapping e086448329

Make controller mapping reports actionable without changing bindings or loading a mapping override. Keep the hardware GUID and active mapping diagnostics optional for SDL2 and SDL3, retain existing connection suffixes, and bound the returned mapping text.

Refs #2546, #2504

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra <[email protected]>

fix(test): the controller diagnostics fixture builds with MinGW 857bb9584d

The fixture counted mapping requests in a variable named gets. MinGW's
stdio.h still declares gets(), so the Windows checks failed to compile
it ("'gets' redeclared as different kind of symbol") and the new test
never ran there. The counter is named takes.

Tested: the generated fixture compiled for the Windows target (clang
--target=x86_64-w64-mingw32, mingw-w64 headers, -std=c11 -Wall -Wextra
-Werror, syntax only): 11 errors before, none now. Linux, GCC 16.2.1
and Clang 23.1.3: 64 checks, 0 failures, as before.
Not tested: the test run on Windows itself (the project's Windows job
does that).

Assisted-by: Claude Fable 5.2
Co-Authored-By: Claude Fable 5.2 <[email protected]>

github-sync force-pushed github/pr/2678 from 857bb9584d to 0caab6bc82

2026-11-12 09:31:53 +00:00

Compare

github-sync commented

2026-11-12 12:37:24 +00:00

Author

Owner

@mooki000 commented on GitHub · 2026-11-12 12:29 UTC · original

This has not been run with a real controller: this PC has none, and the checks here drive the connection code with stand-ins for SDL. Under the evidence rule merged last night (#2712) that is a gap, so I have asked the team's test queue for it: request T-6RQB, any controller on Windows or Linux, a one-minute check (start the game with the pad plugged in and post the hid: controller "…" connected (SDL)… guid=…; mapping=…; mapping-name="…" line from hookshot_log.txt).

The request cannot be claimed until a maintainer adds the testing wanted label here; could one of you add it? A third-party pad (PowerA, 8BitDo) is the most useful, since this line is what #2546 needs to tell a wrong mapping from a wrong prompt set.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 12:29 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2678#issuecomment-6823639976) This has not been run with a real controller: this PC has none, and the checks here drive the connection code with stand-ins for SDL. Under the evidence rule merged last night (#2712) that is a gap, so I have asked the team's test queue for it: request **T-6RQB**, any controller on Windows or Linux, a one-minute check (start the game with the pad plugged in and post the `hid: controller "…" connected (SDL)… guid=…; mapping=…; mapping-name="…"` line from `hookshot_log.txt`). The request cannot be claimed until a maintainer adds the `testing wanted` label here; could one of you add it? A third-party pad (PowerA, 8BitDo) is the most useful, since this line is what #2546 needs to tell a wrong mapping from a wrong prompt set.

github-sync referenced this pull request

2026-11-12 13:39:28 +00:00

[v2.1.0] Nintendo Switch button prompts become incorrect after restart despite saved custom mappings #2546

github-sync commented

2026-11-12 14:44:03 +00:00

Author

Owner

@Joey-Magnus approved on GitHub · 2026-11-12 14:34 UTC · original

I merged this into current main and ran tests/test_sdl_pad_diagnostics.py (64 checks, 0 failures) and run_all.py sdl pad steam controller (4 passed). CI is green.

I checked the loader changes in host/hos_sdl.c. JoystickGUID, PadMapping, SDL3's GetJoystick and SDL2's Free are all looked up with plain GetProcAddress, not GET, so an older SDL without them still loads. Assigning f.GetJoystick under SDL3 is harmless because hos_sdl_pad_id takes the GetGamepadID path when version == 3. Loading SDL_free for SDL2 doesn't change the two existing Free calls, which are SDL3-only paths. hos_sdl_pad_log_connection (:622) frees the mapping with the same library's allocator, never closes the borrowed joystick, and keeps each log line under hos_log's 1024 bytes. A 16-byte struct returned by value has the same ABI on both sides.

As you noted, if #2252 lands its non-SDL pad_sw2 handles must not reach this helper. Whichever merges second should add that guard. A real pad's log line (the requested T-6RQB) would still be good to have, but it doesn't block a diagnostics-only change.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 14:34 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2678#pullrequestreview-5876230718) I merged this into current main and ran tests/test_sdl_pad_diagnostics.py (64 checks, 0 failures) and `run_all.py sdl pad steam controller` (4 passed). CI is green. I checked the loader changes in host/hos_sdl.c. `JoystickGUID`, `PadMapping`, SDL3's `GetJoystick` and SDL2's `Free` are all looked up with plain `GetProcAddress`, not `GET`, so an older SDL without them still loads. Assigning `f.GetJoystick` under SDL3 is harmless because `hos_sdl_pad_id` takes the `GetGamepadID` path when `version == 3`. Loading `SDL_free` for SDL2 doesn't change the two existing `Free` calls, which are SDL3-only paths. `hos_sdl_pad_log_connection` (:622) frees the mapping with the same library's allocator, never closes the borrowed joystick, and keeps each log line under hos_log's 1024 bytes. A 16-byte struct returned by value has the same ABI on both sides. As you noted, if #2252 lands its non-SDL `pad_sw2` handles must not reach this helper. Whichever merges second should add that guard. A real pad's log line (the requested T-6RQB) would still be good to have, but it doesn't block a diagnostics-only change.

github-sync referenced this pull request from a commit

2026-11-12 17:57:17 +00:00

feat(input): log SDL controller GUID and active mapping (#2678)

github-sync referenced this pull request

2026-11-12 17:58:03 +00:00

docs: rewrite the docs: player docs in docs/user/, developer docs in docs/dev/, the readme as a front page #2561

github-sync closed this pull request

2026-11-12 17:58:28 +00:00

Commenting is not possible because the repository is archived.

No reviewers