diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 600501e..eb3f8b5 100644 --- a/internal/app/app_test.go +++ b/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 diff --git a/internal/app/install.go b/internal/app/install.go index e80c731..8fb9f0f 100644 --- a/internal/app/install.go +++ b/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 "+ diff --git a/internal/platform/ps1/pops.go b/internal/platform/ps1/pops.go index 01b62a3..ae1c5ec 100644 --- a/internal/platform/ps1/pops.go +++ b/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 diff --git a/internal/platform/ps1/pops_test.go b/internal/platform/ps1/pops_test.go index c936563..a9edbff 100644 --- a/internal/platform/ps1/pops_test.go +++ b/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) + } +} diff --git a/scripts/demo-smoke.sh b/scripts/demo-smoke.sh index edf9c61..8317652 100755 --- a/scripts/demo-smoke.sh +++ b/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