Repository navigation
feat(input): log SDL controller GUID and active mapping - #2678
Conversation
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 NotRexed#2546, NotRexed#2504 Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <noreply@openai.com>
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 <noreply@anthropic.com>
857bb95 to
0caab6b
Compare
|
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 The request cannot be claimed until a maintainer adds the |
Joey-Magnus
left a comment
There was a problem hiding this comment.
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.
…e controller GUID log line carried in Conflicts, and how each was resolved: - readme.md: kept this branch's front page. NotRexed#2643's paragraph on Play waiting for the game's systemd scope went to docs/user/install.md's Linux section. - docs/development.md (deleted here): NotRexed#2678's controller GUID and mapping log line went to docs/dev/running.md, beside scripted input. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
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 Controllerthrough 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:a711with 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'sSDL_free.host/hos_sdl.handhost/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.--sourcecompares 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 isunknown, unavailable accessors areunavailable.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, GCC16.2.1 20260810, Clang23.1.3, Python 3.12. No GPU, window, controller or performance measurement was used. Initial baseedaef2a99; final commite0864483on87991bc8b. 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.PASS: 64 checks, 0 failuresFAIL: 60 checks, 31 failuresFAIL: 60 checks, 4 failuresThe 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):
Reproduction:
Both changed host C files,
hos_sdl.candpad_sdl.c, pass Clang'sx86_64-w64-mingw32syntax-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:The sole failure was
nx2/test_texture_phi_dominance.py:clang++compiling unchangedemit_spirv_image.cppwas terminated bySIGKILL. 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
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_sw2handles 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 noreply@openai.com