Files
libakgl/TODO.md
Andrew Kesterson 772f960865 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>
2026-07-30 18:17:58 -04:00

35 KiB

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_ACTORHEAP_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

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

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

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

  4. 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".

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

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

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

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

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

  1. *_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:52SUCCEED_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:620FAIL_RETURN inside ATTEMPT.
    • src/controller.c:286-289,306FAIL_ZERO_RETURN / FAIL_NONZERO_RETURN inside ATTEMPT; src/controller.c:383SUCCEED_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,462FAIL_NONZERO_RETURN inside ATTEMPT, skipping the fclose in CLEANUP at line 482.
  2. 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.

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

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

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

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

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

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

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

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

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

  1. 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).

  2. point is documented as "Represents a two-dimensional point" (util.h:12) but has x, y, and z.

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

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

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

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

  4. Unused locals. screenwidth/screenheight (game.c:53-54), curTime (game.c:499), curTime and j (renderer.c:114,116j 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).

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

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

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

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

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

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

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

  12. 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:

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.cakgl_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 extensionsakgl_tilemap_draw, _draw_tileset, and akgl_tilemap_load_layer_image.

Does not need a renderer

  • src/tilemap.cakgl_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.cakgl_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.cakgl_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.

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.