libakgl/TODO.md

# 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.

    **Resolved** alongside the text measurement work; `tests/text.c` asserts a
    NULL filepath reports `AKERR_NULLPOINTER`.

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

Generated with:

```sh
cmake -S . -B build-coverage -DAKGL_COVERAGE=ON -DCMAKE_BUILD_TYPE=Debug
cmake --build build-coverage --parallel
ctest --test-dir build-coverage --output-on-failure
```

Reports land in `build-coverage/coverage/` (`index.html`, `coverage.xml`).

**Line coverage 72.2%, function coverage 78.6%** (1534/2125 lines), up from a
39.6% / 44.3% baseline. `character` is the one intentionally failing suite;
everything else passes.

| File | Lines | Functions |
|---|---|---|
| `src/actor.c` | 205/258 (80%) | 16/18 |
| `src/assets.c` | 0/21 (0%) | 0/1 |
| `src/character.c` | 104/118 (88%) | 6/7 |
| `src/controller.c` | 220/243 (90%) | 9/9 |
| `src/draw.c` | 0/13 (0%) | 0/1 |
| `src/game.c` | 124/230 (54%) | 10/15 |
| `src/heap.c` | 116/116 (100%) | 12/12 |
| `src/json_helpers.c` | 111/111 (100%) | 11/11 |
| `src/physics.c` | 140/140 (100%) | 10/10 |
| `src/registry.c` | 76/102 (74%) | 11/12 |
| `src/renderer.c` | 7/70 (10%) | 1/7 |
| `src/sprite.c` | 93/101 (92%) | 5/5 |
| `src/staticstring.c` | 16/17 (94%) | 2/2 |
| `src/text.c` | 0/28 (0%) | 0/2 |
| `src/tilemap.c` | 201/426 (47%) | 10/20 |
| `src/util.c` | 121/131 (92%) | 7/8 |

Branch coverage reads 18.6% and should not be used as a target. The akerror
control-flow macros (`ATTEMPT`/`CATCH`/`PROCESS`/`FINISH`, `FAIL_*_RETURN`)
expand into large branch trees per call site, most of them unreachable in normal
operation — `src/game.c` reports over 1700 branches across 230 lines. Track line
and function coverage; treat branch coverage as a relative signal within a file.

### Suites

Every suite is registered through the `AKGL_TEST_SUITES` list in
`CMakeLists.txt`, which drives target creation, CTest registration, the
`WORKING_DIRECTORY`/`TIMEOUT` properties, the link line, and the
`FIXTURES_REQUIRED akgl_coverage` list together. Adding `tests/<name>.c` and the
name to that list is all a new suite needs; it can no longer be accidentally
left out of the coverage fixture.

Shared assertion helpers are in `tests/testutil.h`: `TEST_ASSERT`,
`TEST_ASSERT_FEQ`, `TEST_EXPECT_STATUS`, `TEST_EXPECT_OK`, `TEST_EXPECT_ANY_ERROR`,
and `TEST_ASSERT_FLAG`. All except the last expand to a `break` on failure, so
they belong directly inside an `ATTEMPT` block, not inside a loop nested in one.

Done:

- `tests/physics.c` — both backends, the factory, and the full simulation loop
  including thrust clamping, drag, layer masking, parent/child positioning, and
  logic-interrupt handling. 100%.
- `tests/heap.c` — pool exhaustion for all five pools, refcount clamping,
  recursive child release, registry cleanup on release. 100%.
- `tests/json_helpers.c` — every typed accessor, both string accessors'
  allocate-vs-reuse paths, array bounds, and `akgl_get_json_with_default`. 100%.
- `tests/controller.c` — control map push and capacity, the default binding set,
  keyboard and gamepad dispatch including cross-device rejection, the dpad
  handlers, and device add/remove against the dummy drivers. 90%.
- `tests/game.c` — version gating, the save/load roundtrip, foreign-save
  rejection, truncated-table detection, the state lock, and FPS accounting. 54%.
- `tests/actor.c` — extended with the eight control-map handlers, automatic
  facing, movement logic, the animation frame state machine, `akgl_actor_update`,
  and character/sprite binding lookups. 80%.

## Remaining work

### Needs the offscreen renderer harness

`src/renderer.c` (63 lines), `src/text.c` (28), `src/draw.c` (13),
`src/assets.c` (21), `akgl_actor_render`/`actor_visible` in `src/actor.c` (53),
and the drawing half of `src/tilemap.c` all need a live `renderer` global.

Build `tests/harness.c` / `tests/harness.h` with `akgl_test_init_headless()` and
`akgl_test_shutdown_headless()`: set the dummy video and audio drivers,
`SDL_Init()`, `akgl_heap_init()`, `akgl_registry_init()`, create a software
`SDL_CreateWindowAndRenderer`, and point the global `renderer` at it. Four
existing tests hand-roll this today (`tests/sprite.c:194`, `tests/character.c:200`,
`tests/tilemap.c:421`, `tests/charviewer.c:42`); collapse them onto the shared
harness in the same change. The new suites set the driver hints inline and need
no window, so they are unaffected.

Then:

- **`tests/renderer.c`** — `akgl_render_init2d` populating all six function
  pointers and the camera; frame start/end against a NULL `sdl_renderer`; the
  rotated `draw_texture` path including `angle != 0` with a NULL center; the
  `draw_mesh` "not implemented" stub; and `draw_world` layer ordering. Note
  `defflags` at `src/renderer.c:113` is uninitialized until the `if` body runs.
- **`tests/text.c`**, **`tests/draw.c`**, **`tests/assets.c`** — font loading
  into `AKGL_REGISTRY_FONT`, text rendering with wrap on and off, background
  drawing at zero/negative/oversized dimensions, and BGM loading into
  `AKGL_REGISTRY_MUSIC` under the dummy audio driver.
- **`tests/tilemap.c` extensions** — `akgl_tilemap_draw`, `_draw_tileset`, and
  `akgl_tilemap_load_layer_image`.

### Does not need a renderer

- **`src/tilemap.c`** — `akgl_tilemap_scale_actor` is pure math over three
  branches (`src/tilemap.c:824-830`); `akgl_get_json_properties_number`,
  `_float`, and `_double` need only a JSON snippet; `akgl_tilemap_load_physics`
  needs a fixture with the physics property block present, absent, and
  malformed. Together roughly 100 of the 225 uncovered lines.
- **`src/game.c`** — `akgl_game_init`, `akgl_game_update`, `akgl_game_lowfps`,
  and `akgl_game_updateFPS`'s frame loop need a window; revisit after the
  harness lands.
- **`src/registry.c`** — `akgl_registry_load_properties` needs a fixture with a
  `properties` object, plus the missing-file, missing-key, and wrong-value-type
  cases. Assert the loop at `src/registry.c:148-158` does not leak the string
  heap.

## Defects

### Fixed while building the suites

Each was found by a test written to assert correct behavior.

1. **`akgl_physics_simulate` dereferenced `self` before its NULL check.**
   `src/physics.c:132` read `self->gravity_time` at declaration time, three
   lines above `FAIL_ZERO_RETURN(e, self, ...)`. A NULL backend segfaulted.
2. **`akgl_game_save` never flushed or closed its stream.** `CLEANUP` and
   `PROCESS` were transposed, which put the `fclose` inside the `PROCESS`
   switch, where it only ran if an error context existed and reported success.
   An ordinary save produced an empty file.
3. **`akgl_game_save_actors` wrote name-table terminators from a single char.**
   `aksl_fwrite((void *)&nullval, 1, AKGL_ACTOR_MAX_NAME_LENGTH, fp)` emitted
   127 bytes of adjacent stack memory into the save file and produced a sentinel
   the loader could not recognize.
4. **`akgl_game_load_objectnamemap` swallowed read failures.** `CATCH` used
   directly inside `while (1)` breaks the loop, not the function, so a truncated
   or corrupt name table loaded as a successful game.
5. **`akgl_Actor_cmhf_up_on` and `_down_on` dereferenced `basechar` unguarded**,
   unlike their left and right counterparts.
6. **`akgl_actor_logic_movement` checked `actor` twice** instead of checking
   `actor->basechar` before dereferencing it.
7. **The gamepad handlers checked `appstate` three times each**, so a NULL event
   or a missing player actor was never caught and `player->state` was
   dereferenced regardless.

### Known and still open

1. **`akgl_render_and_compare` compares a texture against itself.**
   `src/util.c:228` and `src/util.c:245` both draw `t1`; `t2` is never rendered,
   so the function always passes and the image assertions in `tests/sprite.c`
   assert nothing.
2. **`akgl_tilemap_release` double-frees tileset textures.**
   `src/tilemap.c:849` destroys `dest->tilesets[i].texture` inside the *layers*
   loop; it should be `dest->layers[i].texture`. It also never NULLs the
   pointers, so a second release is a use-after-free.
3. **`akgl_registry_init` never initializes the properties registry.**
   `akgl_registry_init_properties()` is not called from `akgl_registry_init()`
   (`src/registry.c:27`), so `AKGL_REGISTRY_PROPERTIES` stays 0 unless the
   caller initializes it separately. `akgl_set_property` is then a silent no-op
   and `akgl_get_property` always returns the caller's default, which means
   `akgl_physics_init_arcade` and `akgl_render_init2d` silently ignore
   configuration. `akgl_game_init` does call it; a caller that does not use
   `akgl_game_init` does not get it.
4. **`akgl_path_relative_from` is a stub that leaks.** `src/util.c:105` claims a
   heap string, never writes `*dst`, and never releases it. 256 calls exhaust
   the string pool.
5. **`akgl_compare_sdl_surfaces` memcmps without checking geometry.**
   `src/util.c:208` compares `s1->pitch * s1->h` bytes of `s2` without verifying
   the surfaces share dimensions, pitch, or format.
6. **`akgl_string_initialize` overflows by four bytes when `init` is NULL.**
   `src/staticstring.c:17` does `memset(&obj->data, 0x00, sizeof(akgl_String))`,
   but `data` starts four bytes into the struct (after `refcount`), so the memset
   runs four bytes past the end.
7. **Savegame name lengths disagree between writer and reader.**
   `akgl_game_save_actors` writes spritesheet names at
   `AKGL_SPRITE_SHEET_MAX_FILENAME_LENGTH` (512) and character names at
   `AKGL_SPRITE_MAX_CHARACTER_NAME_LENGTH` (128), but `akgl_game_load` reads
   every table at `AKGL_ACTOR_MAX_NAME_LENGTH` (128). A save with any registered
   spritesheet cannot be read back. The current roundtrip test passes only
   because empty registries write nothing but zeroed sentinels.
8. **Heap acquire functions are asymmetric.** `akgl_heap_next_string` increments
   `refcount`; `next_actor`, `next_sprite`, `next_spritesheet`, and
   `next_character` do not. `tests/heap.c` pins the current behavior and says so;
   decide whether to make them symmetric or document the split.
9. **`tests/util.c` defines `test_akgl_collide_point_rectangle_logic` but
   `main()` never calls it.**
10. **`controller.h` declares functions that do not exist.** It declares
    `akgl_controller_handle_button_down`, `_button_up`, `_added`, and `_removed`,
    but `src/controller.c` defines them as `gamepad_handle_*`. Anything compiled
    against the header alone fails to link.
11. **`akgl_controller_pushmap` and `akgl_controller_default` accept negative map
    ids.** Both check `controlmapid >= AKGL_MAX_CONTROL_MAPS` but not
    `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.
13. **A stale build tree in the source directory breaks the coverage run.**
    `CMakeLists.txt:208` and `CMakeLists.txt:223` pass
    `--root "${CMAKE_CURRENT_SOURCE_DIR}"` to gcovr, and gcovr searches for
    `.gcda`/`.gcno` files under the root. The `--object-directory` argument on
    the line below each does *not* narrow that search: per `gcovr --help` it
    only identifies "the path between gcda files and the directory where the
    compiler was originally run". So every instrumented build tree left inside
    the source directory is folded into the report alongside the one actually
    being measured.

    With a `build-coverage/` from an earlier session still present, a freshly
    configured `-DAKGL_COVERAGE=ON` tree fails in `coverage_reset`, before any
    test runs:

        AssertionError: Got function akgl_game_lowfps on multiple lines: 45, 46.

    45 and 46 are that function's line numbers before and after an unrelated
    `#include` was added to `src/game.c`: gcovr found 18 stale `.gcno` files
    describing the old layout, merged them with the current ones, and could not
    reconcile the two. Moving `build-coverage/` aside makes the same tree pass
    18/18. Verified with gcovr 7.0.

    The `build*/` entry in `.gitignore` hides these trees from `git status`,
    which makes the state easier to get into and no easier to notice.

    Fix: pass the build tree to gcovr as an explicit search path instead of
    letting it default to `--root`, so only the tree under measurement is
    considered. Touches the two `add_test` blocks at `CMakeLists.txt:204-211`
    and `CMakeLists.txt:220-229`; nothing outside the `AKGL_COVERAGE` branch.

## Build notes

The vendored SDL satellite libraries are built into per-project subdirectories
that are not on the loader's default search path, and `LD_LIBRARY_PATH` is
searched ahead of RPATH — so a developer who has previously run `rebuild.sh` had
their installed `libakgl.so` shadow the one under test, and a developer who had
not saw every test abort before `main()` with "cannot open shared object file".
`CMakeLists.txt` now sets `BUILD_RPATH` on the library, the utility, and every
test target, and prepends the build tree to `LD_LIBRARY_PATH` for the CTest run.

## API gaps blocking akbasic

`akbasic` (the C port of the BASIC interpreter, `source.starfort.tech/andrew/akbasic`) is
being built to link into `libakgl` as a scripting engine for game authors. Its interpreter
core is complete and passes its whole acceptance corpus against a stdio text sink; the
`libakgl`-backed sink and the graphics, sound and console verbs of Commodore BASIC 7.0 are
blocked on the four gaps below.

These are filed here rather than worked around in `akbasic` because growing `libakgl` to serve
a consumer is the wanted outcome. Each entry says what the BASIC verb needs, what the
`akgl_*` entry point should look like, and what would cover it.

1. **No way to measure rendered text.** `include/akgl/text.h` exposes `akgl_text_loadfont()`
   and `akgl_text_rendertextat()`, and nothing that reports how large a string will be in a
   given font. A terminal-style text surface cannot be built on that: a cursor needs the
   advance width of one cell, and wrapping needs to know where a string crosses the right
   margin. The reference interpreter got this from SDL2_ttf's `font.SizeUTF8("A")` and derived
   its whole character grid from it.

   Wants `akerr_ErrorContext AKERR_NOIGNORE *akgl_text_measure(TTF_Font *font, char *text, int *w, int *h);`
   over `TTF_GetStringSize`, and probably a companion
   `akgl_text_measure_wrapped(font, text, wraplength, w, h)` matching the `wraplength`
   argument `akgl_text_rendertextat` already takes. Tests: a known string in a known font at a
   known size, the empty string, and a wrapped string wide enough to force two lines.

   This is the only one of the four that blocks work already designed and waiting. **`akbasic`
   cannot render any output through `libakgl` until it lands.**

   **Resolved.** `akgl_text_measure(font, text, w, h)` and
   `akgl_text_measure_wrapped(font, text, wraplength, w, h)` are in
   `include/akgl/text.h`, over `TTF_GetStringSize` and `TTF_GetStringSizeWrapped`.
   Neither needs a renderer. A negative `wraplength` is refused with
   `AKERR_OUTOFBOUNDS` rather than passed through, because SDL_ttf reads it as a
   very large unsigned width and silently stops wrapping. `tests/text.c` covers
   both against `tests/assets/akgl_test_mono.ttf`, a 10 KB monospaced ASCII
   subset added for the purpose — being monospaced, it lets the suite assert
   `width("AAAA") == 4 * width("A")` instead of hardcoding glyph metrics that
   FreeType is free to round differently. Same change fixed item 39 below:
   `akgl_text_loadfont` checked `name` twice and never checked `filepath`.

2. **No immediate-mode drawing.** `include/akgl/draw.h` declares exactly one function,
   `akgl_draw_background(int w, int h)`, and `src/draw.c` is at 0% coverage. BASIC 7.0's
   graphics verbs are all immediate-mode plotting against the current screen: `DRAW` (line and
   point), `BOX`, `CIRCLE`, `PAINT` (flood fill), `LOCATE` (set the pixel cursor), `COLOR`,
   and `SSHAPE`/`GSHAPE` (save and restore a rectangle of pixels).

   Wants an `akgl_draw_*` family taking the renderer the host already initialized --
   `akgl_draw_line`, `_rect`, `_filled_rect`, `_circle`, `_point`, `_flood_fill`,
   `_copy_region` -- in the shape of the existing `akgl_render_2d_draw_texture`. SDL3's
   `SDL_RenderLine`/`SDL_RenderRect`/`SDL_RenderFillRect` cover most of it; the circle and the
   flood fill do not exist in SDL3 and need writing. Tests belong with the offscreen renderer
   harness described under "Remaining work": render a known shape, read the target back, and
   compare against a reference surface with the existing `akgl_compare_sdl_surfaces`.

   **Resolved.** `include/akgl/draw.h` now declares `akgl_draw_point`, `_line`,
   `_rect`, `_filled_rect`, `_circle`, `_flood_fill`, `_copy_region` and
   `_paste_region`, all taking the `akgl_RenderBackend *` the host initialized,
   in the shape of `akgl_render_2d_draw_texture`. Decisions worth knowing:

   - **Color is an argument, not state.** There is no current-color global to
     get out of step with the caller's own. Each call saves and restores the
     renderer's draw color, so drawing a line does not change what the host's
     next `SDL_RenderClear` paints. `tests/draw.c` asserts that.
   - **The circle is a midpoint circle**, integer arithmetic with eight-way
     symmetry, plotted eight points per step through `SDL_RenderPoints`.
   - **The flood fill reads the target back, fills on the CPU, and blits only
     the bounding box of what changed.** It keeps a fixed
     `AKGL_DRAW_MAX_FLOOD_SPANS` (4096) stack of horizontal runs at file scope
     rather than recursing per pixel; running out reports `AKERR_OUTOFBOUNDS`
     and leaves the region partially filled, which is stated in the header. It
     is therefore not reentrant — neither is anything else that draws to a
     single `SDL_Renderer`.
   - `_copy_region` allocates when `*dest` is `NULL` and otherwise copies into
     the caller's surface, matching `akgl_get_json_string_value` and friends. A
     region that would be clipped by the target edge is refused rather than
     silently returning a smaller surface.

   `tests/draw.c` draws into a 64x64 software renderer under the dummy video
   driver and reads pixels back, so it did not need the offscreen harness. That
   harness is still wanted for `src/renderer.c`, `src/text.c` and `src/assets.c`.
   `akgl_draw_background` is untouched and still outside the error protocol
   (item 35).

3. **No audio API at all.** `SDL3_mixer` is a vendored dependency and `registry.h` declares
   `AKGL_REGISTRY_MUSIC`, but there is no `src/audio.c`, no `include/akgl/audio.h`, and no
   `akgl_*` symbol that opens a mixer, loads a chunk, or plays a note. BASIC 7.0's sound verbs
   are `SOUND` (a tone on a voice, with a duration), `PLAY` (a string of notes in a
   Commodore-specific notation), `ENVELOPE` (ADSR per voice), `FILTER`, `VOL` and `TEMPO`.

   `PLAY` and `ENVELOPE` want a synthesised voice rather than a sample, which SDL3_mixer does
   not provide directly -- the honest first step is a small tone generator feeding
   `SDL_AudioStream`, with `akgl_audio_init`, `akgl_audio_tone(voice, hz, ms)`,
   `akgl_audio_envelope(voice, a, d, s, r)` and `akgl_audio_volume(level)`. This is the largest
   of the four and the one most worth designing before writing. Tests can run under the dummy
   audio driver and assert state transitions rather than sound.

   **Resolved.** `include/akgl/audio.h` and `src/audio.c` add a three-voice tone
   generator over `SDL_AudioStream`: `akgl_audio_init`, `_shutdown`, `_tone`,
   `_stop`, `_waveform`, `_envelope`, `_volume`, `_voice_active` and `_mix`.
   It is deliberately separate from the SDL3_mixer side of the library, which
   plays audio *assets*; nothing in `audio.c` reads a file. Decisions worth
   knowing:

   - **The voice table works with no device open.** `akgl_audio_init` connects
     it to one; without that a host can still pull samples itself through
     `akgl_audio_mix`. That is not only for embedding — it is what makes the
     suite deterministic. A device pulls samples on SDL's audio thread whenever
     it likes, so a test that opened one and then asserted on voice state would
     be racing the callback. `tests/audio.c` mixes by hand and opens a device
     only in its last test.
   - **Phase is derived from the frame counter, not accumulated.** A float
     increment of `hz / 44100` is not exact, and adding it 44100 times a second
     walks a held note off pitch.
   - **A voice that was never configured is audible.** A zeroed voice has a
     sustain of 0.0, which is silence with no error to explain it, so the table
     defaults to a square wave held at full level.
   - **Three voices summing past full scale are clamped, not scaled**, so one
     voice plays at the level it was asked for rather than a third of it.
   - Everything that touches the table takes the stream lock when a device is
     open, since the callback reads it on another thread.

   Still missing for a complete BASIC sound vocabulary: `FILTER` (SDL3 has no
   filter primitive; this would need writing), `TEMPO` and the `PLAY` note-string
   parser, both of which belong in the interpreter rather than here.

4. **No non-blocking keystroke read.** `include/akgl/controller.h` is built around SDL event
   handlers the host pumps (`akgl_controller_handle_event` and friends), which suits a game
   loop and does not suit `GET` and `GETKEY` -- those ask "is there a keystroke waiting, yes or
   no" and must not require the interpreter to own the event loop. Goal 3 of `akbasic` forbids
   it owning one.

   Wants `akerr_ErrorContext AKERR_NOIGNORE *akgl_controller_poll_key(int *keycode, bool *available);`
   reading a small ring buffer that `akgl_controller_handle_event` already fills, so the host
   keeps pumping events and the interpreter drains characters at its own pace. Tests: push
   synthetic `SDL_EVENT_KEY_DOWN` events through the existing handler and drain them, plus the
   empty-buffer and overflow cases.

   **Resolved.** `akgl_controller_poll_key(int *keycode, bool *available)` and
   `akgl_controller_flush_keys(void)` are in `include/akgl/controller.h`, over a
   fixed `AKGL_CONTROLLER_KEY_BUFFER` (32) ring in `src/controller.c`.
   `akgl_controller_handle_event` records every `SDL_EVENT_KEY_DOWN` *before* it
   scans the control maps, so a key bound to an actor still reaches a polling
   caller — the scan returns as soon as a binding claims the event, and doing it
   afterwards would have lost exactly the keys a game also acts on. An empty
   buffer is success with `available` false, not an error. A full buffer drops
   the newest key rather than overwriting the oldest, matching the Commodore
   keyboard buffer and keeping what was typed first. `tests/controller.c` covers
   drain order, the release-is-not-a-keystroke case, a key shared with a control
   map, flush, both NULL arguments, and overflow plus reuse afterwards.

## Carried over

1. **Make character-to-sprite state bindings release their references symmetrically.**
   `akgl_character_sprite_add()` increments the sprite refcount for every state-map
   binding, but no corresponding removal API exists. Replacing a state binding
   also leaves the previous sprite's refcount incremented. When
   `akgl_heap_release_character()` drops the character refcount to zero, it clears
   the character registry entry and zeroes the structure without enumerating
   `state_sprites`, decrementing the bound sprites, or destroying the SDL property
   map. Implement binding removal/replacement so each removed binding releases
   exactly one sprite reference. On final character release, enumerate every
   remaining binding (the existing `akgl_character_state_sprites_iterate()` release
   path may be reusable), release each reference, destroy `state_sprites`, and then
   clear the character. Add tests for removal, replacement, duplicate sprite
   bindings across multiple states, and final character release.

   `akgl_character_state_sprites_iterate` is still uncovered and is the natural
   place to start; `tests/actor.c` now covers `akgl_character_sprite_get` and the
   composite-state binding behavior it depends on.