Let a control map listen to any keyboard, and fix the dead arrow keys

The sidescroller's controls did nothing. `akgl_controller_handle_event` matched
`event->key.which == curmap->kbid` exactly, with no way to say "whatever
keyboard the player is typing on", so the game did the obvious thing and bound
`SDL_GetKeyboards()[0]`.

That cannot work. The id a key event *carries* is chosen by the video backend
and is not required to be an id `SDL_GetKeyboards()` reports. On X11 without
XInput2 every key event carries `SDL_GLOBAL_KEYBOARD_ID`, which is 0, while
`SDL_AddKeyboard` registers the attached keyboard as `SDL_DEFAULT_KEYBOARD_ID`,
which is 1; with XInput2 the events carry the physical slave device's
`sourceid` while `SDL_GetKeyboards()[0]` may be the master. Either way the map
matched nothing and every key press was dropped.

A `kbid` or `jsid` of 0 now matches any device of that kind. 0 is safe to spend:
SDL documents `which` as 0 when the source is unknown or virtual, and joystick
ids start at 1, so no real device is 0. A non-zero id still matches only that
device, which is what keeps two local players on two keyboards apart, and there
is a test for that half too. `util/charviewer.c` already passed 0 and depended
on the old behaviour by accident; it works under XInput2 now as well.

Two reasons this shipped, both closed:

- Every test in tests/controller.c dispatched an event whose id equalled the id
  the map was bound with, so none of them could see it.
- The sidescroller's smoke test called the control handlers directly instead of
  dispatching events, so it passed against a control map that matched nothing.
  It now pushes synthetic events through akgl_controller_handle_event from a
  deliberately non-zero device id and fails if the press does not arrive.
  Verified by breaking the binding and watching the run exit non-zero.

`test_controller_wildcard_device_ids` was written first and failed against the
unfixed library with "a map bound to keyboard 0 ignored a key press from device
11", which is the whole reason to trust it now that it passes.

The controls were documented, in one sentence. Chapter 19 has a proper table of
them, and chapter 15 explains why not to reach for SDL_GetKeyboards(). The
header said the match was exact and now says what it does.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-01 21:23:02 -04:00
parent 15ac5d5dd0
commit 943bf4324e
8 changed files with 233 additions and 53 deletions

25
TODO.md
View File

@@ -2306,6 +2306,31 @@ The manual documents each of these where a reader would hit it, and points here.
(`ACTOR_STATE_ALIVE`) and `velocity_x`/`velocity_y` instead of `speed_x`/`speed_y`.
The current loader accepts neither.
19. **A control map could not say "any keyboard", so binding one was a coin flip.**
`akgl_controller_handle_event` matched `event->key.which == curmap->kbid` exactly, and
there was no way to express "whatever keyboard the player types on". That is not a
theoretical gap, because the id a key event *carries* is chosen by the video backend
and does not have to be an id `SDL_GetKeyboards()` reports: on X11 without XInput2
every key event carries `SDL_GLOBAL_KEYBOARD_ID` (0) while SDL registers the keyboard
as `SDL_DEFAULT_KEYBOARD_ID` (1), and with XInput2 the events carry the physical slave
device's `sourceid` while `SDL_GetKeyboards()[0]` may be the master. A game that bound
`SDL_GetKeyboards()[0]` -- the obvious thing -- got a map that matched nothing and
controls that silently did nothing. `examples/sidescroller` shipped exactly that.
**Fixed**: `kbid`/`jsid` of 0 now match any device of that kind. 0 is safe to spend
because SDL documents `which` as 0 when the source is unknown or virtual and joystick
ids start at 1, so no real device is 0. A non-zero id still matches only that device,
which is what keeps two local players apart. `util/charviewer.c` was already passing 0
and depending on the old accidental behaviour, so it works under XInput2 now too.
The reason this reached a release is worth keeping: **every test in
`tests/controller.c` dispatched an event whose id equalled the id the map was bound
with**, so none of them could see it, and the sidescroller's smoke test called the
handlers directly rather than dispatching events, so it passed against a control map
that matched nothing. Both are fixed -- `test_controller_wildcard_device_ids` asserts
the contract, and the smoke test now drives `akgl_controller_handle_event` from a
non-zero device id and fails if the press does not arrive.
### Header comments that describe code that has changed
Twenty-seven claims across the public headers were false when checked against `src/`.

View File

@@ -122,8 +122,29 @@ defaults are already in slots 07 and your additions land after them.
A match requires **all three** of: the event type equals the binding's `event_on` or
`event_off`; the device id on the event equals the map's `kbid` (keyboard events) or `jsid`
(gamepad events); and the key or button equals the binding's. The device-id half is what
keeps two players on two keyboards apart, and it is exact — `akgl_controller_list_keyboards`
logs every attached keyboard and its id, which is how you find the number to pass.
keeps two players on two keyboards apart — `akgl_controller_list_keyboards` logs every
attached keyboard and its id, which is how you find the number to pass.
### Bind device 0 unless you mean a specific device
**A `kbid` or `jsid` of `0` matches every device of that kind**, and it is what a
single-player game wants.
Do not reach for `SDL_GetKeyboards()` to fill it in. That call reports what is *attached*;
the id a key event actually *carries* is chosen by the video backend, and the two need not
agree. On X11 without XInput2 every key event carries `SDL_GLOBAL_KEYBOARD_ID`, which is
`0`, while SDL registers the attached keyboard as `SDL_DEFAULT_KEYBOARD_ID`, which is `1`.
With XInput2 the events carry the physical slave device's `sourceid` and
`SDL_GetKeyboards()[0]` may be the master. Binding `SDL_GetKeyboards()[0]` therefore gives
you a map that matches nothing at all, and controls that silently do nothing.
That is not hypothetical — the sidescroller in [Chapter 19](19-tutorial-sidescroller.md)
shipped exactly that bug, and its smoke test passed anyway because the test called the
handlers instead of dispatching events.
`0` is the right spelling for "any" because SDL documents `which` as `0` when the source is
unknown or virtual, and joystick ids start at 1 — so no real device is `0` and nothing is
given up by spending it.
There is no way to remove a binding. `akgl_controller_pushmap` appends and that is the
whole API; the only reset is zeroing `akgl_controlmaps[id]` yourself. **Calling

View File

@@ -23,10 +23,20 @@ cmake --build build --parallel --target sidescroller
./build/examples/sidescroller/sidescroller
```
Arrow keys to run, space to jump, and holding space longer jumps higher. Three flags exist
for the smoke test and are useful by hand too: `--assets DIR` points somewhere other than
the compiled-in asset directory, `--frames N` (or `AKGL_SIDESCROLLER_FRAMES`) exits after
that many frames, and `--autoplay` drives the player from a script instead of the keyboard.
### Controls
| Key | Gamepad | Does |
|---|---|---|
| Left arrow | D-pad left | Run left |
| Right arrow | D-pad right | Run right |
| Space | South button (A) | Jump — hold it longer to jump higher |
Close the window to quit; there is no key bound to it.
Three flags exist for the smoke test and are useful by hand too: `--assets DIR` points
somewhere other than the compiled-in asset directory, `--frames N` (or
`AKGL_SIDESCROLLER_FRAMES`) exits after that many frames, and `--autoplay` drives the
player from a script instead of the keyboard.
## The level
@@ -745,22 +755,37 @@ set_tests_properties(example_sidescroller PROPERTIES
```
Four seconds of scripted play under the headless drivers. `--autoplay` holds *right* and
requests a jump every forty-five frames by calling the handlers a keyboard would have
called:
taps jump every forty-five frames, and it does it by pushing synthetic events through
`akgl_controller_handle_event` rather than by calling the handlers:
```c excerpt=examples/sidescroller/player.c
if ( frame == 1 ) {
PASS(errctx, akgl_actor_cmhf_right_on(ss_game.player, &synthetic));
}
if ( (frame % SS_AUTOPLAY_JUMP_PERIOD) == 0 ) {
ss_game.jump_requested = true;
synthetic.type = SDL_EVENT_KEY_DOWN;
synthetic.key.which = SS_AUTOPLAY_KBID;
synthetic.key.key = SDLK_RIGHT;
PASS(errctx, akgl_controller_handle_event((void *)&akgl_game.state, &synthetic));
FAIL_ZERO_RETURN(
errctx,
AKGL_BITMASK_HAS(ss_game.player->state, AKGL_ACTOR_STATE_MOVING_RIGHT),
AKGL_ERR_BEHAVIOR,
"the right arrow did not reach the player; the control map matched nothing"
);
}
```
That works because a control-map handler takes the actor and an event, and the keyboard
handlers do not read the event beyond requiring one to be there. In four seconds the script
walks the level, jumps, collides with terrain, collects a coin, falls in the pit and
respawns — so the smoke test exercises the collision and the pickup, not just the startup.
**That distinction is the whole value of the smoke test, and the first version got it
wrong.** Calling the handlers directly is easier and it works — but it skips the binding
table, which is the part with something to get wrong. This game shipped with a control map
that matched no keyboard on earth, and a smoke test that called the handlers passed anyway.
A test that cannot fail is not a test, so the script drives the real dispatch and then
asserts the press actually arrived. Break the binding and the run exits non-zero naming the
reason.
`SS_AUTOPLAY_KBID` is deliberately not `0`: the map binds keyboard `0` meaning *any*
keyboard, and driving it from a non-zero device id is what proves that. In four seconds the
script walks the level, jumps, collides with terrain, collects a coin, falls in the pit and
respawns — so the smoke test exercises the input path, the collision and the pickup, not
just the startup.
A run ends with a line naming what happened, which is worth reading when a change moves the
feel:

View File

@@ -315,9 +315,6 @@ akerr_ErrorContext *ss_player_controls(int controlmapid, char *actorname)
{
akgl_ControlMap *controlmap = NULL;
akgl_Control control;
SDL_KeyboardID *keyboards = NULL;
SDL_JoystickID *gamepads = NULL;
int count = 0;
PREPARE_ERROR(errctx);
FAIL_ZERO_RETURN(errctx, actorname, AKERR_NULLPOINTER, "actorname");
@@ -345,26 +342,19 @@ akerr_ErrorContext *ss_player_controls(int controlmapid, char *actorname)
);
/*
* A control map listens to exactly one keyboard and one gamepad, matched by
* id, and a binding whose id does not match the event's is not consulted.
* That is what keeps two local players on two keyboards apart -- and it is
* also why "the arrow keys do nothing" is usually the wrong id rather than
* the wrong key.
* A control map listens to one keyboard and one gamepad, matched by id, and
* 0 means "any". That is what a one-player game wants, and it is the only
* thing that works: SDL_GetKeyboards() reports what is attached, but the id
* a key event carries is chosen by the video backend, and on X11 it is
* SDL_GLOBAL_KEYBOARD_ID (0) without XInput2 and the physical device's
* sourceid with it -- never the SDL_DEFAULT_KEYBOARD_ID of 1 that SDL
* registers when XInput2 is missing. This function used to bind
* SDL_GetKeyboards()[0] and the arrow keys did nothing at all.
*
* Bind a specific id when two players share a machine on two keyboards.
*/
keyboards = SDL_GetKeyboards(&count);
if ( keyboards != NULL ) {
if ( count > 0 ) {
controlmap->kbid = keyboards[0];
}
SDL_free(keyboards);
}
gamepads = SDL_GetGamepads(&count);
if ( gamepads != NULL ) {
if ( count > 0 ) {
controlmap->jsid = gamepads[0];
}
SDL_free(gamepads);
}
controlmap->kbid = 0;
controlmap->jsid = 0;
/* ---- keyboard ---- */
control.event_on = SDL_EVENT_KEY_DOWN;
@@ -418,19 +408,39 @@ akerr_ErrorContext *ss_player_autoplay(int frame)
FAIL_ZERO_RETURN(errctx, ss_game.player, AKERR_NULLPOINTER, "ss_game.player");
PASS(errctx, aksl_memset((void *)&synthetic, 0x00, sizeof(SDL_Event)));
synthetic.type = SDL_EVENT_KEY_DOWN;
/*
* The headless smoke run has no keyboard, so it calls the handlers a
* keyboard would have called. They take the actor and an event, and the
* keyboard handlers do not read the event beyond requiring one -- which is
* what makes a scripted run possible at all.
* Dispatch through akgl_controller_handle_event rather than calling the
* handlers directly, so a scripted run exercises the binding table the way a
* player does. The first version of this called the handlers, which meant
* the smoke test passed while the control map matched nothing at all and the
* arrow keys did nothing -- a test that cannot fail is not a test.
*
* SS_AUTOPLAY_KBID is deliberately not 0: the map binds keyboard 0 meaning
* "any", and driving it from a non-zero device id is what proves that.
*/
if ( frame == 1 ) {
PASS(errctx, akgl_actor_cmhf_right_on(ss_game.player, &synthetic));
synthetic.type = SDL_EVENT_KEY_DOWN;
synthetic.key.which = SS_AUTOPLAY_KBID;
synthetic.key.key = SDLK_RIGHT;
PASS(errctx, akgl_controller_handle_event((void *)&akgl_game.state, &synthetic));
FAIL_ZERO_RETURN(
errctx,
AKGL_BITMASK_HAS(ss_game.player->state, AKGL_ACTOR_STATE_MOVING_RIGHT),
AKGL_ERR_BEHAVIOR,
"the right arrow did not reach the player; the control map matched nothing"
);
}
if ( (frame % SS_AUTOPLAY_JUMP_PERIOD) == 0 ) {
ss_game.jump_requested = true;
synthetic.type = SDL_EVENT_KEY_DOWN;
synthetic.key.which = SS_AUTOPLAY_KBID;
synthetic.key.key = SDLK_SPACE;
PASS(errctx, akgl_controller_handle_event((void *)&akgl_game.state, &synthetic));
} else if ( (frame % SS_AUTOPLAY_JUMP_PERIOD) == SS_AUTOPLAY_JUMP_HOLD ) {
synthetic.type = SDL_EVENT_KEY_UP;
synthetic.key.which = SS_AUTOPLAY_KBID;
synthetic.key.key = SDLK_SPACE;
PASS(errctx, akgl_controller_handle_event((void *)&akgl_game.state, &synthetic));
}
SUCCEED_RETURN(errctx);
}

View File

@@ -72,6 +72,14 @@
/* How far the autoplay script waits between jumps, in frames. */
#define SS_AUTOPLAY_JUMP_PERIOD 45
/* How long it holds the jump key, in frames. A tap jumps lower than a hold. */
#define SS_AUTOPLAY_JUMP_HOLD 8
/*
* The device id the autoplay script stamps on its synthetic key events.
* Deliberately not 0: the control map binds keyboard 0 meaning "any keyboard",
* and driving it from a non-zero id is what proves that binding works.
*/
#define SS_AUTOPLAY_KBID 11
/**
* @brief What one call to ss_collide_resolve found.

View File

@@ -87,7 +87,7 @@ typedef struct {
akgl_Actor *target; /**< The actor these bindings drive. A `NULL` target makes the whole map inert, which is how an unused slot is spelled. */
uint16_t nextMap; /**< Number of bindings in use; the index akgl_controller_pushmap writes to next. */
akgl_Control controls[AKGL_MAX_CONTROLS]; /**< The bindings, scanned in order. */
SDL_KeyboardID kbid; /**< Keyboard this map listens to. A keyboard event from any other id is ignored, which is what keeps two players on two keyboards apart. */
SDL_KeyboardID kbid; /**< Keyboard this map listens to, or 0 for any. A non-zero id ignores every other keyboard, which is what keeps two players on two keyboards apart. */
SDL_JoystickID jsid; /**< Gamepad this map listens to, matched the same way. */
SDL_MouseID mouseid; /**< Mouse this map listens to. Declared but not yet consulted. */
SDL_PenID penid; /**< Pen this map listens to. Declared but not yet consulted. */
@@ -249,10 +249,14 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_controller_pushmap(int controlmapid, akg
* @param actorname Registry name of the actor to drive. Required in practice,
* though a `NULL` is reported as "not found" rather than as
* a null pointer.
* @param kbid SDL keyboard id to listen to. Only events from this
* keyboard match; see akgl_controller_list_keyboards for
* how to find it.
* @param jsid SDL gamepad id to listen to, matched the same way.
* @param kbid SDL keyboard id to listen to, or **0 for any keyboard**,
* which is what a single-player game wants. Do not pass
* `SDL_GetKeyboards()[0]`: that reports what is attached,
* while the id an event carries is chosen by the video
* backend, and the two need not agree. See
* akgl_controller_list_keyboards.
* @param jsid SDL gamepad id to listen to, or 0 for any gamepad,
* matched the same way.
* @return `NULL` on success, otherwise an error context owned by the caller.
* @throws AKERR_OUTOFBOUNDS If @p controlmapid is at or above
* #AKGL_MAX_CONTROL_MAPS, or if the map cannot hold eight more bindings.

View File

@@ -291,14 +291,32 @@ akerr_ErrorContext *akgl_controller_handle_event(void *appstate, SDL_Event *even
//SDL_Log("button/key is processed by controlmap %d control %d", i, j);
//SDL_Log("event %d -> control on %d off %d", event->type, curcontrol->event_on, curcontrol->event_off);
// This controlmap processes this control
/*
* A map bound to device 0 hears every device of that kind.
*
* Binding a specific id is what keeps two local players on two
* keyboards apart, so it stays -- but a game with one player
* cannot ask for "the keyboard" any other way. SDL_GetKeyboards()
* reports what is attached; the id a key event actually carries
* is chosen by the video backend, and on X11 it is
* SDL_GLOBAL_KEYBOARD_ID (0) without XInput2 and the physical
* slave device's sourceid with it. Neither is the
* SDL_DEFAULT_KEYBOARD_ID of 1 that SDL registers when XInput2 is
* missing, so a game that bound SDL_GetKeyboards()[0] got a map
* that matched nothing and controls that did nothing.
*
* 0 is the right spelling for "any": SDL documents `which` as 0
* when the source is unknown or virtual, and joystick ids start
* at 1, so no real device is 0 and nothing is given up.
*/
eventButtonComboMatch = (
((event->type == SDL_EVENT_GAMEPAD_BUTTON_DOWN ||
event->type == SDL_EVENT_GAMEPAD_BUTTON_UP) &&
event->gbutton.which == curmap->jsid &&
(curmap->jsid == 0 || event->gbutton.which == curmap->jsid) &&
event->gbutton.button == curcontrol->button) ||
((event->type == SDL_EVENT_KEY_DOWN ||
event->type == SDL_EVENT_KEY_UP) &&
event->key.which == curmap->kbid &&
(curmap->kbid == 0 || event->key.which == curmap->kbid) &&
event->key.key == curcontrol->key)
);
if ( event->type == curcontrol->event_on && eventButtonComboMatch) {

View File

@@ -372,6 +372,74 @@ akerr_ErrorContext *test_controller_handle_gamepad_events(void)
SUCCEED_RETURN(e);
}
/*
* A control map bound to keyboard 0 hears every keyboard.
*
* There is no way to enumerate "the keyboard the player will actually type on".
* SDL_GetKeyboards() reports what is attached, but the id a key event carries is
* decided by the video backend: X11 without XInput2 sends
* SDL_GLOBAL_KEYBOARD_ID, which is 0, while X11 *with* XInput2 sends the
* physical slave device's sourceid -- and neither is the SDL_DEFAULT_KEYBOARD_ID
* of 1 that SDL_AddKeyboard registers when XInput2 is missing. A game that binds
* SDL_GetKeyboards()[0] therefore gets a map that matches nothing at all.
*
* 0 is the right spelling for "any", because SDL documents `which` as 0 when the
* source is unknown or virtual, so no real device can be 0 and nothing is given
* up by spending it. The same argument applies to a joystick id of 0.
*
* Every other test in this suite dispatches an event whose id equals the id the
* map was bound with, so none of them can see this.
*/
akerr_ErrorContext *test_controller_wildcard_device_ids(void)
{
PREPARE_ERROR(e);
SDL_Event event;
ATTEMPT {
reset_control_maps();
CATCH(e, make_player());
// 0 for both: "whatever keyboard and whatever gamepad".
CATCH(e, akgl_controller_default(0, "player", 0, 0));
// A key event from a real XInput2 device id must still be heard.
player->state = 0;
make_key_event(&event, SDL_EVENT_KEY_DOWN, TEST_KBID, SDLK_LEFT);
TEST_EXPECT_OK(e, akgl_controller_handle_event(&appstate_placeholder, &event),
"dispatching a left key press from a real device id");
TEST_ASSERT(e, AKGL_BITMASK_HAS(player->state, AKGL_ACTOR_STATE_MOVING_LEFT),
"a map bound to keyboard 0 ignored a key press from device %d (state %d)",
TEST_KBID, player->state);
// And so must one carrying SDL_GLOBAL_KEYBOARD_ID, which is 0.
player->state = 0;
make_key_event(&event, SDL_EVENT_KEY_DOWN, 0, SDLK_RIGHT);
TEST_EXPECT_OK(e, akgl_controller_handle_event(&appstate_placeholder, &event),
"dispatching a right key press from the global keyboard");
TEST_ASSERT(e, AKGL_BITMASK_HAS(player->state, AKGL_ACTOR_STATE_MOVING_RIGHT),
"a map bound to keyboard 0 ignored a key press from keyboard 0 (state %d)",
player->state);
/*
* A map bound to a *specific* keyboard still hears only that one. This is
* the half that keeps two local players on two keyboards apart, and the
* wildcard must not cost it.
*/
reset_control_maps();
CATCH(e, akgl_controller_default(0, "player", TEST_KBID, TEST_JSID));
player->state = 0;
make_key_event(&event, SDL_EVENT_KEY_DOWN, TEST_KBID + 1, SDLK_LEFT);
TEST_EXPECT_OK(e, akgl_controller_handle_event(&appstate_placeholder, &event),
"dispatching a left key press from the wrong device");
TEST_ASSERT(e, (player->state == 0),
"a map bound to keyboard %d acted on a press from %d (state %d)",
TEST_KBID, (TEST_KBID + 1), player->state);
} CLEANUP {
} PROCESS(e) {
} FINISH(e, true);
SUCCEED_RETURN(e);
}
akerr_ErrorContext *test_controller_handle_event_edge_cases(void)
{
PREPARE_ERROR(e);
@@ -872,6 +940,7 @@ int main(void)
CATCH(errctx, test_controller_default_bindings());
CATCH(errctx, test_controller_handle_keyboard_events());
CATCH(errctx, test_controller_handle_gamepad_events());
CATCH(errctx, test_controller_wildcard_device_ids());
CATCH(errctx, test_controller_handle_event_edge_cases());
CATCH(errctx, test_controller_gamepad_button_handlers());
CATCH(errctx, test_controller_device_events());