Feature: check for conflicting output file paths

src/igir.ts

@@ -18,6 +18,7 @@ import CandidateMergeSplitValidator from './modules/candidateMergeSplitValidator
 import CandidatePatchGenerator from './modules/candidatePatchGenerator.js';
 import CandidatePostProcessor from './modules/candidatePostProcessor.js';
 import CandidatePreferer from './modules/candidatePreferer.js';
+import CandidateValidator from './modules/candidateValidator.js';
 import CandidateWriter from './modules/candidateWriter.js';
 import DATCombiner from './modules/datCombiner.js';
 import DATFilter from './modules/datFilter.js';
@@ -407,6 +408,13 @@ export default class Igir {
     const postProcessedCandidates = await new CandidatePostProcessor(this.options, progressBar)
       .process(dat, hashedCandidates);
 
+    const invalidCandidates = await new CandidateValidator(progressBar)
+      .validate(dat, postProcessedCandidates);
+    if (invalidCandidates.length > 0) {
+      // Return zero candidates if any candidates failed to validate
+      return new Map();
+    }
+
     await new CandidateMergeSplitValidator(this.options, progressBar)
       .validate(dat, postProcessedCandidates);
 
@@ -460,7 +468,10 @@ export default class Igir {
     dirsToClean: string[],
     datsToWrittenFiles: Map<DAT, File[]>,
   ): Promise<string[]> {
-    if (!this.options.shouldWrite() || !this.options.shouldClean()) {
+    if (!this.options.shouldWrite()
+      || !this.options.shouldClean()
+      || dirsToClean.length === 0
+    ) {
       return [];
     }
 

src/modules/candidateGenerator.ts

@@ -228,7 +228,7 @@ export default class CandidateGenerator extends Module {
     }
 
     // Ignore the Game with conflicting input->output files
-    if (this.hasConflictingOutputFiles(foundRomsWithFiles)) {
+    if (this.hasConflictingOutputFiles(dat, foundRomsWithFiles)) {
       return undefined;
     }
 
@@ -384,7 +384,7 @@ export default class CandidateGenerator extends Module {
     this.progressBar.logTrace(message);
   }
 
-  private hasConflictingOutputFiles(romsWithFiles: ROMWithFiles[]): boolean {
+  private hasConflictingOutputFiles(dat: DAT, romsWithFiles: ROMWithFiles[]): boolean {
     // If we're not writing, then don't bother looking for conflicts
     if (!this.options.shouldWrite()) {
       return false;
@@ -418,7 +418,7 @@ export default class CandidateGenerator extends Module {
         .reduce(ArrayPoly.reduceUnique(), []);
       if (conflictedInputFiles.length > 1) {
         hasConflict = true;
-        let message = `No single archive contains all necessary files, cannot ${this.options.writeString()} these different input files to: ${duplicateOutput}:`;
+        let message = `${dat.getNameShort()}: no single archive contains all necessary files, cannot ${this.options.writeString()} these different input files to: ${duplicateOutput}:`;
         conflictedInputFiles.forEach((conflictedInputFile) => { message += `\n  ${conflictedInputFile}`; });
         this.progressBar.logWarn(message);
       }

src/modules/candidateMergeSplitValidator.ts

@@ -21,14 +21,18 @@ export default class CandidateMergeSplitValidator extends Module {
   }
 
   /**
-   * Validate the {@link ReleaseCandidate}s
+   * Validate the {@link ReleaseCandidate}s.
    */
   async validate(
     dat: DAT,
     parentsToCandidates: Map<Parent, ReleaseCandidate[]>,
   ): Promise<string[]> {
-    this.progressBar.logTrace(`${dat.getNameShort()}: validating merged & split ROM sets`);
+    if (parentsToCandidates.size === 0) {
+      this.progressBar.logTrace(`${dat.getNameShort()}: no parents to validate merged & split ROM sets for`);
+      return [];
+    }
 
+    this.progressBar.logTrace(`${dat.getNameShort()}: validating merged & split ROM sets`);
     await this.progressBar.setSymbol(ProgressBarSymbol.VALIDATING);
     await this.progressBar.reset(parentsToCandidates.size);
 

src/modules/candidateValidator.ts

@@ -0,0 +1,72 @@
+import ProgressBar, { ProgressBarSymbol } from '../console/progressBar.js';
+import ArrayPoly from '../polyfill/arrayPoly.js';
+import DAT from '../types/dats/dat.js';
+import Parent from '../types/dats/parent.js';
+import ReleaseCandidate from '../types/releaseCandidate.js';
+import Module from './module.js';
+
+/**
+ * Validate candidates for write-ability after all generation and filtering has happened.
+ */
+export default class CandidateValidator extends Module {
+  constructor(progressBar: ProgressBar) {
+    super(progressBar, CandidateValidator.name);
+  }
+
+  /**
+   * Validate the {@link ReleaseCandidate}s.
+   */
+  async validate(
+    dat: DAT,
+    parentsToCandidates: Map<Parent, ReleaseCandidate[]>,
+  ): Promise<ReleaseCandidate[]> {
+    if (parentsToCandidates.size === 0) {
+      this.progressBar.logTrace(`${dat.getNameShort()}: no parents to validate candidates for`);
+      return [];
+    }
+
+    this.progressBar.logTrace(`${dat.getNameShort()}: validating candidates`);
+    await this.progressBar.setSymbol(ProgressBarSymbol.VALIDATING);
+    await this.progressBar.reset(parentsToCandidates.size);
+
+    const conflictedOutputPaths = this.validateUniqueOutputPaths(dat, parentsToCandidates);
+    if (conflictedOutputPaths.length > 0) {
+      return conflictedOutputPaths;
+    }
+
+    this.progressBar.logTrace(`${dat.getNameShort()}: done validating candidates`);
+    return [];
+  }
+
+  private validateUniqueOutputPaths(
+    dat: DAT,
+    parentsToCandidates: Map<Parent, ReleaseCandidate[]>,
+  ): ReleaseCandidate[] {
+    const outputPathsToCandidates = [...parentsToCandidates.values()]
+      .flat()
+      .reduce((map, releaseCandidate) => {
+        releaseCandidate.getRomsWithFiles().forEach((romWithFiles) => {
+          const key = romWithFiles.getOutputFile().getFilePath();
+          map.set(key, [...(map.get(key) ?? []), releaseCandidate]);
+        });
+        return map;
+      }, new Map<string, ReleaseCandidate[]>());
+
+    return [...outputPathsToCandidates.entries()]
+      .filter(([outputPath, candidates]) => {
+        const uniqueCandidates = candidates
+          .filter(ArrayPoly.filterUniqueMapped((candidate) => candidate.getGame()))
+          .sort();
+        if (uniqueCandidates.length < 2) {
+          return false;
+        }
+
+        let message = `${dat.getNameShort()}: multiple games writing to the same output path: ${outputPath}`;
+        uniqueCandidates.forEach((candidate) => { message += `\n  ${candidate.getName()}`; });
+        this.progressBar.logError(message);
+        return true;
+      })
+      .flatMap(([, candidates]) => candidates)
+      .reduce(ArrayPoly.reduceUnique(), []);
+  }
+}

src/types/dats/game.ts

@@ -656,7 +656,7 @@ export default class Game implements GameProps {
    */
   hashCode(): string {
     let hashCode = this.getName();
-    hashCode += `|${this.getRoms().map((rom) => rom.hashCode()).join(',')}`;
+    hashCode += `|${this.getRoms().map((rom) => rom.hashCode()).sort().join(',')}`;
     return hashCode;
   }
 

test/igir.test.ts

@@ -396,6 +396,7 @@ describe('with explicit DATs', () => {
       const result = await runIgir({
         commands: ['move', 'extract', 'test'],
         dat: [path.join(inputTemp, 'dats', '*')],
+        datExclude: [path.join(inputTemp, 'dats', 'headerless.*')],
         input: [path.join(inputTemp, 'roms')],
         output: outputTemp,
         datCombine: true,
@@ -405,8 +406,6 @@ describe('with explicit DATs', () => {
       });
 
       expect(result.outputFilesAndCrcs).toEqual([
-        // Note: the "Headered" DAT is alphabetically before the "Headerless" DAT, so headered
-        //  ROMs of the same name are preferred.
         [path.join('igir combined', '0F09A40.rom'), '2f943e86'],
         [path.join('igir combined', '3708F2C.rom'), '20891c9f'],
         [path.join('igir combined', '612644F.rom'), 'f7591b29'],
@@ -429,11 +428,9 @@ describe('with explicit DATs', () => {
         [path.join('igir combined', 'Hardware Target Game Database', 'Patchable', 'C01173E.rom'), 'dfaebe28'],
         [path.join('igir combined', 'KDULVQN.rom'), 'b1c303e4'],
         [path.join('igir combined', 'LCDTestROM.lnx'), '2d251538'],
-        [path.join('igir combined', 'LCDTestROM.lyx'), '42583855'],
         [`${path.join('igir combined', 'Lorem Ipsum.zip')}|loremipsum.rom`, '70856527'],
         [path.join('igir combined', 'One Three', 'One.rom'), 'f817a89f'],
         [path.join('igir combined', 'One Three', 'Three.rom'), 'ff46c5d8'],
-        [path.join('igir combined', 'speed_test_v51.sfc'), '8beffd94'],
         [path.join('igir combined', 'speed_test_v51.smc'), '9adca6cc'],
         [path.join('igir combined', 'Three Four Five', 'Five.rom'), '3e5daf67'],
         [path.join('igir combined', 'Three Four Five', 'Four.rom'), '1cf3ca74'],
@@ -448,7 +445,6 @@ describe('with explicit DATs', () => {
         path.join('headered', 'diagnostic_test_cartridge.a78.7z'),
         path.join('headered', 'fds_joypad_test.fds.zip'),
         path.join('headered', 'speed_test_v51.smc'),
-        path.join('headerless', 'speed_test_v51.sfc.gz'),
         path.join('patchable', '0F09A40.rom'),
         path.join('patchable', '3708F2C.rom'),
         path.join('patchable', '612644F.rom'),

test/modules/candidateValidator.test.ts

@@ -0,0 +1,114 @@
+import CandidateValidator from '../../src/modules/candidateValidator.js';
+import DAT from '../../src/types/dats/dat.js';
+import Game from '../../src/types/dats/game.js';
+import Header from '../../src/types/dats/logiqx/header.js';
+import LogiqxDAT from '../../src/types/dats/logiqx/logiqxDat.js';
+import Parent from '../../src/types/dats/parent.js';
+import ROM from '../../src/types/dats/rom.js';
+import ReleaseCandidate from '../../src/types/releaseCandidate.js';
+import ROMWithFiles from '../../src/types/romWithFiles.js';
+import ProgressBarFake from '../console/progressBarFake.js';
+
+async function datToCandidates(dat: DAT): Promise<Map<Parent, ReleaseCandidate[]>> {
+  const map = new Map<Parent, ReleaseCandidate[]>();
+  for (const parent of dat.getParents()) {
+    const releaseCandidates = await Promise.all(parent.getGames().flatMap((game) => {
+      const releases = game.getReleases().length > 0 ? game.getReleases() : [undefined];
+      return releases.map(async (release) => new ReleaseCandidate(
+        game,
+        release,
+        await Promise.all(game.getRoms().map(async (rom) => {
+          const dummyFile = await rom.toFile();
+          return new ROMWithFiles(rom, dummyFile, dummyFile);
+        })),
+      ));
+    }));
+    map.set(parent, releaseCandidates);
+  }
+  return map;
+}
+
+it('should do nothing with no candidates', async () => {
+  const dat = new LogiqxDAT(new Header(), []);
+  const parentsToCandidates = await datToCandidates(dat);
+
+  const invalidCandidates = await new CandidateValidator(new ProgressBarFake())
+    .validate(dat, parentsToCandidates);
+
+  expect(invalidCandidates).toHaveLength(0);
+});
+
+it('should return nothing if all candidates have unique paths', async () => {
+  const dat = new LogiqxDAT(new Header(), [
+    new Game({
+      name: 'game with no ROMs',
+    }),
+    new Game({
+      name: 'game with one ROM',
+      rom: [
+        new ROM({ name: 'one', size: 1 }),
+      ],
+    }),
+    new Game({
+      name: 'game with two ROMs',
+      rom: [
+        new ROM({ name: 'two', size: 2 }),
+        new ROM({ name: 'three', size: 3 }),
+      ],
+    }),
+  ]);
+  const parentsToCandidates = await datToCandidates(dat);
+
+  const invalidCandidates = await new CandidateValidator(new ProgressBarFake())
+    .validate(dat, parentsToCandidates);
+
+  expect(invalidCandidates).toHaveLength(0);
+});
+
+it('should return something if some candidates have conflicting paths', async () => {
+  const dat = new LogiqxDAT(new Header(), [
+    new Game({
+      name: 'game with no ROMs',
+    }),
+    new Game({
+      name: 'game one',
+      rom: [
+        new ROM({ name: 'one', size: 1 }),
+      ],
+    }),
+    new Game({
+      name: 'game two',
+      rom: [
+        new ROM({ name: 'two', size: 2 }),
+        new ROM({ name: 'three', size: 3 }),
+      ],
+    }),
+    new Game({
+      name: 'game three',
+      rom: [
+        new ROM({ name: 'three', size: 3 }),
+        new ROM({ name: 'four', size: 4 }),
+      ],
+    }),
+    new Game({
+      name: 'game four',
+      rom: [
+        new ROM({ name: 'four', size: 4 }),
+        new ROM({ name: 'five', size: 5 }),
+      ],
+    }),
+  ]);
+  const parentsToCandidates = await datToCandidates(dat);
+
+  const invalidCandidates = await new CandidateValidator(new ProgressBarFake())
+    .validate(dat, parentsToCandidates);
+
+  const invalidCandidateNames = invalidCandidates
+    .map((candidate) => candidate.getName())
+    .sort();
+  expect(invalidCandidateNames).toEqual([
+    'game four',
+    'game three',
+    'game two',
+  ]);
+});