|
|
|
@@ -0,0 +1,858 @@
|
|
|
|
|
# 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
|
|
|
|
|
|
|
|
|
|
- [error](#error)
|
|
|
|
|
- [assert](#assert)
|
|
|
|
|
- [util](#util)
|
|
|
|
|
- [event](#event)
|
|
|
|
|
- [time](#time)
|
|
|
|
|
- [thread](#thread)
|
|
|
|
|
- [log](#log)
|
|
|
|
|
- [system](#system)
|
|
|
|
|
- [console](#console)
|
|
|
|
|
- [asset](#asset)
|
|
|
|
|
- [locale](#locale)
|
|
|
|
|
- [save](#save)
|
|
|
|
|
- [network](#network)
|
|
|
|
|
- [display](#display)
|
|
|
|
|
- [ui](#ui)
|
|
|
|
|
- [entity](#entity)
|
|
|
|
|
- [scene](#scene)
|
|
|
|
|
- [physics](#physics)
|
|
|
|
|
- [animation](#animation)
|
|
|
|
|
- [script](#script)
|
|
|
|
|
- [input](#input)
|
|
|
|
|
- [engine](#engine)
|
|
|
|
|
- [game](#game)
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
## 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](#entity) (`entitymanager.c`,
|
|
|
|
|
`component.c` -- reachable from JerryScript via `moduleentity.c`
|
|
|
|
|
with no return-value check) and [scene](#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](#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](#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](#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](#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](#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](#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](#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](#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](#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](#network),
|
|
|
|
|
[asset](#asset)); several `display/mesh/` files use `//` header
|
|
|
|
|
comments and `/* */` inline comments where CLAUDE.md wants the
|
|
|
|
|
opposite ([display](#display)); `animation/`'s core files all use `//`
|
|
|
|
|
copyright headers instead of `/** */` ([animation](#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_t`s 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.
|
YourWishes