Feature: DUPLICATE report status to disambiguate UNUSED (#1120)

emmercm · May 8, 2024 · 4e03c51 · 4e03c51
1 parent 825d608
commit 4e03c51
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 51 deletions.
diff --git a/docs/output/reporting.md b/docs/output/reporting.md
@@ -4,9 +4,10 @@
 
 When using DATs (the [`--dat <path>` option](../dats/processing.md#scanning-for-dats)), the `igir report` [command](../commands.md) can report on:
 
-- `FOUND`: what ROMs were found, and where the files are on disk
+- `FOUND`: what ROMs were found, and where their files are on disk
 - `IGNORED`: what ROMs were ignored (due to [`--single` 1G1R rules](../roms/filtering-preferences.md))
-- `MISSING`: what ROMs were wanted, but are missing
+- `MISSING`: what ROMs were wanted, but weren't found
+- `DUPLICATE`: what input files _did_ match to a ROM but weren't used when writing
 - `UNUSED`: what input files didn't match to any ROM
 - `DELETED`: what output files were [cleaned](cleaning.md) (`igir clean` command)
 

diff --git a/src/igir.ts b/src/igir.ts
@@ -402,7 +402,7 @@ export default class Igir {
 
     const reportProgressBar = await this.logger.addProgressBar('Generating report', ProgressBarSymbol.WRITING);
     await new ReportGenerator(this.options, reportProgressBar).generate(
-      scannedRomFiles.map((file) => file.getFilePath()),
+      scannedRomFiles,
       cleanedOutputFiles,
       datsStatuses,
     );

diff --git a/src/modules/reportGenerator.ts b/src/modules/reportGenerator.ts
@@ -1,7 +1,7 @@
 import ProgressBar from '../console/progressBar.js';
-import ArrayPoly from '../polyfill/arrayPoly.js';
 import FsPoly from '../polyfill/fsPoly.js';
 import DATStatus, { GameStatus } from '../types/datStatus.js';
+import File from '../types/files/file.js';
 import Options from '../types/options.js';
 import Module from './module.js';
 
@@ -22,7 +22,7 @@ export default class ReportGenerator extends Module {
    * Generate the report.
    */
   async generate(
-    scannedRomFiles: string[],
+    scannedRomFiles: File[],
     cleanedOutputFiles: string[],
     datStatuses: DATStatus[],
   ): Promise<void> {
@@ -47,19 +47,38 @@ export default class ReportGenerator extends Module {
         return csv.split('\n').slice(1).join('\n');
       });
 
-    const usedFiles = new Set(datStatuses
+    const usedFilePaths = new Set(datStatuses
       .flatMap((datStatus) => datStatus.getInputFiles())
       .map((file) => file.getFilePath()));
-    const unusedFiles = scannedRomFiles
-      .reduce(ArrayPoly.reduceUnique(), [])
-      .filter((inputFile) => !usedFiles.has(inputFile))
+    const usedHashes = new Set(datStatuses
+      .flatMap((datStatus) => datStatus.getInputFiles())
+      .map((file) => file.hashCode()));
+
+    const duplicateFilePaths = scannedRomFiles
+      .filter((inputFile) => !usedFilePaths.has(inputFile.getFilePath())
+          && usedHashes.has(inputFile.hashCode()))
+      .map((inputFile) => inputFile.getFilePath())
+      .filter((inputFile) => !usedFilePaths.has(inputFile))
+      .sort();
+    const duplicateCsv = await DATStatus.filesToCsv(duplicateFilePaths, GameStatus.DUPLICATE);
+
+    const unusedFilePaths = scannedRomFiles
+      .filter((inputFile) => !usedFilePaths.has(inputFile.getFilePath())
+          && !usedHashes.has(inputFile.hashCode()))
+      .map((inputFile) => inputFile.getFilePath())
+      .filter((inputFile) => !usedFilePaths.has(inputFile))
       .sort();
-    const unusedCsv = await DATStatus.filesToCsv(unusedFiles, GameStatus.UNUSED);
+    const unusedCsv = await DATStatus.filesToCsv(unusedFilePaths, GameStatus.UNUSED);
 
     const cleanedCsv = await DATStatus.filesToCsv(cleanedOutputFiles, GameStatus.DELETED);
 
     this.progressBar.logInfo(`writing report '${reportPath}'`);
-    const rows = [...matchedFileCsvs, unusedCsv, cleanedCsv].filter((csv) => csv);
+    const rows = [
+      ...matchedFileCsvs,
+      duplicateCsv,
+      unusedCsv,
+      cleanedCsv,
+    ].filter((csv) => csv);
     await FsPoly.writeFile(reportPath, rows.join('\n'));
     this.progressBar.logTrace(`wrote ${datStatuses.length.toLocaleString()} CSV row${datStatuses.length !== 1 ? 's' : ''}: ${reportPath}`);
 

diff --git a/src/types/datStatus.ts b/src/types/datStatus.ts
@@ -27,7 +27,9 @@ export enum GameStatus {
   IGNORED,
   // The Game wanted to be written, but there was no matching ReleaseCandidate
   MISSING,
-  // The input File was not used in any ReleaseCandidate
+  // The input file was not used in any ReleaseCandidate, but a duplicate file was
+  DUPLICATE,
+  // The input File was not used in any ReleaseCandidate, and neither was any duplicate file
   UNUSED,
   // The output File was not from any ReleaseCandidate, so it was deleted
   DELETED,

diff --git a/test/modules/reportGenerator.test.ts b/test/modules/reportGenerator.test.ts
@@ -30,7 +30,7 @@ const datStatusEmpty = new DATStatus(
 const gamesSingle = [
   new Game({
     name: 'One',
-    rom: [new ROM({ name: 'One', size: 123, crc32: 'abcdef01' })],
+    rom: [new ROM({ name: 'One.rom', size: 123, crc32: 'abcdef01' })],
   }),
 ];
 async function buildDatStatusSingle(): Promise<DATStatus> {
@@ -41,7 +41,7 @@ async function buildDatStatusSingle(): Promise<DATStatus> {
         game,
         undefined,
         await Promise.all(game.getRoms().map(async (rom) => {
-          const romFile = await File.fileOf({ filePath: `${rom.getName()}.rom` });
+          const romFile = await rom.toFile();
           return new ROMWithFiles(rom, romFile, romFile);
         })),
       )],
@@ -58,15 +58,15 @@ async function buildDatStatusSingle(): Promise<DATStatus> {
 const gamesMultiple = [
   new Game({
     name: 'Two',
-    rom: [new ROM({ name: 'Two', size: 234, crc32: 'bcdef012' })],
+    rom: [new ROM({ name: 'Two.rom', size: 234, crc32: 'bcdef012' })],
   }),
   new Game({
     name: 'Three',
-    rom: [new ROM({ name: 'Three', size: 345, crc32: 'cdef0123' })],
+    rom: [new ROM({ name: 'Three.rom', size: 345, crc32: 'cdef0123' })],
   }),
   new Game({
     name: 'Four',
-    rom: [new ROM({ name: 'Four', size: 456, crc32: 'def01234' })],
+    rom: [new ROM({ name: 'Four.rom', size: 456, crc32: 'def01234' })],
   }),
   new Game({
     name: 'Five',
@@ -81,7 +81,7 @@ async function buildDatStatusMultiple(): Promise<DATStatus> {
         game,
         undefined,
         await Promise.all(game.getRoms().map(async (rom) => {
-          const romFile = await File.fileOf({ filePath: `${rom.getName()}.rom` });
+          const romFile = await rom.toFile();
           return new ROMWithFiles(rom, romFile, romFile);
         })),
       )],
@@ -97,7 +97,7 @@ async function buildDatStatusMultiple(): Promise<DATStatus> {
 
 async function wrapReportGenerator(
   optionsProps: OptionsProps,
-  romFiles: string[],
+  romFiles: File[],
   cleanedOutputFiles: string[],
   datStatuses: DATStatus[],
   callback: (contents: string) => void | Promise<void>,
@@ -108,58 +108,98 @@ async function wrapReportGenerator(
     reportOutput,
   });
 
-  await new ReportGenerator(options, new ProgressBarFake())
-    .generate(romFiles, cleanedOutputFiles, datStatuses);
-
-  const contents = (await fs.promises.readFile(reportOutput)).toString();
-  await callback(contents);
+  await new ReportGenerator(options, new ProgressBarFake()).generate(
+    romFiles,
+    cleanedOutputFiles,
+    datStatuses,
+  );
 
-  await fsPoly.rm(reportOutput);
+  try {
+    const contents = (await fs.promises.readFile(reportOutput)).toString();
+    await callback(contents);
+  } finally {
+    await fsPoly.rm(reportOutput);
+  }
 }
 
 it('should return empty contents for an empty DAT', async () => {
-  await wrapReportGenerator(new Options(), [], [], [datStatusEmpty], (contents) => {
-    expect(contents).toEqual('');
-  });
+  await wrapReportGenerator(
+    new Options(),
+    [],
+    [],
+    [datStatusEmpty],
+    (contents) => {
+      expect(contents).toEqual('');
+    },
+  );
 });
 
 it('should return one row for every game in a single game DAT', async () => {
-  await wrapReportGenerator(new Options(), [], [], [await buildDatStatusSingle()], (contents) => {
-    expect(contents).toEqual(`DAT Name,Game Name,Status,ROM Files,Patched,BIOS,Retail Release,Unlicensed,Debug,Demo,Beta,Sample,Prototype,Program,Aftermarket,Homebrew,Bad
+  await wrapReportGenerator(
+    new Options(),
+    [],
+    [],
+    [await buildDatStatusSingle()],
+    (contents) => {
+      expect(contents).toEqual(`DAT Name,Game Name,Status,ROM Files,Patched,BIOS,Retail Release,Unlicensed,Debug,Demo,Beta,Sample,Prototype,Program,Aftermarket,Homebrew,Bad
 Single,One,FOUND,One.rom,false,false,true,false,false,false,false,false,false,false,false,false,false`);
-  });
+    },
+  );
 });
 
 it('should return one row for every game in a multiple game DAT', async () => {
-  await wrapReportGenerator(new Options(), [], [], [await buildDatStatusMultiple()], (contents) => {
-    expect(contents).toEqual(`DAT Name,Game Name,Status,ROM Files,Patched,BIOS,Retail Release,Unlicensed,Debug,Demo,Beta,Sample,Prototype,Program,Aftermarket,Homebrew,Bad
+  await wrapReportGenerator(
+    new Options(),
+    [],
+    [],
+    [await buildDatStatusMultiple()],
+    (contents) => {
+      expect(contents).toEqual(`DAT Name,Game Name,Status,ROM Files,Patched,BIOS,Retail Release,Unlicensed,Debug,Demo,Beta,Sample,Prototype,Program,Aftermarket,Homebrew,Bad
 Multiple,Five,FOUND,,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Four,FOUND,Four.rom,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Three,FOUND,Three.rom,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Two,FOUND,Two.rom,false,false,true,false,false,false,false,false,false,false,false,false,false`);
-  });
+    },
+  );
 });
 
-it('should return one row for every unused file in a multiple game DAT', async () => {
-  await wrapReportGenerator(new Options(), [
-    'One.rom',
-    'Two.rom',
-    'Three.rom',
-    'Four.rom',
-  ], [], [await buildDatStatusMultiple()], (contents) => {
-    expect(contents).toEqual(`DAT Name,Game Name,Status,ROM Files,Patched,BIOS,Retail Release,Unlicensed,Debug,Demo,Beta,Sample,Prototype,Program,Aftermarket,Homebrew,Bad
+it('should return one row for every duplicate and unused file in a multiple game DAT', async () => {
+  await wrapReportGenerator(
+    new Options(),
+    [
+      await File.fileOf({ filePath: 'One.rom', size: 123, crc32: 'abcdef01' }),
+      await File.fileOf({ filePath: 'One (Duplicate).rom', size: 123, crc32: 'abcdef01' }),
+      await File.fileOf({ filePath: 'Two (Duplicate).rom', size: 234, crc32: 'bcdef012' }),
+      await File.fileOf({ filePath: 'Two.rom', size: 234, crc32: 'bcdef012' }),
+      await File.fileOf({ filePath: 'Three.rom', size: 345, crc32: 'cdef0123' }),
+      await File.fileOf({ filePath: 'Four.rom', size: 456, crc32: 'def01234' }),
+      await File.fileOf({ filePath: 'Four (Duplicate).rom', size: 456, crc32: 'def01234' }),
+      await File.fileOf({ filePath: 'Five.rom', size: 567, crc32: 'ef012345' }),
+    ],
+    [],
+    [await buildDatStatusMultiple()],
+    (contents) => {
+      expect(contents).toEqual(`DAT Name,Game Name,Status,ROM Files,Patched,BIOS,Retail Release,Unlicensed,Debug,Demo,Beta,Sample,Prototype,Program,Aftermarket,Homebrew,Bad
 Multiple,Five,FOUND,,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Four,FOUND,Four.rom,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Three,FOUND,Three.rom,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Two,FOUND,Two.rom,false,false,true,false,false,false,false,false,false,false,false,false,false
+,,DUPLICATE,Four (Duplicate).rom,false,false,false,false,false,false,false,false,false,false,false,false,false
+,,DUPLICATE,Two (Duplicate).rom,false,false,false,false,false,false,false,false,false,false,false,false,false
+,,UNUSED,Five.rom,false,false,false,false,false,false,false,false,false,false,false,false,false
+,,UNUSED,One (Duplicate).rom,false,false,false,false,false,false,false,false,false,false,false,false,false
 ,,UNUSED,One.rom,false,false,false,false,false,false,false,false,false,false,false,false,false`);
-  });
+    },
+  );
 });
 
 it('should return one row for every cleaned file in a multiple game DAT', async () => {
   await wrapReportGenerator(
     new Options(),
-    ['One.rom', 'Two.rom'],
+    [
+      await File.fileOf({ filePath: 'One.rom', size: 123, crc32: 'abcdef01' }),
+      await File.fileOf({ filePath: 'Two.rom', size: 234, crc32: 'bcdef012' }),
+    ],
     ['Three.rom', 'Four.rom'],
     [await buildDatStatusMultiple()],
     (contents) => {
@@ -176,16 +216,22 @@ Multiple,Two,FOUND,Two.rom,false,false,true,false,false,false,false,false,false,
 });
 
 it('should return one row for every game in multiple DATs', async () => {
-  await wrapReportGenerator(new Options(), [], [], [
-    datStatusEmpty,
-    await buildDatStatusSingle(),
-    await buildDatStatusMultiple(),
-  ], (contents) => {
-    expect(contents).toEqual(`DAT Name,Game Name,Status,ROM Files,Patched,BIOS,Retail Release,Unlicensed,Debug,Demo,Beta,Sample,Prototype,Program,Aftermarket,Homebrew,Bad
+  await wrapReportGenerator(
+    new Options(),
+    [],
+    [],
+    [
+      datStatusEmpty,
+      await buildDatStatusSingle(),
+      await buildDatStatusMultiple(),
+    ],
+    (contents) => {
+      expect(contents).toEqual(`DAT Name,Game Name,Status,ROM Files,Patched,BIOS,Retail Release,Unlicensed,Debug,Demo,Beta,Sample,Prototype,Program,Aftermarket,Homebrew,Bad
 Multiple,Five,FOUND,,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Four,FOUND,Four.rom,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Three,FOUND,Three.rom,false,false,true,false,false,false,false,false,false,false,false,false,false
 Multiple,Two,FOUND,Two.rom,false,false,true,false,false,false,false,false,false,false,false,false,false
 Single,One,FOUND,One.rom,false,false,true,false,false,false,false,false,false,false,false,false,false`);
-  });
+    },
+  );
 });