From 33b7ff65e985a4e59bc4817bd167abca378bda02 Mon Sep 17 00:00:00 2001 From: Stefan Wessels Date: Fri, 7 Aug 2026 21:31:11 -0700 Subject: [PATCH] Work around for cgetc bug in plus4 cc65 --- AGENTS.md | 26 +++++++++++++++-- README.md | 6 ++-- doc/engine.md | 8 +++-- doc/measuring.md | 68 +++++++++++++++++++++++++++++++++++++++---- src/plus4/platPlus4.c | 25 ++++++++++++---- 5 files changed, 113 insertions(+), 20 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 867f173..40d52ec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,9 +19,23 @@ directly, and this was discovered the hard way: - the attacker **count** for a tile and side — no port ever reads the attacker list - `gTile[0]`, `gTile[1]`, `gPiece[1]`, `gColor[0]` — the move log line -**`atari` and `cx16` are not built or tested here** — the tools live on a Windows machine. -Do not delete them, do not knowingly break them, do not edit their platform files -speculatively. Both had pre-existing build failures unrelated to the engine work. +**`cx16` is not built or tested here.** It has a build failure in `src/cx16/platCX16.c` +that predates the engine work, and its tools live on a Windows machine. Do not delete it, do +not knowingly break it, do not edit its platform files speculatively. + +**`apple2`, `plus4` and `atari` are no longer in that category.** All three build here. +`apple2` and `plus4` also *run* here — `../a2m-v2` for the Apple II, VICE's `xplus4` binary +monitor for the Plus/4 — so a change to either can be verified instead of argued (§7 of +`doc/measuring.md`). `atari` builds to a `.atr` with `dir2atr`, but running it is still +Altirra on the Windows machine. + +**Compiling a target says nothing about whether it runs.** The plus4 build linked inside its +budget throughout the rework and was broken end to end — by a **cc65 bug**, not ours: in +bitmap mode `cgetc()` skips its own wait loop (the branch means to skip the cursor drawing) +and reads a character that is not there, underflowing the Kernal's key count at `$EF`. After +that every key read returns garbage and the menus drive themselves. `plat_ReadKeys` works +around it by checking `$EF` before calling `cgetc()`. Prefer running a target over reasoning +about it. **Keys must not change.** Cursor keys, RETURN, RUN/STOP, `M`, `B`, `A`, `D`, `U`, `R` keep their current meanings. New functionality that needs a key is a red flag — raise it rather @@ -75,8 +89,14 @@ measured over a real game. ```bash make OPTIONS=optspeed TARGETS=c64 +make TARGETS=apple2 && make po # bootable chess.po, needs cadius +make TARGETS=atari && make atr # bootable cc65-Chess.atr, needs dir2atr ``` +The image step is a **second `make`**. `TARGETS=` sets the suffix `$(PROGRAM)` already +carries, so `make TARGETS=apple2 po` asks for `cc65-Chess.apple2.apple2` and fails, and a +bare `make po` has no rule to build the binary it needs. + ```bash cc -Isrc -lcurses -funsigned-char src/globals.c src/engine.c src/eval.c src/search.c \ src/board.c src/undo.c src/cpu.c src/human.c src/frontend.c src/main.c \ diff --git a/README.md b/README.md index 5d06de2..6926a79 100644 --- a/README.md +++ b/README.md @@ -82,9 +82,9 @@ cc -Isrc -lcurses -funsigned-char src/globals.c src/engine.c src/eval.c src/sear src/term/platTerm.c -o /tmp/chessterm ``` -**Current state:** `c64`, `c64.chr`, `apple2`, `atmos` and `plus4` build and link inside -their budgets. `atari` and `cx16` have build failures that predate the engine rework and look -like cc65 version drift. +**Current state:** every target except `cx16` builds and links inside its budget. `cx16` has +a build failure in its platform file that predates the engine rework and looks like cc65 +version drift. ## The documentation diff --git a/doc/engine.md b/doc/engine.md index c381a8f..f9ee6c5 100644 --- a/doc/engine.md +++ b/doc/engine.md @@ -102,9 +102,11 @@ walks a ray and stops at the edge automatically (`genSlider`). **The UI never sees a 0x88 square.** `plat.h` and every platform file use the original 0–63 tile numbering, and `ENG_TO_TILE` / `ENG_FROM_TILE` convert at the boundary. This is a hard -rule, not a style preference: several ports (atari, cx16, apple2 among them) cannot be built -or tested on the development machine, and freezing that interface is what let the entire -engine be replaced without editing a single platform file. +rule, not a style preference: when the engine was replaced, several ports (atari, cx16 and +apple2 among them) could not be built or tested on the development machine at all, and +freezing that interface is what let the entire engine be replaced without editing a single +platform file. Most of them can be built and run now. The rule stays, because it is what +made that possible. ## 1.2 The piece byte diff --git a/doc/measuring.md b/doc/measuring.md index 8622af4..b1bcfe0 100644 --- a/doc/measuring.md +++ b/doc/measuring.md @@ -303,6 +303,53 @@ The unit tests cover the engine; this covers whether the game is a game. Read it that things are being drawn, not as a pixel-accurate reference — partial redraws can leave stale glyphs in its terminal model that a real terminal would not show. +### On the real Apple II + +`../a2m-v2` is an Apple II emulator with a scriptable control port, which makes `apple2` the +one 8-bit target that can be driven the same way — except that here the machine really is the +machine, so it also catches what the terminal build cannot: character sets, video modes, +firmware entry points. + +```bash +make TARGETS=apple2 && make po # the image step is a second make +cd ../a2m-v2 && ./build/a2m-v2 --noini \ + --hd s7d0=/chess.po --control-port 6510 +``` + +Run it **windowed**; `--headless` removes the human from the loop. `--model plus` gives a +][+ instead of a //e, and anything touching video or firmware needs checking on both. +`tools/a2m_control_client.py` gives you `mem()`, `get_frame()` (560x192 ARGB) and `key`, which +is enough to boot the disk, walk the menus and watch an AI-vs-AI game play itself. + +Two things to know before trusting it. `get-memory` reads the page table directly and never +reaches the softswitch handler, so **`$C0xx` reads are meaningless** — infer video state from +rendered frames. And **diffing one character cell across frames is a real measurement**: it +separates steady text from flashing text, which is how a character-set bug gets distinguished +from an encoding bug. + +### On the real Plus/4 + +VICE's `xplus4` does the same job through its **binary** monitor. The wire protocol and its +traps are documented in `../c64m/agents/vice-oracle.md`; a minimal Python client lives in +`scratch/vice/`. + +```bash +xplus4 -TEDdsize -autostart-delay 40 -autostart cc65-Chess.plus4 \ + -binarymonitor -binarymonitoraddress ip4://127.0.0.1:6502 +``` + +**`-autostart-delay 40` is not decoration.** VICE's plain `-autostart` never starts the +program on a Plus/4: it writes the keyboard buffer from its vsync hook, races the Kernal's +read-modify-write of the pending count at `$EF`, and the count underflows — after which the +Kernal "types" the function-key macro table and the machine hangs in a `DSAVE`. The program +is loaded correctly the whole time; only the RUN is lost. + +Two habits the Apple II side does not force on you. **Closing the socket resumes emulation**, +so anything measured across two script runs has a gap where the machine ran unobserved — +hold one connection for a whole experiment. And **a checkpoint that never fires is not +evidence until a control fires**: arm one on an address the machine demonstrably executes +first. + --- ## 8. The workflow @@ -328,12 +375,21 @@ its own explanation before the speed is banked. ## 9. What none of this covers -**Four of the six 8-bit targets have never been run.** `c64.chr`, `apple2`, `atmos` and -`plus4` compile clean and link inside their budgets, and no platform file was edited, so they -*should* be fine. That is an argument, not evidence, and it is the largest untested surface in -the project. +**Two targets have never been run.** `c64.chr` and `atmos` compile clean and link inside +their budgets, and no platform file was edited, so they *should* be fine. That is an +argument, not evidence, and it is the largest untested surface in the project. -**`atari` and `cx16` do not build here** and had pre-existing failures unrelated to the engine -work. +`apple2` has since been booted and played here under `../a2m-v2`, `plus4` under VICE (both +§7), and `atari` on the Windows machine under Altirra. All three build here; only `atari` +still needs the other machine to run. + +**Running a target is not the same as compiling it, and the plus4 proved it.** That build +compiled clean and linked inside its budget for the whole rework, and it was broken by a cc65 +bug: in bitmap mode `cgetc()` returns a character that was never typed, underflowing the +Kernal's key count, after which the menus navigated themselves and the game quit. Nothing +short of running it could have found that. + +**`cx16` does not build here** — a pre-existing failure in its platform file, unrelated to +the engine work. **There is no continuous integration.** `make test` is a command someone has to remember. diff --git a/src/plus4/platPlus4.c b/src/plus4/platPlus4.c index 013ba99..ca8e68e 100644 --- a/src/plus4/platPlus4.c +++ b/src/plus4/platPlus4.c @@ -24,6 +24,8 @@ #define COLOR_ADDERSS (LUMINANCE_ADDERSS + 0x0400) #define CHARMAP_ROM (0xD400) #define CHARMAP_RAM (BITMAP_ADDRESS + 0x2000) +// the kernal's count of characters waiting in the keyboard buffer +#define KEY_COUNT (*(unsigned char*)0x00EF) /*-----------------------------------------------------------------------*/ #define BOARD_PIECE_WIDTH 4 @@ -475,12 +477,25 @@ int plat_ReadKeys(char blocking) { char key = 0; int keyMask = 0; - - if(blocking || kbhit()) - key = cgetc(); - else + + // work around a cc65 bug. in bitmap mode - which is all this port ever + // runs in - cgetc() takes a branch that means to skip drawing the text + // cursor but skips its wait loop with it, so it calls the rom's KBDREAD + // with nothing in the buffer. KBDREAD decrements the key count anyway, + // taking it from 0 to 255, and after that every read returns garbage and + // the menus drive themselves. kbhit() is no safer: it reports true on + // the function key count alone, and the cgetc() that follows still + // decrements this one. gate on the real count instead. + if(blocking) + { + while(!KEY_COUNT) + ; + } + else if(!KEY_COUNT) return 0; - + + key = cgetc(); + switch(key) { case 145: // Up