Fix: match .cue/.bin CHDs for merged multi-disc games (#2312)

emmercm · web-flow · commit 1f693032ec33 · 2026-06-21T01:39:12.000Z
diff --git a/src/models/dats/mergedDiscGame.ts b/src/models/dats/mergedDiscGame.ts
@@ -0,0 +1,45 @@
+import type { GameProps } from './game.js';
+import Game from './game.js';
+
+export interface MergedDiscGameProps extends GameProps {
+  readonly subGames: Game[];
+}
+
+/**
+ * A game merged from two or more disc-based games. It keeps the original per-disc games as
+ * first-class sub-games while still exposing their flattened ROMs to every other consumer.
+ */
+export default class MergedDiscGame extends Game {
+  private readonly subGames: Game[];
+
+  constructor(props: MergedDiscGameProps) {
+    // Seed the inherited `roms` field from the same ROM instances the sub-games hold, so that an
+    // object spread (`{ ...mergedDiscGame }`) and serialization both see every ROM. getRoms() below
+    // keeps the sub-games authoritative.
+    super({
+      ...props,
+      roms: props.subGames.flatMap((game) => game.getRoms()),
+    });
+    this.subGames = props.subGames;
+  }
+
+  /**
+   * Return the original per-disc games this merged game was built from.
+   */
+  getSubGames(): Game[] {
+    return this.subGames;
+  }
+
+  /**
+   * Return a new merged game with some different properties, preserving the sub-games.
+   */
+  override withProps(props: GameProps): MergedDiscGame {
+    // Note: any `roms` in `props` is ignored — the constructor always re-derives `roms` from
+    // `subGames`. Pipeline callers only ever change props such as `cloneOf`.
+    return new MergedDiscGame({
+      ...this,
+      ...props,
+      subGames: this.subGames,
+    });
+  }
+}
diff --git a/src/modules/candidates/candidateGenerator.ts b/src/modules/candidates/candidateGenerator.ts
@@ -9,6 +9,7 @@ import type FileFactory from '../../factories/fileFactory.js';
 import type DAT from '../../models/dats/dat.js';
 import Disk from '../../models/dats/disk.js';
 import type Game from '../../models/dats/game.js';
+import MergedDiscGame from '../../models/dats/mergedDiscGame.js';
 import type ROM from '../../models/dats/rom.js';
 import type Archive from '../../models/files/archives/archive.js';
 import ArchiveEntry from '../../models/files/archives/archiveEntry.js';
@@ -289,6 +290,17 @@ export default class CandidateGenerator extends Module {
       return new Map();
     }
 
+    if (game instanceof MergedDiscGame) {
+      // Resolve each disc independently so every disc is the single-archive case the code already
+      // handles correctly (including pulling an unknown .cue from the same CHD that holds its .bins).
+      return this.findOptimalInputFilesForMergedDiscGame(
+        dat,
+        game,
+        romsAndInputFiles,
+        indexedFiles,
+      );
+    }
+
     const archiveWithEveryRom = this.findArchiveFileWithEveryRomForGame(
       dat,
       game,
@@ -310,6 +322,71 @@ export default class CandidateGenerator extends Module {
     );
   }
 
+  /**
+   * Resolve input files for a {@link MergedDiscGame} one sub-game at a time. Any ROM not resolved
+   * to a single containing archive falls back to its first matched input file.
+   */
+  private findOptimalInputFilesForMergedDiscGame(
+    dat: DAT,
+    game: MergedDiscGame,
+    romsAndInputFiles: [ROM, File[]][],
+    indexedFiles: IndexedFiles,
+  ): Map<ROM, File> {
+    // All intermediate matching is keyed by a stable ROM name + hash code string rather than by ROM
+    // instance, so it survives any earlier reinstantiation
+    const entryKey = (rom: ROM): string => `${rom.getName()}|${rom.hashCode()}`;
+    const entriesByKey = new Map<string, [ROM, File[]]>(
+      romsAndInputFiles.map((romAndInputFiles) => [
+        entryKey(romAndInputFiles[0]),
+        romAndInputFiles,
+      ]),
+    );
+
+    // Resolve each sub-game's single containing archive, keyed by entryKey
+    const filesByKey = new Map<string, File>();
+    for (const subGame of game.getSubGames()) {
+      const subGameRoms = subGame.getRoms();
+      const subGameRomsAndInputFiles = subGameRoms
+        .map((subGameRom) => entriesByKey.get(entryKey(subGameRom)))
+        .filter((entry) => entry !== undefined);
+      if (subGameRomsAndInputFiles.length === 0) {
+        continue;
+      }
+
+      const archiveWithEveryRom = this.findArchiveFileWithEveryRomForGame(
+        dat,
+        subGame,
+        subGameRoms,
+        subGameRomsAndInputFiles,
+        indexedFiles,
+      );
+      if (archiveWithEveryRom === undefined) {
+        continue;
+      }
+      for (const [rom, inputFile] of archiveWithEveryRom) {
+        filesByKey.set(entryKey(rom), inputFile);
+      }
+    }
+
+    const resolved = new Map<ROM, File>();
+    for (const [rom, inputFiles] of romsAndInputFiles) {
+      const inputFile = filesByKey.get(entryKey(rom)) ?? inputFiles.at(0);
+      if (inputFile !== undefined) {
+        resolved.set(rom, inputFile);
+      }
+    }
+    return resolved;
+  }
+
+  /**
+   * Find a single input {@link Archive} that contains every one of a {@link Game}'s {@link ROM}s, and
+   * return a map from each ROM to its matching entry within that archive. Preferring one archive for
+   * the whole game avoids output-path conflicts when raw-copying and avoids leaving archives partially
+   * used when zipping. Returns `undefined` when extracting (the source archive doesn't matter) or when
+   * no single archive holds every ROM, leaving the caller to fall back to per-ROM matching. ROMs with
+   * no matching entry are omitted from the returned map, so it is always a `Map<ROM, File>` of only
+   * resolved ROMs.
+   */
   private findArchiveFileWithEveryRomForGame(
     dat: DAT,
     game: Game,
@@ -455,7 +532,7 @@ export default class CandidateGenerator extends Module {
     // An Archive was found, use that as the only possible input file
     // For each of this Game's ROMs, find the matching ArchiveEntry from this Archive
     return new Map(
-      romsAndInputFiles.map(([rom, inputFiles]) => {
+      romsAndInputFiles.flatMap(([rom, inputFiles]) => {
         const archiveEntries = inputFiles.filter(
           (inputFile) =>
             inputFile.getFilePath() === archiveWithEveryRom.getFilePath() &&
@@ -484,7 +561,7 @@ export default class CandidateGenerator extends Module {
             ?.find((file) => file.getExtractedFilePath().toLowerCase().endsWith('.cue'));
         }
 
-        return [rom, archiveEntry as ArchiveEntry<Archive>];
+        return archiveEntry === undefined ? [] : [[rom, archiveEntry]];
       }),
     );
   }
diff --git a/src/modules/dats/datDiscMerger.ts b/src/modules/dats/datDiscMerger.ts
@@ -3,7 +3,8 @@ import path from 'node:path';
 import type ProgressBar from '../../console/progressBar.js';
 import { ProgressBarSymbol } from '../../console/progressBar.js';
 import type DAT from '../../models/dats/dat.js';
-import Game from '../../models/dats/game.js';
+import type Game from '../../models/dats/game.js';
+import MergedDiscGame from '../../models/dats/mergedDiscGame.js';
 import type Options from '../../models/options.js';
 import IntlUtil from '../../utils/intlUtil.js';
 import Module from '../module.js';
@@ -69,21 +70,22 @@ export default class DATDiscMerger extends Module {
         .filter(([, count]) => count > 1)
         .map(([romName]) => romName)
         .toSorted();
-      if (duplicateRomNames.length > 1) {
-        // De-conflict the filenames by adding a subfolder of the original game's name
-        const deconflictedRoms = games.flatMap((game) =>
-          game.getRoms().map((rom) => rom.withName(path.join(game.getName(), rom.getName()))),
-        );
-        return new Game({
-          name: gameName,
-          roms: deconflictedRoms,
-          discMerged: true,
-        });
-      }
 
-      return new Game({
+      const subGames =
+        duplicateRomNames.length > 1
+          ? // De-conflict the filenames by adding a subfolder of the original game's name
+            games.map((game) =>
+              game.withProps({
+                roms: game
+                  .getRoms()
+                  .map((rom) => rom.withName(path.join(game.getName(), rom.getName()))),
+              }),
+            )
+          : games;
+
+      return new MergedDiscGame({
         name: gameName,
-        roms: roms,
+        subGames,
         discMerged: true,
       });
     });
diff --git a/test/models/dats/mergedDiscGame.test.ts b/test/models/dats/mergedDiscGame.test.ts
@@ -0,0 +1,53 @@
+import Game from '../../../src/models/dats/game.js';
+import MergedDiscGame from '../../../src/models/dats/mergedDiscGame.js';
+import ROM from '../../../src/models/dats/rom.js';
+import SingleValueGame from '../../../src/models/singleValueGame.js';
+
+const discOne = new Game({
+  name: 'Game (Disc 1)',
+  roms: [
+    new ROM({ name: 'Game (Disc 1).cue', size: 97, crc32: '11111111' }),
+    new ROM({ name: 'Game (Disc 1).bin', size: 100, crc32: '22222222' }),
+  ],
+});
+const discTwo = new Game({
+  name: 'Game (Disc 2)',
+  roms: [
+    new ROM({ name: 'Game (Disc 2).cue', size: 97, crc32: '33333333' }),
+    new ROM({ name: 'Game (Disc 2).bin', size: 200, crc32: '44444444' }),
+  ],
+});
+
+describe('getSubGames', () => {
+  it('should return the sub-games it was constructed with', () => {
+    const merged = new MergedDiscGame({ name: 'Game', subGames: [discOne, discTwo] });
+    expect(merged.getSubGames()).toEqual([discOne, discTwo]);
+  });
+});
+
+describe('getRoms', () => {
+  it('should return the flattened sub-game ROM instances', () => {
+    const merged = new MergedDiscGame({ name: 'Game', subGames: [discOne, discTwo] });
+    expect(merged.getRoms()).toEqual([...discOne.getRoms(), ...discTwo.getRoms()]);
+  });
+
+  it('should seed the inherited roms field so an object spread sees the flattened ROMs', () => {
+    const merged = new MergedDiscGame({ name: 'Game', subGames: [discOne, discTwo] });
+    // Several call sites build a SingleValueGame via `new SingleValueGame({ ...game })`, which
+    // reads the raw `roms` field, not getRoms(). The constructor must seed `super` from the
+    // sub-games for that spread to carry every ROM.
+    const singleValueGame = new SingleValueGame({ ...merged });
+    expect(singleValueGame.getRoms()).toEqual([...discOne.getRoms(), ...discTwo.getRoms()]);
+  });
+});
+
+describe('withProps', () => {
+  it('should return a MergedDiscGame that preserves sub-games and applies the new props', () => {
+    const merged = new MergedDiscGame({ name: 'Game', subGames: [discOne, discTwo] });
+    const reWrapped = merged.withProps({ cloneOf: 'Parent' });
+    expect(reWrapped).toBeInstanceOf(MergedDiscGame);
+    expect(reWrapped.getSubGames()).toEqual([discOne, discTwo]);
+    expect(reWrapped.getCloneOf()).toEqual('Parent');
+    expect(reWrapped.getRoms()).toEqual([...discOne.getRoms(), ...discTwo.getRoms()]);
+  });
+});
diff --git a/test/modules/candidates/candidateGenerator.test.ts b/test/modules/candidates/candidateGenerator.test.ts
diff --git a/test/modules/dats/datDiscMerger.test.ts b/test/modules/dats/datDiscMerger.test.ts
