AGENTS.md described style in one paragraph that said little more than "follow the surrounding code", which was not actionable once files had drifted apart. Replace it with an explicit specification. The canonical style is Emacs cc-mode "stroustrup" with tabs enabled: a 4-column offset with 8-column tabs, so depth 1 is four spaces, depth 2 is one tab, depth 3 is a tab plus four spaces. Most of src/ already followed this; what looked like randomly mixed tabs and spaces was simply cc-mode output. Document the byte ladder explicitly, since editors that expand tabs or assume a 4-column tab silently corrupt it. Rules beyond indentation are derived from counted majorities in the existing code rather than invented: errctx over e (92 to 45), padded control parens (140 to 7), pointer binding to the identifier (486 to 5), and always-brace (only four unbraced bodies exist). Brace and else placement are called out as house conventions that cc-mode does not enforce, so "run the indenter" and "follow the guide" cannot conflict. Also record the naming, error-handling, and API-surface rules that a consistency sweep showed were being broken silently -- in particular that a *_RETURN macro inside an ATTEMPT block skips CLEANUP. Add the tooling to apply it: .dir-locals.el applies the style to every c-mode buffer scripts/reindent.el batch reindent via Emacs scripts/reindent.sh reindent or --check the tree scripts/hooks/pre-commit reindents staged sources reindent.el deliberately avoids Emacs' tabify: its 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, and lines starting inside a string literal are skipped. The hook checks staged content rather than the working tree, so what is committed is what was verified. It re-stages a fixed file only when the index and working tree agree, otherwise a partial `git add -p` would sweep unstaged work into the commit. Enable with: git config core.hooksPath scripts/hooks Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
15 KiB
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.shand the pre-commit hook both skip it.
Known defect (tracked in
TODO.md). The safety net is currently self-defeating.mkcontrollermappings.shhas noset -eand does not checkcurl's exit status, so when the fetch fails it writes a header withAKGL_SDL_GAMECONTROLLER_DB_LEN 0and 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 dirtySDL_GameControllerDB.hafter a build as suspect and check the length constant before committing it.
Build, Test, and Development Commands
Configure an out-of-tree development build:
cmake -S . -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo
Build all library, utility, and test targets:
cmake --build build --parallel
Run the complete test suite with failure details:
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 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.
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:
((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
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:
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:
("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-offset4). -
Tabs are 8 columns wide and are used for indentation (
indent-tabs-modet,tab-width8). 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 TAB3 12 TAB+ 4 spaces4 16 TABTAB5 20 TABTAB+ 4 spaces -
No trailing whitespace. Files end with a single newline.
-
caselabels sit at the same column as theirswitch(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, andwhileof ado/whilestay on the closing-brace line:} else {, not a bareelseon the next line. Note that this is a house convention rather than somethingcc-modeenforces — thestroustrupstyle governs indentation only and will not move a brace or anelsefor you.- Always brace, even single-statement bodies.
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) andcc-modewill 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. Neverchar* 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 theFAIL_*andCATCHmacros: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_onis wrong; it should beakgl_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 likepointandRectanglePointsare defects, not precedent. - Constants and macros:
AKGL_UPPER_SNAKE_CASE. A constant belongs to the subsystem it describes: a character limit isAKGL_CHARACTER_MAX_*, notAKGL_SPRITE_MAX_CHARACTER_*. Name a constant for what its value is — a nanoseconds-per-millisecond scale factor isAKGL_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_CASEis for constants. statichelpers drop theakgl_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:
destfor an output parameter (notdst),selffor a backend receiver,objfor the instance being initialized or inspected,efor an incoming error context to be inspected. - The local error context is named
errctx(92 sites to 45). New code useserrctx; convertewhen you touch a function for other reasons. - Header/source pairs are named by feature:
include/akgl/sprite.handsrc/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
ATTEMPTblock use the_BREAKvariants (FAIL_ZERO_BREAK,FAIL_NONZERO_BREAK,FAIL_BREAK) andCATCH. Outside it use the_RETURNvariants. - Never use a
*_RETURNmacro inside anATTEMPTblock. It returns past theCLEANUPblock, so every release,fclose, and free inCLEANUPis skipped. This has already caused a heap-string leak on the success path ofakgl_get_json_tilemap_property. CLEANUPmust precedePROCESS. Transposing them moves the cleanup body into thePROCESSswitch, where it runs only when an error context exists.CATCHreports failure bybreaking, which binds to the innermost enclosing loop orswitch. ACATCHwritten directly inside awhileexits the loop rather than the function — put theATTEMPTblock inside the loop.- 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.
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.