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