M2 complete: disk saving works via MSXgl's tool/disk_save module

Replaces the hand-rolled PHYDIO/DSKIO sector writer, which never worked,
with engine/src/tool/disk_save.h — added in MSXgl v1.3.0 for exactly this
("save to disk from a ROM application"). Saves are now a real FAT file,
KISSAT00.SAV, on the disk in drive A.

Four non-obvious requirements, all documented in CLAUDE.md:
- LibModules needs both "tool/disk_save" and "dos"
- ROMDelayBoot = true, or the Disk ROM's INIT never runs
- boot then takes 40-90s of emulated time (use `set throttle off`)
- DiskSave_Check() returns SAVEDATA_UNSIGNED for good files: with APPSIGN
  it wants the first 4 bytes to be g_AppSignature, but DiskSave_Save()
  writes the payload raw and never adds it

Also fixes a save-timing bug: the write now happens in AdvanceDay() after
the day increments, so the file describes the morning the player wakes to.
Saving during the evening made a reload replay that day and double-count
its cups.

Verified: play a full day, cold boot, resume at day 2 with served intact.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

projects/kissaten_yugure/CLAUDE.md

@@ -62,83 +62,53 @@ Read `docs/CONVERSATION.md` for the original design discussion and rationale.
 - Erase blits must repaint the *actual* background at that spot — the counter
   area has three bands (wainscot / counter top highlight / counter top).
 
-## Disk saving from a cartridge ROM — status: UNRESOLVED
+## Disk saving from a cartridge ROM — WORKING
 
-M2 wants saves on a real `.dsk`. The game is a cartridge MegaROM, which makes
+Saves go to a real file on a real disk: `KISSAT00.SAV` in the root of the
-this awkward. What is established so far, all verified on
+disk in drive A. The disk stays a normal FAT disk you can inspect and copy.
-`Philips_NMS_8250` + `-carta` + `-diska`:
 
-- **A cartridge ROM does boot fine on a disk machine.** Not a problem.
+**Use `engine/src/tool/disk_save.h`.** MSXgl v1.3.0 added this module
-- **`ROMDelayBoot = true` is required.** A cartridge's INIT normally runs
+specifically for "save to disk from a ROM application". Do not hand-roll
-  during the BIOS slot scan, and MSXgl never returns from it — so the Disk
+sector I/O — an earlier attempt using PHYDIO (Main ROM 0144h) and a direct
-  ROM's own INIT never happens, `NMBDRV` stays 0 and the disk hooks are never
+CALSLT to DSKIO (4010h) had both entry points return carry-clear *while
-  patched. This option installs an H.STKE hook and returns to the scan
+transferring nothing*, proven with a sentinel that survived the call intact.
-  instead. **Boot becomes much slower** — the game appears at ~40-90s of
-  emulated time, so early screenshots will show a blue BASIC screen and look
-  like a hang when they are not.
-- **Any call into the disk system must be made from RAM.** Both PHYDIO and
-  CALSLT reach the Disk ROM at 4000h — the same page as this cartridge — so
-  the calling instruction is switched out mid-call unless it lives elsewhere.
-  `Disk_InitStub()` copies a small stub to RAM for exactly this. Calling
-  directly from ROM black-screens the machine.
-- With the above in place, `NMBDRV` reads 2 and `MASTER` is non-zero, so the
-  Disk ROM *has* initialised by the time the game runs.
 
-**The remaining blocker:** both `PHYDIO` (Main ROM 0144h) and a direct
+Four things are required, none of them obvious:
-`CALSLT` to `DSKIO` (Disk ROM 4010h) return **carry clear — "success" — while
-never touching the destination buffer.** Proven with a 0xAA sentinel that
-survives the call intact. Page 0 is confirmed to be the Main ROM at call time
-(0x002D reads 1 = MSX2, 0x0144 holds 0xC3 = `JP`), so the entry points
-themselves are addressable.
 
-Because a clean error code is *not* proof that a read happened, `Disk_Probe()`
+1. **`LibModules` must include both `"tool/disk_save"` and `"dos"`.** The
-now verifies the buffer actually changed and treats "no change" as no disk.
+   module alone will not link. See `projects/samples/s_save.js` for the
-The game degrades gracefully: no disk means day 1, nothing persists, and the
+   reference configuration, and `s_save.c` for usage.
-evening card says so. Nothing crashes on either C-BIOS or a real-BIOS machine.
+2. **`ROMDelayBoot = true`.** A cartridge's INIT normally runs during the BIOS
+   slot scan and MSXgl never returns from it, so the Disk ROM's own INIT never
+   happens and there is no disk system to talk to. This option installs an
+   H.STKE hook and returns to the scan instead.
+3. **Boot becomes slow.** The game appears at roughly 40-90s of *emulated*
+   time. Screenshots before that show a blue BASIC screen and look exactly
+   like a hang. Use `set throttle off` in the test script so this costs
+   seconds of wall clock, not minutes.
+4. **`DiskSave_Check()` returns `SAVEDATA_UNSIGNED` for perfectly good
+   files.** With `AppSignature = true` (`-DAPPSIGN`), `DiskSave_Check()`
+   requires the file's first four bytes to equal `g_AppSignature` — but
+   `DiskSave_Save()` writes the payload raw and never adds it. Treat
+   `SAVEDATA_UNSIGNED` as success; `SaveState` carries its own magic, version
+   and checksum, which is a stronger check regardless.
 
-Next things to try: check whether the H.PHYD RAM hook is genuinely patched at
+**Save timing:** written in `AdvanceDay()` *after* the day counter increments,
-that moment; try a different disk machine profile; or reconsider the medium
+so the file always describes the morning the player wakes to. Saving during
-(see PLAN.md §8 item 1).
+the evening instead makes a reload replay the day just finished and
+double-count its cups.
 
-**RTC CMOS is not a viable fallback**, despite `RTC_USE_SAVEDATA` being TRUE
+Absent or unusable disk is not an error the player has to handle — it just
-in `msxgl_config.h`. Block 3 of the RP-5C01 is 13 nibbles, and MSXgl's
+means nothing persists, and the morning card says so.
-`RTC_SaveData()` (`engine/src/clock.c`) stores exactly **6 bytes** — against a
-~40-byte `SaveState`. `RTC_SaveDataSigned()` is tighter still. Remaining
-options are cartridge SRAM, the FM-PAC SRAM (MSXgl has a `PAC` module), or
-shrinking the save to a 6-byte fingerprint — which would cost per-character
-affinity and money, so it is a design decision, not just a storage one.
-Reference: MSX2 Technical Handbook ch.5, CLOCK-IC section.
 
-## Architecture conventions
+Test:
-- VRAM page 0: visible shop scene bitmap. Page 1: asset warehouse (sprite
+```
-  frames, portraits, UI tiles) blitted with VDP commands (HMMM/HMMV).
+openmsx -machine Philips_NMS_8250 -carta emul/rom/kissaten.rom -diska save.dsk
-- Seasons and time-of-day are palette swaps only (32-byte tables); never
+```
-  duplicate background art per season.
+Create a blank 720K disk with `tools/build/msxtar/msxtar -cf save.dsk --dos1 --size=720K`.
-- Sprite mode 2, two layered 16×16 sprites per customer (outline + fill).
-- Portraits are 48×48, three expressions per character max.
-- Game state lives in a single `SaveState` struct (~40 bytes); see DESIGN.md.
-  Implemented in `kissaten.c` with a magic word, version byte and checksum;
-  a failed or absent load is not an error, it just means day one.
-- The day is a three-phase machine (morning / open / evening) and the clock is
-  **event-driven** — customers are the clock. `CUSTOMERS_PER_DAY` is the
-  single pacing dial.
-- Dialogue scenes are data, not code: trigger conditions → portrait,
-  expression, text, optional choice, effects.
 
-## Code style
+**RTC CMOS is not an alternative** for a save this size, despite
-- C99, MSXgl idioms (u8/u16 types, `msxgl_` module prefixes)
+`RTC_USE_SAVEDATA` being TRUE in `msxgl_config.h`. Block 3 of the RP-5C01 is
-- Keep ISR/VBlank work minimal; game logic in main loop
+13 nibbles and MSXgl's `RTC_SaveData()` stores exactly **6 bytes**, against a
-- Comment bank-switching boundaries explicitly
+~40-byte `SaveState`. (MSXgl has no cartridge-SRAM mapper target either; the
-- Tools are Node.js (plain JS, no framework), living in `tools/`
+`PAC` module's FM-PAC SRAM, 8 x 1024 bytes, is the other real option.)
-
-## Scope ladder (build in this order; every stage must be playable)
-1. ✅ Shop scene + one customer + brew minigame + serve loop (no story)
-2. ◐ Day/night cycle + save + money — day counter, three phases and the
-   journal line work; **persistence is blocked** (see the disk section below).
-   Money deliberately deferred to stage 5, since nothing gates on it before.
-3. Dialogue engine + two regulars with short arcs  ← real milestone
-4. Seasons/weather/palette system
-5. Remaining cast, upgrades, endings
-
-`docs/PLAN.md` expands this into ten milestones with per-stage requirements
-and a decision queue. `docs/SCRIPT.md` holds the full first-draft script.

projects/kissaten_yugure/docs/PLAN.md

@@ -76,10 +76,9 @@ rains" patterns.
 
 **Produces:** `SaveState` struct, phase state machine, save/load module.
 
-**Status:** day counter, three phases, journal line and `SaveState` are done
+**Status: ✅ complete.** Day counter, three phases, journal line, `SaveState`
-and playable. **Persistence is blocked** — sector I/O from a cartridge ROM
+and disk persistence all working. Verified by playing a day, cold-booting, and
-reports success without transferring data; full findings in the disk section
+resuming at day 2 with the served count intact.
-of `../CLAUDE.md`.
 
 **Pacing dial, revisited at the end.** `CUSTOMERS_PER_DAY` in `kissaten.c` is
 the only number that sets day length. Worth reconsidering at M10 as a
@@ -482,7 +481,7 @@ milestone that consumes them.
 
 | # | Needed for | Decision | My recommendation |
 |---|---|---|---|
-| 1 | M2 | ~~Save medium~~ → **save disk (.dsk), asset supplied** | ⚠ decided, but **not working yet** — see the disk section in `../CLAUDE.md`. Sector I/O reports success without transferring. **The RTC CMOS fallback I previously suggested is not viable**: block 3 is 13 nibbles, and MSXgl's `RTC_SaveData()` stores **6 bytes**, against a ~40-byte `SaveState`. Real fallbacks are cartridge SRAM, the FM-PAC SRAM, or shrinking the save to a 6-byte fingerprint |
+| 1 | M2 | ~~Save medium~~ → **save disk (.dsk)** | ✅ working, via MSXgl's `tool/disk_save` module. Saves `KISSAT00.SAV` to drive A. Four non-obvious requirements — see the disk section in `../CLAUDE.md` |
 | 2 | M2 | ~~Day length~~ → **5 min / 5 customers** | ✅ done. One constant, `CUSTOMERS_PER_DAY` in `kissaten.c` |
 | 3 | M2 | ~~Clock model~~ → **event-driven** | ✅ done. Customers are the clock |
 | 4 | M3 | ~~Who writes the dialogue~~ → **drafted by me, you edit** | ✅ resolved |
@@ -510,8 +509,9 @@ project, and both want answering now.
    `../CLAUDE.md` for everything established so far.
 2. ✅ **Aki's beats as prose** — done, along with the other three arcs, in
    `SCRIPT.md`. Awaiting your edit pass.
-3. ⬜ **Unblock saving.** Either keep digging at the disk path, or switch
+3. ⬜ **M3 vertical slice** — Aki's first six beats, hardcoded, with a scratch
-   medium. Note the RTC CMOS fallback is off the table (§8 item 1).
+   portrait. The next real milestone, and the one that decides whether the
+   game works (§2, M3).
 4. ⬜ **Start the portrait question** (§5) — still the only item with external
    lead time, and now the longest pole by some distance.
 

projects/kissaten_yugure/kissaten.c

@@ -7,6 +7,7 @@
 // INCLUDES
 //=============================================================================
 #include "msxgl.h"
+#include "tool/disk_save.h"
 
 // Fonts data
 #include "font/font_mgl_sample6.h"
@@ -177,8 +178,7 @@ u8  g_SpacePrev = 0;
 u8  g_Phase = PHASE_MORNING;
 u8  g_DayServed = 0;		// customers served so far today
 u8  g_DayQuality = 0;		// summed brew quality today, for the journal line
-u8  g_DiskErr = 0;			// last disk error code (0 = fine)
+bool g_SaveOk = TRUE;		// did the last write succeed (shown next morning)
-bool g_HasDisk = FALSE;		// a usable save disk was found at boot
 
 //=============================================================================
 // SAVE STATE  (DESIGN.md §6)
@@ -213,134 +213,26 @@ typedef struct
 
 SaveState g_Save;
 
-// One 512-byte sector staging buffer. Must live in RAM (page 3) — PHYDIO
-// writes straight into it, and pages 1-2 are ROM on a cartridge target.
-u8 g_Sector[512];
-
-// Sector holding the save, derived from the disk's own BPB at boot rather
-// than hardcoded, so a 360K disk works as well as the 720K one.
-u16 g_SaveSector = 14;
-
 //=============================================================================
-// DISK ACCESS
+// DISK SAVE
 //=============================================================================
-
-// Number of disk drives, maintained by the Disk ROM at boot. Zero means there
-// is no disk interface at all — and on such a machine (C-BIOS, for one) the
-// PHYDIO hook is not patched and calling it is not safe.
-#define M_NMBDRV_ADDR	0xF347
-#define M_MASTER_ADDR	0xF348		// main Disk ROM slot, 0 if there is none
-
-// PHYDIO trampoline, executed from RAM.
 //
-// PHYDIO (Main-ROM 0144h) reaches the Disk ROM through an inter-slot call, and
+// Uses MSXgl's tool/disk_save module (added in v1.3.0, "save to disk from a
-// the Disk ROM lives at 4000h — the same page as this cartridge. For the
+// ROM application"). It writes a real named file — KISSAT0.SAV — through the
-// duration of the call our ROM is switched out of page 1, so the instruction
+// Disk ROM, so the save disk stays a normal, inspectable FAT disk.
-// making the call must not itself be in page 1. Copying these twelve bytes to
+//
-// RAM and calling *them* keeps the call site alive across the switch.
+// This replaced a hand-rolled PHYDIO/DSKIO sector writer that never worked:
-u8 g_DiskStub[21];
+// both entry points returned carry-clear without transferring anything. Do
+// not go back to raw sectors; use this module.
+//
+// Requirements, both non-obvious:
+//   - LibModules must include BOTH "tool/disk_save" and "dos".
+//   - ROMDelayBoot = true, or the Disk ROM's INIT never runs (see CLAUDE.md).
 
-void Disk_InitStub()
+// Save slot. Multiple save files would be entries 1, 2, ... on the same disk.
-{
+#define SAVE_ENTRY		0
-	// Calls DSKIO (Disk ROM 4010h) through CALSLT rather than the Main ROM's
-	// PHYDIO wrapper at 0144h: PHYDIO jumps to a RAM hook that this machine
-	// leaves unpatched, so it returns carry-clear — reporting success while
-	// never touching the buffer. Going straight at the Disk ROM avoids it.
-	//
-	// IY high byte must hold the slot ID, and MASTER lives at F348h, so a
-	// word load from F347h lands it in IYh exactly as CALSLT wants.
-	static const u8 code[21] =
-	{
-		0xF3,							// di
-		0xFD, 0x2A, 0x47, 0xF3,			// ld iy, (0xF347) ; IYh = MASTER
-		0xDD, 0x21, 0x10, 0x40,			// ld ix, #0x4010  ; DSKIO
-		0xCD, 0x1C, 0x00,				// call 0x001C     ; CALSLT
-		0x38, 0x04,						// jr c, err
-		0x2E, 0x00,						// ld l, #0        ; success
-		0xFB,							// ei
-		0xC9,							// ret
-		0x6F,							// err: ld l, a    ; BIOS error code
-		0xFB,							// ei
-		0xC9,							// ret
-	};
-	Mem_Copy(code, g_DiskStub, sizeof(code));
-}
 
-//-----------------------------------------------------------------------------
+bool g_HasDisk = FALSE;		// a usable save disk was found at boot
-// Absolute sector read/write into g_Sector.
-//   carry clear = read, carry set = write
-//   A=drive B=sector count C=media ID DE=first sector HL=buffer
-// Returns the BIOS error code, or 0 on success.
-u8 Disk_Access(u16 sector, u8 write) __NAKED
-{
-	sector;	// on stack
-	write;
-
-__asm
-	push	ix
-	ld		ix, #0
-	add		ix, sp
-
-	ld		e, 4(ix)			// sector low
-	ld		d, 5(ix)			// sector high
-	ld		a, 6(ix)			// write flag
-
-	ld		hl, #_g_Sector
-	ld		b, #1				// one sector
-	ld		c, #0xF9			// media ID: 720K DS/DD
-	or		a					// test flag; also clears carry (= read)
-	jr		z, phydio_go
-	scf							// carry set = write
-phydio_go:
-	ld		a, #0				// drive A:. 'ld a,n' preserves flags
-	call	_g_DiskStub			// -> PHYDIO, from RAM. Returns code in L
-
-	pop		ix
-	ret
-__endasm;
-}
-
-//-----------------------------------------------------------------------------
-// Read the BIOS Parameter Block and work out where the data area starts.
-// Returns FALSE if no disk responded at all.
-bool Disk_Probe()
-{
-	u8 drives = *(const u8*)M_NMBDRV_ADDR;
-
-	// Both of these are set by the Disk ROM's INIT. Without them the entry
-	// points below are not safe to call — on C-BIOS this work area is never
-	// initialised at all, and calling anyway black-screens the machine.
-	if (drives == 0 || drives > 8)
-		return FALSE;
-	if (*(const u8*)M_MASTER_ADDR == 0)
-		return FALSE;
-
-	Disk_InitStub();
-
-	// Sentinel: the disk entry points can return carry-clear ("success")
-	// without having touched the buffer, so a clean error code is not on its
-	// own proof that a read happened. Verify the buffer actually changed.
-	Mem_Set(0xAA, g_Sector, 16);
-	g_DiskErr = Disk_Access(0, 0);
-	if (g_DiskErr != 0)
-		return FALSE;
-	if (g_Sector[0] == 0xAA && g_Sector[11] == 0xAA)
-		return FALSE;	// nothing was read; treat the disk as unusable
-
-	u16 bytesPerSec  = g_Sector[11] | ((u16)g_Sector[12] << 8);
-	u16 reserved     = g_Sector[14] | ((u16)g_Sector[15] << 8);
-	u8  numFATs      = g_Sector[16];
-	u16 rootEntries  = g_Sector[17] | ((u16)g_Sector[18] << 8);
-	u16 secPerFAT    = g_Sector[22] | ((u16)g_Sector[23] << 8);
-
-	if (bytesPerSec != 512)		// anything else is not a disk we made
-		return FALSE;
-
-	// Root directory occupies ceil(entries * 32 / 512) sectors
-	u16 rootSecs = (u16)((rootEntries + 15) >> 4);
-	g_SaveSector = reserved + (u16)numFATs * secPerFAT + rootSecs;
-	return TRUE;
-}
 
 //-----------------------------------------------------------------------------
 u8 Save_Checksum(const SaveState* s)
@@ -352,25 +244,42 @@ u8 Save_Checksum(const SaveState* s)
 	return (u8)(0 - sum);
 }
 
+//-----------------------------------------------------------------------------
+// A missing drive or a blank disk is not an error the player has to deal
+// with — it just means nothing will persist.
+void Disk_Init()
+{
+	DiskSave_SetName("KISSAT");		// 6 chars max
+	DiskSave_SetExtension("SAV");	// 3 chars max
+	g_HasDisk = (DiskSave_Initialize() == SAVEDATA_VALID);
+}
+
 //-----------------------------------------------------------------------------
 // TRUE if a valid save was loaded; FALSE means "start a new game", which is
-// also what a blank disk gives us — no error path for the player to hit.
+// also what a blank disk gives us.
 bool Save_Load()
 {
 	if (!g_HasDisk)
 		return FALSE;
 
-	g_DiskErr = Disk_Access(g_SaveSector, 0);
+	// SAVEDATA_UNSIGNED is expected, not an error: with AppSignature on,
-	if (g_DiskErr != 0)
+	// DiskSave_Check() wants the file's first 4 bytes to be g_AppSignature,
+	// but DiskSave_Save() writes the payload raw and never adds it — so a
+	// perfectly good file always reports UNSIGNED. Only a genuinely absent or
+	// unreadable file is a reason to bail; the magic and checksum below are a
+	// stronger check than the signature would have been anyway.
+	u8 status = DiskSave_Check(SAVE_ENTRY);
+	if ((status != SAVEDATA_VALID) && (status != SAVEDATA_UNSIGNED))
 		return FALSE;
 
-	SaveState* s = (SaveState*)g_Sector;
+	if (!DiskSave_Load(SAVE_ENTRY, (u8*)&g_Save, sizeof(SaveState)))
-	if (s->magic != SAVE_MAGIC || s->version != SAVE_VERSION)
-		return FALSE;
-	if (s->checksum != Save_Checksum(s))
 		return FALSE;
 
-	Mem_Copy(s, &g_Save, sizeof(SaveState));
+	// Trust nothing that came off a disk
+	if (g_Save.magic != SAVE_MAGIC || g_Save.version != SAVE_VERSION)
+		return FALSE;
+	if (g_Save.checksum != Save_Checksum(&g_Save))
+		return FALSE;
 	return TRUE;
 }
 
@@ -384,11 +293,7 @@ bool Save_Write()
 	g_Save.version  = SAVE_VERSION;
 	g_Save.checksum = Save_Checksum(&g_Save);
 
-	Mem_Set(0, g_Sector, sizeof(g_Sector));
+	return DiskSave_Save(SAVE_ENTRY, (const u8*)&g_Save, sizeof(SaveState));
-	Mem_Copy(&g_Save, g_Sector, sizeof(SaveState));
-
-	g_DiskErr = Disk_Access(g_SaveSector, 1);
-	return (g_DiskErr == 0);
 }
 
 //-----------------------------------------------------------------------------
@@ -605,6 +510,10 @@ void EnterMorning()
 	Dlg_Clear();
 	Dlg_Line(0, "Morning. The shutters go");
 	Dlg_Line(1, "up, the kettle goes on.");
+	if (!g_HasDisk)
+		Dlg_Line(2, "(no save disk in drive A)");
+	else if (!g_SaveOk)
+		Dlg_Line(2, "(save failed)");
 	Dlg_Prompt("[SPACE] open the shop");
 }
 
@@ -630,18 +539,17 @@ void EnterEvening()
 	else
 		Dlg_Line(1, "Tomorrow, a steadier hand.");
 
-	if (Save_Write())
+	Dlg_Prompt("[SPACE] sleep");
-		Dlg_Prompt("[SPACE] sleep  (saved)");
-	else if (!g_HasDisk)
-		Dlg_Prompt("[SPACE] sleep  (no disk)");
-	else
-		Dlg_Prompt("[SPACE] sleep  (SAVE FAILED)");
 }
 
 //-----------------------------------------------------------------------------
+// Save *after* the day rolls over, so the file always describes the morning
+// the player will wake up to. Saving during the evening instead would make a
+// reload replay the day just finished and double-count its cups.
 void AdvanceDay()
 {
 	g_Save.day++;
+	g_SaveOk = Save_Write();
 	EnterMorning();
 }
 
@@ -861,13 +769,10 @@ void main()
 	g_Frame = 0;
 	g_SpaceHit = FALSE;
 	g_SpacePrev = 1;	// treat SPACE as held at boot: no phantom first edge
-	g_DiskErr = 0;
-	g_SaveSector = 14;
-
 	// Save disk. A missing or blank disk is not an error the player has to
 	// deal with — it just means today is day one and nothing will persist.
 	Save_Reset();
-	g_HasDisk = Disk_Probe();
+	Disk_Init();
 	Save_Load();
 
 	EnterMorning();

projects/kissaten_yugure/project_config.js

@@ -4,7 +4,9 @@
 // ASCII-safe name: used for output filenames (ROM, symbols, intermediates)
 ProjName    = "kissaten";
 ProjModules = [ ProjName ];
-LibModules  = [ "system", "bios", "vdp", "print", "input", "memory", "math" ];
+// "tool/disk_save" needs "dos" alongside it — see projects/samples/s_save.js
+LibModules  = [ "system", "bios", "vdp", "print", "input", "memory", "math",
+                "tool/disk_save", "dos" ];
 
 Machine = "2";