Drop the savegame name-width aliases
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>
This commit is contained in:
31
TODO.md
31
TODO.md
@@ -718,11 +718,10 @@ Each was found by a test written to assert correct behavior.
|
||||
header documented that as behaviour. It is `AKERR_OUTOFBOUNDS` now, and a
|
||||
negative count is refused too.
|
||||
7. **Savegame name lengths disagree between writer and reader.** **Fixed in
|
||||
0.5.0.** Four `AKGL_GAME_SAVE_*_NAME_WIDTH` constants now drive both sides,
|
||||
so the two cannot drift again. The writer used each object's own maximum name
|
||||
length -- 512 for a spritesheet, which is a filename -- and the reader used
|
||||
`AKGL_ACTOR_MAX_NAME_LENGTH` for all four. The other three are 128 as well, so
|
||||
only the spritesheet table was wrong.
|
||||
0.5.0.** The reader names the same constant the writer does, per table --
|
||||
`AKGL_SPRITE_SHEET_MAX_FILENAME_LENGTH` for spritesheets and so on. It used
|
||||
`AKGL_ACTOR_MAX_NAME_LENGTH` for all four; the other three are 128 as well,
|
||||
so only the spritesheet table was wrong, and that was enough.
|
||||
|
||||
**The failure was worse than "cannot be read back", which is what made the
|
||||
test interesting.** The tables carry no length prefix and end at a zeroed
|
||||
@@ -731,16 +730,26 @@ Each was found by a test written to assert correct behavior.
|
||||
success with silently wrong maps. A test that only asserted the load
|
||||
succeeded passed against the broken reader.
|
||||
|
||||
So `akgl_game_load` now checks that the stream is at EOF once the four tables
|
||||
are read. That turns a width disagreement into `AKERR_IO` instead of a
|
||||
corruption, and it is the assertion `tests/game.c` actually hangs the test
|
||||
on. The new roundtrip test registers a name in each of the four registries,
|
||||
with a full-length one in the spritesheet registry, and against a mismatched
|
||||
reader it fails with `AKERR_IO`.
|
||||
So `akgl_game_load` checks the stream is at EOF once the four tables are
|
||||
read. That is what turns a width disagreement into `AKERR_IO` instead of a
|
||||
corruption, and it is the assertion `tests/game.c` hangs on. The new
|
||||
roundtrip test registers a name in each of the four registries, with a
|
||||
full-length one in the spritesheet registry, and fails with `AKERR_IO`
|
||||
against a mismatched reader.
|
||||
|
||||
That EOF check has to move when the objects themselves start being written;
|
||||
there is a comment at the site saying so.
|
||||
|
||||
A first pass at this introduced four `AKGL_GAME_SAVE_*_NAME_WIDTH` aliases,
|
||||
one per table, on the reasoning that the on-disk format's widths are a
|
||||
separate concern from the object model's. They were removed. Each expanded to
|
||||
exactly one existing constant and had exactly one use per side, so they were
|
||||
indirection with no second consumer -- and the divergence they anticipated
|
||||
cannot happen quietly anyway: raising one of those lengths 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 table. What actually
|
||||
guards the widths is the EOF check, which works however they are spelled.
|
||||
|
||||
8. **Heap acquire functions are asymmetric.** `akgl_heap_next_string` increments
|
||||
`refcount`; `next_actor`, `next_sprite`, `next_spritesheet`, and
|
||||
`next_character` do not. `tests/heap.c` pins the current behavior and says so;
|
||||
|
||||
Reference in New Issue
Block a user