andrew/libakgl
1
0
Fork 0
Code Issues 67 Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Twenty-seven header comments describe code that has changed #63

New Issue
Open
opened 2026-08-02 18:34:53 -04:00 by tachikoma · 0 comments
tachikoma commented 2026-08-02 18:34:53 -04:00
Collaborator
Copy Link

Source: TODO.md, "Header comments that describe code that has changed" (at bbb7b8f)

Twenty-one manual chapters and two tutorial games were written against src/
rather than against the header comments. Twenty-seven claims across the public
headers were false when checked.

AGENTS.md already warns that TODO.md "carried eleven entries describing code
that had already changed"; this is the same failure in the headers, and Doxygen
publishes it.

Nothing catches these. WARN_IF_UNDOCUMENTED proves a symbol has a comment,
not that the comment is true, and api_surface strips comments precisely because
prose is not a declaration.

The full list is in the manual, each noted in the chapter that covers the
subsystem. The ones that would actively mislead a caller:

  • physics.h:8-9,195 -- the physics.engine property and akgl_game_init
    calling the factory. Neither exists, and a caller who believes it writes
    the program that segfaults on frame one (filed separately).
  • renderer.h -- frame_start/frame_end/draw_texture "dereference self
    before it is checked". Each function's first statement is
    FAIL_ZERO_RETURN(errctx, self, ...).
  • sprite.h -- speed is "seconds, scaled to milliseconds". It is
    milliseconds scaled to nanoseconds. Also claims frames is unbounded
    (bounded at src/sprite.c:207) and that akgl_sprite_initialize overreads
    via memcpy (it uses aksl_strncpy).
  • character.h -- speedtime "in seconds" (milliseconds), and sprite_add
    never releasing a displaced sprite (src/character.c:60,77-79 releases it).
  • actor.h -- the cmhf block comment says the _off handlers zero
    acceleration, thrust, environmental and velocity. They zero only ax/tx
    or ay/ty; zeroing ey was the gravity-cancel defect fixed in 0.6.0, and
    two @notes still describe it.
  • json_helpers.h -- the conventions block says dest is not NULL-checked and
    that only akgl_get_json_string_value checks its key. All eleven accessors
    check dest, and all seven key-taking accessors check key.
  • assets.h:17-18 -- "akgl_game_init (or a bare akgl_audio_init) has to
    have run first". akgl_mixer is created only in akgl_game_init;
    akgl_audio_init opens the synthesizer's stream and never touches it, so
    akgl_load_start_bgm after only that hands NULL to MIX_LoadAudio.
  • registry.h:54 -- akgl_registry_init creating seven registries and not
    properties. It creates eight including properties; the genuinely false part
    is that akgl_game_init never calls it at all    , calling the eight
    individually in a different order.
  • tilemap.h:59-60,354-357,447-450,462-468 -- object and tileset counts
    unbounded, and akgl_tilemap_release double-freeing. All fixed; the surviving
    half of the last one is that release does not release the map's actors.
  • controller.h:232-234,262-263 -- a negative controlmapid not rejected. Both
    call sites check.
  • README.md (already corrected) -- "ONLY supports TilED TMJ tilemaps with
    tileset external references". Backwards: "source" appears nowhere in
    src/tilemap.c, and akgl_tilemap_load_tilesets_each reads
    columns/firstgid/tilecount/image inline. Only embedded tilesets
    load.

Worth deciding while closing this: whether anything can be made to catch the
next one. A doc comment that names a status, a bound or a check is a claim the test
suite could assert, and the three that would have been caught cheaply are the ones
naming a status.

Files: include/akgl/*.h, docs/

Filed by Tachikoma (Claude Code, Opus 5, 1M context)

**Source:** TODO.md, "Header comments that describe code that has changed" (at bbb7b8f) Twenty-one manual chapters and two tutorial games were written against `src/` rather than against the header comments. **Twenty-seven claims across the public headers were false when checked.** `AGENTS.md` already warns that `TODO.md` "carried eleven entries describing code that had already changed"; this is the same failure in the headers, **and Doxygen publishes it.** **Nothing catches these.** `WARN_IF_UNDOCUMENTED` proves a symbol *has* a comment, not that the comment is true, and `api_surface` strips comments precisely because prose is not a declaration. The full list is in the manual, each noted in the chapter that covers the subsystem. The ones that would actively mislead a caller: - [ ] `physics.h:8-9,195` -- the `physics.engine` property and `akgl_game_init` calling the factory. **Neither exists**, and a caller who believes it writes the program that segfaults on frame one (filed separately). - [ ] `renderer.h` -- `frame_start`/`frame_end`/`draw_texture` "dereference `self` before it is checked". Each function's first statement is `FAIL_ZERO_RETURN(errctx, self, ...)`. - [ ] `sprite.h` -- `speed` is "seconds, scaled to milliseconds". It is milliseconds scaled to nanoseconds. Also claims `frames` is unbounded (bounded at `src/sprite.c:207`) and that `akgl_sprite_initialize` overreads via `memcpy` (it uses `aksl_strncpy`). - [ ] `character.h` -- `speedtime` "in seconds" (milliseconds), and `sprite_add` never releasing a displaced sprite (`src/character.c:60,77-79` releases it). - [ ] `actor.h` -- the `cmhf` block comment says the `_off` handlers zero acceleration, thrust, environmental *and* velocity. They zero only `ax`/`tx` or `ay`/`ty`; zeroing `ey` was the gravity-cancel defect fixed in 0.6.0, and two `@note`s still describe it. - [ ] `json_helpers.h` -- the conventions block says `dest` is not NULL-checked and that only `akgl_get_json_string_value` checks its key. All eleven accessors check `dest`, and all seven key-taking accessors check `key`. - [ ] `assets.h:17-18` -- "`akgl_game_init` (or a bare `akgl_audio_init`) has to have run first". `akgl_mixer` is created only in `akgl_game_init`; `akgl_audio_init` opens the synthesizer's stream and never touches it, so `akgl_load_start_bgm` after only that hands NULL to `MIX_LoadAudio`. - [ ] `registry.h:54` -- `akgl_registry_init` creating seven registries and not properties. It creates eight including properties; **the genuinely false part is that `akgl_game_init` never calls it at all**, calling the eight individually in a different order. - [ ] `tilemap.h:59-60,354-357,447-450,462-468` -- object and tileset counts unbounded, and `akgl_tilemap_release` double-freeing. All fixed; the surviving half of the last one is that `release` does not release the map's actors. - [ ] `controller.h:232-234,262-263` -- a negative `controlmapid` not rejected. Both call sites check. - [ ] `README.md` (already corrected) -- "ONLY supports TilED TMJ tilemaps with tileset **external** references". Backwards: `"source"` appears nowhere in `src/tilemap.c`, and `akgl_tilemap_load_tilesets_each` reads `columns`/`firstgid`/`tilecount`/`image` inline. **Only embedded tilesets load.** **Worth deciding while closing this:** whether anything can be made to catch the next one. A doc comment that names a status, a bound or a check is a claim the test suite could assert, and the three that would have been caught cheaply are the ones naming a status. **Files:** `include/akgl/*.h`, `docs/` --- Filed by Tachikoma (Claude Code, Opus 5, 1M context)
tachikoma added this to the 0.9.x milestone 2026-08-02 18:34:53 -04:00
tachikoma added the docsblast-radius:high labels 2026-08-02 18:34:53 -04:00
tachikoma added the status::grooming label 2026-08-02 18:49:28 -04:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
abi-break
Cannot land without a soname bump
 api-gap
A caller cannot express something
 blast-radius:high
Ordering: highest blast radius
 blast-radius:low
Ordering: lowest blast radius
 blast-radius:medium
Ordering: middling blast radius
 defect
Wrong behaviour reachable from the public API
 design-decision
Needs a decision written down before code
 docs
Header comments, manual, AGENTS.md
 hygiene
Style, dead code, consistency
 packaging
Build, install, pkg-config, CMake config
 performance
Has a number in PERFORMANCE.md behind it
 status::blocked
Waiting on something else -- a decision, a dependency, or another issue
 status::grooming
Not yet worked out: scope, approach or priority still being settled
 status::ready
Understood well enough to start: scope and approach are settled
 test-coverage
A gap in the suites, not a defect
No Label blast-radius:high docs status::grooming
Milestone
No items
No Milestone 0.9.x
Projects
Clear projects
No project
Assignees
Clear assignees
tachikoma andrew
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: andrew/libakgl#63
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?