Merge pull request 'Move outstanding work from TODO.md into the issue tracker' (#27) from todo-to-issues into main
Some checks failed
akbasic CI Build / cmake_build (push) Failing after 3m16s
akbasic CI Build / sanitizers (push) Failing after 4m29s
akbasic CI Build / akgl_build (push) Failing after 0s
akbasic CI Build / mutation_test (push) Failing after 0s
akbasic CI Build / coverage (push) Failing after 4m9s
Some checks failed
akbasic CI Build / cmake_build (push) Failing after 3m16s
akbasic CI Build / sanitizers (push) Failing after 4m29s
akbasic CI Build / akgl_build (push) Failing after 0s
akbasic CI Build / mutation_test (push) Failing after 0s
akbasic CI Build / coverage (push) Failing after 4m9s
Reviewed-on: #27
This commit is contained in:
26
CLAUDE.md
26
CLAUDE.md
@@ -23,7 +23,8 @@ scripting engine for game authors.
|
||||
| Read | For |
|
||||
|---|---|
|
||||
| [`MAINTENANCE.md`](MAINTENANCE.md) | Everything above. Start here |
|
||||
| [`TODO.md`](TODO.md) | Outstanding defects, with file, line and consequence. §0.1 first — it retires the byte-for-byte fidelity constraint several later sections were written on |
|
||||
| [the issue tracker](https://source.starfort.tech/andrew/akbasic/issues) | **Outstanding defects and gaps.** Labelled by kind and blast radius; `status::grooming` means the scope is not settled yet |
|
||||
| [`TODO.md`](TODO.md) | The record: settled design decisions, the deviation register, defects already fixed, and the reasoning behind the measurements. §0.1 first — it retires the byte-for-byte fidelity constraint several later sections were written on |
|
||||
| [`README.md`](README.md) | What the project is and why, for somebody who has not seen it |
|
||||
| [`docs/`](docs/README.md) | The language itself: eighteen chapters, verb and function reference. [Chapter 14](docs/14-architecture.md) is the interpreter's architecture — the step loop, the pools, the two kinds of error, and how to debug it. [Chapter 15](docs/15-error-codes.md) is the error-code appendix. [Chapters 17](docs/17-tutorial-breakout.md) and [18](docs/18-tutorial-breakout-artwork.md) are tutorials that build the games in `examples/breakout/` |
|
||||
| `deps/libakerror/AGENTS.md` | The `ATTEMPT`/`CLEANUP`/`PROCESS`/`HANDLE`/`FINISH` protocol, authoritatively |
|
||||
@@ -44,19 +45,26 @@ repeating where you will see them:
|
||||
|
||||
- **Add tests in the same commit as the behaviour change**, and assert the *correct* contract
|
||||
even where the code is currently wrong. A known-failing test goes in
|
||||
`AKBASIC_KNOWN_FAILING_TESTS` with a `TODO.md` entry; it does not get pinned to the buggy
|
||||
`AKBASIC_KNOWN_FAILING_TESTS` with an open issue; it does not get pinned to the buggy
|
||||
behaviour, because that turns the eventual fix into a test failure.
|
||||
- **Never work around a missing dependency capability here.** File it in that repository's
|
||||
`TODO.md` — what the BASIC verb requires, what the entry point should look like, what tests
|
||||
would cover it. `MAINTENANCE.md` explains why, and names the four gaps this closed upstream.
|
||||
- **Never work around a missing dependency capability here.** Open an issue in that
|
||||
repository's tracker on <https://source.starfort.tech> — what the BASIC verb requires, what
|
||||
the entry point should look like, what tests would cover it. `MAINTENANCE.md` explains why,
|
||||
and names the four gaps this closed upstream. **Recording it here instead is not filing it**:
|
||||
two `akgl_ui` gaps sat in `TODO.md` for a release because changing a submodule is that
|
||||
repository's decision — which is true of changing it and not of reporting it.
|
||||
- **Never edit generated output** — `build/` trees, the generated `akerror.h`, `akgl.pc`,
|
||||
`include/akgl/SDL_GameControllerDB.h`. Change the template or the generator script.
|
||||
- **Do not reformat code you are not otherwise changing.** Several files mix tabs and spaces
|
||||
and there is no repo-wide formatter; style conversions get their own commit.
|
||||
- **Do not edit `tests/reference/`.** Those expectations came from the Go implementation and
|
||||
are never edited to suit this interpreter. A deliberate divergence goes in `TODO.md` and
|
||||
`docs/13-differences.md`.
|
||||
- **Update `TODO.md` when you learn something about a defect**, including that it is worse or
|
||||
better than recorded. Publishing a problem you cannot fix yet is a contribution.
|
||||
are never edited to suit this interpreter. A deliberate divergence goes in
|
||||
`tests/reference/README.md`'s divergence table and `docs/13-differences.md`.
|
||||
- **Open an issue for outstanding work; do not add it to `TODO.md`.**
|
||||
<https://source.starfort.tech/andrew/akbasic/issues>, or `tea issues create --repo
|
||||
andrew/akbasic`. Name the file and line, the functional consequence, and what closing it would
|
||||
touch. Publishing a problem you cannot fix yet is a contribution — and **when you learn
|
||||
something about an open issue, say so in the issue**, including that it is worse than
|
||||
recorded, already fixed, or resting on a premise that has expired.
|
||||
- **Add yourself — program, model and version — as a commit co-author.** `libakgl`'s
|
||||
`AGENTS.md` requires it and this repository follows the same rule.
|
||||
|
||||
@@ -38,8 +38,8 @@ the idiom of the `ak*` C libraries it builds on.
|
||||
|
||||
2. **Finish the language.** The full Dartmouth BASIC and Commodore 128 BASIC 7.0 verb and
|
||||
function set. `deps/basicinterpret/README.md` ends with the original's list of what was
|
||||
unimplemented, and that list was the work queue. What remains is in `TODO.md` and is
|
||||
summarised for a BASIC programmer in `docs/13-differences.md`. A few entries are
|
||||
unimplemented, and that list was the work queue. Every group of it is done; what remains is
|
||||
in the issue tracker and is summarised for a BASIC programmer in `docs/13-differences.md`. A few entries are
|
||||
deliberately out of scope on a modern PC — `BANK`, `FAST`, `MONITOR`, and `SPRDEF`, which
|
||||
is an interactive editor rather than a programmable verb. Keep that reasoning rather than
|
||||
reviving them.
|
||||
@@ -60,9 +60,12 @@ the idiom of the `ak*` C libraries it builds on.
|
||||
### Missing capabilities get filed upstream, not worked around
|
||||
|
||||
**When `libakgl` — or `libakstdlib` — cannot supply something a verb needs, do not work
|
||||
around it here.** Add a numbered item to that repository's `TODO.md` describing the missing
|
||||
API: what the BASIC verb requires, what the `akgl_*` or `aksl_*` entry point should look
|
||||
like, and what tests would cover it. Follow the prose-paragraph style of the entries already
|
||||
around it here.** Open an issue in that repository's tracker on
|
||||
<https://source.starfort.tech> describing the missing API: what the BASIC verb requires, what
|
||||
the `akgl_*` or `aksl_*` entry point should look like, and what tests would cover it.
|
||||
**Recording it in this repository instead is not filing it** — two `akgl_ui` gaps sat in
|
||||
`TODO.md` for a release on the reasoning that changing a submodule is that repository's
|
||||
decision, which is true of *changing* it and not of *reporting* it. Follow the prose-paragraph style of the entries already
|
||||
there. Growing the dependency to serve the interpreter is a wanted outcome, not a detour.
|
||||
|
||||
It works. Four gaps were filed this way — text measurement, immediate-mode drawing, audio,
|
||||
@@ -201,7 +204,7 @@ compile-time test `libakstdlib` could not write against `libakerror` — and `ak
|
||||
floor is enforced instead by an `#error` feature-testing `AKERR_FIRST_CONSUMER_STATUS`, which
|
||||
`akstdlib.h`, `akgl/error.h` and our own `include/akbasic/error.h` all carry — include any of
|
||||
them and you inherit the guard. The missing version file is filed in
|
||||
`deps/libakstdlib/TODO.md` §2.3; when it lands, add the `1.0` floor to the `find_dependency`
|
||||
`libakstdlib` issue #5 and `libakerror` issue #16; when it lands, add the `1.0` floor to the `find_dependency`
|
||||
calls.
|
||||
|
||||
### Embedding all three dependencies collides four ways
|
||||
@@ -253,7 +256,7 @@ target with the same name already exists"*. We shadow `add_custom_target` and re
|
||||
to `akerror_coverage` on the way past. `libakstdlib` (both targets) and `libakgl` (its
|
||||
`mutation` target) namespace themselves correctly. **The real fix is upstream in
|
||||
`libakerror`** — the same `CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR` test it already
|
||||
applies to `mutation` — and it is filed in `deps/libakstdlib/TODO.md` §2.3. Delete the
|
||||
applies to `mutation` — and it is filed as `libakerror` issue #15. Delete the
|
||||
workaround when it lands.
|
||||
|
||||
**4. Stale build trees poison the coverage report.** See below; it is the reason for
|
||||
@@ -324,7 +327,7 @@ parameters but not all of them fails the run. Every public declaration under
|
||||
|
||||
`CMakeLists.txt` declares `AKBASIC_TESTS`, `AKBASIC_WILL_FAIL_TESTS` and
|
||||
`AKBASIC_KNOWN_FAILING_TESTS`. The first must exit 0. The second aborts by design. The
|
||||
third **asserts the correct contract for a defect that is documented in `TODO.md`** and is
|
||||
third **asserts the correct contract for a defect that has an open issue** and is
|
||||
expected to fail.
|
||||
|
||||
A green `ctest` therefore does not mean defect-free. When a known-failing test starts
|
||||
@@ -336,7 +339,7 @@ there when it is next needed.
|
||||
|
||||
The dependencies use the same split under their own prefixes: `AKSL_TESTS` /
|
||||
`AKSL_WILL_FAIL_TESTS` / `AKSL_KNOWN_FAILING_TESTS`, and `AKERR_TESTS` /
|
||||
`AKERR_WILL_FAIL_TESTS`. `libakstdlib` 0.2.0 fixed all six defects its `TODO.md` §2.1 listed
|
||||
`AKERR_WILL_FAIL_TESTS`. `libakstdlib` 0.2.0 fixed all six defects it had confirmed
|
||||
and left `AKSL_KNOWN_FAILING_TESTS` empty.
|
||||
|
||||
### Test target names
|
||||
@@ -351,7 +354,8 @@ name. That is not cosmetic: `add_executable` creates a dependency's targets even
|
||||
`tests/reference/` is the Go implementation's own acceptance suite, byte-compared.
|
||||
**Nothing in it is ever edited to suit this interpreter.** If a case fails, either this
|
||||
interpreter is wrong or the divergence is deliberate — and a deliberate one goes in
|
||||
`TODO.md` and `docs/13-differences.md`, not into the expectation file. `tests/reference/README.md`
|
||||
`tests/reference/README.md`'s divergence table and `docs/13-differences.md`, not into the
|
||||
expectation file. `tests/reference/README.md`
|
||||
says the same thing at more length.
|
||||
|
||||
`tests/language/` is ours and may be changed freely. A new language feature needs a
|
||||
@@ -463,7 +467,8 @@ is there so the question "between them *where*" has an answer.
|
||||
Breakout reaches the high end of that bracket for a reason worth knowing: two of its eight
|
||||
sprites *are the screen*, a captured HUD strip and a captured play field, so the field's box
|
||||
covers everything and the bounding-box reject can never throw those pairs out. That is
|
||||
`TODO.md` §9 item 9, and fixing it would take this row down as a side effect.
|
||||
the sprite-slot cost recorded in `TODO.md` §9 item 9, and fixing it — issue #23 — would take
|
||||
this row down as a side effect.
|
||||
|
||||
**Read a benchmark as a gap between two rows of the same run, not as an absolute.** libakgl's
|
||||
`PERFORMANCE.md` records a whole laptop reading 15% high on a later run, including rows nothing
|
||||
@@ -553,7 +558,7 @@ stale:
|
||||
| `__AKERR_ERROR_NAMES` | `akerr_name_for_status()`; the table is private to the library now |
|
||||
| `AKERR_STATUS_RANGE_OK` / `AKERR_STATUS_NAME_OK` | success is a `NULL` `akerr_ErrorContext *`, like everything else |
|
||||
|
||||
`libakerror`'s own `TODO.md` §2 says the limit plainly: ownership enforcement covers *naming*,
|
||||
`libakerror`'s own tracker says the limit plainly (its issue #4): ownership enforcement covers *naming*,
|
||||
which is the part the library mediates. It cannot detect two components compiling the same
|
||||
integer into a `HANDLE` `case` label without ever registering a name — that never reaches the
|
||||
registry. §3 adds that there is no way to ask who owns a status or to enumerate reservations,
|
||||
|
||||
633
TODO.md
633
TODO.md
@@ -1,11 +1,24 @@
|
||||
# TODO
|
||||
# Record
|
||||
|
||||
Implementation plan for the Go → C port of `deps/basicinterpret`.
|
||||
**Outstanding work is in the issue tracker, not in this file:**
|
||||
<https://source.starfort.tech/andrew/akbasic/issues>
|
||||
|
||||
This file is written to be executed by AI agents, not read by humans for inspiration. Every
|
||||
item names the files it touches, the exact deliverable, and the command that proves it done.
|
||||
Work the phases in order. Inside a phase, items with no `Depends:` line may be done in
|
||||
parallel.
|
||||
This file was the implementation plan for the Go → C port of `deps/basicinterpret`, written to be
|
||||
executed by AI agents rather than read for inspiration. **The port is done**, so what it is now is
|
||||
the record: the design decisions that are settled, the deviations from the reference and why each
|
||||
was taken, the defects that were found and fixed, and the reasoning behind the measurements.
|
||||
|
||||
Each open item that moved leaves a line saying what it was and which issue carries it, because an
|
||||
entry explaining *why* a defect matters is worth keeping beside the work it constrains — but the
|
||||
tracking happens there, not here.
|
||||
|
||||
Issues are labelled by kind and blast radius, and milestoned by what they can land in: `0.1.x` for
|
||||
anything that changes no public contract, `0.2.0` for new verbs and observable behaviour changes,
|
||||
`1.0.0` for the design decisions. Everything filed carries `status::grooming`.
|
||||
|
||||
Two gaps this file recorded and deliberately did not file — the missing `HUD` anchors and a
|
||||
dismissable dialog — are now `libakgl` #79 and #80. §7's rule is right that *changing* a dependency
|
||||
is that repository's decision; it does not follow that reporting the gap is.
|
||||
|
||||
---
|
||||
|
||||
@@ -44,7 +57,7 @@ What it does **not** change:
|
||||
2. `deps/libakerror/AGENTS.md` — the `ATTEMPT`/`CLEANUP`/`PROCESS`/`HANDLE`/`FINISH` protocol.
|
||||
3. `deps/libakerror/UPGRADING.md` — 1.0.0's status registry. Required before writing an error
|
||||
code; the mechanism it replaced is gone.
|
||||
4. `deps/libakstdlib/TODO.md` §2.1 — six **confirmed** defects in the library this port calls
|
||||
4. `deps/libakstdlib`'s issue tracker — the defects and gaps in the library this port calls
|
||||
into. §1.9 below says which calls are cleared for use; that section is not optional
|
||||
reading, it bans a family of functions the port would otherwise reach for by reflex.
|
||||
5. `deps/libakgl/AGENTS.md` — the no-`malloc` rule and the commit co-author requirement.
|
||||
@@ -55,16 +68,17 @@ What it does **not** change:
|
||||
|
||||
- **Do not** mark an item done until its acceptance command passes on a clean out-of-tree
|
||||
build. "It compiles" is not acceptance.
|
||||
- **Do** update this file in the same commit as the work: strike the item, or replace it with
|
||||
the defect it uncovered. This file holds outstanding items only.
|
||||
- **Do** update the tracker in the same commit as the work: close the issue, or replace it with
|
||||
the defect it uncovered. Outstanding items live there; this file holds the record.
|
||||
- **Do** add the agent program name, model name and version as a commit co-author. That rule
|
||||
comes from `libakgl` and applies here.
|
||||
- **Never** hand-edit generated output. `build/` trees and the generated `akerror.h` are
|
||||
off-limits; change the generator.
|
||||
- **Never** reformat a file you are not otherwise changing.
|
||||
- When a step is blocked because `libakgl` cannot supply a capability, **do not work around
|
||||
it here.** File it in `deps/libakgl/TODO.md` in the numbered prose style of that file's
|
||||
"Carried over" section and note the block in §7 below.
|
||||
it here.** File it against `libakgl` in its issue tracker -- what the BASIC verb requires,
|
||||
what the `akgl_*` entry point should look like, and what tests would cover it -- and note the
|
||||
block in §7 below.
|
||||
|
||||
**Style**, restated so nobody has to go look: C99, 4-space indent, tabs at width 8
|
||||
(`stroustrup`), function-body braces in column 0 on their own line, control braces on the same
|
||||
@@ -140,7 +154,7 @@ One implementation, `src/symtab.c`, keyed by `aksl_strhash_djb2()` (already in
|
||||
`libakstdlib`) with linear probing and a fixed slot array. **Use the existing hash; do not
|
||||
write another one.** Table full is an error, not a resize.
|
||||
|
||||
Caveat, from `deps/libakstdlib/TODO.md` §1.6/§2.2.6: the wrapper sign-extends `char`, so a
|
||||
Caveat, recorded upstream: the wrapper sign-extends `char`, so a
|
||||
high-bit byte hashes wrong — `"\xff\xfe"` returns 5859874 where the `unsigned char` answer is
|
||||
5868578. BASIC identifiers are 7-bit ASCII (the scanner only accepts `IsLetter`/`IsDigit` plus
|
||||
a type suffix), so this cannot bite the symbol tables. It **would** bite if anyone later keys
|
||||
@@ -729,81 +743,21 @@ Also on the queue, from the reference's own defect list:
|
||||
|
||||
### What a demoscene program wanted and could not have
|
||||
|
||||
`examples/megademo/megademo.bas` was written to push the interpreter to its edges, and these
|
||||
are the edges it hit. None of them blocked the demo — every one has a workaround, and the
|
||||
program carries all six — but each workaround is a routine every future game will carry too,
|
||||
and that is the argument for the verb. Ordered by how much BASIC each one would delete.
|
||||
`examples/megademo/megademo.bas` was written to push the interpreter to its edges, and these are
|
||||
the edges it hit. **None of them blocked the demo** — every one has a workaround and the program
|
||||
carries all six — but each workaround is a routine every future game will carry too, and that is
|
||||
the argument for the verb.
|
||||
|
||||
- **`RND` is still missing.** Chapter 17 teaches the LCG workaround and the demo's `RANDOM`
|
||||
label carries it: nine tokens, two convention globals (`RMAX#` in, `RND#` out), and a seed
|
||||
the caller must remember to take from `TI#`. That is the right *teaching* example and the
|
||||
wrong thing to make every program transcribe. Closing it is the standard function shape —
|
||||
dispatch-table row, exec handler, tests, a Chapter 12 row — and Chapter 17's section becomes
|
||||
a history lesson instead of a requirement. `ASC` is in the same boat with a worse workaround
|
||||
(`INSTR` into a 41-character alphabet string); `INT` and `SQR` are not, because integer
|
||||
assignment and `^ 0.5` already are those functions.
|
||||
- **The palette cannot be written.** `PALETTE[17]` in `src/graphics_tables.c` is `static
|
||||
const`, and `COLOR` can only choose among its sixteen entries. Palette rotation was the
|
||||
cheapest animation of the whole era — one register write cycled every pixel of a colour —
|
||||
and here the same effect is a full redraw: the demo's title "colour cycle" re-strokes the
|
||||
logo every frame (`DRAWTEXT` with `CM# = 1`), and its raster bars erase and redraw five
|
||||
ramp lines apiece per frame. Closing it means palette state moves from a static table to
|
||||
the graphics backend, plus a verb form to set an index (`COLOR` grew nothing new on a C128,
|
||||
so this would be ours — Chapter 13 material), plus the docs figures that assume Pepto's
|
||||
values stay put.
|
||||
- **`GSHAPE` stamps in one mode.** BASIC 7.0's takes a mode argument — replace, OR, AND,
|
||||
XOR — and XOR is the one that mattered: stamp to draw, stamp again to erase, no damage to
|
||||
what was underneath. That is software sprites beyond the eight real ones, and it is the
|
||||
difference between the demo's raster bars chewing holes in the starfield (they erase in
|
||||
black) and leaving it intact. Needs a blend argument through `SSHAPE`/`GSHAPE`'s render
|
||||
path in `src/graphics_akgl.c`, and libakgl may need to offer the blend mode first — check
|
||||
before filing there.
|
||||
- **Nothing reports whether `PLAY` has drained.** The queue depth exists in
|
||||
`akbasic_AudioState` (`count - head`); no function reads it. The demo's `TUNE` label
|
||||
requeues each batch on a wall-clock guess — it stamps `MT#` with the batch's computed
|
||||
length plus a breath, and the sums drift against `TEMPO` and had better stay under the
|
||||
queue's 128-note cap when a requeue lands early. An `RPLAY(v)` returning notes pending —
|
||||
same shape as `RSPPOS` — deletes the arithmetic. It also fixes the quiet failure Chapter 7
|
||||
warns about, where a program `QUIT`s before its music finishes and never knows.
|
||||
- **`PLAY` is monophonic: one queue, one clock, one instrument per batch.** An earlier
|
||||
version of this entry claimed there was no noise waveform, and that was wrong —
|
||||
`ENVELOPE`'s sixth argument selects triangle, sawtooth, square or noise
|
||||
(`src/runtime_audio.c`), which Chapter 7 does not document and should. The real gap is in
|
||||
`akbasic_play_service()` (`src/play.c`): every voice's notes go into **one** queue with one
|
||||
release clock, and the envelope applied when a note sounds is whatever the last `T`
|
||||
*parsed* selected — so a program that writes three `V` tracks gets them end to end, on one
|
||||
instrument, and `M` is a no-op precisely because there is nothing to synchronise. The
|
||||
backend already has three independent voices and `tone()` takes a voice number; what is
|
||||
wanted is a queue head and clock per voice, and the envelope resolved at parse time into
|
||||
the note rather than read from shared state at release. The demo works around it the way
|
||||
1985 did on one channel: an interleaved kick–bass–arpeggio line (the `TUNE` label), which
|
||||
is authentic and should not be mandatory.
|
||||
- **`DATA` is too small a pipe for assets, and `SPRSAV`'s type-in form is 24 by 21.** The
|
||||
pool is 512 items and the demo's stroke font already holds ~350, so a full-screen picture
|
||||
can never arrive by `DATA` — and the one sprite form that reads from the program is fixed
|
||||
at the VIC-II's 24x21 monochrome. The demo's scene 5 shows both the ceiling and the way
|
||||
around it: a 160x120 sixteen-colour image rides in **string literals** instead — RLE, two
|
||||
characters a run, ~4 KB in eighteen `IM$()` assignments, decoded with `INSTR` and painted
|
||||
in 5x5 blocks (`examples/megademo/vaporwave.py` generates the block) — while the 63-byte
|
||||
bird beside it is all `DATA` can actually carry. Strings are the workaround, not the
|
||||
answer: every program that wants a picture must also carry a decoder. What is wanted is
|
||||
either a bigger `DATA` pool (it is one constant) or, better, a `SPRSAV`/`BLOAD` form that
|
||||
accepts packed image data from program text at a stated width and height.
|
||||
- **The only vsync is spinning on `TI#`, and the spin is worse than it looks.** Chapter 13
|
||||
documents the polling; the demo's first cut had the loop — read `TI#`, branch to yourself
|
||||
until it moves — and it did not just waste CPU, it **starved the `PLAY` queue dry**. The
|
||||
chain is measurable: the host calls `settime()` once per `runtime_run()` batch, and
|
||||
`akbasic_play_service()` can therefore release at most one note per batch (after a release,
|
||||
`nextms` sits ahead of the frozen clock). A batch of 256 *held* `SLEEP` steps is nearly
|
||||
free, but a batch of 256 *executed* spin statements is slow enough to drop the release rate
|
||||
below a sixteenth-note tune's fifteen notes a second — and the queue never reclaims slots
|
||||
until it drains completely, so the backlog compounded across scenes and overflowed the
|
||||
128-note queue with `PLAY queue is full`. Reproduced both ways in isolation: the same tune
|
||||
requeued eight times over a `SLEEP` loop survives, over a `TI#` spin it dies on the third
|
||||
batch. The demo's `VSYNC` label now sleeps a frame instead. A verb that parks the program
|
||||
until the next batch boundary (`SLEEP 0` is an available spelling) is the same
|
||||
hold-without-blocking shape `SLEEP` and `GETKEY` already have in `src/runtime_console.c`,
|
||||
and it would make the honest spelling also the safe one.
|
||||
Ordered by how much BASIC each one would delete: `RND` and `ASC` (#16), the unwritable palette
|
||||
(#17), `GSHAPE`'s missing blend modes (#18), no way to ask whether `PLAY` has drained (#19),
|
||||
`PLAY` being monophonic (#20), `DATA` being too small a pipe for assets (#21), and the `TI#` spin
|
||||
that starves the audio queue (#22).
|
||||
|
||||
**The last of those is the one worth reading before writing a game loop.** Polling `TI#` does not
|
||||
merely waste CPU: the host calls `settime()` once per `runtime_run()` batch, so a batch of spin
|
||||
statements drops the note release rate below what a sixteenth-note tune needs, the queue never
|
||||
reclaims slots until it drains completely, and the backlog compounds until `PLAY queue is full`.
|
||||
Reproduced both ways in isolation. Sleep a frame instead.
|
||||
|
||||
---
|
||||
|
||||
@@ -1259,7 +1213,7 @@ deviations from the reference's *program*: `main.go` and the SDL half of
|
||||
|
||||
57. **`DIRECTORY` is refused for want of an upstream wrapper.** `libakstdlib` has no
|
||||
`aksl_opendir`, and this project's rule is that a missing capability is filed upstream
|
||||
rather than worked around — so it is filed in `deps/libakstdlib/TODO.md` and the verb says
|
||||
rather than worked around — so it is filed against `libakstdlib` and the verb says
|
||||
so. Calling `opendir(3)` here would mean reporting through `errno` in a file where
|
||||
everything else reports through an `akerr_ErrorContext *`.
|
||||
|
||||
@@ -1692,28 +1646,11 @@ the reference's semantics reproduced faithfully; the third is the port's own.
|
||||
zero-iteration body uses, and the `NEXT` pops on an `exiting` flag. Neither verb has to know
|
||||
where the loop ends. `tests/for_next.c`.
|
||||
|
||||
19. **A `FOR` body runs once with the overshot counter.** The loop condition is tested against
|
||||
the counter *before* the increment, so the increment's result reaches the body before
|
||||
anything compares it to the limit. `FOR I = 1 TO 9 STEP 3` runs its body with 1, 4, 7 **and
|
||||
then 10**. Invisible whenever the step lands exactly on the limit, which is every case in
|
||||
the golden corpus and every case in the reference's own tests.
|
||||
|
||||
Related, and the same off-by-one from the other end: `FOR I = 1 TO 1` does not run its body
|
||||
at all, where every BASIC ever written runs it once. The two errors compensate for a step of
|
||||
1, which is why neither was noticed.
|
||||
|
||||
**Not fixed, because the corpus pins it.**
|
||||
`tests/reference/language/flowcontrol/forloopwaitingforcommand.bas` depends on
|
||||
`FOR I# = 1 TO 1` skipping its body, and `tests/reference/README.md` forbids editing an
|
||||
expectation to suit this interpreter. Registered as `tests/for_semantics.c` in
|
||||
`AKBASIC_KNOWN_FAILING_TESTS`, asserting the correct contract. Fixing it means deciding what
|
||||
to do with that golden file, and that is a decision rather than a patch.
|
||||
|
||||
20. **A `FOR` counter does not survive its loop.** It is created in the environment the loop
|
||||
pushes, which pops when the loop ends, so reading it afterwards finds a fresh variable
|
||||
holding zero. A C128 leaves it at the value that ended the loop and plenty of published
|
||||
listings read it there. Same known-failing test as item 19; the fix is a scoping decision
|
||||
about where a loop counter is created, not an ordering one.
|
||||
19. **A `FOR` body runs once with the overshot counter**, `FOR I = 1 TO 1` does not run its
|
||||
body at all, and the counter does not survive the loop. The first two compensate for a step
|
||||
of 1, which is why neither was noticed. Pinned by `tests/for_semantics.c` in
|
||||
`AKBASIC_KNOWN_FAILING_TESTS`; fixing it means deciding what to do with
|
||||
`forloopwaitingforcommand.bas`, which depends on the current behaviour. **Issue #5.**
|
||||
|
||||
### Found while fixing the DEF recursion hang
|
||||
|
||||
@@ -1756,31 +1693,10 @@ the reference's semantics reproduced faithfully; the third is the port's own.
|
||||
A *structure* is deliberately given no truth value -- it always exists, so the question
|
||||
has no answer worth guessing at.
|
||||
|
||||
25. **A statement containing a failed multi-line `DEF` call still completes, and prints a
|
||||
junk value.** The call's error is reported and the run stops, but the *enclosing*
|
||||
statement carries on with whatever `*dest` was left holding:
|
||||
|
||||
```
|
||||
10 DEF BAD(N#)
|
||||
20 RETURN N# / 0
|
||||
30 PRINT 99
|
||||
40 PRINT BAD(1)
|
||||
```
|
||||
|
||||
prints `99`, the division-by-zero line, and then
|
||||
`(UNDEFINED STRING REPRESENTATION FOR 0)`.
|
||||
|
||||
**Pre-existing, and measured as such**: identical before and after the re-entrancy
|
||||
fix, on a build stashed back to compare. It is visible more often now only because
|
||||
runaway recursion reaches it where it used to hang instead.
|
||||
|
||||
The cause is that `akbasic_runtime_process_line_run()` swallows a script's error by
|
||||
design -- goal 3 -- so the loop inside `akbasic_runtime_user_function()` sees the run
|
||||
end normally and hands back a value nobody should use. The fix is for the multi-line
|
||||
path to notice the run did not return through `RETURN` and refuse rather than produce
|
||||
a value; it is not done here because it is a decision about what a failed call
|
||||
*evaluates to*, and `tests/language/functions/recursion.bas` deliberately does not
|
||||
pin the current answer.
|
||||
25. **A statement containing a failed multi-line `DEF` call still completes, and prints a junk
|
||||
value.** `akbasic_runtime_process_line_run()` swallows a script's error by design, so the
|
||||
loop inside `akbasic_runtime_user_function()` sees the run end normally. It is a decision
|
||||
about what a failed call *evaluates to*. **Issue #7.**
|
||||
|
||||
### Found while building structures
|
||||
|
||||
@@ -1863,111 +1779,38 @@ the reference's semantics reproduced faithfully; the third is the port's own.
|
||||
### Found while writing the error-code appendix
|
||||
|
||||
21. **A dependency's status code reaches `ER#` where the interpreter has one of its own.**
|
||||
`VAL("XYZ")` reports `AKERR_VALUE` — 144 on this machine — rather than
|
||||
`AKBASIC_ERR_VALUE` (517), because `akbasic_fn_val()` (`src/runtime_functions.c:261`)
|
||||
hands the string to `aksl_atof()` and returns whatever comes back. Both register the
|
||||
name `Value Error`, so `ERR(ER#)` cannot tell them apart and only the number can.
|
||||
|
||||
**The consequence is that the number is not portable.** `libakerror`'s codes are offsets
|
||||
from the platform's largest `errno`, so a program testing `IF ER# = 144` is right on one
|
||||
machine and wrong on the next, where 517 — which is absolute — is what it should have
|
||||
been able to test. `docs/15-error-codes.md` documents the behaviour as it stands and
|
||||
tells a program to compare the code rather than the text.
|
||||
|
||||
Not fixed here because it is a decision about a boundary rather than a patch: every
|
||||
place the interpreter calls into `libakstdlib` on a *program's* behalf could translate
|
||||
the failure into the akbasic band, and the list of those places wants collecting before
|
||||
the first one is changed. The disk verbs are the interesting counter-case — `ENOENT` out
|
||||
of `DOPEN` is genuinely more useful to a program than a flat 519 — so this is not
|
||||
"translate everything", which is exactly why it needs deciding rather than doing.
|
||||
`VAL("XYZ")` reports `AKERR_VALUE` rather than `AKBASIC_ERR_VALUE`, and the errno-derived
|
||||
number is not portable across machines where 517 would have been. A decision about a
|
||||
boundary rather than a patch -- the disk verbs are the counter-case, since `ENOENT` out of
|
||||
`DOPEN` is more useful than a flat 519. **Issue #13.**
|
||||
|
||||
### Found while making line numbers optional
|
||||
|
||||
26. **`akbasic_environment_set_label()` files into the active scope, not the root.**
|
||||
`src/environment.c:246` walks up until `obj->runtime->environment == obj`, which is the
|
||||
*currently executing* scope, despite the comment above it saying "Only the top-level
|
||||
environment creates labels". So a `LABEL` reached inside a `GOSUB` or `FOR` body is filed
|
||||
into that scope and dies when the body pops, while `akbasic_runtime_scan_labels()`
|
||||
(`src/runtime.c:1286`) files the same label into the real root.
|
||||
26. **`akbasic_environment_set_label()` files into the active scope, not the root**, so a
|
||||
prescanned label and a re-filed one behave differently. One condition, plus a test. Wants
|
||||
checking against deviation 32 first. **Issue #9.**
|
||||
|
||||
**The consequence is that a prescanned label and a re-filed one behave differently**, and
|
||||
which one you get depends on whether control has passed through the `LABEL` statement
|
||||
inside a scope. Nothing in either corpus does that, so nothing catches it. The fix is one
|
||||
condition -- walk to `parent == NULL` -- plus a test that `GOSUB`s past a `LABEL` and then
|
||||
branches to it from the top level. Wants checking against deviation 32 first, because
|
||||
"`LABEL` still executes and re-files itself" is deliberate and the re-filing is the part
|
||||
worth keeping.
|
||||
27. **Three of the four prescans report against the wrong line**, and work around it by
|
||||
putting `Line %d:` in the message text -- so the message names the number twice and the
|
||||
prefix is wrong. `akbasic_runtime_check_targets()` shows the fix. **Issue #10.**
|
||||
|
||||
27. **Three of the four prescans report against the wrong line.**
|
||||
`akbasic_runtime_set_mode()`'s handler builds its BASIC error line from
|
||||
`environment->lineno`, which after a load is wherever the loader stopped -- usually the
|
||||
last line of the program. The label, `DATA` and `TYPE` scans therefore print `? 47 :` for
|
||||
a fault on line 3, and work around it by putting `Line %d:` in the message text
|
||||
(`src/structtype.c:342`), so the message names the number twice and the prefix is wrong.
|
||||
28. **Two lines carrying the same written line number still silently keep the last.** The one
|
||||
collision that can appear in an existing program, so it carries corpus risk the other three
|
||||
did not. **Issue #11.**
|
||||
|
||||
`akbasic_runtime_check_targets()` does not have the problem because it points
|
||||
`environment->lineno` at the line it is walking. That is the fix, and it belongs in the
|
||||
wrapper rather than in four places: give each prescan a way to say which line it failed
|
||||
on, or have each set the cursor as the target check does. Cosmetic, but it is the first
|
||||
number a person reads when a program will not start.
|
||||
|
||||
28. **Two lines carrying the same written line number still silently keep the last.**
|
||||
`akbasic_runtime_file_line()` refuses a collision that involves an *assigned* number, but
|
||||
leaves numbered-onto-numbered alone, which is what a file with `10 PRINT "A"` twice has
|
||||
always done. A duplicate in a hand-written file is a mistake rather than an intent, and
|
||||
refusing it would be consistent with the other three cases.
|
||||
|
||||
Not changed with the rest because it is the one collision that can appear in an existing
|
||||
program, so it carries corpus risk the others do not: 41 reference cases and 22 local ones
|
||||
would all have to be shown clean first. `src/runtime.c`, in the `FAIL_NONZERO_RETURN` that
|
||||
already names the slot.
|
||||
|
||||
29. **`akbasic_renumber()` keeps two 9999-entry `static` locals.**
|
||||
`src/renumber.c` holds `map[]` and `rewritten[]` at file scope, and
|
||||
`akbasic_runtime_check_targets()` now adds a `static` scratch buffer beside them. That is
|
||||
against the no-file-scope-mutable-state rule in `MAINTENANCE.md`, and it means `RENUMBER`
|
||||
and the target prescan are not reentrant across two `akbasic_Runtime`s in one process --
|
||||
the exact thing that rule exists to guarantee.
|
||||
|
||||
They are `static` because they will not fit on a default stack, which is the same reason
|
||||
`akbasic_Runtime` itself is too big for one. The honest fix is to hang them off the
|
||||
runtime like every other pool, which costs another ~2.5MB inline per interpreter for
|
||||
something used by one verb and one prescan. Recorded rather than done, because "make it
|
||||
reentrant" and "do not grow the runtime by a third for a scratch buffer" are both right
|
||||
and picking between them is a decision.
|
||||
29. **`akbasic_renumber()` keeps two 9999-entry `static` locals**, so `RENUMBER` and the target
|
||||
prescan are not reentrant across two runtimes in one process. Hanging them off the runtime
|
||||
costs ~2.5MB per interpreter for something one verb uses; both answers are defensible.
|
||||
**Issue #12.**
|
||||
|
||||
### Mutation-testing gaps in the files the game work touched
|
||||
|
||||
Recorded the way §8's CI note records `src/audio_tables.c`: a real test gap rather than a
|
||||
reason to avoid the file.
|
||||
|
||||
34. **`src/runtime.c` has never had a complete mutation run, and the one attempted here
|
||||
was cut off at 551 of 997 mutants** after ninety minutes. What it did cover included
|
||||
all three regions the value-pool, trap and skipped-block work touched, and the
|
||||
survivors there are worth naming:
|
||||
|
||||
- `report_and_reraise()`: **closed.** Deleting the `LOG_ERROR_WITH_MESSAGE` survived,
|
||||
because nothing looked at the log -- and that line *is* the visible half of item
|
||||
33's second fix. `tests/trap_verbs.c` now redirects `akerr_log_method` into a buffer
|
||||
and drives the path with a sink that refuses every write, which kills it.
|
||||
- The skipped-block guard (`src/runtime.c`, the `BEND` test): four survivors, and
|
||||
three of them are equivalent for any program that can be written -- `loopFirstLine
|
||||
!= 0` mutated to `!= 1` differs only for a loop on line 1, `waitingForCommand[0]`
|
||||
to `[1]` only for a one-character wait, and flipping `strcmp(...) == 0` to `!= 0`
|
||||
moves the release from the `FOR` to the `NEXT` that must follow it. A test with a
|
||||
loop on line 1 would kill the first. The other two are noted rather than chased.
|
||||
- A NULL-argument guard in `akbasic_runtime_reserve_globals()`, which is the same
|
||||
survivor class every file in this tree has: nothing calls these with NULL.
|
||||
|
||||
The remaining 446 mutants are unexamined. A full run is a release-workflow job, not a
|
||||
per-commit one -- `.gitea/workflows/release.yaml` already mutates the whole tree -- and
|
||||
what this entry is for is that the *partial* result has been read rather than filed
|
||||
unopened.
|
||||
|
||||
Measured alongside: `src/variable.c` scores 68.2% with **every mutant on the inline-
|
||||
storage lines killed**, and `src/runtime_trap.c` 82.5% with **no survivors** in
|
||||
`set_error_variables()`. Both files' remaining survivors are pre-existing and are
|
||||
mostly NULL guards and `MAX - 1` arithmetic in string helpers.
|
||||
34. **`src/runtime.c` has never had a complete mutation run** -- the one attempted was cut off
|
||||
at 551 of 997 mutants. What it covered has been read rather than filed unopened, and three
|
||||
of the four skipped-block survivors are equivalent mutants. **Issue #24.**
|
||||
|
||||
### Found while writing a game against the interpreter
|
||||
|
||||
@@ -2187,110 +2030,18 @@ the same shape as item 12 above — a parser function that consumes one operator
|
||||
should loop — and the third is why both games are built out of `LABEL` and `GOTO`.
|
||||
|
||||
35. **An expression mixing `-` and `+` at the same level is evaluated right to left.**
|
||||
`a - b + c` computes `a - (b + c)`. A C128, and every BASIC this descends from, folds a
|
||||
run of same-precedence additive operators left to right.
|
||||
`10 - 3 + 2` is 5 where a C128 gives 9, silently. The cause is the grammar's shape: two
|
||||
additive precedence levels where there should be one. **Issue #1.**
|
||||
|
||||
```basic
|
||||
PRINT 10 - 3 + 2
|
||||
PRINT 1 - 2 - 3
|
||||
```
|
||||
36. **Only one unparenthesised `AND` or `OR` is matched per expression**, so a three-term
|
||||
condition is refused. `logicalandor()` is an `if` where `subtraction()` is a `while` -- the
|
||||
same defect as item 12, on the operator pair item 12 did not reach. **Issue #2.** Worth
|
||||
doing in the same sitting as item 35.
|
||||
|
||||
```output
|
||||
5
|
||||
-4
|
||||
```
|
||||
|
||||
The second line is right and the first is not; 9 is the answer. **Nothing fails** — this
|
||||
is item 4's failure mode over again, a program that quietly computes something else — and
|
||||
the two tutorials work around it by parenthesising every mixed `+` and `-` as a rule
|
||||
rather than where it happens to matter.
|
||||
|
||||
**The cause is the grammar's shape, not a missing loop.** `subtraction()`
|
||||
(`src/parser.c:425`) sits *above* `addition()` (`:447`) as its own precedence level, and
|
||||
both loop correctly — but `subtraction()`'s right operand is parsed by `addition()`, which
|
||||
then consumes the `+` that should have been the *next* term of the subtraction. `1 - 2 - 3`
|
||||
is right because the outer loop sees both minuses; `10 - 3 + 2` is wrong because the inner
|
||||
one eats the plus first.
|
||||
|
||||
The fix is one additive level rather than two: a single function matching `PLUS` and
|
||||
`MINUS` in one `while`, building left-associatively, with `multiplication()` beneath it.
|
||||
That is what item 12 left half-done — it made each of the two levels fold left without
|
||||
noticing that there should only have been one. Touches `src/parser.c` and wants cases in
|
||||
`tests/parser_expressions.c` beside item 12's, asserting `10 - 3 + 2` is 9 and
|
||||
`10 + 3 - 2` is 11 (which already passes, and is the asymmetry that gives it away).
|
||||
|
||||
36. **Only one unparenthesised `AND` or `OR` is matched per expression.** A second one is left
|
||||
in the token stream, and in the place a program actually writes them — a condition — the
|
||||
`THEN` check then finds the wrong token and the line is refused:
|
||||
|
||||
```basic
|
||||
X# = 1
|
||||
Y# = 2
|
||||
IF X# = 1 AND Y# = 2 THEN PRINT "TWO IS FINE"
|
||||
IF X# = 1 AND Y# = 2 AND X# = 1 THEN PRINT "THREE IS NOT"
|
||||
```
|
||||
|
||||
```output
|
||||
TWO IS FINE
|
||||
? 4 : PARSE ERROR Incomplete IF statement
|
||||
```
|
||||
|
||||
Parenthesising is the workaround and it works — `IF (A AND B) AND C THEN` parses — which
|
||||
is what the tutorials tell a reader to do. It is a refusal rather than a wrong answer, so
|
||||
it costs a puzzled minute rather than an evening; item 42 in §5 fixed the case of *one*
|
||||
`AND` and stopped there.
|
||||
|
||||
`logicalandor()` (`src/parser.c:329`) is `if ( akbasic_parser_match(obj, OPS, 2) )` at `:339`
|
||||
where `subtraction()` at `:425` is `while`. **This is item 12's defect exactly**, on the
|
||||
one operator pair item 12 did not reach, and the reference has the same shape at
|
||||
`basicparser.go`. The fix is the same fix: `while`, folding left, reassigning `left` from
|
||||
the leaf built on the previous pass. `tests/parser_expressions.c` for the expression form
|
||||
and `tests/parser_commands.c` for the `IF` that made it visible.
|
||||
|
||||
Worth doing together with item 35: they are one afternoon, one file, and between them they
|
||||
are the two rules a tutorial currently has to spend a paragraph on.
|
||||
|
||||
37. **A `GOTO` that leaves a `FOR` or a `DO` leaks the loop's scope**, so a program that uses
|
||||
the pattern as its main loop stops after the thirty-second time round:
|
||||
|
||||
```basic
|
||||
N# = 0
|
||||
LABEL TOP
|
||||
DO
|
||||
N# = N# + 1
|
||||
IF N# < 100 THEN GOTO TOP
|
||||
LOOP UNTIL N# > 99
|
||||
PRINT "SURVIVED " + N#
|
||||
```
|
||||
|
||||
```output
|
||||
? 3 : PARSE ERROR Environment pool exhausted at line 3 (32 in use)
|
||||
```
|
||||
|
||||
Thirty-two is `AKBASIC_MAX_ENVIRONMENTS` (`include/akbasic/types.h:31`), and the branch
|
||||
that reports is the `DO` being entered for the thirty-third time rather than the `GOTO`
|
||||
that did the damage — the same misattribution item 2 had. The `FOR` form behaves
|
||||
identically.
|
||||
|
||||
**This is a real pattern rather than a contrivance.** A game's main loop is a loop the
|
||||
program leaves from the middle of: losing a life, clearing a level and quitting are all
|
||||
"stop what you are doing and go somewhere else". Both Breakout listings in `examples/`
|
||||
are built out of `LABEL`/`GOTO` for exactly this reason, which is the shape a reader of
|
||||
chapters 17 and 18 is now told to copy, and it is a workaround rather than a preference —
|
||||
a `DO ... LOOP` around the frame would be the natural way to write either one.
|
||||
|
||||
The mechanism is the one item 2 documents from the other end: a loop's environment is
|
||||
pushed when the line is parsed and popped by its `NEXT` or `LOOP`, and nothing else pops
|
||||
it. `GOTO` sets the next line and returns (`akbasic_cmd_goto()`, `src/runtime_commands.c`);
|
||||
it has no idea it has just left a scope.
|
||||
|
||||
A fix has to know which scopes the branch target is *outside* of, which the environment
|
||||
stack can answer if a loop's environment records the source line that pushed it: on a
|
||||
`GOTO`, pop every loop scope whose line is not on the path to the target. That is more
|
||||
machinery than items 35 and 36 want between them, and it is worth pricing against simply
|
||||
documenting it — chapters 13 and 17 both now say "loop with `GOTO`, never jump out of a
|
||||
`DO`", which is a rule a reader can follow. Wants `tests/for_next.c` either way, because
|
||||
the reduction above is four lines and currently nothing asserts it in either direction.
|
||||
37. **A `GOTO` that leaves a `FOR` or a `DO` leaks the loop's scope**, so a program using the
|
||||
pattern as its main loop dies on the thirty-third pass -- and the error names the loop being
|
||||
entered rather than the `GOTO` that did the damage. Both Breakout listings are built out of
|
||||
`LABEL`/`GOTO` because of it. **Issue #3.**
|
||||
|
||||
### Found while integrating libakgl's collision subsystem
|
||||
|
||||
@@ -2298,33 +2049,13 @@ should loop — and the third is why both games are built out of `LABEL` and `GO
|
||||
`akgl_CollisionShape` and a pooled proxy. Three things were deliberately left for later, and
|
||||
each is here so the reasoning does not have to be reconstructed.
|
||||
|
||||
38. **Finish moving `COLLISION` and `BUMP` onto the subsystem.** **Half done.** The contact
|
||||
is no longer discarded: `RCOLLISION(n, f)` reports what was hit, the normal, the depth,
|
||||
the contact point and the minimum translation axis, and
|
||||
`tests/akgl_backends.c::test_contact_geometry` pins the normal's *sign* for both parties
|
||||
of a sprite-against-sprite hit -- getting that backwards tells both to move the same way,
|
||||
and nothing else would have noticed. What stands is the pairing and the threshold below. What landed is the narrowphase
|
||||
and the shapes; the seam is still `collisions(self, uint16_t *mask)` and the pairing is
|
||||
still all-against-all over eight slots with a bounding-box reject in front of it.
|
||||
|
||||
That is the right shape *at eight sprites* and the benchmark says so: 54.9 ns a scan spread
|
||||
out, 211.8 ns for breakout's layout, against a 1.20 ms frame. A uniform grid over eight
|
||||
proxies costs more in per-proxy insert and move bookkeeping than an all-pairs loop over 28
|
||||
pairs saves, which is why the proxies are deliberately **not** registered with any
|
||||
partitioner — `akgl_collision_test()` takes two positioned proxies and needs no world.
|
||||
|
||||
**The threshold is what to record, not the work.** Registering the proxies and letting
|
||||
`akgl_collision_query_box()` or the partitioner's `each_pair` do the pairing becomes worth
|
||||
it somewhere above eight, and `AKBASIC_MAX_SPRITES` is a `#define`
|
||||
(`include/akbasic/sprite.h:38`). Anybody raising it should have the number in front of them
|
||||
rather than discovering the cost afterwards: re-run `tests/collision_perf.c` at the new
|
||||
ceiling and compare against the frame row. Note that libakgl's own measurement puts the
|
||||
naive all-pairs sweep at 0.7% of a frame at 64 actors and 11% at 256, so the crossover is
|
||||
well above anything a Commodore-shaped sprite budget will reach.
|
||||
|
||||
What remains is only the pairing seam above. The contact itself is consumed:
|
||||
`akbasic_collision_service()` pulls it per sprite and the `RCOLLISION` fields are what both
|
||||
breakout games now bounce on.
|
||||
38. **Finish moving `COLLISION` and `BUMP` onto the subsystem.** **Half done** -- the contact
|
||||
is no longer discarded and `RCOLLISION(n, f)` reports all of it, with
|
||||
`tests/akgl_backends.c::test_contact_geometry` pinning the normal's *sign* for both parties.
|
||||
What stands is the pairing seam, which is all-against-all over eight slots and is the right
|
||||
shape at eight: 54.9 ns a scan against a 1.20 ms frame. **The threshold is the thing to
|
||||
record, not the work** -- anybody raising `AKBASIC_MAX_SPRITES` should re-run
|
||||
`tests/collision_perf.c` first. **Issue #14.**
|
||||
|
||||
39. ~~**Convert the breakout listings and chapters 17 and 18 to the collision verbs.**~~
|
||||
**Done.** Both games and both tutorials now register their bricks with `SOLID`, arm
|
||||
@@ -2348,7 +2079,7 @@ each is here so the reasoning does not have to be reconstructed.
|
||||
ball that passes through the wall.
|
||||
|
||||
Chapter 18 also lost a rule it depended on and no longer stated: a `FOR` with equal bounds
|
||||
does not run its body (§6 item 19), which is why every list in the chapter has its one-item
|
||||
does not run its body (§6 item 19, now issue #5), which is why every list in the chapter has its one-item
|
||||
case written out beside the loop. It is now stated in Step 7 where it is first used, rather
|
||||
than cross-referenced to a step that the rewrite deleted.
|
||||
|
||||
@@ -2395,40 +2126,9 @@ each is here so the reasoning does not have to be reconstructed.
|
||||
candidates during the walk, close the walk, then dispatch.
|
||||
|
||||
41. **A multi-line `DEF` called outside a running program does not run its body**, and says
|
||||
nothing about it.
|
||||
|
||||
```basic
|
||||
DEF TRIPLE(N#)
|
||||
T# = N# * 3
|
||||
RETURN T#
|
||||
PRINT TRIPLE(14)
|
||||
```
|
||||
|
||||
At the REPL that prints `(UNDEFINED STRING REPRESENTATION FOR 0)`. In a program run from a
|
||||
file the same function answers 42. No error either way.
|
||||
|
||||
`akbasic_runtime_user_function()` (`src/runtime.c`) runs a multi-line body by handing
|
||||
control to the call's environment and spinning a line loop until `RETURN` pops back out,
|
||||
guarded on `mode == AKBASIC_MODE_RUN` -- which is true only of a program running from a
|
||||
file. In `REPL` mode the loop is skipped entirely and the result is whatever is in the
|
||||
caller's return slot, which is zero. The single-expression form has no such loop and is
|
||||
unaffected, which is most of why this has gone unnoticed; and `tests/user_functions.c`
|
||||
drives every case through `run_program`, so the whole suite is in RUN mode.
|
||||
|
||||
**The obvious fix is wrong, and was tried.** Widening the guard to
|
||||
`mode != AKBASIC_MODE_QUIT` makes the interpreter *hang* rather than answer wrongly:
|
||||
`akbasic_runtime_process_line_run()` does not advance a REPL-mode runtime the way this
|
||||
loop assumes, so the environment never comes back. Trading a silent wrong answer for a
|
||||
lock-up is worse. The fix wants the REPL's own line cycle driving the body, which is a
|
||||
change to how a call is executed rather than to a condition.
|
||||
|
||||
It also bounds item 40: `akbasic_runtime_call_function()` reaches a multi-line body only
|
||||
while a program is running, which is exactly the case a verb calling a callback is in and
|
||||
not the case a host poking at a stopped runtime is in.
|
||||
|
||||
A test belongs in `tests/user_functions.c` asserting the correct contract -- a multi-line
|
||||
function answering 42 at the REPL -- registered in `AKBASIC_KNOWN_FAILING_TESTS` until it
|
||||
does.
|
||||
nothing about it -- the loop is guarded on `mode == AKBASIC_MODE_RUN`, and the whole suite
|
||||
drives every case through `run_program`. **The obvious fix is wrong and was tried**: widening
|
||||
the guard makes the interpreter hang rather than answer wrongly. **Issue #8.**
|
||||
|
||||
42. ~~**A name that merely contains a branch verb was treated as a branch.**~~ **Fixed.** The
|
||||
target prescan walks a line character by character looking for `GOTO`, `GOSUB`,
|
||||
@@ -2453,7 +2153,7 @@ each is here so the reasoning does not have to be reconstructed.
|
||||
## 7. Filing gaps against `libakgl`
|
||||
|
||||
When phase 7 or phase 8 needs something `libakgl` does not have, **stop and file it** in
|
||||
`deps/libakgl/TODO.md` in that file's numbered prose style: what the BASIC verb requires, what
|
||||
`libakgl`'s issue tracker: what the BASIC verb requires, what
|
||||
the `akgl_*` entry point should look like, and what tests would cover it. Growing `libakgl` to
|
||||
serve the interpreter is a wanted outcome, not a detour.
|
||||
|
||||
@@ -2489,12 +2189,11 @@ covers libakgl's three widget helpers and nothing else. What a program cannot ha
|
||||
what closing each would cost:
|
||||
|
||||
1. **A top-centre or bottom-centre `HUD` anchor.** `akgl_UiAnchor` has the four corners
|
||||
and dead centre, so `HUD` has exactly those five. Closing it upstream is two appended
|
||||
enum values and two arms in `akgl_ui_label()`'s switch (`deps/libakgl/src/ui.c`), using
|
||||
the `CLAY_ATTACH_POINT_CENTER_TOP` and `..._CENTER_BOTTOM` constants the dialog helper
|
||||
in the same file already uses, with the horizontal inset dropped. Appending keeps 0
|
||||
through 4 meaning what they mean, so it is additive. **Not filed, and not done here:
|
||||
changing a dependency submodule is that repository's decision, not this one's.**
|
||||
and dead centre, so `HUD` has exactly those five. Two appended enum values and two arms in
|
||||
`akgl_ui_label()`'s switch, using constants the dialog helper in the same file already uses.
|
||||
**Filed upstream as `libakgl` #79** — this entry used to say "not filed, because changing a
|
||||
dependency submodule is that repository's decision", which was the right instinct about
|
||||
*changing* it and the wrong outcome for *reporting* it.
|
||||
2. **Per-widget styles.** `UISTYLE` sets one look shared by everything, because
|
||||
`akgl_UiStyle` is per-call and threading an optional style number through three verbs'
|
||||
argument shapes buys less than it costs. Nothing upstream is missing.
|
||||
@@ -2502,7 +2201,8 @@ what closing each would cost:
|
||||
panels, custom elements, hit-testing. Reachable from C against `akgl/ui.h` in the same
|
||||
window; deliberately not reachable from BASIC.
|
||||
4. **A dialog the player can dismiss.** libakgl has no dismiss contract — a declarative
|
||||
frame is the visibility flag — so `DIALOG` with no argument is the only way down.
|
||||
frame is the visibility flag — so `DIALOG` with no argument is the only way down. **Filed
|
||||
upstream as `libakgl` #80.**
|
||||
|
||||
One cost carried rather than fixed: libakgl re-rasterizes every line of widget text every
|
||||
frame, which is its own open `TODO.md` item. A `HUD` rebuilt every frame is the expensive
|
||||
@@ -2587,32 +2287,10 @@ None of the three was on any list. All were found by writing down what a chapter
|
||||
running it — which is the argument for the harness rather than a footnote to it.
|
||||
|
||||
**A fourth is open, and it is the same defect as item 2 on a path that fix did not cover.**
|
||||
Writing `docs/14-architecture.md` meant describing where the boundary between a script's
|
||||
error and the host's error actually sits, and the answer is: around parsing and around
|
||||
`interpret()`, but **not around scanning**. `akbasic_runtime_process_line_repl()` and
|
||||
`akbasic_runtime_process_line_run()` both call `akbasic_scanner_scan()` under a bare `PASS`
|
||||
(`src/runtime.c:825` and `:921`), so any error the scanner raises leaves `step()` as an
|
||||
interpreter error. The reachable one is the token ceiling:
|
||||
|
||||
```sh norun
|
||||
$ printf 'PRINT 1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1+1\n' | ./build/basic
|
||||
src/scanner.c:add_token:87: 515 (Out Of Bounds) : Line 0 has more than 32 tokens
|
||||
... eight more frames ...
|
||||
akbasic terminated on an unhandled error 515 (Out Of Bounds)
|
||||
```
|
||||
|
||||
Exit status 1, a stack trace on stderr, and the prompt gone — for a line a person typed. An
|
||||
embedding host is handed a context for what is plainly the script's mistake, which is what
|
||||
goal 3 exists to prevent. It should print `? N : PARSE ERROR Line N has more than 32 tokens`
|
||||
and stop the run, exactly as a parse error does two lines further down.
|
||||
|
||||
The fix is the same `ATTEMPT`/`HANDLE_DEFAULT`/`akbasic_runtime_error()` wrapper already
|
||||
sitting around `akbasic_parser_parse()` in both functions, moved up to cover the scan.
|
||||
Three call sites to check, because `process_line_runstream()` has the same bare `PASS` and
|
||||
its answer may want to differ: a bad line arriving from a file mid-load is not the same
|
||||
situation as one typed at a prompt. Wants a test in `tests/scanner_tokens.c` asserting the
|
||||
error line rather than the raised status, and one in `tests/housekeeping_verbs.c` asserting
|
||||
the REPL survives it — the shape item 2's test already has.
|
||||
The boundary between a script's error and the host's error sits around parsing and around
|
||||
`interpret()`, but **not around scanning** — so the token ceiling takes the prompt down with a
|
||||
stack trace for a line a person typed. **Issue #4**, which also names the third call site whose
|
||||
answer may want to differ.
|
||||
|
||||
**A separate one was found while designing optional line numbers, and is fixed.**
|
||||
`akbasic_runtime_process_line_runstream()` filed the *raw* buffer, line number and all, while
|
||||
@@ -2638,13 +2316,11 @@ and `src/graphics_tables.c` 100%, `src/runtime_input.c` 100%, `src/runtime_graph
|
||||
`src/runtime_audio.c` 98%, `src/play.c` 87%. The `akbasic_akgl` target is not in the coverage
|
||||
figure — it is not built in a default configuration.
|
||||
|
||||
**The denominator moved a long way and this is not an explanation of why.** The line total
|
||||
went from 4076 to 6222 between the previous entry and this one, which is far more than the
|
||||
work since added — so one of the two measurements is counting something the other is not,
|
||||
and which one is right has not been established. Both figures were produced with
|
||||
`gcovr --filter 'src/.*'` over a clean tree. Recorded rather than quietly overwritten,
|
||||
because a coverage percentage whose denominator nobody can account for is worth exactly as
|
||||
much as the accounting. The *ratio* is above the gate on either reading.
|
||||
**The denominator moved a long way and this is not an explanation of why.** The line total went
|
||||
from 4076 to 6222 between two entries, which is far more than the work added between them, so one
|
||||
of the two measurements is counting something the other is not. Recorded rather than quietly
|
||||
overwritten, because a coverage percentage whose denominator nobody can account for is worth
|
||||
exactly as much as the accounting. The ratio is above the gate on either reading. **Issue #26.**
|
||||
|
||||
**A note on reading that number, because it was misread once while producing it.** A stale
|
||||
`build-cov/` left in the source directory from an earlier session was silently folded into the
|
||||
@@ -2656,7 +2332,7 @@ and if a coverage number looks suspiciously unchanged, `find . -name '*.gcda'` b
|
||||
it.
|
||||
|
||||
Branch coverage reads 18.0% and is not a target, for the reason `libakgl/TODO.md` and
|
||||
`libakstdlib/TODO.md` both give: the akerror control-flow macros expand into large branch trees
|
||||
`libakstdlib` both give: the akerror control-flow macros expand into large branch trees
|
||||
at every call site, most of them unreachable in normal operation. Track line and function
|
||||
coverage.
|
||||
|
||||
@@ -2745,29 +2421,21 @@ What remains, in priority order:
|
||||
2. ~~**A line editor for the akgl sink.**~~ **Done** — `readline` in `src/sink_akgl.c`, over
|
||||
the keystroke ring, borrowing frames from the host through an `akbasic_AkglPump`. It cannot
|
||||
type a shifted character, which is `libakgl` item 10 rather than work outstanding here.
|
||||
3. **§4 — the language completion work queue**, and it is the only substantial thing left.
|
||||
Done: groups G and I, the `GET`/`GETKEY`/`SCNCLR` part of E, group B, multiple statements
|
||||
per line, and array references in parameter lists. Left: groups A, D, F and J, plus
|
||||
`RESTORE` and `RENUMBER` out of B and the sprite group H. None needs anything from
|
||||
`libakgl` except H.
|
||||
3. **The language completion queue is done, and this entry used to say otherwise.** Every group
|
||||
in §4's table — A through J, `RESTORE` and `RENUMBER` included — is implemented and tested.
|
||||
The list that used to sit here naming A, D, F, J and H as outstanding was stale.
|
||||
|
||||
**Two pieces of structural work come before the verbs, and both were found rather than
|
||||
guessed.** Neither is a verb and neither can be skipped by the group that needs it:
|
||||
**What survives it is one piece of structural work, and it is not a verb.** Block skipping
|
||||
works by source *line*, so a whole `FOR`/`NEXT` written on one line never reaches its `NEXT`
|
||||
— inherited structure that did not matter until a line could hold more than one statement,
|
||||
and `DO`/`LOOP`/`WHILE`/`UNTIL` are exactly the verbs people write on one line. **Issue #6.**
|
||||
|
||||
- **Block skipping works by source line, and group A needs it to work by statement.**
|
||||
`waitingForCommand` skips forward to a verb one *line* at a time, so a whole loop written
|
||||
on one line — `FOR I# = 1 TO 3 : PRINT I# : NEXT I#` — never reaches its `NEXT`. That is
|
||||
inherited structure rather than a slip, and it did not matter until a line could hold more
|
||||
than one statement. `DO`/`LOOP`/`WHILE`/`UNTIL` are exactly the verbs people write on one
|
||||
line, so group A starts here or it ships something that does not work.
|
||||
- **`READ` has no DATA pointer, and `RESTORE` is the verb that needs one.** §4 has the
|
||||
detail; the short version is that `READ` skips forward to the next `DATA` *reached in
|
||||
execution order*, so a `DATA` line placed before its `READ` is never found at all and a
|
||||
second `READ` re-reads the same line. It is a live defect, not only a blocked verb.
|
||||
`FILTER` is the one verb still refused, and deliberately: `akgl_audio_*` synthesises raw
|
||||
waveforms and mixes them, there is no filter stage to configure, and SDL3 supplies no
|
||||
primitive to build one from. It is refused with `AKBASIC_ERR_DEVICE` rather than silently
|
||||
ignored — a program that asks for a low-pass and gets an unfiltered square wave has been
|
||||
lied to. The libakgl half is that repository's issue #58.
|
||||
|
||||
Ordering suggestion, unchanged in spirit from the original: A (after the restructure), then
|
||||
D and J, which are self-contained, then F, which is 21 verbs of `aksl_f*` and mostly
|
||||
mechanical. H needs a look at `libakgl`'s sprite and actor API before anything is filed.
|
||||
4. ~~**CI does not cover `-DAKBASIC_WITH_AKGL=ON`.**~~ **Done** — the `akgl_build` job in
|
||||
`.gitea/workflows/ci.yaml`. It turned out much cheaper than the deferral assumed, and the
|
||||
two reasons are worth keeping because both were guesses that measurement corrected:
|
||||
@@ -3037,56 +2705,17 @@ reduced against `build/basic`, the stdio build, unless it says otherwise.
|
||||
`tests/structure_verbs.c`, which today only exercises blocks whose bodies are plain
|
||||
statements.
|
||||
|
||||
3. **In the standalone SDL build, nothing the graphics verbs draw can be seen unless the
|
||||
program shrinks the text area first.** **Half done.** The tee sink now forwards
|
||||
`WINDOW` (§6 item 31), so `sink_window()` is reachable and a program *can* take rows
|
||||
back from the text layer -- which is the half that made this unanswerable. What stands
|
||||
is the default: the text area is the whole window, so a program that does not call
|
||||
`WINDOW` still cannot see a drawing.
|
||||
3. **In the standalone SDL build, the text layer owns every row of the window by default.**
|
||||
**Half done.** The tee sink now forwards `WINDOW`, so a program *can* take rows back, and the
|
||||
drawing verbs render into a layer the frame composites under the text and the sprites, so a
|
||||
drawing survives the frame it was made in.
|
||||
|
||||
Whether that default is right is a real question rather than a defect, and it is
|
||||
entangled with what no `WINDOW` call fixes -- the frontend never clears and SDL
|
||||
double-buffers, so a drawing has to be re-issued every frame, and it has to fit inside
|
||||
one batch to survive a capture (item 5). Both are now documented in chapters 6, 13 and
|
||||
14. **Sprites remain the only thing visible for free**, so the trick the game uses is
|
||||
still the right one for a game; it should not be the *only* way, and it no longer is.
|
||||
|
||||
The original report, whose second bullet is fixed and whose first stands:
|
||||
|
||||
```basic
|
||||
GRAPHIC 1, 1
|
||||
COLOR 1, 3
|
||||
BOX 1, 100, 300, 700, 500
|
||||
PRINT "THE TEXT SHOWS AND THE BOX DOES NOT"
|
||||
LABEL SPIN
|
||||
GOTO SPIN
|
||||
```
|
||||
|
||||
The text appears; the box never does. Two mechanisms combine, and each alone would be
|
||||
survivable:
|
||||
|
||||
- `akbasic_sink_akgl_render()` fills **every row of the text area, opaque, every frame**
|
||||
(`src/sink_akgl.c:564`), and the area is the whole window — `state->rows = h / cellh` at
|
||||
`:505`. `akbasic_frontend_akgl_pump()` calls it after `akbasic_runtime_run()` and before
|
||||
`SDL_RenderPresent`, so it paints over everything the program drew during those steps.
|
||||
The comment at `:564` explains why it must repaint rather than track dirty rows, and that
|
||||
reasoning is sound; what it does not account for is that the rows it owns are all of them.
|
||||
- `WINDOW` would shrink the text area out of the way, and the akgl sink implements it
|
||||
(`sink_window`, `src/sink_akgl.c:413`) — but **the tee sink in front of it never assigns
|
||||
`obj->window`** (`src/sink_tee.c:143-151` assigns `clear` and conditionally `moveto`, and
|
||||
stops). So `WINDOW` refuses with "needs a text device with a character grid"
|
||||
(`src/runtime_console.c:179`) in the one build that has a character grid.
|
||||
|
||||
The result is that **chapter 6 cannot be run in the interpreter it documents**. Its figures
|
||||
are right because `tools/screenshot.c` deliberately omits the text layer — the comment in
|
||||
that file says so — so the harness that proves the chapter is exactly the harness that hides
|
||||
this. `docs_screenshots` passes and always would.
|
||||
|
||||
Sprites are unaffected: `akbasic_sprite_akgl_render()` runs after the sink, which is why the
|
||||
game draws its entire screen by `SSHAPE`-ing what it drew and installing it with `SPRSAV`.
|
||||
That is a fine trick and it should not be the only way.
|
||||
|
||||
The tee half is three lines and is worth doing whatever is decided about the rest.
|
||||
What stands is the default, and it is a real question rather than a defect: a program that
|
||||
does not call `WINDOW` still cannot see a drawing. **The consequence worth keeping is that
|
||||
chapter 6 cannot be run in the interpreter it documents** — its figures are right because
|
||||
`tools/screenshot.c` deliberately omits the text layer, so the harness that proves the chapter
|
||||
is exactly the harness that hides this, and `docs_screenshots` passes and always would.
|
||||
**Issue #23.**
|
||||
|
||||
4. ~~**Mixed-type arithmetic is decided by the left operand alone, and nothing says so.**~~
|
||||
**Done, as documentation, by decision.** The behaviour is unchanged: a dialect is
|
||||
|
||||
Reference in New Issue
Block a user