Closes Defects items 20, 22, 23 and 29, and the residual of item 38.
The tilemap load leak was five pooled strings per load; the property-lookup
fix in an earlier commit took it to two, and the last two were each a claim
with no matching release -- the string every layer's `type` was read into, and
the dirname the map's relative paths resolve against. tests/tilemap.c asserts
the pool is exactly where it started after one load/release cycle and after 64.
Finding those two was a matter of 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.
Same file, same class, fixed with it: akgl_tilemap_load_layer_objects released
its scratch string after reading each object's name and then kept using the
slot, because akgl_get_json_string_value reuses a non-NULL destination without
taking another reference. The slot was free while still live, so any other
claim could have been handed it.
akgl_character_sprite_add wrote over an existing binding without releasing the
sprite it displaced, so a character that rebinds a state while alive leaked a
sprite slot per rebind -- teardown only gives back what the map holds at the
end. 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.
The write was unchecked too.
Three failure-path leaks moved into CLEANUP blocks: akgl_render_2d_init's two
pooled strings, akgl_controller_open_gamepads' enumeration array, and
akgl_text_rendertextat's surface and texture -- the last being a leak per frame
on a HUD line.
akgl_text_unloadallfonts() closes every font in the registry and destroys it,
which is what item 38 left open. Deliberately not a whole akgl_game_shutdown:
tearing down the mixer, SDL_ttf and SDL in the right order is a design
question, and this is the part that was simply missing. It is a new public
symbol, which 0.5.0 already covers -- this release has not shipped.
Every fix has a test that fails against the old code.
25/25 pass, memcheck clean, reindent --check, check_api_surface and
check_error_protocol all clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes internal-consistency items 16 and 17.
Ten *_RETURN macros sat inside ATTEMPT blocks, which return past CLEANUP and
skip every release in it. The one that mattered was the success path of
akgl_get_json_tilemap_property: it leaked two of the string pool's 256 entries
on every lookup that *found* what it was asked for, and a map load does that
several times per layer. tests/tilemap.c now runs each of its three paths --
found, absent, wrong type -- twice the pool size and asserts the pool is where
it started. Against the old code that test does not merely fail, it segfaults,
which is Defects item 30 seen from the outside: pool exhaustion arriving as a
NULL strncpy rather than as AKGL_ERR_HEAP.
Two of the conversions 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 are followed by `*collide = false;`, which would have overwritten
the hit that broke out; each corner test writes the flag itself, so that line
is gone rather than moved. The same function also released its scratch string
once per loop iteration while continuing to use it, so the slot was free while
still live -- one claim now covers the whole scan.
akgl_controller_default is the other behavioural one: its SUCCEED_RETURN was
the last statement in the ATTEMPT block, so the path that falls out of FINISH
reached the closing brace of a non-void function.
scripts/check_error_protocol.py keeps both rules enforced -- a *_RETURN inside
ATTEMPT, and a return out of a HANDLE block -- as the error_protocol test.
Neither produces a compiler diagnostic and neither fails a test run until the
pool it drains is empty, which is why both have already shipped once.
For item 17: the eight typed JSON accessors that validated their container and
then wrote through dest unconditionally now check key and dest as the two
string accessors always did; the null physics backend checks its actors like
the arcade one; and akgl_render_2d_frame_start, _frame_end and _shutdown check
self, which the first two read straight through. tests/renderer.c calls all
three with NULL, which segfaulted before.
25/25 pass, memcheck clean, reindent --check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes internal-consistency items 1 through 6, 12 and 13. Every include guard
is _AKGL_<FILE>_H_, every in-project header include is angled, and every
exported function, type and global carries the akgl_ prefix. This is an ABI
break; the soname goes to libakgl.so.0.5. TODO.md carries the full rename
table.
The renames were driven by renaming each declaration and letting the compiler
find the uses, not by pattern substitution: renderer, physics and camera are
also parameter and struct-member names, and a sed would have rewritten
map->physics and every akgl_RenderBackend *renderer parameter without a word.
Item 4 turned out not to be cosmetic. The library exported a global called
renderer and tests/character.c defined an SDL_Renderer *renderer of its own;
the executable's definition preempted the library's, akgl_sprite_load_json
read a SDL_Renderer * through an akgl_RenderBackend *, and every texture load
in that suite failed. The suite reported success anyway, because libakerror's
unhandled-error handler ends in exit(errctx->status), exit keeps only the low
byte, and AKGL_ERR_SDL is exactly 256. So character had been green while
running one of its four tests, and every suite in the tree was unable to fail
on the most common status in a library built on SDL.
Both are fixed. tests/testutil.h gains TEST_TRAP_UNHANDLED_ERRORS(), which
collapses any status a byte cannot carry onto 1, and every suite installs it.
character binds a real backend with akgl_render_2d_bind. Its fourth test then
runs for the first time and fails on a defect it has asserted all along, so
akgl_heap_release_character now walks state_sprites with
AKGL_ITERATOR_OP_RELEASE and destroys the property set before zeroing the slot
-- TODO.md Defects item 21 and half of Carried over item 1.
AKGL_TIME_ONESEC_MS said "one second in milliseconds" and held 1000000, so
akgl_game_state_lock waited roughly sixteen minutes rather than one second. It
is AKGL_TIME_ONEMS_NS now, the budget is its own named constant, and
tests/game.c holds the mutex from a second thread to assert the wait -- the
contended path had no coverage at all.
Headers are self-contained and it is enforced: AKGL_PUBLIC_HEADERS drives both
install() and a generated translation unit per header, so a header that ships
is a header that is checked. Writing that found registry.h, which used
SDL_PropertiesID in eight declarations and included no SDL header.
23/23 suites pass, memcheck is clean, reindent --check is clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A host that owns its own window -- an embedded interpreter, which must not
create one -- wants the six 2D function pointers without the window, the
camera and the property registry that came with them. Until now the only
options were to let libakgl own the window or to assign the vtable by hand.
akgl_render_bind2d() installs the methods and nothing else, deliberately
leaving self->sdl_renderer alone so a caller's own renderer survives it and
a caller without one gets a backend that reports AKERR_NULLPOINTER rather
than crashing. akgl_render_init2d() calls it once its window exists.
tests/renderer.c is new and needs no display: the binding, frame start and
end, both draw_texture paths and the draw_mesh stub against a software
renderer under the dummy video driver. src/renderer.c goes from 10% line
coverage to 56%.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
