diff --git a/TODO.md b/TODO.md index 1fab2e0..a9f6024 100644 --- a/TODO.md +++ b/TODO.md @@ -211,175 +211,172 @@ resolved for every pair it listed. ### 4. Types and macros -19. **`float`/`double` are used raw where `types.h` defines aliases.** `types.h` - exports `float32_t` and `float64_t`, and the actor and character structs use - `float32_t` throughout — but `akgl_get_json_number_value` takes `float *` - (`json_helpers.h:55`), `akgl_get_json_double_value` takes `double *` (line 66), - `akgl_PhysicsBackend`'s six drag/gravity fields are `double` - (`physics.h:22-27`), and `akgl_Tilemap`'s perspective fields are `float` - (`tilemap.h:105-108`). Either use the aliases consistently or delete them. +**Items 19 through 23 are resolved in 0.5.0.** -20. **`AKGL_COLLIDE_RECTANGLES` (`include/akgl/util.h:27`) has unbalanced - parentheses** — three opens, two closes — so any use is a syntax error. It has - no callers and duplicates `akgl_collide_rectangles`. Delete it. +19. **`float`/`double` used raw where `types.h` defines aliases.** The four + signatures and twelve struct fields that spelled them out now use + `float32_t` and `float64_t` like the actor and character structs. They are + plain typedefs, so this is a spelling change and not an ABI one. + `float64_t`'s doc no longer says "unused so far". -21. **Bitmask macros are unparenthesized.** `AKGL_BITMASK_HAS(x, y)` expands to - `(x & y) == y` with no outer parens, so `!AKGL_BITMASK_HAS(a, b)` parses as - `!(a & b) == b`. No in-tree caller negates it today (`AKGL_BITMASK_HASNOT` - exists for that), so this is latent rather than live. `AKGL_BITMASK_CLEAR(x)` - (`game.h:86`) also carries a trailing semicolon inside the macro body, so - normal use produces an empty statement. Parenthesize all five and drop the - semicolon. +20. **`AKGL_COLLIDE_RECTANGLES` had unbalanced parentheses.** Deleted. Three + opens against two closes meant any expansion was a syntax error, it had no + callers, and it duplicated `akgl_collide_rectangles`. -22. **The state and iterator bit macros mix forms.** `AKGL_ITERATOR_OP_UPDATE` - (`iterator.h:15`) is written `1` while its 31 siblings are `1 << n`, and none - of the `1 << n` values in `iterator.h` or `actor.h:15-49` are parenthesized. - The hand-maintained trailing bit-pattern comments in `actor.h:34-49` are also - wrong for the high word — they restart at `0000 0000 0000 0001` for bit 16 - rather than showing the full 32-bit value. +21. **Bitmask macros were unparenthesized.** All five are fully parenthesized, + and `AKGL_BITMASK_CLEAR` no longer carries a semicolon inside its body. -23. **`akgl_Frame` (`game.h:27-31`) is defined and never used** anywhere in - `src/`, `include/`, `tests/`, or `util/`. + `tests/bitmasks.c` covers the composition cases, and writing them turned up + something worth recording: the obvious test does **not** catch this. For a + bit that is *set*, `!AKGL_BITMASK_HAS(mask, bit)` misparses to + `!(mask & bit) == bit`, which is `0 == bit` -- false, the same answer the + correct parse gives. It only diverges for a bit that is *not* set and whose + value is not 1: `!(0)` is 1, and `1 == 64` is false where the answer should + be true. The suite now uses that shape, and fails against the old macros. + +22. **The state and iterator bit macros mixed forms.** + `AKGL_ITERATOR_OP_UPDATE` is `(1 << 0)` like its 31 siblings, and every + `1 << n` in `iterator.h` and `actor.h` is parenthesized, with the + hand-aligned value columns preserved. + + The bit-pattern comments in `actor.h` are not wrong so much as unlabelled: + each shows the pattern *within its own 16-bit half*, which is why bit 16 + looks like it restarts at bit 0. The section headings say so now. Rendering + the full 32-bit value instead would push those lines past the 100-column + fill. + + **Newly recorded, and still open:** `1 << 31` is undefined behaviour on a + signed `int`. It is `(1 << 31)` in both tables and wants to be an unsigned + shift, but `akgl_Actor::state` is `int32_t` and `akgl_Iterator::flags` is + `uint32_t`, so the two tables do not want the same answer. Worth deciding + deliberately rather than sneaking a `u` in. + +23. **`akgl_Frame` was defined and never used.** Deleted. ### 5. `AKGL_ACTOR_STATE_STRING_NAMES` disagrees with `actor.h` -24. **The array bound differs between declaration and definition.** - `include/akgl/actor.h:57` declares `[AKGL_ACTOR_MAX_STATES+1]` (33); - `src/actor_state_string_names.c:6` defines `[32]`. Any consumer that trusts - the declared bound and reads index 32 reads past the object. +**All three resolved in 0.5.0.** -25. **Two entries name the wrong bit.** `actor.h` assigns bit 11 to - `AKGL_ACTOR_STATE_MOVING_IN` and bit 12 to `AKGL_ACTOR_STATE_MOVING_OUT`, but - `actor_state_string_names.c:18-19` puts `"AKGL_ACTOR_STATE_UNDEFINED_11"` and - `"AKGL_ACTOR_STATE_UNDEFINED_12"` at those indices — names for macros that do - not exist. Since `akgl_registry_init_actor_state_strings` - (`src/registry.c:79-94`) builds `AKGL_REGISTRY_ACTOR_STATE_STRINGS` from this - array and `akgl_character_load_json_state_int_from_strings` - (`src/character.c:101`) resolves character-JSON state names through it, a - character JSON can never bind a sprite to `MOVING_IN` or `MOVING_OUT`. +24. **The array bound differed between declaration and definition.** The + header declared `[AKGL_ACTOR_MAX_STATES+1]` (33) and the definition was a + literal `[32]`, so a consumer trusting the declared bound read past the + object. Both are `[AKGL_ACTOR_MAX_STATES]` now, and the definition is sized + by the macro rather than by a literal. -26. **The generation comment is stale.** `actor.h:53-55` says the file "is built - by a utility script and not kept in git, see the Makefile for - lib_src/actor_state_string_names.c". There is no Makefile (the project is - CMake), no `lib_src/`, no such script under `scripts/` or `util/`, and the - file *is* tracked in git at `src/actor_state_string_names.c`. Either restore - the generator — which would fix items 24 and 25 by construction — or delete - the comment and maintain the file by hand. +25. **Two entries named the wrong bit.** Indices 11 and 12 said + `AKGL_ACTOR_STATE_UNDEFINED_11` and `_12` where `actor.h` has `MOVING_IN` + and `MOVING_OUT`, so no character JSON could ever bind a sprite to either + state -- the name it would have to write was not in the registry. + + `tests/registry.c` now walks the whole table: every entry non-`NULL`, every + entry resolving to its own bit through + `AKGL_REGISTRY_ACTOR_STATE_STRINGS`, no two entries sharing a name (a + duplicate silently overwrites and makes one bit unreachable), and + `MOVING_IN`/`MOVING_OUT` named explicitly so a regression reads as what it + is. + +26. **The generation comment was stale.** There is no generator, no Makefile + and no `lib_src/`. The comment is gone and the file's own header now says it + is maintained by hand and states the two invariants that keep breaking. ### 6. Doxygen drift -27. **Three struct doc comments in `tilemap.h` are rotated by one.** - `akgl_TilemapObject` (line 31) is documented as "Stores tileset metadata, - texture, and frame offsets" (that is `akgl_Tileset`); `akgl_TilemapLayer` - (line 46) as "Represents an object embedded in a tilemap layer" (that is - `akgl_TilemapObject`); `akgl_Tileset` (line 61) as "Stores tile, image, or - object data for one map layer" (that is `akgl_TilemapLayer`). +27. **Three struct doc comments in `tilemap.h` were rotated by one.** Already + correct -- the doxygen rewrite fixed this before it was checked here. + `akgl_TilemapObject`, `akgl_TilemapLayer` and `akgl_Tileset` each describe + themselves. -28. **`point` is documented as "Represents a two-dimensional point"** - (`util.h:12`) but has `x`, `y`, and `z`. +28. **`point` documented as two-dimensional with an `x`, `y` and `z`.** Already + correct, and the type is `akgl_Point` now. -29. **Doc comments live on the definition for public functions.** The convention - is header-side documentation, but `akgl_path_relative_root` (`util.c:23`), - `akgl_path_relative_from` (`util.c:97`), the four `gamepad_handle_*` - (`controller.c:114+`), the `akgl_game_save_*` iterators and - `akgl_game_load_*` helpers (`game.c:173+`), and the tilemap helpers - (`tilemap.c:90+`) are documented only in the `.c`. This is the same set as - item 7 — resolving that resolves this. +29. **Doc comments on the definition rather than the header.** Resolved with + item 7: the four controller handlers and the six tilemap loader helpers had + their documentation moved to the header when they were declared there. -### 7. Formatting +### 7. Formatting and hygiene -30. **A minority of files use a 2-column offset instead of the canonical - 4-column one.** The mix of tabs and spaces across most of `src/` is *not* - disorder: it is exactly what Emacs `cc-mode` emits for the `stroustrup` style - with `indent-tabs-mode t` and `tab-width 8` — depth 1 is four spaces, depth 2 - is one tab, depth 3 is a tab plus four spaces, and so on. Twelve of the - seventeen `.c` files follow that ladder cleanly. +**Items 31 through 36, 38 and 41 are resolved in 0.5.0. Item 37 is not; see +below.** - The genuine outliers indent at 2 columns: `src/json_helpers.c` (82 lines, - except `akgl_get_json_with_default` at line 149 which is 4), `src/util.c` - (37 lines, from `akgl_rectangle_points` at line 118 onward), `src/assets.c` - (9), `src/staticstring.c` (7), `src/actor.c` (8, in `akgl_actor_add_child` - at line 272), plus `include/akgl/util.h` (7) and - `include/akgl/staticstring.h` (2). +31. **Leftover debug code.** The four `SDL_Log` lines in `src/controller.c` + guarded by `event->type == 768 && event->key.which == 11 && + event->key.key == 13` -- decimal literals for one keyboard on one + developer's machine, inside the per-event inner loop -- are gone. - **Resolved.** `AGENTS.md` now specifies the canonical style and the exact - `cc-mode` settings; `.dir-locals.el` applies them in Emacs; and - `scripts/reindent.sh` applies them in batch. The whole tree (32 files across - `src/`, `include/`, `tests/`, and `util/`) has been reindented and is a fixed - point of `scripts/reindent.sh --check`. Verified whitespace-only: apart from - three trailing blank lines removed at EOF in `src/heap.c`, `src/registry.c`, - and `tests/tilemap.c`, `git diff -w` over the reindent is empty, and the test - results are unchanged (13/14, `character` still the intentional failure). - `scripts/hooks/pre-commit` keeps it that way — enable with - `git config core.hooksPath scripts/hooks`. +32. **Large commented-out blocks.** The five abandoned `SDL_GetBasePath()` + path-prefixing lines across `assets.c`, `sprite.c`, `character.c` and + `tilemap.c` are gone. They were superseded by `akgl_path_relative`. -31. **Leftover debug code ships in the library.** `src/controller.c:91-97` logs - four lines whenever `event->type == 768 && event->key.which == 11 && - event->key.key == 13` — hardcoded decimal values for a specific keyboard ID - on a specific developer's machine, inside the per-event inner loop. +33. **Unused locals.** `curTime` in `akgl_game_update` and in + `akgl_render_2d_draw_world`, `j` in `akgl_render_2d_draw_world` (shadowed + by its own inner loop), `target` in `akgl_character_sprite_get`, both + `result` declarations in `util.c`, and `screenwidth`/`screenheight` in + `akgl_game_init`. `opflags` in `akgl_heap_release_character` is no longer + unused -- it is what drives the state-sprite walk added for **Defects** + item 21. -32. **Large commented-out blocks.** `src/sprite.c:115-120,157,185-198,210`; - `src/character.c:192-199,215`; `src/assets.c:18-27,50`; - `src/tilemap.c:583,596-598,640`. All are the same abandoned - `SDL_GetBasePath()` path-prefixing approach, superseded by - `akgl_path_relative`. Delete them. +34. **`akgl_game_update`'s default flags OR-ed the same bit twice.** It reads + `AKGL_ITERATOR_OP_UPDATE | AKGL_ITERATOR_OP_LAYERMASK` now. This is a + statement of intent rather than a behaviour change, and the reason is worth + knowing: **nothing in that loop reads either bit**. It never compares + `actor->layer` to the layer it is sweeping, which is exactly **Performance** + item 32 -- every actor updated sixteen times a frame -- and that is still + open. -33. **Unused locals.** `screenwidth`/`screenheight` (`game.c:53-54`), `curTime` - (`game.c:499`), `curTime` and `j` (`renderer.c:114,116` — `j` is also shadowed - by the inner loop at line 128), `target` (`character.c:59`), `result` - (`util.c:37,73`), `opflags` (`heap.c:146`, declared and cleared but never - read). +35. **`akgl_draw_background` was the only public function outside the error + protocol.** It now takes an `akgl_RenderBackend *` like every other entry + point in `draw.h`, returns an error context, checks the backend and its + `sdl_renderer`, and restores the draw colour it found instead of leaving it + changed. -34. **`akgl_game_update`'s default flags OR the same bit twice.** - `src/game.c:496` reads - `(AKGL_ITERATOR_OP_LAYERMASK | AKGL_ITERATOR_OP_LAYERMASK)`. Given the loop - that follows walks layers and calls `updatefunc`, the intent was almost - certainly `AKGL_ITERATOR_OP_UPDATE | AKGL_ITERATOR_OP_LAYERMASK`. + It was listed under **Remaining work** as needing the offscreen renderer + harness. It does not, and never did -- what it needed was to stop reading + the global. `tests/draw.c` covers the checkerboard pattern, the restored + draw colour, zero and negative sizes, and a `NULL` backend. -35. **`akgl_draw_background` is the only public function outside the error - protocol.** `include/akgl/draw.h:14` returns `void` and reports nothing; - every other public entry point returns `akerr_ErrorContext *`. It also calls - `SDL_SetRenderDrawColor` and `SDL_RenderFillRect` without checking `renderer` - or `renderer->sdl_renderer`. +36. **`akgl_registry_init_actor` was the only initializer that destroyed the + registry it was replacing.** All eight go through one `registry_create()` + helper now, so the other seven stop leaking an `SDL_PropertiesID` per call + after the first -- which a game that resets between levels makes on every + level. -36. **`akgl_registry_init_actor` is the only registry initializer that destroys - an existing registry** before recreating it (`src/registry.c:47-49`). The - other seven leak the old `SDL_PropertiesID` on a second call. Either all of - them should do it or none should. + Fixed alongside it, because it is the same function: `akgl_registry_init()` + never called `akgl_registry_init_properties()`, which is **Defects → Known + and still open** item 3. `AKGL_REGISTRY_PROPERTIES` stayed 0 for any caller + that did not also go through `akgl_game_init`, making `akgl_set_property` a + silent no-op and `akgl_get_property` always return the caller's default -- + so `akgl_physics_init_arcade` and `akgl_render_2d_init` quietly ignored + their configuration. `tests/registry.c` sets a property and reads it back. -37. **Redundant casts obscure the code.** `(json_t *)json` where `json` is - already `json_t *`, `(akgl_Tilemap *)dest`, `(akgl_Actor *)actorobj`, - `(char *)&obj->name` on an array that already decays — roughly 180 pointer - casts across `src/`, a large majority of them no-ops, densest in - `src/tilemap.c:150-624` and `src/character.c:144-172`. - They suppress exactly the conversion warnings that would catch a real - mismatch. +37. **Redundant casts obscure the code.** **Still open, and deliberately so.** + Roughly 180 pointer casts across `src/`, a large majority no-ops -- + `(json_t *)json` where `json` is already `json_t *`, `(char *)&obj->name` on + an array that decays anyway -- densest in `src/tilemap.c` and + `src/character.c`. -38. **`struct`-qualified parameters in two definitions.** - `akgl_physics_null_move` (`src/physics.c:29`) and `akgl_physics_arcade_move` - (`src/physics.c:80`) are defined with `struct akgl_PhysicsBackend *self` - while the header and the other eight physics functions use the typedef. + The argument for fixing it is real: a no-op cast suppresses exactly the + conversion warning that would catch a mismatch. The argument for not doing + it *here* is that it is the largest and least mechanical change on this + list, it touches almost every line of the two files with the most + outstanding functional defects, and the benefit only arrives once the build + actually turns those warnings on -- which it does not today. Doing this + without `-Wall -Wextra` on the library target buys nothing but churn. -39. **`text.c:19` validates the wrong argument.** - `FAIL_ZERO_RETURN(errctx, name, AKERR_NULLPOINTER, "Null filepath")` checks - `name` a second time; `filepath` is never checked and is passed straight to - `TTF_OpenFont`. Copy-paste of line 18. + **Do them together**, in that order: enable the warnings, see what they say, + then remove the casts that were hiding something and the ones that were not. - **Resolved** alongside the text measurement work; `tests/text.c` asserts a - NULL filepath reports `AKERR_NULLPOINTER`. +38. **`struct`-qualified parameters in two definitions.** `akgl_physics_null_move` + and `akgl_physics_arcade_move` use the typedef like the other eight. -40. **`akgl_path_relative` and `akgl_path_relative_from` disagree on the output - parameter.** **Moot in 0.5.0**: `akgl_path_relative_from` is deleted (see - **Defects → Known and still open** item 4) and `akgl_path_relative_root` is - `static`, so `akgl_path_relative` is the only member of the family left and - there is nothing left to disagree with. It still takes `akgl_String *dst` - rather than the `**` form the rest of the library uses, which stays as - **item 41**'s question of naming and this one's question of shape -- but - with one function it is a decision to make when something needs it, not a - drift between siblings. +39. **`text.c` validated the wrong argument.** Resolved earlier, alongside the + text measurement work. + +40. **`akgl_path_relative_from` disagreed on the output parameter.** Moot: the + function is deleted. See **Defects → Known and still open** item 4. + +41. **`dst` vs `dest` for output parameters.** `akgl_string_copy` and + `akgl_path_relative` take `dest` now, like everything else. -41. **`dst` vs `dest` for output parameters.** `akgl_string_copy` and the - `akgl_path_relative*` family use `dst`; everything else uses `dest`. ## Test suites that could not fail diff --git a/include/akgl/actor.h b/include/akgl/actor.h index 7cbd935..0066606 100644 --- a/include/akgl/actor.h +++ b/include/akgl/actor.h @@ -37,49 +37,50 @@ #include // ---- LOW WORD STATUSES ---- +// +// The trailing column is the bit pattern within its own 16-bit half -- the +// low half here, the high half below -- not the full 32-bit value. Read as a +// whole word it looks like bit 16 restarts at bit 0, which it does not. -#define AKGL_ACTOR_STATE_FACE_DOWN 1 << 0 // 1 0000 0000 0000 0001 -#define AKGL_ACTOR_STATE_FACE_LEFT 1 << 1 // 2 0000 0000 0000 0010 -#define AKGL_ACTOR_STATE_FACE_RIGHT 1 << 2 // 4 0000 0000 0000 0100 -#define AKGL_ACTOR_STATE_FACE_UP 1 << 3 // 8 0000 0000 0000 1000 -#define AKGL_ACTOR_STATE_ALIVE 1 << 4 // 16 0000 0000 0001 0000 -#define AKGL_ACTOR_STATE_DYING 1 << 5 // 32 0000 0000 0010 0000 -#define AKGL_ACTOR_STATE_DEAD 1 << 6 // 64 0000 0000 0100 0000 -#define AKGL_ACTOR_STATE_MOVING_LEFT 1 << 7 // 128 0000 0000 1000 0000 -#define AKGL_ACTOR_STATE_MOVING_RIGHT 1 << 8 // 256 0000 0001 0000 0000 -#define AKGL_ACTOR_STATE_MOVING_UP 1 << 9 // 512 0000 0010 0000 0000 -#define AKGL_ACTOR_STATE_MOVING_DOWN 1 << 10 // 1024 0000 0100 0000 0000 -#define AKGL_ACTOR_STATE_MOVING_IN 1 << 11 // 2048 0000 1000 0000 0000 -#define AKGL_ACTOR_STATE_MOVING_OUT 1 << 12 // 4096 0001 0000 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_13 1 << 13 // 8192 0010 0000 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_14 1 << 14 // 16384 0100 0000 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_15 1 << 15 // 32768 1000 0000 0000 0000 +#define AKGL_ACTOR_STATE_FACE_DOWN (1 << 0) // 1 0000 0000 0000 0001 +#define AKGL_ACTOR_STATE_FACE_LEFT (1 << 1) // 2 0000 0000 0000 0010 +#define AKGL_ACTOR_STATE_FACE_RIGHT (1 << 2) // 4 0000 0000 0000 0100 +#define AKGL_ACTOR_STATE_FACE_UP (1 << 3) // 8 0000 0000 0000 1000 +#define AKGL_ACTOR_STATE_ALIVE (1 << 4) // 16 0000 0000 0001 0000 +#define AKGL_ACTOR_STATE_DYING (1 << 5) // 32 0000 0000 0010 0000 +#define AKGL_ACTOR_STATE_DEAD (1 << 6) // 64 0000 0000 0100 0000 +#define AKGL_ACTOR_STATE_MOVING_LEFT (1 << 7) // 128 0000 0000 1000 0000 +#define AKGL_ACTOR_STATE_MOVING_RIGHT (1 << 8) // 256 0000 0001 0000 0000 +#define AKGL_ACTOR_STATE_MOVING_UP (1 << 9) // 512 0000 0010 0000 0000 +#define AKGL_ACTOR_STATE_MOVING_DOWN (1 << 10) // 1024 0000 0100 0000 0000 +#define AKGL_ACTOR_STATE_MOVING_IN (1 << 11) // 2048 0000 1000 0000 0000 +#define AKGL_ACTOR_STATE_MOVING_OUT (1 << 12) // 4096 0001 0000 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_13 (1 << 13) // 8192 0010 0000 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_14 (1 << 14) // 16384 0100 0000 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_15 (1 << 15) // 32768 1000 0000 0000 0000 // ----- HIGH WORD STATUSES ----- -#define AKGL_ACTOR_STATE_UNDEFINED_16 1 << 16 // 65536 0000 0000 0000 0001 -#define AKGL_ACTOR_STATE_UNDEFINED_17 1 << 17 // 131072 0000 0000 0000 0010 -#define AKGL_ACTOR_STATE_UNDEFINED_18 1 << 18 // 262144 0000 0000 0000 0100 -#define AKGL_ACTOR_STATE_UNDEFINED_19 1 << 19 // 524288 0000 0000 0000 1000 -#define AKGL_ACTOR_STATE_UNDEFINED_20 1 << 20 // 1048576 0000 0000 0001 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_21 1 << 21 // 2097152 0000 0000 0010 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_22 1 << 22 // 4194304 0000 0000 0100 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_23 1 << 23 // 8388608 0000 0000 1000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_24 1 << 24 // 16777216 0000 0001 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_25 1 << 25 // 33554432 0000 0010 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_26 1 << 26 // 67108864 0000 0100 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_27 1 << 27 // 134217728 0000 1000 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_28 1 << 28 // 268435456 0001 0000 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_29 1 << 29 // 536870912 0010 0000 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_30 1 << 30 // 1073741824 0100 0000 0000 0000 -#define AKGL_ACTOR_STATE_UNDEFINED_31 1 << 31 // 2147483648 1000 0000 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_16 (1 << 16) // 65536 0000 0000 0000 0001 +#define AKGL_ACTOR_STATE_UNDEFINED_17 (1 << 17) // 131072 0000 0000 0000 0010 +#define AKGL_ACTOR_STATE_UNDEFINED_18 (1 << 18) // 262144 0000 0000 0000 0100 +#define AKGL_ACTOR_STATE_UNDEFINED_19 (1 << 19) // 524288 0000 0000 0000 1000 +#define AKGL_ACTOR_STATE_UNDEFINED_20 (1 << 20) // 1048576 0000 0000 0001 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_21 (1 << 21) // 2097152 0000 0000 0010 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_22 (1 << 22) // 4194304 0000 0000 0100 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_23 (1 << 23) // 8388608 0000 0000 1000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_24 (1 << 24) // 16777216 0000 0001 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_25 (1 << 25) // 33554432 0000 0010 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_26 (1 << 26) // 67108864 0000 0100 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_27 (1 << 27) // 134217728 0000 1000 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_28 (1 << 28) // 268435456 0001 0000 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_29 (1 << 29) // 536870912 0010 0000 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_30 (1 << 30) // 1073741824 0100 0000 0000 0000 +#define AKGL_ACTOR_STATE_UNDEFINED_31 (1 << 31) // 2147483648 1000 0000 0000 0000 /** @brief Bits in an actor's state word. Fixed by the width of `int32_t state`. */ #define AKGL_ACTOR_MAX_STATES 32 -// This is an array of strings equal to actor states from 1-32. -// This is built by a utility script and not kept in git, see -// the Makefile for lib_src/actor_state_string_names.c /** * @brief Bit position -> state name, as text. Index `i` names the bit `1 << i`. * @@ -87,17 +88,14 @@ * #AKGL_REGISTRY_ACTOR_STATE_STRINGS out of this, which is what lets character * JSON write `"AKGL_ACTOR_STATE_FACE_LEFT"` instead of `2`. A bit whose name * here does not match its `#define` above cannot be referred to from JSON at - * all. + * all -- which was the case for `MOVING_IN` and `MOVING_OUT` until 0.5.0. * - * @warning Three known defects, all tracked in TODO.md items 24-26: the - * definition in `src/actor_state_string_names.c` is 32 entries while - * this declares 33, so reading index 32 reads past the object; bits 11 - * and 12 are named `UNDEFINED_11`/`UNDEFINED_12` rather than - * `MOVING_IN`/`MOVING_OUT`; and the comment above about a generator - * script is stale -- there is no such script, and the file is tracked - * in git and maintained by hand. + * Maintained by hand in `src/actor_state_string_names.c`, and exactly + * #AKGL_ACTOR_MAX_STATES entries long: it was declared one longer than it was + * defined until 0.5.0, so a consumer trusting the declared bound read past the + * object. `tests/registry.c` checks every entry against its bit. */ -extern char *AKGL_ACTOR_STATE_STRING_NAMES[AKGL_ACTOR_MAX_STATES+1]; +extern char *AKGL_ACTOR_STATE_STRING_NAMES[AKGL_ACTOR_MAX_STATES]; /** @brief Every facing bit. Clear this before setting one, so an actor faces exactly one way. */ #define AKGL_ACTOR_STATE_FACE_ALL (AKGL_ACTOR_STATE_FACE_DOWN | AKGL_ACTOR_STATE_FACE_LEFT | AKGL_ACTOR_STATE_FACE_RIGHT | AKGL_ACTOR_STATE_FACE_UP) diff --git a/include/akgl/draw.h b/include/akgl/draw.h index 37ab41b..ced1e70 100644 --- a/include/akgl/draw.h +++ b/include/akgl/draw.h @@ -39,18 +39,27 @@ /** * @brief Paint an 8x8 grey checkerboard over a region, the way an image editor shows transparency. * - * A diagnostic backdrop, not a general primitive -- `charviewer` uses it so a - * sprite's transparent pixels are visible rather than blending into black. It - * is the one function in this file that does not follow the file's conventions: - * it draws through the *global* `akgl_renderer` rather than a backend the caller - * passes in, it leaves the renderer's draw colour changed, and it reports - * nothing. + * A diagnostic backdrop rather than a general primitive -- `charviewer` uses it + * so a sprite's transparent pixels are visible instead of blending into black. * - * @param w Width of the region to cover, in pixels, starting at x = 0. - * @param h Height of the region, starting at y = 0. Both round up to whole 8px - * cells, so a 12-pixel height paints 16. + * Until 0.5.0 this was the one function in the library outside the error + * protocol: it returned `void`, drew through the *global* `akgl_renderer` + * without checking it, and left the renderer's draw colour changed. It now + * takes a backend like every other entry point here and restores the colour it + * found, which is also what makes it testable without a world. + * + * @param self The backend to draw through. Required, along with its + * `sdl_renderer`. + * @param w Width of the region to cover, in pixels, starting at x = 0. Zero + * or negative paints nothing and is not reported. + * @param h Height of the region, starting at y = 0. Both round up to whole + * 8px cells, so a 12-pixel height paints 16. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKERR_NULLPOINTER If @p self or `self->sdl_renderer` is `NULL`. + * @throws AKGL_ERR_SDL If the draw colour cannot be read or set, or a cell + * cannot be filled. */ -void akgl_draw_background(int w, int h); +akerr_ErrorContext AKERR_NOIGNORE *akgl_draw_background(akgl_RenderBackend *self, int w, int h); /** * @brief Plot a single pixel. diff --git a/include/akgl/game.h b/include/akgl/game.h index da5c059..c694234 100644 --- a/include/akgl/game.h +++ b/include/akgl/game.h @@ -74,13 +74,6 @@ /* ==================== GAME STATE VARIABLES =================== */ -/** @brief Describes a renderable frame. Declared but not used by anything in the library. */ -typedef struct { - float32_t w; /**< Width in pixels. */ - float32_t h; /**< Height in pixels. */ - SDL_Texture *texture; /**< The frame's texture. */ -} akgl_Frame; - /** @brief Stores application-defined game-state flags. */ typedef struct { int32_t flags; /**< Meaning is entirely the application's; the library never reads it. Guard changes with akgl_game_state_lock. */ @@ -132,19 +125,29 @@ extern SDL_FRect *akgl_camera; /** * @brief True when every bit of `y` is set in `x`. Not "any of them" -- all of them. - * @warning Unparenthesized: it expands to `(x & y) == y`, so `!AKGL_BITMASK_HAS(a, b)` - * parses as `!(a & b) == b`. Use #AKGL_BITMASK_HASNOT rather than - * negating this. TODO.md item 21. + * + * Fully parenthesized, so it composes: `!AKGL_BITMASK_HAS(a, b)` and + * `AKGL_BITMASK_HAS(a, b) && cond` both mean what they read as. Until 0.5.0 it + * expanded to a bare `(x & y) == y`, so a negation bound to the `&` and parsed + * as `!(a & b) == b`. Nothing in the tree negated it -- #AKGL_BITMASK_HASNOT + * exists for that -- which is the only reason it was latent rather than live. */ -#define AKGL_BITMASK_HAS(x, y) (x & y) == y -/** @brief True when at least one bit of `y` is missing from `x`. Same parenthesization caveat as #AKGL_BITMASK_HAS. */ -#define AKGL_BITMASK_HASNOT(x, y) (x & y) != y +#define AKGL_BITMASK_HAS(x, y) ((((x) & (y)) == (y))) +/** @brief True when at least one bit of `y` is missing from `x`. */ +#define AKGL_BITMASK_HASNOT(x, y) ((((x) & (y)) != (y))) /** @brief Set every bit of `y` in `x`. Modifies `x`. */ -#define AKGL_BITMASK_ADD(x, y) x |= y +#define AKGL_BITMASK_ADD(x, y) ((x) |= (y)) /** @brief Clear every bit of `y` in `x`. Modifies `x`. */ -#define AKGL_BITMASK_DEL(x, y) x &= ~(y) -/** @brief Clear every bit of `x`. Carries its own trailing semicolon, so do not add another. */ -#define AKGL_BITMASK_CLEAR(x) x = 0; +#define AKGL_BITMASK_DEL(x, y) ((x) &= ~(y)) +/** + * @brief Clear every bit of `x`. Modifies `x`. + * + * Carries no trailing semicolon: write `AKGL_BITMASK_CLEAR(flags);` like any + * other statement. It used to include one, so every ordinary use produced an + * empty statement after it and a use as the whole body of an unbraced `if` + * would have taken the following statement with it. + */ +#define AKGL_BITMASK_CLEAR(x) ((x) = 0) /** * @brief Bring the whole library up: error codes, pools, registries, SDL, audio, fonts, gamepads. diff --git a/include/akgl/iterator.h b/include/akgl/iterator.h index daa98a5..58cba1a 100644 --- a/include/akgl/iterator.h +++ b/include/akgl/iterator.h @@ -28,38 +28,38 @@ typedef struct { uint8_t layerid; /**< Layer to restrict the sweep to. Read only when #AKGL_ITERATOR_OP_LAYERMASK is set. */ } akgl_Iterator; -#define AKGL_ITERATOR_OP_UPDATE 1 // 1 Call the actor's updatefunc -#define AKGL_ITERATOR_OP_RENDER 1 << 1 // 2 Call the actor's renderfunc -#define AKGL_ITERATOR_OP_RELEASE 1 << 2 // 4 Release the object back to its heap layer -#define AKGL_ITERATOR_OP_LAYERMASK 1 << 3 // 8 Skip anything whose layer != layerid -#define AKGL_ITERATOR_OP_TILEMAPSCALE 1 << 4 // 16 Scale actors to the tilemap; otherwise force scale 1.0 -#define AKGL_ITERATOR_OP_UNDEFINED_5 1 << 5 // 32 -#define AKGL_ITERATOR_OP_UNDEFINED_6 1 << 6 // 64 -#define AKGL_ITERATOR_OP_UNDEFINED_7 1 << 7 // 128 -#define AKGL_ITERATOR_OP_UNDEFINED_8 1 << 8 // 256 -#define AKGL_ITERATOR_OP_UNDEFINED_9 1 << 9 // 512 -#define AKGL_ITERATOR_OP_UNDEFINED_10 1 << 10 // 1024 -#define AKGL_ITERATOR_OP_UNDEFINED_11 1 << 11 // 2048 -#define AKGL_ITERATOR_OP_UNDEFINED_12 1 << 12 // 4096 -#define AKGL_ITERATOR_OP_UNDEFINED_13 1 << 13 // 8192 -#define AKGL_ITERATOR_OP_UNDEFINED_14 1 << 14 // 16384 -#define AKGL_ITERATOR_OP_UNDEFINED_15 1 << 15 // 32768 -#define AKGL_ITERATOR_OP_UNDEFINED_16 1 << 16 // 65536 -#define AKGL_ITERATOR_OP_UNDEFINED_17 1 << 17 // 131072 -#define AKGL_ITERATOR_OP_UNDEFINED_18 1 << 18 // 262144 -#define AKGL_ITERATOR_OP_UNDEFINED_19 1 << 19 // 524288 -#define AKGL_ITERATOR_OP_UNDEFINED_20 1 << 20 // 1048576 -#define AKGL_ITERATOR_OP_UNDEFINED_21 1 << 21 // 2097152 -#define AKGL_ITERATOR_OP_UNDEFINED_22 1 << 22 // 4194304 -#define AKGL_ITERATOR_OP_UNDEFINED_23 1 << 23 // 8388608 -#define AKGL_ITERATOR_OP_UNDEFINED_24 1 << 24 // 16777216 -#define AKGL_ITERATOR_OP_UNDEFINED_25 1 << 25 // 33554432 -#define AKGL_ITERATOR_OP_UNDEFINED_26 1 << 26 // 67108864 -#define AKGL_ITERATOR_OP_UNDEFINED_27 1 << 27 // 134217728 -#define AKGL_ITERATOR_OP_UNDEFINED_28 1 << 28 // 268435456 -#define AKGL_ITERATOR_OP_UNDEFINED_29 1 << 29 // 536870912 -#define AKGL_ITERATOR_OP_UNDEFINED_30 1 << 30 // 1073741824 -#define AKGL_ITERATOR_OP_UNDEFINED_31 1 << 31 // 2147483648 +#define AKGL_ITERATOR_OP_UPDATE (1 << 0) // 1 Call the actor's updatefunc +#define AKGL_ITERATOR_OP_RENDER (1 << 1) // 2 Call the actor's renderfunc +#define AKGL_ITERATOR_OP_RELEASE (1 << 2) // 4 Release the object back to its heap layer +#define AKGL_ITERATOR_OP_LAYERMASK (1 << 3) // 8 Skip anything whose layer != layerid +#define AKGL_ITERATOR_OP_TILEMAPSCALE (1 << 4) // 16 Scale actors to the tilemap; otherwise force scale 1.0 +#define AKGL_ITERATOR_OP_UNDEFINED_5 (1 << 5) // 32 +#define AKGL_ITERATOR_OP_UNDEFINED_6 (1 << 6) // 64 +#define AKGL_ITERATOR_OP_UNDEFINED_7 (1 << 7) // 128 +#define AKGL_ITERATOR_OP_UNDEFINED_8 (1 << 8) // 256 +#define AKGL_ITERATOR_OP_UNDEFINED_9 (1 << 9) // 512 +#define AKGL_ITERATOR_OP_UNDEFINED_10 (1 << 10) // 1024 +#define AKGL_ITERATOR_OP_UNDEFINED_11 (1 << 11) // 2048 +#define AKGL_ITERATOR_OP_UNDEFINED_12 (1 << 12) // 4096 +#define AKGL_ITERATOR_OP_UNDEFINED_13 (1 << 13) // 8192 +#define AKGL_ITERATOR_OP_UNDEFINED_14 (1 << 14) // 16384 +#define AKGL_ITERATOR_OP_UNDEFINED_15 (1 << 15) // 32768 +#define AKGL_ITERATOR_OP_UNDEFINED_16 (1 << 16) // 65536 +#define AKGL_ITERATOR_OP_UNDEFINED_17 (1 << 17) // 131072 +#define AKGL_ITERATOR_OP_UNDEFINED_18 (1 << 18) // 262144 +#define AKGL_ITERATOR_OP_UNDEFINED_19 (1 << 19) // 524288 +#define AKGL_ITERATOR_OP_UNDEFINED_20 (1 << 20) // 1048576 +#define AKGL_ITERATOR_OP_UNDEFINED_21 (1 << 21) // 2097152 +#define AKGL_ITERATOR_OP_UNDEFINED_22 (1 << 22) // 4194304 +#define AKGL_ITERATOR_OP_UNDEFINED_23 (1 << 23) // 8388608 +#define AKGL_ITERATOR_OP_UNDEFINED_24 (1 << 24) // 16777216 +#define AKGL_ITERATOR_OP_UNDEFINED_25 (1 << 25) // 33554432 +#define AKGL_ITERATOR_OP_UNDEFINED_26 (1 << 26) // 67108864 +#define AKGL_ITERATOR_OP_UNDEFINED_27 (1 << 27) // 134217728 +#define AKGL_ITERATOR_OP_UNDEFINED_28 (1 << 28) // 268435456 +#define AKGL_ITERATOR_OP_UNDEFINED_29 (1 << 29) // 536870912 +#define AKGL_ITERATOR_OP_UNDEFINED_30 (1 << 30) // 1073741824 +#define AKGL_ITERATOR_OP_UNDEFINED_31 (1 << 31) // 2147483648 #endif // _AKGL_ITERATOR_H_ diff --git a/include/akgl/json_helpers.h b/include/akgl/json_helpers.h index a5d7070..41824a9 100644 --- a/include/akgl/json_helpers.h +++ b/include/akgl/json_helpers.h @@ -30,6 +30,7 @@ #define _AKGL_JSON_HELPERS_H_ #include +#include #include #include @@ -85,7 +86,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_integer_value(json_t *obj, char * @throws AKERR_TYPE If @p key is present but is not a number. Integers and * reals are both accepted, so `1` and `1.0` behave alike. */ -akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_number_value(json_t *obj, char *key, float *dest); +akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_number_value(json_t *obj, char *key, float32_t *dest); /** * @brief Read a number out of a JSON object as a `double`. * @@ -101,7 +102,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_number_value(json_t *obj, char * @throws AKERR_KEY If @p key is absent. * @throws AKERR_TYPE If @p key is present but is not a number. */ -akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_double_value(json_t *obj, char *key, double *dest); +akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_double_value(json_t *obj, char *key, float64_t *dest); /** * @brief Read a string out of a JSON object into a pooled akgl_String. * diff --git a/include/akgl/physics.h b/include/akgl/physics.h index 0d22286..02bf4b2 100644 --- a/include/akgl/physics.h +++ b/include/akgl/physics.h @@ -41,12 +41,12 @@ typedef struct akgl_PhysicsBackend { akerr_ErrorContext AKERR_NOIGNORE *(*collide)(struct akgl_PhysicsBackend *self, akgl_Actor *a1, akgl_Actor *a2); /**< Resolve a collision between two actors. Not called by the simulation loop yet. */ akerr_ErrorContext AKERR_NOIGNORE *(*move)(struct akgl_PhysicsBackend *self, akgl_Actor *actor, float32_t dt); /**< Commit one actor's velocity to its position. */ - double drag_x; /**< Fraction of environmental velocity shed per second along x. 0 disables. From `physics.drag.x`. */ - double drag_y; /**< Drag along y. From `physics.drag.y`. */ - double drag_z; /**< Drag along z. From `physics.drag.z`. */ - double gravity_x; /**< Acceleration along x, world units per second squared. Applied toward screen left. From `physics.gravity.x`. */ - double gravity_y; /**< Acceleration along y. Applied down the screen, since y grows downward. From `physics.gravity.y`. */ - double gravity_z; /**< Acceleration along z. Applied away from the camera. From `physics.gravity.z`. */ + float64_t drag_x; /**< Fraction of environmental velocity shed per second along x. 0 disables. From `physics.drag.x`. */ + float64_t drag_y; /**< Drag along y. From `physics.drag.y`. */ + float64_t drag_z; /**< Drag along z. From `physics.drag.z`. */ + float64_t gravity_x; /**< Acceleration along x, world units per second squared. Applied toward screen left. From `physics.gravity.x`. */ + float64_t gravity_y; /**< Acceleration along y. Applied down the screen, since y grows downward. From `physics.gravity.y`. */ + float64_t gravity_z; /**< Acceleration along z. Applied away from the camera. From `physics.gravity.z`. */ SDL_Time gravity_time; /**< Timestamp of the previous step, in nanoseconds. The simulation's `dt` is measured from this. */ SDL_Time timer_gravity; /**< Unused. Nothing in the library reads or writes it. */ } akgl_PhysicsBackend; diff --git a/include/akgl/staticstring.h b/include/akgl/staticstring.h index 7f0cc3e..1ae8f90 100644 --- a/include/akgl/staticstring.h +++ b/include/akgl/staticstring.h @@ -50,10 +50,10 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_string_initialize(akgl_String *obj, char * @brief Copy the contents of one pooled string into another. * * A bounded `strncpy` between two already-claimed pool slots. It copies bytes - * only: `refcount` is left alone, so @p dst keeps whatever pool state it had. + * only: `refcount` is left alone, so @p dest keeps whatever pool state it had. * * @param src Source string. Required. Read up to @p count bytes. - * @param dst Destination string. Required. Overwritten in place; the pool + * @param dest Destination string. Required. Overwritten in place; the pool * slot must already have been claimed. * @param count Maximum bytes to copy. 0 selects #AKGL_MAX_STRING_LENGTH, the * whole buffer. A @p count shorter than the source truncates @@ -62,10 +62,10 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_string_initialize(akgl_String *obj, char * #AKGL_MAX_STRING_LENGTH overrun both buffers and are not * rejected. * @return `NULL` on success, otherwise an error context owned by the caller. - * @throws AKERR_NULLPOINTER If @p src or @p dst is `NULL`. + * @throws AKERR_NULLPOINTER If @p src or @p dest is `NULL`. * @throws errno Whatever `errno` holds if `strncpy` returns something other - * than @p dst. In practice `strncpy` always returns its destination, so + * than @p dest. In practice `strncpy` always returns its destination, so * this path is unreachable rather than merely rare. */ -akerr_ErrorContext AKERR_NOIGNORE *akgl_string_copy(akgl_String *src, akgl_String *dst, int count); +akerr_ErrorContext AKERR_NOIGNORE *akgl_string_copy(akgl_String *src, akgl_String *dest, int count); #endif //_AKGL_STATICSTRING_H_ diff --git a/include/akgl/tilemap.h b/include/akgl/tilemap.h index 8f1cb8d..06c8ad0 100644 --- a/include/akgl/tilemap.h +++ b/include/akgl/tilemap.h @@ -71,8 +71,8 @@ /** @brief One object placed in a Tiled object layer. */ typedef struct { - float x; /**< Position in map pixels, from the Tiled object. Copied onto the actor it spawns. */ - float y; /**< Position in map pixels. */ + float32_t x; /**< Position in map pixels, from the Tiled object. Copied onto the actor it spawns. */ + float32_t y; /**< Position in map pixels. */ int gid; /**< Global tile id, for a tile object. Not read by the loader. */ int id; /**< Tiled's object id. Not read by the loader. */ int height; /**< Height in map pixels. Read into the map's perspective bands, not into this field. */ @@ -87,7 +87,7 @@ typedef struct { /** @brief One layer of a tilemap: a tile grid, an image, or a group of objects. */ typedef struct { short type; /**< Which of the `AKGL_TILEMAP_LAYER_TYPE_*` kinds this is; decides which of the members below mean anything. */ - float opacity; /**< 0.0 to 1.0, from Tiled. Recorded but not yet applied at draw time. */ + float32_t opacity; /**< 0.0 to 1.0, from Tiled. Recorded but not yet applied at draw time. */ bool visible; /**< From Tiled. Recorded but not yet consulted at draw time. */ int height; /**< Tile layer: height in tiles. Image layer: the texture's height in pixels. */ int width; /**< Tile layer: width in tiles. Image layer: the texture's width in pixels. */ @@ -143,10 +143,10 @@ typedef struct { int p_vanishing_y; /**< Y of the `p_vanishing` marker: the row at which actors are smallest. 0 disables perspective. */ int p_foreground_h; /**< Height of the `p_foreground` marker object. Recorded; not used in the current rate calculation. */ int p_vanishing_h; /**< Height of the `p_vanishing` marker object. Recorded, likewise. */ - float p_foreground_scale; /**< Actor scale at `p_foreground_y`. Defaults to 1.0. */ - float p_vanishing_scale; /**< Actor scale at `p_vanishing_y`. Defaults to 1.0. */ - float p_scale; /**< Unused. Left from an earlier formulation of the perspective maths. */ - float p_rate; /**< Scale change per pixel of y between the two markers. Derived at load; what akgl_tilemap_scale_actor interpolates with. */ + float32_t p_foreground_scale; /**< Actor scale at `p_foreground_y`. Defaults to 1.0. */ + float32_t p_vanishing_scale; /**< Actor scale at `p_vanishing_y`. Defaults to 1.0. */ + float32_t p_scale; /**< Unused. Left from an earlier formulation of the perspective maths. */ + float32_t p_rate; /**< Scale change per pixel of y between the two markers. Derived at load; what akgl_tilemap_scale_actor interpolates with. */ akgl_Tileset tilesets[AKGL_TILEMAP_MAX_TILESETS]; /**< The tilesets, in the order Tiled listed them. */ akgl_TilemapLayer layers[AKGL_TILEMAP_MAX_LAYERS]; /**< The layers, in draw order: index 0 is furthest back. */ @@ -505,7 +505,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_tilemap_scale_actor(akgl_Tilemap *map, a * not numeric. * @throws AKGL_ERR_HEAP If the string pool is exhausted. */ -akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_properties_number(json_t *obj, char *key, float *dest); +akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_properties_number(json_t *obj, char *key, float32_t *dest); /** * @brief Read a Tiled custom property declared as `float`. * @@ -522,7 +522,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_properties_number(json_t *obj, * not numeric. * @throws AKGL_ERR_HEAP If the string pool is exhausted. */ -akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_properties_float(json_t *obj, char *key, float *dest); +akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_properties_float(json_t *obj, char *key, float32_t *dest); /** * @brief Read a Tiled custom property declared as `float`, at full precision. * @@ -541,7 +541,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_properties_float(json_t *obj, c * not numeric. * @throws AKGL_ERR_HEAP If the string pool is exhausted. */ -akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_properties_double(json_t *obj, char *key, double *dest); +akerr_ErrorContext AKERR_NOIGNORE *akgl_get_json_properties_double(json_t *obj, char *key, float64_t *dest); /** * @brief Turn one Tiled object into a live actor, creating it if it is not already registered. * diff --git a/include/akgl/types.h b/include/akgl/types.h index 0e3956f..6d90d1d 100644 --- a/include/akgl/types.h +++ b/include/akgl/types.h @@ -17,7 +17,7 @@ /** @brief Single-precision float. Positions, velocities, and scale factors. */ typedef float float32_t; -/** @brief Double-precision float. Declared for symmetry; unused so far. */ +/** @brief Double-precision float. The physics backend keeps its drag and gravity constants at this width. */ typedef double float64_t; #endif // _AKGL_TYPES_H_ diff --git a/include/akgl/util.h b/include/akgl/util.h index fa0c06b..d11ff68 100644 --- a/include/akgl/util.h +++ b/include/akgl/util.h @@ -41,13 +41,6 @@ typedef struct akgl_RectanglePoints { akgl_Point bottomright; /**< (x + w, y + h). */ } akgl_RectanglePoints; -/** - * @brief Do not use. Three open parentheses, two closes -- any expansion is a - * syntax error. It duplicates akgl_collide_rectangles(), has no callers, - * and is slated for deletion. TODO.md item 20. - */ -#define AKGL_COLLIDE_RECTANGLES(r1x, r1y, r1w, r1h, r2x, r2y, r2w, r2h) ((r1x < (r2x + r2w)) || ((r1x + r1w) > r2x) - /** * @brief Expand a rectangle into its four corner points. * @@ -116,11 +109,11 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_collide_rectangles(SDL_FRect *r1, SDL_FR * @param root Directory to fall back to, normally `dirname` of the file that * contained @p path. Required, even when unused. * @param path The path to resolve, relative or absolute. Required. - * @param dst Receives the resolved absolute path. Required, and must already be + * @param dest Receives the resolved absolute path. Required, and must already be * a claimed pool string -- this writes into it, it does not claim * one for you. * @return `NULL` on success, otherwise an error context owned by the caller. - * @throws AKERR_NULLPOINTER If @p root, @p path, or @p dst is `NULL`. + * @throws AKERR_NULLPOINTER If @p root, @p path, or @p dest is `NULL`. * @throws AKERR_OUTOFBOUNDS If `root + "/" + path` would not fit in * #AKGL_MAX_STRING_LENGTH. * @throws ENOENT If neither spelling names an existing file. Any other `errno` @@ -134,7 +127,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akgl_collide_rectangles(SDL_FRect *r1, SDL_FR * error context it was handling. Each such call consumes one slot of * libakerror's fixed 128-entry context array for the life of the process. */ -akerr_ErrorContext AKERR_NOIGNORE *akgl_path_relative(char *root, char *path, akgl_String *dst); +akerr_ErrorContext AKERR_NOIGNORE *akgl_path_relative(char *root, char *path, akgl_String *dest); // These are REALLY slow routines that are only useful in testing harnesses /** diff --git a/src/actor_state_string_names.c b/src/actor_state_string_names.c index 7d34926..914e1fe 100644 --- a/src/actor_state_string_names.c +++ b/src/actor_state_string_names.c @@ -1,39 +1,59 @@ /** * @file actor_state_string_names.c - * @brief Implements the actor state string names subsystem. + * @brief Bit position -> state name, as text. Maintained by hand. + * + * An earlier comment in `actor.h` said this file was "built by a utility script + * and not kept in git, see the Makefile for lib_src/actor_state_string_names.c". + * None of that is true: there is no Makefile (the project is CMake), no + * `lib_src/`, no such script under `scripts/` or `util/`, and this file is + * tracked. It is maintained by hand, and the two things that keeps costing are + * worth stating here rather than rediscovering. + * + * **Entry `i` must name the bit `1 << i` in `actor.h`.** Bits 11 and 12 read + * `UNDEFINED_11` and `UNDEFINED_12` here for as long as `actor.h` has called + * them `MOVING_IN` and `MOVING_OUT`, which meant a character JSON could not + * bind a sprite to either state -- the name it would have to write did not + * exist in the registry akgl_registry_init_actor_state_strings builds out of + * this array. + * + * **The array is sized by #AKGL_ACTOR_MAX_STATES**, not by a literal. It was + * `[32]` here against `[AKGL_ACTOR_MAX_STATES+1]` in the header, so a consumer + * trusting the declared bound read one entry past the object. */ -char *AKGL_ACTOR_STATE_STRING_NAMES[32] = { - "AKGL_ACTOR_STATE_FACE_DOWN", - "AKGL_ACTOR_STATE_FACE_LEFT", - "AKGL_ACTOR_STATE_FACE_RIGHT", - "AKGL_ACTOR_STATE_FACE_UP", - "AKGL_ACTOR_STATE_ALIVE", - "AKGL_ACTOR_STATE_DYING", - "AKGL_ACTOR_STATE_DEAD", - "AKGL_ACTOR_STATE_MOVING_LEFT", - "AKGL_ACTOR_STATE_MOVING_RIGHT", - "AKGL_ACTOR_STATE_MOVING_UP", - "AKGL_ACTOR_STATE_MOVING_DOWN", - "AKGL_ACTOR_STATE_UNDEFINED_11", - "AKGL_ACTOR_STATE_UNDEFINED_12", - "AKGL_ACTOR_STATE_UNDEFINED_13", - "AKGL_ACTOR_STATE_UNDEFINED_14", - "AKGL_ACTOR_STATE_UNDEFINED_15", - "AKGL_ACTOR_STATE_UNDEFINED_16", - "AKGL_ACTOR_STATE_UNDEFINED_17", - "AKGL_ACTOR_STATE_UNDEFINED_18", - "AKGL_ACTOR_STATE_UNDEFINED_19", - "AKGL_ACTOR_STATE_UNDEFINED_20", - "AKGL_ACTOR_STATE_UNDEFINED_21", - "AKGL_ACTOR_STATE_UNDEFINED_22", - "AKGL_ACTOR_STATE_UNDEFINED_23", - "AKGL_ACTOR_STATE_UNDEFINED_24", - "AKGL_ACTOR_STATE_UNDEFINED_25", - "AKGL_ACTOR_STATE_UNDEFINED_26", - "AKGL_ACTOR_STATE_UNDEFINED_27", - "AKGL_ACTOR_STATE_UNDEFINED_28", - "AKGL_ACTOR_STATE_UNDEFINED_29", - "AKGL_ACTOR_STATE_UNDEFINED_30", - "AKGL_ACTOR_STATE_UNDEFINED_31", +#include + +char *AKGL_ACTOR_STATE_STRING_NAMES[AKGL_ACTOR_MAX_STATES] = { + "AKGL_ACTOR_STATE_FACE_DOWN", // 1 << 0 + "AKGL_ACTOR_STATE_FACE_LEFT", // 1 << 1 + "AKGL_ACTOR_STATE_FACE_RIGHT", // 1 << 2 + "AKGL_ACTOR_STATE_FACE_UP", // 1 << 3 + "AKGL_ACTOR_STATE_ALIVE", // 1 << 4 + "AKGL_ACTOR_STATE_DYING", // 1 << 5 + "AKGL_ACTOR_STATE_DEAD", // 1 << 6 + "AKGL_ACTOR_STATE_MOVING_LEFT", // 1 << 7 + "AKGL_ACTOR_STATE_MOVING_RIGHT", // 1 << 8 + "AKGL_ACTOR_STATE_MOVING_UP", // 1 << 9 + "AKGL_ACTOR_STATE_MOVING_DOWN", // 1 << 10 + "AKGL_ACTOR_STATE_MOVING_IN", // 1 << 11 + "AKGL_ACTOR_STATE_MOVING_OUT", // 1 << 12 + "AKGL_ACTOR_STATE_UNDEFINED_13", // 1 << 13 + "AKGL_ACTOR_STATE_UNDEFINED_14", // 1 << 14 + "AKGL_ACTOR_STATE_UNDEFINED_15", // 1 << 15 + "AKGL_ACTOR_STATE_UNDEFINED_16", // 1 << 16 + "AKGL_ACTOR_STATE_UNDEFINED_17", // 1 << 17 + "AKGL_ACTOR_STATE_UNDEFINED_18", // 1 << 18 + "AKGL_ACTOR_STATE_UNDEFINED_19", // 1 << 19 + "AKGL_ACTOR_STATE_UNDEFINED_20", // 1 << 20 + "AKGL_ACTOR_STATE_UNDEFINED_21", // 1 << 21 + "AKGL_ACTOR_STATE_UNDEFINED_22", // 1 << 22 + "AKGL_ACTOR_STATE_UNDEFINED_23", // 1 << 23 + "AKGL_ACTOR_STATE_UNDEFINED_24", // 1 << 24 + "AKGL_ACTOR_STATE_UNDEFINED_25", // 1 << 25 + "AKGL_ACTOR_STATE_UNDEFINED_26", // 1 << 26 + "AKGL_ACTOR_STATE_UNDEFINED_27", // 1 << 27 + "AKGL_ACTOR_STATE_UNDEFINED_28", // 1 << 28 + "AKGL_ACTOR_STATE_UNDEFINED_29", // 1 << 29 + "AKGL_ACTOR_STATE_UNDEFINED_30", // 1 << 30 + "AKGL_ACTOR_STATE_UNDEFINED_31", // 1 << 31 }; diff --git a/src/assets.c b/src/assets.c index 62a1a8b..a636ba3 100644 --- a/src/assets.c +++ b/src/assets.c @@ -24,7 +24,6 @@ akerr_ErrorContext *akgl_load_start_bgm(char *fname) //CATCH(errctx, akgl_heap_next_string(&tmpstr)); //CATCH(errctx, akgl_string_initialize(tmpstr, NULL)); - //SDL_snprintf((char *)&tmpstr->data, AKGL_MAX_STRING_LENGTH, "%s%s", SDL_GetBasePath(), fname); SDL_Log("Loading music asset from %s", fname); akgl_bgm = MIX_LoadAudio(akgl_mixer, fname, true); FAIL_ZERO_BREAK(errctx, akgl_bgm, AKERR_NULLPOINTER, "Failed to load music asset %s : %s", fname, SDL_GetError()); diff --git a/src/character.c b/src/character.c index 3657613..c0c0238 100644 --- a/src/character.c +++ b/src/character.c @@ -56,7 +56,6 @@ akerr_ErrorContext *akgl_character_sprite_add(akgl_Character *basechar, akgl_Spr akerr_ErrorContext *akgl_character_sprite_get(akgl_Character *basechar, int state, akgl_Sprite **dest) { - akgl_Sprite *target = NULL; PREPARE_ERROR(errctx); char stateval[32]; FAIL_ZERO_RETURN(errctx, dest, AKERR_NULLPOINTER, "NULL pointer to sprite pointer (**dest)"); @@ -65,8 +64,6 @@ akerr_ErrorContext *akgl_character_sprite_get(akgl_Character *basechar, int stat SDL_itoa(state, (char *)&stateval, 10); *dest = (akgl_Sprite *)SDL_GetPointerProperty(basechar->state_sprites, (char *)&stateval, NULL); FAIL_ZERO_RETURN(errctx, *dest, AKERR_KEY, "Sprite for state %d (%b) not found in the character's registry", state, state); - target = *dest; - //SDL_Log("Sprite state %d (%s) has character %s", state, (char *)&stateval, target->name); SUCCEED_RETURN(errctx); } @@ -228,7 +225,6 @@ akerr_ErrorContext *akgl_character_load_json(char *filename) CATCH(errctx, akgl_heap_next_character(&obj)); //CATCH(errctx, akgl_heap_next_string(&tmpstr)); //CATCH(errctx, akgl_string_initialize(tmpstr, NULL)); - //SDL_snprintf((char *)&tmpstr->data, AKGL_MAX_STRING_LENGTH, "%s%s", SDL_GetBasePath(), filename); json = (json_t *)json_load_file(filename, 0, &error); FAIL_ZERO_BREAK( errctx, diff --git a/src/controller.c b/src/controller.c index 7d34a89..5418903 100644 --- a/src/controller.c +++ b/src/controller.c @@ -279,13 +279,6 @@ akerr_ErrorContext *akgl_controller_handle_event(void *appstate, SDL_Event *even event->key.which == curmap->kbid && event->key.key == curcontrol->key) ); - if ( event->type == 768 && event->key.which == 11 && event->key.key == 13 ) { - SDL_Log("Event type=%d, keyboard=%d, key=%d", event->type, event->key.which, event->key.key); - SDL_Log("ControlMap[%d].Controls[%d] keyboard=%d, key=%d", i, j, curmap->kbid, curcontrol->key); - SDL_Log("event %d -> control on %d off %d", event->type, curcontrol->event_on, curcontrol->event_off); - SDL_Log("eventButtonComboMatch for controlmap %d id %d = %d", - i, j, eventButtonComboMatch); - } if ( event->type == curcontrol->event_on && eventButtonComboMatch) { CATCH(errctx, curcontrol->handler_on(curmap->target, event)); goto _akgl_controller_handle_event_success; diff --git a/src/draw.c b/src/draw.c index e218dfd..ae9c399 100644 --- a/src/draw.c +++ b/src/draw.c @@ -28,32 +28,6 @@ typedef struct { */ static FloodSpan floodspans[AKGL_DRAW_MAX_FLOOD_SPANS]; -/* Draw a Gimpish background pattern to show transparency in the image */ -void akgl_draw_background(int w, int h) -{ - SDL_Color col[2] = { - { 0x66, 0x66, 0x66, 0xff }, - { 0x99, 0x99, 0x99, 0xff }, - }; - int i, x, y; - SDL_FRect rect; - const int dx = 8, dy = 8; - - rect.w = (float)dx; - rect.h = (float)dy; - for (y = 0; y < h; y += dy) { - for (x = 0; x < w; x += dx) { - /* use an 8x8 checkerboard pattern */ - i = (((x ^ y) >> 3) & 1); - SDL_SetRenderDrawColor(akgl_renderer->sdl_renderer, col[i].r, col[i].g, col[i].b, col[i].a); - - rect.x = (float)x; - rect.y = (float)y; - SDL_RenderFillRect(akgl_renderer->sdl_renderer, &rect); - } - } -} - /** * @brief Remember the renderer's draw color and replace it with @p color. * @@ -116,6 +90,63 @@ static akerr_ErrorContext *pop_draw_color(akgl_RenderBackend *self, SDL_Color *p SUCCEED_RETURN(errctx); } +/* Draw a Gimpish background pattern to show transparency in the image */ +akerr_ErrorContext *akgl_draw_background(akgl_RenderBackend *self, int w, int h) +{ + PREPARE_ERROR(errctx); + SDL_Color col[2] = { + { 0x66, 0x66, 0x66, 0xff }, + { 0x99, 0x99, 0x99, 0xff }, + }; + SDL_Color previous; + SDL_FRect rect; + const int dx = 8, dy = 8; + bool pushed = false; + bool drawfailed = false; + int i = 0; + int x = 0; + int y = 0; + + FAIL_ZERO_RETURN(errctx, self, AKERR_NULLPOINTER, "self"); + FAIL_ZERO_RETURN(errctx, self->sdl_renderer, AKERR_NULLPOINTER, "No valid SDL rendering backend"); + + rect.w = (float)dx; + rect.h = (float)dy; + + ATTEMPT { + CATCH(errctx, push_draw_color(self, col[0], &previous)); + pushed = true; + + // The two SDL calls are checked through a flag rather than with + // FAIL_ZERO_BREAK. Inside these loops a `break` would leave the loop, + // not the ATTEMPT block, and the fill would carry on with the failure + // unnoticed -- the hazard AGENTS.md describes for CATCH inside a loop. + for ( y = 0; (y < h) && (drawfailed == false); y += dy ) { + for ( x = 0; x < w; x += dx ) { + /* use an 8x8 checkerboard pattern */ + i = (((x ^ y) >> 3) & 1); + if ( SDL_SetRenderDrawColor(self->sdl_renderer, col[i].r, col[i].g, col[i].b, col[i].a) == false ) { + drawfailed = true; + break; + } + rect.x = (float)x; + rect.y = (float)y; + if ( SDL_RenderFillRect(self->sdl_renderer, &rect) == false ) { + drawfailed = true; + break; + } + } + } + FAIL_NONZERO_BREAK(errctx, drawfailed, AKGL_ERR_SDL, "%s", SDL_GetError()); + } CLEANUP { + if ( pushed == true ) { + IGNORE(pop_draw_color(self, &previous)); + } + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + /** * @brief Fill the four-connected region of @p oldpixel around a seed pixel. * diff --git a/src/game.c b/src/game.c index 94d47f4..c0c7158 100644 --- a/src/game.c +++ b/src/game.c @@ -131,9 +131,6 @@ void akgl_game_lowfps(void) akerr_ErrorContext *akgl_game_init(void) { - int screenwidth = 0; - int screenheight = 0; - int i = 0; PREPARE_ERROR(errctx); // First, before anything that can raise: everything below reports through @@ -639,10 +636,13 @@ akerr_ErrorContext *akgl_game_update(akgl_Iterator *opflags) { PREPARE_ERROR(errctx); akgl_Iterator defflags = { - .flags = (AKGL_ITERATOR_OP_LAYERMASK | AKGL_ITERATOR_OP_LAYERMASK), + // Was (LAYERMASK | LAYERMASK). Nothing in the loop below reads either + // bit today, so this is a statement of intent rather than a behaviour + // change -- and the reason the loop ignores LAYERMASK is TODO.md + // Performance item 32, which is still open. + .flags = (AKGL_ITERATOR_OP_UPDATE | AKGL_ITERATOR_OP_LAYERMASK), .layerid = 0 }; - SDL_Time curTime = SDL_GetTicksNS(); akgl_Actor *actor = NULL; if ( opflags == NULL ) { diff --git a/src/json_helpers.c b/src/json_helpers.c index cb8a442..9628c96 100644 --- a/src/json_helpers.c +++ b/src/json_helpers.c @@ -52,7 +52,7 @@ akerr_ErrorContext *akgl_get_json_integer_value(json_t *obj, char *key, int *des SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akgl_get_json_number_value(json_t *obj, char *key, float *dest) +akerr_ErrorContext *akgl_get_json_number_value(json_t *obj, char *key, float32_t *dest) { PREPARE_ERROR(errctx); FAIL_ZERO_RETURN(errctx, obj, AKERR_NULLPOINTER, "NULL pointer reference"); @@ -65,7 +65,7 @@ akerr_ErrorContext *akgl_get_json_number_value(json_t *obj, char *key, float *de SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akgl_get_json_double_value(json_t *obj, char *key, double *dest) +akerr_ErrorContext *akgl_get_json_double_value(json_t *obj, char *key, float64_t *dest) { PREPARE_ERROR(errctx); FAIL_ZERO_RETURN(errctx, obj, AKERR_NULLPOINTER, "NULL pointer reference"); diff --git a/src/registry.c b/src/registry.c index f8da9e7..f8e0d08 100644 --- a/src/registry.c +++ b/src/registry.c @@ -25,6 +25,34 @@ SDL_PropertiesID AKGL_REGISTRY_MUSIC = 0; SDL_PropertiesID AKGL_REGISTRY_FONT = 0; SDL_PropertiesID AKGL_REGISTRY_PROPERTIES = 0; +/** + * @brief Create one registry, replacing whatever set was held under it. + * + * All eight initializers go through this so they behave the same way on a + * second call. Only akgl_registry_init_actor used to destroy the old set -- + * akgl_heap_init_actor's counterpart, so resetting actors between levels was + * the one path that did not leak -- and the other seven abandoned their + * SDL_PropertiesID every time they were called again. + * + * @param dest The registry handle to (re)create. Required. + * @param what What it holds, for the failure message. Required. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKERR_NULLPOINTER If @p dest is `NULL`, or if SDL cannot create the + * property set. + */ +static akerr_ErrorContext *registry_create(SDL_PropertiesID *dest, const char *what) +{ + PREPARE_ERROR(errctx); + FAIL_ZERO_RETURN(errctx, dest, AKERR_NULLPOINTER, "dest"); + if ( *dest != 0 ) { + SDL_DestroyProperties(*dest); + *dest = 0; + } + *dest = SDL_CreateProperties(); + FAIL_ZERO_RETURN(errctx, *dest, AKERR_NULLPOINTER, "Error initializing %s registry", what); + SUCCEED_RETURN(errctx); +} + akerr_ErrorContext *akgl_registry_init(void) { PREPARE_ERROR(errctx); @@ -36,6 +64,12 @@ akerr_ErrorContext *akgl_registry_init(void) CATCH(errctx, akgl_registry_init_actor_state_strings()); CATCH(errctx, akgl_registry_init_font()); CATCH(errctx, akgl_registry_init_music()); + // Missing until 0.5.0. Without it AKGL_REGISTRY_PROPERTIES stayed 0, + // akgl_set_property was a silent no-op, akgl_get_property always + // returned the caller's default, and akgl_physics_init_arcade and + // akgl_render_2d_init quietly ignored their configuration. Only callers + // going through akgl_game_init, which calls it separately, escaped that. + CATCH(errctx, akgl_registry_init_properties()); } CLEANUP { } PROCESS(errctx) { } FINISH(errctx, true); @@ -45,35 +79,28 @@ akerr_ErrorContext *akgl_registry_init(void) akerr_ErrorContext *akgl_registry_init_actor(void) { PREPARE_ERROR(errctx); - if ( AKGL_REGISTRY_ACTOR != 0 ) { - SDL_DestroyProperties(AKGL_REGISTRY_ACTOR); - } - AKGL_REGISTRY_ACTOR = SDL_CreateProperties(); - FAIL_ZERO_RETURN(errctx, AKGL_REGISTRY_ACTOR, AKERR_NULLPOINTER, "Error initializing actor registry"); + PASS(errctx, registry_create(&AKGL_REGISTRY_ACTOR, "actor")); SUCCEED_RETURN(errctx); } akerr_ErrorContext *akgl_registry_init_font(void) { PREPARE_ERROR(errctx); - AKGL_REGISTRY_FONT = SDL_CreateProperties(); - FAIL_ZERO_RETURN(errctx, AKGL_REGISTRY_FONT, AKERR_NULLPOINTER, "Error initializing font registry"); + PASS(errctx, registry_create(&AKGL_REGISTRY_FONT, "font")); SUCCEED_RETURN(errctx); } akerr_ErrorContext *akgl_registry_init_music(void) { PREPARE_ERROR(errctx); - AKGL_REGISTRY_MUSIC = SDL_CreateProperties(); - FAIL_ZERO_RETURN(errctx, AKGL_REGISTRY_MUSIC, AKERR_NULLPOINTER, "Error initializing music registry"); + PASS(errctx, registry_create(&AKGL_REGISTRY_MUSIC, "music")); SUCCEED_RETURN(errctx); } akerr_ErrorContext *akgl_registry_init_properties(void) { PREPARE_ERROR(errctx); - AKGL_REGISTRY_PROPERTIES = SDL_CreateProperties(); - FAIL_ZERO_RETURN(errctx, AKGL_REGISTRY_PROPERTIES, AKERR_NULLPOINTER, "Error initializing properties registry"); + PASS(errctx, registry_create(&AKGL_REGISTRY_PROPERTIES, "properties")); SUCCEED_RETURN(errctx); } @@ -82,8 +109,7 @@ akerr_ErrorContext *akgl_registry_init_actor_state_strings(void) int i = 0; int flag = 0; PREPARE_ERROR(errctx); - AKGL_REGISTRY_ACTOR_STATE_STRINGS = SDL_CreateProperties(); - FAIL_ZERO_RETURN(errctx, AKGL_REGISTRY_ACTOR_STATE_STRINGS, AKERR_NULLPOINTER, "Error initializing actor state strings registry"); + PASS(errctx, registry_create(&AKGL_REGISTRY_ACTOR_STATE_STRINGS, "actor state strings")); for ( i = 0 ; i < AKGL_ACTOR_MAX_STATES; i++ ) { flag = (1 << i); SDL_SetNumberProperty( @@ -97,24 +123,21 @@ akerr_ErrorContext *akgl_registry_init_actor_state_strings(void) akerr_ErrorContext *akgl_registry_init_sprite(void) { PREPARE_ERROR(errctx); - AKGL_REGISTRY_SPRITE = SDL_CreateProperties(); - FAIL_ZERO_RETURN(errctx, AKGL_REGISTRY_SPRITE, AKERR_NULLPOINTER, "Error initializing sprite registry"); + PASS(errctx, registry_create(&AKGL_REGISTRY_SPRITE, "sprite")); SUCCEED_RETURN(errctx); } akerr_ErrorContext *akgl_registry_init_spritesheet(void) { PREPARE_ERROR(errctx); - AKGL_REGISTRY_SPRITESHEET = SDL_CreateProperties(); - FAIL_ZERO_RETURN(errctx, AKGL_REGISTRY_SPRITESHEET, AKERR_NULLPOINTER, "Error initializing spritesheet registry"); + PASS(errctx, registry_create(&AKGL_REGISTRY_SPRITESHEET, "spritesheet")); SUCCEED_RETURN(errctx); } akerr_ErrorContext *akgl_registry_init_character(void) { PREPARE_ERROR(errctx); - AKGL_REGISTRY_CHARACTER = SDL_CreateProperties(); - FAIL_ZERO_RETURN(errctx, AKGL_REGISTRY_CHARACTER, AKERR_NULLPOINTER, "Error initializing character registry"); + PASS(errctx, registry_create(&AKGL_REGISTRY_CHARACTER, "character")); SUCCEED_RETURN(errctx); } diff --git a/src/renderer.c b/src/renderer.c index 79b282e..bd59005 100644 --- a/src/renderer.c +++ b/src/renderer.c @@ -126,9 +126,7 @@ akerr_ErrorContext *akgl_render_2d_draw_world(akgl_RenderBackend *self, akgl_Ite { PREPARE_ERROR(errctx); akgl_Iterator defflags; - SDL_Time curTime = SDL_GetTicksNS(); akgl_Actor *actor = NULL; - int j = 0; FAIL_ZERO_RETURN(errctx, self, AKERR_NULLPOINTER, "self"); diff --git a/src/sprite.c b/src/sprite.c index 820c46d..0dd5a11 100644 --- a/src/sprite.c +++ b/src/sprite.c @@ -136,7 +136,6 @@ akerr_ErrorContext *akgl_sprite_load_json(char *filename) CATCH(errctx, akgl_heap_next_string(&spritename)); CATCH(errctx, akgl_string_initialize(spritename, NULL)); - //SDL_snprintf((char *)&tmpstr->data, AKGL_MAX_STRING_LENGTH, "%s%s", SDL_GetBasePath(), filename); json = (json_t *)json_load_file(filename, 0, &error); FAIL_ZERO_BREAK( errctx, @@ -221,7 +220,6 @@ akerr_ErrorContext *akgl_spritesheet_initialize(akgl_SpriteSheet *sheet, int spr //CATCH(errctx, akgl_string_initialize(tmpstr, NULL)); strncpy((char *)&sheet->name, filename, AKGL_SPRITE_SHEET_MAX_FILENAME_LENGTH); - //snprintf((char *)&tmpstr->data, AKGL_MAX_STRING_LENGTH, "%s%s", SDL_GetBasePath(), filename); sheet->texture = IMG_LoadTexture(akgl_renderer->sdl_renderer, filename); FAIL_ZERO_BREAK(errctx, sheet->texture, AKGL_ERR_SDL, "Failed loading asset %s : %s", filename, SDL_GetError()); diff --git a/src/staticstring.c b/src/staticstring.c index 8026fa9..bd279a1 100644 --- a/src/staticstring.c +++ b/src/staticstring.c @@ -20,15 +20,15 @@ akerr_ErrorContext *akgl_string_initialize(akgl_String *obj, char *init) SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akgl_string_copy(akgl_String *src, akgl_String *dst, int count) +akerr_ErrorContext *akgl_string_copy(akgl_String *src, akgl_String *dest, int count) { PREPARE_ERROR(errctx); FAIL_ZERO_RETURN(errctx, src, AKERR_NULLPOINTER, "NULL argument"); - FAIL_ZERO_RETURN(errctx, dst, AKERR_NULLPOINTER, "NULL argument"); + FAIL_ZERO_RETURN(errctx, dest, AKERR_NULLPOINTER, "NULL argument"); if ( count == 0 ) { count = AKGL_MAX_STRING_LENGTH; } - if ( (char *)dst->data != strncpy((char *)&dst->data, (char *)&src->data, count) ) { + if ( (char *)dest->data != strncpy((char *)&dest->data, (char *)&src->data, count) ) { FAIL_RETURN(errctx, errno, "strncpy"); } SUCCEED_RETURN(errctx); diff --git a/src/tilemap.c b/src/tilemap.c index 2280f1d..7a4ff20 100644 --- a/src/tilemap.c +++ b/src/tilemap.c @@ -100,7 +100,7 @@ akerr_ErrorContext *akgl_get_json_properties_integer(json_t *obj, char *key, int SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akgl_get_json_properties_number(json_t *obj, char *key, float *dest) +akerr_ErrorContext *akgl_get_json_properties_number(json_t *obj, char *key, float32_t *dest) { PREPARE_ERROR(errctx); json_t *property = NULL; @@ -110,7 +110,7 @@ akerr_ErrorContext *akgl_get_json_properties_number(json_t *obj, char *key, floa SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akgl_get_json_properties_float(json_t *obj, char *key, float *dest) +akerr_ErrorContext *akgl_get_json_properties_float(json_t *obj, char *key, float32_t *dest) { PREPARE_ERROR(errctx); json_t *property = NULL; @@ -120,7 +120,7 @@ akerr_ErrorContext *akgl_get_json_properties_float(json_t *obj, char *key, float SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akgl_get_json_properties_double(json_t *obj, char *key, double *dest) +akerr_ErrorContext *akgl_get_json_properties_double(json_t *obj, char *key, float64_t *dest) { PREPARE_ERROR(errctx); json_t *property = NULL; @@ -558,7 +558,6 @@ akerr_ErrorContext *akgl_tilemap_load(char *fname, akgl_Tilemap *dest) ATTEMPT { //CATCH(errctx, akgl_heap_next_string(&tmpstr)); //CATCH(errctx, akgl_string_initialize(tmpstr, NULL)); - //SDL_snprintf(tmpstr->data, AKGL_MAX_STRING_LENGTH, "%s%s", SDL_GetBasePath(), fname); CATCH(errctx, aksl_realpath(fname, (char *)&dirnamestr->data, sizeof(dirnamestr->data))); dirname((char *)&dirnamestr->data); diff --git a/src/util.c b/src/util.c index e80219e..aa32564 100644 --- a/src/util.c +++ b/src/util.c @@ -21,7 +21,7 @@ #include /** - * @brief Resolve @p path against @p root and write the absolute result to @p dst. + * @brief Resolve @p path against @p root and write the absolute result to @p dest. * * The second half of akgl_path_relative: joins the two with a `/` and resolves * the join, so symlinks and `..` are folded out and the result is absolute. It @@ -36,10 +36,10 @@ * @param path Relative path to resolve. Required. An absolute @p path still gets * @p root pasted in front of it, which will not exist -- so callers * must not send absolute paths down this branch. - * @param dst Receives the resolved absolute path. Required, and must already be + * @param dest Receives the resolved absolute path. Required, and must already be * a claimed pool string. * @return `NULL` on success, otherwise an error context owned by the caller. - * @throws AKERR_NULLPOINTER If @p root, @p path, or @p dst is `NULL`. + * @throws AKERR_NULLPOINTER If @p root, @p path, or @p dest is `NULL`. * @throws AKERR_OUTOFBOUNDS If `root + path` is at least #AKGL_MAX_STRING_LENGTH * bytes. The message reports both the combined length and the limit. * @throws ENOENT If the joined path does not exist. Any other `errno` @@ -51,19 +51,18 @@ * scratch strings are never released. This is exactly the hazard AGENTS.md * warns about; it wants a `FAIL_BREAK`. */ -static akerr_ErrorContext *path_relative_root(char *root, char *path, akgl_String *dst) +static akerr_ErrorContext *path_relative_root(char *root, char *path, akgl_String *dest) { PREPARE_ERROR(errctx); akgl_String *pathbuf; akgl_String *strbuf; - char *result; int rootlen; int pathlen; int count; FAIL_ZERO_RETURN(errctx, root, AKERR_NULLPOINTER, "NULL argument"); FAIL_ZERO_RETURN(errctx, path, AKERR_NULLPOINTER, "NULL argument"); - FAIL_ZERO_RETURN(errctx, dst, AKERR_NULLPOINTER, "NULL argument"); + FAIL_ZERO_RETURN(errctx, dest, AKERR_NULLPOINTER, "NULL argument"); PASS(errctx, akgl_heap_next_string(&strbuf)); PASS(errctx, akgl_heap_next_string(&pathbuf)); @@ -86,7 +85,7 @@ static akerr_ErrorContext *path_relative_root(char *root, char *path, akgl_Strin )); RESTORE_GCC_WARNINGS CATCH(errctx, aksl_realpath((char *)&pathbuf->data, (char *)&strbuf->data, sizeof(strbuf->data))); - CATCH(errctx, akgl_string_copy(strbuf, dst, 0)); + CATCH(errctx, akgl_string_copy(strbuf, dest, 0)); } CLEANUP { IGNORE(akgl_heap_release_string(strbuf)); IGNORE(akgl_heap_release_string(pathbuf)); @@ -95,16 +94,15 @@ static akerr_ErrorContext *path_relative_root(char *root, char *path, akgl_Strin SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akgl_path_relative(char *root, char *path, akgl_String *dst) +akerr_ErrorContext *akgl_path_relative(char *root, char *path, akgl_String *dest) { PREPARE_ERROR(errctx); akgl_String *strbuf; - char *result; bool relative_to_root = false; FAIL_ZERO_RETURN(errctx, root, AKERR_NULLPOINTER, "NULL argument"); FAIL_ZERO_RETURN(errctx, path, AKERR_NULLPOINTER, "NULL argument"); - FAIL_ZERO_RETURN(errctx, dst, AKERR_NULLPOINTER, "NULL argument"); + FAIL_ZERO_RETURN(errctx, dest, AKERR_NULLPOINTER, "NULL argument"); PASS(errctx, akgl_heap_next_string(&strbuf)); @@ -112,7 +110,7 @@ akerr_ErrorContext *akgl_path_relative(char *root, char *path, akgl_String *dst) // Is path relative to our current working directory? CATCH(errctx, aksl_realpath(path, (char *)&strbuf->data, sizeof(strbuf->data))); // Yes it is. strbuf->data contains the absolute path. - CATCH(errctx, akgl_string_copy(strbuf, dst, 0)); + CATCH(errctx, akgl_string_copy(strbuf, dest, 0)); } CLEANUP { IGNORE(akgl_heap_release_string(strbuf)); } PROCESS(errctx) { @@ -129,7 +127,7 @@ akerr_ErrorContext *akgl_path_relative(char *root, char *path, akgl_String *dst) } FINISH(errctx, true); if ( relative_to_root == true ) { - PASS(errctx, path_relative_root(root, path, dst)); + PASS(errctx, path_relative_root(root, path, dest)); } SUCCEED_RETURN(errctx); } diff --git a/tests/bitmasks.c b/tests/bitmasks.c index 0a4c9cd..fb9fc3d 100644 --- a/tests/bitmasks.c +++ b/tests/bitmasks.c @@ -1,9 +1,21 @@ +/** + * @file bitmasks.c + * @brief The AKGL_BITMASK_* macros, including the ways an unparenthesized one breaks. + * + * These are macros, so the only thing that checks them is a call site. Most of + * this file is the ordinary arithmetic; the composition cases at the bottom are + * the ones that matter, because until 0.5.0 the macros expanded to bare + * expressions and every one of them was a silent misparse waiting for a caller. + */ + #include #include int main(void) { int mask = 0; + int counter = 0; + AKGL_BITMASK_ADD(mask, AKGL_ACTOR_STATE_ALIVE); if ( mask != AKGL_ACTOR_STATE_ALIVE ) return 1; @@ -27,5 +39,70 @@ int main(void) AKGL_BITMASK_ADD(mask, AKGL_ACTOR_STATE_FACE_DOWN); if ( mask != (AKGL_ACTOR_STATE_ALIVE | AKGL_ACTOR_STATE_MOVING_DOWN | AKGL_ACTOR_STATE_FACE_DOWN) ) return 1; + + // ---- composition, which is what the parentheses are for ---- + + // Negation. Unparenthesized, AKGL_BITMASK_HAS expanded to a bare + // `(x & y) == y`, so `!` bound to the `&` and the whole thing read as + // `!(mask & bit) == bit`. + // + // Note which case catches that and which does not. For a bit that *is* + // set, `!(nonzero)` is 0 and `0 == bit` is false -- the same answer the + // correct parse gives, so a test written that way passes either way. It + // only diverges for a bit that is *not* set and whose value is not 1: + // `!(0)` is 1, and `1 == 64` is false where the answer should be true. + // That is the shape below, and it is why this was worth a real case rather + // than an obvious one. + mask = AKGL_ACTOR_STATE_ALIVE; + if ( !AKGL_BITMASK_HAS(mask, AKGL_ACTOR_STATE_ALIVE) ) + return 1; + if ( AKGL_BITMASK_HAS(mask, AKGL_ACTOR_STATE_DEAD) ) + return 1; + if ( !AKGL_BITMASK_HAS(mask, AKGL_ACTOR_STATE_DEAD) ) { + counter = 1; + } else { + return 1; + } + if ( counter != 1 ) + return 1; + + // The same divergence through AKGL_BITMASK_HASNOT, which was written the + // same way: `!(0) != 64` is true where the answer should be false. + if ( !AKGL_BITMASK_HASNOT(mask, AKGL_ACTOR_STATE_DEAD) ) + return 1; + if ( AKGL_BITMASK_HASNOT(mask, AKGL_ACTOR_STATE_ALIVE) ) + return 1; + + // As an operand of a lower-precedence operator. + if ( AKGL_BITMASK_HAS(mask, AKGL_ACTOR_STATE_ALIVE) == false ) + return 1; + if ( (AKGL_BITMASK_HAS(mask, AKGL_ACTOR_STATE_ALIVE) & 1) != 1 ) + return 1; + + // The mutating macros as expressions rather than statements, and with an + // argument that is itself an expression: `x |= y` unparenthesized cannot be + // used in either position. + mask = 0; + counter = (AKGL_BITMASK_ADD(mask, AKGL_ACTOR_STATE_DYING)); + if ( counter != AKGL_ACTOR_STATE_DYING ) + return 1; + if ( (AKGL_BITMASK_CLEAR(mask)) != 0 ) + return 1; + if ( mask != 0 ) + return 1; + + // AKGL_BITMASK_CLEAR carries no trailing semicolon, so it is a statement + // like any other and does not swallow the one after it when it is the whole + // body of an unbraced `if`. + mask = AKGL_ACTOR_STATE_ALIVE; + counter = 0; + if ( mask != 0 ) + AKGL_BITMASK_CLEAR(mask); + counter = 1; + if ( mask != 0 ) + return 1; + if ( counter != 1 ) + return 1; + return 0; } diff --git a/tests/draw.c b/tests/draw.c index 209cd17..97eaf11 100644 --- a/tests/draw.c +++ b/tests/draw.c @@ -554,6 +554,80 @@ akerr_ErrorContext *test_draw_backend_without_a_renderer(void) SUCCEED_RETURN(errctx); } +/** + * @brief akgl_draw_background paints its checkerboard and restores the draw colour. + * + * Until 0.5.0 this function returned `void`, drew through the global renderer + * with no check on it, and left the draw colour changed. TODO.md listed it + * under "needs the offscreen renderer harness" purely because of that global; + * taking a backend is what makes it testable here alongside everything else in + * this file. + */ +akerr_ErrorContext *test_draw_background(void) +{ + PREPARE_ERROR(errctx); + SDL_Surface *shot = NULL; + SDL_Color after; + SDL_Color light = { 0x99, 0x99, 0x99, 0xff }; + SDL_Color dark = { 0x66, 0x66, 0x66, 0xff }; + + ATTEMPT { + CATCH(errctx, clear_target()); + // A deliberately odd colour, so "restored" cannot be confused with + // "happened to already be black". + FAIL_ZERO_BREAK( + errctx, + SDL_SetRenderDrawColor(akgl_renderer->sdl_renderer, 0x12, 0x34, 0x56, 0x78), + AKGL_ERR_SDL, "%s", SDL_GetError()); + + TEST_EXPECT_OK(errctx, + akgl_draw_background(akgl_renderer, TEST_TARGET_SIZE, TEST_TARGET_SIZE), + "painting the transparency checkerboard"); + + shot = SDL_RenderReadPixels(akgl_renderer->sdl_renderer, NULL); + FAIL_ZERO_BREAK(errctx, shot, AKGL_ERR_SDL, "%s", SDL_GetError()); + + // The pattern is 8x8 cells alternating on ((x ^ y) >> 3) & 1, so (0,0) + // and (8,8) share a colour and (8,0) is the other one. + TEST_ASSERT(errctx, pixel_is(shot, 0, 0, dark), "cell (0,0) is not the first checker colour"); + TEST_ASSERT(errctx, pixel_is(shot, 7, 7, dark), "cell (0,0) is not filled to its edge"); + TEST_ASSERT(errctx, pixel_is(shot, 8, 0, light), "cell (1,0) is not the second checker colour"); + TEST_ASSERT(errctx, pixel_is(shot, 0, 8, light), "cell (0,1) is not the second checker colour"); + TEST_ASSERT(errctx, pixel_is(shot, 8, 8, dark), "cell (1,1) is not the first checker colour"); + + FAIL_ZERO_BREAK( + errctx, + SDL_GetRenderDrawColor(akgl_renderer->sdl_renderer, &after.r, &after.g, &after.b, &after.a), + AKGL_ERR_SDL, "%s", SDL_GetError()); + TEST_ASSERT(errctx, + (after.r == 0x12) && (after.g == 0x34) && (after.b == 0x56) && (after.a == 0x78), + "the draw colour was left at %02x%02x%02x%02x rather than restored", + after.r, after.g, after.b, after.a); + + // Degenerate sizes paint nothing and are not an error. + CATCH(errctx, clear_target()); + TEST_EXPECT_OK(errctx, akgl_draw_background(akgl_renderer, 0, 0), + "painting a zero-sized background"); + TEST_EXPECT_OK(errctx, akgl_draw_background(akgl_renderer, -8, -8), + "painting a negative-sized background"); + SDL_DestroySurface(shot); + shot = SDL_RenderReadPixels(akgl_renderer->sdl_renderer, NULL); + FAIL_ZERO_BREAK(errctx, shot, AKGL_ERR_SDL, "%s", SDL_GetError()); + TEST_ASSERT(errctx, pixel_is(shot, 0, 0, testblack), + "a zero or negative sized background painted something"); + + TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER, + akgl_draw_background(NULL, 8, 8), + "painting a background through a NULL backend"); + } CLEANUP { + if ( shot != NULL ) { + SDL_DestroySurface(shot); + } + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + int main(void) { PREPARE_ERROR(errctx); @@ -594,6 +668,7 @@ int main(void) CATCH(errctx, test_draw_copy_and_paste_region()); CATCH(errctx, test_draw_preserves_render_draw_color()); CATCH(errctx, test_draw_backend_without_a_renderer()); + CATCH(errctx, test_draw_background()); } CLEANUP { SDL_Quit(); } PROCESS(errctx) { diff --git a/tests/registry.c b/tests/registry.c index 51ead59..15a57c1 100644 --- a/tests/registry.c +++ b/tests/registry.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include "testutil.h" @@ -158,6 +159,132 @@ akerr_ErrorContext *test_akgl_get_property_copies_only_the_value(void) SUCCEED_RETURN(errctx); } +/** + * @brief Every actor-state bit must be nameable, and every name must map to its own bit. + * + * akgl_registry_init_actor_state_strings walks AKGL_ACTOR_STATE_STRING_NAMES + * and registers entry `i` under the value `1 << i`. Character JSON resolves + * state names through that registry, so an entry that disagrees with the + * `#define` in actor.h is a state no character can ever bind a sprite to. Bits + * 11 and 12 were `UNDEFINED_11` and `UNDEFINED_12` here while actor.h called + * them MOVING_IN and MOVING_OUT, so both were unreachable by name. + * + * The loop also pins the array's length: it reads exactly + * AKGL_ACTOR_MAX_STATES entries, which is what the header declares and what + * the definition is now sized by. It was declared one longer than defined. + */ +akerr_ErrorContext *test_actor_state_string_names(void) +{ + PREPARE_ERROR(errctx); + int i = 0; + int j = 0; + Sint64 registered = 0; + + ATTEMPT { + CATCH(errctx, akgl_registry_init_actor_state_strings()); + + for ( i = 0; i < AKGL_ACTOR_MAX_STATES; i++ ) { + TEST_ASSERT(errctx, AKGL_ACTOR_STATE_STRING_NAMES[i] != NULL, + "actor state bit %d has no name", i); + + registered = SDL_GetNumberProperty( + AKGL_REGISTRY_ACTOR_STATE_STRINGS, + AKGL_ACTOR_STATE_STRING_NAMES[i], + 0); + TEST_ASSERT(errctx, registered == (Sint64)(1 << i), + "state name %s resolves to %lld, expected %d", + AKGL_ACTOR_STATE_STRING_NAMES[i], + (long long)registered, + (1 << i)); + + // No two entries may share a name, or the later one silently + // overwrites the earlier in the registry and one bit becomes + // unreachable without anything looking wrong. + for ( j = 0; j < i; j++ ) { + TEST_ASSERT(errctx, + strcmp(AKGL_ACTOR_STATE_STRING_NAMES[i], + AKGL_ACTOR_STATE_STRING_NAMES[j]) != 0, + "actor state bits %d and %d share the name %s", + j, i, AKGL_ACTOR_STATE_STRING_NAMES[i]); + } + } + + // The two that were unreachable, named explicitly so a regression reads + // as what it is rather than as an index. + TEST_ASSERT(errctx, + SDL_GetNumberProperty(AKGL_REGISTRY_ACTOR_STATE_STRINGS, + "AKGL_ACTOR_STATE_MOVING_IN", 0) + == (Sint64)AKGL_ACTOR_STATE_MOVING_IN, + "AKGL_ACTOR_STATE_MOVING_IN is not resolvable by name"); + TEST_ASSERT(errctx, + SDL_GetNumberProperty(AKGL_REGISTRY_ACTOR_STATE_STRINGS, + "AKGL_ACTOR_STATE_MOVING_OUT", 0) + == (Sint64)AKGL_ACTOR_STATE_MOVING_OUT, + "AKGL_ACTOR_STATE_MOVING_OUT is not resolvable by name"); + } CLEANUP { + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + +/** + * @brief Re-initializing a registry must replace the old set, not abandon it. + * + * Only akgl_registry_init_actor destroyed the set it was replacing; the other + * seven leaked an SDL_PropertiesID on every call after the first. A game that + * resets between levels calls these repeatedly. + * + * Also checks that akgl_registry_init() brings up the properties registry. + * It did not until 0.5.0, so akgl_set_property was a silent no-op for any + * caller that had not also gone through akgl_game_init. + */ +akerr_ErrorContext *test_akgl_registry_init_is_repeatable(void) +{ + PREPARE_ERROR(errctx); + SDL_PropertiesID first = 0; + akgl_String *readback = NULL; + + ATTEMPT { + CATCH(errctx, akgl_registry_init()); + TEST_ASSERT(errctx, AKGL_REGISTRY_PROPERTIES != 0, + "akgl_registry_init did not create the properties registry"); + + // Whether the old set was *destroyed* is not observable from here -- + // SDL hands out no liveness query, and it reuses ids -- so that half is + // the memcheck run's job, and `cmake --build build --target memcheck` + // gates on it. What is observable is that re-initializing really does + // replace: a key written before the call must be gone after it. + first = AKGL_REGISTRY_SPRITE; + SDL_SetNumberProperty(AKGL_REGISTRY_SPRITE, "akgl_test_sentinel", 1); + TEST_ASSERT(errctx, + SDL_GetNumberProperty(AKGL_REGISTRY_SPRITE, "akgl_test_sentinel", 0) == 1, + "could not write a sentinel into the sprite registry"); + CATCH(errctx, akgl_registry_init_sprite()); + TEST_ASSERT(errctx, AKGL_REGISTRY_SPRITE != 0, + "re-initializing the sprite registry left it unset"); + TEST_ASSERT(errctx, + SDL_GetNumberProperty(AKGL_REGISTRY_SPRITE, "akgl_test_sentinel", 0) == 0, + "re-initializing the sprite registry kept the old contents"); + + // Configuration set through the registry has to survive and read back, + // which is the behaviour the missing initializer was breaking. + CATCH(errctx, akgl_registry_init()); + TEST_EXPECT_OK(errctx, akgl_set_property("test.property", "1234"), + "setting a property after akgl_registry_init"); + TEST_EXPECT_OK(errctx, akgl_get_property("test.property", &readback, NULL), + "reading a property back after akgl_registry_init"); + TEST_ASSERT(errctx, strcmp((char *)&readback->data, "1234") == 0, + "property read back as '%s', expected '1234'", + (char *)&readback->data); + } CLEANUP { + if ( readback != NULL ) { + IGNORE(akgl_heap_release_string(readback)); + } + } PROCESS(errctx) { + } FINISH(errctx, true); + SUCCEED_RETURN(errctx); +} + int main(void) { PREPARE_ERROR(errctx); @@ -166,6 +293,8 @@ int main(void) TEST_TRAP_UNHANDLED_ERRORS(); CATCH(errctx, test_akgl_registry_init_creation_failures()); CATCH(errctx, test_akgl_get_property_copies_only_the_value()); + CATCH(errctx, test_actor_state_string_names()); + CATCH(errctx, test_akgl_registry_init_is_repeatable()); } CLEANUP { } PROCESS(errctx) { } FINISH_NORETURN(errctx); diff --git a/util/charviewer.c b/util/charviewer.c index 421829d..997e8c8 100644 --- a/util/charviewer.c +++ b/util/charviewer.c @@ -144,8 +144,8 @@ SDL_AppResult SDL_AppIterate(void *appstate) AKGL_BITMASK_CLEAR(opflags.flags); AKGL_BITMASK_ADD(opflags.flags, AKGL_ITERATOR_OP_UPDATE); AKGL_BITMASK_ADD(opflags.flags, AKGL_ITERATOR_OP_RENDER); - akgl_draw_background((int)akgl_camera->w, (int)akgl_camera->h); ATTEMPT { + CATCH(errctx, akgl_draw_background(akgl_renderer, (int)akgl_camera->w, (int)akgl_camera->h)); CATCH(errctx, akgl_tilemap_draw(akgl_gamemap, akgl_camera, i)); SDL_EnumerateProperties(AKGL_REGISTRY_ACTOR, &akgl_registry_iterate_actor, (void *)&opflags); } CLEANUP {