fix: point VMCDIR.TXT at disc 1's folder, not its VCD file

internal/app/app_test.go

@@ -206,8 +206,8 @@ func TestInstallMultiDiscPS1(t *testing.T) {
 			if !strings.Contains(string(body), "_CD1.VCD") || !strings.Contains(string(body), "_CD2.VCD") {
 				t.Errorf("%s/DISCS.TXT does not list both discs: %q", dir, body)
 			}
-			// VMCDIR.TXT goes in the later discs only, naming disc 1's VCD, so
-			// that a save made on disc 1 is there after the swap.
+			// VMCDIR.TXT goes in the later discs only, naming disc 1's support
+			// directory, so that a save made on disc 1 is there after the swap.
 			vmc := filepath.Join(mp, "POPS", dir, "VMCDIR.TXT")
 			got, err := os.ReadFile(vmc)
 			if i == 0 {
@@ -219,8 +219,13 @@ func TestInstallMultiDiscPS1(t *testing.T) {
 			if err != nil {
 				return err
 			}
-			if !strings.Contains(string(got), "_CD1.VCD") {
-				t.Errorf("%s/VMCDIR.TXT = %q, want disc 1's VCD", dir, got)
+			// A directory name, with no extension. The .VCD form looks right
+			// and silently costs the player their save at the disc change.
+			if strings.Contains(string(got), ".VCD") {
+				t.Errorf("%s/VMCDIR.TXT = %q: it must name disc 1's directory, not its VCD file", dir, got)
+			}
+			if !strings.Contains(string(got), "_CD1") {
+				t.Errorf("%s/VMCDIR.TXT = %q, want disc 1's support directory", dir, got)
 			}
 		}
 		return nil

internal/app/install.go

@@ -590,6 +590,12 @@ func (s *Services) installPS1(ctx context.Context, g model.Game, opts InstallOpt
 				"`ps2hdd setup ps1 --launchers`; the game itself does not need reinstalling.",
 			ps1.POPStarterELF, ps1.CommonPartition, ps1.POPSDir))
 	}
+	if total > ps1.MaxDiscsInDiscsFile {
+		rep.Warnings = append(rep.Warnings, fmt.Sprintf(
+			"%s has %d discs, and DISCS.TXT describes at most %d. Every disc is installed and "+
+				"each will boot from its own launcher, but the in-game disc-swap menu will not "+
+				"work for this title.", g.Title, total, ps1.MaxDiscsInDiscsFile))
+	}
 	if !ps1.BootNameFitsOPL(launcherELF) {
 		rep.Warnings = append(rep.Warnings, fmt.Sprintf(
 			"%q is longer than the 64 characters OPL allows for a boot filename, so the entry "+

internal/platform/ps1/pops.go

@@ -73,8 +73,18 @@ const (
 	// a copy, so it is taken from the runtime the user installed.
 	POPStarterELF = "POPSTARTER.ELF"
 
-	// maxVCDNameLen is POPStarter's filename limit.
-	maxVCDNameLen = 89
+	// maxVCDNameLen is the longest VCD filename POPStarter reliably handles.
+	//
+	// Its wiki says 89, but the buffer DISCS.TXT paths are read into caps a
+	// name at around 73, and exceeding that breaks disc swapping rather than
+	// failing loudly. The smaller number is used for every title: a name is
+	// only an identifier, and a shorter one costs nothing next to a
+	// multi-disc game that cannot change discs.
+	maxVCDNameLen = 73
+
+	// MaxDiscsInDiscsFile is how many discs DISCS.TXT can describe. POPStarter
+	// reads four lines and the feature breaks beyond that.
+	MaxDiscsInDiscsFile = 4
 )
 
 // RuntimeFile is a component of the POPS runtime.
@@ -337,10 +347,16 @@ func GameDirName(vcdName string) string {
 	return strings.TrimSuffix(vcdName, filepath.Ext(vcdName))
 }
 
-// VMCDirContents renders a VMCDIR.TXT body: the VCD filename whose memory card
-// this disc should share.
+// VMCDirContents renders a VMCDIR.TXT body: the name of the support directory
+// whose memory card this disc should share.
+//
+// It is a directory name, not a filename, and carries no extension --
+// "SLUS_005.94.Metal Gear Solid_CD1", never "...CD1.VCD". POPStarter looks for
+// a folder of that name beside this one under __common/POPS; given a name it
+// cannot find, it silently falls back to giving this disc a card of its own,
+// and a save made on disc 1 is then invisible on disc 2.
 func VMCDirContents(firstDiscVCD string) string {
-	return firstDiscVCD + "\n"
+	return GameDirName(firstDiscVCD) + "\n"
 }
 
 // DiscsFileContents renders the DISCS.TXT body listing a title's VCDs in disc

internal/platform/ps1/pops_test.go

@@ -3,6 +3,7 @@ package ps1_test
 import (
 	"crypto/sha256"
 	"encoding/hex"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -242,13 +243,27 @@ func TestDiscsFile(t *testing.T) {
 	}
 }
 
-// VMCDIR.TXT names the VCD whose memory card the disc shares, one line. A
-// documented way to get this wrong is to put the title in it instead.
+// VMCDIR.TXT names the support DIRECTORY whose memory card the disc shares --
+// one line, no extension. Writing the VCD's filename there instead looks
+// almost right and fails silently: POPStarter cannot find a folder of that
+// name, gives the disc a card of its own, and the save made on disc 1 is
+// missing after the swap.
 func TestVMCDirContents(t *testing.T) {
 	first := ps1.VCDName("SLUS_005.94", "Metal Gear Solid", 1, 2)
-	if got, want := ps1.VMCDirContents(first), first+"\n"; got != want {
+	got := ps1.VMCDirContents(first)
+	if want := ps1.GameDirName(first) + "\n"; got != want {
 		t.Errorf("VMCDIR.TXT = %q, want %q", got, want)
 	}
+	if strings.Contains(got, ps1.VCDExt) {
+		t.Errorf("VMCDIR.TXT = %q: it names a directory, so it must not carry the .VCD extension", got)
+	}
+	// The documented limits: at most 103 bytes, and no path separators.
+	if len(got) > 103 {
+		t.Errorf("VMCDIR.TXT is %d bytes, over POPStarter's 103", len(got))
+	}
+	if strings.ContainsAny(got, `/\:`) {
+		t.Errorf("VMCDIR.TXT = %q contains a path separator, which POPStarter rejects", got)
+	}
 }
 
 func sha256Hex(s string) string {
@@ -327,3 +342,48 @@ func TestCheckRuntimeDoesNotHashThePOPStarterLauncher(t *testing.T) {
 		}
 	}
 }
+
+// A VCD name must fit the buffer DISCS.TXT paths are read into, or disc
+// swapping breaks without saying so.
+func TestVCDNameFitsTheDiscsFileBuffer(t *testing.T) {
+	long := strings.Repeat("The Longest Title Anyone Ever Shipped ", 5)
+	for disc := 1; disc <= 4; disc++ {
+		n := ps1.VCDName("SLUS_005.94", long, disc, 4)
+		if len(n) > 73 {
+			t.Errorf("disc %d name is %d characters: %q", disc, len(n), n)
+		}
+		// The disc suffix is what tells the discs apart and must survive
+		// truncation; a title cut back to a shared prefix would install four
+		// files with one name.
+		if !strings.Contains(n, fmt.Sprintf("_CD%d.VCD", disc)) {
+			t.Errorf("disc %d lost its suffix: %q", disc, n)
+		}
+	}
+}
+
+// Every line of DISCS.TXT is a VCD filename, extension included: POPStarter
+// opens them, unlike VMCDIR.TXT which names a folder.
+func TestDiscsFileListsFilenames(t *testing.T) {
+	var names []string
+	for i := 1; i <= 3; i++ {
+		names = append(names, ps1.VCDName("SCUS_941.6"+string(rune('2'+i)), "Final Fantasy VII", i, 3))
+	}
+	body := ps1.DiscsFileContents(names)
+	lines := strings.Split(strings.TrimRight(body, "\n"), "\n")
+	if len(lines) != 3 {
+		t.Fatalf("DISCS.TXT has %d lines, want one per disc: %q", len(lines), body)
+	}
+	for i, l := range lines {
+		if !strings.HasSuffix(l, ps1.VCDExt) {
+			t.Errorf("line %d is %q; every line names a VCD file", i+1, l)
+		}
+		if len(l) > 73 {
+			t.Errorf("line %d is %d characters, over the path buffer POPStarter reads it into", i+1, len(l))
+		}
+	}
+	// Line order is disc order: the swap combo picks a line number, not a
+	// disc label.
+	if !strings.Contains(lines[0], "_CD1") || !strings.Contains(lines[2], "_CD3") {
+		t.Errorf("DISCS.TXT is not in disc order: %q", body)
+	}
+}

scripts/demo-smoke.sh

@@ -288,8 +288,14 @@ test -f "$CD2/DISCS.TXT" || fail "disc 2 has no DISCS.TXT"
 grep -q "_CD2.VCD" "$CD1/DISCS.TXT" || fail "DISCS.TXT does not list both discs"
 # VMCDIR.TXT points the later discs at disc 1's card, or a save made on disc 1
 # is gone after the swap. Disc 1 owns the card and must not have one.
+#
+# It names disc 1's DIRECTORY, with no extension. The .VCD form looks almost
+# right and fails silently: POPStarter finds no folder of that name, gives the
+# disc a card of its own, and the save is missing at the disc change.
 test -f "$CD2/VMCDIR.TXT" || fail "disc 2 has no VMCDIR.TXT"
-grep -q "_CD1.VCD" "$CD2/VMCDIR.TXT" || fail "VMCDIR.TXT does not name disc 1"
+grep -q "_CD1" "$CD2/VMCDIR.TXT" || fail "VMCDIR.TXT does not name disc 1"
+grep -q "\.VCD" "$CD2/VMCDIR.TXT" \
+  && fail "VMCDIR.TXT names a file, not disc 1's directory: $(cat "$CD2/VMCDIR.TXT")" || true
 test -f "$CD1/VMCDIR.TXT" && fail "disc 1 was pointed at another card" || true
 test -e "$DEMO/partitions/pops/SLUS_005.94.Metal Gear Solid" \
   && fail "support files were written into __.POPS, where POPStarter does not look" || true