libakgl/AGENTS.md

# Repository Guidelines

## Project Structure & Module Organization

This is a C library intended to support the development of video games. Public C headers live in `include/akgl/`; keep declarations there aligned with their implementations in `src/`. Tests are standalone C programs under `tests/`, with JSON, image, and map fixtures in `tests/assets/`. The `util/` directory contains the `charviewer` utility and its sample assets. Third-party and companion libraries are vendored in `deps/`. Treat `build/` and generated `akgl.pc` files as build outputs rather than hand-maintained source; `include/akgl/SDL_GameControllerDB.h` is generated too, but is tracked on purpose — see below.

## Generated and Vendored Sources

### `include/akgl/SDL_GameControllerDB.h` is generated, and is tracked deliberately

`mkcontrollermappings.sh` regenerates this header by fetching the community
controller database from `raw.githubusercontent.com`. **It is committed to the
repository on purpose**: it is the offline fallback that keeps the library
buildable if upstream is renamed, rate-limited, taken down, or simply
unreachable from the build machine. Do not delete it, do not add it to
`.gitignore`, and do not "clean up" the fact that a generated file is tracked.

Working rules:

- **Never hand-edit it.** It is machine-written; changes belong in
  `mkcontrollermappings.sh`.
- **Do not commit incidental regeneration churn.** The build regenerates the
  header on every run, and the script stamps `$(date)` into a comment, so an
  ordinary build leaves the file modified with nothing of substance changed.
  Revert that before committing: `git checkout -- include/akgl/SDL_GameControllerDB.h`.
- **Update it as a deliberate, standalone commit** when you actually want newer
  mappings, so the diff is reviewable and bisectable.
- **Never commit a copy with `AKGL_SDL_GAMECONTROLLER_DB_LEN 0`.** That is the
  signature of a failed fetch, not an empty upstream — see the defect note
  below. Check the constant before staging this file.
- Formatting tooling ignores it: `scripts/reindent.sh` and the pre-commit hook
  both skip it.

> **Known defect (tracked in `TODO.md`).** The safety net is currently
> self-defeating. `mkcontrollermappings.sh` has no `set -e` and does not check
> `curl`'s exit status, so when the fetch fails it writes a header with
> `AKGL_SDL_GAMECONTROLLER_DB_LEN 0` and an empty array **over the good tracked
> copy, and exits 0**. Because CMake re-runs the generator on every build, an
> offline build silently destroys the very fallback the file exists to provide.
> Until that is fixed, treat a dirty `SDL_GameControllerDB.h` after a build as
> suspect and check the length constant before committing it.

## Build, Test, and Development Commands

Configure an out-of-tree development build:

```sh
cmake -S . -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo
```

Build all library, utility, and test targets:

```sh
cmake --build build --parallel
```

Run the complete test suite with failure details:

```sh
ctest --test-dir build --output-on-failure
```

Run one test while iterating, for example `ctest --test-dir build -R sprite --output-on-failure`. The `rebuild.sh` script also installs into a developer-specific `/home/andrew/local` prefix and removes existing outputs; prefer the portable commands above unless that exact workflow is intended.

Run only the performance suites with `ctest --test-dir build -L perf --output-on-failure`, or leave them out of an ordinary run with `-LE perf`. They take about 30 seconds together, print a table of nanoseconds per operation, and fail only when a measurement exceeds a budget set at roughly ten times the recorded baseline. `AKGL_BENCH_SCALE` scales every iteration count — `AKGL_BENCH_SCALE=0.1 ctest --test-dir build -L perf` for a quick look — and below 1.0 the budgets are reported but not enforced. The recorded baseline and what it means are in `PERFORMANCE.md`.

Check for memory defects with `cmake --build build --target memcheck`, or
`scripts/memcheck.sh` directly — it takes ctest's selection flags, so
`scripts/memcheck.sh -R tilemap` and `scripts/memcheck.sh -LE perf` both work.
It runs the suites that already exist under valgrind (`ctest -T memcheck`) with
the headless drivers forced, and exits non-zero when valgrind finds a definite
leak, an invalid access, or a read of uninitialised memory — which `ctest -T
memcheck` on its own will not do. The whole run takes about thirty seconds
because the perf suites scale themselves down under valgrind. Suppressions for
third-party findings live in `scripts/valgrind.supp`; add one only when the
finding genuinely cannot be fixed from this repository.

Run mutation testing with `cmake --build build --target mutation`. For a quick smoke run, use `scripts/mutation_test.py --target src/tilemap.c --max-mutants 10`; the harness mutates only a scratch copy and excludes the intentionally failing character test.

Generate HTML and Cobertura coverage reports with `cmake -S . -B build-coverage -DAKGL_COVERAGE=ON -DCMAKE_BUILD_TYPE=Debug`, then build and run CTest. Reports are written to `build-coverage/coverage/`; this mode requires `gcovr` and GCC or Clang.

## Continuous integration

`.gitea/workflows/ci.yaml` runs four jobs on every push, and each one exists
because the others cannot do its work:

| Job | Build | What it is for |
|---|---|---|
| `cmake_build` | Debug + `AKGL_COVERAGE=ON` | The unit suites and the coverage report. The perf suites run here too, at `AKGL_BENCH_SCALE=0.02`, for path coverage only — a timing taken under gcov is a measurement of gcov. |
| `performance` | RelWithDebInfo | `ctest -L perf`. The only job where budgets are enforced, because `tests/benchutil.h` enforces them only when compiled optimized at full scale. Keeps the tables as an artifact. |
| `memory_check` | RelWithDebInfo | `scripts/memcheck.sh`, every suite under valgrind. `continue-on-error` until TODO.md "Memory checking" items 34-39 are closed; keeps the valgrind logs as an artifact. |
| `mutation_test` | Debug | One focused source file, so CI stays bounded. |

The `character` suite is excluded wherever a whole run is selected: it fails
deliberately, and pinning current-but-wrong behavior is not what it is for.

## Coding Style

The canonical style is Emacs `cc-mode` **`stroustrup`**, with tabs enabled. This
is not advisory — new and edited code must match what `cc-mode` produces, so that
reindenting a region never shows up as a diff.

### Emacs setup

`.dir-locals.el` in the repository root already applies the style to every
`c-mode` buffer, so nothing needs configuring by hand:

```elisp
((c-mode . ((c-file-style . "stroustrup")
            (indent-tabs-mode . t)
            (tab-width . 8)
            (fill-column . 100)
            (require-final-newline . t))))
```

Interactively: `C-x h C-M-\` (`mark-whole-buffer` + `indent-region`) reindents
the buffer. A correctly formatted file is a fixed point of that operation —
reindenting must produce no change.

### Reindenting from the command line

```sh
scripts/reindent.sh                 # reindent every tracked C source in place
scripts/reindent.sh src/tilemap.c   # reindent only the named files
scripts/reindent.sh --check         # list non-conforming files, change nothing
```

`--check` exits 1 when something needs reindenting and 2 if Emacs is missing, so
it is usable from CI. The script drives Emacs in batch mode through
`scripts/reindent.el`, which reindents, normalises leading whitespace to the
canonical tab/space mix, strips trailing whitespace, and ensures a final
newline. `include/akgl/SDL_GameControllerDB.h` is generated and always skipped.

Note that `reindent.el` deliberately does **not** use Emacs' `tabify`: that
function's `tabify-regexp` is `" [ \t]+"`, which is not anchored to the start of
a line, so it rewrites runs of spaces anywhere and destroys the hand-aligned
value columns in the bit-flag tables in `actor.h` and `iterator.h`. Only leading
whitespace is ever rewritten.

### The pre-commit hook

`scripts/hooks/pre-commit` reindents staged C sources before the commit is
written. Enable it once per clone:

```sh
git config core.hooksPath scripts/hooks
```

It inspects the *staged* content rather than the working tree, so what lands in
the commit is what was checked. When a file needs reindenting it is fixed in the
working tree and re-staged — but only if the index and working tree agree for
that file. If they differ, re-staging would sweep unstaged work into the commit,
so the hook stops and tells you to reindent and stage it yourself. It steps
aside during a merge, and warns rather than blocking if Emacs is unavailable.
`git commit --no-verify` bypasses it.

For reference, `cc-mode` defines the style as:

```elisp
("stroustrup"
 (c-basic-offset . 4)
 (c-comment-only-line-offset . 0)
 (c-offsets-alist . ((statement-block-intro . +)
                     (substatement-open      . 0)
                     (substatement-label     . 0)
                     (label                  . 0)
                     (statement-cont         . +))))
```

### Indentation and whitespace

- **4 columns per level** (`c-basic-offset` 4).
- **Tabs are 8 columns wide and are used for indentation** (`indent-tabs-mode`
  `t`, `tab-width` 8). Emacs emits the largest possible run of tabs and pads the
  remainder with spaces, which produces this ladder. Editors that expand tabs, or
  that assume a 4-column tab, will silently corrupt it:

  | Depth | Columns | Bytes |
  |---|---|---|
  | 1 | 4  | 4 spaces |
  | 2 | 8  | `TAB` |
  | 3 | 12 | `TAB` + 4 spaces |
  | 4 | 16 | `TAB` `TAB` |
  | 5 | 20 | `TAB` `TAB` + 4 spaces |

- No trailing whitespace. Files end with a single newline.
- `case` labels sit at the same column as their `switch` (`label . 0`).
- Continuation lines of a wrapped expression indent one level (`statement-cont . +`).

Most of `src/` already conforms. The known non-conforming files are
`src/json_helpers.c`, `src/util.c` (from `akgl_rectangle_points` onward),
`src/assets.c`, `src/staticstring.c`, parts of `src/actor.c`, and the headers
`include/akgl/util.h` and `include/akgl/staticstring.h` — all of which use a
2-column offset. Convert a file to the canonical style in its own commit, not
mixed into a behavioral change.

### Braces

- **Function bodies open on their own line, in column 0.**
- **Control statements keep the brace on the same line**, one space before it.
- **`else`, `else if`, and `while` of a `do`/`while` stay on the closing-brace
  line**: `} else {`, not a bare `else` on the next line. Note that this is a
  house convention rather than something `cc-mode` enforces — the `stroustrup`
  style governs indentation only and will not move a brace or an `else` for you.
- **Always brace, even single-statement bodies.**

```c
akerr_ErrorContext *akgl_actor_update(akgl_Actor *obj)
{
    if ( obj->curSpriteFrameId == 0 ) {
        obj->curSpriteReversing = false;
    } else if ( obj->parent != NULL ) {
        obj->curSpriteFrameId -= 1;
    } else {
        obj->curSpriteFrameId += 1;
    }
}
```

### Spacing

- **Spaces inside control-flow parentheses**: `if ( x == y ) {`, `while ( done == false ) {`,
  `for ( i = 0; i < len; i++ ) {`. This is the dominant existing convention
  (140 sites to 7) and `cc-mode` will not add or remove it.
- No space between a function name and its argument list, at both call and
  definition sites, and no padding inside call parentheses: `SDL_Log("x %d", n)`.
- Binary operators and assignment are surrounded by single spaces; unary
  operators are not separated from their operand.
- **The pointer `*` binds to the identifier**, not the type: `char *name`,
  `akgl_Actor **dest`. Never `char* name`.
- When a call is too long for one line, put each argument on its own line
  indented one level past the callee, with the closing `)` on its own line. This
  is the established shape for the `FAIL_*` and `CATCH` macros:

  ```c
  FAIL_ZERO_RETURN(
      errctx,
      SDL_SetPointerProperty(AKGL_REGISTRY_ACTOR, name, (void *)obj),
      AKERR_KEY,
      "Unable to add actor to registry"
      );
  ```

### No repository-wide formatter

There is no `clang-format` or linter wired into the build, and `cc-mode`'s
indentation engine is not exactly reproducible by `clang-format`. Do not
introduce one without discussion, and do not reformat code you are not otherwise
changing — unrelated whitespace churn makes review harder and is the reason the
style drifted in the first place.

## Naming Conventions

- **Public functions**: `akgl_<subsystem>_<verb>`, all lower snake_case —
  `akgl_actor_set_character`, `akgl_heap_next_string`. No camelCase, and never
  embed a type name (`akgl_Actor_cmhf_left_on` is wrong; it should be
  `akgl_actor_cmhf_left_on`).
- **Public types**: `akgl_TypeName` — `akgl_Actor`, `akgl_SpriteSheet`,
  `akgl_PhysicsBackend`. Every type exported from a header takes the prefix;
  bare names like `point` and `RectanglePoints` are defects, not precedent.
- **Constants and macros**: `AKGL_UPPER_SNAKE_CASE`. A constant belongs to the
  subsystem it describes: a character limit is `AKGL_CHARACTER_MAX_*`, not
  `AKGL_SPRITE_MAX_CHARACTER_*`. Name a constant for what its value *is* — a
  nanoseconds-per-millisecond scale factor is `AKGL_TIME_ONEMS_NS`.
- **Exported globals** take the `akgl_` prefix like any other public symbol. Do
  not add bare names (`renderer`, `camera`, `window`), leading-underscore names
  (reserved at file scope), or SCREAMING_SNAKE names for mutable objects —
  `AKGL_UPPER_SNAKE_CASE` is for constants.
- **`static` helpers drop the `akgl_` prefix**, which exists only to avoid
  external collisions.
- **Include guards**: `_AKGL_<FILE>_H_`, matching the file name. Guard names
  without the project prefix risk colliding with system headers.
- **Parameter names must match between the declaration and the definition** —
  Doxygen publishes the header spelling. Conventional names: `dest` for an
  output parameter (not `dst`), `self` for a backend receiver, `obj` for the
  instance being initialized or inspected, `e` for an incoming error context to
  be inspected.
- **The local error context is named `errctx`** (92 sites to 45). New code uses
  `errctx`; convert `e` when you touch a function for other reasons.
- **Header/source pairs are named by feature**: `include/akgl/sprite.h` and
  `src/sprite.c`.

## Error-Handling Protocol

The `akerror` control-flow macros have a shape that must be followed exactly,
because the failure mode is silent.

- Inside an `ATTEMPT` block use the **`_BREAK`** variants (`FAIL_ZERO_BREAK`,
  `FAIL_NONZERO_BREAK`, `FAIL_BREAK`) and `CATCH`. Outside it use the
  **`_RETURN`** variants.
- **Never use a `*_RETURN` macro inside an `ATTEMPT` block.** It returns past the
  `CLEANUP` block, so every release, `fclose`, and free in `CLEANUP` is skipped.
  This has already caused a heap-string leak on the success path of
  `akgl_get_json_tilemap_property`.
- `CLEANUP` must precede `PROCESS`. Transposing them moves the cleanup body into
  the `PROCESS` switch, where it runs only when an error context exists.
- `CATCH` reports failure by `break`ing, which binds to the innermost enclosing
  loop or `switch`. A `CATCH` written directly inside a `while` exits the loop
  rather than the function — put the `ATTEMPT` block inside the loop.
- **Never `return` from inside a `HANDLE` block either.** `FINISH` ends with
  `RELEASE_ERROR`, so leaving before it means the handled context is never given
  back to `AKERR_ARRAY_ERROR` — one leaked slot per call, and the 129th call
  aborts the process with "Unable to pull an error context from the array!".
  `SUCCEED_RETURN` is safe because it releases first; a bare `return`, or a
  `return f(...)` that tail-calls the fallback, is not. Set a flag in the
  `HANDLE` block and act on it after `FINISH`. This has already cost a
  process-killing leak in `akgl_path_relative`; see TODO.md, "Performance".
- Validate every pointer parameter before dereferencing it, including the ones a
  sibling function happens not to check.

## API Surface

Every function with external linkage must be declared in a header, and every
declaration must have a definition. A non-`static` function that appears in no
header is still in the ABI but is unreachable by callers; a declaration with no
definition is a link error for anyone compiling against the header alone. If a
function is exposed only so tests can reach it, declare it under the existing
"part of the internal API" comment block in the relevant header.

Headers must be self-contained: include what you use, so that a translation unit
including a single `akgl` header compiles without a particular include order.
Within the project use the angled form, `#include <akgl/sibling.h>`.

Declare no-argument functions as `(void)`, not `()`.

## Rules

- Add yourself (agent program name, model name and version) as a co-author on every commit message
- Avoid dynamic memory allocation. Use the `akgl_heap_*` methods to retrieve necessary objects at runtime, and to release them when done. If the akgl heap facilities don't provide the kind of object needed, create a new heap layer to support that type of object.

## Testing Guidelines

Tests use simple executable return codes and are registered through CTest in `CMakeLists.txt`; there is no declared coverage threshold. Add focused tests as `tests/<feature>.c`, create a matching `test_<feature>` target, and register it with `add_test`. Put reusable fixtures in `tests/assets/` and keep paths compatible with tests launched from the build tree. Coverage mode wraps the suite in a CTest fixture so counters are reset before tests and reports are generated afterward.

### Performance suites

`tests/perf.c` (nothing that draws) and `tests/perf_render.c` (everything that
does) are benchmarks, built and registered like any other suite but listed in
`AKGL_PERF_SUITES` so they carry the `perf` label and a longer timeout. The
harness is `tests/benchutil.h`. Three rules keep the numbers honest, and all
three were learned by getting them wrong first:

- **Nothing that checks an error goes inside the clock.** `PASS` and `CATCH`
  call `akerr_valid_error_address`, which walks `AKERR_ARRAY_ERROR` — more work
  than several of the calls being measured. Use `BENCH_LOOP`, which stashes the
  context and stops at the first failure, and hand it to `PASS` afterwards.
- **Flush the renderer before stopping the clock.** SDL batches: a `draw_*` call
  queues a command and returns. `BENCH_FLUSH_STOP` in `tests/perf_render.c` is
  there because the first version of that suite measured queueing, reported a
  tilemap frame 250 times faster than it is, and paid the real cost at teardown
  inside `SDL_DestroyTexture`.
- **A drawing benchmark needs a raw-SDL control that does the same pixel work.**
  Without one there is no way to separate what libakgl costs from what the
  rasterizer costs, and the answer is not the one you would guess — see
  `PERFORMANCE.md`.

Budgets are per-operation ceilings at roughly ten times the recorded baseline;
they are enforced only in an optimized build at full scale. When a change moves
a number for a good reason, re-record the baseline in `PERFORMANCE.md` in the
same commit and say why it moved.

These two suites are also the memory-check vehicle. `tests/benchutil.h` detects
valgrind from `LD_PRELOAD` and drops the scale to
`AKGL_BENCH_VALGRIND_SCALE`, so the same binaries walk every path once instead
of a hundred thousand times, and the timings a checked run prints are labelled
as meaningless. **Do not add memory-check suites**: a new path worth checking
belongs in a benchmark, where it gets both.

## Commit & Pull Request Guidelines

Recent commits use concise, imperative summaries such as `Fix a scale bug...` and often explain related changes in one sentence. Keep each commit scoped and describe user-visible behavior. Pull requests should summarize the change, identify affected modules, list the CMake/CTest commands run, and link relevant issues. Include screenshots only for rendering, map, or `charviewer` changes.