The rules section now says where new work goes: an issue on the forge, with the
file and line, the functional consequence, and what closing it would touch,
carrying status::grooming until its scope is settled. TODO.md is the record of
decisions and measurements, and adding outstanding items back to it is what the
split exists to prevent -- a description of work still to do goes stale the
moment somebody does it.
Two rules go with it. Say what you learn about an open issue in the issue,
because an issue nobody has re-checked is worth what an unverified comment is
worth. And file a dependency's defect against that dependency rather than
working around it here.
The 'do not trust a comment, a TODO entry, or a CI exclusion' lesson keeps its
evidence and gains the three entries found stale during the migration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TODO.md carried two records in one file: what had been done, with the
measurements behind it, and what was left. The second half is what a tracker
is for, and keeping it here has already cost something -- AGENTS.md records a
round where eleven entries described code that had already changed, and this
file admitted to three more.
Every open item is now an issue on source.starfort.tech/andrew/libakgl,
labelled by kind and blast radius and milestoned by what it 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. Four are epics: the performance plan (#60),
coverage (#61), actor rotation (#62), and the false header comments (#63).
Verified against the tree before filing rather than transcribed. Three entries
were already fixed and were not filed: the akgl_path_relative context leak, the
akgl_draw_background test extension, and the SDL enumeration audit -- keyboards,
gamepads and mappings are all freed in CLEANUP today. Two were reworded because
the code had moved: the fonts item is a missing teardown entry point rather than
a missing API, since akgl_text_unloadallfonts exists, and draw_world's tilemap
call is already bounded by numlayers, so only the per-layer actor rescan remains.
TODO.md keeps 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.
TODO.txt is deleted. Four of its eight entries had shipped -- actor-to-actor
collision, actor-to-world collision, automatic facing, image layers -- and the
four that had not are #74 through #77, with the GPU renderer's research links
kept because that is the part that took the time.
Every reference that named an item number or a moved section is repointed, in
the manual, the headers, the tests and the examples.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merges the clay-ui branch: the akgl_ui subsystem on clay v0.14, the uidemo example, chapter 22, the weaker-model tutorial-test fixes, and the CI repairs that produced this repository's first fully green run.
The memory job's first-ever complete valgrind pass on the runner reported
every real suite and all three example games clean -- and 414 findings in
docs_examples and docs_screenshots. Those two tests are bash scripts that
drive gcc and run the compiled snippets as children; valgrind wraps the test
command and does not trace children, so all 414 were /bin/bash's own
by-design leaks and not one byte of libakgl. Memchecking them proves nothing
the suites and example games do not already prove as real binaries under the
same valgrind run.
memcheck.sh now excludes exactly those two by name, with the reasoning in a
comment beside the exclusion -- per the suppressions file's own rule that
nothing gets quieted without a stated reason. Verified locally: full run over
the RelWithDebInfo tree, every suite and example under valgrind, zero
findings, exit 0.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
The env-list fix got every test green on the runner for the first time --
37 of 37, the JUnit publish included -- and the job then failed in the
artifact upload: upload-artifact@v4 speaks GitHub's v2 artifact service and
hard-refuses any host that is not github.com ("not currently supported on
GHES"). Gitea implements the v3 artifact API, so the three upload steps --
coverage, the benchmark tables, the valgrind logs -- pin to v3.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
The example games failed on the CI runner with "ALSA: Couldn't open audio
device" despite SDL_AUDIODRIVER=dummy sitting right there in their CMake --
because it never reached the process. set_tests_properties parses its
PROPERTIES arguments as name/value pairs, so a semicolon-separated value
list is split and every element after the first is consumed as a bogus
property name: ENVIRONMENT kept only SDL_VIDEODRIVER=dummy, and the
LD_LIBRARY_PATH prepend list kept only its first directory. ctest
--show-only=json-v1 shows the truncation plainly.
Nobody could see it. Video survived because SDL3 falls through its driver
list to dummy on a headless machine with no hint at all; audio survived on
every developer machine because the real device works there; the library
paths survived because RPATH covered them. A runner with no sound card was
the first environment where any of it mattered.
Every list-valued test property now goes through set_property(TEST ...),
which takes real list arguments -- the examples' ENVIRONMENT triplets and
vendored-path modifications, the suites' LD_LIBRARY_PATH prepends, and the
docs harness's driver variables. Verified by property dump (3 environment
entries and all 7 prepends present) and by running the example tests with
ALSA_CONFIG_PATH=/dev/null, which now pass where a real ALSA open would
fail: the dummy driver is finally the one being asked.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
The submodule fix got the cmake_build job through checkout, dependencies,
configure and the whole compile for the first time in the run history -- and
into the next latent failure: `doxygen Doxyfile` generates dot graphs and the
Doxyfile fails on warnings, but the runner image has no `dot`, so every graph
reported "Problems running dot: exit code=127" and the step failed after an
otherwise clean build. Locally graphviz was installed, which is why the
pre-flight missed it. One package in the one job that builds documentation.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
CI has never gone green in the retained run history, and every failure is
the same 20-second death during checkout: six submodules -- SDL, SDL_image,
SDL_mixer, SDL_ttf, jansson, semver -- were registered with git@github.com:
SSH URLs, and the runner has no GitHub key. The https submodules (libccd,
tg, clay, and the two starfort ones) clone fine in the same run. All six now
use https, which is what every stanza added since already did; these are
public upstream repositories nothing here pushes to, so SSH bought nothing.
Second problem, waiting right behind the first: the character, sprite and
tilemap suites were the only three of seventeen that never set the dummy
video/audio/software-render hints, so on a headless runner they die in
SDL_Init with "No available video device" before testing anything. They now
set the same three hints the other fourteen suites set. (How this survived:
every local run happens on a machine with a display, and CI never got far
enough to run a test.)
Pre-flighted locally against every job the workflow defines, headless with
no driver variables in the environment, exactly as the runner would: Debug
with -Werror and coverage builds clean and all 37 tests pass including the
coverage fixtures; RelWithDebInfo with -Werror builds clean and the perf
suites pass their budgets at full scale; doxygen Doxyfile exits 0 under
FAIL_ON_WARNINGS; and the new ui suite runs under valgrind with zero errors
and no leaks.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
The AGENTS.md "Testing a Tutorial" pass: chapter 22 was handed to a reader on
a weaker model with the library tree stripped of examples/ and docs/, and
they had to build a menu/HUD/dialog program from the chapter alone. They
built one that compiled and ran -- and reported success their own screenshots
disproved, which is why the protocol says to verify: their synthesized key
events set only the scancode, akgl_ui_menu_handle_event matches keycodes, and
their program never actually left the title screen. With that one line fixed,
everything they had built from the chapter worked.
Three findings survived verification against eight reported:
- The chapter never stated its prerequisites, so the reader burned most of
the session on library bring-up it does not cover and invented goto error
handling for main(). A new "What this chapter assumes" paragraph points at
chapters 3, 7 and 17, and at the demo's startup() and main() as the
complete reference.
- Nothing named the menu's exact keys, or that they are keycodes rather than
scancodes -- invisible with a real keyboard, fatal for synthesized events,
which the demo's own --demo script teaches. The menus section now names
SDLK_UP/DOWN/RETURN, the gamepad buttons, and the key.key trap.
- Verifying their dialog screenshot showed a message wrapping past the fixed
AKGL_UI_DIALOG_HEIGHT runs visibly out of the panel. Deliberate -- same as
the hand-rolled textbox, and visible overflow beats silent truncation --
but undocumented; akgl_ui_dialog's header note now says so.
Rejected for the record, per protocol: akgl_game_init already initializes the
property registry (src/game.c:219), and the font they "found" in a different
directory was the build tree's file(COPY) of tests/assets.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
Two TODO.md notes the UI work created. The text texture ring cache (plan
item 2) now has its second consumer, and the bigger one: the UI executor
draws every TEXT render command through akgl_text_rendertextat, one wrapped
line per command per frame, so a five-row menu re-rasterizes five lines at
60 Hz against the original case of one HUD line changing once a second.
And akgl_ControlMap's mouseid/penid fields stay dead deliberately -- the
mouse path lives in the UI subsystem because control maps and pointer state
are different consumers, and the note says where the work lands when a game
wants mouse-driven actors.
0.8.0 to 0.9.0: a new public header, a new status code, three new drawing
primitives and a new subsystem is new API, and while the major version is 0
the soname carries major.minor -- libakgl.so.0.9 is what makes 0.8.0 and
0.9.0 unmixable at load time, which is the honesty the bump is for.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
docs/22-ui.md covers the clay-backed UI: what clay owns against what libakgl
owns, bring-up and the arena refusal contract, the frame bracket and where it
sits in a real frame, the consumed-event rules (with the one-frame-stale hit
test stated plainly), the three widgets, menus across keyboard, gamepad and
mouse, writing raw CLAY() with an application-owned press edge, images and
styles, and how layout errors surface from frame_end with a captured trace.
Every listing is a checked block: one linked-and-run example with pinned
output, eight excerpts quoting examples/uidemo and examples/jrpg/textbox.c,
and the chapter figure is a frame out of uidemo via docs_game_figures.
The chapter owes and pays the comparison the tutorials earn: the dialog
widget's default style reproduces the JRPG textbox palette, the two listings
sit side by side, and the trade is stated -- 125 lines you own completely
against one call that costs you the subsystem. Neither is deprecated;
chapter 21 keeps teaching the hand-rolled panel on purpose.
The appendix moves to 23 (links in chapters 4, 5, 6, 7, 13 and the TOC
updated) and gains the ui.h status cross-reference, the AKGL_UI_* limits
table, and the new draw primitives' rows. Its status-band figures were stale
at COUNT 5 / LIMIT 261 while the band already held six codes; now correct at
7 / 263. Chapter 4 gains AKGL_ERR_UI (262) in the excerpt, the table and the
name list.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
examples/uidemo is the program the UI chapter quotes. Three screens, three
ways of building an interface: the title screen is one akgl_UiMenu plus a
heading label; the options screen is written in raw CLAY() declarations --
hover highlighting inside the declaration, clicks paired with the
application's own press edge -- because the widgets are a convenience, not a
boundary; and the play screen is two HUD labels and the one-call dialog over
a checkerboard standing in for a game world. No tilemap, no actors, no
physics: everything left on screen is the subject.
Its route_event() is the documented call-order contract in the flesh -- UI
first, consumed events stop there, then the current screen's keys -- and the
screen routing is the focus model, stated rather than invented.
The headless smoke run (example_uidemo, --frames 240 --demo) tours every
path through the real event chain: a mouse click on the centred menu (the
consumed path, landing on the middle row by symmetry), keyboard into and out
of the options screen, the dialog opened and dismissed, and Quit confirmed
from the menu. The run prints its score and settings so a pass can be read,
not just counted. docs_game_figures gains a uidemo frame -- the play screen
with the dialog up -- for the chapter to come.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
A pointer parked over a menu row re-claimed the selection on every frame's
declaration, so keyboard navigation fought the mouse sixty times a second
and lost -- press Up, and the hover put the selection straight back. Found
by the UI demo's scripted tour: its mouse click leaves the pointer resting
on the menu, and the Up keystroke that followed did nothing.
Hover now moves the selection only on frames the pointer actually moved (or
clicked), through a frame-latched motion edge beside the existing press
edge. Parking the mouse claims nothing; moving it onto a row still selects,
clicking still selects and activates. Regression test drives the exact
sequence: click a row, move the selection away by keyboard, redeclare, and
assert the stationary pointer stole nothing back.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
The other half of the hybrid decision: an application that wants a dialog
box, a score counter or a main menu gets each in one call, without touching a
CLAY() macro -- and an application that outgrows them still has clay's whole
DSL, because the widgets are nothing but functions that declare the same
elements between the same frame bracket.
akgl_ui_dialog is the JRPG textbox in one line: bottom panel spanning the
layout, AKGL_UI_DIALOG_HEIGHT tall, one-pixel border, wrapped text. The NULL
style *is* the textbox palette -- near-black fill, parchment edge and ink,
8px padding -- deliberately, so the widget and the 125-line hand-rolled panel
it stands beside in the comparison chapter produce the same picture. Showing
and hiding is the frame's business: call it while somebody is talking, don't
while nobody is; a declarative frame is the visible flag.
akgl_ui_label pins a fit-sized text chip to a corner or the centre, inset by
the style's padding. akgl_ui_menu declares a centred vertical menu from a
caller-owned struct (no heap layer: it owns no texture, no font, no registry
entry), with the selected row drawn inverted, hover moving the selection and
the frame-latched press edge activating it. akgl_ui_menu_handle_event drives
the same struct from Up/Down/Return and D-pad/South with wrapping, slotting
into the event chain after akgl_ui_handle_event; routing events to a menu is
the application's focus model, stated rather than invented.
Tests cover the dialog's geometry and palette by pixel, label anchoring by
quadrant, menu validation, the inverted highlight, keyboard and gamepad
wrap/activate/pass-through, and the full two-frame mouse protocol -- click
aimed at the second row's real box via Clay_GetElementData, seen by the next
frame's declaration as selection plus activation.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
akgl_ui_handle_event is the mouse half of the input story -- keyboard and
gamepad stay with the control maps, because keyboard focus is something an
application declares rather than something a pointer position implies. It
handles motion, left-button presses and releases, the wheel and window
resizes; everything else, and everything while the subsystem is inert,
reports consumed=false and succeeds, the same pass-everything contract
akgl_controller_handle_event already has. The documented call order is UI
first, control maps second, skipping on consumed.
A press, release or wheel is consumed when the pointer is over any UI
element. The hit test asks clay against the layout the previous frame
retained -- this frame's does not exist while events are polled, and one
frame of staleness is the standard model's accepted cost -- with clay's
internal full-screen Clay__RootContainer excluded, because being inside the
window is not being over the interface (unexcluded, every click anywhere was
consumed; the test that pins this caught it).
Pointer state is fed to clay per event, as clay documents; the press edge is
latched once per frame_begin for the widgets to come, rather than trusting
Clay_PointerData's per-SetPointerState edge, which advances per mouse event.
frame_begin also drives Clay_UpdateScrollContainers from the accumulated
wheel (32 pixels per notch) and a wall-clock dt.
Tests pin the whole contract: pass-through before init, no consumption
before any layout exists, press/release inside vs beside a panel, right
button ignored, motion never consumed, wheel consumed only over the UI,
resize reaching the layout engine, and key events left alone.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
The UI subsystem now draws. akgl_ui_frame_begin/frame_end bracket one frame's
CLAY() declarations: begin clears the error stash and starts the clay layout,
end computes it and walks the render commands through a backend -- RECTANGLE
as (rounded) fills, BORDER as radius-shortened edge fills plus corner arcs,
TEXT through akgl_text_rendertextat one wrapped line per command, IMAGE as an
akgl_Sprite's first frame stretched to the bounding box, and the SCISSOR pair
through akgl_draw_set_clip, cleared again on any exit so a failed frame
cannot leave the world clipped.
akgl_ui_font_register maps registry font names onto clay's uint16_t fontIds.
The table keeps the *name* and resolves it per use -- fonts are not reference
counted, and a cached TTF_Font* would dangle where a name reports "gone"
honestly. Clay_TextElementConfig.fontSize is deliberately ignored: libakgl
bakes the size into the handle at load, so one face at two sizes is two ids.
The measure bridge passes clay's non-NUL-terminated slices straight to
SDL_ttf's explicit-length TTF_GetStringSize -- no copy, no scratch. Its
signature has no error channel, so failures report zero-by-zero and stash a
message that frame_end raises, the same route clay's own void error handler
uses; layout errors take precedence over drawing and one bad frame leaves the
next one clean.
Tests drive real CLAY() layouts through the software renderer and read pixels
back: fill placement, border edges with an empty middle, a child clipped by
its container, text landing in its colour, slice-vs-whole measurement
agreement, fontId table dedupe/refusal/exhaustion, and the bracket's
begin/begin, end-without-begin and failure-recovery contracts.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
The UI subsystem's render executor needs all three -- clay emits rectangles
with corner radii, per-side borders, and scissor commands -- but they earn
their place in draw.h on the same terms as everything else there: a rounded
panel, a ring gauge and a clipped viewport are things a game wants with or
without a layout engine.
akgl_draw_filled_rounded_rect decomposes into three band fills plus four
quarter-circle triangle fans through SDL_RenderGeometry, with the radius
clamped to half the shorter side. akgl_draw_arc strokes between an outer and
inner radius as one triangle strip, stepping in proportion to the swept angle
under a fixed vertex cap -- no VLAs, unlike clay's own SDL3 reference
renderer. akgl_draw_set_clip wraps SDL_SetRenderClipRect and is the one draw
entry point where a NULL rectangle is legal, because that is how a clip is
cleared.
Pixel-readback tests cover the corner geometry (inside the arc filled,
outside it untouched), the radius clamp, the zero-radius fall-through, ring
and quarter-arc coverage at mid-stroke, sweep bounds, and clip set/clear
round trips.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
clay v0.14 (deps/clay, zlib licence) supplies the layout engine for the new
akgl_ui subsystem. Sources-listed rather than add_subdirectory()'d, on the
semver/libccd precedent: clay's CMakeLists declares no library target, builds
its examples by default, and requires CMake 3.27 against this project's 3.10.
src/ui_clay.c is the one CLAY_IMPLEMENTATION translation unit, compiled -w on
the vendored-code terms but with clay's symbols deliberately exported --
consumers' CLAY() macros resolve against libakgl.so, and akgl/ui.h warns them
never to link a second clay.
The subsystem is runtime-optional the way collision is: always compiled in,
inert until akgl_ui_init(), which bounds clay to the overridable AKGL_UI_*
ceilings, checks Clay_MinMemorySize() against a static BSS arena (no malloc),
and refuses with both numbers in the message when it does not fit. Measured:
812544 bytes at the default 1024 elements / 4096 measured words, against a
1 MiB arena. clay's void-callback layout errors are logged as they happen and
the first is stashed for the error protocol to raise later.
AKGL_ERR_UI joins the status band before AKGL_ERR_LIMIT, named in
akgl_error_init. New `ui` test suite covers validation, the init/shutdown
lifecycle, and arena exhaustion via the akgl_ui_arena_limit test hook (same
contract as akgl_ccd_arena_set_limit). clay.h is installed beside semver.h,
its licence beside libccd's, and the headers suite proves akgl/ui.h
self-contained -- clay.h compiles clean under -Wall.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
docs_examples proves every listing in docs/ compiles and still matches the file
it was quoted from. It cannot prove a chapter teaches anything -- a document can
be made entirely of verified excerpts and still be unfollowable, because what a
reader needs is the glue between them.
So AGENTS.md gains "Testing a Tutorial": hand the chapter to a subagent on a
weaker model with the library, its headers and the art but not the finished
program, and make them build and run the game. The weaker model is the point; it
will not paper over a gap with knowledge the document did not give it.
The section carries the rules that make the result mean something -- copy the
tree with examples/ and docs/ removed rather than asking them not to look, let
them read the public headers because a real reader has them, do not make them
hand-author a .tmj, give them a screenshot helper, and verify their binary
yourself -- plus the sandbox recipe, and the warning to prove the build path
works before handing it over.
It also records how to read the report, because a weaker model states its own
mistakes as documentation defects with total confidence. Reproduce before
believing; and a rejected finding usually still leaves something behind.
The evidence for doing any of this is in the table at the end: four read-only
reviews of chapters 20 and 21 found real gaps and missed all three of the
defects the first build-and-run found, including a published example output the
chapter could not produce.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
A reader drew their own town, put the elder 115 pixels from anywhere the player
could stand, and spent the rest of their time looking for the bug in
jrpg_cmhf_talk. There is no bug: the handler finds nobody within TALK_RANGE and
returns success, which is indistinguishable from the game not being wired up.
The chapter stated the constant and not what it means in the editor.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
Two readers were given the library, the art, the chapter, and no reference
program, and told to compile and run. Both got a working game. These are the
defects that only came out because a compiler and a frame loop were involved --
four earlier read-only reviews had missed every one.
libakstdlib's header was never named. The chapters used aksl_strncpy,
aksl_snprintf and aksl_atoi throughout and said only "libakstdlib is documented
in deps/libakstdlib"; the obvious guess from the function prefix is <aksl.h> and
it is wrong. That was the only compile error the sidescroller reader hit, and it
was the chapter's fault. Both chapters now carry a per-file include table and say
which header it is.
The sidescroller's asset directory defaulted to ".". The chapter said the code
"falls back to `.`" without showing the #ifndef or that assetdir is initialised
from the macro, so a reader gets a game that only finds its art when launched
from exactly the right directory.
The sidescroller's summary line reported 0.0,0.0 airborne while the screenshot
showed the player standing on the ground. Teardown runs in main's CLEANUP block
and releases the actor pool before the summary prints. The chapter published an
example output a reader could not reproduce; the capture that makes it true is
now shown beside it.
The JRPG's CMake block interpolated ${JRPG_REPO_ROOT}, which is this repository's
own variable and undefined for anybody else -- so the font path came out wrong
and akgl_text_loadfont failed long after everything else had loaded. It is
${CMAKE_CURRENT_SOURCE_DIR} now, and the chapter says why both definitions are
needed rather than one.
Chapter 21 described --demo and a summary line in its build section that no step
wrote. Both are now a section: the script table, playback through
akgl_controller_handle_event rather than the handlers, the fixed clock step, and
printing the position before teardown. It also now says libakgl keeps no frame
counter, which a reader assumed it did.
Both map sections now say to draw it in Tiled and point at chapter 13, naming the
fields the loader needs that Tiled fills in automatically -- both readers
hand-wrote a .tmj before being given one and hit exactly those.
Excerpts across docs/ go from 190 to 198.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
Two agents were given nothing but the chapter text and asked to build the game
from it. Between them they named every place the chapters showed a fragment and
called it an explanation. This closes those.
Chapter 20 now shows what it previously only named: the whole ss_Game struct
rather than a truncated one, SS_COIN_COUNT and SS_HAZARD_COUNT, ss_ActorData,
the player and blob body rectangles, asset_path, hitbox, find_actor, respawn,
both jump handlers, the control-map function head, the gamepad buttons, the
blob's probe arithmetic, and a complete standalone CMakeLists rather than three
lines out of one. It also names the library globals a reader is expected to use
without declaring, lists all seven actor hooks rather than the two this game
replaces, and says in order what each of the two hook functions ends up doing.
Chapter 21 gains the same treatment: the header's includes and declarations, the
textbox colours and their SDL_Color type, TTF_Font, the jrpg_Townsfolk row type,
character_load in full, the player lookup, the frame loop, and per-file include
tables for both games.
Two claims were wrong and are corrected. "Each step is complete on its own" was
not true of a cumulative tutorial. And the first argument to
akgl_controller_handle_event is not a game-state word the dispatcher reads -- the
header says nothing reads it and it is required only as a non-NULL token.
Every step that produces code now ends with what the reader should see when they
run it. Excerpts across docs/ go from 137 to 190.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
They read as design commentary: why a choice was made, what the previous state
of the project was, which library defect a line works around. That is useful to
somebody who already knows the library and useless to somebody trying to build a
game, which is who a tutorial is for.
Both chapters now open with a screenshot of what the reader is building and a
numbered list of the steps to get there, and each step is its own section: what
you are trying to achieve, the code that achieves it, and the two or three things
about that code that are easy to get wrong. Chapter 20 starts from first
principles -- what a sprite, a character, an actor and a tilemap are, and the
four error macros -- and chapter 21 assumes it and covers what a top-down game
adds.
Where a line exists because of a defect, the line comes first and the defect
comes after it, with a pointer to the TODO entry that will remove the need. A
reader who is copying code should not have to read an apology before they can use
it.
The library's own history is gone from both. That belongs in the design chapters
and in git.
Two new docs_examples setups stage the tutorials' own art, so the sprite and
character files a reader is shown are loaded exactly as printed rather than
through a generic fixture. `json excerpt=` is new for the same reason: a Tiled
object is now quoted out of the real map instead of being retyped. Excerpts
across docs/ go from 91 to 137.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
Both examples gain --screenshot PATH and --screenshot-frame N. The capture sits
between the world being drawn and the frame being presented, because
SDL_RenderPresent is where the target stops being readable, and it works under
the dummy video driver and the software renderer like the rest of the headless
path does.
`cmake --build build --target docs_game_figures` regenerates
docs/images/sidescroller.png and docs/images/jrpg.png by running each game to a
chosen frame. Same contract as docs_screenshots: deliberate, never part of a
build, because the PNGs are tracked. There is no --check counterpart, and the
target says why -- the sidescroller drives its physics from the wall clock, so a
byte comparison would fail for reasons that have nothing to do with the
documentation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
akgl_rectangle_points, akgl_collide_point_rectangle, akgl_Point and
akgl_RectanglePoints go. They were the intermediate form of an implementation
that changed: akgl_collide_rectangles was eight corner-containment tests built on
them, and it has been four span comparisons since the cross-case fix. Nothing
outside tests/ called either function, and a point-in-rectangle test is four
comparisons a caller can write without a struct conversion in front of them.
akgl_collide_rectangles stays. It has two correct callers in the sidescroller
asking a game-level overlap question -- a coin, a hazard, from an updatefunc --
where a bool is the whole answer and a proxy plus a narrowphase call would be
computing a normal nothing reads. TODO.md records the split rather than leaving
it to be rediscovered.
Public API removal, so 194 exported akgl_ symbols against 196, and the manual's
counts move with them. The perf suite loses its rectangle_points row; the
all-pairs sweep stays as the control it is now labelled, and PERFORMANCE.md says
what 0.8.0 measured against it -- 188.5 us for 32,640 pairs at 256 actors, where
a whole step with collision attached is 54.1 us doing strictly more.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
test_reset_and_empty called spawn() with an uninitialized stack akgl_CollisionShape
and built it on the next line. akgl_collision_proxy_initialize copies the shape,
so grid_insert read uninitialized half-extents to work out which cells to file
the proxy under -- eighteen findings inside grid_cellrect, and the test still
passed because the proxy_sync two lines later overwrote all of it.
Found by scripts/memcheck.sh, which is the only thing that could have found it:
the assertion it makes is about the query afterwards, and the query answers
correctly either way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
libccd is compiled into libakgl.so and is BSD-3-Clause, so the notice has to
travel with the binary form. cmake --install now puts BSD-LICENSE at
share/doc/akgl/BSD-LICENSE.libccd, and the README says which dependency is in
that position and which are not -- everything else in deps/ is either a separate
shared object carrying its own notice, a header, or compiled into nothing.
The plan's last step was to delete akgl_rectangle_points,
akgl_collide_point_rectangle and akgl_collide_rectangles. That step is not taken,
and TODO.md carries the reasoning rather than leaving it to be rediscovered:
akgl_collide_rectangles has two correct callers in the sidescroller asking a
game-level overlap question that wants a bool, and it was fixed two commits into
this same series -- deleting it now would be a strange thing to do to a caller.
The other two are the weak case and are named as the candidates if a 0.9.0 wants
to trim.
akgl_RectanglePoints' comment still claimed akgl_collide_rectangles works corner
by corner, which stopped being true when the cross case was fixed. Corrected in
place, with the cross case named so the note explains why the shape changed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
Collision changed the collide slot's signature and grew four public structs, so
this is an ABI break and the soname moves to libakgl.so.0.8. The manual's counts
move with it: 195 functions across 23 headers, 183 of which return an error
context.
Target 12 -- "collision for 256 actors under 2 ms without the caller writing a
broad phase" -- is met. A new benchmark times the whole step with a world
attached, which is the number the target is actually about rather than a
narrowphase call in isolation: 15.4 us at 64 actors, and 54.1 us at 256 in a
purpose-built -DAKGL_MAX_HEAP_ACTOR=256 configuration. Over that 4x range the
step grew 3.5x while the all-pairs control grew 15.6x, which is the n-squared
the index exists to avoid. Plan item 7 is rewritten to record what shipped and
why both partitioners exist; the Construct and Phaser citations stay.
Three new TODO entries. Actor rotation, scoped to the five places it touches and
the one place it does not -- collision_support is written so rotating `dir` in
and the answer back is the whole narrowphase change, and rotational *response*
is explicitly a different piece of work. The swept narrowphase FLAG_BULLET
reserves, with the tunnelling arithmetic written out so a game can check its own
numbers against 1280 px/s. And the ccd arena being single-threaded and sized by
measurement at 7,264 bytes per GJK/EPA box pair.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
cell_solid and feet_blocked go, and so does the prediction in the player's
movementlogicfunc. That prediction was only ever exact because the town has zero
gravity and zero drag -- v is t, so `x + tx * dt` is where the step lands. A map
with gravity would have had to fold ey in as well, which is the game
re-implementing the integrator. Resolution runs after the move now, so there is
nothing to predict.
The map's decoration layer carries `collidable`, which retires JRPG_LAYER_SOLID:
akgl_TilemapLayer has no name member, so the game and the map had to agree on an
index out of band and inserting a layer in Tiled broke it.
The edge of the world is not on any layer, so it is four static proxies covering
the outer ring plus a tile of overhang -- one pool slot per side instead of a
hundred solid tiles, and the first use of the static-proxy path in either
example. They carry LAYER_STATIC explicitly: shape_box defaults a shape to
LAYER_ACTOR, and a wall left on that layer is a wall everything walks through.
Only the player gets a shape. NPCs stand still and are spoken to; the follower
is a child snapped to its parent every step.
Verified against the old implementation with the demo script rewritten to hold
one direction: holding left into a building stops the player at x=122 with both,
and holding up into the map's edge stops them at y=-4 with both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
collision.c was 383 lines that re-implemented akgl_physics_simulate's own
arithmetic, because movementlogicfunc runs before gravity, drag and the move
and so had to predict where the actor would end up. The whole file goes. What
replaces it is three lines in main.c, a shape on the player and the blob, and a
`collidable` property on the map's terrain layer -- which also retires
SS_TERRAIN_LAYER_ID, a compiled-in layer id that broke whenever somebody
reordered layers in Tiled.
grounded stays a probe rather than becoming a contact test. A contact says
something pushed back this step; grounded asks whether there is a floor to push
off, and that stays true through the frame after landing. The blob keeps its own
wall and ledge probes for the same reason: the resolver does not say which side
of the blob it was pushed from, and says nothing at all about a floor that is
missing.
The summary line now carries the player's resting position, because exiting 0
was not evidence that collision ran. It reports 136.0,160.0 grounded after 240
autoplay frames -- feet at y=192, flush on the surface of terrain row 12.
The tutorial's collision section is rewritten around the library's, keeping the
quarter-of-a-pixel story as what a predicting resolver costs. The docs harness
learns `json excerpt=`, so the map property the chapter quotes is checked
against the map.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
The new chapter 15 covers the opt-in setup, where resolution runs in the step
and why it runs after the move, shapes and the z-extrusion trap, the asymmetric
masks and the defaults that matter more than the mechanism, tiles as a source
rather than proxies, the response hook and the three ways it is easy to write
wrongly, sensors, the four queries, both partitioners, and what is still not
implemented.
Collision falsified claims in eight other chapters. The physics chapter said
"There is no collision detection, at all" and that collide always raises
AKERR_API; actors had six behaviour hooks; the heap had five pools and four
non-claiming acquires; the status band held five codes; and the design
philosophy had exactly two pluggable subsystems. The appendix gains a
collision.h status section, the three new pool ceilings and a table of the
collision constants that are deliberately not in a public header.
Also fixes chapter labels the renumbering commit left pointing at their old
numbers -- "18. Utilities" linked to 19-utilities.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzBDV2fqgnUAcqCKqKvc71
A pure rename plus link rewrite and nothing else. Collision is a pluggable
subsystem with two implementations, shapes, a response hook and a tile binding;
that is a chapter, and burying it inside the physics chapter is the shape this
manual otherwise avoids -- rendering and drawing are 8 and 9, spritesheets and
characters and actors are 10, 11 and 12.
Kept separate from writing the chapter because **nothing validates a
cross-reference target.** The example harness reads fenced blocks; it does not
follow links. A rename mixed into five hundred lines of new prose is not
reviewable, and a broken link would land silently. As its own commit it is
reviewable as a rename, and a grep for dangling `](NN-` targets is clean.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven new benchmark rows, budgets set from a measured full-scale run rather
than guessed, and the three existing rows the work moved re-recorded from that
same run. The all-pairs sweep stays and is relabelled `control:` -- it is the
cost a caller paid before the library had a broad phase, measured on the same
machine in the same run, which is the only honest way to read a reduction.
What the numbers say:
- The box fast path is two to seven times cheaper than the general solver, and
the disjoint case is cheaper still because the proxies' bounds reject it before
any shape arithmetic runs. 9 to 20 ns is what a tile game actually pays.
- The grid beats the tree by 2.6x on the same population, which is the argument
for the default measured here rather than cited from another engine. The tree
also rebuilds on every move and that is not in its row, so a moving scene is
worse than 2.6x.
- A grid `move` that changes nothing is 11.5 ns. That is the incremental claim
in one number.
Two rows moved on a backend with **no collision world attached**, so no
collision code runs in either, and the commit says so rather than letting the
feature take credit:
- akgl_Actor grew from 415 to 464 bytes -- a 40-byte shape, an override flag and
a proxy pointer. The step sweeps the whole pool, so that is about 3 KB more
working set per frame. The empty-pool row is unchanged at 58.7 ns, which is
what identifies the cost as per live actor rather than per slot.
- Reaching the collision check through CATCH cost more than the check. PASS and
CATCH call akerr_valid_error_address, which walks AKERR_ARRAY_ERROR, so an
early-returning function is not free when it is reached through one. The
no-collision path now routes around the machinery entirely, which took the
64-actor sweep from 2,018.7 ns back to 1,588.5. AGENTS.md records that lesson
for writing benchmarks; this is the same thing in production code.
The remainder is the struct, and it is paid whether or not a game uses
collision.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A binary space partition on libakstdlib's tree links and lists, registered in
the factory as "bsp". It runs the same assertions the grid does, because
tests/partition.c is a table over partitioners and adding a row is all it takes
to be held to the contract.
**Use the grid.** This is here to prove the vtable works and to have something
to measure the grid against, and the file says so at the top. It rebuilds
whenever the proxy set changes, which is the shape PERFORMANCE.md records Phaser
using and capping out around five thousand bodies. It would earn its place in a
world with wildly non-uniform object sizes, or one larger than the grid's fixed
cell array covers.
The difference is visible in the code rather than buried in a benchmark: the
grid's `move` compares four integers and returns when a proxy has not left its
cells, and this one marks the whole partition stale. The incremental-move
assertion in the suite is therefore grid-only, and says why.
aksl_tree_iterate is not used, and the file carries the three reasons so the
next reader does not rediscover them: it is a complete traversal whose only
control signal stops the entire walk, so there is no way to prune a subtree --
which is the only operation a spatial query is made of; it carries no per-node
context, and a pruning descent needs each node's bounds and plane; and its
breadth-first modes allocate, while only the depth-first ones are malloc-free
and those are the ones without pruning. aksl_tree_insert is unusable for a
different reason again: it is a comparator-ordered BST, and a spatial insert has
to compare a leaf against a plane, which aksl_TreeCompareFunc cannot express.
What is used is the link structure and the lists, neither of which allocates,
which is why they fit here at all. The descent is an explicit stack rather than
recursion, so the depth bound is an array bound the compiler can see -- this
library already has one documented way to blow the C stack and does not need a
second.
Split planes are the median of the item centres on the longer axis, not the
spatial midpoint: actors in a tile game cluster on the floor, and a midpoint
split gives one empty child and one full one for several levels running. A split
that separates nothing degrades the node to a leaf rather than recursing forever
on the same set.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The collide slot has existed unused since the backend was written. It is wired
in now, and its signature changes from (self, a1, a2) to (self, actor, dt): the
old shape had nowhere to put a normal, a depth, or a piece of map geometry, and
no answer to which of the two actors it was supposed to be resolving.
**Resolution runs after `move`.** That is the decision everything else follows
from. A resolver called from movementlogicfunc runs before gravity, drag and the
move, so it has to predict where the actor will end up -- which means
re-implementing the integrator's arithmetic in the game, and being wrong
whenever the integrator changes. examples/sidescroller/collision.c carries forty
lines of exactly that, and says so.
Only `move` is subdivided. Gravity, drag, the thrust integration and the speed
ellipse all still run once against the whole dt, and sum(subdt) is dt, so an
actor with nothing to collide with takes one sub-step of exactly dt -- `dt/1.0f`
is dt exactly in IEEE-754 -- and follows the arithmetic path it always did. That
is what lets the integrator stay untouched and every recorded physics number
stay valid, and tests/physics.c and tests/physics_sim.c pass unchanged with no
world attached.
Collision is opt-in. A backend with no collision world costs one comparison per
actor per step and does nothing else.
Also here:
- `self->gravity` was never null-checked and is dereferenced unconditionally, so
a hand-built backend with only `move` filled in crashed rather than reporting.
- Children are re-snapped in a post-pass, gated on collision being on. The
in-loop snap uses whatever position the parent had when the child's slot came
up in pool order, which is already sometimes a frame stale; that was harmless
while a parent only moved by v*dt and is not once a parent can be pushed out
of a wall mid-step.
- tests/physics.c's deliberate tripwire has been visited. It asserted that
arcade collide always raised AKERR_API, with a comment saying it existed so
that implementing collision would have to come back here. What replaces it
asserts the opt-in contract instead.
- physics_sim.c's hard-coded "%d of 7 simulations" is derived now. A literal
goes stale the first time somebody adds a simulation, which is this commit.
Three new whole-motion simulations, and the third took two attempts to make
honest:
- Landing and resting: 0.0000 px of drift over 120 steps after settling.
- Walking after landing: 107 px in a second, which is the symptom a player sees
when a resting actor is caught on the ground it is standing on.
- A fast fall not tunnelling. The first version used gravity, so the step size
grew every frame and the actor happened to land *inside* the floor rather than
stepping over it -- it passed with sub-stepping disabled and proved nothing.
It uses a constant 1200 px/s now, so every step is exactly 60 pixels against a
32-pixel window in which an overlap exists, and the samples miss it cleanly.
With sub-stepping the actor stops at y=288; without it, y=1300.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A seventh behaviour hook, collidefunc, defaulting to akgl_actor_collide_block.
Contacts are delivered one actor at a time with the normal already pointing the
way that actor has to move, which removes the "which one is a1" ambiguity that
made the old physics `collide` slot unimplementable -- and means a game
overriding one actor's response never has to reason about the other's.
The default moves the actor out along the normal by the penetration depth and
removes the component of its motion that was going into the surface. Three
things about that are easy to write wrongly, and each has a test that names the
symptom rather than the assertion:
- **It writes `e` and `t`, never `v`.** akgl_physics_simulate recomputes velocity
as `e + t` at the top of every step, so a write to `v` is discarded before
anything reads it. Worse than useless: an actor holding a direction into a
wall would keep accumulating thrust while standing still and leave at speed
the instant the wall ended. Breaking this leaves the fall running at 300 px/s
through the floor.
- **It removes the component, not the axis.** Zeroing the whole thrust vector
costs the actor its motion *along* the surface as well, which is a character
scraping a ceiling stopping dead and a character landing on a floor losing its
run. Sliding along a wall is the difference between a wall and glue.
- **A separating contact is left alone.** An actor already moving out of a
surface that gets its velocity zeroed is an actor stuck to a wall it is
walking away from, and it reads to a player as the collision grabbing them.
`e` and `t` are treated separately rather than as their sum, so an actor
pressing into a floor loses its downward gravity without losing the sideways run
it is also doing.
A sensor is reported and never pushed -- walking through a coin is not being
blocked by it.
The default deliberately does not pre-load the environmental term with a step of
gravity. A resolver running *before* the step has to, or the step it is
correcting for re-adds gravity and commits a quarter pixel of penetration, which
is invisible and still fatal because every horizontal move then reads as
blocked. This library resolves after the move, so the case does not arise; the
sidescroller example carries that workaround because it had no choice.
Also asserted: the other six hooks survive. A seventh that displaced one of them
would be a silent regression somewhere unrelated.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A tile layer with a `collidable` boolean custom property is collision geometry.
Before this a game had to hard-code a layer index -- both examples do, because
akgl_TilemapLayer does not retain the name Tiled wrote -- and that index changes
the moment somebody reorders layers in the editor.
Solid tiles are **not** given proxies. At the maximum map size that is a quarter
of a million per layer, tens of megabytes of index to describe data that is
already a dense grid sitting in the tilemap. The world keeps a borrowed pointer
and reads layers[i].data[] over whatever cell range a query covers: nine array
reads for a 32-pixel actor on 16-pixel tiles, nothing to build at level load,
and nothing to maintain per frame. Static geometry that is not tile-aligned is
still an ordinary proxy with AKGL_COLLISION_FLAG_STATIC; both mechanisms exist
and tiles use the free one because there are a hundred thousand of them.
Four public queries come with it, all of which answer without resolving:
solid_at, box_blocked, query_box and settle. They are what a game reaches for
when it wants to know rather than to be pushed -- a ledge probe ahead of a
walking enemy, a check that a doorway is clear -- and the sidescroller cannot
drop its hand-rolled collision without them.
akgl_collision_settle is the one worth naming. Resolution stops a shape entering
geometry and has nothing to say about one that began inside it: what it does
instead is refuse every move, so an actor spawned in a wall is simply stuck.
Level authors produce that constantly, so settling walks a shape up a tile at a
time and refuses loudly rather than searching forever.
Two defects found by writing the tests:
- The fixture put an akgl_Tilemap on the stack and segfaulted before the first
assertion. It is about 26 MB -- the layer and tileset arrays are sized for the
worst case the format allows -- and tilemap.h says so. It is static now, with
a comment saying why, because the next person to write a map fixture will
reach for a local first as well.
- The far-edge nudge used AKGL_COLLISION_EPSILON, which is 1e-6. That is a
sensible tolerance on a unit vector and a meaningless one on a map coordinate:
`float` carries about seven significant digits, so at a coordinate of 144 the
smallest representable step is around 1.5e-5 and `144.0f - 1e-6f` is exactly
144.0f. The nudge did nothing, a box resting flush on the floor read as inside
it, and every move it tried looked blocked -- an actor standing on the ground
unable to walk. Two different quantities were sharing one constant; the tile
one is now its own, at a thousandth of a pixel, which is what the sidescroller
example independently arrived at.
Both breaks verified: removing the nudge and ignoring the `collidable` bit each
turn the suite red with the symptom named.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akgl_Partitioner is a record of function pointers and an initializer, the same
shape as the render and physics backends. `move` is its own slot rather than
remove-then-insert, and that is the whole design: a uniform grid's move is a
comparison and a return when the proxy has not left the cells it was in, which
is the steady state for a walking actor and the permanent state for a static
one. Spelling it as remove-and-insert would turn the incremental grid into the
rebuild-every-frame tree that PERFORMANCE.md already measured and rejected.
The grid is a dense 128x128 array of cell heads over the world, cells keyed on
tile size, with two intrusive chains per entry -- one through the cell so a query
can walk it, one through the proxy so removal unlinks a whole span without
searching. Everything is an int16_t index rather than a pointer, so the
structure is relocatable and clearing it is a memset. A proxy covering more
cells than AKGL_COLLISION_GRID_MAX_SPAN spills onto one chain every query walks,
which is what makes the cell pool's ceiling provable rather than hopeful.
tests/partition.c compares every query against a linear scan over the same
proxies and asserts containment in one direction, not equality. That asymmetry
is the contract: over-reporting costs a narrowphase call, under-reporting is a
wall an actor walks through. The suite is a table over partitioners so the same
assertions run against every implementation, which is what the second one
landing later will be held to.
Three deliberate breaks, and two of them were not caught the first time:
- Removing the min-cell rule so a shared-cell pair reports repeatedly: caught.
- Dropping the unlink in `move`: **not caught**, initially. Stale entries do not
produce wrong query answers, because the bounds re-check filters them out --
they leak the cell pool until the grid stops working, on a timescale no unit
test reaches by accident. Counting live pool entries across 200 moves is the
only thing that sees it, and now does: 505 entries against an expected 5.
- Swapping floorf for a truncating cast: **not caught, and correctly so.**
Everything below zero is clamped to cell 0 either way, so today the two are
indistinguishable. The comment claiming truncation is a defect was overstating
it and now says what is actually true: the clamp is the only thing hiding it,
and a negative world origin or a relaxed clamp would make it real with no test
standing in front of it.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akgl_collision_test takes two proxies and fills in a contact: a unit normal, a
depth, and a point. Three paths, cheapest first.
The proxies' bounds reject most pairs in four comparisons and no arithmetic,
which is free because the bounds were computed when the proxy was synced.
A box against a box is answered in closed form. That is not only cheaper than
the general solver, it is *exact*, and the difference shows up where it matters
most: an actor resting on a floor wants a normal of precisely (0, -1, 0), and an
iterative solver converges to something like (0.0001, -0.99999, 0), which
accumulates into a slow sideways creep. A tile game is almost entirely boxes, so
this is the path that runs.
Everything else goes to ccdMPRPenetration. MPR rather than GJK+EPA because MPR
allocates nothing at all, converges in fewer iterations, and its one weakness --
a coarser contact *point* -- is on a field the blocking resolver never reads.
EPA stays compiled and available behind the arena for a caller who one day wants
an accurate manifold. The header says the point is approximate so nobody builds
a damage falloff on it.
The normal points out of the second shape and toward the first, so a caller
moving `a` along it by `depth` separates the pair. libccd hands back the
opposite convention; converting once here saves every resolver from remembering
the sign, and getting it backwards would compile, would pass any "do these
collide" test, and would drag actors into walls. There is a test that asserts
the direction, and inverting the conversion turns it red.
AKGL_COLLISION_TEST_PLANAR flattens the normal into the xy plane. The extrusion
the setters apply already makes z the most expensive axis, so this is a net for
a caller who set a depth by hand -- and the failure it catches is silent: the
narrowphase reports a contact, the resolver pushes the actor into the screen,
and the actor does not move on screen while remaining inside the floor. It also
rescues the degenerate case, two shapes at exactly the same centre, where there
is no planar direction at all and a zero-length normal would be a wall that
stops nothing. Level authors stack things constantly.
Three things the tests found rather than assumed:
- The guard has two copies, one per path, and the box one is where it earns its
place. Removing both is caught; removing only the iterative one is not,
because the iterative solver is seeded from the line between the two centres
and so converges to a planar answer on its own for a planar offset. That is
measured and written down in the test rather than papered over with a fixture
contrived to force it.
- A concentric pair has no preferred sign on the degenerate axis, so the test
asserts the magnitude of the normal rather than its direction. The first
version asserted -1 and failed against an equally correct +1.
- The box fast path and the iterative solver are two implementations of one
answer, so they are driven over the same arrangements and required to agree on
the decision and the axis. A shortcut nothing cross-checks is a shortcut
waiting to diverge.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A proxy is what the broad phase will index: an actor's shape, where it is, and
the bounds that follow from those. It is a sixth heap layer, two per actor --
one for the actor itself and headroom for static geometry a game registers that
is not tile-aligned. Solid *tiles* deliberately get none: the broad phase will
read those out of the tilemap's own array, because one proxy per tile is tens of
megabytes to index data that is already a grid.
The pool follows the existing convention rather than improving on it.
akgl_heap_next_collision_proxy finds a free slot and does not claim it;
akgl_collision_proxy_initialize claims it. That is TODO.md item 8's asymmetry,
and it is kept on purpose: an acquire abandoned before initialization leaks
nothing, because the slot still reads as free, and a sixth pool with its own
rule would be worse than one defect with five instances. Fixing item 8 means
fixing all five together with every call site audited, and that is its own
change.
The proxy holds a *copy* of the shape rather than a pointer. An actor's own
logic may rewrite its shape mid-step -- a character crouching, a projectile
arming -- and a broad phase whose bounds came from a shape that has since
changed is a bug that only appears when two things happen in the same frame.
akgl_heap_release_actor now releases the actor's proxy. Without it the proxy
holds a borrowed `owner` into a slot that has just been zeroed, so the broad
phase keeps a registration whose owner reads as a free actor, and the next
contact against it is a collision with nothing. Verified by removing the release
and watching test_proxy_dies_with_its_actor go red.
The tests pin the convention as well as the behaviour: that two acquires with no
initialize between them return the same slot is asserted, not merely tolerated,
so that changing the convention has to change a test that says why it exists.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A shape is a convex volume positioned relative to an actor's origin. It lives on
akgl_Character, so every goblin sharing a character shares one shape definition
the way they already share speeds and the state-to-sprite map, and an actor may
override it with akgl_Actor::shape_override.
The flag is not redundant with an empty shape. "This actor deliberately has no
collider" and "this actor has not been given one yet" are different states, and
without somewhere to record the difference akgl_actor_set_character cannot tell
whether it is allowed to overwrite what it finds.
include/akgl/collision.h is a leaf: it names actors and tilemaps as incomplete
struct pointers rather than including their headers, because both of those need
akgl_CollisionShape by value and a cycle forms otherwise. The `headers` suite
compiles it as the first include of a translation unit, so that property is
enforced rather than merely intended -- which is why the tilemap types got struct
tags two commits ago.
The masks are asymmetric and the defaults are the load-bearing part: layermask
ACTOR, collidemask STATIC. An actor given a shape and nothing else collides with
map geometry and with no other actor. "Everything with a hitbox shoves everything
else" would be a surprising default for a town full of scenery and a painful one
to discover after the fact; opting in to actor-versus-actor is one bit, opting
out of it would have been a hunt through every NPC a game spawns.
Every shape gets a z depth, because the narrowphase behind this is three
dimensional and answers with the *minimum* separating translation. A thin
extrusion makes z the cheapest axis, and an actor pushed along z goes nowhere a
player can see while remaining inside the floor -- a collision system reporting
success while doing nothing. The ratio of 2 is the smallest for which the z
overlap of any two shapes the setters build provably exceeds any planar
penetration they can reach.
The test for that asserts the inequality across a spread of sizes rather than
asserting the constant is 2, so a change to the constant fails here rather than
in a game.
Two things about that test are worth recording, because the first version had
both:
- It could not fail. TEST_ASSERT expands to FAIL_BREAK, which reports by
`break`ing, and the assertion was inside a nested `for` -- so the break left
the loop rather than the ATTEMPT block and the failure evaporated. This is the
hazard AGENTS.md documents for CATCH; it applies to the whole family. Verified
by setting the ratio to 0.5 and watching the suite still pass, then fixing it
and watching it report the 512x512 case by name.
- tests/collision_arena.c had the same bug in a loop added in the previous
commit, and it is fixed here too.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
libakgl does not call malloc at runtime. That is stated in seven places in the
manual and is AGENTS.md's second standing rule, and every object comes from a
fixed array in akgl/heap.h. libccd's EPA path does not know that: it builds its
expanding polytope out of realloc and free, and ccdGJKPenetration documents a
-2 return for when that fails.
The alternative was to compile only the three files MPR needs and leave EPA out
of the build. That works and it puts a copy of somebody else's source list in
this repository, where it rots silently on the next submodule bump. This instead
compiles all of libccd and points its allocator at a bump allocator over static
BSS, which keeps the promise literally true -- and keeps EPA available rather
than amputated, for the day a precise contact manifold is worth having.
The arena is reset at the top of each query rather than freed block by block, so
the lifetime is one narrowphase call, `free` is a no-op, and allocation is a
pointer bump. Exhaustion returns NULL, which libccd already handles by unwinding
to -2, which becomes AKGL_ERR_COLLISION naming the high-water mark -- a loud
failure with a number in it rather than a silently missed collision, which would
be a floor an actor falls through reported as success.
One GJK/EPA box pair costs 7,264 bytes, measured and printed by the suite. The
64 KB ceiling is that with about nine times headroom, and the high-water mark is
reported so the next reader can re-derive it rather than trust this line.
The redirect lives in src/ccd_arena_shim.h, injected with -include, and not in
CMake's COMPILE_DEFINITIONS. It was in COMPILE_DEFINITIONS first, and that is a
mistake worth recording: CMake cannot carry a function-like macro through a -D,
so it dropped __CCD_ALLOC_MEMORY without a diagnostic. libccd went on calling
the C library's realloc while the shim's `free` quietly discarded the results --
strictly worse than doing nothing, and invisible, because it leaks rather than
crashes.
What caught it was insisting the test prove the wiring rather than the outcome.
The first version asserted the arena balanced back to zero after a query, which
turned out to be the wrong assertion for a different reason -- free is a no-op
by design, so it cannot balance -- but a test that had merely checked "two boxes
collide" would have passed throughout, against an allocator nothing was using.
The suite now asserts what is actually provable: that a query allocates from the
arena at all, and that the process survives, since glibc aborts when the real
free(3) is handed a pointer it never issued.
Also here: AKGL_ERR_COLLISION, with the name registered -- tests/error.c asserts
the band and the names agree, and it caught the missing one immediately.
-fvisibility=hidden and CCD_STATIC_DEFINE keep every ccd* symbol out of
libakgl.so's dynamic table, which `nm -D` confirms is empty; AGENTS.md records a
shipped defect where an exported `renderer` was preempted by a same-named symbol
elsewhere, and a game linking a system libccd would hit exactly that.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were sitting in deps/ untracked, with dangling .git pointer files into
.git/modules/{libccd,tg} and no .gitmodules entry at all. A clean clone of this
repository got neither, and `git submodule update` had nothing to update. tg's
worktree had also been emptied -- all 95 files staged as deletions inside its
own index -- so the directory held nothing but the pointer.
Both are now real submodules, pinned by SHA and checked against the tag:
deps/libccd 7931e764a19ef6b21b443376c699bbc9c6d4fba8 = v2.1
deps/tg 823f78db7e104ad5074288b4f362f0d3ef0fb0b6 = v0.7.9
Named "deps/libccd" and "deps/tg" to match the eight entries already there,
which meant re-registering rather than adopting the existing .git/modules
directories -- those were named "libccd" and "tg" while every other module lives
under .git/modules/deps/. Both checkouts were verified pristine at their tags
first, so nothing was lost by re-cloning.
URLs are https://, which is what both remotes already pointed at. Five of the
existing eight entries use git@github.com:, which a CI checkout with no SSH key
cannot use; that is not this commit's problem and is left alone.
tg is wired into nothing, deliberately, and TODO.md now says so with the reason
it is kept anyway -- indexed point-in-polygon against concave polygons is the
one job GJK and MPR structurally cannot do. That entry is a deadline: either the
consumer appears or the submodule comes out.
Nothing consumes either dependency yet, so the build and the full suite are
unchanged.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`typedef struct { ... } akgl_Tilemap;` declares no struct tag, so
`struct akgl_Tilemap` does not exist and no header can forward-declare it. That
is fine while every consumer includes tilemap.h, and it is a blocker the moment
a leaf header needs to name the type without pulling it in -- which the coming
collision header does, since tilemap.h reaches physics.h and a cycle forms
otherwise.
All four types in the file get a tag rather than just akgl_Tilemap: they are
siblings declared the same way in the same header, and tagging one of four is
the kind of inconsistency that makes the next reader wonder what is special
about it.
Layout-identical and no behaviour: a tag names a type, it does not change one.
Full suite, reindent and doxygen unchanged.
sprite.h and iterator.h declare their types the same way and are deliberately
left alone; nothing needs to forward-declare them yet.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It asked whether either rectangle enclosed one of the other's four corners --
eight akgl_collide_point_rectangle calls, stopping at the first hit. That is a
different question from "do these overlap", and it has the wrong answer for one
arrangement: a tall thin rectangle crossing a short wide one overlaps in a plus
sign with no corner of either inside the other, and all eight tests said no.
A long thin platform crossing a tall thin character is exactly that shape, so
this is a shape a 2D game produces, not a curiosity. util.h carried an @note
describing it and docs/18-utilities.md had a diagram of it, both under the
heading of a limitation rather than a defect, and there was no test for it at
all -- nor for full containment, nor for a shared edge.
It is four comparisons now, on both axes. `<=` rather than `<` because
akgl_collide_point_rectangle is inclusive on all four edges and these two have
always agreed that touching counts; a span test written with `<` would have
silently changed a contract both the header and the manual state.
The test was written first and failed on the cross before the fix went in.
Two answers change for a caller upgrading, and both are in the header note and
the chapter:
- The cross reports `true`, which is the point.
- The comparison is in float rather than through akgl_Point's int members, so a
sub-pixel overlap is no longer truncated away. A pickup test that was
accidentally forgiving by up to a pixel is no longer forgiving. Both tutorials
use this for coins and hazards; both still pass.
Faster as a side effect rather than a goal, and worth recording because the
numbers move a documented budget: 24.9 ns -> 6.1 overlapping, 57.9 -> 6.1
disjoint, and the all-pairs sweep over 64 actors 115 us -> 12.2. The disjoint
case gained most because it was the one that ran all eight tests before
answering.
The three moved rows are re-recorded in PERFORMANCE.md and nothing else is.
akgl_rectangle_points is untouched by this change and reads 6.1 in the same run
against the 4.0 recorded, so 6 ns is this run's floor and the new figure means
"too cheap to measure" rather than "exactly 6.1" -- said in the prose so the
next reader does not re-baseline the table around it.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sidescroller's controls did nothing. `akgl_controller_handle_event` matched
`event->key.which == curmap->kbid` exactly, with no way to say "whatever
keyboard the player is typing on", so the game did the obvious thing and bound
`SDL_GetKeyboards()[0]`.
That cannot work. The id a key event *carries* is chosen by the video backend
and is not required to be an id `SDL_GetKeyboards()` reports. On X11 without
XInput2 every key event carries `SDL_GLOBAL_KEYBOARD_ID`, which is 0, while
`SDL_AddKeyboard` registers the attached keyboard as `SDL_DEFAULT_KEYBOARD_ID`,
which is 1; with XInput2 the events carry the physical slave device's
`sourceid` while `SDL_GetKeyboards()[0]` may be the master. Either way the map
matched nothing and every key press was dropped.
A `kbid` or `jsid` of 0 now matches any device of that kind. 0 is safe to spend:
SDL documents `which` as 0 when the source is unknown or virtual, and joystick
ids start at 1, so no real device is 0. A non-zero id still matches only that
device, which is what keeps two local players on two keyboards apart, and there
is a test for that half too. `util/charviewer.c` already passed 0 and depended
on the old behaviour by accident; it works under XInput2 now as well.
Two reasons this shipped, both closed:
- Every test in tests/controller.c dispatched an event whose id equalled the id
the map was bound with, so none of them could see it.
- The sidescroller's smoke test called the control handlers directly instead of
dispatching events, so it passed against a control map that matched nothing.
It now pushes synthetic events through akgl_controller_handle_event from a
deliberately non-zero device id and fails if the press does not arrive.
Verified by breaking the binding and watching the run exit non-zero.
`test_controller_wildcard_device_ids` was written first and failed against the
unfixed library with "a map bound to keyboard 0 ignored a key press from device
11", which is the whole reason to trust it now that it passes.
The controls were documented, in one sentence. Chapter 19 has a proper table of
them, and chapter 15 explains why not to reach for SDL_GetKeyboards(). The
header said the match was exact and now says what it does.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Twenty-one chapters and two games were written against src/ rather than against
the header comments, and the exercise turned up two classes of problem. Both are
recorded here because publishing a problem you cannot fix yet is a contribution,
and because the second class is the more dangerous one: every item in it was
documented, in a header, incorrectly.
The one to fix first is defect 1. akgl_game_update calls
akgl_physics->simulate() with no NULL check and akgl_default_physics is zeroed
BSS, so a program that never calls an initializer segfaults on its first frame
-- measured, exit 139, not an AKERR_NULLPOINTER. physics.h tells the reader
akgl_game_init selects a backend from a `physics.engine` property, which is
false in both halves, so a caller who believes the header writes exactly the
program that crashes. That is the worst first-contact experience in the library
and the fix is a NULL check.
Defect 2 is the subtlest. A child actor's offset is counted twice: physics.c
writes x as parent->x + vx, an absolute coordinate, and actor.c then draws at
parent->x + obj->x while actor_visible three lines above treats obj->x as
absolute. Two readings of one field inside one function, and actor.h documents
both of them without noticing. Confirmed by rendering the frame with and
without the guard and hashing the readback.
The rest run from silent invisibility (an actor on layer >= 16 is updated and
simulated and never drawn; a map-spawned actor has no facing bit and so no
sprite) through dead API (speedtime is loaded, mis-cast, and read by nothing;
there is no way to play a sound effect from a file at all) to asset provenance:
tests/assets/World_A1.png and util/assets/Actor1.png carry RPG Maker's default
filenames with no licence file, and util/assets/littleguy.json -- the sample
data for the one demo the library ships -- does not load.
The second section lists the 27 header comments that describe code that has
since changed. Nothing catches these: WARN_IF_UNDOCUMENTED proves a symbol has
a comment, not that the comment is true, and check_api_surface.sh strips
comments precisely because prose is not a declaration. Doxygen publishes them.
A third section marks entries in this file that are themselves stale, including
item 15, which describes a leak that src/util.c already fixes with exactly the
technique the item proposes. AGENTS.md warns that this file "carried eleven
entries describing code that had already changed"; that is still accumulating,
and a premise nobody has re-checked is where the next defect is hiding.
plan.md is the plan the work was executed from, kept because it records the
decisions and, in three places, corrections to itself -- including getting the
error-code count wrong in the document whose purpose was fixing wrong
documentation.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
examples/sidescroller and examples/jrpg are complete programs, built with the
library and exercised headless by ctest. The chapters quote them with
`c excerpt=examples/...` blocks rather than restating the code, so a chapter
cannot drift from a program that compiles -- the excerpt check fails the moment
the source moves. 34 excerpts in one chapter, 21 in the other.
The two are complementary. The sidescroller is the physics tutorial: gravity, a
jump, coins, hazards. The JRPG is the content-pipeline tutorial: a town map,
NPCs spawned from map objects, four-way per-facing animation, a text box, a
follower.
Both smoke tests drive real SDL_Events through akgl_controller_handle_event and
step the physics clock at a fixed 1/60s rather than sleeping, so a scripted run
is deterministic and finishes in under five seconds.
Writing them is what turned up most of the defects recorded in the next commit,
because a game exercises paths a unit test does not. Each workaround says in the
chapter which library gap forced it:
- collision is written in a custom movementlogicfunc, because
akgl_physics_arcade_collide raises AKERR_API and akgl_physics_simulate never
calls collide at all;
- the sidescroller cancels the step's own gravity when it blocks downward,
because otherwise a quarter-pixel of penetration makes the *horizontal* sweep
report blocked and the character walks backwards a tile at a time;
- both clear movement_controls_face on every map-spawned actor, because the
default facefunc leaves a stopped actor with no facing bit, no sprite, and no
draw;
- the JRPG's follower gets a renderfunc that nulls obj->parent for the duration
of the draw, because a child's offset is counted twice.
Assets are CC0 from three Kenney packs, vendored with per-pack licence text,
per-file provenance, and the geometry contract in
docs/tutorials/assets/README.md. CC0 specifically rather than merely free: a
reader who copies a tutorial into their own game inherits no obligation.
scripts/fetch_tutorial_assets.sh refreshes them in the shape
mkcontrollermappings.sh was fixed into for 0.5.0 -- it checks curl's status,
refuses a pack page that does not say CC0, verifies the archive and the staged
dimensions, and leaves the tracked bytes untouched on any failure. Both failure
paths were tested, and a no-op refresh is byte-identical.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/ is a narrative manual, not a second reference. Every header already
carries a substantial @file/@brief block and Doxyfile sets WARN_IF_UNDOCUMENTED
with WARN_AS_ERROR, so an undocumented symbol already fails CI. The gap was
navigation and worked examples. Chapters teach a task and link to the Doxygen
output; where a declaration or a constant table has to be in front of the
reader it arrives as a `c excerpt=` block, so the text *is* the header and
cannot diverge from it. That also preserves the hand-aligned bit-flag tables
scripts/reindent.el goes out of its way not to destroy.
The manual does not re-document its dependencies. libakerror owns the
ATTEMPT/CLEANUP/PROCESS/HANDLE/FINISH protocol, SDL3 owns renderers and events,
Tiled owns the map format, jansson owns json_t. A chapter that restated any of
them would be wrong the day upstream changed and nothing here would notice --
the same drift this work exists to fix, arriving from a different direction. So
each chapter says what libakgl adds or constrains and links out for the rest.
Chapter 4 is the exception and the reason for it: libakerror documents the
mechanism, but only libakgl can say which statuses its own functions raise and
what they mean here, and that was written down nowhere. It carries three tables
-- libakgl's five status codes, the libakerror statuses libakgl actually raises
with their meaning in this library, and the exit-status trap where
`exit(AKGL_ERR_SDL)` is a wait status of 0 because the band starts at 256.
Every chapter was written against src/ rather than against the header comments,
which is how 27 false claims in those comments came to light. Where a chapter
documents a known defect rather than a design decision it says so and points at
TODO.md.
README.md keeps the development process and hands the reader to docs/. Its
task-oriented FAQ is deleted rather than moved, because one source of truth per
topic is the whole point and that FAQ's examples did not compile.
Census: 39 compiled snippets, 89 verbatim header excerpts, 4 JSON documents run
through the real loaders, one linked-and-executed program with its output
compared byte for byte, one generated figure. 11 norun blocks, each justified.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
libakgl exports 157 functions and the only user-facing documentation was a FAQ
in README.md whose examples did not compile: two unbalanced `PASS()` calls, a
`sprite->frameids = [0, 1, 2, 3];` that is not C in any dialect, a stray `9`
inside a bitmask expression, an `int screenwidth = NULL`, and -- in the first
snippet a reader ever saw -- the exact `strncpy` call AGENTS.md forbids.
That is not a reader routing around typos. It is what happens to samples that
nothing executes. This is akbasic's documentation harness with the interpreter
taken out and the ability to link and run put in.
The contract is a set of fence info strings, so an example is ordinary markdown
that still highlights on the forge:
```c compiled with -fsyntax-only -Wall -Werror
```c wrap=NAME the same, wrapped in tests/docs_preludes/NAME.pre/.post
```c run=NAME linked against akgl and run headless
```c excerpt=PATH must still appear verbatim in PATH
```c screenshot=NAME also the source of docs/images/NAME.png
```json kind=KIND loaded through the real akgl_*_load_json
```output the exact stdout of the runnable block above it
`run=` is why this links at all: -fsyntax-only proves a call typechecks, not
that the startup order works or that an ATTEMPT block gives back what it took.
`json kind=` exists because the asset formats are documented in prose and read
by four loaders with nothing tying the two together -- util/assets/littleguy.json
is already invalid against the loader it ships with.
A fence with no info string is a hard error, and so is an unknown one. The
failure mode this exists to prevent is passing because it quietly ran nothing,
so an unannotated block is a missing decision rather than a default. Exit status
is the number of failed examples; 2 for a usage error, which is a different
thing and has to be distinguishable.
Proven to fail on all ten of its failure modes -- untagged fence, non-compiling
snippet, stale excerpt, orphan output block, unknown info string, run= output
mismatch, invalid JSON, missing figure, a -Werror warning, and a dangling
preload= -- because a check that has never failed has not been tested.
`-Werror` here although AKGL_WERROR stays off for the library: that option is
off so a new compiler's diagnostic cannot break a consumer's build, and a doc
snippet is not a consumer. A sample that warns is a sample that teaches the
warning.
Registered as `docs_examples` and `docs_screenshots`. No docs-path filter in CI,
deliberately: documentation goes stale because the code moved, not because
somebody edited a chapter.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`scripts/reindent.sh` and `scripts/hooks/pre-commit` each carried their own
copy of the directory list, and neither included `tools` or `examples`. Both
hold ordinary hand-written C, so `--check` reported a clean tree without having
looked at them and the hook let them through unformatted -- which defeats the
one thing that keeps this tree a fixed point of `indent-region`.
The comment in `reindent.sh` already said the scope is "directories whose C
sources are hand-maintained", so this is the list catching up with the stated
intent rather than a new policy.
The two copies are now marked as being two copies of one decision, because they
have already disagreed once and nothing detects it when they do.
`scripts/reindent.sh --check` is clean with the wider scope.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Doxyfile` sets `WARN_AS_ERROR = FAIL_ON_WARNINGS` and CI builds the docs as a
step of `cmake_build`, so both of these failed the job:
- `akgl_character_initialize` documented `@param basechar` for a parameter the
declaration spells `obj`. This is exactly the mismatch AGENTS.md warns about
under Naming Conventions -- "parameter names must match between the
declaration and the definition, Doxygen publishes the header spelling" -- and
it cost two errors, one for the argument that does not exist and one for the
argument left undocumented.
- `require_at_eof` asked for an explicit link to `#AKERR_EOF`. That symbol is
libakerror's and `Doxyfile`'s INPUT is `include/akgl src`, so the link can
never resolve. Backticks say the same thing to a reader without asking
Doxygen to find a page it does not have.
`doxygen Doxyfile` now exits 0 with no diagnostics.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
New "The plan" subsection under Performance in TODO.md: nine entries in
blast-radius order. Six are next (string-pool free list, text texture ring
cache, non-raising sprite lookup, hot-path log demotion, the tileset scan
fix with its two latent draw bugs, the draw_world layer loop); three are on
record but unscheduled (uniform-grid broad phase behind target 12, the
realistic map fixture for target 13, the footprint refactor for 14 and 15).
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
New PERFORMANCE.md section putting the baseline next to what Construct and
Phaser publish about their own frames: their numbers assume GPU command
submission, so what transfers here is the culling and caching (text dirty
flags, collision cells), not the batching -- the control rows already showed
per-tile submission overhead at 0.2%. All claims cite primary sources.
Co-Authored-By: Claude Code (Claude Fable 5, claude-fable-5) <noreply@anthropic.com>
Every libakgl test suite could report success while failing. libakerror's
default unhandled-error handler ended in exit(errctx->status), an exit
status is one byte wide, and libakgl's band starts at 256 -- so
AKGL_ERR_SDL, the most common failure a library built on SDL can have,
exited 0 and CTest recorded a pass. tests/character.c aborted at its
second of four tests on a bad renderer and was green for months.
0.5.0 worked around that here with TEST_TRAP_UNHANDLED_ERRORS() in
tests/testutil.h, and TODO.md ended the entry saying any consumer's
suites have the same problem and it was worth raising upstream. It was.
2.0.1 fixes it at the source: akerr_exit() owns the 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). The
trap and its 21 call sites are gone.
Verified by putting the original failure back rather than by reading the
release notes: a FAIL_BREAK(AKGL_ERR_SDL) in tests/character.c's main
exits 125 and CTest reports a failure. A standalone consumer raising the
same status unhandled exits 125 where it exited 0 before.
tests/actor.c installs its own handler and called exit(errctx->status)
from it, which is the same defect one layer up. It calls akerr_exit()
now.
2.0.0 also makes the error pool and the status registry thread safe,
which libakgl needs more than it knew: audio_stream_callback raises
error contexts on SDL's audio thread. With an unlocked pool that
callback and the main thread could scan AKERR_ARRAY_ERROR at the same
time and be handed the same slot. The comment there says so.
This is a hard dependency floor, not a preference. 2.0.0 moved
__akerr_last_ignored to thread-local storage and made akerr_next_error()
return a context that already holds its reference, and both expand at
libakgl's call sites -- and at a consumer's, because akerror.h is part
of libakgl's public interface. Mixing headers and libraries across that
line double-counts every reference and never returns a pool slot. The
soname moved to libakerror.so.2; include/akgl/error.h now also feature-
tests AKERR_EXIT_STATUS_UNREPRESENTABLE, which is the narrowest probe
for 2.0.1 since libakerror publishes no version macro.
0.7.0 for that reason: libakgl's own ABI is unchanged, but the one it
re-exports through its headers is not.
TODO.md records the pkg-config gap this makes sharper -- akgl.pc names
no dependencies at all, so nothing tells a pkg-config consumer which
libakerror it needs.
Clean build, 26/26 ctest, memcheck clean, warning-clean at -Wall
-Werror. libakgl.so.0.7 links libakerror.so.2.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8T5FAYXE8HEJqFLCYwNNc
Updates deps/libakstdlib to 669b2b3. That commit is a TODO entry rather
than new code, so the interesting part is what libakgl was not yet
calling: it links libakstdlib and uses ten of its wrappers while calling
the raw libc function at a good many more sites. Four of those were
carrying a real defect.
akgl_game_save checked every write and threw away the close. Every write
goes through aksl_fwrite, and for a record this size all of them land in
stdio's buffer -- nothing reaches the device until the close flushes it,
so a full disk, an exceeded quota or an NFS server going away is
reported only there. The function returned success over a savegame that
was never written. aksl_fclose surfaces it. tests/game.c reaches the
path through /dev/full, which accepts writes and fails at the flush;
against the unfixed code the test reports "expected a failure, got
success" with ENOSPC.
The close has to drop the FILE * before the status is examined, because
fclose(3) disassociates the stream either way and CLEANUP would close it
again. Written the other way round it aborted in glibc with "double free
detected in tcache" the first time a close actually failed, which is how
that ordering was found.
akgl_game_load's end-of-file check used fgetc(3), which spells "end of
file" and "the read failed" with the same EOF -- a disk error there read
as a clean end and passed the check it was meant to fail. aksl_fgetc
tells them apart. Extracted to require_at_eof, because inverting the
sense of a call inline needs a nested ATTEMPT whose break binds to the
wrong switch.
Two snprintf sites were truncating under a silenced
-Wformat-truncation. The compiler was right about both. The tileset one
handed the shortened path to IMG_LoadTexture, so a length error was
reported as a missing file, with SDL naming a path the caller never
wrote. Both use aksl_snprintf now and the suppression is gone; nothing
in the tree uses those macros any more. tests/tilemap.c covers the join,
and against the unfixed code it gets AKGL_ERR_SDL where it wants
AKERR_OUTOFBOUNDS.
The two src/game.c strncpy sites the fixed-width copy sweep missed --
the savegame name field and the libversion stamp -- use aksl_strncpy and
sizeof rather than a repeated 32.
Also makes tests/physics_sim.c deterministic. It placed gravity_time at
"now - dt" and let the real clock supply the step, so a machine busy
enough to deschedule the process between that store and the
SDL_GetTicksNS() inside simulate got a longer step. It went red once,
under a parallel ctest. It now sets max_timestep to the step it wants
and gravity_time to zero, so the bound supplies the step and the
scheduler cannot reach it.
TODO.md records what is deliberately not adopted: the pure-arithmetic
wrappers, which can only fail on NULL and which the tree already spells
two ways, and the collections, which the registries do not want because
SDL owns the property-set lifetime. AGENTS.md has the rule and the
fclose ordering trap.
26/26 ctest, memcheck clean, warning-clean at -Wall -Werror.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8T5FAYXE8HEJqFLCYwNNc
tests/physics_sim.c runs the arcade backend the way a game does -- a
Mario-esque jump and fall, Zelda-style top-down walking, and a run
reversed at full speed -- and prints what the actor actually did.
tests/physics.c already checked that akgl_physics_simulate does the
arithmetic it claims. Every one of these got past it.
The first step was however long the level took to load. gravity_time was
never initialized and dt was unbounded, so a 250 ms load produced a
250 ms step: 100 px of fall where a 60 Hz frame under the same gravity
is 0.44 px, straight through whatever was underneath. Both initializers
seed the clock, and simulate bounds dt to max_timestep -- a new
physics.max_timestep property, default 0.05 s, read like gravity and
drag. Zero disables the bound. The field replaces the dead timer_gravity,
so the struct is the same size.
Releasing a vertical key cancelled gravity. The _off handlers zeroed ay,
ey, ty and vy together, and ey is where the arcade backend accumulates
gravity -- tapping down mid-jump stopped the character in the air. They
clear ax/tx (or ay/ty) and nothing else now. Velocity was never theirs
to clear either: simulate recomputes v as e + t every step.
Diagonal movement was 41% too fast. Thrust was capped per axis, so an
actor holding two directions got both caps at once and travelled their
diagonal. It is capped as a vector against the sx/sy/sz ellipse, which
also keeps a character whose horizontal and vertical top speeds differ
moving at the ratio it asked for. An axis with a zero max speed stays
out of the magnitude and is forced to zero, as the old clamp did.
Each fix was checked by reverting it and confirming the simulation goes
red: 100.1 px, vy 0.0 after a down tap, and 141% diagonal respectively.
tests/actor.c and tests/physics.c pinned the old behaviour in both
places and now assert the new contract.
Bumped to 0.6.0: akgl_PhysicsBackend changed. TODO.md records the four
things the simulations found and this does not fix -- no terminal
velocity, no deceleration on release, Euler's frame-rate dependence, and
a drag coefficient large enough to invert velocity.
26/26 ctest, memcheck clean, warning-clean at -Wall -Werror.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B8T5FAYXE8HEJqFLCYwNNc
Four gaps in the maintenance guide, each one something a maintainer will hit.
The two enforced checks were never named in it. api_surface
(scripts/check_api_surface.sh) and error_protocol
(scripts/check_error_protocol.py) fail builds and appear in ctest output, so the
guide now says what each one asserts and why -- including that the API check
strips comments before looking, because four of the nineteen symbols it found
were mentioned in header prose and that is not the same as being declared.
A new "Copying Into Fixed-Width Fields" section: never strncpy or memcpy into
one of this library's fixed-width fields, use aksl_strncpy, bound n at
sizeof(dest) - 1 to keep the truncate-not-reject contract, and know the one
place the NUL padding is load-bearing. Ten sites had the unterminated form and
one had the overreading memcpy form, and every name in this library is a
registry key.
The testing section gains the error-context ownership trap. TEST_EXPECT_* release
whatever the statement returns, and some functions return the context they were
given -- so a CLEANUP that also releases it double-releases, and a
double-released context corrupts the failure rather than reporting it. A test
written that way passes against broken code. That is not hypothetical; it is
what the first AKERR_OUTOFBOUNDS test did.
Also recorded: what ctest actually runs beyond the C suites, the gcovr
positional search path and why it must stay, and the surviving GCOV returncode 5
trap. Plus rebuild.log to .gitignore -- it had been sitting in git status long
enough to stop being noticed, which is the state a genuinely unexpected file
needs to stand out from.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TODO.md item 37 said the cast sweep buys nothing until the build turns on the
warnings those casts suppress. This is that precondition, and the argument
turned out to be right with evidence: -Wall found three genuine signedness
mismatches in sprite.c, where uint32_t width, height and speed were passed
straight to akgl_get_json_integer_value(..., int *). A cast would have silenced
all three. Fixing them properly also turned up that speed is scaled by a million
into a 32-bit field, so anything past 4294 ms overflowed rather than being held.
The other real finding was ten -Wstringop-truncation warnings, every one a
fixed-width strncpy. Those leave a name field unterminated when the input fills
it -- and those fields are registry keys, handed to strcmp and SDL property
calls that do not stop at the field. Same overread class fixed twice already
this release. All ten now use aksl_strncpy from akstdlib, which always
terminates and never NUL-pads, bounded to size - 1 so an over-long name still
truncates rather than being refused. staticstring.h documented the old strncpy
semantics explicitly and has been rewritten to match.
Two sites in game.c are deliberately left on strncpy: both stage into a
pre-zeroed buffer for the savegame's fixed-width fields, where the NUL padding
is part of the format. Neither is flagged.
sprite.c also had a memcpy of a full 128-byte field from a NUL-terminated
string, which reads past the end of any shorter name. Not flagged by anything;
found while converting its neighbours.
-Werror is an option, default OFF, on only in the cmake_build and performance CI
jobs. libakgl is consumed with add_subdirectory -- akbasic does exactly that --
so a target-level -Werror turns every diagnostic a newer compiler invents into a
broken build for somebody else. The warning set is not even stable across build
types here: an -O2 build reports ten warnings that -O0 and -fsyntax-only do not,
because they need the middle end. The two jobs that set it are one of each.
-Wextra is deliberately not adopted. It adds 22 findings, 17 inherent to the
design: 13 -Wunused-parameter from backend vtables and SDL callbacks that must
match a signature, and 4 -Wimplicit-fallthrough from libakerror's own
PROCESS/HANDLE/HANDLE_GROUP, which fall through by design. Filed upstream as
deps/libakerror TODO item 8; the submodule bump carries it.
deps/semver/semver.c is listed directly in add_library, so the target's PRIVATE
options reach it. Exempted with -w so a future semver update cannot fail this
build -- verified by forcing -Wextra -Werror and watching our files fail while
semver compiled clean.
Verified: RelWithDebInfo, Debug and coverage builds all clean under -Werror;
-Werror actually fails on a planted unused variable; an embedded consumer with
AKGL_WERROR unset still configures and builds. 25/25 pass, memcheck clean,
reindent --check, check_api_surface and check_error_protocol clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Upstream filed the per-axis scale item while the drawn-height-from-width bug
was still open, and recorded akbasic working around both together. That half is
fixed, so a non-square sprite now draws at its own proportions through the
library's own renderfunc. The uniform-scale half stands.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A "Fixing Defects" section in AGENTS.md: read what the code does before
deciding what it should do. Six rules, every one of them broken in this
repository during the 0.5.0 work, and every one cheap to check and expensive to
assume.
The one that prompted it: the savegame fix grew four name-width aliases to let
the on-disk format diverge from the object model, and akgl_game_load already
refuses a save from another build sixteen lines above the tables being edited.
The abstraction defended a case that cannot arise.
The rest are the same mistake in other clothes -- two tests that passed against
the unfixed library, a bitmask case that gives the same answer under both
parses, three CI exclusions resting on a premise nobody had re-checked, and
eleven TODO entries describing code that had already changed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
They were four #defines, one per table, each expanding to exactly one existing
constant and used exactly once per side. That is indirection with no second
consumer, and this repository's own rule is to abstract when the second
consumer is real and not before.
The reasoning behind them does not survive contact either. They were meant to
let the on-disk format's field widths diverge from the object model's -- but
raising AKGL_ACTOR_MAX_NAME_LENGTH is an ABI change, which bumps the version,
and akgl_game_load refuses a save whose libversion does not match before it
reads a single name table. The divergence they anticipated cannot happen
quietly.
The reader now names the same constant the writer does, per table, which is all
the fix ever needed to be. What actually catches a disagreement is the EOF
check at the end of akgl_game_load, and that works however the widths are
spelled: reverting the spritesheet reader to the wrong constant still fails the
roundtrip test with AKERR_IO.
25/25 pass, reindent --check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes Known-and-still-open items 7 and 13, and Defects items 18, 25 and 27.
The savegame name tables carry no length prefix, so writer and reader have to
agree on a field width exactly. The writer used each object's own maximum name
length -- 512 for a spritesheet, a filename -- and the reader used
AKGL_ACTOR_MAX_NAME_LENGTH for all four. Four AKGL_GAME_SAVE_*_NAME_WIDTH
constants drive both sides now.
The failure turned out to be worse than "cannot be read back": a reader
stepping the wrong width does not run off anything, it finds a run of zeros
inside an entry, stops early, and reports success with silently wrong maps. A
test asserting only that the load succeeded passed against the broken reader.
So akgl_game_load checks it is at EOF once the tables are read, which turns a
width disagreement into AKERR_IO instead of a corruption. That is the assertion
the new roundtrip test -- the first with all four registries populated -- hangs
on.
akgl_get_json_with_default gains a third HANDLE_GROUP for AKERR_OUTOFBOUNDS,
which is what the array index accessors report, so "this element is optional"
works for an array element and not only for an object member. The arm goes
above the one holding the memcpy: HANDLE_GROUP emits no break and every arm
falls into that body.
That test needed a second attempt. with_default returns *the context it was
given* when it does not handle the status, so TEST_EXPECT_OK -- which releases
whatever the statement returns -- double-released it against a CLEANUP block
that released it too, and a double-released context corrupts the failure rather
than reporting it. The first draft passed against the unfixed library.
akgl_text_rendertextat returns success without rasterizing for the empty
string, matching akgl_text_measure, which has always accepted it. The check
sits after the font and backend guards, so drawing nothing still refuses what
drawing something refuses. tests/text.c had this case written and unasserted
waiting for the two halves of the header to agree.
character_load_json_state_int_from_strings guards dest rather than testing
states twice. Not asserted: the function is static with one call site that
passes a real pointer, so the guard cannot fire, and reaching it from a test
would mean giving it external linkage purely for that.
Both gcovr invocations take the build tree as an explicit positional search
path. gcovr searches --root when given none, which is the source directory,
where build trees live; --object-directory does not narrow it. Verified by
building two instrumented trees with a source edit between them: the old
invocation fails with "Got function write_exact on multiple lines: 46, 48" and
exits 64, the new one exits 0 and the full coverage run passes with the stale
tree still present.
25/25 pass, reindent --check, check_api_surface and check_error_protocol clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both cited defects that are fixed. 17 is met outright; 16 is qualified, because
the tilemap cycle is the one that was leaking and the one now asserted, and
nothing yet asserts the same of a sprite, spritesheet or character cycle -- a
gap in the tests rather than a known leak.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A deliberate standalone commit, as AGENTS.md asks for this file.
Content is byte-identical -- 2255 mappings, not one changed -- which is the
point: it demonstrates the rewritten mkcontrollermappings.sh produces exactly
what the old one did on the success path. The only differences are the $(date)
stamp and the include guard, which is _AKGL_SDL_GAMECONTROLLERDB_H_ now so it
matches every other header in include/akgl/.
That rename was made in the generator rather than by hand, per the rule about
generated files, and the tracked copy carries it so the next regeneration shows
no spurious diff.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes Defects -> Known and still open item 12, both halves.
include/akgl/SDL_GameControllerDB.h is tracked on purpose: it is the offline
fallback that keeps the library buildable when upstream is unreachable. The
script that writes it had no set -e and never checked curl, so a failed fetch
wrote AKGL_SDL_GAMECONTROLLER_DB_LEN 0 and an empty array over the good copy
and exited 0. The safety net destroyed itself, and because CMake re-ran the
generator on every build, an offline build was enough to do it.
The script now 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. Any of
those failing leaves the tracked header exactly as it was and exits non-zero.
Verified against all five failure modes -- unresolvable host, 404, truncated
response, 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. AKGL_CONTROLLERDB_URL and
AKGL_CONTROLLERDB_MIN_LINES are environment-overridable, which is how.
The build no longer runs it at all. The add_custom_command declared a relative
OUTPUT, which CMake resolves against the binary directory while the script
writes to the source directory, so the declared output never appeared and every
build re-ran the command -- needing network access and leaving the tree dirty.
It is an explicit `controllerdb` target now, and the header is no longer listed
as a library source, which is all it was there for.
The empty-initializer problem goes with it: an empty array initializer is a
constraint violation in ISO C that compiles only as a GCC extension, and the
minimum-count check guarantees at least one entry.
Also here, because it rested on a premise that turned out to be false: the
character suite is no longer excluded from CI's coverage and memory-check jobs,
nor from the mutation harness by default. It was excluded on the grounds that
it failed deliberately. It did not -- it was reporting success while running
one of its four tests.
25/25 pass, memcheck clean, shellcheck clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes Defects item 26 and Performance item 32.
akgl_actor_render set dest.h from curSprite->width, so every actor was drawn
square and a non-square sprite was stretched or squashed. Invisible in the
fixtures because they are all square, so tests/actor.c gets a 48x24 sprite and
a render backend whose draw_texture records the rectangle it is handed instead
of drawing it. That recording backend is the first coverage akgl_actor_render
has had at all -- every other test in the file stubs renderfunc out.
akgl_game_update looped over AKGL_TILEMAP_MAX_LAYERS with the actor sweep
nested inside it and never compared an actor's layer to the layer it was on, so
every live actor's updatefunc ran sixteen times a frame. The sweep is hoisted
out: updating an actor is not a per-layer operation, and
akgl_render_2d_draw_world already walks the layers for the half that is.
AKGL_ITERATOR_OP_LAYERMASK is honoured rather than ignored now, so a caller who
wants one layer can still ask, and gets each of those actors once.
Counting is the assertion, deliberately. 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. tests/game.c counts calls into
a stub updatefunc and reports 16 against the old code.
PERFORMANCE.md records the timing side as what it honestly is: the gap between
the akgl_game_update and draw_world rows of the same run, 92 us before and
noise in both directions after. The absolute table is not re-taken -- a later
run on this machine read every row about 15% high, including rows nothing has
touched.
25/25 pass, reindent --check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eleven entries still described the old code: known-and-still-open items 1, 2,
5, 9 and 11, error-handling items 16, 17 and 18, and defects 20, 29 and 30. The
code and the tests were right; the document was not.
The edits were lost the same way each time. The scripts that made them built
the whole file in memory and wrote it once at the end, so an assertion failure
partway through -- on an entry whose text had already been reworded -- threw
away that script's earlier, correct replacements along with the failed one.
Written one entry at a time now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
83.9% line and 91.2% function, from 79.6% / 87.2%. src/tilemap.c moved the
most, 47% to 62%, because bounding the loaders and proving the pool accounting
needed them driven rather than merely called.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
akgl_game_update_fps installs the default itself now, so the benchmark no
longer has to. Leaving the hook NULL there is what keeps that true.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes Defects items 30 and 31 and Known-and-still-open items 1, 2, 5, 9 and 11.
Both string accessors in json_helpers.c ended their ATTEMPT block with
FINISH(errctx, false), which swallows the failure, and then strncpy'd through
the pointer akgl_heap_next_string never set. So the one condition the pool
exists to report -- it is full, which in practice means something is not
releasing -- arrived as a segfault somewhere else entirely. It is
FINISH(errctx, true) now, and tests/json_helpers.c claims every slot and
asserts AKGL_ERR_HEAP comes back out of both. That test segfaults against the
old code, which is also how the tilemap leak test in the previous commit
confirmed this one.
akgl_tilemap_release tested layers[i].texture and destroyed
tilesets[i].texture, so every tileset texture was freed twice on one release
and no image layer's texture was freed at all. Pointers are cleared as they go,
so a second release is safe instead of a use-after-free.
akgl_game_update_fps called game.lowfpsfunc() unguarded, on a path taken on
frame one because fps is 0 for the first second. Only akgl_game_init installs
it, and renderer.h documents the other path deliberately -- a host that owns
its window binds a backend instead. It installs the default when it finds NULL.
akgl_controller_pushmap and akgl_controller_default checked only the upper
bound, so a negative id indexed before akgl_controlmaps.
The two test-harness helpers were quietly worthless. akgl_render_and_compare
drew t1 on both passes, so it always reported a match and every image assertion
built on it asserted nothing; and akgl_compare_sdl_surfaces memcmp'd
s1->pitch * s1->h bytes out of both surfaces without checking that the second
was the same size, so a smaller one was read past its end. Both fixed, both
tested. tests/util.c also now calls the collide-point test it has defined and
never run.
25/25 pass, memcheck clean, reindent --check and check_error_protocol clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes Defects items 16 and 17 and Known-and-still-open item 6. All three let
an asset file, or a caller's argument, write past a fixed array.
akgl_sprite_load_json took its frame count straight from the document and wrote
that many entries into a 16-byte frameids -- through a uint32_t * cast of a
uint8_t *, so each write touched four bytes and the overrun reached four bytes
past the array, into the rest of akgl_Sprite and then the next pool slot. The
count is checked first now, each id is read into an int and narrowed
deliberately, and a frame number too large for a uint8_t is refused rather than
truncated into an index for a different tile.
The tilemap loader had the same shape twice: objects[j] with no check against
AKGL_TILEMAP_MAX_OBJECTS_PER_LAYER and tilesets[i] with none against
AKGL_TILEMAP_MAX_TILESETS. akgl_tilemap_load_layers already bounded its own
loop, so the pattern was in the same file. The object one is the reachable
half -- 128 objects is not a large object layer.
akgl_string_initialize zeroed sizeof(akgl_String) starting at `data`, which
begins after the refcount in front of it, so it ran four bytes past the end of
the object and onto the *next* slot's refcount -- the field the allocator reads
to decide whether a slot is free. Same file, same class, fixed with it:
akgl_string_copy accepted a count larger than the buffers, reading past one
pool slot and writing past another, which the header documented as behaviour.
Every case has a test that fails against the old code, with five new fixtures.
Exactly-the-maximum is asserted alongside one-past in each, so the bound cannot
be fixed by making the limit off by one.
25/25 pass, memcheck clean, reindent --check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes internal-consistency items 19 through 36, 38 and 41. Item 37, the ~180
redundant casts, is deliberately left open with its reasoning in TODO.md: the
benefit only arrives once the build turns on the warnings those casts suppress,
and doing it before that is churn across the two files with the most
outstanding functional defects.
The two that were real bugs are in the actor state table.
AKGL_ACTOR_STATE_STRING_NAMES was declared [AKGL_ACTOR_MAX_STATES+1] and
defined [32], so a consumer trusting the declared bound read past the object;
and indices 11 and 12 were named UNDEFINED_11 and UNDEFINED_12 where actor.h
has MOVING_IN and MOVING_OUT, so no character JSON could bind a sprite to
either state. 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.
The bitmask macros are parenthesized and AKGL_BITMASK_CLEAR has lost the
semicolon inside its body. Writing tests/bitmasks.c for that turned up
something worth knowing: the obvious test does not catch it. For a bit that is
set, the misparse `!(mask & bit) == bit` gives the same answer as the correct
one. It only diverges for an unset bit whose value is not 1, and that is the
shape the suite uses now.
akgl_draw_background was the last public function outside the error protocol.
It takes a backend like everything else in draw.h, restores the draw colour it
found, and is tested -- TODO.md had it filed under "needs the offscreen
renderer harness", which was never true; what it needed was to stop reading the
global.
All eight registry initializers go through one helper, so the seven that leaked
an SDL_PropertiesID on every call after the first no longer do. Fixed in the
same place because it is the same function: akgl_registry_init never called
akgl_registry_init_properties, which made akgl_set_property a silent no-op for
anyone not going through akgl_game_init -- Defects, Known and still open item 3.
Also: AKGL_COLLIDE_RECTANGLES (three open parens, two closes) and akgl_Frame
deleted, float32_t/float64_t used consistently, the developer-specific debug
logging removed from the controller inner loop, the abandoned SDL_GetBasePath
comments removed, nine unused locals removed, and dst renamed to dest.
akgl_game_update's default flags no longer OR the same bit twice. That changes
nothing today, and the reason is Performance item 32: the loop never reads
either bit, which is why every actor is updated sixteen times a frame. Still
open.
25/25 pass, memcheck clean, reindent --check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes internal-consistency item 18. The convention was settled -- AGENTS.md
records errctx winning 92 sites to 45 -- but the other 45 were still there, and
four files favoured `e` throughout while six favoured errctx, sometimes within
one file.
Its own commit because it is a rename and nothing else. All 333 changed lines
are a single identifier substitution: applying \be\b -> errctx to each removed
line reproduces the added line exactly, and the counts match at 333 either way.
`e` keeps its meaning where the convention actually wants it -- an incoming
error context being inspected, as in akgl_get_json_with_default(e, ...) -- which
is why that function was left alone.
25/25 pass, reindent --check 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 7 through 15. Nineteen non-static functions
were in the ABI with no declaration anywhere, so no consumer could call them
and any consumer could collide with them.
The four gamepad_handle_* functions are the ones that mattered: controller.h
declared akgl_controller_handle_button_down and three siblings that did not
exist, so anything compiled against the header alone failed to link. The
definitions carry the declared names now, which also closes Defects -> Known
and still open item 10, and their documentation moved to the header.
The rest are either declared under a "part of the internal API" block --
akgl_game_save_actors and akgl_game_load_versioncmp, which tests/game.c had to
declare for itself, plus six tilemap loader helpers the untested-loader work
wants to reach -- or static, which is what the four save iterators and
load_objectnamemap should always have been. akgl_path_relative_from is deleted:
declared nowhere, called from nowhere, never wrote its output, and leaked a
pooled string on every call, so it closes Known and still open item 4 and item
40 by ceasing to exist.
scripts/check_api_surface.sh keeps it closed. It reads the built library's
dynamic symbol table and every public header with comments stripped, and fails
on an exported akgl_* symbol that is declared nowhere. Stripping comments is
the whole point -- four of these were mentioned in controller.h prose, which is
how they went unnoticed.
The pool-size ceilings are defined once, in heap.h, so the #ifndef override
hook fires for the first time; actor.h, sprite.h and character.h were defining
the same four unconditionally from headers heap.h includes above its own guard.
tests/header_pool_override.c fails the compile if that regresses.
Also here: (void) rather than () on the twelve no-argument entry points,
AKERR_NOIGNORE only on declarations, static helpers with the akgl_ prefix
dropped, and the six parameter-name mismatches. akgl_get_json_with_default had
its two contexts swapped rather than merely misspelled -- the incoming one was
`err` and its own was `e`, which is the name reserved for an incoming one.
24/24 pass, 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>
akgl_Actor::scale is one float32_t applied to both dest.w and dest.h, so
"twice as wide, the same height" cannot be expressed. The VIC-II has had
separate x- and y-expand bits since 1982 and Commodore BASIC 7.0's SPRITE
verb exposes both, so a BASIC interpreter drawing through this library
cannot implement its own documented verb.
Reached from akbasic, whose sprites are 24x21. It works around this and
defect 26 together by installing its own renderfunc on each actor rather
than patching around the library.
Two other defects were filed alongside this one before the rebase and
both are gone: the drawn-height-from-width bug is already defect 26, and
the akgl_path_relative context leak was fixed in c622754.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CMake chains command overrides one level deep: a second override rebinds
_add_test to the first and the builtin becomes unreachable to everyone. So two
projects in one tree cannot both shadow add_test, and akbasic has to shadow it
first for libakerror and libakstdlib -- which left its own registrations
recursing to CMake's depth limit.
The unconditional shadow arrived with the vendored-dependency fix in 18399f2.
That half stays; only the guard comes back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It was reporting and shrugging. A definite leak, a read past the end of an
allocation or a branch on uninitialised memory now fails the build on the
push that introduces it, which is the only point at which it is cheap to
fix.
The six findings the job had on its first run were fixed rather than
excused, so the gate closes on a clean tree instead of on a backlog.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six findings, all of them libakgl's, all closed. The memcheck run is clean.
Not one of the four json_load_file calls in src/ was ever matched by a
json_decref, so every asset load abandoned its parsed document: 1.5 KB per
sprite, 2.1 KB per character, 9 KB per load of the 2x2 fixture map, and on
the order of a megabyte for a real one. Each loader now releases it in the
CLEANUP block it already had, on the success path as well as the failure
one. akgl_registry_load_properties needed its loop moved inside the ATTEMPT
block first: props is a borrowed reference into the document and is read
after the block ends, so every exit from that loop leaked the whole tree.
akgl_get_property copied a fixed AKGL_MAX_STRING_LENGTH bytes out of what
SDL handed back, which is SDL's strdup of the value -- four bytes for
"0.0". That read up to 4 KiB past the end of somebody else's allocation on
every property read, returned whatever was there past the terminator, and
would have faulted on a value that landed at the end of a page. It copies
the value and its terminator now, and refuses a value too long for an
akgl_String rather than truncating it into an unterminated buffer. The
header note that described the overread as a quirk describes correct
behaviour instead.
The four savegame name tables wrote a fixed-width field starting at the
registry key, and SDL sizes that allocation to the name. They read past it
on every entry and put what they found into the save file: up to half a
kilobyte of this process's heap per registered object, in a file a player
might send to somebody. They stage through a zeroed buffer now, and a
negative-array-size typedef fails the build if a table's width ever outgrows
it.
akgl_controller_list_keyboards never freed the array SDL_GetKeyboards
allocated for it.
A font could be opened and published and never handed back -- there was no
way to close one, so a game that changed fonts between scenes leaked ten
kilobytes each time, and loading over a live name leaked the font it
displaced. akgl_text_unloadfont is that missing half, and akgl_text_loadfont
calls it when it replaces a name, after the new font has opened so a failed
reload leaves the caller with the font they had. A new public symbol takes
the version to 0.4.0 and the soname with it: an 0.3 consumer cannot be
handed this library and told it is the same ABI.
tests/registry.c fills a destination with a sentinel and asserts the bytes
past the terminator survive a read, which fails against the old copy.
tests/text.c covers unload, double unload, unloading a name that was never
registered, and replacement closing the displaced font. The JSON releases
have no test of their own and cannot sensibly have one -- nothing in the
public API can observe a jansson refcount -- so the memcheck run is their
test, which is an argument for gating it rather than against.
The two remaining findings are in deps/semver's own unit test, which is
vendored. They are suppressed by function name, so a rewrite of those cases
comes back as a finding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The perf suites were already running in CI, because they are ordinary CTest
tests -- but only in the coverage job, which is a Debug build with gcov
instrumentation. That is the one place their numbers cannot mean anything,
and it was spending full-scale iteration counts to prove it. They now run
there at AKGL_BENCH_SCALE=0.02, for path coverage alone, which takes the
whole coverage run to four seconds.
The `performance` job is where the timings are taken: RelWithDebInfo, full
scale, `ctest -L perf`, which is the only configuration in which
tests/benchutil.h enforces a budget. It keeps the tables as an artifact
whether it passes or fails -- a red run says which measurement moved, and a
green one is the next baseline for PERFORMANCE.md. The step uses --verbose
rather than --output-on-failure, because --output-on-failure prints nothing
at all for a passing benchmark, and sets pipefail, without which the status
reaching the runner is tee's and a blown budget reads as a pass.
The `memory_check` job runs scripts/memcheck.sh over every suite under
valgrind and keeps the logs. It carries continue-on-error, and that is a
deliberate, temporary lie: the six findings in TODO.md "Memory checking" are
real and all of them are libakgl's, so gating on it today would only teach
everyone to ignore a red build. The flag comes off with the commit that
closes the last item, and item 34 -- the missing json_decref in all four
asset loaders -- is four lines.
Both new jobs were run locally against a clean copy of the tree with the
exact commands the workflow uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`cmake --build build --target memcheck` runs every registered CTest suite
under valgrind. There are no new test programs, and there should not be any:
tests/benchutil.h notices valgrind in LD_PRELOAD and drops the benchmark
scale to 0.0005, which turns the perf suites into the broadest path coverage
in the tree at a cost valgrind can survive. A benchmark walks one path a
hundred thousand times; a leak check wants every path walked once. Same
binaries, one flag apart, and the whole run is about thirty seconds.
scripts/memcheck.sh wraps `ctest -T memcheck` because that command records
defects and still exits 0, which cannot gate anything. It also forces the
headless drivers, so the vendor GPU stack is never loaded -- that removes
thousands of unfixable findings inside amdgpu_dri.so without suppressing
anything, and it is what the suites are written for anyway. Only definite
losses, invalid accesses and uninitialised reads count; "still reachable" is
what SDL and FreeType keep for the process lifetime and says nothing about
this library. The three suppressions in scripts/valgrind.supp are each a
decision that a finding belongs to somebody else.
Six defects, all filed in TODO.md under "Memory checking", the first of
which is the one that matters: not one of the four json_load_file calls in
src/ is ever matched by a json_decref, so every asset load abandons its
parsed document -- 1.5 KB per sprite, 2.1 KB per character, 9 KB per load of
the 2x2 fixture map, and on the order of a megabyte for a real one. A game
that reloads a level on death leaks a level's worth of JSON every time.
akgl_get_property also reads up to 4 KiB past the end of every property
value it copies, and the savegame name tables read past the end of every
registry key and write what they find into the file.
tests/util.c zeroes three fixtures it used to leave as stack garbage. The
library was never at fault there -- the tests handed it uninitialised floats
and then made one real call with them -- but sixteen findings of noise in a
new gate is how a gate gets ignored.
The unit suites' TIMEOUT goes from 30 to 300 because CTest applies the same
property to the checked run, where everything is twenty times slower.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The six defects the perf suites turned up were written out in full in both
files. TODO.md is where a defect belongs -- file, line, functional
consequence, blast radius, what closing it touches -- and duplicating that
in a report guarantees the two drift, with no way to tell which copy is
current.
PERFORMANCE.md keeps what is actually its argument: that stress testing
reaches failures a suite calling each function a handful of times does not,
and that two of the six kill the process rather than slowing it down. The
detail is one reference away.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tests/perf.c and tests/perf_render.c drive the hot paths hard enough to
time them -- pool acquire and release at both ends of the scan, the
registry, the state-to-sprite lookup, the per-actor update and render, the
physics sweep, an all-pairs collision sweep, the JSON accessors, path
resolution, the drawing primitives, text, asset loading, a screenful of
tiles, and a whole frame through akgl_game_update. They are registered like
any other suite but carry the `perf` label, so `ctest -L perf` runs only
them and `-LE perf` leaves them out. Every measurement is held to a budget
at roughly ten times the recorded baseline, enforced only in an optimized
build at full scale.
Three things had to be got right before the numbers meant anything, and
each was wrong first:
- No error checking inside the clock. PASS and CATCH call
akerr_valid_error_address, which walks AKERR_ARRAY_ERROR -- more work than
several of the calls being measured.
- Flush the renderer before stopping it. SDL batches, so the first version
measured queueing, reported a tilemap frame 250 times faster than it is,
and paid the real cost at teardown inside SDL_DestroyTexture.
- A drawing benchmark needs a raw-SDL control doing the same pixel work
with the same access pattern. With one, akgl_tilemap_draw turns out to
cost 0.2% of the frame it appeared to own: 16.26 ms against a control's
16.23 ms for the same 1200 blits. The rasterizer is the frame.
PERFORMANCE.md records the baseline, the frame budget it adds up to, and
what the numbers say -- including that the string pool's acquire is 64x
slower full than empty because it is a megabyte of PATH_MAX buffers, that a
handled missing-sprite condition costs nine times the update it replaces,
that text rasterizes and throws away a texture on every call, and that 28 MB
of the library's BSS is one akgl_Tilemap.
TODO.md gains a Performance section: five defects the stress tests found
that the unit suites do not reach (a tilemap load leaking five pooled
strings, two JSON accessors that turn pool exhaustion into a segfault
rather than AKGL_ERR_HEAP, akgl_game_update crashing without
akgl_game_init, and its actor update sweep running once per tilemap layer),
and eighteen targets with today's number and whether it is met.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akgl_path_relative took its ENOENT branch by returning from inside the
HANDLE block, which skips the RELEASE_ERROR that FINISH ends with. The
handled context was never given back, so 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.
That branch is not an edge case: every asset path inside a tilemap is
resolved relative to the map's own directory, so it is taken several times
per map load. A game that loaded fifty levels died in the loader. It was
found by a benchmark that loaded the fixture map in a loop, which is the
first thing in this tree to call it more than a hundred times.
The fallback now runs after FINISH, so the context is released on the way
past. tests/util.c resolves AKERR_MAX_ARRAY_ERROR * 2 paths through the
branch and asserts the pool is where it started -- against the old code
that test does not fail, it terminates the suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All ten items under "API gaps blocking akbasic" are closed. Items 7, 9 and
10 add public symbols and item 9 adds three fields to akgl_AudioVoice, so
the version and the soname go with them -- an 0.2 consumer cannot be handed
this library and told it is compatible.
Also refreshes the coverage numbers against a fresh run (79.6% line, 87.2%
function, all 21 suites), records the mutation smoke runs over the two files
that changed most, and files one new defect the text tests turned up:
akgl_text_rendertextat refuses the empty string while akgl_text_measure
accepts it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
controller.h declares three handler pointers taking an akgl_Actor * and
included nothing that declares that type, so any translation unit reaching
for it before akgl/actor.h failed with "unknown type name 'akgl_Actor'".
src/controller.c never noticed because it includes akgl/game.h first, and
tests/controller.c carried a comment explaining the workaround.
Included rather than forward-declared: akgl_Actor is a typedef of a named
struct and repeating a typedef is C11, not C99. It closes no cycle --
actor.h reaches only types.h and character.h.
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; covering another header means
another file shaped like this one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ring held a keycode and dropped the rest of the event, so a line editor
built on akgl_controller_poll_key() could not reach a single shifted
character: no double quote, no colon, no parenthesis, and no lower case.
The modifier state was discarded two layers below the caller, and polling
SDL_GetModState() at read time cannot recover it -- the key is long
released by then, which is the point of a ring.
akgl_Keystroke carries the keycode, the modifiers and the UTF-8 text SDL
composed, and akgl_controller_poll_keystroke() hands back all three.
akgl_controller_poll_key() is unchanged for its callers: a game asking
whether the up arrow was pressed already gets what it wants.
The text comes from SDL_EVENT_TEXT_INPUT, which is the only correct way to
get a character out of SDL, and is attached to the press still waiting for
it -- tracked with a flag rather than by "whatever entry is newest", so a
press dropped by a full buffer cannot hand its text to an older key. Text
with no press behind it (an IME commit, a dead key) is buffered on its own
with keycode 0, and the keycode-only poller discards those on the way past.
Oversized text is cut on a code point boundary, and a sequence the string
ends in the middle of is dropped rather than read past its terminator.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BASIC's SOUND ramps a tone's pitch toward a limit, and nothing here could
do it. Faking it from the caller means re-issuing tones from its own step
loop, which ties audible pitch to how often that loop runs -- a siren that
changes shape with the frame rate. The step has to be taken on the mixer's
frame counter, which only this library can see.
akgl_audio_sweep(voice, from_hz, to_hz, step_hz, ms) steps once every
1/60 second, the rate the machine this vocabulary comes from used, which
divides 44100 exactly so a step always lands on a whole frame. Direction
comes from the two frequencies rather than the sign of the step, the last
step clamps to the target, and equal frequencies are a held tone rather
than an error. akgl_audio_tone() is the same start path with a step of 0,
so a voice reused for a plain tone stops sweeping.
A swept voice accumulates its phase instead of deriving it from the frame
counter: deriving assumes a constant frequency and would jump the waveform
at every step. tests/audio.c drives the mixer by hand and asserts the exact
frame the pitch moves on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akgl_text_rendertextat() called renderer->draw_texture() with no check on
the global renderer, its sdl_renderer or the function pointer itself, so a
backend that has an SDL_Renderer but was never bound segfaults on the first
line of text -- the state akgl_render_bind2d() exists to get a host out of.
All three are now checked, with AKERR_NULLPOINTER as the draw entry points
report, and checked before anything is rasterized rather than after.
tests/text.c grows a software renderer with a bound backend and covers all
three refusals plus the successful draw, wrapped and unwrapped: src/text.c
goes from 58% to 100% line coverage. A made-up SDL_Renderer pointer is not
enough to test this -- SDL refuses the bogus handle on its own and the call
fails for the wrong reason, which a deleted check survives.
Also documents what the tests found: SDL_ttf refuses the empty string, so
drawing an empty line is an error while measuring one is not. Filed in
TODO.md rather than pinned in the suite.
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>
Embedded with add_subdirectory(), the deps/ submodules a recursive clone
just checked out were ignored and find_package(SDL3 REQUIRED) ran instead,
so configure failed on a machine with nothing installed. Each dependency is
now added by akgl_add_vendored_dependency(), which skips it when the target
is already declared or the submodule is missing; the find_package lookups
run afterwards for whatever is left. The CTest suppression moves with them
and is lifted before this project registers its own suites.
Two consequences of the move: pkg-config is only looked for when something
has to come from the system, and the build-tree RPATH keys on whether
anything was vendored rather than on being the top-level project, so an
embedded build's tests can find the satellite libraries too.
Verified by configuring and building a consumer that embeds this repository
with CMAKE_DISABLE_FIND_PACKAGE_* set for all seven dependencies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akbasic built a line editor on akgl_controller_poll_key() and found the ring
carries a keycode and nothing else. No shifted character is reachable, so a
BASIC string literal cannot be typed at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Writing an honest @throws list means reading the implementation against the
contract its header claimed, and the two disagreed in more places than the
status codes. Recorded as items 14-25 under Defects, ordered by blast radius,
each with file, line, consequence and what closing it would touch. They are
also noted inline as @note or @warning on the function concerned, so a reader
of the generated documentation finds them without coming here first.
The three that matter most:
- akgl_path_relative returns from inside its ENOENT handler, skipping the
RELEASE_ERROR that FINISH carries, so it leaks one of libakerror's 128
error-context slots per call. That is not a rare path - it is the ordinary
one, taken whenever an asset names a neighbour relative to its own file.
- akgl_sprite_load_json writes json_array_size() entries into a 16-element
frameids array with no bound check, and does it through a uint32_t * cast of
a uint8_t *. The same unbounded shape appears twice more in the tilemap
loader, for objects per layer and tilesets per map.
- akgl_heap_release_character zeroes the character without walking its
state-to-sprite map, so every sprite reference it took is lost and the SDL
property set holding the map is leaked. Releasing characters between levels
exhausts the sprite pool. akgl_character_state_sprites_iterate exists to do
that walk and is not called from there.
Also: the background music never loops, because MIX_PROP_PLAY_LOOPS_NUMBER is
set on property set 0 rather than one akgl_load_start_bgm owns; and
akgl_get_json_with_default defaults on AKERR_INDEX while the array accessors
report a short array as AKERR_OUTOFBOUNDS, so the optional-element form
silently does not work for arrays.
No code changes. Every src/*.c:NNN citation in the new section was checked
against the file after the preceding commit shifted the line numbers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Doxygen comments were generated from the declarations, so 217 @throws
lines across 21 headers read "When the corresponding validation or operation
fails" and told a caller nothing beyond the status name. The @param lines were
the same shape: every output was "Output destination populated by the
function", every instance "Object to initialize, inspect, or modify".
Rewritten against the implementations, following the pattern libakstdlib
already uses:
- @throws names the condition. akgl_sprite_load_json separated AKERR_KEY
(absent) from AKERR_TYPE (present, wrong type) from AKERR_OUTOFBOUNDS
(filename too long, or array indexed past its end), and gained AKGL_ERR_SDL
and AKGL_ERR_HEAP, which it raises and never declared.
- Parameters say whether they are required, what a NULL means, and what is
written on a failure path. Where an argument is not checked, the doc says so:
akgl_heap_next_actor's dest is a crash on NULL, not an error, and
akgl_render_2d_frame_start dereferences self before testing it.
- The conventions move up to the file blocks so the per-function docs stay
short. json_helpers.h states once that absence is an error here and that
json_t * results are borrowed; heap.h explains the pool model and the
acquire asymmetry; physics.h carries the thrust/environmental/velocity table.
- Struct fields, enum values, macros and exported globals are documented,
including the dead ones - sprite_w/sprite_h, movetimer, p_scale and
timer_gravity are read by nothing, and say so.
Also fixes ten comments in error.h and audio.h that opened with /** rather
than /**<, so Doxygen attached them to the following entity and rendered the
text as part of the macro's value. Verified against the generated HTML.
Comments only - no declaration changed. Doxygen builds clean under
WARN_AS_ERROR, scripts/reindent.sh --check passes, 19/19 suites pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akbasic has now consumed all four of the API gaps this file listed as blocking
it: its text sink, graphics backend, audio backend and input backend are written
and tested, and the BASIC 7.0 graphics, sound and console verbs work against
them. Doing that turned up six things worth fixing here -- five of which still
are.
Four are workarounds akbasic had to write to build at all, and each is commented
at its site over there with the words "filed upstream":
- An embedded libakgl requires SDL, SDL_image, SDL_mixer, SDL_ttf and jansson to
be installed, while vendoring all five. The dependency block is gated on being
top-level, so an add_subdirectory() consumer takes the find_package path and
fails with the submodules sitting right there in deps/.
- include/akgl/controller.h does not compile on its own: it declares handlers
taking an akgl_Actor * and includes nothing that declares the type. src/
never notices because it includes game.h first.
- akgl_render_init2d() fuses two separable jobs -- creating a window, and
installing the backend vtable -- so a caller who already owns an SDL_Renderer
has no way to get the second without the first. That caller is exactly the
embedding host this section exists to serve.
- akgl_text_rendertextat() dereferences renderer->draw_texture with no check,
which is the state item 7 leaves a host in. Same class of defect the draw
commit added a test for; the text path still has it.
The fifth is a real API gap: SOUND's frequency sweep has no equivalent, and it
cannot be faked in the interpreter without tying audible pitch to the host's
frame rate, so it has to be advanced on the mixer's own frame counter.
The sixth was that 42b60f7 shipped 22 public symbols under an unchanged VERSION
and soname. 1066ac7 fixed that independently while this was being written -- the
two crossed rather than one following the other -- so it is filed as resolved
rather than dropped. The reason is the part worth keeping: akbasic could not
feature-test for the new API and had to pin libakgl by submodule commit in its
README until that landed, which is the concrete cost of an additive release
under an unchanged soname.
All five outstanding items were re-verified against this tree after the rebase,
not just carried forward: controller.h still includes neither actor.h nor a
forward declaration, render_init2d still calls SDL_CreateWindowAndRenderer
before installing the vtable, text.c:62 still reaches through draw_texture
unchecked, the vendored-dependency block is still gated on being top-level, and
there is still no akgl_audio_sweep.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Require libakstdlib 0.2 when consuming an installed dependency, matching the API adopted by the preceding commit.
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Pass fixed buffer sizes to the bounded formatting and realpath wrappers. Route save-game records through exact-transfer helpers so the required fread/fwrite count parameter stays local to the complete-record contract.
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
A mutation smoke run scored src/audio.c at 50% and src/draw.c at 70% and named
three real holes:
- Nothing asserted that an unconfigured voice is audible, which is the entire
reason the voice table defaults to a square wave at full level instead of a
zeroed struct. Deleting the defaults survived.
- No envelope test used a non-zero attack and decay together, so the decay
measuring from the start of the note rather than from the end of the attack
survived.
- The circle was only checked at its four axis points, which a mis-signed
octant reflection survives. It now checks that every plotted pixel has a
mirror in the other three quadrants.
Also adds a backend that exists but was never given an SDL_Renderer, which is
the state a host is in between allocating one and initializing it; every draw
entry point has to report it rather than dereference it.
audio 50% -> 75%, draw 70% -> 90%. The survivors are recorded in TODO.md and
are honestly untestable from here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
77.2% line and 83.1% function coverage across 2637 lines, up from 72.2% / 78.6%,
with the three new suites at 91% (audio), 95% (draw) and 58% (text). All 19
suites pass: the note recording character as an intentional failure was stale,
and neither tests/character.c nor src/character.c has changed since it was
written.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GCC cannot see that it is only read on a path where flood_region() wrote it,
and warns under -Wmaybe-uninitialized.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was no audio API at all: SDL3_mixer was vendored and AKGL_REGISTRY_MUSIC
was declared, but nothing opened a mixer, and a mixer plays recordings anyway.
BASIC 7.0's SOUND, PLAY, ENVELOPE and VOL describe notes, so this adds a tone
generator -- three voices, five waveforms, an ADSR envelope each, mixed to one
float stream and fed to an SDL_AudioStream.
The voice table works whether or not a device is open. akgl_audio_init connects
it to one; a host that owns its own audio pipeline can call akgl_audio_mix
instead. That is also what makes the tests deterministic: a device pulls samples
on SDL's audio thread whenever it likes, so tests/audio.c mixes by hand and only
opens a device in its last test.
Phase is computed from the frame counter rather than accumulated, because a
float increment of hz/44100 added 44100 times a second walks a held note off
pitch. A voice that has never been configured defaults to a square wave at full
level, since a zeroed voice would have a sustain of 0.0 and make no sound with
no error to explain why. Voices summing past full scale are clamped rather than
scaled, so one voice plays at the level it was asked for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BASIC 7.0's graphics verbs -- DRAW, BOX, CIRCLE, PAINT, SSHAPE and GSHAPE -- all
plot against the current screen right now rather than adding to a scene, and
draw.h had exactly one function in it. This adds akgl_draw_point, _line, _rect,
_filled_rect, _circle, _flood_fill, _copy_region and _paste_region, each taking
the akgl_RenderBackend the host already initialized.
Color is an argument rather than a current-color global, so there is no second
copy of state to disagree with the caller's own, and each call restores the
renderer's draw color when it is done. SDL3 has no circle and no flood fill: the
circle is a midpoint circle, and the fill reads the target back, walks the
region with a fixed 4096-entry span stack, and blits back only the bounding box
of what changed. Exhausting that stack reports AKERR_OUTOFBOUNDS and says in the
header that the region is left partially filled.
tests/draw.c draws into a 64x64 software renderer under the dummy video driver
and reads the pixels back, so it needs no display and no offscreen harness.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akbasic's GET and GETKEY ask "is there a keystroke waiting, yes or no" and must
not require the interpreter to pump SDL events itself. akgl_controller_poll_key
drains one key per call from a fixed 32-entry ring that
akgl_controller_handle_event fills, so the host keeps pumping and the embedded
interpreter reads at its own pace. akgl_controller_flush_keys discards a
backlog.
The recording happens before the control-map scan, not after: that scan returns
as soon as a binding claims the event, so recording afterwards would have
dropped exactly the keys a game also acts on. A full buffer refuses the newest
keystroke rather than overwriting the oldest, so a caller reading a line gets
what was typed first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akbasic needs the advance width of one character cell to build a terminal-style
text surface, and the wrapped size to know where a string crosses the right
margin. akgl_text_measure and akgl_text_measure_wrapped report both over
TTF_GetStringSize and TTF_GetStringSizeWrapped; neither needs a renderer, so
this half of the text subsystem is testable without the offscreen harness.
A negative wrap length is refused with AKERR_OUTOFBOUNDS: SDL_ttf reads it as a
very large unsigned width and quietly stops wrapping, which would return a
measurement that is wrong rather than an error the caller can see.
Also fixes akgl_text_loadfont, which checked name twice and passed an unchecked
filepath to TTF_OpenFont (TODO item 39).
The fixture font is a 10 KB monospaced ASCII subset of Liberation Mono, renamed
because "Liberation" is a Reserved Font Name; see
tests/assets/akgl_test_mono.LICENSE.txt. Monospaced so the suite can assert
width("AAAA") == 4 * width("A") rather than hardcode glyph metrics.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
akbasic is a C port of the BASIC interpreter being built to link into libakgl as
a scripting engine for game authors. Its interpreter core is complete and passes
its whole acceptance corpus against a stdio text sink; four gaps here block the
libakgl-backed sink and the graphics, sound and console verbs of Commodore
BASIC 7.0.
Filed rather than worked around downstream, because growing libakgl to serve a
consumer is the wanted outcome. Each entry says what the BASIC verb needs, what
the akgl_* entry point should look like, and what would cover it.
Only the first blocks work already designed and waiting. text.h can load a font
and render a string and cannot say how large that string will be, so there is no
way to build a terminal-style surface on it -- a cursor needs the advance width
of a cell and wrapping needs to know where a string crosses the margin. The
reference interpreter derived its whole character grid from SDL2_ttf's
SizeUTF8("A"). akgl_text_measure() over TTF_GetStringSize closes it.
The other three block verbs not yet started: draw.h declares one function and
src/draw.c is at 0%, so DRAW/BOX/CIRCLE/PAINT have nothing to plot with; there
is no audio API at all despite SDL3_mixer being vendored, and PLAY/ENVELOPE want
a synthesised voice SDL3_mixer does not provide; and controller.h is built
around event handlers the host pumps, which suits a game loop but not GET/GETKEY
-- those ask whether a keystroke is waiting and must not require the interpreter
to own the event loop, which goal 3 forbids anyway.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Defects #13: gcovr searches for gcov data under --root, which
CMakeLists.txt:208 and :223 set to the source directory, so any
instrumented build tree left there is merged into the report. The
--object-directory argument beside each does not narrow the search; it
only maps gcda files back to the compiler's working directory.
A leftover build-coverage/ therefore fails coverage_reset before any
test runs, with gcovr unable to reconcile a function reported on two
different lines. The build*/ entry in .gitignore keeps the offending
tree out of git status, so the state is easy to get into and hard to
spot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bump deps/libakstdlib five commits to 95e5002, which versions the library
at 0.1.0 with an soname and a ConfigVersion file, upgrades its own
libakerror to 1.0.0, namespaces its coverage target when embedded, and
raises its libc-wrapper coverage. The public header diff is purely
additive -- nothing removed, no signature changed -- so libakgl's nine
aksl_* call sites needed no edits. What broke was the build.
A default `cmake -S . -B build` stopped configuring:
add_executable cannot create target "test_version" because another
target with the same name already exists. The existing target is an
executable created in source directory ".../deps/libakstdlib"
The versioning commit added tests/test_version.c to libakstdlib.
EXCLUDE_FROM_ALL keeps that target from being built, but
add_subdirectory still creates it, so it collided with the version suite
added in the preceding commit. Build libakgl's test programs as
akgl_test_<name> instead. The CTest names are unchanged -- ctest -R
version still selects it -- and a dependency is now free to ship a test
of any name.
Ask find_package for libakstdlib 0.1 rather than any version. 0.1.0
ships akstdlibConfigVersion.cmake with SameMinorVersion compatibility,
mirroring its libakstdlib.so.0.1 soname, so this accepts any 0.1.x and
refuses 0.2 and 1.0. Unversioned, the non-embedded path would have taken
an ABI-incompatible libakstdlib without complaint.
libakerror is deliberately left unversioned in find_package. It installs
akerrorConfig.cmake and akerrorTargets.cmake but no
akerrorConfigVersion.cmake, so find_package(akerror 1.0) fails against a
correct install rather than a stale one. Its floor stays with the #error
in include/akgl/error.h. Worth fixing upstream.
Verified: clean configure, build and 16/16 ctest; a -DAKGL_COVERAGE=ON
tree configures, builds and passes 18/18, so libakstdlib's embedded
coverage-target rename does not collide here. Against a temp-prefix
install of 95e5002, find_package(akstdlib) accepts 0.1 and 0.1.0 and
refuses 0.2 and 1.0, and find_package(akerror 1.0) fails for the missing
version file as described.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first .gitignore entry was "./build/*". A leading ./ is not a valid
gitignore pattern, so it matched nothing and build/ turned up as
untracked in every git status. Use build*/, which also covers the
instrumented trees scripts/coverage.py and -DAKGL_COVERAGE=ON create.
Nothing under a build* path is tracked, so this hides no existing file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The version was written down twice and agreed with itself by luck.
AKGL_VERSION was hand-maintained in include/akgl/game.h, and nothing
connected it to the build, which had no version at all:
- project() declared no VERSION, so the @PROJECT_VERSION@ substitution
in akgl.pc.in expanded to nothing and every installed akgl.pc shipped
an empty Version field. pkg-config --modversion returned "" and
--atleast-version could not work at all.
- libakgl.so had no VERSION or SOVERSION, so there was no soname and no
symlink chain. A stale libakgl could be paired with new headers
silently, which is the failure libakerror added its own soname to
prevent.
- MY_LIBRARY_VERSION at CMakeLists.txt:288 was never set anywhere. It
built main_lib_dest as "lib/akgl-", and main_lib_dest was then never
read. Dead line, removed.
project(akgl VERSION 0.1.0) is now the only place the number appears. It
drives the generated include/akgl/version.h, the shared library VERSION
and SOVERSION, and the Version field in akgl.pc. Held at 0.1.0 to match
the constant it replaces, so akgl_game_load_versioncmp still compares
savegames against the same value.
include/akgl/version.h is generated by configure_file from version.h.in
and publishes AKGL_VERSION, AKGL_VERSION_MAJOR/MINOR/PATCH and
AKGL_VERSION_AT_LEAST(major, minor, patch). That macro is the point:
libakstdlib could not write that test against libakerror, which
publishes no version macro, and had to feature-test on
AKERR_FIRST_CONSUMER_STATUS instead. game.h now includes the generated
header rather than defining the version itself, so consumers see
AKGL_VERSION exactly where they saw it before.
While the major version is 0 the soname carries major.minor, giving
libakgl.so.0.1. A plain SOVERSION 0 would assert that 0.1.0 and 0.2.0
are ABI-interchangeable, and they are not -- the preceding commit alone
added akgl_error_init. At 1.0.0 the soname becomes the major alone,
matching libakerror.
akgl_version() in src/version.c reports the version of the libakgl that
is actually linked, where AKGL_VERSION reports what the caller compiled
against. Comparing the two is the only way to catch a stale shared
library on the loader path; the macro alone cannot see it. It returns a
string rather than an akerr_ErrorContext * because it cannot fail, for
the same reason akerr_name_for_status does.
Add tests/version.c: the string and the numeric components must describe
one version, the linked library must agree with the headers, and
AKGL_VERSION_AT_LEAST must order correctly at the major, minor and patch
boundaries. Verified out of band that bumping project() to 1.2.3 moves
version.h, akgl.pc and the soname together, and that an install produces
the libakgl.so -> .so.0.1 -> .so.0.1.0 chain with SONAME libakgl.so.0.1.
Also ignore include/akgl/version.h. include/ precedes the build tree on
the include path, so a stray copy there would shadow the generated one
and pin every consumer to whatever version it was generated at.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
libakerror 1.0.0 replaced the consumer-sized status-name array with a
private registry and made status-code ownership explicit and enforced.
AKERR_MAX_ERR_VALUE and __AKERR_ERROR_NAMES are gone, and the registry
entry points raise akerr_ErrorContext * instead of returning int. See
deps/libakerror/UPGRADING.md.
The break was not only source-level. libakgl's codes sat at
AKERR_LAST_ERRNO_VALUE + 18 through + 22, and 1.0.0 claimed exactly
those five offsets for its own AKERR_STATUS_* registry codes, so every
AKGL_ERR_* was aliasing a libakerror status. HANDLE(e,
AKGL_ERR_LOGICINTERRUPT) at physics.c:222 would have swallowed a
foreign-name refusal.
- Move the band to AKERR_FIRST_CONSUMER_STATUS (256) as fixed offsets,
so a libc that grows an errno cannot move the codes, and add
AKGL_ERR_OWNER, AKGL_ERR_LIMIT and AKGL_ERR_COUNT to describe it.
- Reserve the range and register the names through the owned entry
points, PASS-ing each: these are AKERR_NOIGNORE, and the old
akerr_name_for_status calls discarded failure silently.
- Drop the AKERR_MAX_ERR_VALUE=256 compile definition.
- Guard on AKERR_FIRST_CONSUMER_STATUS in include/akgl/error.h, which
now includes <akerror.h> so the guard is reliable. The embedded build
is fine, but the find_package path can pick up a stale installed
header, and 1.0.0 has an soname, so that pairing is an ABI mismatch
rather than a compile problem. Same guard libakstdlib already carries.
Registration also moves out of akgl_heap_init into a new akgl_error_init
in src/error.c. It was in the heap pool's initializer only because that
was the first thing akgl_game_init called, and the upgrade turned five
fire-and-forget name calls into a library-wide ownership claim that can
fail. That placement was hiding a defect: game.c raises AKGL_ERR_SDL
when SDL_CreateMutex fails, five lines before akgl_heap_init ran, so the
earliest error path in the library was guaranteed to print "Unknown
Error". akgl_error_init is now the first statement in akgl_game_init.
Callers that drive subsystems directly must call akgl_error_init first;
it is idempotent, so ordering it precisely is not required. The eleven
test suites that relied on akgl_heap_init to name their statuses now
call it explicitly, or their failure messages would have degraded to
"Unknown Error".
Add tests/error.c: assert every code reads back its registered name,
that the name table and AKGL_ERR_COUNT agree, that a foreign owner is
refused with AKERR_STATUS_NAME_FOREIGN and AKERR_STATUS_RANGE_OVERLAP,
and that repeating the init is a no-op. That last one is a live
constraint, not a triviality -- libakerror treats only an identical
reservation as a repeat, so a subset or superset raises.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pre-commit hook is version controlled under scripts/hooks/, but Git
configuration is not cloned, so the hook does nothing until each clone is
pointed at it. Nothing said so anywhere a new contributor would look.
Cover installation, verification, what the hook actually does, the Emacs
requirement, bypassing, and uninstalling.
Two things worth calling out explicitly:
- core.hooksPath replaces the hooks directory outright rather than adding
to it, so any pre-existing .git/hooks entries silently stop firing.
Document a symlink alternative for anyone who keeps local hooks.
- The documented --check exit codes are 0 conforming, 1 non-conforming,
2 environment failure, so a CI job failing on any non-zero status will
not mistake a missing Emacs for a clean tree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`--check` prefixed every path with the repository root unconditionally, so
passing an absolute path produced a bogus `$root//abs/path`, cp failed, and
the subsequent cmp reported the file as non-conforming. The exit status
happened to be 1, which looked correct while being reached for the wrong
reason.
In-place mode was unaffected -- it cds to the root and hands the list
straight to Emacs, where absolute paths resolve normally, which is why the
pre-commit hook worked.
Normalize both the copy and the compare through an abspath helper so
relative and absolute paths behave identically in either mode.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
Mechanical whitespace-only change across src/, include/, tests/, and
util/, applied with scripts/reindent.sh. No behavioral change.
Most files already conformed. The genuine outliers indented at 2 columns
(json_helpers.c, util.c from akgl_rectangle_points onward, assets.c,
staticstring.c, akgl_actor_add_child, util.h, staticstring.h) or used
spaces where the canonical form is a tab (draw.c). cc-mode also re-aligns
line-continuation backslashes in multi-line macros to its default
c-backslash-column, which is required for the tree to be a fixed point of
the indenter.
Verified whitespace-only: `git diff -w` over this change is empty apart
from three trailing blank lines removed at EOF in src/heap.c,
src/registry.c, and tests/tilemap.c. The build produces no new errors and
ctest is unchanged at 13/14, with `character` still the one intentional
failure.
The tree is now a fixed point of `scripts/reindent.sh --check`.
include/akgl/SDL_GameControllerDB.h is generated and excluded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AGENTS.md described style in one paragraph that said little more than
"follow the surrounding code", which was not actionable once files had
drifted apart. Replace it with an explicit specification.
The canonical style is Emacs cc-mode "stroustrup" with tabs enabled: a
4-column offset with 8-column tabs, so depth 1 is four spaces, depth 2 is
one tab, depth 3 is a tab plus four spaces. Most of src/ already followed
this; what looked like randomly mixed tabs and spaces was simply cc-mode
output. Document the byte ladder explicitly, since editors that expand
tabs or assume a 4-column tab silently corrupt it.
Rules beyond indentation are derived from counted majorities in the
existing code rather than invented: errctx over e (92 to 45), padded
control parens (140 to 7), pointer binding to the identifier (486 to 5),
and always-brace (only four unbraced bodies exist). Brace and else
placement are called out as house conventions that cc-mode does not
enforce, so "run the indenter" and "follow the guide" cannot conflict.
Also record the naming, error-handling, and API-surface rules that a
consistency sweep showed were being broken silently -- in particular that
a *_RETURN macro inside an ATTEMPT block skips CLEANUP.
Add the tooling to apply it:
.dir-locals.el applies the style to every c-mode buffer
scripts/reindent.el batch reindent via Emacs
scripts/reindent.sh reindent or --check the tree
scripts/hooks/pre-commit reindents staged sources
reindent.el deliberately avoids Emacs' tabify: its tabify-regexp is
" [ \t]+", which is not anchored to the start of a line, so it rewrites
runs of spaces anywhere and destroys the hand-aligned value columns in
the bit-flag tables in actor.h and iterator.h. Only leading whitespace is
ever rewritten, and lines starting inside a string literal are skipped.
The hook checks staged content rather than the working tree, so what is
committed is what was verified. It re-stages a fixed file only when the
index and working tree agree, otherwise a partial `git add -p` would
sweep unstaged work into the commit.
Enable with: git config core.hooksPath scripts/hooks
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cover control map capacity, the default binding set, keyboard and gamepad
dispatch including cross-device rejection, the dpad handlers, and device
add and remove against the dummy SDL drivers. Synthesize SDL events directly
rather than pumping the event queue, so the suite needs no real hardware.
Fix the three-way appstate check in the four gamepad handlers, which tested
appstate in place of both the event pointer and the looked-up player actor,
so a null event or a missing player was dereferenced instead of reported.
Record the new coverage status in TODO.md along with the seven defects the
suites uncovered and fixed, eleven that remain open, and the reason the
build tree has to precede LD_LIBRARY_PATH for the CTest run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Raise line coverage from 39.6 to 61.8 percent with four new suites and an
extension to the actor suite, and register every suite through a single
CMake list so a new test file cannot be left out of the coverage fixture.
Give the test targets a build-tree RPATH and prepend the build tree to
LD_LIBRARY_PATH for CTest, so a developer with a previously installed
libakgl.so exercises the library that was just compiled.
Fix six defects the new tests exposed:
- akgl_physics_simulate read self->gravity_time before its NULL check, so a
NULL backend crashed instead of reporting AKERR_NULLPOINTER.
- akgl_game_save transposed CLEANUP and PROCESS, which placed the fclose
inside the PROCESS switch. An ordinary save never flushed or closed its
stream and produced an empty file.
- akgl_game_save_actors wrote each name table terminator from the address of
a single char, emitting stack contents into the save file and a sentinel
the loader could not recognize.
- akgl_game_load_objectnamemap used CATCH directly inside while(1), where the
break leaves the loop rather than propagating, so a truncated name table
loaded as a successful game.
- akgl_Actor_cmhf_up_on and _down_on dereferenced actor->basechar with no
NULL check, unlike their left and right counterparts.
- akgl_actor_logic_movement checked actor twice instead of checking
actor->basechar before dereferencing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Record the 39.6 percent line and 44.3 percent function coverage baseline
measured with the AKGL_COVERAGE build, and plan twelve test suites to raise
it to roughly 70 percent lines and 80 percent functions. Tier the work by
what each suite needs: pure logic suites for physics, json_helpers, heap,
and savegame handling that require no renderer; renderer suites gated behind
a shared offscreen harness; and controller and frame-loop coverage deferred.
Note that vendored dependency shared objects are absent from the loader path
in a fresh out-of-tree build, which reports every test as zero coverage, and
record ten suspected defects found while reading the uncovered code so the
new tests assert correct behavior rather than pinning the current one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Instrument libakgl in opt-in coverage builds, use CTest fixtures to reset counters and emit Cobertura XML and detailed HTML reports, and upload those reports from Gitea CI. Document the local coverage workflow for contributors.
Co-authored-by: Codex (GPT-5) <noreply@openai.com>
Add file, structure, global, and function documentation across all libakgl-owned headers and sources, including parameter contracts and likely AKERR/AKGL_ERR exceptions. Add a strict Doxyfile and build the generated API documentation in Gitea CI with warnings treated as failures.
Co-authored-by: Codex (GPT-5) <noreply@openai.com>
Build libakgl with recursive vendored dependencies, run the passing CTest suite with JUnit reporting, and publish a bounded static-string mutation gate with a 50 percent score threshold.
Co-authored-by: Codex (GPT-5) <noreply@openai.com>
Port the scratch-copy mutation runner used by libakerror and libakstdlib, covering libakgl source files with sampling, thresholds, JUnit reports, and test timeouts. Add a CMake target and documentation, preserve the intentionally failing character test exclusion, and migrate the sprite test to the renderer backend so mutation baselines are green.
Co-authored-by: Codex (GPT-5) <noreply@openai.com>
Migrate tilemap tests to pointer-based game globals and current loader signatures, initialize the renderer callback used by image comparisons, and refresh the character fixture schema. Preserve loaded tileset names by removing a duplicate heap allocation.
Co-authored-by: Codex (GPT-5) <noreply@openai.com>
tilemap layers, and a custom `movementlogicfunc` (which is where `AKGL_ERR_LOGICINTERRUPT`
tilemap layers, and a custom `movementlogicfunc` (which is where `AKGL_ERR_LOGICINTERRUPT`
stops being a table row and becomes something the reader writes).
stops being a table row and becomes something the reader writes).
This game forces the chapter to make three honest statements, all from the defect record:
This game forces the chapter to make three honest statements, all from `TODO.md`:
- **`akgl_physics_arcade_collide` is not implemented** — it raises `AKERR_API`, and
- **`akgl_physics_arcade_collide` is not implemented** — it raises `AKERR_API`, and
`akgl_physics_simulate` never calls `collide` at all. `arcade_move` does no clamping and
`akgl_physics_simulate` never calls `collide` at all. `arcade_move` does no clamping and
@@ -433,7 +433,7 @@ covers.
This plan does not fix it — replacing test fixtures is a separate change with its own blast
This plan does not fix it — replacing test fixtures is a separate change with its own blast
radius, and it is not blocking the docs. It does two things: the tutorial assets get a clean
radius, and it is not blocking the docs. It does two things: the tutorial assets get a clean
provenance story that does not depend on those files, and the finding goes into the tracker
provenance story that does not depend on those files, and the finding goes into `TODO.md`
with file, functional consequence and blast radius, per the house practice of documenting
with file, functional consequence and blast radius, per the house practice of documenting
defects against yourself.
defects against yourself.
@@ -473,12 +473,12 @@ project exists to fix is documentation asserting things nobody checked:
`norun` block is a decision that has to be justified.
`norun` block is a decision that has to be justified.
4. **Do not restate signatures** — link the Doxygen reference, or `excerpt=` the header.
4. **Do not restate signatures** — link the Doxygen reference, or `excerpt=` the header.
5. **Note the known defect where a reader would hit it**, with the guard to apply, linked
5. **Note the known defect where a reader would hit it**, with the guard to apply, linked
to its issue.
to `TODO.md`.
Wave 2 is the integration pass, and the only genuinely serial step: build the TOC, insert
Wave 2 is the integration pass, and the only genuinely serial step: build the TOC, insert
cross-references between chapters and both tutorials, verify no chapter re-teaches upstream
cross-references between chapters and both tutorials, verify no chapter re-teaches upstream
material, reconcile terminology, cut the README's FAQ half, file the issues the writing turned
material, reconcile terminology, cut the README's FAQ half, add the `TODO.md` entries, then
up, then run the full gate.
run the full gate.
## Verification
## Verification
@@ -516,7 +516,7 @@ Specific things that must hold before this is done:
## Out of scope, deliberately
## Out of scope, deliberately
- Replacing the RPG-Maker-named test fixtures (issue #52 instead).
- Replacing the RPG-Maker-named test fixtures (recorded in `TODO.md` instead).
- Fixing the false claims in `physics.h` and `registry.h`. The chapters document what the
- Fixing the false claims in `physics.h` and `registry.h`. The chapters document what the
code *does*; correcting the header comments is a separate commit, since `AGENTS.md`
code *does*; correcting the header comments is a separate commit, since `AGENTS.md`
requires style and behaviour changes to stay unbundled.
requires style and behaviour changes to stay unbundled.
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
