Compare commits
3 Commits
clay-ui
...
todo-to-is
| Author | SHA1 | Date | |
|---|---|---|---|
|
963d511224
|
|||
|
eabb9ad376
|
|||
| bbb7b8f494 |
30
AGENTS.md
30
AGENTS.md
@@ -124,7 +124,7 @@ Every suite runs in every job. The `character` suite used to be excluded from
|
||||
the coverage and memory-check jobs on the grounds that it failed deliberately.
|
||||
It did not: it was reporting success while running one of its four tests, for
|
||||
two independent reasons, and both are fixed. See TODO.md, "Test suites that
|
||||
could not fail".
|
||||
could not fail", which is the record of how that happened.
|
||||
|
||||
## Coding Style
|
||||
|
||||
@@ -309,7 +309,7 @@ and are unaffected either way.
|
||||
**Never silence a warning with a cast.** Three `-Wpointer-sign` findings in
|
||||
`src/sprite.c` were real signedness mismatches -- `uint32_t *` passed where an
|
||||
`int *` was expected -- and a cast would have hidden every one. That is the
|
||||
whole argument of `TODO.md` item 37. Read into a correctly typed local, check
|
||||
whole argument of issue #5. Read into a correctly typed local, check
|
||||
the range, and assign.
|
||||
|
||||
### No repository-wide formatter
|
||||
@@ -496,8 +496,8 @@ to that device are accepted and the `ENOSPC` surfaces at the flush.
|
||||
|
||||
**Do not convert the pure-arithmetic calls.** `memset`, `memcpy`, `strlen`,
|
||||
`strcmp` and friends can only fail on a NULL argument, and the tree is
|
||||
inconsistent about them already; see `TODO.md`, "libakstdlib wrappers not yet
|
||||
adopted", before starting a sweep.
|
||||
inconsistent about them already; see issue #19, which is the decision this
|
||||
waits on, before starting a sweep.
|
||||
|
||||
**Never silence `-Wformat-truncation`.** `include/akgl/error.h` still exports
|
||||
`DISABLE_GCC_WARNING_FORMAT_TRUNCATION` and nothing uses it. Both sites that
|
||||
@@ -549,12 +549,13 @@ cheap to check and expensive to assume.
|
||||
a bit that is **not** set whose value is **not** 1. The obvious test passes
|
||||
either way.
|
||||
|
||||
- **Do not trust a comment, a TODO entry, or a CI exclusion that states a
|
||||
- **Do not trust a comment, an issue, or a CI exclusion that states a
|
||||
premise.** Verify it. `tests/character.c` was excluded from two CI jobs and
|
||||
from the mutation harness because it "fails deliberately". It did not: it was
|
||||
exiting 0 while running one of its four tests. `TODO.md` carried eleven
|
||||
entries describing code that had already changed. A premise nobody has
|
||||
re-checked is where the next defect is hiding.
|
||||
exiting 0 while running one of its four tests. `TODO.md`, when it still held
|
||||
outstanding work, carried eleven entries describing code that had already
|
||||
changed, and three more were found stale when it was migrated to the tracker.
|
||||
A premise nobody has re-checked is where the next defect is hiding.
|
||||
|
||||
- **When a measurement moves, say what you measured.** Absolute benchmark
|
||||
numbers drift with the machine. The `akgl_game_update` fix is recorded as the
|
||||
@@ -564,6 +565,19 @@ cheap to check and expensive to assume.
|
||||
|
||||
## Rules
|
||||
|
||||
- **Outstanding work goes in the issue tracker, not in a file.** Open an issue at
|
||||
<https://source.starfort.tech/andrew/libakgl/issues> — `tea issues create --repo andrew/libakgl`
|
||||
— with the file and line it touches, the functional consequence, and what closing it would
|
||||
touch. Label it by kind and blast radius, and leave `status::grooming` on it until its scope
|
||||
and approach are settled. **Do not add outstanding items to `TODO.md`**; that file is the
|
||||
record of decisions, measurements and defects already closed, and the reason for the split is
|
||||
that a description of work still to do goes stale the moment somebody does it.
|
||||
- **When you learn something about an open issue, say so in the issue** — including that it is
|
||||
worse than recorded, that it is already fixed, or that its premise has expired. An issue
|
||||
nobody has re-checked is worth what an unverified comment is worth.
|
||||
- **A defect in a dependency is filed against that dependency**, not worked around here.
|
||||
`libakerror`, `libakstdlib` and `akbasic` all have trackers on the same forge. Comment the
|
||||
workaround at its site with the words "filed upstream" and delete it when the fix lands.
|
||||
- Add yourself (agent program name, model name and version) as a co-author on every commit message
|
||||
- Avoid dynamic memory allocation. Use the `akgl_heap_*` methods to retrieve necessary objects at runtime, and to release them when done. If the akgl heap facilities don't provide the kind of object needed, create a new heap layer to support that type of object.
|
||||
|
||||
|
||||
@@ -563,7 +563,7 @@ day a GPU backend makes the pixels cheap.
|
||||
|
||||
## Defects these tests found
|
||||
|
||||
Six, and they are filed where defects live: `TODO.md`, under **Performance ->
|
||||
Six, and they are recorded where defects live: `TODO.md`, under **Performance ->
|
||||
Defects the perf suites found**, items 28-33, each with its file, line,
|
||||
functional consequence, and what fixing it would touch. They are not repeated
|
||||
here.
|
||||
|
||||
@@ -184,7 +184,7 @@ AKGL_BENCH_SCALE=0.1 ctest --test-dir build -L perf # a tenth of the iterations
|
||||
|
||||
Each measurement is the best of five runs and is held to a budget set at roughly ten times the recorded baseline, so a suite that turns red means an algorithmic regression rather than a busy machine. Budgets are enforced only in an optimized build at full scale; below `AKGL_BENCH_SCALE=1.0`, and in a coverage build, they are reported without failing.
|
||||
|
||||
`PERFORMANCE.md` carries the recorded baseline, the frame budget it adds up to, and what the numbers say — including the raw-SDL control rows that separate what libakgl costs from what the rasterizer costs. The six defects the stress tests turned up, and the targets the numbers are measured against, are in `TODO.md` under **Performance**.
|
||||
`PERFORMANCE.md` carries the recorded baseline, the frame budget it adds up to, and what the numbers say — including the raw-SDL control rows that separate what libakgl costs from what the rasterizer costs. The six defects the stress tests turned up, and the targets the numbers are measured against, are in `TODO.md` under **Performance**; the plan for the missed targets is issue #60.
|
||||
|
||||
## Memory checking
|
||||
|
||||
@@ -211,7 +211,7 @@ this tree, that notice has to travel with it.
|
||||
It is the only dependency in that position. SDL3, SDL3_image, SDL3_mixer, SDL3_ttf, jansson,
|
||||
libakerror and libakstdlib are all linked as separate shared objects and carry their own
|
||||
notices; `deps/semver` is a single header, MIT, installed alongside ours; and `deps/tg` is
|
||||
vendored but **compiled into nothing** -- see `TODO.md`, "tg is vendored and has no
|
||||
vendored but **compiled into nothing** -- see `TODO.md`, "Why deps/tg is still in the
|
||||
consumer".
|
||||
|
||||
libccd is given a static arena rather than the allocator it ships with, so "libakgl does not
|
||||
|
||||
27
TODO.txt
27
TODO.txt
@@ -1,27 +0,0 @@
|
||||
Rendering should move to the SDL GPU renderer so i can do lighting and particles etc
|
||||
- Example suitable for my most primitive use case: https://github.com/TheSpydog/SDL_gpu_examples/blob/main/Examples/Blit2DArray.c
|
||||
- Try vulkan and D3D tutorials to come up to speed on the moving pieces, then figure ou the details from the examples and API docs
|
||||
- https://vulkan-tutorial.com/Drawing_a_triangle/Graphics_pipeline_basics/Introduction
|
||||
- https://learn.microsoft.com/en-us/previous-versions/windows/desktop/bb153302(v=vs.85)
|
||||
- http://www.directxtutorial.com/LessonList.aspx?listid=112
|
||||
- Shaders compiled with: https://github.com/libsdl-org/SDL_shadercross. There are no docs on this tool.
|
||||
- Shader language is tricky. MS is abandoning HLSL and everyone unifying on Spir-V. But Spir-V is intermediate, HLSL is more high level so easier to pick up.
|
||||
- https://flathub.org/apps/org.shadered.SHADERed
|
||||
- Really good HLSL tutorials: https://www.youtube.com/playlist?list=PL78XDi0TS4lEMvytsE_MoWEpzBcukXv9b
|
||||
- Spir-V tutorials: https://github.com/google/spirv-tutor
|
||||
|
||||
Actors should collide with each other
|
||||
- Box2D is going to be overkill for this for simpler platforms. We're going to benefit from a simpler approach.
|
||||
- https://katyscode.wordpress.com/2013/01/18/2d-platform-games-collision-detection-for-dummies/ looks promising
|
||||
|
||||
World collision geometry should be loaded from object layers on the map
|
||||
|
||||
Actors should collide with the world
|
||||
|
||||
Actors should be able to follow a path defined with a polyline on a layer on the map
|
||||
|
||||
Actors should be able to calculate their facing state automatically depending on their movement state (for path following)
|
||||
|
||||
Maps should be able to have image layers
|
||||
|
||||
Map parallax should work
|
||||
@@ -68,7 +68,7 @@ library's.
|
||||
## What is not implemented
|
||||
|
||||
Named here rather than discovered later. Each gets a callout in the chapter where you would
|
||||
hit it, and an entry in `TODO.md`.
|
||||
hit it, and an issue in the tracker.
|
||||
|
||||
| Gap | Behaviour today | Chapter |
|
||||
|---|---|---|
|
||||
|
||||
@@ -117,7 +117,7 @@ pkg-config --cflags --libs akgl akerror akstdlib sdl3 sdl3-ttf sdl3-mixer jansso
|
||||
`sdl3-image` is needed at link time but appears in no public header, so it belongs in the
|
||||
private half.
|
||||
|
||||
Recorded in `TODO.md` under "akgl.pc names no dependencies", along with why the fix wants
|
||||
Recorded as issue #17, along with why the fix wants
|
||||
its own commit: adding a `Requires:` line changes what `pkg-config --libs akgl` emits for
|
||||
every existing consumer.
|
||||
|
||||
|
||||
@@ -173,7 +173,7 @@ So the context has exactly one owner at a time, and it is never both yours and i
|
||||
returned releases the same slot twice, and a double-released context corrupts the failure
|
||||
instead of reporting it. That is not hypothetical: it is how the first draft of the
|
||||
`AKERR_OUTOFBOUNDS` test in `tests/json_helpers.c` passed against the unfixed library.
|
||||
`AGENTS.md` documents the shape under "Testing Guidelines"; `TODO.md` item 18 records the
|
||||
`AGENTS.md` documents the shape under "Testing Guidelines"; `TODO.md` records the
|
||||
history.
|
||||
|
||||
The correct shape: take the result into a local, `NULL` your own pointer immediately, and
|
||||
|
||||
@@ -105,7 +105,7 @@ normal shape is acquire-then-initialize and never acquire-then-use.
|
||||
### The refcount asymmetry
|
||||
|
||||
This is the one thing about the heap that will surprise you, and it is a known defect
|
||||
rather than a design (`TODO.md`, "Known and still open" item 8; also the `@warning` on
|
||||
rather than a design (issue #14; also the `@warning` on
|
||||
`heap.h` itself).
|
||||
|
||||
| Acquire | Takes the reference? | Who takes it |
|
||||
|
||||
@@ -35,8 +35,8 @@ same `SDL_Texture`, and the texture is loaded once.
|
||||
entry `i` to `1 << i`, which is what lets a character definition write
|
||||
`"AKGL_ACTOR_STATE_FACE_LEFT"` instead of `2`. Two entries in that name table disagree with
|
||||
`actor.h`: bits 11 and 12 are `UNDEFINED_11` and `UNDEFINED_12` rather than `MOVING_IN` and
|
||||
`MOVING_OUT`, **so those two states cannot be named from JSON at all** (`TODO.md` items
|
||||
24–26). The individual name registrations are not checked, either.
|
||||
`MOVING_OUT`, **so those two states cannot be named from JSON at all** (`TODO.md`,
|
||||
"`AKGL_ACTOR_STATE_STRING_NAMES`"). The individual name registrations are not checked, either.
|
||||
|
||||
**The music registry is empty.** `akgl_registry_init_music` creates it and nothing in the
|
||||
library ever writes to it. It is there for you.
|
||||
@@ -51,8 +51,7 @@ around.
|
||||
Read against `src/registry.c` and `src/game.c`, this is what is true:
|
||||
|
||||
- `akgl_registry_init` creates **eight**, including `AKGL_REGISTRY_PROPERTIES`. The
|
||||
properties call was added in 0.5.0 (`TODO.md`, "Known and still open" item 3, marked
|
||||
fixed). Its order is spritesheet, sprite, character, actor, actor-state-strings, font,
|
||||
properties call was added in 0.5.0 (`TODO.md`, "Formatting and hygiene"). Its order is spritesheet, sprite, character, actor, actor-state-strings, font,
|
||||
music, properties.
|
||||
- **`akgl_game_init` never calls `akgl_registry_init`.** It calls the eight individual
|
||||
initializers itself, in a different order: actor, sprite, spritesheet, character, font,
|
||||
@@ -113,12 +112,11 @@ document. The fields:
|
||||
|
||||
**Two distinct names that agree on their first 127 bytes truncate to the same key**, and
|
||||
the second `SDL_SetPointerProperty` silently replaces the first. The objects are different;
|
||||
the registry cannot tell. That is recorded in `TODO.md`, "Truncated registry keys can
|
||||
collide", and it is not being fixed because the fix is a contract change — refusing an
|
||||
the registry cannot tell. That is issue #54, and it is not fixed yet because the fix is a contract change — refusing an
|
||||
over-long name with `AKERR_OUTOFBOUNDS` — and every one of those headers currently promises
|
||||
the opposite.
|
||||
|
||||
There is a second half that the `TODO.md` entry understates. It says the truncated name *is*
|
||||
There is a second half that the issue understates. It says the truncated name *is*
|
||||
the registry key. That is true for sprites and spritesheets, which register under their own
|
||||
copied field. It is **not** true for actors and characters:
|
||||
|
||||
|
||||
@@ -347,7 +347,7 @@ Three further things to know:
|
||||
- **A save containing any registered spritesheet cannot be read back.** The writer uses
|
||||
`AKGL_SPRITE_SHEET_MAX_FILENAME_LENGTH` (512) for that table and the reader used
|
||||
`AKGL_ACTOR_MAX_NAME_LENGTH` (128) for all four. That is fixed as of 0.5.0 (`TODO.md`,
|
||||
"Known and still open" item 7) — and what catches it is the `require_at_eof` check, which
|
||||
"Defects") — and what catches it is the `require_at_eof` check, which
|
||||
turns a field-width disagreement into `AKERR_IO` instead of a silent load with wrong maps.
|
||||
A reader whose widths disagree finds a run of zeros mid-entry, stops early, and would
|
||||
otherwise report success.
|
||||
|
||||
@@ -187,7 +187,7 @@ land.
|
||||
The first failure aborts the frame and propagates unchanged; there is no
|
||||
draw-what-you-can behaviour.
|
||||
|
||||
> **Known defect (`TODO.md`, "Performance", item 6).** The layer loop always runs
|
||||
> **Known defect (issue #26).** The layer loop always runs
|
||||
> all 16 iterations and rescans all 64 actor slots on each — 1024 refcount checks
|
||||
> per frame for a one-layer map. Invisible at 60 fps under the software renderer,
|
||||
> measurable on a 2 ms GPU frame. Bounding the walk by `numlayers` and building
|
||||
|
||||
@@ -295,14 +295,14 @@ game that wants one binds its own `changeframefunc`.
|
||||
- **Truncated names are registry keys.** `akgl_Sprite::name` is 128 bytes and
|
||||
`akgl_SpriteSheet::name` is 512. A longer name truncates *silently*, and two names
|
||||
that truncate the same collide — the second registration replaces the first with no
|
||||
error. Recorded in `TODO.md`, "Truncated registry keys can collide"; the fix is a
|
||||
error. Recorded as issue #54; the fix is a
|
||||
contract change, since the headers currently promise truncation.
|
||||
- **The heap acquire functions are asymmetric.** `akgl_heap_next_string` increments
|
||||
`refcount`; `next_sprite` and `next_spritesheet` do not. `TODO.md` item 8. See
|
||||
`refcount`; `next_sprite` and `next_spritesheet` do not. Issue #14. See
|
||||
[Chapter 05](05-the-heap.md).
|
||||
- **No test asserts a clean sprite or spritesheet load/release cycle.** The tilemap
|
||||
cycle is asserted over 64 iterations; the sprite, spritesheet and character ones
|
||||
are not. `TODO.md`, "Targets", row 16.
|
||||
are not. `TODO.md`, "Targets", row 16; issue #13.
|
||||
- **`akgl_spritesheet_coords_for_frame` bounds-checks nothing.** See above.
|
||||
|
||||
## Where to look next
|
||||
|
||||
@@ -310,7 +310,7 @@ character out from under a live actor leaves a dangling `basechar`.
|
||||
pointer, while `akgl_heap_release_character` clears it under the character's own
|
||||
truncated 128-byte copy. For a name over 127 bytes those differ, and the registry
|
||||
entry survives the release — pointing at a zeroed slot. Latent for ordinary names;
|
||||
related to `TODO.md`, "Truncated registry keys can collide", which covers the
|
||||
related to issue #54, which covers the
|
||||
collision half.
|
||||
- **`speedtime` is written through an `int *` cast of a `uint64_t` field.**
|
||||
`src/character.c` passes `(int *)&obj->speedtime` to
|
||||
|
||||
@@ -331,7 +331,7 @@ backend accumulates gravity, so tapping down mid-jump stopped the character in t
|
||||
air. Velocity was never theirs to clear either — `simulate` recomputes `v` as
|
||||
`e + t` every step. Those notes predate the fix.
|
||||
|
||||
> **Known defect (`TODO.md`, "Arcade physics feel").** There is no friction and no
|
||||
> **Known defect (issue #30).** There is no friction and no
|
||||
> deceleration: zeroing `tx` on release **stops the actor dead**. Correct for Zelda,
|
||||
> wrong for Mario. Bind your own `_off` handler that decays `tx` over time if you
|
||||
> want momentum.
|
||||
@@ -431,12 +431,12 @@ for `akgl_Actor`.
|
||||
`akgl_actor_initialize` writes the registry entry under the *caller's* `name`
|
||||
pointer while `akgl_heap_release_actor` clears it under the actor's truncated
|
||||
128-byte copy. Identical for ordinary names; divergent past 127 bytes, and then
|
||||
the registry keeps an entry pointing at a zeroed slot. Related to `TODO.md`,
|
||||
"Truncated registry keys can collide".
|
||||
the registry keeps an entry pointing at a zeroed slot. That is issue #38;
|
||||
issue #54 covers the related truncation collision.
|
||||
- **`akgl_actor_initialize` silently replaces a same-named actor**, and the one it
|
||||
displaced becomes unreachable rather than being released.
|
||||
- **No friction on release**, and **no terminal velocity** under gravity —
|
||||
`TODO.md`, "Arcade physics feel". [Chapter 14](14-physics.md) covers both.
|
||||
issues #29 through #32. [Chapter 14](14-physics.md) covers both.
|
||||
|
||||
## Where to look next
|
||||
|
||||
|
||||
@@ -362,7 +362,7 @@ pass per layer; see [Chapter 8](08-rendering.md).
|
||||
## Known defects
|
||||
|
||||
Documented here because this is where a reader hits them. Each is cross-referenced to
|
||||
`TODO.md`.
|
||||
its issue.
|
||||
|
||||
**A failed load releases nothing.** `akgl_tilemap_load` zeroes the destination up front and
|
||||
then loads physics, geometry, layers and tilesets in order. If any of that fails, the
|
||||
@@ -375,8 +375,8 @@ leak them.
|
||||
|
||||
**`akgl_tilemap_release` does not release the actors an object layer created.** It destroys
|
||||
tileset and image-layer textures and clears each pointer as it goes — that half is correct
|
||||
as of 0.5.0, and a second release is safe rather than a use-after-free (`TODO.md`, "Known
|
||||
and still open" item 2). The actors, and their references on characters and sprites, are
|
||||
as of 0.5.0, and a second release is safe rather than a use-after-free (`TODO.md`,
|
||||
"Defects"). The actors, and their references on characters and sprites, are
|
||||
yours to release. The header's `@warning` describing a tileset double-free is stale; the
|
||||
loop it describes was fixed and `tests/tilemap.c` releases the fixture three times and
|
||||
asserts every texture pointer is `NULL`.
|
||||
|
||||
@@ -177,7 +177,7 @@ cannot get in the way of that. The first version of that helper set `gravity_tim
|
||||
`now - dt` and let the real clock supply the step; a machine busy enough to deschedule the
|
||||
process between that store and the `SDL_GetTicksNS()` inside `simulate` produced a longer
|
||||
step and a red suite, exactly once, under a parallel `ctest`. **That the engine cannot be
|
||||
stepped exactly is itself a finding**, recorded in `TODO.md` under "Arcade physics feel".
|
||||
stepped exactly is itself a finding**, recorded as issue #31.
|
||||
|
||||
Two more rules from that suite, both learned the hard way. `main` must call `SDL_Init` —
|
||||
without it SDL sets its clock epoch on first use, `SDL_GetTicksNS()` returns something
|
||||
@@ -332,7 +332,7 @@ this section used to carry.
|
||||
Found by `tests/physics_sim.c`, which 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. Three other defects that suite found *are* fixed in
|
||||
0.6.0. These four are not. All are in `TODO.md`, "Arcade physics feel".
|
||||
0.6.0. These four are not. All four are issues #29 through #32.
|
||||
|
||||
| Gap | What you see | The workaround today |
|
||||
|---|---|---|
|
||||
|
||||
@@ -225,7 +225,7 @@ akgl_collision_world_init(&world, "bsp", 16.0f, 16.0f);
|
||||
|
||||
- **No rotation.** No actor carries an angle, `akgl_actor_render` hard-codes
|
||||
`SDL_FLIP_NONE`, and every shape is axis-aligned. The support functions are written so that
|
||||
adding it touches one static function in the narrowphase; see `TODO.md`.
|
||||
adding it touches one static function in the narrowphase; see issue #62.
|
||||
- **No restitution and no friction.** The default response blocks. Anything bouncier is your
|
||||
`collidefunc`.
|
||||
- **No continuous collision.** Sub-stepping bounds how far an actor travels between tests, and
|
||||
|
||||
@@ -353,7 +353,7 @@ re-derives them from the header prose:
|
||||
`akgl_controller_default` check `controlmapid < 0` as well as the upper bound and raise
|
||||
`AKERR_OUTOFBOUNDS`. `tests/controller.c` passes `-1` and `-4096` to each. The
|
||||
`@warning` blocks in `include/akgl/controller.h` saying only the upper bound is checked
|
||||
are stale; `TODO.md`, "Known and still open" item 11.
|
||||
are stale; `TODO.md`, "Defects", records the fix.
|
||||
- **The `akgl_controller_handle_*` functions exist.** `controller.h` once declared four
|
||||
names that were defined under different spellings, so they linked nowhere. Fixed in
|
||||
0.5.0, and `scripts/check_api_surface.sh` runs as the `api_surface` test to stop that
|
||||
|
||||
@@ -127,8 +127,8 @@ Measuring is 340 times cheaper than drawing, which tells you the whole cost is t
|
||||
and the upload. **Six HUD readouts is 76 µs a frame. That is fine at 60 fps on a laptop and
|
||||
it is 4% of a 2 ms GPU frame** — and it is being paid every frame for a score that changes
|
||||
once a second. A one-line cache keyed on (font, string, colour) would take it to nothing;
|
||||
`PERFORMANCE.md` calls it the single clearest optimisation in the library, and `TODO.md`
|
||||
carries it as a target.
|
||||
`PERFORMANCE.md` calls it the single clearest optimisation in the library, and it is
|
||||
issue #22.
|
||||
|
||||
So: **fine for a HUD line, wrong for a static body of text redrawn every frame.** If you are
|
||||
drawing a page of dialogue that does not change, rasterize it yourself once with SDL3_ttf,
|
||||
@@ -209,7 +209,7 @@ guards, so drawing nothing still refuses everything drawing something refuses.
|
||||
> refused, and `text.h:120-122` still warns that a failure after rasterizing leaks the
|
||||
> surface and the texture. Both describe pre-0.5.0 behaviour. `src/text.c:110-112` returns
|
||||
> success for `""`, and `src/text.c:140-146` destroys both objects in a `CLEANUP` block that
|
||||
> runs on every path. `TODO.md` items 27 and 23 record both as fixed. The header comments
|
||||
> runs on every path. `TODO.md` records both as fixed. The header comments
|
||||
> want correcting in their own commit.
|
||||
|
||||
## Fonts, the pools, and what is not shared
|
||||
|
||||
@@ -302,7 +302,7 @@ touches `akgl_mixer`, so calling only that leaves `akgl_load_start_bgm` handing
|
||||
`src/assets.c:43` sets `MIX_PROP_PLAY_LOOPS_NUMBER` on it, and `src/assets.c:45` plays the
|
||||
track with it. **0 is SDL's "no property set" sentinel**, not a set this function owns, so the
|
||||
write is rejected — unchecked — and the play call is given no options. The music plays once
|
||||
and stops, and `akgl_load_start_bgm` reports success either way. `TODO.md` item 19; the fix
|
||||
and stops, and `akgl_load_start_bgm` reports success either way. Issue #16; the fix
|
||||
is `SDL_CreateProperties()` into `bgmprops`, checked, and destroyed in `CLEANUP`.
|
||||
|
||||
**`AKGL_REGISTRY_MUSIC` exists and nothing populates it.** `akgl_registry_init_music()`
|
||||
|
||||
@@ -161,8 +161,8 @@ Three things to know:
|
||||
|
||||
> **Stale defect note.** `util.h:124-128` warns that the fallback path "returns straight out
|
||||
> of the ENOENT handler and so never releases the error context it was handling", costing one
|
||||
> of libakerror's 128 context slots per call for the life of the process, and `TODO.md` item
|
||||
> 15 records the same thing as open. **Both are out of date.** `src/util.c:115-129` sets a
|
||||
> of libakerror's 128 context slots per call for the life of the process, and `TODO.md`
|
||||
> recorded the same thing as open. **Both were out of date.** `src/util.c:115-129` sets a
|
||||
> flag in the `HANDLE(errctx, ENOENT)` block and calls `path_relative_root` *after* `FINISH`,
|
||||
> which is exactly the fix that entry proposes, and the code carries a comment explaining
|
||||
> why. The leak is gone; the two notes describing it are not.
|
||||
@@ -332,7 +332,7 @@ for an object member.
|
||||
> `HANDLE_GROUP(e, AKERR_OUTOFBOUNDS)` arm, placed *above* the arm holding the `memcpy`
|
||||
> because `HANDLE_GROUP` emits no `break` and every arm falls into that body, and
|
||||
> `tests/json_helpers.c` covers it through both the integer and object index accessors.
|
||||
> `TODO.md` item 18 records it as fixed. The header comment wants correcting in its own
|
||||
> `TODO.md` records it as fixed. The header comment wants correcting in its own
|
||||
> commit.
|
||||
|
||||
`defsize` is trusted, not derived: it is a `memcpy` through `void *` with no type
|
||||
|
||||
@@ -412,7 +412,7 @@ window:
|
||||
```
|
||||
|
||||
Without this line the first frame calls through a null function pointer. Making the library
|
||||
pick a default is tracked in `TODO.md` under "Known and still open"; until it does, this call
|
||||
pick a default is issue #36; until it does, this call
|
||||
belongs in every libakgl program.
|
||||
|
||||
### The frame loop
|
||||
@@ -524,7 +524,7 @@ Assign it right after `akgl_game_init()`:
|
||||
```
|
||||
|
||||
The hook exists so a game can shed work when it is running slowly. This one has nothing to
|
||||
shed. The first-second false positive is recorded in `TODO.md`.
|
||||
shed. The first-second false positive is recorded in `TODO.md`, "Performance".
|
||||
|
||||
---
|
||||
|
||||
@@ -763,8 +763,7 @@ Add three custom properties to the **map itself** (Map → Map Properties):
|
||||
| `physics.drag.y` | float | `1.5` | Air resistance on the vertical axis |
|
||||
|
||||
Gravity of 900 px/s² with a drag of 1.5 gives a terminal fall speed of 600 px/s. Drag is what
|
||||
bounds a fall, so do not set it to zero. (A `terminal_velocity` setting is in `TODO.md` under
|
||||
"Arcade physics feel".)
|
||||
bounds a fall, so do not set it to zero. (A `terminal_velocity` setting is issue #29.)
|
||||
|
||||
Putting physics in the map rather than in code is what lets a swimming level and a walking
|
||||
level differ by data.
|
||||
@@ -816,7 +815,7 @@ Then the map:
|
||||
|
||||
**Load into `akgl_gamemap`, which already points at storage the library owns.** Do not
|
||||
declare an `akgl_Tilemap` on the stack: it is about 26 MB, several times a default thread
|
||||
stack, and you get a segfault before the loader writes a byte. Shrinking it is `TODO.md`
|
||||
stack, and you get a segfault before the loader writes a byte. Shrinking it is issue #28
|
||||
targets 14 and 15.
|
||||
|
||||
That one call creates an actor for every `actor` object in the object layer, binds each to
|
||||
@@ -1200,7 +1199,7 @@ Snap to zero below a pixel per second, because an exponential decay never actual
|
||||
|
||||
The library's own `akgl_actor_cmhf_left_off` zeroes `tx` outright, which stops the actor dead
|
||||
in one frame — right for a top-down game, wrong for a sidescroller. Friction and deceleration
|
||||
in the backend are tracked in `TODO.md` under "Arcade physics feel"; when they land, this
|
||||
in the backend are issues #29 through #32; when they land, this
|
||||
whole section becomes a setting.
|
||||
|
||||
### The whole movement function, in order
|
||||
@@ -1569,7 +1568,7 @@ else.
|
||||
underneath the registry that still points at them.
|
||||
|
||||
Do not call `akgl_tilemap_release` unless you are loading a second level — it has a
|
||||
double-free, recorded in `TODO.md` under "Known and still open" item 2. A process that is
|
||||
double-free, recorded in `TODO.md` under "Defects". A process that is
|
||||
exiting can leave the textures to `SDL_Quit`.
|
||||
|
||||
### Reporting a failure from `main`
|
||||
|
||||
@@ -536,7 +536,7 @@ state is skipped rather than reported, so every NPC in the town disappears on fr
|
||||
silently, and so does the player as soon as they stop walking.
|
||||
|
||||
Clearing the field leaves the facing bits wherever they were last set, which is what a
|
||||
top-down game wants. `TODO.md` tracks making the default behave; until then this loop belongs
|
||||
top-down game wants. Issue #39 tracks making the default behave; until then this loop belongs
|
||||
in every game with a standing NPC in it.
|
||||
|
||||
The loop covers the player as well as the NPCs, which is what you want — a player who stops
|
||||
@@ -748,7 +748,7 @@ The physics step writes a child's `x` and `y` as an absolute world position, and
|
||||
`akgl_actor_render` adds the parent's position to it a second time. Detaching takes the
|
||||
branch that does not add it, and `CLEANUP` puts the parent back on every path including the
|
||||
failing one — an actor left with a `NULL` parent would be simulated as a free agent on the
|
||||
next step. This is `TODO.md`, "Known and still open"; when the two agree on one reading, this
|
||||
next step. This is issue #37; when the two agree on one reading, this
|
||||
hook becomes `akgl_actor_render` again.
|
||||
|
||||
Run now. A second character walks a fixed distance behind and below the player.
|
||||
@@ -892,7 +892,7 @@ Guard against the empty string:
|
||||
```
|
||||
|
||||
`akgl_text_rendertextat` refuses a zero-width string while `akgl_text_measure` accepts one.
|
||||
The disagreement is recorded in `TODO.md` under "Known and still open"; the check above is
|
||||
The disagreement is issue #37; the check above is
|
||||
the guard until it is settled.
|
||||
|
||||
### Drawing it over the world
|
||||
@@ -1132,8 +1132,8 @@ than the `NULL` chapter 20 passes. `AKGL_ITERATOR_OP_UPDATE` asks for the update
|
||||
registry and `SDL_Quit` destroys it, taking the last reference to every font with no way left
|
||||
to close them.
|
||||
|
||||
`akgl_tilemap_release` is not called. It double-frees tileset textures, which is `TODO.md`
|
||||
"Known and still open" item 2; `SDL_Quit` reclaims them correctly. A game that loads a second
|
||||
`akgl_tilemap_release` is not called. It double-freed tileset textures, which is `TODO.md`
|
||||
"Defects"; `SDL_Quit` reclaims them correctly. A game that loads a second
|
||||
level has to unwind properly, and that is what the fix will make possible.
|
||||
|
||||
The pools are static storage and the process is exiting, so there is nothing else to free.
|
||||
|
||||
@@ -301,7 +301,8 @@ A ninth child is `AKERR_OUTOFBOUNDS`; an over-long name is silently truncated.
|
||||
|
||||
`akgl_sprite_load_json` bounds the `frames` array against `AKGL_SPRITE_MAX_FRAMES` before
|
||||
writing anything, and refuses a frame number that will not fit a `uint8_t` rather than
|
||||
truncating it into an index naming a different tile (`TODO.md` item 16).
|
||||
truncating it into an index naming a different tile (`TODO.md`, "Found while rewriting
|
||||
the Doxygen comments").
|
||||
|
||||
**`AKGL_SPRITE_MAX_REGISTRY_SIZE` is dead.** It is defined in `sprite.h` and referenced
|
||||
nowhere in `src/`, `include/`, `tests/` or `util/`. It bounds nothing. Do not size anything
|
||||
@@ -335,8 +336,8 @@ Three corrections to what those comments say, all verified against `src/tilemap.
|
||||
- **`AKGL_TILEMAP_MAX_OBJECTS_PER_LAYER` *is* enforced.** The comment above says it is not.
|
||||
`akgl_tilemap_load_layer_objects` bounds `j` at the top of the loop body and raises
|
||||
`AKERR_OUTOFBOUNDS`; `akgl_tilemap_load_tilesets` does the same for
|
||||
`AKGL_TILEMAP_MAX_TILESETS`. Both landed in 0.5.0 (`TODO.md`, "Known and still open"
|
||||
item 17), and the header comment was not updated with them.
|
||||
`AKGL_TILEMAP_MAX_TILESETS`. Both landed in 0.5.0 (`TODO.md`, "Found while rewriting
|
||||
the Doxygen comments"), and the header comment was not updated with them -- issue #63.
|
||||
- **Width and height are not bounded individually.** `akgl_tilemap_load` checks
|
||||
`width * height >= AKGL_TILEMAP_MAX_WIDTH * AKGL_TILEMAP_MAX_HEIGHT`, and the comparison
|
||||
is `>=`, so the true ceiling is **262143 tiles** in any shape. A 1024×256 map loads; a
|
||||
|
||||
@@ -58,7 +58,7 @@ that is not C in any dialect, and, in the first snippet a reader ever saw, the e
|
||||
chapters turned up **twenty-seven** header claims that were false against `src/`.
|
||||
|
||||
Where a chapter documents behaviour that is a known defect rather than a design decision,
|
||||
it says so and points at `TODO.md`. See `MAINTENANCE.md` if you are editing an example.
|
||||
it says so and points at its issue. See `MAINTENANCE.md` if you are editing an example.
|
||||
|
||||
## Assets
|
||||
|
||||
|
||||
@@ -347,8 +347,7 @@ static akerr_ErrorContext *frame(long frameno)
|
||||
* - akgl_tilemap_release is **not** called. Its layer loop destroys
|
||||
* `tilesets[i].texture` rather than `layers[i].texture`, so it double-frees
|
||||
* every tileset texture and never frees an image layer's, and it NULLs
|
||||
* nothing, so a second call is a use-after-free. TODO.md, "Known and still
|
||||
* open" item 2. `SDL_Quit` reclaims the textures correctly; calling the
|
||||
* nothing, so a second call is a use-after-free. TODO.md, "Defects". `SDL_Quit` reclaims the textures correctly; calling the
|
||||
* function that is supposed to would be worse than not.
|
||||
* - The pools are static storage. There is nothing to free and the process is
|
||||
* about to exit.
|
||||
|
||||
@@ -83,8 +83,8 @@ akerr_ErrorContext *jrpg_textbox_draw(void)
|
||||
// akgl_text_rendertextat refuses the empty string -- SDL_ttf reports "Text
|
||||
// has zero width" and the library passes that on as AKERR_NULLPOINTER,
|
||||
// while akgl_text_measure accepts it. The two disagree, so anything drawing
|
||||
// a line that might be empty checks for it. TODO.md, "Known and still
|
||||
// open".
|
||||
// a line that might be empty checks for it. TODO.md, "Found while
|
||||
// rewriting the Doxygen comments".
|
||||
if ( textbox_text[0] == '\0' ) {
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
* `movementlogicfunc`, which is the only hook the step calls.
|
||||
*
|
||||
* Two of the library's documented gaps are worked around here rather than
|
||||
* papered over. Both are in `TODO.md` under "Arcade physics feel".
|
||||
* papered over. Both are issues #29 through #32.
|
||||
*/
|
||||
|
||||
#include <math.h>
|
||||
@@ -324,7 +324,7 @@ akerr_ErrorContext *ss_player_controls(int controlmapid, char *actorname)
|
||||
FAIL_ZERO_RETURN(errctx, actorname, AKERR_NULLPOINTER, "actorname");
|
||||
/* akgl_controller_pushmap checks the upper bound and not the lower one, so
|
||||
* a negative id indexes before the start of akgl_controlmaps. TODO.md,
|
||||
* "Known and still open" item 11. */
|
||||
* "Defects". */
|
||||
FAIL_NONZERO_RETURN(
|
||||
errctx,
|
||||
((controlmapid < 0) || (controlmapid >= AKGL_MAX_CONTROL_MAPS)),
|
||||
|
||||
@@ -74,7 +74,7 @@ struct akgl_CollisionWorld;
|
||||
*
|
||||
* Sub-stepping bounds how far an actor moves between collision tests, which
|
||||
* stops anything moving at ordinary speeds from passing through a wall. A
|
||||
* projectile is not moving at ordinary speeds. See `TODO.md`.
|
||||
* projectile is not moving at ordinary speeds. See issue #56.
|
||||
*/
|
||||
#define AKGL_COLLISION_FLAG_BULLET 0x00000008u
|
||||
|
||||
@@ -557,8 +557,8 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_collision_sync_actors(akgl_CollisionWorl
|
||||
* Follows the convention every pool here uses: akgl_heap_next_collision_proxy
|
||||
* finds a free slot and does **not** claim it, and this takes the reference.
|
||||
* Write the two adjacent, because until the reference is taken the slot is still
|
||||
* free and the next acquire hands out the same pointer. That is `TODO.md` "Known
|
||||
* and still open" item 8, it applies to four of the five existing pools, and
|
||||
* free and the next acquire hands out the same pointer. That asymmetry is issue
|
||||
* #14, it applies to four of the five existing pools, and
|
||||
* this one does not depart from it -- a sixth convention would be worse than the
|
||||
* defect.
|
||||
*
|
||||
|
||||
@@ -231,7 +231,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_controller_handle_removed(void *appstate
|
||||
*
|
||||
* @warning A **negative** @p controlmapid is not rejected -- only the upper
|
||||
* bound is checked -- and indexes before the start of
|
||||
* ::akgl_controlmaps. TODO.md, "Known and still open" item 11.
|
||||
* ::akgl_controlmaps. TODO.md, "Defects".
|
||||
*/
|
||||
akerr_ErrorContext AKERR_NOIGNORE *akgl_controller_pushmap(int controlmapid, akgl_Control *control);
|
||||
|
||||
|
||||
@@ -51,7 +51,7 @@
|
||||
// AKERR_OUTOFBOUNDS on truncation instead. That is the better answer: the
|
||||
// compiler was right both times, and silencing it left the truncation
|
||||
// happening and unreported. Kept because they are public and a consumer may
|
||||
// have them; see TODO.md.
|
||||
// have them; see issue #20.
|
||||
#define DISABLE_GCC_WARNING_FORMAT_TRUNCATION \
|
||||
_Pragma("GCC diagnostic push") \
|
||||
_Pragma("GCC diagnostic ignored \"-Wformat-truncation\"")
|
||||
|
||||
@@ -217,7 +217,7 @@ void akgl_game_update_fps(void);
|
||||
*
|
||||
* @note This is a partial implementation: the name tables are written but the
|
||||
* objects themselves are not, so the file is not yet enough to restore a
|
||||
* session. See also TODO.md, "Known and still open" item 7 -- the writer
|
||||
* session. See also TODO.md, "Defects" -- the writer
|
||||
* and the reader disagree on the name-field widths, so a save with any
|
||||
* registered spritesheet cannot be read back.
|
||||
*/
|
||||
|
||||
@@ -22,8 +22,8 @@
|
||||
* reference for you, and the other four do not -- their caller is
|
||||
* expected to take it, which in practice the `*_initialize` function
|
||||
* does. Until one is taken the slot is still free and the next
|
||||
* allocation hands out the same pointer. TODO.md, "Known and still
|
||||
* open" item 8.
|
||||
* allocation hands out the same pointer. Whether to make them symmetric
|
||||
* or document the split is issue #14.
|
||||
*/
|
||||
|
||||
#ifndef _AKGL_HEAP_H_
|
||||
|
||||
@@ -23,7 +23,7 @@
|
||||
* no-op akgl_set_property and an akgl_get_property that always hands
|
||||
* back the caller's default, which in turn means akgl_render_2d_init
|
||||
* and akgl_physics_init_arcade quietly ignore their configuration.
|
||||
* TODO.md, "Known and still open" item 3.
|
||||
* TODO.md, "Formatting and hygiene".
|
||||
*/
|
||||
|
||||
#ifndef _AKGL_REGISTRY_H_
|
||||
@@ -170,7 +170,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_load_properties(char *fname);
|
||||
* @note The name table it reads disagrees with actor.h in two places: bits 11
|
||||
* and 12 are named `UNDEFINED_11`/`UNDEFINED_12` rather than `MOVING_IN`
|
||||
* and `MOVING_OUT`, so those two states cannot be named from JSON at all.
|
||||
* TODO.md items 24-26.
|
||||
* TODO.md, "AKGL_ACTOR_STATE_STRING_NAMES".
|
||||
*/
|
||||
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_actor_state_strings(void);
|
||||
/**
|
||||
|
||||
@@ -123,8 +123,9 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_text_unloadallfonts(void);
|
||||
* @note The empty string is **refused**, not drawn as nothing: SDL_ttf reports
|
||||
* "Text has zero width" and this passes that on as `AKERR_NULLPOINTER`.
|
||||
* akgl_text_measure() accepts it, so the two disagree. A caller drawing a
|
||||
* line of text that may be empty has to check for it. TODO.md, "Known and
|
||||
* still open".
|
||||
* line of text that may be empty has to check for it. Stale: the two agree
|
||||
* as of 0.5.0 -- an empty string returns success without rasterizing. See
|
||||
* TODO.md, "Found while rewriting the Doxygen comments", and issue #63.
|
||||
*/
|
||||
akerr_ErrorContext AKERR_NOIGNORE *akgl_text_rendertextat(TTF_Font *font, char *text, SDL_Color color, int wraplength, int x, int y);
|
||||
/**
|
||||
|
||||
@@ -484,7 +484,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_tilemap_load_tilesets(akgl_Tilemap *dest
|
||||
* than `layers[i].texture`, so tileset textures are destroyed twice and
|
||||
* image-layer textures never. Nothing is set to `NULL` either, so a
|
||||
* second call is a use-after-free. It also does not release the actors
|
||||
* the map's object layers created. TODO.md, "Known and still open"
|
||||
* the map's object layers created. TODO.md, "Defects"
|
||||
* item 2.
|
||||
*/
|
||||
akerr_ErrorContext AKERR_NOIGNORE *akgl_tilemap_release(akgl_Tilemap *dest);
|
||||
|
||||
@@ -93,7 +93,7 @@ typedef struct {
|
||||
* There is no rotation, here or anywhere in this library yet -- `util.h` says so
|
||||
* and both example games depend on it. Adding it means rotating @p dir into the
|
||||
* shape's local frame at the top of this function and rotating the answer back,
|
||||
* and nothing else in the narrowphase changes. See `TODO.md`, "Actor rotation".
|
||||
* and nothing else in the narrowphase changes. See issue #62.
|
||||
*/
|
||||
static void collision_support(const void *obj, const ccd_vec3_t *dir, ccd_vec3_t *dest)
|
||||
{
|
||||
|
||||
@@ -411,7 +411,7 @@ static void save_spritename_iterator(void *userdata, SDL_PropertiesID props, con
|
||||
* save_actorname_iterator.
|
||||
* @note This is the table the loader disagrees with: it reads every table at
|
||||
* #AKGL_ACTOR_MAX_NAME_LENGTH, so a save containing any spritesheet cannot
|
||||
* be read back. TODO.md, "Known and still open" item 7.
|
||||
* be read back. TODO.md, "Defects".
|
||||
*/
|
||||
static void save_spritesheetname_iterator(void *userdata, SDL_PropertiesID props, const char *name)
|
||||
{
|
||||
|
||||
@@ -66,7 +66,7 @@ static akerr_ErrorContext *load_fixture(void)
|
||||
* usually because something is not releasing -- arrived as a segfault inside
|
||||
* strncpy.
|
||||
*
|
||||
* This is what TODO.md Performance item 29 looked like from the outside: not
|
||||
* This is what the tilemap string-pool leak looked like from the outside: not
|
||||
* "the tilemap loader leaks five strings a load", but a crash somewhere else
|
||||
* entirely, fifty levels later.
|
||||
*/
|
||||
|
||||
@@ -483,7 +483,7 @@ static akerr_ErrorContext *bench_asset_load(void)
|
||||
// The 52nd load would find the pool empty, and what happens then is
|
||||
// not an AKGL_ERR_HEAP: akgl_get_json_string_value finishes with
|
||||
// pass-up false, swallows the failed claim, and dereferences the
|
||||
// pointer it never set. Both defects are filed in TODO.md; this loop
|
||||
// pointer it never set. Both defects are recorded in TODO.md; this loop
|
||||
// works around the first so it never reaches the second.
|
||||
for ( j = 0; j < AKGL_MAX_HEAP_STRING; j++ ) {
|
||||
akgl_heap_strings[j].refcount = 0;
|
||||
|
||||
@@ -31,7 +31,7 @@
|
||||
* inside simulate produced a longer step and a red suite. It failed
|
||||
* exactly once, under a parallel ctest, which is the worst way to find
|
||||
* out. That the engine cannot be stepped exactly is itself a finding;
|
||||
* see TODO.md.
|
||||
* see issues #29 through #32.
|
||||
*/
|
||||
|
||||
#include <SDL3/SDL.h>
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
*
|
||||
* akgl_render_2d_init() is not covered here: it creates a window from the
|
||||
* property registry and writes the `camera` global, which is what the offscreen
|
||||
* harness described in TODO.md exists to make testable.
|
||||
* harness described in issue #6 exists to make testable.
|
||||
*/
|
||||
|
||||
#include <SDL3/SDL.h>
|
||||
|
||||
Reference in New Issue
Block a user