# Record **Outstanding work is in the issue tracker, not in this file:** This file used to carry both halves of the record -- what had been done and what was left -- and the second half is what a tracker is for. Every open item moved there. What stays here is the part a tracker has no place for: **why a decision went the way it did, what the measurement was, and which arguments turned out to be wrong.** The rule that produced this split is the one this file kept breaking: an entry describing work still to do goes stale the moment somebody does it, and stale entries are where the next defect hides. `AGENTS.md` records the last round of that -- eleven entries here described code that had already changed -- and this file admitted to three more when it was migrated. **A record of finished work cannot go stale in that way.** Issues are labelled by kind (`defect`, `performance`, `test-coverage`, `api-gap`, `packaging`, `docs`, `design-decision`, `abi-break`, `hygiene`) and by blast radius, and milestoned by what they can land in: `0.9.x` for anything that breaks no ABI, `0.10.0` for new or changed public symbols, `1.0.0` for the design work, `Unscheduled` for constraints waiting on a trigger. Four issues are epics with the rest hanging off them: the performance plan (#60), coverage (#61), actor rotation (#62), and the false header comments (#63). --- ## Test suites that could not fail Found while renaming the exported globals. Both are fixed, and this is the entry that argues hardest for not trusting a green suite. **Every suite reported success on any status whose low byte is zero.** libakerror's default unhandled-error handler ends in `exit(errctx->status)`. `exit` keeps only the low byte of what it is given, and libakgl's status band starts at `AKERR_FIRST_CONSUMER_STATUS`, which is 256. `AKGL_ERR_SDL` is therefore exactly 256, `exit(256)` is a wait status of 0, and CTest recorded a pass. **The most common failure status in a library built on SDL was the one status that could not fail a test.** **`tests/character.c` was green while running one of its four tests.** It aborted in `test_character_sprite_mgmt` with `Failed loading asset .../spritesheet.png : Parameter 'renderer' is invalid` -- the symbol collision described under "Public naming" below -- and exited 0 because that status was `AKGL_ERR_SDL`. Two independent defects had to line up, and they did, for long enough that this file recorded the suite as passing and wondered which change had fixed it. Nothing had; it had stopped running. **Fixed twice.** 0.5.0 worked around it here, with a `TEST_TRAP_UNHANDLED_ERRORS()` in `tests/testutil.h` that installed a handler collapsing any status a byte cannot carry onto 1. Once installed, no other suite changed colour, so this was not masking anything beyond `character` -- but it could have been at any time, and nothing would have said so. That entry ended "any consumer's test suites have this problem; that is worth raising upstream", and it was. **libakerror 2.0.1 fixes it at the source**: `akerr_exit()` owns the status-to-exit-code mapping and the default handler calls it, so 0 exits 0, 1 through 255 exit themselves, and anything else exits `AKERR_EXIT_STATUS_UNREPRESENTABLE` (125) rather than a low byte that is either a lie or a claim of success. There is no wider `exit()` to reach for -- `_exit()`, `_Exit()`, `quick_exit()` and the raw `exit_group` syscall all truncate the same way, and even `waitid()` reports the truncated value. The workaround is gone as of 0.6.0, along with its 21 call sites. Verified by putting the original failure back: a `FAIL_BREAK(AKGL_ERR_SDL)` in `tests/character.c`'s `main` now exits 125 and CTest records a failure, where it exited 0 and passed before. ## Internal consistency A sweep of `src/` and `include/` found 41 consistency and convention problems. All are resolved except the two now filed as #4 and #5. What follows is the part worth keeping: what changed, and which of them were not cosmetic after all. ### Public naming, resolved in 0.5.0 An ABI break, carrying the soname to `libakgl.so.0.5`. What a consumer had to rename: | Was | Is | |---|---| | `_ASSETS_H_`, `_CONTROLLER_H_`, `_DRAW_H_`, `_ERROR_H_`, `_JSON_HELPERS_H_`, `_PHYSICS_H_`, `_REGISTRY_H_`, `_RENDERER_H_`, `_TEXT_H_`, `_TILEMAP_H_`, `_UTIL_H_`, `_STRING_H_` | `_AKGL__H_` | | `akgl_Actor_cmhf_*` (8 functions) | `akgl_actor_cmhf_*` | | `akgl_game_updateFPS` | `akgl_game_update_fps` | | `akgl_render_init2d`, `akgl_render_bind2d` | `akgl_render_2d_init`, `akgl_render_2d_bind` | | `akgl_sprite_sheet_coords_for_frame` | `akgl_spritesheet_coords_for_frame` | | `point`, `RectanglePoints` | `akgl_Point`, `akgl_RectanglePoints` | | `window`, `bgm`, `game`, `gamemap`, `renderer`, `physics`, `camera` | the same, `akgl_`-prefixed | | `_akgl_renderer`, `_akgl_physics`, `_akgl_camera`, `_akgl_gamemap` | `akgl_default_renderer`, `akgl_default_physics`, `akgl_default_camera`, `akgl_default_gamemap` | | `HEAP_ACTOR`, `HEAP_SPRITE`, `HEAP_SPRITESHEET`, `HEAP_CHARACTER`, `HEAP_STRING` | `akgl_heap_actors`, `akgl_heap_sprites`, `akgl_heap_spritesheets`, `akgl_heap_characters`, `akgl_heap_strings` | | `GAME_ControlMaps` | `akgl_controlmaps` | | `AKGL_SPRITE_MAX_CHARACTER_NAME_LENGTH` | `AKGL_CHARACTER_MAX_NAME_LENGTH` | | `AKGL_TIME_ONESEC_MS` | `AKGL_TIME_ONEMS_NS` | `include/akgl/staticstring.h` also stopped guarding with `_STRING_H_` -- a name several libc implementations use for their own `` -- and its `#include "string.h"` is now `#include `. **The renames were done by renaming each declaration and letting the compiler find every use, rather than by pattern substitution.** That distinction matters for `renderer`, `physics` and `camera`, which are also parameter and struct-member names: a `sed` would have rewritten `map->physics` and every `akgl_RenderBackend *renderer` parameter, and nothing would have complained. **One rename had a live bug behind it.** `akgl_game_state_lock` counted its retry loop against a constant named "one second in milliseconds" that held `1000000`, so it retried 10,000 times at 100 ms and blocked for roughly sixteen minutes before reporting failure. The budget is now `AKGL_GAME_STATE_LOCK_BUDGET_MS` (1000), with the cadence named separately as `AKGL_GAME_STATE_LOCK_RETRY_MS`. `tests/game.c` carries `test_game_state_lock_budget`, which holds the mutex from a second thread and asserts the call gives up between half a second and five. The old code fails it by timing the suite out; a build that gave up without waiting at all fails the lower bound. **Nothing exercised the contended path before** -- the existing lock test only ever took an uncontended mutex, which never reaches the retry loop. **Renaming the exported globals was not cosmetic, and the proof was sitting in the test suite.** `renderer` was exported from the shared library, and `tests/character.c` defined an `SDL_Renderer *renderer` of its own. Both had external linkage and the same spelling, so the executable's definition preempted the library's: `akgl_sprite_load_json` read an `SDL_Renderer *` through an `akgl_RenderBackend *`, every texture load in that suite failed with `Parameter 'renderer' is invalid`, and the suite still reported success. **A consuming game with a variable called `renderer` would have hit the same thing with no test to notice.** ### Header/implementation surface drift, resolved in 0.5.0 **Nineteen non-static functions were defined in `src/` but declared in no header.** Each is now declared or `static`. The controller handlers were the interesting case: they were defined as `gamepad_handle_*` while `controller.h` declared the `akgl_controller_handle_*` names it never defined -- four symbols *mentioned* in prose, which is not the same as being declared. **This is enforced now.** `scripts/check_api_surface.sh` reads the built library's dynamic symbol table, strips comments out of every public header, and fails when an exported `akgl_*` symbol is declared nowhere. It runs as the `api_surface` CTest test. **Stripping comments is the point**, and is exactly how those four went unnoticed. **Headers relied on their includers for types.** `iterator.h` used `uint32_t` without ``; `json_helpers.h` used `json_t` without ``; `util.h` used `SDL_FRect` and `bool` without any SDL include. Each compiled only because of `.c`-file include ordering. Resolved, and enforced rather than merely fixed: `AKGL_PUBLIC_HEADERS` in `CMakeLists.txt` is the single list behind both `install()` and a generated translation unit per header -- each including exactly that header and nothing before it -- linked into the `headers` suite. **A header that ships is a header that is checked.** Writing that check found a case the hand-written list had missed: `registry.h` uses `SDL_PropertiesID` in eight declarations and included no SDL header at all. That is the argument for generating the check off the install list rather than hand-listing the headers somebody thought were at risk. **Object-pool size macros were defined twice and the override hook was dead.** They live once, in `heap.h`, inside the `#ifndef` guards that were always supposed to make them overridable. `tests/header_pool_override.c` is the regression test: it defines its own ceilings, includes `heap.h`, and `#error`s if the guard did not fire. **The assertion is the compile.** **Empty parameter lists.** `akgl_game_init`, `akgl_game_update_fps`, `akgl_heap_init`, `akgl_heap_init_actor` and the eight `akgl_registry_init*` functions declare and define `(void)`. Before C23 `()` means "unspecified arguments" and suppresses argument checking, **so these were the entry points a caller could pass anything to.** Also resolved: `akgl_game_init_screen` declared and never defined; static helpers using three naming styles; six parameter-name disagreements between declaration and definition (`akgl_get_json_with_default` was the interesting one -- it took the incoming context as `err` and named its *own* context `e`, and renaming it was what surfaced that the two had been swapped rather than merely misspelled); and `AKERR_NOIGNORE` applied inconsistently at definition sites. ### Error-handling pattern, resolved in 0.5.0 **`*_RETURN` macros were used inside `ATTEMPT` blocks, which skips `CLEANUP`.** Ten sites, found by scanning rather than from the list -- which named six and missed the four in `akgl_collide_rectangles`. The one that mattered was the success path of `akgl_get_json_tilemap_property`, which leaked two of the string pool's 256 entries on every lookup that *found* what it was asked for. A map load does that several times per layer. **Two needed more than swapping the macro.** In `akgl_get_json_tilemap_property` a plain `break` would have fallen through to the "property not found" `FAIL_RETURN` after `FINISH`, reporting a miss for something found, so the success path sets a flag. In `akgl_collide_rectangles` the eight early exits were followed by `*collide = false;`, which would have overwritten the hit that broke out of the block; each corner test writes the flag itself, so that line is gone rather than moved. **`scripts/check_error_protocol.py` keeps this closed**, as the `error_protocol` test. It also enforces the other rule with a silent failure mode -- no `return` out of a `HANDLE` block. Also resolved: NULL-check discipline varying by function (the eight typed JSON accessors that validated their container and then wrote through `dest` unconditionally; the null physics backend; `akgl_render_2d_frame_start`, `_frame_end` and `_shutdown`, which `tests/renderer.c` now calls with `NULL` and which segfaulted before), and the `errctx`/`e` split -- all 45 remaining sites are `errctx`, in its own commit because it is a rename and nothing else, with `e` keeping its meaning as an incoming context being inspected. ### Types and macros, resolved in 0.5.0 **`AKGL_COLLIDE_RECTANGLES` had unbalanced parentheses.** Three opens against two closes meant any expansion was a syntax error. It had no callers and duplicated `akgl_collide_rectangles`, so it was deleted. **Bitmask macros were unparenthesized.** All five are fully parenthesized, and `AKGL_BITMASK_CLEAR` no longer carries a semicolon inside its body. `tests/bitmasks.c` covers the composition cases, and writing them turned up something worth recording: **the obvious test does not catch this.** For a bit that is *set*, `!AKGL_BITMASK_HAS(mask, bit)` misparses to `!(mask & bit) == bit`, which is `0 == bit` -- false, the same answer the correct parse gives. It only diverges for a bit that is *not* set and whose value is not 1: `!(0)` is 1, and `1 == 64` is false where the answer should be true. The suite uses that shape, and fails against the old macros. Also resolved: `float`/`double` spelled raw where `types.h` defines `float32_t` and `float64_t`; the state and iterator bit macros mixing forms; and `akgl_Frame`, defined and never used, deleted. The bit-pattern comments in `actor.h` were not wrong so much as unlabelled: each shows the pattern *within its own 16-bit half*, which is why bit 16 looks like it restarts at bit 0. The section headings say so now. Rendering the full 32-bit value instead would push those lines past the 100-column fill. ### `AKGL_ACTOR_STATE_STRING_NAMES`, resolved in 0.5.0 **The array bound differed between declaration and definition.** The header declared `[AKGL_ACTOR_MAX_STATES+1]` (33) and the definition was a literal `[32]`, so a consumer trusting the declared bound read past the object. **Two entries named the wrong bit.** Indices 11 and 12 said `AKGL_ACTOR_STATE_UNDEFINED_11` and `_12` where `actor.h` has `MOVING_IN` and `MOVING_OUT`, so **no character JSON could ever bind a sprite to either state** -- the name it would have to write was not in the registry. `tests/registry.c` now walks the whole table: every entry non-`NULL`, every entry resolving to its own bit, no two entries sharing a name (a duplicate silently overwrites and makes one bit unreachable), and `MOVING_IN`/`MOVING_OUT` named explicitly so a regression reads as what it is. The generation comment was stale -- there is no generator, no Makefile and no `lib_src/`. The file's own header says it is maintained by hand and states the two invariants that keep breaking. ### Formatting and hygiene, resolved in 0.5.0 **Leftover debug code.** Four `SDL_Log` lines in `src/controller.c` guarded by `event->type == 768 && event->key.which == 11 && event->key.key == 13` -- decimal literals for one keyboard on one developer's machine, inside the per-event inner loop. **`akgl_game_update`'s default flags OR-ed the same bit twice.** It reads `AKGL_ITERATOR_OP_UPDATE | AKGL_ITERATOR_OP_LAYERMASK` now. This was a statement of intent rather than a behaviour change, and the reason is worth knowing: **nothing in that loop read either bit**, which was the per-layer update sweep defect fixed later. **`akgl_draw_background` was the only public function outside the error protocol.** It takes an `akgl_RenderBackend *` like every other entry point in `draw.h`, returns an error context, checks the backend and its `sdl_renderer`, and restores the draw colour it found. It was recorded as needing the offscreen renderer harness. **It did not, and never did** -- what it needed was to stop reading the global. **`akgl_registry_init_actor` was the only initializer that destroyed the registry it was replacing.** All eight go through one `registry_create()` helper now, so the other seven stop leaking an `SDL_PropertiesID` per call after the first -- which a game that resets between levels makes on every level. Fixed alongside it, because it is the same function: `akgl_registry_init()` never called `akgl_registry_init_properties()`, so `AKGL_REGISTRY_PROPERTIES` stayed 0 for any caller that did not also go through `akgl_game_init`, making `akgl_set_property` a silent no-op and `akgl_get_property` always return the caller's default -- so `akgl_physics_init_arcade` and `akgl_render_2d_init` quietly ignored their configuration. Also resolved: five abandoned `SDL_GetBasePath()` blocks; a dozen unused locals; `struct`-qualified parameters in two physics definitions; `text.c` validating the wrong argument; and `dst` vs `dest` for output parameters. ## Coverage **Line coverage 83.9%, function coverage 91.2%** (2433/2901 lines, 176/193 functions), up from 79.6% / 87.2% before the 0.5.0 defect work and from a 39.6% / 44.3% baseline. Snapshot at 0.9.0; the gaps behind it are #61. 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`). A coverage tree rebuilt after a source edit fails `coverage_reset` with `GCOV returncode was 5`; `find build-coverage -name '*.gcda' -delete` clears it. | File | Lines | Functions | |---|---|---| | `src/actor.c` | 205/258 (80%) | 16/18 | | `src/assets.c` | 0/21 (0%) | 0/1 | | `src/audio.c` | 229/248 (92%) | 22/23 | | `src/character.c` | 122/126 (97%) | 7/7 | | `src/controller.c` | 314/349 (90%) | 17/17 | | `src/draw.c` | 281/286 (98%) | 12/12 | | `src/error.c` | 9/9 (100%) | 1/1 | | `src/game.c` | 142/243 (58%) | 13/18 | | `src/heap.c` | 116/116 (100%) | 12/12 | | `src/json_helpers.c` | 123/123 (100%) | 11/11 | | `src/physics.c` | 144/144 (100%) | 10/10 | | `src/registry.c` | 83/111 (75%) | 12/13 | | `src/renderer.c` | 61/83 (74%) | 8/8 | | `src/sprite.c` | 102/110 (93%) | 5/5 | | `src/staticstring.c` | 17/18 (94%) | 2/2 | | `src/text.c` | 79/80 (99%) | 7/7 | | `src/tilemap.c` | 276/444 (62%) | 13/20 | | `src/util.c` | 128/130 (98%) | 7/7 | | `src/version.c` | 2/2 (100%) | 1/1 | `src/tilemap.c` moved the most, 47% to 62%, because the bounds and leak work needed the loaders driven rather than merely called. **Branch coverage reads 24.7% and should not be used as a target.** The akerror control-flow macros 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. ### Mutation testing **Macros expand at call sites, so mutation testing is the only thing that checks them.** `scripts/mutation_test.py`, run when touching control flow, refcounting, stack traces or handlers. Sampled runs (`--max-mutants 8` to 10 each, so these are samples rather than exhaustive scores): | File | Score | Surviving mutants | |---|---|---| | `src/draw.c` | 90% | Deleting the `FAIL_ZERO_BREAK` on `SDL_CreateTextureFromSurface` in `akgl_draw_flood_fill` | | `src/audio.c` | 75% | Deleting `SUCCEED_RETURN` from the static `check_voice`; deleting `spec.freq` before opening a device | | `src/text.c` | 50% | Three in `akgl_text_rendertextat`, which had no test yet, plus one `SUCCEED_RETURN` deletion | | `src/controller.c` | 40% | Three `SDL_Log` deletions; `keybuffer_head`'s initialiser; `count > 0` in `keybuffer_attach_text`; the `mod` a text-only entry is given | **What the runs found, and what they did not.** `src/audio.c` scored 50% first and named two real gaps, both now tested: nothing asserted that an *unconfigured* voice is audible (the whole reason the table defaults to a square wave at full level rather than a zeroed struct), and no envelope test used a non-zero attack *and* decay together, so the decay measuring from the wrong origin survived. `src/draw.c` scored 70% first and named one: the circle was only checked at its four axis points, which a mis-signed octant reflection survives, so it now checks that every plotted pixel has a mirror in the other three quadrants. In `src/controller.c`, one real gap -- nothing checked that a text-only ring entry reports no modifiers. The rest are **equivalent mutants rather than misses**, and knowing which is which is the point: a ring buffer whose head starts at 1 behaves identically, `count > 0` cannot be false where it is checked, deleting the `break` on a `switch`'s last case changes nothing, and the mixer's `v <=` bound reads one voice past a zeroed table, which is undefined rather than observable. `SDL_Log` deletions are unobservable by construction. Deleting a `SUCCEED_RETURN` leaves a non-void function falling off its end, which is undefined rather than observably wrong. The three `src/text.c` survivors are gone: it is tested against a software renderer, and deleting any of its three backend checks fails the suite. **That was not free** -- the first draft of the test used a made-up `SDL_Renderer` pointer, which SDL refuses on its own, so the deleted-check mutant survived it. A live renderer was what made the check observable. ## Defects ### Fixed while building the suites Each was found by a test written to assert correct behaviour. 1. **`akgl_physics_simulate` dereferenced `self` before its NULL check.** `src/physics.c:132` read `self->gravity_time` at declaration time, three lines above the `FAIL_ZERO_RETURN`. 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. ### Fixed in 0.5.0 and later - **`akgl_render_and_compare` compared a texture against itself.** Both passes drew `t1`, so it always reported a match and **every image assertion built on it asserted nothing.** - **`akgl_tilemap_release` double-freed tileset textures.** The layers loop tested `layers[i].texture` and destroyed `tilesets[i].texture`, so every tileset texture was freed twice on a single release and no image layer's texture was freed at all. Each pointer is cleared as it goes now, which also makes a second release safe rather than a use-after-free. - **`akgl_path_relative_from` was a stub that leaked.** It claimed a heap string, never wrote its output, and never released it, so 256 calls exhausted the string pool. Declared in no header, called from nowhere, duplicating what `akgl_path_relative` already does. **Fixing an unfinished function nobody can reach is worse than deleting it.** - **`akgl_compare_sdl_surfaces` memcmp'd without checking geometry.** `tests/util.c` compares a 32x32 surface against an 8x8 one in both directions, which used to read 4 KiB past the end of the smaller. - **`akgl_string_initialize` overflowed by four bytes when `init` is NULL.** The four bytes it ran past the end of the object landed on the *next* pool slot's `refcount`, which is the field the allocator reads to decide whether a slot is free -- **so the overrun could hand a live string out twice.** Fixed alongside, same class: `akgl_string_copy` accepted a `count` above `AKGL_MAX_STRING_LENGTH`, which read past the end of one pool slot and wrote past the end of another. The header documented that as behaviour. - **Savegame name lengths disagreed between writer and reader.** The reader used `AKGL_ACTOR_MAX_NAME_LENGTH` for all four tables; only the spritesheet table was wrong, and that was enough. **The failure was worse than "cannot be read back", which is what made the test interesting**: the tables carry no length prefix and end at a zeroed sentinel, so a reader stepping the wrong width does not run off anything -- it finds a run of zeros somewhere inside an entry, stops early, and reports success with silently wrong maps. A test that only asserted the load succeeded passed against the broken reader. So `akgl_game_load` checks the stream is at EOF once the four tables are read, which is what turns a width disagreement into `AKERR_IO` instead of a corruption. **That EOF check has to move when the objects themselves start being written; there is a comment at the site saying so.** A first pass introduced four `AKGL_GAME_SAVE_*_NAME_WIDTH` aliases, on the reasoning that the on-disk format's widths are a separate concern from the object model's. **They were removed**: each expanded to exactly one existing constant and had exactly one use per side, so they were indirection with no second consumer -- and the divergence they anticipated cannot happen quietly anyway, since raising one of those lengths is an ABI change and `akgl_game_load` refuses a save whose `libversion` does not match before it reads a single table. - **`akgl_controller_pushmap` and `akgl_controller_default` accepted negative map ids.** Both check the lower bound now. - **A failed controller-DB fetch silently destroyed the tracked fallback.** `mkcontrollermappings.sh` runs under `set -euo pipefail`, fetches into a temporary directory, and moves the result into place only after checking curl's exit status (with `--fail`, so an HTTP error is a status rather than an error page in the body), a plausible minimum mapping count, and that no mapping carries a quote or backslash that would break the C string literal it becomes. **Verified against all five failure modes**: an unresolvable host, a 404, a truncated response, an empty-but-successful response -- the original failure exactly -- and a response containing a quote. Each refuses, and the header's checksum is unchanged after every one. The build no longer runs it. The `add_custom_command` declared `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, so the declared output never appeared, the command was permanently out of date, and **every build re-ran it** -- needing network access and leaving the tree dirty. Regenerating against the real upstream produced byte-identical mappings -- 2255 entries, no content change -- so the rewrite is faithful. - **A stale build tree in the source directory broke the coverage run.** gcovr searches for `.gcda`/`.gcno` under its search paths, and with none given it searches `--root` -- the source directory, which is where developers keep their build trees. `--object-directory` does not narrow that; per gcovr's own help it only identifies "the path between gcda files and the directory where the compiler was originally run". **Verified by reproducing it**: two instrumented trees built inside the source directory with a source edit between them fail the old invocation with `Got function write_exact on multiple lines: 46, 48` and exit 64; the new one exits 0. - **22 public symbols shipped without a version or soname bump**, contradicting this repository's own rule that for 0.x libraries the soname carries `MAJOR.MINOR` deliberately. Two consequences, and the first is the one that bites: a binary compiled against the new headers links happily against a `libakgl.so.0.1` built from the old tree, because the soname says they are the same ABI, and fails at symbol resolution rather than at configure time. And `AKGL_VERSION_AT_LEAST(0, 1, 0)` was true for both trees, **so a consumer could not feature-test for the new API at all** -- `akbasic` pinned the requirement by submodule commit instead, and said so in its README, which is not a thing a released library should make anybody do. **Worth remembering the next time a handful of symbols looks too small to bump for.** ### Found while rewriting the Doxygen comments Each came out of reading an implementation against the contract its header claimed. They are recorded inline as `@note` or `@warning` on the function concerned. - **`akgl_sprite_load_json` did not bound the `frames` array.** The count is bounded against `AKGL_SPRITE_MAX_FRAMES` before anything is written, and each element is read into an `int` and narrowed deliberately rather than written through a `uint32_t *` cast of a `uint8_t *`. A frame number that does not fit a `uint8_t` is refused rather than truncated into an index that names a different tile. `tests/sprite.c` covers exactly the maximum, one past it, and the wide frame number; **against the old code the middle case loads happily.** - **Two more unbounded array loads in the tilemap loader**, both fixed with the bound at the top of the loop body. The object one is the reachable half -- 128 objects is not a large object layer and `akgl_TilemapObject` is not small. - **`akgl_get_json_with_default` defaulted on a status the array accessors never raise.** Worth knowing for the next test written against this function: **it returns the context it was given** when it does not handle the status, so `TEST_EXPECT_OK` -- which releases whatever the statement returns -- will double-release it against a `CLEANUP` block that also releases it, and a double-released context corrupts the failure instead of reporting it. The first draft of that test did exactly that and passed against the unfixed library. - **`akgl_character_sprite_add` leaked a sprite reference when a state is remapped.** The new reference is taken *before* the write and given back if the write fails, so there is no window where a sprite is bound with nothing behind it. - **`akgl_heap_release_character` abandoned the whole state-to-sprite map.** `src/heap.c:150` zeroed the character without walking `state_sprites`, so every sprite reference the character took was lost and the `SDL_PropertiesID` holding the map was never destroyed. **The test was already written**: `tests/character.c` had asserted this contract all along -- "character did not reduce reference count of its child sprites when released" -- and had never once run, for the two reasons under "Test suites that could not fail". - **`akgl_actor_render` computed a sprite's drawn height from its width.** Every actor was drawn square, so a non-square sprite was stretched or squashed -- invisible in the fixtures because they are all square. `tests/actor.c` renders a 48x24 sprite through a backend whose `draw_texture` records the rectangle it is handed rather than drawing it. **That recording backend is also the first coverage `akgl_actor_render` has had at all.** - **`akgl_text_rendertextat` refused the empty string while `akgl_text_measure` accepted it.** The check sits after the font, text and backend guards rather than before them, so drawing nothing still refuses the things drawing something refuses -- a caller does not get a different contract for an empty string. - **`character_load_json_state_int_from_strings` checked the same argument twice.** **Not asserted, deliberately.** The function is `static` and has one call site, which passes `&stateval`, so a `NULL` `dest` is unreachable from the public API and the guard cannot fire. Testing it would mean giving the function external linkage purely to reach a defensive check. ## Performance The first measured baseline is in `PERFORMANCE.md`, produced by `tests/perf.c` and `tests/perf_render.c` (`ctest --test-dir build -L perf`). Both suites hold every measurement to a budget set at roughly ten times the recorded baseline, so an algorithmic regression fails the suite rather than being discovered by a player. **Read `PERFORMANCE.md` before arguing with anything below** -- every claim has a number behind it, and several of the things expected to be slow are not. The plan for the missed targets is #60. ### Defects the perf suites found - **`akgl_path_relative` leaked an error context on every root-fallback resolution.** It took its `ENOENT` branch by `return`ing from inside the `HANDLE` block, which skips the `RELEASE_ERROR` that `FINISH` ends with. One entry of `AKERR_ARRAY_ERROR` was lost per call, and the 129th call hit "Unable to pull an error context from the array!" and **exited the process**. Every tilemap load resolves several paths this way, so a game that loaded fifty levels died in the loader. `tests/util.c` resolves `AKERR_MAX_ARRAY_ERROR * 2` paths through that branch; **against the old code that test does not fail, it terminates the suite.** This is the *only* `return` from inside a `HANDLE` block in `src/` -- the other candidates use `SUCCEED_RETURN`, which releases correctly. Worth a grep before anyone writes a new one, and it is now in `AGENTS.md`'s error-handling protocol, which warned about `*_RETURN` inside `ATTEMPT` but not about returning out of `HANDLE`. - **`akgl_tilemap_load` leaked five pooled strings per load**, in two parts. Three were `akgl_get_json_tilemap_property` leaking two scratch strings on every *successful* property lookup. The remaining two were each a claim with no matching release. **Finding the last two meant dumping the contents of every still-claimed slot after a cycle rather than reading the code again**; `'tilelayer'` and an assets directory named themselves immediately. Fixed alongside, same class: `akgl_tilemap_load_layer_objects` released its scratch string after reading each object's name and then kept using the slot -- and `akgl_get_json_string_value` reuses a non-`NULL` destination *without* taking another reference, so the slot was free while still live. - **Two JSON accessors turned string-pool exhaustion into a segfault.** `tests/json_helpers.c` claims every slot in the pool and asserts `AKGL_ERR_HEAP` comes back out of both accessors. **Against the old code that test segfaults rather than failing** -- and so did the new tilemap property-lookup test, which is how this was confirmed rather than merely believed. - **`akgl_game_update` segfaulted if `akgl_game_init` did not run.** `game.fps` is 0 for the first second of the process, which is under the threshold, so the unguarded `akgl_game_lowfps` call fired on frame one. Only `akgl_game_init` installed the default -- and `include/akgl/renderer.h` documents the other path on purpose: a host that owns its own window calls `akgl_render_2d_bind` instead of `akgl_render_2d_init`. **An embedder following that documentation crashed on its first frame, and `akbasic` is exactly that embedder.** - **`akgl_game_update` ran the actor update sweep once per tilemap layer.** The sweep is hoisted out of the layer loop -- updating an actor is not a per-layer operation. `tests/game.c` counts calls into a stub `updatefunc`; against the old code it reports 16. **Counting is the assertion on purpose**: the defect was invisible in the frame total because the tilemap blits are three orders of magnitude larger, so a timing test would have measured the rasterizer. ### Targets What a library like this *should* hit. The frame budget throughout is 16.67 ms (60 fps); where a target is stated per-operation it is because that is the number that survives a change of renderer. The one that governs the rest: **libakgl's own bookkeeping should never be the reason a frame is late.** Everything the library decides -- pool scans, registry lookups, state-to-sprite mapping, physics, visibility, error contexts -- should fit in 5% of a frame, leaving 95% for the pixels and the game's own logic. At 64 actors and a screenful of tiles that is a ceiling of about 800 us. | # | Target | Today | Verdict | |---|---|---|---| | 1 | Library bookkeeping under 5% of a 60 fps frame at 64 actors + 1200 tiles | ~0.3% (excluding pixel work) | **met** | | 2 | One actor's logic update under 200 ns | 68.4 ns | **met** | | 3 | One actor's render bookkeeping, excluding the blit, under 500 ns | ~250 ns, by subtraction | **met, weakly measured** | | 4 | Physics sweep under 25 ns per *live* actor, proportional to live actors rather than pool size | 19 ns per live actor, but 63.9 ns for an *empty* pool | **partly** | | 5 | Tilemap draw bookkeeping under 100 ns per tile, excluding the blit | ~25 ns | **met** | | 6 | Pool acquire under 100 ns regardless of how full the pool is | 3.9 ns empty, 250.9 ns on the last free string slot | **missed** (#21) | | 7 | Pool release proportional to the bytes actually used | 47.2 ns, a fixed 4 KiB wipe | **missed** (#21) | | 8 | Re-drawing an unchanged line of text under 1 us | 12.6 us, every frame, no cache | **missed** (#22) | | 9 | Zero texture creation or destruction per frame in steady state | one create + one destroy per line of text per frame | **missed** (#22) | | 10 | A handled, routine condition costs no more than twice the path that succeeds | 616.5 ns vs 68.4 ns -- 9x | **missed** (#23) | | 11 | 256 actors simulated, updated and made ready to draw in under 1 ms | ~22 us at 64 actors; linear, so ~90 us extrapolated | **met, untested at that size** | | 12 | Collision for 256 actors under 2 ms without the caller writing a broad phase | 54.1 us at `-DAKGL_MAX_HEAP_ACTOR=256` | **met** | | 13 | Level load under 100 ms for a map with 8 tilesets, 4 layers and 64 actors | 11.9 ms for a 2x2 map with one tileset | **unknown at that size** (#27) | | 14 | Fixed per-load overhead under 1% of a level load | 11.5% -- zeroing 26 MB of `akgl_Tilemap` | **missed** (#28) | | 15 | Static footprint under 4 MB in the default configuration | 28 MB, 94% of it one tilemap | **missed** (#28) | | 16 | No pool leaks across a load/release cycle of any asset type | tilemap is clean, asserted over 64 cycles; sprite, spritesheet and character cycles are not asserted | **met where tested** (#13) | | 17 | Pool exhaustion reports `AKGL_ERR_HEAP` and never crashes | all five pools report it | **met** | | 18 | Every benchmark held to 10x its recorded baseline, enforced in CI | done, `ctest -L perf` | **met** | Notes on the ones worth arguing about: - **6 and 7 are the same fix**, and **8 and 9 are one cache**, and **14 and 15 are the same refactor.** Each pair is one issue for that reason. - **10 is a design target, not a speed target.** The answer is not a faster error context; it is that "this character has no sprite for this state" is a question with a boolean answer. - **12 is a scope decision, not a defect, and it is done.** The structure is the incremental uniform grid, not the tree -- cells keyed on tile size, static arrays, insert/remove as actors move, wired into the `collide` slot of `akgl_PhysicsBackend`. Construct measured that design at a 96% check reduction over brute force and rejected quadtrees as costlier to maintain; Phaser's rebuild-the-RTree-every-frame approach carries a documented ~5,000-body ceiling and is the counterexample. Measured here rather than cited: **the grid beats the BSP by 2.6x on the same 64-proxy population** (44.3 ns against 116.1 ns per query), before counting that the tree rebuilds whenever a proxy moves and the grid's `move` is 11.9 ns when the cell rectangle has not changed. Over a 4x range in actor count the step grew 3.5x and the all-pairs control grew 15.6x, **which is the n2 the index exists to avoid.** **The BSP ships anyway.** A vtable with one implementation behind it has never been asked to be a vtable; `tests/partition.c` runs its whole contract against a table of both, so the seam is proven rather than asserted. It would earn its place in a world with wildly non-uniform object sizes or one larger than the grid's 128x128 cell array covers. ## Memory checking `cmake --build build --target memcheck` runs **the suites that already exist** under valgrind -- `ctest -T memcheck` with the headless drivers forced, wrapped by `scripts/memcheck.sh` so that a finding is an exit status rather than a line in a log nobody reads. **There are no memory-check test programs, and there should never be any**: `tests/benchutil.h` notices that it is running under valgrind and divides every benchmark's iteration count by two thousand, which turns the perf suites into the broadest path coverage in the tree at a cost valgrind can survive. The whole run is about thirty seconds. **The two halves fit together on purpose.** A benchmark is a program that walks one path a hundred thousand times; a leak check wants every path walked once. Same binaries, same registration, one flag apart. Third-party findings are suppressed in `scripts/valgrind.supp`, and the run forces `SDL_VIDEO_DRIVER=dummy` / `SDL_RENDER_DRIVER=software` / `SDL_AUDIO_DRIVER=dummy` so the vendor GPU stack is never loaded -- that removes thousands of unfixable findings inside `amdgpu_dri.so` **without suppressing anything at all.** Only *definite* losses and invalid accesses are counted. ### Defects the memory checker found **All six are fixed**, and the CI job that runs memcheck gates: the next one of these fails the build on the push that introduces it. Every size below is measured. 1. **Every JSON loader leaked its parsed document.** Four `json_load_file` calls in `src/` and not one `json_decref` anywhere in the library: | Loader | Site | Leaked per call | |---|---|---:| | `akgl_sprite_load_json` | `src/sprite.c:140` | ~1,500 bytes | | `akgl_character_load_json` | `src/character.c:232` | ~2,150 bytes | | `akgl_tilemap_load` | `src/tilemap.c:693` | ~9,000 bytes (2x2 fixture map) | | `akgl_registry_load_properties` | `src/registry.c:134` | not exercised by any test | Blast radius: every asset load, forever. A 128x128 map leaks on the order of a megabyte per load, and **this is the one item in this list that grew without bound.** `akgl_registry_load_properties` needed its loop moved inside the `ATTEMPT` block first: `props` is a borrowed reference into the document and was read after the block ended. **The test is the memcheck run** -- nothing in the public API can observe a jansson refcount, and inventing a hook to prove it would be testing the test. 2. **`akgl_get_property` read up to 4 KiB past the end of the property value.** It copied a fixed `AKGL_MAX_STRING_LENGTH` bytes out of whatever `SDL_GetStringProperty` returns, and what it returns is an `SDL_strdup` of the value -- four bytes for `"0.0"`. Valgrind reported an invalid read on **every call**. This had been in `registry.h` as a `@note` about the copy being "a fixed #AKGL_MAX_STRING_LENGTH bytes rather than the length" -- **filed as waste. It was not waste, it was an out-of-bounds read**, and on a value that lands at the end of a page it is a segfault in a getter. 3. **The savegame name tables read past the end of every registry key, and wrote what they found into the file.** Two consequences, and the second is the interesting one: the read can fault if the key sits at the end of a page, **and whatever it reads goes into the save file, so a savegame contained up to 500 bytes of this process's heap per registered object.** That is a file a player might send someone. All four iterators write through `write_name_field` now, and a negative-array-size typedef fails the build if any of the four widths ever outgrows the staging buffer. 4. **`akgl_controller_list_keyboards` leaked the array SDL gives it.** One `SDL_free`, in a `CLEANUP` block so a failure inside the loop cannot take the array with it. The same question was asked of every other SDL enumeration in `src/controller.c`; `SDL_GetGamepads` and `SDL_GetGamepadMapping` are freed too. 5. **A font, once loaded, was never freed and could not be.** 10,523 bytes per font. **It was a gap in the API rather than only a leak**: a game that switches fonts between scenes, or a tool like `charviewer` that reloads one while the user picks a size, had no way to give the old one back. `akgl_text_unloadfont` is a new public symbol; `akgl_text_loadfont` calls it when it replaces a live name, **but only after the new font has opened**, so a failed reload leaves the caller with the font they already had. What is still missing is a teardown that calls it -- #15. 6. **Vendored `deps/semver`'s own unit test leaks 188 bytes.** Not libakgl's code and not libakgl's to fix. **Suppressed**, which is what this list said the answer would be if it ever became noise, and gating the job made it noise. The two entries in `scripts/valgrind.supp` name the functions rather than the file, so a rewrite of those cases stops being suppressed and comes back as a finding. **They are the only entries there that hide a real leak in a program this build runs**; the alternative was a fork of a vendored dependency. ### Not defects, and why - **`tests/util.c` fixtures were reading uninitialised stack floats.** Three null-pointer tests declared `SDL_FRect` and `point` fixtures without initializing them, which is sixteen "conditional jump depends on uninitialised value" findings for a test that is not about coordinates. The library was never at fault; **a memory checker that reports noise gets ignored**, which is the only reason this was worth touching. - **"Still reachable" at exit is not counted.** SDL's global state, the hint table, the property registry and FreeType's library instance are one per process and are reclaimed by `SDL_Quit` / `TTF_Quit`. Counting them would bury the six findings above under a hundred that mean nothing. - **The GPU driver's findings are avoided rather than suppressed.** Running the suite against the real driver produces thousands of findings inside `amdgpu_dri.so`; running it headless produces none, and headless is what the suites are written for. ### Found while embedding libakgl in a consumer **Shadowing `add_test()` unconditionally broke every embedding consumer.** Fixed in the same release it was introduced in, and kept because the CMake behaviour behind it is not obvious and will catch the next person. **CMake chains command overrides exactly one level deep.** Overriding `add_test` makes the builtin available as `_add_test`; overriding it a *second* time rebinds `_add_test` to the *first* override and the builtin becomes unreachable to everybody, permanently -- there is no `__add_test`. Verified directly rather than inferred from the documentation: ```cmake function(add_test) _add_test(${ARGV}) endfunction() # _add_test -> builtin function(add_test) _add_test(${ARGV}) endfunction() # _add_test -> the first override if(COMMAND __add_test) ... endif() # absent ``` So two projects in one tree cannot both shadow it. `akbasic` shadows it first -- it has to, for `libakerror` and `libakstdlib`, which register their tests unconditionally -- and its own registrations then recursed until CMake stopped at "Maximum recursion depth of 1000 exceeded". **A consumer had no way to work around it**: once the builtin is gone it is gone for that project too. The override is guarded on being top-level again. An embedded `libakgl` leaves `add_test` alone and lets its consumer suppress what it does not want -- machinery the consumer has to have anyway. ## 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 had 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` 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 that blocked akbasic `akbasic` (the C port of the BASIC interpreter, `source.starfort.tech/andrew/akbasic`) links `libakgl` as a scripting engine for game authors. **All ten items are resolved**, and its `libakgl`-backed text sink, graphics backend, sound backend and input backend are written and tested. Decisions worth keeping, because each was a fork in the road rather than an implementation detail: - **Text measurement.** `akgl_text_measure` and `_measure_wrapped` sit over `TTF_GetStringSize`/`TTF_GetStringSizeWrapped`. 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.** The test font is monospaced on purpose: it lets the suite assert `width("AAAA") == 4 * width("A")` instead of hardcoding glyph metrics that FreeType is free to round differently. - **Drawing: colour is an argument, not state.** There is no current-colour global to get out of step with the caller's own. Each call saves and restores the renderer's draw colour, so drawing a line does not change what the host's next `SDL_RenderClear` paints. The flood fill reads the target back, fills on the CPU, and blits only the bounding box of what changed, keeping a fixed `AKGL_DRAW_MAX_FLOOD_SPANS` (4096) stack of horizontal runs at file scope rather than recursing per pixel. **It is therefore not reentrant -- neither is anything else that draws to a single `SDL_Renderer`.** - **Audio: the voice table works with no device open.** 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. Phase is derived from the frame counter rather than accumulated, because 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 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. - **The sweep steps once every 1/60 second** (`AKGL_AUDIO_SWEEP_TICK_HZ`), because that is the rate the machine this vocabulary comes from advanced its sweep at. It divides 44100 exactly, so a step boundary always lands on a whole frame -- which is what lets the suite assert the exact frame the pitch moves on rather than a tolerance. Direction comes from the two frequencies, not from the sign of the step. **A swept voice accumulates its phase** instead of deriving it: deriving assumes a constant frequency, and under a sweep it jumps the waveform at every step, which is an audible click. - **Keystrokes: the ring records every `SDL_EVENT_KEY_DOWN` *before* the control-map scan**, 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. A full buffer drops the newest key rather than overwriting the oldest, matching the Commodore keyboard buffer. **The composed text is attached to the press that is still waiting for it**, tracked by a flag rather than by "whatever entry is newest": without the flag, a press dropped by a full buffer hands its text to some older key sitting at the end of the ring -- wrong in exactly the case where things are already going badly. - **`akgl_render_2d_bind` deliberately does not touch `self->sdl_renderer`**, which is the whole point -- a host that already owns one keeps it, and a host that does not gets a backend whose entry points all report `AKERR_NULLPOINTER` instead of crashing. - **Vendored dependencies are added on both paths now**, guarded by `if(NOT TARGET ...)`, through an `akgl_add_vendored_dependency( )` macro -- a macro rather than a function because `add_subdirectory()` inside a function runs in that function's variable scope. Verified by configuring and building a scratch consumer with `CMAKE_DISABLE_FIND_PACKAGE_*` set for all seven dependencies, so any reliance on an installed copy would have failed the configure. - **`tests/headers.c` is a whole suite for one `#include` on purpose**: a second `#include` after the first proves nothing about the second, because by then the first has dragged its dependencies in. Still missing for a complete BASIC vocabulary, and filed: the oscillating `SOUND` direction and `FILTER` (#58). `TEMPO` and the `PLAY` note-string parser belong in the interpreter rather than here. ## `akgl_collide_rectangles` stays; the corner helpers did not The collision plan's last step was to **delete `akgl_rectangle_points`, `akgl_collide_point_rectangle` and `akgl_collide_rectangles`** once the shape API landed. Two of the three went, in 0.8.0, along with `akgl_Point` and `akgl_RectanglePoints`. The third stays, and this is the reasoning rather than an oversight. **Why the two went.** They were the intermediate form of an implementation that changed. `akgl_collide_rectangles` was eight corner-containment tests built on them; it is four span comparisons now and does not go through either. Nothing outside `tests/` called them, and a point-in-rectangle test is four comparisons a caller can write without a struct conversion in front of it. **Why the third stays.** - **It has callers that are correct.** `examples/sidescroller/player.c` uses it twice, for coins and for hazards. Those are overlap *questions* asked from an `updatefunc` -- "am I touching this" -- and the right answer is a `bool`. Routing them through a collision shape would mean a proxy, a broad-phase insert and a narrowphase call to compute a normal and a depth that nothing reads. - **It was fixed in this same series**, not left broken. It missed the cross case and truncated through `akgl_Point`'s `int` members; it is exact in `float32_t` now. **Deleting it two commits after fixing it is a strange thing to do to a caller.** - **A game-level overlap question is not always a physics question.** A minimap marker, a UI hit test, "is the cursor over this". None of those wants to be pushed, and none of them belongs in a collision world. ## Why libakstdlib's collections are not used `aksl_HashMap`, `aksl_List` and `aksl_TreeNode` exist and libakgl uses SDL property sets for every registry instead. **That is not an oversight** -- the registries hand `SDL_PropertiesID` values to callers and SDL owns the lifetime. Recorded so nobody re-derives it. The wrappers that were adopted, and why each was worth it: `aksl_fclose` (an unchecked close in `akgl_game_save` lost buffered data silently), `aksl_fgetc` (`fgetc(3)` spells EOF and a read error the same way), `aksl_snprintf` at two sites that were truncating under a silenced `-Wformat-truncation`, and `aksl_strncpy` at the two `src/game.c` sites the fixed-width copy sweep missed. Whether to adopt the pure-arithmetic wrappers is #19. ## Why `deps/tg` is still in the tree `deps/tg` is pinned at v0.7.9 and nothing uses it. It was evaluated alongside `deps/libccd` for the collision work and lost, for two reasons that are properties of the library rather than preferences: **it is 2D permanently** (its own documentation says the z coordinate serves no purpose but export, and no predicate reads it), and **its predicates return `bool`** -- resolving a collision needs a minimum translation vector, which `tg_geom_intersects` cannot supply. It is kept rather than dropped because there is one job it does that libccd structurally cannot: **indexed point-in-polygon against concave polygons with holes.** GJK and MPR are convex-only, always. **That is a deadline, not a permanent exemption** -- #55, with #75 as the consumer that would justify it. ## The ccd arena `src/collision_arena.c` is one static arena of `AKGL_CCD_ARENA_BYTES` (64 KB) with a bump pointer, reset at the top of every `akgl_collision_test`. **The size is a measurement, not a guess**: one GJK/EPA box pair measures 7,264 bytes. Exhaustion is reported, not crashed -- `ccdGJKPenetration` returns -2 and `akgl_collision_test` raises `AKGL_ERR_COLLISION` with the high-water mark in the message, **so the failure names its own fix.** MPR, the default on the frame path, allocates nothing at all. `akgl_ccd_arena_highwater()` is public so a game can assert its own ceiling. The threading constraint that follows from a single arena is #57.