Files
dusk/.claude/code-check.md
T
YourWishes 56230dd340 Fix ref-count underflow, PSP timezone units, thread restart race, color codegen
- util/ref.c: refUnlock's assert(count >= 0) on an unsigned count was
  tautological, so a double-unlock silently underflowed to UINT32_MAX
  instead of asserting. Now asserts count > 0 before decrementing.
- duskpsp/time/timepsp.c: timeGetRealTimeZonePSP returned hours while
  every other platform (and timeepoch.c's math) expects seconds.
- thread.c: threadHandler reset threadId outside the mutex, after
  signaling STOPPED, letting a caller's immediate threadStart() race
  threadStartRequest()'s "thread id not 0" assert. threadId is now
  reset inside the same locked section.
- tools/color.py: whole-number CSV channels (0, 1) produced invalid C
  float literals (0f, 1f) for the generated COLOR_*_3F/_4F macros.
  Route through float() so they always stringify with a decimal point.

Also adds .claude/code-check.md: a full review pass over every
subsystem in src/dusk/, with the above (plus several other findings
not yet acted on) written up in detail.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-31 15:02:17 -05:00

42 KiB

Dusk Code Check

A working checklist for walking through src/dusk/ (the core, platform- agnostic engine layer) one subsystem at a time and reviewing how it actually looks today -- not just what it's supposed to do. Pairs with STATUS.md (maturity snapshot) and ROADMAP.md (forward-looking work); this doc is about auditing what's already there.

Each section below: Purpose, Key files, and a Review status line to fill in as each system gets walked through (e.g. Not started / In progress / Reviewed 2026-08-01: <one-line finding>).

First full pass completed 2026-08-01 (all 23 sections below reviewed). See "Top cross-cutting findings" right after the index for the highest-priority items across all of them before diving into any one section -- this doc is long, don't make someone read all 23 to find the load-bearing bugs.

Game-specific code lives in src/duskrpg/ (not covered here) and platform-specific implementations live in src/dusk<platform>/ / src/duskgl/ / src/dusksdl2/ (also not covered here) -- see CLAUDE.md for the layer structure.


Index


Top cross-cutting findings

Ranked roughly by real-world impact, not by section order. Each links to its full writeup below.

  1. Bounds/capacity checks that matter at runtime are assert* calls, which compile to no-ops in release (DUSK_ASSERTIONS_FAKED, set whenever NDEBUG is defined) -- confirmed by reading assert.h directly. This shows up in entity (entitymanager.c, component.c -- reachable from JerryScript via moduleentity.c with no return-value check) and scene (sceneCreate's pool-exhaustion path, reachable from new Scene()). Untrusted script input can silently get back an invalid ID that later indexes a fixed array out of bounds in a release build -- the exact "validate untrusted data with errorThrow, not asserts" case CLAUDE.md/project memory already calls out, just not applied here.
  2. FIXED 2026-08-01. util/ref.c's refUnlock decremented an unsigned count guarded only by a tautological count >= 0 assert (util) -- a double-unlock silently underflowed to UINT32_MAX instead of tripping an assert. This wasn't just a util/ nit: it's the lock/unlock primitive asset/loader/assetentry.c builds on (see asset), so an over-unlocked asset entry became permanently un-reapable rather than erroring loudly. Now asserts count > 0 before decrementing; the pointless count >= 0 check in refLock was removed too.
  3. texture.c:42-52's textureInit() reads texture->format to decide which data fields to validate before the struct is zeroed a few lines later (display) -- it's validating whatever stale value happened to be in the caller's (possibly stack) memory, not the format parameter actually being requested.
  4. FIXED 2026-08-01 (confirmed real before fixing). PSP's timeGetRealTimeZonePSP returned hours; every other platform (timeGetRealTimeZoneLinux/Dolphin) returns seconds, and timeEpochInit/timeEpochGetHours etc. add the timezone value directly to a Unix-epoch-seconds timestamp -- confirmed by reading all three platform implementations plus timeepoch.c directly, not just trusting the review agent's claim. The PSP function's own comment even said "Return timezone offset in hours". Now divides by 1000000.0 only (microseconds -> seconds), matching the other platforms. See time.
  5. Discussed, not changed. consolePrint aborts the process (via stringFormatVA's assert(ret < destSize)) instead of truncating when a single debug line exceeds 511 bytes. This is a real, if rare, crash risk (any format producing >511 bytes of runtime-variable content -- a long path, a long error string -- takes the whole process down over a debug print), but changing stringFormatVA itself would affect every caller in the codebase, and a console-local truncating rewrite is easy to do wrong (vsnprintf directly, bypassing the project's string-utils convention) without more time to get right. Left as-is pending a decision on which fix shape is wanted. See console.
  6. FIXED 2026-08-01. thread.c's threadHandler signaled THREAD_STATE_STOPPED before resetting threadId = 0, and threadStartRequest wasn't synchronized against that reset -- a real (if rare-window) race, not hypothetical: the test suite was already working around it by re-calling threadInit() instead of exercising true stop/start reuse. threadId is now reset inside the same mutex-locked section before the STOPPED signal/unlock; test_thread_restart now loops 20x on the same thread_t without re-initializing, which would have caught the old race. See thread.
  7. FIXED 2026-08-01. tools/color.py emitted invalid C float literals (0f, 1f, etc.) for any whole-number CSV channel -- confirmed by actually running the generator against color.csv before and after. Was dormant only because COLOR_*_3F/_4F were unreferenced anywhere in the repo; the first real use of a float color variant wouldn't have compiled. Now routes channel values through Python float() before interpolating (always stringifies with a decimal point, e.g. 0.0/1.0), and test/display/test_color.c gained two tests that reference COLOR_BLACK_3F/COLOR_WHITE_4F/ COLOR_RED_3B/COLOR_GRAY_3F directly -- giving the previously-dead generated macros real compile-time + value coverage instead of silently bit-rotting again. See display.
  8. assetModelLoaderAsync is confirmed fully dead -- model loading is genuinely synchronous end-to-end (verified by tracing the state machine, not just repeating STATUS.md's claim). See asset.
  9. No cycle guard on prefab extends chains in scenePrefabResolveAndApply -- a self-referencing or A->B->A prefab table is a stack-overflow crash from what looks like a harmless typo in hand-authored data. See scene.
  10. Untested-code hot spots worth prioritizing if test investment is on the table: util/ref.c/random.c/crypt.c/endian.c (zero tests), animation/easing.c (13 of 16 functions untested), event/, console/, system/, log/, input/, engine/, locale/, save/, and most of display/ (mesh, shader, texture, tileset, text, screen, framebuffer) have no dedicated tests at all.

Convention-only items (real, but lower stakes than the above): raw stdlib string functions (strlen/strchr/strstr/etc.) show up instead of util/string.h wrappers in network/http/ and asset/loader/locale/assetlocaleloader.c (network, asset); several display/mesh/ files use // header comments and /* */ inline comments where CLAUDE.md wants the opposite (display); animation/'s core files all use // copyright headers instead of /** */ (animation).


error

Purpose: The engine's single error-handling convention. Every fallible function returns errorret_t, built with errorOk() / errorThrow(fmt, ...) / errorChain(call) and consumed with errorIsOk()/errorIsNotOk()/errorCatch(). No errno, no raw error codes anywhere else in the engine.

Key files: error.c/h.

Review status: Reviewed 2026-08-01. Solid, and tested unusually well (test/error/test_error.c covers thread isolation and concurrent throws, not just happy path). Two issues: errorChainImpl (error.c:86-91) hand-rolls snprintf/strlen instead of util/string.h's stringFormat/stringFormatVA wrappers that errorThrowImpl uses one function above it -- an inconsistency, not just a style nit, since CLAUDE.md requires the wrappers everywhere. errorThrowWithCode (error.h:100) is dead code -- zero call sites anywhere in src//test/.


assert

Purpose: Debug-time invariant checks -- assertNotNull, assertTrue, assertFalse, assertUnreachable, assertIsMainThread. Distinct from error/: asserts are for programmer mistakes/invariants, errorret_t is for runtime-recoverable failures (see CLAUDE.md's note on validating untrusted data with errorThrow rather than asserting).

Key files: assert.c/h.

Review status: Reviewed 2026-08-01. Clean, no issues found. Impl functions + macros wrapping __FILE__/__LINE__ and a proper DUSK_ASSERTIONS_FAKED no-op path for release builds; assertNotNullImpl (assert.c:109-111) does a real "touch" dereference after the null check as a belt-and-braces addition. Test coverage exercises both success and expect_assert_failure failure paths for every variant, including assertDeprecated/assertIsMainThread.


util

Purpose: Small stand-alone helpers with no engine-state dependencies: array (dynamic array), crypt (hashing/checksum), endian (byte-order swap), math (clamp/lerp/pow2/etc.), memory (memoryAllocate/Free/ Zero/Copy/Compare -- the only allocator the rest of the engine is allowed to call directly), random, ref (refcounted handle with lock/unlock callbacks), sort (bubble sort used for one-time static list ordering), string (stringCopy/Compare/Format/etc. -- required in place of libc str* functions per CLAUDE.md).

Key files: array.c/h, crypt.c/h, endian.c/h, math.c/h, memory.c/h, random.c/h, ref.c/h, sort.c/h, string.c/h.

Review status: Reviewed 2026-08-01. Several real issues, worst was in ref.c: refUnlock (ref.c:37-39) decremented count (uint32_t) guarded only by assertTrue(ref->count >= 0, ...) -- tautologically always true on an unsigned type, so a double-unlock silently underflowed to UINT32_MAX and kept invoking onUnlock instead of tripping an assert (confirmed by reading ref.c directly). Fixed 2026-08-01 -- refUnlock now asserts count > 0 before decrementing; also fixed refInit's doc comment, which claimed a fresh ref starts at count 1 when the code has always started it at 0. Same tautological unsigned >= 0 assert remains in memory.c:98 (lower stakes -- memoryFree's own counter, not a shared lock/unlock primitive other subsystems build on -- left as-is). memory.h:11 declares static size_t MEMORY_POINTERS_IN_USE = 0; directly in the header -- confirmed present -- meaning every TU including it gets its own private copy (only "works" because the accessors all live in memory.c). random.c's randomInt (confirmed, random.c:15-16) does rand() % (max - min) with no max > min check (UB on max <= min), and srand() is never called anywhere in the repo -- unseeded, deterministic RNG. sort.c:1 has #include <stdlib.h> before the copyright header (confirmed). Test coverage: array/math/memory/sort/ string are thorough; ref.c, random.c, crypt.c, endian.c have zero tests. randomInt/randomFloat also appear unused engine-wide (dead code, not just untested).


event

Purpose: Generic pub/sub primitive -- event_t wraps a caller-owned array of eventcallback_t/user-pointer pairs; eventInit/eventSubscribe/ eventInvoke. Used for things like uifullbox_t's transition-end callback and uiloading_t's show/hide callbacks. Per STATUS.md: "Clean, small, finished", built to replace the old input system's callback pattern.

Key files: event.c/h.

Review status: Reviewed 2026-08-01. Well-formed -- correctly uses swap-with-last for O(1) unsubscribe, asserts on capacity overflow and duplicate-subscribe, no dead code (eventUnsubscribe used in 3 other files). No errorret_t needed here since these are programmer-error assertions rather than runtime-data validation, consistent with CLAUDE.md's assert-vs-error guidance. One gap: test/event/ doesn't exist -- zero unit coverage, including the capacity-exceeded and double-subscribe assert paths.


time

Purpose: Frame delta/elapsed time (TIME.delta, exposed to JS via moduleTime) plus timeepoch_t wall-clock timestamps (used by uifps.c for the FPS counter).

Key files: time.c/h, timeepoch.c/h.

Review status: Reviewed 2026-08-01. time.c:52-56 unconditionally formats an epoch string into a 256-byte stack buffer every single timeUpdate() call purely to feed a commented-out consolePrint -- burns the leap-year/day-counting loop three times a frame for a discarded value. timeepoch.c's day-counting loops have no handling/guard for negative epoch/timezone values (pre-1970). Test coverage is solid for time.c (mocked ticks, boundary/backwards-time cases) but timeepoch.c's date math (leap year, rollover, format specifiers) has no dedicated tests despite being the more bug-prone half. The PSP-specific timezone unit bug flagged in the cross-cutting findings (#4) was confirmed and fixed 2026-08-01 -- timeGetRealTimeZonePSP was returning hours where timeepoch.c expects seconds.


thread

Purpose: Thin cross-platform thread/mutex/thread-local wrappers. Used by the asset system's background loading and the POSIX console's input-polling thread.

Key files: thread.c/h, threadmutex.c/h, threadlocal.h.

Review status: Reviewed 2026-08-01. Conventions followed throughout. Real race condition, fixed 2026-08-01: threadHandler used to unlock stateMutex and signal THREAD_STATE_STOPPED before resetting thread->threadId = 0, but threadStartRequest asserts threadId == 0 with no synchronization against that reset -- threadStop() immediately followed by threadStart() on the same struct could intermittently trip the assert. Telling sign it was real, not theoretical: test/thread/test_thread.c already re-called threadInit() before restarting specifically "so threadId/state are reset" -- the test suite was working around the bug rather than exercising true reuse. threadId is now reset inside the same mutex-locked section, before the signal/unlock; the test now loops stop/start 20x on the same thread_t with no threadInit() in between. Otherwise test coverage is good (real pthreads, try-lock coordination, a 4-thread x 10k-iteration mutex contention test).


log

Purpose: logDebug/logError -- the engine's only print/debug-output path. Header-only declarations; each platform provides the actual implementation (see src/dusk<platform>/log/).

Key files: log.h (declarations only, no .c in core).

Review status: Reviewed 2026-08-01. Clean 2-function API; all three platform implementations (dusklinux, duskpsp, duskdolphin) match the declared signatures with correct va_copy usage for the dual stdout+file writes. Dolphin's logError correctly falls back to raw VIDEO/console init when the framebuffer isn't ready yet and blocks on a START-button dismiss loop; PSP writes to ms0:/PSP/GAME/Dusk/*.log. No bugs found. No test/log/ exists -- understandable for I/O/side-effect code, but zero coverage.


system

Purpose: Very early engine init (systemInit), the systemplatform_t enum (via the systemplatformlist.h X-macro pattern), and native OS dialog boxes (systemdialogtype_t: render-blocking vs tick-blocking).

Key files: system.c/h, systemplatformlist.h.

Review status: Reviewed 2026-08-01. X-macro platform enum and compile-time #error guards for missing platform macros used correctly; all three platforms define the required systemInitPlatform/systemGetActiveDialogTypePlatform/ systemGetCyclesPlatform. Dead code confirmed: systemPSPGetLanguage() (duskpsp/system/systempsp.c:51) and systemGetLanguageDolphin() (duskdolphin/system/systemdolphin.c:40) are defined but never called anywhere -- likely intended for locale wiring that hasn't landed. No test/system/ exists.


console

Purpose: In-engine dev console: fixed-size scrollback (CONSOLE_HISTORY_MAX lines x CONSOLE_LINE_MAX chars), consolePrint, a dirty flag consumers (namely ui/debug/uiconsole.c) watch to know when to rebuild their own cached representation. POSIX builds also poll stdin on a background thread for interactive script execution.

Key files: console.c/h, consoledefs.h.

Review status: Reviewed 2026-08-01. Two real issues. consolePrint (console.c:32-44) gets its length from stringFormatVA(buffer, CONSOLE_LINE_MAX, ...), and stringFormatVA (util/string.c:100) asserts ret < destSize rather than truncating -- so any single debug line over 511 bytes aborts the process instead of just getting cut off, a harsh failure mode for a dev console. Separately, consoleUpdate's doc ("Processes pending queued script lines") and CONSOLE_EXEC_BUFFER_MAX (consoledefs.h:12) describe a queued-script- execution feature that doesn't exist -- console_t has no exec-queue fields, consoleUpdate only toggles visible, and CONSOLE_EXEC_BUFFER_MAX is unreferenced elsewhere (dead constant, stale doc). No test/console/ exists.


asset

Purpose: The asset pipeline: dusk.dsk zip archive reading (assetfile.c/h), lock/ref-counted entries (asset/loader/assetentry.c/h, built on util/ref.h), batch loading (assetbatch.c/h), and the loader registry (asset/loader/assetloader.c/h, plus one subfolder per loader type: dmf/ mesh+model, display/ texture+tileset, locale/, json/, animation/). See CLAUDE.md's "Adding a new asset loader type" for the extension pattern. Per STATUS.md: model loading's "async" path is actually synchronous -- the one known stub in core.

Key files: asset.c/h, assetfile.c/h, assetbatch.c/h, loader/assetentry.c/h, loader/assetloader.c/h.

Review status: Reviewed 2026-08-01. Core is well-structured and the multi-pass reaper correctly handles parent/child dependency chains. Found: assetFileInit (assetfile.c:29) has if(!zip_stat(...) == 0) -- precedence makes this (!zip_stat(...)) == 0, which happens to evaluate correctly for zip's 0/nonzero convention but reads as broken and should just be zip_stat(...) != 0. More seriously, assetEntryLock/Unlock sit on top of util/ref.c's confirmed double-unlock underflow bug (see util above) -- an over-unlocked asset entry silently becomes un-reapable forever instead of asserting. Confirmed via code-trace (not just citing STATUS.md): assetModelLoaderAsync (assetmodelloader.c:19-23) is genuinely dead -- assetEntryStartLoading unconditionally sets ASSET_ENTRY_STATE_PENDING_SYNC, and assetModelLoaderSync's state machine never transitions to PENDING_ASYNC the way mesh/texture/ tileset/locale/json/animation all do, so model loading runs fully synchronously via assetRequireLoaded's busy-loop; grep confirms assetModelLoaderAsync has no callers. assetlocaleloader.c uses raw stdlib (strstr/strncmp/strchr/strtol/atoi) instead of util/string.h wrappers throughout, and tests line-prefixes via memoryCompare(lineBuffer, "msgid", 5) without confirming the reused stack buffer actually holds 5 valid bytes at that point -- a short line could spuriously prefix-match against stale buffer contents. Test coverage: good breadth for entry lifecycle, tileset/JSON/animation loaders, and locale parsing; no dedicated tests for mesh/model/texture loader internals or assetfile.c's line reader.


locale

Purpose: Active-locale tracking (LOCALE.locale/LOCALE.entry) over .po-derived locale assets loaded through the asset system. See assets/locale/en_US.po -- per memory, currently placeholder/test content, not real shipped game text.

Key files: localemanager.c/h, localeinfo.h.

Review status: Reviewed 2026-08-01. Clean, no bugs found -- correctly balances assetEntryLock/Unlock in both SetLocale and Dispose (unlocks the old entry before adopting the new one). Minor nit: localeinfo.h declares static const localeinfo_t globals (LOCALE_EN_US etc.) directly in the header, so every including TU gets its own duplicate copy -- harmless given how few TUs include it. No test/locale/ directory; test/asset/test_assetlocale.c tests the loader, not localemanager.c itself.


save

Purpose: Slot-based save data (SAVE_FILE_COUNT_MAX fixed slots), yyjson documents persisted with size + CRC32 checksum. saveLoad/ saveSave read/write JSON fresh every call; callers own the returned doc and must free it. Actual file I/O goes through platform stream hooks (saveplatform_t, one impl per platform) -- core save.c must never touch the filesystem directly. savesettings.c/h layers engine settings (display/audio/input/general) on top. Per CLAUDE.md: "mature, but undocumented" beyond the one paragraph there -- no tests.

Key files: save.c/h, savefile.h, savestream.c/h, savesettings.c/h.

Review status: Reviewed 2026-08-01. Clean and convention-compliant -- save.c genuinely never touches the filesystem directly (every I/O path goes through saveXxxPlatform macros, all three platforms define them), yyjson doc ownership is honored at every call site checked, and CRC32 is validated before parsing with every error path freeing buf. One real gap: saveFileLoad's memoryAllocate(file->size) (savestream.c:122) allocates straight from an on-disk size field with no sanity bound before the allocation -- a corrupted/tampered save could request an oversized allocation, worth a max-size guard given the "never trust incoming data" principle (arguably applies to save files, not just network packets). Confirmed no test/save/ exists. saveDelete/ saveExists/saveGet are public API not yet called anywhere in the repo -- likely just unconsumed so far, not truly dead.


network

Purpose: Connection-state layer only (networkstate_t: DISCONNECTED/CONNECTING/CONNECTED/DISCONNECTING) plus a one-off HTTP client (network/http/: request/header/URL/thread/process split across files). Not a multiplayer/replication protocol yet -- that's ROADMAP.md items 8-17. Platform-specific connection polling (PSP's sceNetApctl, GameCube/Wii's if_config()) lives under src/dusk<platform>/network/. Has HTTP tests only (test/network/http/).

Key files: network.c/h, networkinfo.c/h, http/networkhttp.c/h, http/networkhttprequest.c/h, http/networkhttpthread.c/h.

Review status: Reviewed 2026-08-01. Genuinely stays a thin connection-state layer (grepped for "packet"/"replicat"/"multiplayer"/ "udp" -- no hits, no scope creep). The HTTP client correctly validates untrusted server input with errorThrow rather than trusting it (malformed status lines, oversized headers, bad Content-Length, early-closed connections all throw) -- exactly what CLAUDE.md calls out. networkhttpthread.c's request handoff is actually correct despite request fields being written outside the mutex: the final state = PENDING write happens under the same mutex the worker acquires, giving a valid release/acquire handoff (worth a comment, not a bug). One recurring convention violation: networkhttpprocess.c/networkhttpurl.c use raw stdlib strlen/strchr/strstr/strncasecmp/strcspn instead of util/string.h wrappers in several places -- util/string.h currently has no direct substitute for strchr/strstr/strcspn, so this may be a missing-utility gap rather than pure oversight. No leaks found (request/response bodies freed exactly once via networkHttpRequestReset, redirect loop frees the intermediate body). Real test coverage exists: test/network/http/test_networkhttp.c runs six tests against a fake loopback server (GET/POST/PUT, query encoding, redirects, connection-refused), each asserting zero leaked allocations.


display

Purpose: The engine's most mature, most-optimized subsystem -- platform-agnostic rendering built on top of duskgl/duskdolphin. Subfolders: mesh/ (primitive builders: cube/sphere/capsule/plane/ triprism/quad, plus the shared meshvertex_t/mesh_t API), shader/ (currently one material type, unlit), spritebatch/ (the shared SPRITEBATCH_SPRITES_MAX-capacity quad batcher everything 2D goes through -- flushes on shader/material change), texture/ (texture + tileset + palette), text/ (font_t/FONT_DEFAULT's baked-in bitmap glyphs, textBuildSpriteCache/textDrawSpriteCache's build-once/ translate-per-frame pattern), screen/ (scan-safe area), framebuffer/. color.csv + tools/color/csv/__main__.py code-generate color.h -- never hand-edit the generated header.

Key files: display.c/h, mesh/mesh.c/h, spritebatch/spritebatch.c/h, spritebatch/spritebatchsprite.c/h, text/font.c/h, text/text.c/h, texture/texture.c/h, texture/tileset.c/h, shader/shader.c/h.

Review status: Reviewed 2026-08-01. "Most mature" oversold it slightly -- real bugs found, several confirmed by direct read:

  • texture.c:42-52 (confirmed) -- textureInit() branches on texture->format to validate the incoming data union before memoryZero(texture, ...) runs a few lines later, so it's reading whatever stale/uninitialized value was already in the caller's struct, not the format parameter actually being requested. Validation is effectively checking garbage.
  • spritebatchsprite.c:20-25 -- spriteBatchSpriteTilesetPosition's zero-width/height early return zeroes sprite.min/max but leaves uvMin/uvMax uninitialized (stack garbage); the existing zero-size test doesn't check uv fields, so this isn't caught.
  • mesh.c:43 -- meshFlush's offset assert (vertexOffset < vertCount - 1) wrongly rejects a valid offset at the very last vertex; off-by-one.
  • text.c:34-37 -- bounds assert uses tileIndex <= tileCount (should be <), currently unreachable in practice since a preceding clamp already guards it, but the assert itself is wrong.
  • framebuffer.c:23-28 -- frameBufferBind(NULL) recurses into binding the backbuffer without errorChain, discarding any failure and unconditionally returning errorOk().
  • displayInit()'s six static "SIMPLE" primitive meshes (quad/cube/sphere/plane/capsule/triprism) are never disposed in displayDispose() -- one VBO+VAO leaked per primitive on the non-legacy GL path.
  • Confirmed and fixed 2026-08-01. tools/color.py (not the tools/color/csv/__main__.py path CLAUDE.md's doc comment names -- that's a stale path, only a __pycache__/ remnant exists there; the live script is tools/color.py) emitted invalid C float literals for whole-number CSV channels (0, 1 -- every black/white/primary color), e.g. color3f(0f, 0f, 0f). Was harmless only because COLOR_*_3F/_4F were never referenced anywhere in src//test/ -- confirmed by actually running the generator before and after the fix, not just reading the source. Now routes channel values through Python float() first (always stringifies with a decimal point); test/display/test_color.c gained tests referencing COLOR_BLACK_3F/COLOR_WHITE_4F/COLOR_RED_3B/COLOR_GRAY_3F directly so the previously-dead macros now have real compile-time + value coverage.
  • Style drift: mesh.h uses // header comments instead of /** */; sphere/capsule/triprism use /* */ block comments inside function bodies where CLAUDE.md mandates //; every primitive builder's JSDoc still documents a @param color that no longer exists in the signature (leftover from before color moved out of meshvertex_t).
  • The primitive *Buffer() flexible-geometry functions (sphereBuffer, capsuleBuffer, etc.) are confirmed unused outside their own *Init() -- only the fixed default mesh instances are actually consumed (via script/module/display/modulemesh.c).

Test coverage: test/display/ only registers test_color.c and test_spritebatchsprite.c -- mesh/shader/texture/tileset/palette/text/ font/screen/framebuffer all have zero unit tests.


ui

Purpose: Flat, static, singleton-driven widget framework -- no scene graph. Top level is a compile-time X-macro registry (uielementlist.h/uielement.c/h) of {init, update, draw, dispose, order} tuples (screens: frame/confirm/settings/overlays/console/fps), spliced with duskrpg's own list. Composition below that is uimenu_t (widget/uimenu.c/h) -- a flat tagged-union array of widgets laid out in a simple row/column grid via the MENU_* authoring macros; no nesting. Widgets: uilabel/uiwidgetlabel (cached glyph sprites, the pattern every other widget's text now uses), uibutton, uicheckbox, uislider, uidropdown, uitab, uiscrolling (empty placeholder). focus/ is the gamepad/keyboard directional-navigation stack (no mouse/ pointer hit-testing exists anywhere). frame/ is the 9-slice box plus the confirm dialog and settings screens. overlay/ is fullscreen fades/loading/crop bars. debug/ is the FPS counter and dev console renderer.

As of 2026-07-31: slider/tab/frame (uiFrameDrawCached)/duskrpg's textbox all cache their sprite geometry instead of rebuilding it every frame; console uses a fixed-size vertex buffer instead of alloc/free. Still open: sprite-batch flushes are keyed on material/color, so per-widget highlight-color changes still force a flush per widget (see ROADMAP.md item 5 in the debt backlog). No JS bindings exist for UI yet. test/ui//test/display/test_spritebatchsprite.c cover the new caching logic only -- nothing exercises real uiXxxDraw() calls, since those need a live GL context (FONT_DEFAULT/UI_FRAME textures) this repo's test binaries don't have.

Key files: ui.c/h, uielement.c/h, focus/uifocus.c/h, widget/uimenu.c/h, widget/uilabel.c/h, frame/uiframe.c/h, debug/uiconsole.c/h.

Review status: Reviewed 2026-07-31 (this session's sprite-caching pass) -- see notes above; per-material flush granularity and hit-testing still open.


entity

Purpose: Fixed-size-array ECS. entitymanager_t owns ENTITY_COUNT_MAX entities, each with up to ENTITY_COMPONENT_COUNT_MAX components stored in a tagged union (componentdata_t -- flagged in PSP_OPTIMIZATION_PLAN.md as the biggest PSP memory-waste candidate, since every entity reserves space for every component type's largest variant). componentlist.h's X() macro auto-generates the component enum/union/dispatch table from entity/component/**; engine-level entityprefablist.h is an empty sentinel (real prefabs live in duskrpg/entity/entityprefablist.h, resolved via entityPrefabResolveAndApply). No JSON serialize/deserialize (removed by design in favor of C-coded prefabs or JerryScript Entity/Component).

Key files: entitymanager.c/h, entity.c/h, component.c/h, componentlist.h, entityprefab.c/h.

Review status: Reviewed 2026-08-01. Structurally sound (sentinel prefab list, X-macro shape both match CLAUDE.md), but there's a real security-relevant gap: every capacity/bounds guard (entitymanager.c:35, entity.c:56, component.c:37-39,58-62,75-76, 140-141) is an assert* call, and assert.h compiles these to ((void)0) under DUSK_ASSERTIONS_FAKED (auto-set whenever NDEBUG is defined -- confirmed by reading assert.h directly, this is a real release-mode no-op, not a hypothetical). moduleentity.c:30,67 create entities/components from JerryScript with no return-value check, so a script that exceeds ENTITY_COUNT_MAX/the per-entity component cap silently gets back an invalid ID in release builds, which then indexes fixed arrays out-of-bounds later. This directly contradicts the project's own "untrusted-data limits need errorThrow, not asserts" guidance -- and script input is about as untrusted as it gets. Minor: component.h:100/component.c:52 misplace the * in void * componentGetData(. Dead code: entityUpdateRemove/ entityDisposeRemove/entityDisposeAdd have zero call sites.


scene

Purpose: Same shape as entity/ one level up -- SCENE_COUNT_MAX fixed-count scenes, each an isolated entity-manager pool; engine-level sceneprefablist.h is an empty sentinel (real prefabs in duskrpg/scene/). As of the Scene.set() work (2026-07-31), JerryScript can install a {init, update, dispose} module as the active scene via modulescene.c's moduleSceneSetStatic/moduleSceneUpdateCurrent/ moduleSceneTeardownCurrent (see script below) -- scene IDs are reused from a small pool immediately after sceneDestroy, so don't assume a freshly created scene has a "new" ID.

Key files: scene.c/h, scenebase.h, sceneprefab.c/h.

Review status: Reviewed 2026-08-01. Shares entity/'s assert-not- errorThrow bounds gap (sceneCreate hits assertUnreachable then falls through to SCENE_ID_INVALID on pool exhaustion in release -- reachable from new Scene() in JS). Additional finding: scenePrefabResolveAndApply/scenePrefabInit (sceneprefab.c:31-54) recurse through a prefab's extends chain with no cycle guard -- a self-referencing or A→B→A prefab table causes unbounded recursion/ stack overflow from what's otherwise a one-typo mistake in a hand-authored prefab list. No dead code, no use-after-free. Tests cover create/destroy/isolation/update cadence well but have zero coverage for pool exhaustion, sceneRender(), or sceneprefab.c (no prefab test file exists at all).


physics

Purpose: physicsworld_t step simulation over physicsbodytype_t (static/dynamic/kinematic) and physicshapetype_t (cube/sphere/capsule/ plane/custom, plus mesh shapes via physicsshapemesh.c/h). triggersystem.c/h layers trigger-volume enter/exit events on top. Per STATUS.md: "mature, recently churned" -- a revert/re-disable of "old ent code" suggests component wiring isn't fully settled yet. Well tested (test/physics/).

Key files: physicsworld.c/h, physicsbodytype.h, physicsshape.h, physicsshapemesh.c/h, triggersystem.c/h.

Review status: Reviewed 2026-08-01. STATUS.md's "component wiring isn't fully settled" caveat looks stale on closer look: physicsWorldStep/triggerSystemStep are live-wired in scene.c:89-90 and duskrpg's player prefab actively uses both PHYSICS and TRIGGER -- no #if 0/TODO/commented-out code found; the disable-then-revert in history reads like a same-day whole-ECS revert, not something physics-specific. Two real bugs found instead: physicsbodytype.h:23-27 documents kinematic bodies as capable of being a player controller, but physicsworld.c:124,166-167 only ever writes onGround for the dynamic side of a collision pair -- a kinematic controller's onGround is permanently false (latent today, since the one existing player prefab uses DYNAMIC, not KINEMATIC). physicstest.c:213's barycentric interior-test branch has no guard against a zero-area triangle (divide-by-zero/NaN risk in mesh-sphere collision). Dead code: physicsShapeMeshCreate (mesh-terrain collision) has zero production callers despite being well-tested in isolation; entityPhysicsGetShape/GetCollideMask are unwired to the JS module. Test depth is genuinely strong for what's implemented (gravity convergence, dynamic-vs-dynamic separation, mask filtering, mesh sphere/capsule resting) -- "well tested" is earned for the features that exist, though the unused mesh-collision path and missing trigger→JS bridge mean the subsystem's integration maturity is a bit narrower than its test depth suggests.


animation

Purpose: Keyframe + easing only -- no blend trees or state machines yet. keyframe_t/keyframeset_t interpolate values over time via easing.c/h's easing functions; animation.c/h ties a keyframe set to playback state. Per STATUS.md: terse commit history ("ANIM") suggests still iterating.

Key files: animation.c/h, easing.c/h, keyframe.c/h, keyframeset.c/h.

Review status: Reviewed 2026-08-01. All 8 core files use // copyright headers instead of the required /** */ block, inconsistent with the correctly-formatted entityanimation.{c,h} next door. Math checks out: all 16 easing functions satisfy f(0)=0/f(1)=1 with no precision issues, and keyframe interpolation clamps (doesn't extrapolate) outside the time range with no reachable divide-by-zero -- though it doesn't validate that keyframe times are ascending. 15 of 16 easing.h declarations lack Javadoc. Biggest gap: no test_easing.c exists -- only linear/in-quad get incidental coverage via test_keyframe.c, so 13 of 16 easing functions are completely untested. Dead code (test-only, unused by any duskrpg game code): keyframeSetGetValues, keyframeSetFireCallback, animationSetEvent, entityAnimationSetKeyframes/GetValue.


script

Purpose: Embedded JerryScript integration. scriptmanager.c/h owns the JerryScript context lifecycle and the exec/call helpers (scriptManagerExec/ExecFile/CallGlobal/CallValue, the last two sharing exception + promise-draining logic). scriptproto.c/h is the scriptproto_t prototype-wrapping helper every module builds on. module/ holds each registered JS-facing module:

  • modulebase.h -- the shared macro toolkit (moduleBaseFunction, moduleBaseRequireArgs/RequireString/etc., moduleBaseDefineProp/ Func/StaticFunc, moduleBaseSetWrappedPointer, moduleBaseThrowError).
  • moduleplatform.h, entity/moduleentity.c/h + entity/component/* (generic Component plus typed Position/Physics/ Renderable wrappers via modulecomponentlist.c/h), scene/modulescene.c/h (Scene, including Scene.set()), display/modulemesh.c/h, time/moduletime.c/h, require/modulerequire.c/h (CommonJS-style require()/module.exports, with a directory stack for relative resolution -- scriptManagerExecFile now pushes its own file's directory before exec so a top-level entry script's relative require() calls resolve correctly).
  • modulelist.c/h -- registers/disposes every module above in order; moduleListDispose() is now actually called from scriptManagerDispose() (was a latent bug -- it wasn't, until 2026-07-31).

Entry point convention: assets/scripts/init.js does require('./scenefile.js') + Scene.set(module). Has unit tests (test/script/), including one that reads the real shipped overworldscene.js off disk rather than an embedded copy.

Key files: scriptmanager.c/h, scriptproto.c/h, module/modulebase.h, module/modulelist.c/h, module/require/modulerequire.c/h, module/scene/modulescene.c/h.

Review status: Reviewed 2026-07-31 (this session's Scene.set() + require()-dir-stack work) -- see notes above.


input

Purpose: Bind-based input (inputaction.c/h maps INPUT_BIND_* names to raw inputbutton_ts per platform), with a configurable analog deadzone. Platform button/axis backends live under src/dusk<platform>/input/; capability macros (DUSK_INPUT_GAMEPAD/ KEYBOARD/POINTER) gate what's available per target. Pointer axis values are plumbed through (DUSK_INPUT_POINTER), but nothing in ui/ consumes them yet -- no hit-testing exists (see ui above).

Key files: input.c/h, inputaction.c/h, inputbutton.c/h.

Review status: Reviewed 2026-08-01. Clean and convention-compliant; deadzone math (inputDeadzone, input.c:244) is correct and safe. One naming nit: inputBind's param is named act in input.h:216 while its own doc comment calls it action. Confirmed, not just cited from STATUS.md: pointer input is genuinely wired end-to-end, not a dead-end at the platform layer -- SDL2's inputUpdateSDL2 populates INPUT.platform.mouseX/Y/scroll from SDL_GetMouseState, inputButtonGetValueSDL2 handles the pointer-axis cases, and duskrpg/input/inputbindmap.h binds mouse_x/mouse_y to INPUT_BIND_POINTERX/Y. The dead end is specifically at the consumption layer: grepping src/dusk/ui/ for those binds or any pointer-axis read turns up nothing -- exactly as STATUS.md says, just now confirmed rather than assumed. No tests exist for input/.


engine

Purpose: Top-level init/update/dispose orchestration (engineInit/engineUpdate/engineDispose) -- the thing main.c calls, which in turn calls into every subsystem above plus game/gameInit etc. ENGINE.running/ENGINE.version are the only bits of global state outside the per-subsystem structs.

Key files: engine.c/h.

Review status: Reviewed 2026-08-01, confirmed by direct read of engineInit/engineDispose. Init order is safe (time -> console -> system -> input -> asset -> scriptManager -> save/saveSettings -> locale -> display -> ui -> network -> scene -> game), with display before UI and scriptManager before scene/game so JS Entity/Scene wrappers exist before any script runs. engineDispose is not a clean reverse of that order, though: localeManagerDispose() runs before uiDispose()/displayDispose(), even though locale was initialized before both -- correct reverse order would tear down UI/display first, and as written there's a real risk of UI/display teardown code touching already-freed locale data (e.g. any localized string still held/logged during dispose). Separately, void engineExit(void) (engine.c:95) is defined but not declared in engine.h, and a repo-wide grep finds no caller anywhere -- dead, unreachable, and in violation of the "every public function must be declared in its header" rule. No tests exist for engine/.


game

Purpose: Intentionally header-only in core -- just declares gameInit/gameUpdate/gameDispose. The real implementation is src/duskrpg/game/game.c (currently: runs scripts/init.js on init).

Key files: game.h (no .c in core).

Review status: Reviewed 2026-08-01. Confirmed: no .c file exists in src/dusk/game/ (only game.h + CMakeLists.txt) -- a pure interface, as intended. The real src/duskrpg/game/game.c implementation is a thin JS-bootstrap shim: gameInit just does errorChain(scriptManagerExecFile("scripts/init.js", NULL)), while gameUpdate/gameDispose are both empty (errorOk() only). Not a bug -- consistent with the ongoing migration of game logic into JS -- but worth flagging as "not yet fleshed out" rather than broken if anything is expected to happen there per-frame at the C layer. No tests exist for this layer.