Record internal consistency findings and the controller DB defect

Add an "Internal consistency" section to TODO.md: 41 numbered findings
from a sweep of src/ and include/, each cited by file:line and grouped by
blast radius. These are convention problems, not a re-audit of behavior,
but several have live functional consequences:

- AKGL_TIME_ONESEC_MS is misnamed. Its value is nanoseconds-per-
  millisecond, but akgl_game_state_lock uses it as a one-second budget
  against a 100ms delay loop, so the lock blocks ~16 minutes.
- SUCCEED_RETURN inside an ATTEMPT block at tilemap.c:52 bypasses the
  CLEANUP two lines below, leaking two heap strings on every successful
  tilemap property lookup, against a 256-entry pool.
- AKGL_ACTOR_STATE_STRING_NAMES is declared [33] and defined [32], and
  names bits 11 and 12 as UNDEFINED where actor.h defines MOVING_IN and
  MOVING_OUT, so no character JSON can bind a sprite to those states.
- 19 non-static functions are defined in src/ but declared in no header,
  and akgl_game_init_screen is declared but never defined.
- AKGL_COLLIDE_RECTANGLES has unbalanced parentheses and cannot compile.
- text.c:19 checks `name` twice and never validates `filepath`.

Item 30 is marked resolved by the reindent in the preceding commit.

Also add Defects #12: a failed controller-DB fetch silently destroys the
tracked fallback. include/akgl/SDL_GameControllerDB.h is committed on
purpose so the library still builds if upstream disappears, but
mkcontrollermappings.sh has no `set -e` and never checks curl's exit
status -- on failure it overwrites the good header with
AKGL_SDL_GAMECONTROLLER_DB_LEN 0 and an empty initializer, and exits 0.
Verified against an unresolvable host. CMakeLists.txt:89-93 compounds
this: the custom command's OUTPUT is relative, so CMake resolves it
against the binary dir while the script writes to the source dir, leaving
the command permanently out of date and re-running on every build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-30 18:17:58 -04:00
parent 28bde4176d
commit 772f960865

391
TODO.md
View File

@@ -1,5 +1,369 @@
# TODO # TODO
## Internal consistency
Findings from a sweep of `src/` and `include/`. These are consistency and
convention problems, not new functional defects — where one has a functional
consequence it is called out. Ordered roughly by blast radius. Items that
overlap the existing **Defects** list are cross-referenced rather than repeated.
### 1. Public naming conventions
1. **Include guards use three different schemes.** `_AKGL_ACTOR_H_`,
`_AKGL_CHARACTER_H_`, `_AKGL_GAME_H_`, `_AKGL_HEAP_H_`, `_AKGL_ITERATOR_H_`,
`_AKGL_SPRITE_H_`, and `_AKGL_TYPES_H_` carry the project prefix; `_ASSETS_H_`,
`_CONTROLLER_H_`, `_DRAW_H_`, `_ERROR_H_`, `_JSON_HELPERS_H_`, `_PHYSICS_H_`,
`_REGISTRY_H_`, `_RENDERER_H_`, `_TEXT_H_`, `_TILEMAP_H_`, and `_UTIL_H_` do
not. Pick `_AKGL_<FILE>_H_` everywhere.
The worst case is `include/akgl/staticstring.h:6`, which guards with
`_STRING_H_` — a name several libc implementations use for their own
`<string.h>`. The same file then does `#include "string.h"` (line 9) with
quotes, which is a relative-first lookup that only reaches the system header
by accident. Rename the guard to `_AKGL_STATICSTRING_H_` and use
`#include <string.h>`.
2. **Function names contradict the stated convention.** `AGENTS.md` says public
symbols use `akgl_` and types use `akgl_TypeName`. Exceptions:
- `akgl_Actor_cmhf_left_on` and its seven siblings (`include/akgl/actor.h:196-252`)
embed the *type* name in a *function* name, unlike every other actor entry
point (`akgl_actor_*`). Rename to `akgl_actor_cmhf_*`.
- `akgl_game_updateFPS` (`include/akgl/game.h:104`) is camelCase; every other
function is snake_case.
- `akgl_render_init2d` (`include/akgl/renderer.h:83`) puts the `2d` at the end
while the six functions it installs are `akgl_render_2d_*`. Make it
`akgl_render_2d_init`.
- `akgl_sprite_sheet_coords_for_frame` (`include/akgl/sprite.h:86`) spells it
`sprite_sheet`; `akgl_spritesheet_initialize` and `akgl_heap_next_spritesheet`
spell it `spritesheet`.
3. **Two public types have no prefix at all.** `point` and `RectanglePoints`
(`include/akgl/util.h:13-25`) are unprefixed, and `RectanglePoints` is
PascalCase with no namespace. Both are dumped into every translation unit
that includes `util.h`. Rename to `akgl_Point` / `akgl_RectanglePoints`.
4. **Global variables use four different conventions.** `window`, `bgm`, `game`,
`gamemap`, `renderer`, `physics`, and `camera` (`src/game.c:26-43`) are
unprefixed single common words exported from a shared library — `renderer`
and `camera` in particular are very likely to collide with a consuming game.
Alongside them the same file exports `akgl_mixer` and `akgl_tracks` (prefixed),
`_akgl_renderer` / `_akgl_camera` / `_akgl_physics` / `_akgl_gamemap`
(underscore-prefixed, which is reserved at file scope), and elsewhere
`HEAP_ACTOR``HEAP_STRING` (`src/heap.c:17-21`) and `GAME_ControlMaps`
(`src/controller.c:13`) are SCREAMING_SNAKE, which the convention reserves for
constants and macros. Settle on `akgl_` for all exported objects.
5. **`AKGL_SPRITE_MAX_CHARACTER_NAME_LENGTH`** (`include/akgl/character.h:13`) is a
character constant carrying the `SPRITE` prefix, and lives in `character.h`.
Rename to `AKGL_CHARACTER_MAX_NAME_LENGTH`.
6. **`AKGL_TIME_ONESEC_MS` is misnamed and the error is live.**
`include/akgl/game.h:22` defines it as `1000000`. One second in milliseconds
is `1000`; `1000000` is the number of nanoseconds in a *millisecond*. The name
says "one second" but the value means "one millisecond", so:
- `src/character.c:209` and `src/sprite.c:141` use it correctly as a
milliseconds-to-nanoseconds scale factor.
- `src/game.c:136` uses it as documented — a one-second budget for the state
lock — in a loop that advances `totaltime += 100` alongside `SDL_Delay(100)`.
The loop therefore runs 10,000 iterations of 100 ms, so `akgl_game_state_lock`
blocks for roughly 16 minutes rather than 1 second before reporting failure.
Rename to `AKGL_TIME_ONEMS_NS` to match `AKGL_TIME_ONESEC_NS`, and give
`akgl_game_state_lock` a real one-second budget.
### 2. Header/implementation surface drift
7. **Nineteen non-static functions are defined in `src/` but declared in no
header.** They have external linkage and public-looking names, so they are
part of the ABI whether intended or not, and no consumer can call them:
`akgl_game_load_objectnamemap`, `akgl_game_load_versioncmp`,
`akgl_game_save_actors`, `akgl_game_save_actorname_iterator`,
`akgl_game_save_charactername_iterator`, `akgl_game_save_spritename_iterator`,
`akgl_game_save_spritesheetname_iterator` (`src/game.c`);
`akgl_get_json_properties_double`, `akgl_get_json_properties_float`,
`akgl_get_json_properties_number`, `akgl_tilemap_load_layer_image`,
`akgl_tilemap_load_layer_object_actor`, `akgl_tilemap_load_physics`
(`src/tilemap.c`); `akgl_path_relative_from`, `akgl_path_relative_root`
(`src/util.c`); `gamepad_handle_added`, `gamepad_handle_button_down`,
`gamepad_handle_button_up`, `gamepad_handle_removed` (`src/controller.c`).
Each should be either declared in its header or made `static`. Note that
`tilemap.h` already has a "part of the internal API, exposed here for unit
testing" block — the tilemap entries belong there.
8. **`akgl_game_init_screen` is declared but never defined**
(`include/akgl/game.h:100`). Same failure mode as **Defects → Known and still
open #10**, which covers the four `akgl_controller_handle_*` declarations;
fold this one into that item.
9. **Static helpers use three different naming styles.** `actor_visible`
(`src/actor.c:185`) is bare; `akgl_character_load_json_inner` and
`akgl_character_load_json_state_int_from_strings` (`src/character.c:101,132`)
and `akgl_sprite_load_json_spritesheet` (`src/sprite.c:51`) carry the full
public prefix; `gamepad_handle_*` (`src/controller.c:121+`) uses a third
subsystem word that appears nowhere else. Adopt one rule — the clearest is
that `static` helpers drop the `akgl_` prefix, since it exists to avoid
external collisions.
10. **Parameter names disagree between declaration and definition.** Doxygen
documents the header spelling, so the generated docs describe names the
implementation does not use:
| Header | Implementation |
|---|---|
| `character.h:41` `basechar` | `character.c:21` `obj` |
| `character.h:69` `props` | `character.c:75` `registry` |
| `heap.h:121` `ptr` | `heap.c:143` `basechar` |
| `registry.h:97` `value` | `registry.c:163` `src` |
| `json_helpers.h:134` `e` | `json_helpers.c:149` `err` |
| `tilemap.h:134,143` `dest` | `tilemap.c:646,767` `map` |
The `tilemap.h` pair is the most misleading: the parameter is the map being
*read* and drawn, but it is named `dest` and documented as "Output
destination populated by the function".
11. **Object-pool size macros are defined twice, and the override hook is
dead.** `heap.h:15-29` wraps `AKGL_MAX_HEAP_ACTOR`, `_SPRITE`, `_SPRITESHEET`,
`_CHARACTER`, and `_STRING` in `#ifndef` guards so a consumer can override
them, but `actor.h:65`, `sprite.h:19-20`, and `character.h:14` define the same
four unconditionally and are included from `heap.h:9-11`. Whichever header
lands first wins and the `#ifndef` never fires, so the override mechanism
cannot work. Define each pool size once — `heap.h` is the natural home.
12. **Headers rely on their includers for types.** `iterator.h` uses `uint32_t`
without `<stdint.h>`; `json_helpers.h` uses `json_t` without `<jansson.h>`;
`util.h` uses `SDL_FRect` and `bool` without any SDL include; `text.h` uses
`SDL_Color` and `akerr_ErrorContext` without including SDL or `akerror.h`;
`controller.h` uses `akgl_Actor` without including `actor.h`. Each compiles
only because of `.c`-file include ordering. Headers should be self-contained.
13. **Include spelling is split between quoted and angled forms for the same
directory.** `actor.h:10-11`, `character.h:10-11`, `controller.h:11`,
`game.h:11-14`, `json_helpers.h:10`, and `staticstring.h:9` use
`#include "sibling.h"`; `physics.h:11-13`, `registry.h:9-10`, `renderer.h:13`,
`tilemap.h:10-12`, and `util.h:10` use `#include <akgl/sibling.h>`. The
angled form is correct for an installed library.
14. **Empty parameter lists.** `akgl_heap_init()`, `akgl_heap_init_actor()`,
`akgl_registry_init*()`, `akgl_game_init()`, `akgl_game_init_screen()`, and
`akgl_game_updateFPS()` declare `()` rather than `(void)`, while
`akgl_controller_list_keyboards(void)`, `akgl_controller_open_gamepads(void)`,
`akgl_game_lowfps(void)`, and `akgl_game_state_lock(void)` use `(void)`.
Before C23 the two are not equivalent — `()` suppresses argument checking.
`akgl_heap_init_actor` is even declared `()` in `heap.h:57` and defined
`(void)` in `heap.c:48`.
15. **`AKERR_NOIGNORE` is applied inconsistently at definition sites.** Headers
use it uniformly (except `akgl_sprite_sheet_coords_for_frame`, `sprite.h:86`,
which omits it). Definitions are split even within one file: `registry.c:55,120,163,172`
repeat it, `registry.c:27,44,63,71,79,96,104,112` do not. Since the attribute
is already on the declaration, drop it from all definitions.
### 3. Error-handling pattern
16. **`*_RETURN` macros are used inside `ATTEMPT` blocks, which skips `CLEANUP`.**
The established pattern is `FAIL_*_BREAK` / `CATCH` inside `ATTEMPT` and
`*_RETURN` outside it. Violations:
- `src/tilemap.c:52``SUCCEED_RETURN` on the success path of
`akgl_get_json_tilemap_property` returns directly from inside `ATTEMPT`,
bypassing the `CLEANUP` at line 54 that releases `tmpstr` and `typestr`.
Every successful property lookup leaks two heap strings, and the string
pool is only 256 entries.
- `src/tilemap.c:620``FAIL_RETURN` inside `ATTEMPT`.
- `src/controller.c:286-289,306``FAIL_ZERO_RETURN` / `FAIL_NONZERO_RETURN`
inside `ATTEMPT`; `src/controller.c:383``SUCCEED_RETURN` inside `ATTEMPT`,
which also leaves `akgl_controller_default` with no return statement on the
path that falls out of `FINISH`.
- `src/game.c:457,462``FAIL_NONZERO_RETURN` inside `ATTEMPT`, skipping the
`fclose` in `CLEANUP` at line 482.
17. **NULL-check discipline varies by function.** In `src/json_helpers.c` only
`akgl_get_json_string_value` (line 71) and `akgl_get_json_array_index_string`
(line 128) validate `key`/`dest`; the other eight accessors validate only the
container and then dereference `dest` unconditionally. In `src/physics.c` the
arcade backends check `actor` but `akgl_physics_null_gravity`,
`_null_collide`, and `_null_move` (lines 15-34) check only `self`. In
`src/renderer.c:65,74`, `akgl_render_2d_frame_start` and `_frame_end`
dereference `self->sdl_renderer` with no check on `self`, while
`akgl_render_2d_draw_texture` (line 82) checks `self` first.
18. **Error-context variable naming is split between `errctx` and `e`,
sometimes within one file.** `src/util.c` uses `e` in the path helpers
(lines 32-116) and `errctx` in the geometry helpers (lines 118-259);
`src/heap.c` uses `errctx` everywhere except `akgl_heap_init_actor` (line 50).
`src/actor.c`, `character.c`, `json_helpers.c`, `registry.c`, `sprite.c`, and
`tilemap.c` favor `errctx`; `game.c`, `physics.c`, `renderer.c`, and
`controller.c` favor `e`. Pick one.
### 4. Types and macros
19. **`float`/`double` are used raw where `types.h` defines aliases.** `types.h`
exports `float32_t` and `float64_t`, and the actor and character structs use
`float32_t` throughout — but `akgl_get_json_number_value` takes `float *`
(`json_helpers.h:55`), `akgl_get_json_double_value` takes `double *` (line 66),
`akgl_PhysicsBackend`'s six drag/gravity fields are `double`
(`physics.h:22-27`), and `akgl_Tilemap`'s perspective fields are `float`
(`tilemap.h:105-108`). Either use the aliases consistently or delete them.
20. **`AKGL_COLLIDE_RECTANGLES` (`include/akgl/util.h:27`) has unbalanced
parentheses** — three opens, two closes — so any use is a syntax error. It has
no callers and duplicates `akgl_collide_rectangles`. Delete it.
21. **Bitmask macros are unparenthesized.** `AKGL_BITMASK_HAS(x, y)` expands to
`(x & y) == y` with no outer parens, so `!AKGL_BITMASK_HAS(a, b)` parses as
`!(a & b) == b`. No in-tree caller negates it today (`AKGL_BITMASK_HASNOT`
exists for that), so this is latent rather than live. `AKGL_BITMASK_CLEAR(x)`
(`game.h:86`) also carries a trailing semicolon inside the macro body, so
normal use produces an empty statement. Parenthesize all five and drop the
semicolon.
22. **The state and iterator bit macros mix forms.** `AKGL_ITERATOR_OP_UPDATE`
(`iterator.h:15`) is written `1` while its 31 siblings are `1 << n`, and none
of the `1 << n` values in `iterator.h` or `actor.h:15-49` are parenthesized.
The hand-maintained trailing bit-pattern comments in `actor.h:34-49` are also
wrong for the high word — they restart at `0000 0000 0000 0001` for bit 16
rather than showing the full 32-bit value.
23. **`akgl_Frame` (`game.h:27-31`) is defined and never used** anywhere in
`src/`, `include/`, `tests/`, or `util/`.
### 5. `AKGL_ACTOR_STATE_STRING_NAMES` disagrees with `actor.h`
24. **The array bound differs between declaration and definition.**
`include/akgl/actor.h:57` declares `[AKGL_ACTOR_MAX_STATES+1]` (33);
`src/actor_state_string_names.c:6` defines `[32]`. Any consumer that trusts
the declared bound and reads index 32 reads past the object.
25. **Two entries name the wrong bit.** `actor.h` assigns bit 11 to
`AKGL_ACTOR_STATE_MOVING_IN` and bit 12 to `AKGL_ACTOR_STATE_MOVING_OUT`, but
`actor_state_string_names.c:18-19` puts `"AKGL_ACTOR_STATE_UNDEFINED_11"` and
`"AKGL_ACTOR_STATE_UNDEFINED_12"` at those indices — names for macros that do
not exist. Since `akgl_registry_init_actor_state_strings`
(`src/registry.c:79-94`) builds `AKGL_REGISTRY_ACTOR_STATE_STRINGS` from this
array and `akgl_character_load_json_state_int_from_strings`
(`src/character.c:101`) resolves character-JSON state names through it, a
character JSON can never bind a sprite to `MOVING_IN` or `MOVING_OUT`.
26. **The generation comment is stale.** `actor.h:53-55` says the file "is built
by a utility script and not kept in git, see the Makefile for
lib_src/actor_state_string_names.c". There is no Makefile (the project is
CMake), no `lib_src/`, no such script under `scripts/` or `util/`, and the
file *is* tracked in git at `src/actor_state_string_names.c`. Either restore
the generator — which would fix items 24 and 25 by construction — or delete
the comment and maintain the file by hand.
### 6. Doxygen drift
27. **Three struct doc comments in `tilemap.h` are rotated by one.**
`akgl_TilemapObject` (line 31) is documented as "Stores tileset metadata,
texture, and frame offsets" (that is `akgl_Tileset`); `akgl_TilemapLayer`
(line 46) as "Represents an object embedded in a tilemap layer" (that is
`akgl_TilemapObject`); `akgl_Tileset` (line 61) as "Stores tile, image, or
object data for one map layer" (that is `akgl_TilemapLayer`).
28. **`point` is documented as "Represents a two-dimensional point"**
(`util.h:12`) but has `x`, `y`, and `z`.
29. **Doc comments live on the definition for public functions.** The convention
is header-side documentation, but `akgl_path_relative_root` (`util.c:23`),
`akgl_path_relative_from` (`util.c:97`), the four `gamepad_handle_*`
(`controller.c:114+`), the `akgl_game_save_*` iterators and
`akgl_game_load_*` helpers (`game.c:173+`), and the tilemap helpers
(`tilemap.c:90+`) are documented only in the `.c`. This is the same set as
item 7 — resolving that resolves this.
### 7. Formatting
30. **A minority of files use a 2-column offset instead of the canonical
4-column one.** The mix of tabs and spaces across most of `src/` is *not*
disorder: it is exactly what Emacs `cc-mode` emits for the `stroustrup` style
with `indent-tabs-mode t` and `tab-width 8` — depth 1 is four spaces, depth 2
is one tab, depth 3 is a tab plus four spaces, and so on. Twelve of the
seventeen `.c` files follow that ladder cleanly.
The genuine outliers indent at 2 columns: `src/json_helpers.c` (82 lines,
except `akgl_get_json_with_default` at line 149 which is 4), `src/util.c`
(37 lines, from `akgl_rectangle_points` at line 118 onward), `src/assets.c`
(9), `src/staticstring.c` (7), `src/actor.c` (8, in `akgl_actor_add_child`
at line 272), plus `include/akgl/util.h` (7) and
`include/akgl/staticstring.h` (2).
**Resolved.** `AGENTS.md` now specifies the canonical style and the exact
`cc-mode` settings; `.dir-locals.el` applies them in Emacs; and
`scripts/reindent.sh` applies them in batch. The whole tree (32 files across
`src/`, `include/`, `tests/`, and `util/`) has been reindented and is a fixed
point of `scripts/reindent.sh --check`. Verified whitespace-only: apart from
three trailing blank lines removed at EOF in `src/heap.c`, `src/registry.c`,
and `tests/tilemap.c`, `git diff -w` over the reindent is empty, and the test
results are unchanged (13/14, `character` still the intentional failure).
`scripts/hooks/pre-commit` keeps it that way — enable with
`git config core.hooksPath scripts/hooks`.
31. **Leftover debug code ships in the library.** `src/controller.c:91-97` logs
four lines whenever `event->type == 768 && event->key.which == 11 &&
event->key.key == 13` — hardcoded decimal values for a specific keyboard ID
on a specific developer's machine, inside the per-event inner loop.
32. **Large commented-out blocks.** `src/sprite.c:115-120,157,185-198,210`;
`src/character.c:192-199,215`; `src/assets.c:18-27,50`;
`src/tilemap.c:583,596-598,640`. All are the same abandoned
`SDL_GetBasePath()` path-prefixing approach, superseded by
`akgl_path_relative`. Delete them.
33. **Unused locals.** `screenwidth`/`screenheight` (`game.c:53-54`), `curTime`
(`game.c:499`), `curTime` and `j` (`renderer.c:114,116``j` is also shadowed
by the inner loop at line 128), `target` (`character.c:59`), `result`
(`util.c:37,73`), `opflags` (`heap.c:146`, declared and cleared but never
read).
34. **`akgl_game_update`'s default flags OR the same bit twice.**
`src/game.c:496` reads
`(AKGL_ITERATOR_OP_LAYERMASK | AKGL_ITERATOR_OP_LAYERMASK)`. Given the loop
that follows walks layers and calls `updatefunc`, the intent was almost
certainly `AKGL_ITERATOR_OP_UPDATE | AKGL_ITERATOR_OP_LAYERMASK`.
35. **`akgl_draw_background` is the only public function outside the error
protocol.** `include/akgl/draw.h:14` returns `void` and reports nothing;
every other public entry point returns `akerr_ErrorContext *`. It also calls
`SDL_SetRenderDrawColor` and `SDL_RenderFillRect` without checking `renderer`
or `renderer->sdl_renderer`.
36. **`akgl_registry_init_actor` is the only registry initializer that destroys
an existing registry** before recreating it (`src/registry.c:47-49`). The
other seven leak the old `SDL_PropertiesID` on a second call. Either all of
them should do it or none should.
37. **Redundant casts obscure the code.** `(json_t *)json` where `json` is
already `json_t *`, `(akgl_Tilemap *)dest`, `(akgl_Actor *)actorobj`,
`(char *)&obj->name` on an array that already decays — roughly 180 pointer
casts across `src/`, a large majority of them no-ops, densest in
`src/tilemap.c:150-624` and `src/character.c:144-172`.
They suppress exactly the conversion warnings that would catch a real
mismatch.
38. **`struct`-qualified parameters in two definitions.**
`akgl_physics_null_move` (`src/physics.c:29`) and `akgl_physics_arcade_move`
(`src/physics.c:80`) are defined with `struct akgl_PhysicsBackend *self`
while the header and the other eight physics functions use the typedef.
39. **`text.c:19` validates the wrong argument.**
`FAIL_ZERO_RETURN(errctx, name, AKERR_NULLPOINTER, "Null filepath")` checks
`name` a second time; `filepath` is never checked and is passed straight to
`TTF_OpenFont`. Copy-paste of line 18.
40. **`akgl_path_relative` and `akgl_path_relative_from` disagree on the output
parameter.** `akgl_path_relative` and `akgl_path_relative_root` take
`akgl_String *dst`; `akgl_path_relative_from` takes `akgl_String **dst`
(`src/util.c:105`). The `**` form matches the rest of the library
(`akgl_get_json_string_value`, `akgl_get_property`, `akgl_heap_next_string`),
which allocate when `*dest` is NULL. Related: **Defects → Known and still
open #4**, which covers the fact that `akgl_path_relative_from` never writes
`*dst` at all.
41. **`dst` vs `dest` for output parameters.** `akgl_string_copy` and the
`akgl_path_relative*` family use `dst`; everything else uses `dest`.
## Coverage status ## Coverage status
Generated with: Generated with:
@@ -195,6 +559,33 @@ Each was found by a test written to assert correct behavior.
11. **`akgl_controller_pushmap` and `akgl_controller_default` accept negative map 11. **`akgl_controller_pushmap` and `akgl_controller_default` accept negative map
ids.** Both check `controlmapid >= AKGL_MAX_CONTROL_MAPS` but not ids.** Both check `controlmapid >= AKGL_MAX_CONTROL_MAPS` but not
`controlmapid < 0`, so a negative id indexes before `GAME_ControlMaps`. `controlmapid < 0`, so a negative id indexes before `GAME_ControlMaps`.
12. **A failed controller-DB fetch silently destroys the tracked fallback.**
`include/akgl/SDL_GameControllerDB.h` is committed deliberately so the
library still builds if upstream disappears. But `mkcontrollermappings.sh`
has no `set -e` and never checks `curl`'s exit status: on a failed fetch it
writes `mappings.txt` empty, then overwrites the good tracked header with
`AKGL_SDL_GAMECONTROLLER_DB_LEN 0` and an empty initializer — and exits 0.
Verified by pointing the script at an unresolvable host.
Two compounding problems:
- `add_custom_command` at `CMakeLists.txt:89-93` declares
`OUTPUT include/akgl/SDL_GameControllerDB.h` as a *relative* path, which
CMake resolves against the binary directory, while the script writes to
the source directory. The declared output never appears, so the command is
permanently out of date and re-runs on every build — making every build
depend on network access and leaving the file dirty in the working tree
each time.
- `const char *SDL_GAMECONTROLLER_DB[] = {\n};` is an empty initializer,
which is a constraint violation in ISO C and compiles only as a GCC
extension.
Fix: have the script fetch to a temporary file, check `curl`'s status and a
plausible minimum line count, and leave the existing header untouched on
failure (exiting non-zero). Separately, make regeneration explicit — a
dedicated `controllerdb` target, or an `OUTPUT` that matches where the
script actually writes — so an ordinary build neither needs the network nor
dirties the tree.
## Build notes ## Build notes