Feature: breaking: allow excess sets option

src/modules/argumentsParser.ts

@@ -567,6 +567,11 @@ export default class ArgumentsParser {
         requiresArg: true,
         default: MergeMode[MergeMode.FULLNONMERGED].toLowerCase(),
       })
+      .option('allow-excess-sets', {
+        group: groupRomSet,
+        description: 'Allow writing archives that have excess files when not extracting or zipping',
+        type: 'boolean',
+      })
       .option('allow-incomplete-sets', {
         group: groupRomSet,
         description: 'Allow writing games that don\'t have all of their ROMs',

src/modules/candidateGenerator.ts

@@ -261,6 +261,15 @@ export default class CandidateGenerator extends Module {
       return undefined;
     }
 
+    // If the found files have excess and we aren't allowing it, then return no candidate
+    if (!this.options.shouldZip()
+      && !this.options.shouldExtract()
+      && !this.options.getAllowExcessSets()
+      && this.hasExcessFiles(dat, game, foundRomsWithFiles, indexedFiles)
+    ) {
+      return undefined;
+    }
+
     return new ReleaseCandidate(game, release, foundRomsWithFiles);
   }
 
@@ -288,8 +297,8 @@ export default class CandidateGenerator extends Module {
     // Group this Game's ROMs by the input Archives that contain them
     const inputArchivesToRoms = romsAndInputFiles.reduce((map, [rom, files]) => {
       files
-        .filter((file) => file instanceof ArchiveEntry)
-        .map((file): Archive => (file as ArchiveEntry<never>).getArchive())
+        .filter((file): file is ArchiveEntry<Archive> => file instanceof ArchiveEntry)
+        .map((archive): Archive => archive.getArchive())
         .forEach((archive) => {
           const roms = map.get(archive) ?? [];
           roms.push(rom);
@@ -315,7 +324,29 @@ export default class CandidateGenerator extends Module {
           && CandidateGenerator.onlyCueFilesMissingFromChd(game, roms);
       })
       .map(([archive]) => archive);
-    const archiveWithEveryRom = archivesWithEveryRom.at(0);
+
+    const filesByPath = indexedFiles.getFilesByFilePath();
+    const filteredArchivesWithEveryRom = archivesWithEveryRom
+      // Sort the Archives such that the Archive with the least number of entries is preferred
+      .sort((a, b) => {
+        const aEntries = (filesByPath.get(a.getFilePath()) ?? []).length;
+        const bEntries = (filesByPath.get(b.getFilePath()) ?? []).length;
+        return aEntries - bEntries;
+      })
+      // Filter out Archives with excess entries
+      .filter((archive) => {
+        const unusedEntryPaths = this.findArchiveUnusedEntryPaths(
+          archive,
+          romsAndInputFiles.flatMap(([, inputFiles]) => inputFiles),
+          indexedFiles,
+        );
+        if (unusedEntryPaths.length > 0) {
+          this.progressBar.logTrace(`${dat.getNameShort()}: ${game.getName()}: not preferring archive that contains every ROM, plus the excess entries:\n${unusedEntryPaths.map((entryPath) => `  ${entryPath}`).join('\n')}`);
+        }
+        return unusedEntryPaths.length === 0;
+      });
+
+    const archiveWithEveryRom = filteredArchivesWithEveryRom.at(0);
 
     // Do nothing if any of...
     if (
@@ -494,4 +525,71 @@ export default class CandidateGenerator extends Module {
       .filter((rom) => path.extname(rom.getName()).toLowerCase() !== '.cue');
     return missingCueRoms.length > 0 && missingNonCueRoms.length === 0;
   }
+
+  private hasExcessFiles(
+    dat: DAT,
+    game: Game,
+    romsWithFiles: ROMWithFiles[],
+    indexedFiles: IndexedFiles,
+  ): boolean {
+    // For this Game, find every input file that is an ArchiveEntry
+    const inputArchiveEntries = romsWithFiles
+      // We need to rehydrate information from IndexedFiles because raw-copying/moving archives
+      // would have lost this information
+      .map((romWithFiles) => {
+        const inputFile = romWithFiles.getInputFile();
+        return indexedFiles.findFiles(romWithFiles.getRom())
+          ?.find((foundFile) => foundFile.getFilePath() === inputFile.getFilePath());
+      })
+      .filter((inputFile): inputFile is ArchiveEntry<Archive> => inputFile instanceof ArchiveEntry);
+    // ...then translate those ArchiveEntries into a list of unique Archives
+    const inputArchives = inputArchiveEntries
+      .map((archiveEntry) => archiveEntry.getArchive())
+      .filter(ArrayPoly.filterUniqueMapped((archive) => archive.getFilePath()));
+
+    for (const inputArchive of inputArchives) {
+      const unusedEntryPaths = this.findArchiveUnusedEntryPaths(
+        inputArchive,
+        inputArchiveEntries,
+        indexedFiles,
+      );
+      if (unusedEntryPaths.length > 0) {
+        this.progressBar.logTrace(`${dat.getNameShort()}: ${game.getName()}: cannot use '${inputArchive.getFilePath()}' as an input file, it has the excess entries:\n${unusedEntryPaths.map((entryPath) => `  ${entryPath}`).join('\n')}`);
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  /**
+   * Given an input {@link archive} and a set of {@link inputFiles} that match to a {@link ROM} from
+   * a {@link Game}, determine if every entry from the {@link archive} was matched.
+   */
+  private findArchiveUnusedEntryPaths(
+    archive: Archive,
+    inputFiles: File[],
+    indexedFiles: IndexedFiles,
+  ): ArchiveEntry<Archive>[] {
+    if (this.options.shouldZip()
+      || this.options.shouldExtract()
+      || this.options.getAllowExcessSets()
+    ) {
+      // We don't particularly care where input files come from
+      return [];
+    }
+
+    // Find the Archive's entries (all of them, not just ones that match ROMs in this Game)
+    // NOTE(cemmer): we need to use hashCode() because a Game may have duplicate ROMs that all got
+    //  matched to the same input file, so not every archive entry may be in {@link inputFiles}
+    const archiveEntryHashCodes = new Set(inputFiles
+      .filter((entry) => entry.getFilePath() === archive.getFilePath())
+      .filter((file): file is ArchiveEntry<Archive> => file instanceof ArchiveEntry)
+      .map((entry) => entry.hashCode()));
+
+    // Find which of the Archive's entries didn't match to a ROM from this Game
+    return (indexedFiles.getFilesByFilePath().get(archive.getFilePath()) ?? [])
+      .filter((file): file is ArchiveEntry<Archive> => file instanceof ArchiveEntry)
+      .filter((entry) => !archiveEntryHashCodes.has(entry.hashCode()));
+  }
 }

src/types/indexedFiles.ts

@@ -136,6 +136,15 @@ export default class IndexedFiles {
       .filter(ArrayPoly.filterUniqueMapped((file) => file.toString()));
   }
 
+  @Memoize()
+  getFilesByFilePath(): Map<string, File[]> {
+    return this.getFiles().reduce((map, file) => {
+      const existingFiles = map.get(file.getFilePath()) ?? [];
+      map.set(file.getFilePath(), [...existingFiles, file]);
+      return map;
+    }, new Map<string, File[]>());
+  }
+
   getSize(): number {
     return this.getFiles().length;
   }

src/types/options.ts

@@ -108,6 +108,7 @@ export interface OptionsProps {
   readonly removeHeaders?: string[],
 
   readonly mergeRoms?: string,
+  readonly allowExcessSets?: boolean,
   readonly allowIncompleteSets?: boolean,
 
   readonly filterRegex?: string,
@@ -256,6 +257,8 @@ export default class Options implements OptionsProps {
 
   readonly mergeRoms?: string;
 
+  readonly allowExcessSets: boolean;
+
   readonly allowIncompleteSets: boolean;
 
   readonly filterRegex: string;
@@ -423,6 +426,7 @@ export default class Options implements OptionsProps {
     this.removeHeaders = options?.removeHeaders;
 
     this.mergeRoms = options?.mergeRoms;
+    this.allowExcessSets = options?.allowExcessSets ?? false;
     this.allowIncompleteSets = options?.allowIncompleteSets ?? false;
 
     this.filterRegex = options?.filterRegex ?? '';
@@ -1026,9 +1030,12 @@ export default class Options implements OptionsProps {
     return MergeMode[mergeMode as keyof typeof MergeMode];
   }
 
+  getAllowExcessSets(): boolean {
+    return this.allowExcessSets;
+  }
+
   getAllowIncompleteSets(): boolean {
-    // If we're only reading, then go ahead and report on incomplete sets
-    return this.allowIncompleteSets || !this.shouldWrite();
+    return this.allowIncompleteSets;
   }
 
   getFilterRegex(): RegExp[] | undefined {

test/igir.test.ts

@@ -177,9 +177,8 @@ describe('with explicit DATs', () => {
         [`${path.join('One', 'GD-ROM.chd')}|track03.bin`, '61a363f1'],
         [`${path.join('One', 'GD-ROM.chd')}|track04.bin`, 'fc5ff5a0'],
         [`${path.join('One', 'Lorem Ipsum.zip')}|loremipsum.rom`, '70856527'],
-        [`${path.join('One', 'One Three.zip')}|${path.join('1', 'one.rom')}`, 'f817a89f'],
-        [`${path.join('One', 'One Three.zip')}|${path.join('2', 'two.rom')}`, '96170874'],
-        [`${path.join('One', 'One Three.zip')}|${path.join('3', 'three.rom')}`, 'ff46c5d8'],
+        [path.join('One', 'One Three', 'One.rom'), 'f817a89f'],
+        [path.join('One', 'One Three', 'Three.rom'), 'ff46c5d8'],
         [path.join('One', 'Three Four Five', 'Five.rom'), '3e5daf67'],
         [path.join('One', 'Three Four Five', 'Four.rom'), '1cf3ca74'],
         [path.join('One', 'Three Four Five', 'Three.rom'), 'ff46c5d8'],
@@ -230,9 +229,8 @@ describe('with explicit DATs', () => {
         ['GD-ROM.chd|track03.bin', '61a363f1'],
         ['GD-ROM.chd|track04.bin', 'fc5ff5a0'],
         ['Lorem Ipsum.zip|loremipsum.rom', '70856527'],
-        [`${path.join('One Three.zip')}|${path.join('1', 'one.rom')}`, 'f817a89f'],
-        [`${path.join('One Three.zip')}|${path.join('2', 'two.rom')}`, '96170874'],
-        [`${path.join('One Three.zip')}|${path.join('3', 'three.rom')}`, 'ff46c5d8'],
+        [path.join('One Three', 'One.rom'), 'f817a89f'],
+        [path.join('One Three', 'Three.rom'), 'ff46c5d8'],
         [path.join('Three Four Five', 'Five.rom'), '3e5daf67'],
         [path.join('Three Four Five', 'Four.rom'), '1cf3ca74'],
         [path.join('Three Four Five', 'Three.rom'), 'ff46c5d8'],
@@ -295,6 +293,8 @@ describe('with explicit DATs', () => {
         [path.join('nes', 'smdb', 'Hardware Target Game Database', 'Dummy', 'Fizzbuzz.nes'), '370517b5'],
         ['one.rom', '00000000'], // explicitly not deleted, it is not in an extension subdirectory
         [`${path.join('rar', 'Headered', 'LCDTestROM.lnx.rar')}|LCDTestROM.lnx`, '2d251538'],
+        [path.join('rom', 'One', 'One Three', 'One.rom'), 'f817a89f'],
+        [path.join('rom', 'One', 'One Three', 'Three.rom'), 'ff46c5d8'],
         [path.join('rom', 'One', 'Three Four Five', 'Five.rom'), '3e5daf67'],
         [path.join('rom', 'One', 'Three Four Five', 'Four.rom'), '1cf3ca74'],
         [path.join('rom', 'One', 'Three Four Five', 'Three.rom'), 'ff46c5d8'],
@@ -313,9 +313,6 @@ describe('with explicit DATs', () => {
         [path.join('smc', 'Headered', 'speed_test_v51.smc'), '9adca6cc'],
         [`${path.join('zip', 'Headered', 'fds_joypad_test.fds.zip')}|fds_joypad_test.fds`, '1e58456d'],
         [`${path.join('zip', 'One', 'Lorem Ipsum.zip')}|loremipsum.rom`, '70856527'],
-        [`${path.join('zip', 'One', 'One Three.zip')}|${path.join('1', 'one.rom')}`, 'f817a89f'],
-        [`${path.join('zip', 'One', 'One Three.zip')}|${path.join('2', 'two.rom')}`, '96170874'],
-        [`${path.join('zip', 'One', 'One Three.zip')}|${path.join('3', 'three.rom')}`, 'ff46c5d8'],
       ]);
       expect(result.cwdFilesAndCrcs).toHaveLength(0);
       expect(result.movedFiles).toHaveLength(0);
@@ -529,14 +526,15 @@ describe('with explicit DATs', () => {
     });
   });
 
-  it('should move zipped files', async () => {
+  it('should move zipped files, allowing excess sets', async () => {
     await copyFixturesToTemp(async (inputTemp, outputTemp) => {
       const result = await runIgir({
         commands: ['move'],
         dat: [path.join(inputTemp, 'dats')],
         input: [path.join(inputTemp, 'roms', 'zip')],
         output: outputTemp,
         dirDatName: true,
+        allowExcessSets: true,
       });
 
       expect(result.outputFilesAndCrcs).toEqual([
@@ -727,9 +725,8 @@ describe('with explicit DATs', () => {
         [`${path.join('One', 'GD-ROM.chd')}|track03.bin -> ${path.join('<input>', 'chd', 'GD-ROM.chd')}|track03.bin`, '61a363f1'],
         [`${path.join('One', 'GD-ROM.chd')}|track04.bin -> ${path.join('<input>', 'chd', 'GD-ROM.chd')}|track04.bin`, 'fc5ff5a0'],
         [`${path.join('One', 'Lorem Ipsum.zip')}|loremipsum.rom -> ${path.join('<input>', 'zip', 'loremipsum.zip')}|loremipsum.rom`, '70856527'],
-        [`${path.join('One', 'One Three.zip')}|${path.join('1', 'one.rom')} -> ${path.join('<input>', 'zip', 'onetwothree.zip')}|${path.join('1', 'one.rom')}`, 'f817a89f'],
-        [`${path.join('One', 'One Three.zip')}|${path.join('2', 'two.rom')} -> ${path.join('<input>', 'zip', 'onetwothree.zip')}|${path.join('2', 'two.rom')}`, '96170874'],
-        [`${path.join('One', 'One Three.zip')}|${path.join('3', 'three.rom')} -> ${path.join('<input>', 'zip', 'onetwothree.zip')}|${path.join('3', 'three.rom')}`, 'ff46c5d8'],
+        [`${path.join('One', 'One Three', 'One.rom')} -> ${path.join('<input>', 'raw', 'one.rom')}`, 'f817a89f'],
+        [`${path.join('One', 'One Three', 'Three.rom')} -> ${path.join('<input>', 'raw', 'three.rom')}`, 'ff46c5d8'],
         [`${path.join('One', 'Three Four Five', 'Five.rom')} -> ${path.join('<input>', 'raw', 'five.rom')}`, '3e5daf67'],
         [`${path.join('One', 'Three Four Five', 'Four.rom')} -> ${path.join('<input>', 'raw', 'four.rom')}`, '1cf3ca74'],
         [`${path.join('One', 'Three Four Five', 'Three.rom')} -> ${path.join('<input>', 'raw', 'three.rom')}`, 'ff46c5d8'],

test/modules/argumentsParser.test.ts

@@ -154,6 +154,8 @@ describe('options', () => {
     expect(options.getSymlinkRelative()).toEqual(false);
 
     expect(options.getMergeRoms()).toEqual(MergeMode.FULLNONMERGED);
+    expect(options.getAllowExcessSets()).toEqual(false);
+    expect(options.getAllowIncompleteSets()).toEqual(false);
 
     expect(options.getFilterRegex()).toBeUndefined();
     expect(options.getFilterRegexExclude()).toBeUndefined();
@@ -835,6 +837,15 @@ describe('options', () => {
     expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--merge-roms', 'merged', '--merge-roms', 'split']).getMergeRoms()).toEqual(MergeMode.SPLIT);
   });
 
+  it('should parse "allow-excess-sets"', () => {
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--allow-excess-sets']).getAllowExcessSets()).toEqual(true);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--allow-excess-sets', 'true']).getAllowExcessSets()).toEqual(true);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--allow-excess-sets', 'false']).getAllowExcessSets()).toEqual(false);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--allow-excess-sets', '--allow-excess-sets']).getAllowExcessSets()).toEqual(true);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--allow-excess-sets', 'false', '--allow-excess-sets', 'true']).getAllowExcessSets()).toEqual(true);
+    expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--allow-excess-sets', 'true', '--allow-excess-sets', 'false']).getAllowExcessSets()).toEqual(false);
+  });
+
   it('should parse "allow-incomplete-sets"', () => {
     expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--allow-incomplete-sets']).getAllowIncompleteSets()).toEqual(true);
     expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--allow-incomplete-sets', 'true']).getAllowIncompleteSets()).toEqual(true);