Files
libakgl/src/registry.c
Andrew Kesterson 19530f6a97 Fix the type, macro and state-table defects, and the leftover debris
Closes internal-consistency items 19 through 36, 38 and 41. Item 37, the ~180
redundant casts, is deliberately left open with its reasoning in TODO.md: the
benefit only arrives once the build turns on the warnings those casts suppress,
and doing it before that is churn across the two files with the most
outstanding functional defects.

The two that were real bugs are in the actor state table.
AKGL_ACTOR_STATE_STRING_NAMES was declared [AKGL_ACTOR_MAX_STATES+1] and
defined [32], so a consumer trusting the declared bound read past the object;
and indices 11 and 12 were named UNDEFINED_11 and UNDEFINED_12 where actor.h
has MOVING_IN and MOVING_OUT, so no character JSON could bind a sprite to
either state. tests/registry.c now walks the whole table -- every entry
non-NULL, every entry resolving to its own bit, no two entries sharing a name.

The bitmask macros are parenthesized and AKGL_BITMASK_CLEAR has lost the
semicolon inside its body. Writing tests/bitmasks.c for that turned up
something worth knowing: the obvious test does not catch it. For a bit that is
set, the misparse `!(mask & bit) == bit` gives the same answer as the correct
one. It only diverges for an unset bit whose value is not 1, and that is the
shape the suite uses now.

akgl_draw_background was the last public function outside the error protocol.
It takes a backend like everything else in draw.h, restores the draw colour it
found, and is tested -- TODO.md had it filed under "needs the offscreen
renderer harness", which was never true; what it needed was to stop reading the
global.

All eight registry initializers go through one helper, so the seven that leaked
an SDL_PropertiesID on every call after the first no longer do. Fixed in the
same place because it is the same function: akgl_registry_init never called
akgl_registry_init_properties, which made akgl_set_property a silent no-op for
anyone not going through akgl_game_init -- Defects, Known and still open item 3.

Also: AKGL_COLLIDE_RECTANGLES (three open parens, two closes) and akgl_Frame
deleted, float32_t/float64_t used consistently, the developer-specific debug
logging removed from the controller inner loop, the abandoned SDL_GetBasePath
comments removed, nine unused locals removed, and dst renamed to dest.

akgl_game_update's default flags no longer OR the same bit twice. That changes
nothing today, and the reason is Performance item 32: the loop never reads
either bit, which is why every actor is updated sixteen times a frame. Still
open.

25/25 pass, memcheck clean, reindent --check clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 07:33:36 -04:00

253 lines
8.3 KiB
C

/**
* @file registry.c
* @brief Implements the registry subsystem.
*/
#include <SDL3/SDL.h>
#include <string.h>
#include <akerror.h>
#include <jansson.h>
#include <akstdlib.h>
#include <akgl/heap.h>
#include <akgl/sprite.h>
#include <akgl/registry.h>
#include <akgl/iterator.h>
#include <akgl/actor.h>
#include <akgl/json_helpers.h>
SDL_PropertiesID AKGL_REGISTRY_ACTOR = 0;
SDL_PropertiesID AKGL_REGISTRY_ACTOR_STATE_STRINGS = 0;
SDL_PropertiesID AKGL_REGISTRY_SPRITE = 0;
SDL_PropertiesID AKGL_REGISTRY_SPRITESHEET = 0;
SDL_PropertiesID AKGL_REGISTRY_CHARACTER = 0;
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);
ATTEMPT {
CATCH(errctx, akgl_registry_init_spritesheet());
CATCH(errctx, akgl_registry_init_sprite());
CATCH(errctx, akgl_registry_init_character());
CATCH(errctx, akgl_registry_init_actor());
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);
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_init_actor(void)
{
PREPARE_ERROR(errctx);
PASS(errctx, registry_create(&AKGL_REGISTRY_ACTOR, "actor"));
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_init_font(void)
{
PREPARE_ERROR(errctx);
PASS(errctx, registry_create(&AKGL_REGISTRY_FONT, "font"));
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_init_music(void)
{
PREPARE_ERROR(errctx);
PASS(errctx, registry_create(&AKGL_REGISTRY_MUSIC, "music"));
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_init_properties(void)
{
PREPARE_ERROR(errctx);
PASS(errctx, registry_create(&AKGL_REGISTRY_PROPERTIES, "properties"));
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_init_actor_state_strings(void)
{
int i = 0;
int flag = 0;
PREPARE_ERROR(errctx);
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(
AKGL_REGISTRY_ACTOR_STATE_STRINGS,
AKGL_ACTOR_STATE_STRING_NAMES[i],
flag);
}
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_init_sprite(void)
{
PREPARE_ERROR(errctx);
PASS(errctx, registry_create(&AKGL_REGISTRY_SPRITE, "sprite"));
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_init_spritesheet(void)
{
PREPARE_ERROR(errctx);
PASS(errctx, registry_create(&AKGL_REGISTRY_SPRITESHEET, "spritesheet"));
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_init_character(void)
{
PREPARE_ERROR(errctx);
PASS(errctx, registry_create(&AKGL_REGISTRY_CHARACTER, "character"));
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_registry_load_properties(char *fname)
{
json_t *json = NULL;
json_t *props = NULL;
const char *pkey = NULL;
json_t *pvalue = NULL;
json_error_t error;
akgl_String *tmpstr;
PREPARE_ERROR(errctx);
FAIL_ZERO_RETURN(errctx, fname, AKERR_NULLPOINTER, "null filename");
// One ATTEMPT around the whole function, rather than one for the parse and
// a bare loop after it. `props` is a borrowed reference into `json` and is
// read by the loop, so the document cannot be released until the loop is
// done -- and with the loop outside the block, every path out of it leaked
// the document. The per-property ATTEMPT stays nested inside the loop,
// which is what AGENTS.md requires: a CATCH written directly in a loop
// breaks the loop rather than leaving the function.
ATTEMPT {
SDL_Log("Loading from %s", fname);
json = json_load_file(fname, 0, &error);
FAIL_ZERO_BREAK(
errctx,
json,
AKERR_NULLPOINTER,
"Error while loading properties from %s on line %d: %s-",
fname,
error.line,
error.text);
CATCH(errctx, akgl_get_json_object_value(json, "properties", &props));
json_object_foreach(props, pkey, pvalue) {
ATTEMPT {
CATCH(errctx, akgl_heap_next_string(&tmpstr));
CATCH(errctx, akgl_get_json_string_value(props, (char *)pkey, &tmpstr));
SDL_SetStringProperty(AKGL_REGISTRY_PROPERTIES, pkey, tmpstr->data);
SDL_Log("Set property %s = %s", pkey, tmpstr->data);
CATCH(errctx, akgl_heap_release_string(tmpstr));
} CLEANUP {
} PROCESS(errctx) {
} FINISH(errctx, true);
}
} CLEANUP {
// Every value was copied into the property store on the way past, so
// nothing outlives the document.
if ( json != NULL ) {
json_decref(json);
json = NULL;
}
} PROCESS(errctx) {
} FINISH(errctx, true);
SDL_Log("Properties loaded");
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_set_property(char *name, char *value)
{
PREPARE_ERROR(errctx);
FAIL_ZERO_RETURN(errctx, name, AKERR_NULLPOINTER, "NULL char *");
FAIL_ZERO_RETURN(errctx, value, AKERR_NULLPOINTER, "NULL char *");
SDL_SetStringProperty(AKGL_REGISTRY_PROPERTIES, name, value);
SUCCEED_RETURN(errctx);
}
akerr_ErrorContext *akgl_get_property(char *name, akgl_String **dest, char *def)
{
const char *value = NULL;
size_t valuelen = 0;
PREPARE_ERROR(errctx);
FAIL_ZERO_RETURN(errctx, name, AKERR_NULLPOINTER, "NULL char *");
FAIL_ZERO_RETURN(errctx, dest, AKERR_NULLPOINTER, "NULL akgl_String *");
ATTEMPT {
if ( *dest == NULL ) {
CATCH(errctx, akgl_heap_next_string(dest));
}
// Copy the value's own length, not the destination's capacity. What
// SDL hands back is its strdup of the value -- four bytes for "0.0" --
// so copying a fixed AKGL_MAX_STRING_LENGTH read up to 4 KiB past the
// end of somebody else's allocation on every call. It returned garbage
// past the terminator, and it would have faulted outright on a value
// that happened to sit at the end of a page.
value = SDL_GetStringProperty(AKGL_REGISTRY_PROPERTIES, (const char *)name, (const char *)def);
// An unset property with a NULL default still reports AKERR_NULLPOINTER,
// which is what the header has always promised. It used to arrive from
// inside aksl_memcpy; now it is raised here, before anything is measured.
FAIL_ZERO_BREAK(
errctx,
value,
AKERR_NULLPOINTER,
"Property %s is not set and no default was given",
name);
valuelen = strlen(value);
FAIL_NONZERO_BREAK(
errctx,
(valuelen >= AKGL_MAX_STRING_LENGTH),
AKERR_OUTOFBOUNDS,
"Property %s is %zu bytes, which does not fit an akgl_String (%d)",
name,
valuelen,
AKGL_MAX_STRING_LENGTH);
CATCH(errctx, aksl_memcpy((*dest)->data, (void *)value, (valuelen + 1)));
} CLEANUP {
} PROCESS(errctx) {
} FINISH(errctx, true);
SUCCEED_RETURN(errctx);
}