libakgl/TODO.md

# TODO

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

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

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