Merge branch 'main' into emmercm/555-fix

.github/workflows/test.yml

@@ -106,6 +106,15 @@ jobs:
         with:
           pattern: 'renovate.json5'
 
+  shellcheck:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Run ShellCheck
+        uses: ludeeus/action-shellcheck@master
+        with:
+          additional_files: '.husky/*'
+
   # !!! This check should be required by GitHub !!!
   test-status-check:
     if: always()
@@ -115,6 +124,7 @@ jobs:
       - node-e2e
       - node-package
       - renovate-lint
+      - shellcheck
     runs-on: ubuntu-latest
     steps:
       - uses: re-actors/alls-green@release/v1

.husky/pre-commit

@@ -1,4 +1,5 @@
 #!/usr/bin/env sh
+# shellcheck disable=SC1091
 . "$(dirname -- "$0")/_/husky.sh"
 
 # Change to the project's root directory
@@ -7,14 +8,15 @@ cd "$(dirname -- "$0")/.." || exit 1
 # Ensure node is available (same as asciinema-rec.sh)
 if [ ! -x "$(command -v node)" ]; then
   if [ ! -x "$(command -v nvm)" ] && [ -f ~/.nvm/nvm.sh ]; then
+    # shellcheck disable=SC1090
     . ~/.nvm/nvm.sh
   fi
   if [ ! -x "$(command -v nvm)" ]; then
     nvm install || true
     nvm use || exit 1
   fi
 fi
-node --version &> /dev/null || exit 1
+node --version > /dev/null 2>&1 || exit 1
 
 # Ensure node modules are installed
 if [ ! -d "node_modules" ]; then

README.md

@@ -56,7 +56,7 @@ $ igir --help
   | $$  | $$|    \  | $$  | $$    $$   ROM collection manager
   | $$  | $$|    \  | $$  | $$    $$   https://igir.io/
   | $$  | $$ \$$$$  | $$  | $$$$$$$\
- _| $$_ | $$__| $$ _| $$_ | $$  | $$   v1.9.0
+ _| $$_ | $$__| $$ _| $$_ | $$  | $$   v1.9.1
 |   $$ \ \$$    $$|   $$ \| $$  | $$
  \$$$$$$  \$$$$$$  \$$$$$$ \$$   \$$
 
@@ -128,9 +128,9 @@ ROM filtering options:
   -L, --language-filter       List of comma-separated languages to filter to (supported: DA, DE,
                               EL, EN, ES, FI, FR, IT, JA, KO, NL, NO, PT, RU, SV, ZH)    [string]
   -R, --region-filter         List of comma-separated regions to filter to (supported: ARG, ASI,
-                              AUS, BRA, CAN, CHN, DAN, EUR, FRA, FYN, GER, GRE, HK, HOL, ITA, JPN
-                              , KOR, MEX, NOR, NZ, POR, RUS, SPA, SWE, TAI, UK, UNK, USA, WORLD)
-                                                                                         [string]
+                              AUS, BEL, BRA, CAN, CHN, DAN, EUR, FRA, FYN, GER, GRE, HK, HOL, ITA
+                              , JPN, KOR, MEX, NOR, NZ, POR, RUS, SPA, SWE, TAI, UK, UNK, USA, WO
+                              RLD)                                                       [string]
       --no-bios               Filter out BIOS files, opposite of --only-bios            [boolean]
       --no-device             Filter out MAME devies, opposite of --only-device         [boolean]
       --no-unlicensed         Filter out unlicensed ROMs, opposite of --only-unlicensed [boolean]
@@ -157,9 +157,9 @@ One game, one ROM (1G1R) options:
                                , DE, EL, EN, ES, FI, FR, IT, JA, KO, NL, NO, PT, RU, SV, ZH)
                                                                                          [string]
   -r, --prefer-region          List of comma-separated regions in priority order (supported: ARG,
-                                ASI, AUS, BRA, CAN, CHN, DAN, EUR, FRA, FYN, GER, GRE, HK, HOL, I
-                               TA, JPN, KOR, MEX, NOR, NZ, POR, RUS, SPA, SWE, TAI, UK, UNK, USA,
-                                WORLD)                                                   [string]
+                                ASI, AUS, BEL, BRA, CAN, CHN, DAN, EUR, FRA, FYN, GER, GRE, HK, H
+                               OL, ITA, JPN, KOR, MEX, NOR, NZ, POR, RUS, SPA, SWE, TAI, UK, UNK,
+                                USA, WORLD)                                              [string]
       --prefer-revision-newer  Prefer newer ROM revisions over older                    [boolean]
       --prefer-revision-older  Prefer older ROM revisions over newer                    [boolean]
       --prefer-retail          Prefer retail releases (see --only-retail)               [boolean]

package.json

@@ -1,6 +1,6 @@
 {
   "name": "igir",
-  "version": "1.9.0",
+  "version": "1.9.1",
   "description": "🕹 A video game ROM collection manager to help filter, sort, patch, archive, and report on collections on any OS.",
   "keywords": [
     "1g1r",

scripts/asciinema-rec.sh

@@ -11,19 +11,22 @@ DEMO_DIR="demo"
 
 
 if [[ "${1:-}" == "play" ]]; then
+  # shellcheck disable=SC1091
   . demo-magic.sh
-  TYPE_SPEED=15
-  DEMO_PROMPT="$ "
-  DEMO_CMD_COLOR="\033[1;37m"
-  DEMO_COMMENT_COLOR="\033[0;90m"
+  export TYPE_SPEED=15
+  export DEMO_PROMPT="$ "
+  export DEMO_CMD_COLOR="\033[1;37m"
+  export DEMO_COMMENT_COLOR="\033[0;90m"
   cd "${DEMO_DIR}"
   # NOTE(cemmer): these have to be functions, `pei` won't pick up on aliases
+  # shellcheck disable=SC2317
   npx() {
     shift # discard "igir@latest"
     node ../build/index.js "$@"
   }
+  # shellcheck disable=SC2317
   tree() {
-    command tree -N -I *.rsl* "$@"
+    command tree -N -I -- *.rsl* "$@"
   }
   # BEGIN PLAYBACK
 
@@ -75,6 +78,7 @@ fi
 # Ensure node is available (same as .husky/pre-commit)
 if [[ ! -x "$(command -v node)" ]]; then
   if [[ ! -x "$(command -v nvm)" && -f ~/.nvm/nvm.sh ]]; then
+    # shellcheck disable=SC1090
     . ~/.nvm/nvm.sh
   fi
   if [[ ! -x "$(command -v nvm)" ]]; then

src/modules/candidateGenerator.ts

@@ -103,7 +103,7 @@ export default class CandidateGenerator extends Module {
     release: Release | undefined,
     hashCodeToInputFiles: Map<string, File[]>,
   ): Promise<ReleaseCandidate | undefined> {
-    const romsToInputFiles = this.getInputFilesForGame(game, hashCodeToInputFiles);
+    const romsToInputFiles = CandidateGenerator.getInputFilesForGame(game, hashCodeToInputFiles);
 
     // For each Game's ROM, find the matching File
     const romFiles = await Promise.all(
@@ -182,7 +182,7 @@ export default class CandidateGenerator extends Module {
     return new ReleaseCandidate(game, release, foundRomsWithFiles);
   }
 
-  private getInputFilesForGame(
+  private static getInputFilesForGame(
     game: Game,
     hashCodeToInputFiles: Map<string, File[]>,
   ): Map<ROM, File> {
@@ -219,7 +219,7 @@ export default class CandidateGenerator extends Module {
     }, new Map<Archive, ROM[]>());
 
     // Only filter the input files if this game has multiple ROMs, and we found some archives
-    if (!this.options.shouldExtract() && game.getRoms().length > 1 && inputArchivesToRoms.size) {
+    if (game.getRoms().length > 1 && inputArchivesToRoms.size) {
       // Filter to the Archives that contain every ROM in this Game
       const archivesWithEveryRom = [...inputArchivesToRoms.entries()]
         .filter(([, roms]) => roms.length === game.getRoms().length)

src/modules/movedRomDeleter.ts

@@ -62,7 +62,16 @@ export default class MovedROMDeleter extends Module {
 
     return [...groupedMovedRoms.entries()]
       .map(([filePath, movedEntries]) => {
-        const movedEntriesStrings = movedEntries.map((entry) => entry.toString());
+        // NOTE(cemmer): games can have ROMs with duplicate checksums, which means an Archive of
+        //  that game's ROMs will contain some duplicate files. When extracting or zipping, we would
+        //  have generated multiple ReleaseCandidates with the same input File, resulting in the
+        //  duplicate files in the Archive not being considered "moved." Therefore, we should use
+        //  the unique set of ArchiveEntry hash codes to know if every ArchiveEntry was "consumed"
+        //  during writing.
+        const movedEntryHashCodes = new Set(
+          movedEntries.flatMap((file) => file.hashCodes()),
+        );
+
         const inputEntries = groupedInputRoms.get(filePath) ?? [];
 
         const unmovedEntries = inputEntries.filter((entry) => {
@@ -76,7 +85,7 @@ export default class MovedROMDeleter extends Module {
           }
 
           // Otherwise, the entry needs to have been explicitly moved
-          return movedEntriesStrings.indexOf(entry.toString()) === -1;
+          return entry.hashCodes().some((hashCode) => !movedEntryHashCodes.has(hashCode));
         });
         if (unmovedEntries.length) {
           this.progressBar.logWarn(`${filePath}: not deleting moved file, ${unmovedEntries.length.toLocaleString()} archive entr${unmovedEntries.length !== 1 ? 'ies were' : 'y was'} unmatched:${unmovedEntries.sort().map((entry) => `\n  ${entry}`)}`);

test/igir.test.ts

@@ -234,7 +234,6 @@ describe('with explicit DATs', () => {
       path.join('roms', 'raw', 'fizzbuzz.nes'),
       path.join('roms', 'raw', 'four.rom'),
       path.join('roms', 'raw', 'loremipsum.rom'),
-      path.join('roms', 'raw', 'one.rom'),
       path.join('roms', 'raw', 'three.rom'),
     ]);
   });
@@ -548,6 +547,8 @@ describe('with inferred DATs', () => {
       path.join('roms', 'raw', 'two.rom'),
       path.join('roms', 'raw', 'unknown.rom'),
       path.join('roms', 'unheadered', 'speed_test_v51.sfc.gz'),
+      path.join('roms', 'zip', 'fourfive.zip'),
+      path.join('roms', 'zip', 'onetwothree.zip'),
     ]);
   });
 

test/modules/movedRomDeleter.test.ts

@@ -1,6 +1,16 @@
+import path from 'path';
+
+import CandidateGenerator from '../../src/modules/candidateGenerator.js';
+import FileIndexer from '../../src/modules/fileIndexer.js';
 import MovedROMDeleter from '../../src/modules/movedRomDeleter.js';
 import ROMScanner from '../../src/modules/romScanner.js';
 import fsPoly from '../../src/polyfill/fsPoly.js';
+import Zip from '../../src/types/files/archives/zip.js';
+import File from '../../src/types/files/file.js';
+import DAT from '../../src/types/logiqx/dat.js';
+import Game from '../../src/types/logiqx/game.js';
+import Header from '../../src/types/logiqx/header.js';
+import ROM from '../../src/types/logiqx/rom.js';
 import Options from '../../src/types/options.js';
 import ProgressBarFake from '../console/progressBarFake.js';
 
@@ -16,4 +26,154 @@ it('should do nothing if no ROMs moved', async () => {
   expect(exists).not.toContain(false);
 });
 
-// TODO(cemmer): more meaningful tests
+it('should delete raw files', () => {
+  // TODO(cemmer)
+});
+
+describe('should delete archives', () => {
+  describe.each(['extract', 'zip'])('command: %s', (command) => {
+    test.each(([
+      // Game with duplicate ROMs
+      [[
+        new Game({
+          name: 'Euro Demo 42 (Europe)',
+          rom: [
+            new ROM('Euro Demo 42 (Europe).cue', 1374, '96b2b896'),
+            new ROM('Euro Demo 42 (Europe) (Track 01).bin', 326168304, '738a3744'),
+            new ROM('Euro Demo 42 (Europe) (Track 02).bin', 12199824, '1b77f37f'),
+            new ROM('Euro Demo 42 (Europe) (Track 03).bin', 1147776, 'c39c78b2'),
+            new ROM('Euro Demo 42 (Europe) (Track 04).bin', 1147776, 'c39c78b2'),
+            new ROM('Euro Demo 42 (Europe) (Track 05).bin', 10746288, '30a1b973'),
+            new ROM('Euro Demo 42 (Europe) (Track 06).bin', 1147776, 'c39c78b2'),
+            new ROM('Euro Demo 42 (Europe) (Track 07).bin', 1147776, 'c39c78b2'),
+            new ROM('Euro Demo 42 (Europe) (Track 08).bin', 37260384, '0c2c3820'),
+            new ROM('Euro Demo 42 (Europe) (Track 09).bin', 32182416, '80784294'),
+            new ROM('Euro Demo 42 (Europe) (Track 10).bin', 26619936, '227a67cf'),
+            new ROM('Euro Demo 42 (Europe) (Track 11).bin', 19189968, '2ed908e7'),
+            new ROM('Euro Demo 42 (Europe) (Track 12).bin', 32356464, 'f6f33c3b'),
+          ],
+        }),
+      ], [
+        'Euro Demo 42 (Europe).zip',
+      ]],
+      // Multiple games with shared ROMs
+      [[
+        new Game({
+          name: 'Zero 4 Champ II (Japan)',
+          rom: [
+            new ROM('Zero 4 Champ II (Japan).cue', 4187, 'a8c5c66e'),
+            new ROM('Zero 4 Champ II (Japan) (Track 01).bin', 7916832, '22144d0f'),
+            new ROM('Zero 4 Champ II (Japan) (Track 02).bin', 25542720, 'a3ecc4eb'),
+            new ROM('Zero 4 Champ II (Japan) (Track 03).bin', 13041840, '5f4717be'),
+            new ROM('Zero 4 Champ II (Japan) (Track 04).bin', 11741184, 'a929f0e1'),
+            new ROM('Zero 4 Champ II (Japan) (Track 05).bin', 11475408, '8ce00863'),
+            new ROM('Zero 4 Champ II (Japan) (Track 06).bin', 10151232, '1b052c36'),
+            new ROM('Zero 4 Champ II (Japan) (Track 07).bin', 13135920, '54d7853c'),
+            new ROM('Zero 4 Champ II (Japan) (Track 08).bin', 24213840, 'aad2c61a'),
+            new ROM('Zero 4 Champ II (Japan) (Track 09).bin', 11799984, '10f0d49b'),
+            new ROM('Zero 4 Champ II (Japan) (Track 10).bin', 16334640, '9eb8f90f'),
+            new ROM('Zero 4 Champ II (Japan) (Track 11).bin', 11338992, 'fa3d048a'),
+            new ROM('Zero 4 Champ II (Japan) (Track 12).bin', 12084576, '33167322'),
+            new ROM('Zero 4 Champ II (Japan) (Track 13).bin', 10513440, '09fb1bd2'),
+            new ROM('Zero 4 Champ II (Japan) (Track 14).bin', 16325232, '7404385c'),
+            new ROM('Zero 4 Champ II (Japan) (Track 15).bin', 15266832, '7f2b9388'),
+            new ROM('Zero 4 Champ II (Japan) (Track 16).bin', 11153184, '0c360c9b'),
+            new ROM('Zero 4 Champ II (Japan) (Track 17).bin', 11148480, '2f712235'),
+            new ROM('Zero 4 Champ II (Japan) (Track 18).bin', 11357808, '8a7b39b2'),
+            new ROM('Zero 4 Champ II (Japan) (Track 19).bin', 7067760, '616c4702'),
+            new ROM('Zero 4 Champ II (Japan) (Track 20).bin', 13279392, '27706096'),
+            new ROM('Zero 4 Champ II (Japan) (Track 21).bin', 7923888, 'fecb096f'),
+            new ROM('Zero 4 Champ II (Japan) (Track 22).bin', 14888160, '9528018f'),
+            new ROM('Zero 4 Champ II (Japan) (Track 23).bin', 6385680, '26c2432a'),
+            new ROM('Zero 4 Champ II (Japan) (Track 24).bin', 16367568, 'ce330910'),
+            new ROM('Zero 4 Champ II (Japan) (Track 25).bin', 846720, '11bf5dbd'),
+            new ROM('Zero 4 Champ II (Japan) (Track 26).bin', 3281040, 'f9188f3a'),
+            new ROM('Zero 4 Champ II (Japan) (Track 27).bin', 846720, '11bf5dbd'),
+            new ROM('Zero 4 Champ II (Japan) (Track 28).bin', 4887456, '8d2dbe56'),
+            new ROM('Zero 4 Champ II (Japan) (Track 29).bin', 12945408, '8d234a26'),
+            new ROM('Zero 4 Champ II (Japan) (Track 30).bin', 17611776, '92ee3fb4'),
+            new ROM('Zero 4 Champ II (Japan) (Track 31).bin', 21073920, '7b03db99'),
+            new ROM('Zero 4 Champ II (Japan) (Track 32).bin', 14556528, '1f3057c2'),
+            new ROM('Zero 4 Champ II (Japan) (Track 33).bin', 10520496, '887d8c2a'),
+            new ROM('Zero 4 Champ II (Japan) (Track 34).bin', 11219040, '05d3e3a4'),
+            new ROM('Zero 4 Champ II (Japan) (Track 35).bin', 12411504, 'b4cabb3b'),
+            new ROM('Zero 4 Champ II (Japan) (Track 36).bin', 15285648, '8f997117'),
+            new ROM('Zero 4 Champ II (Japan) (Track 37).bin', 11284896, '798a830f'),
+            new ROM('Zero 4 Champ II (Japan) (Track 38).bin', 24498432, 'ac1cbe07'),
+            new ROM('Zero 4 Champ II (Japan) (Track 39).bin', 19533360, '36df83e6'),
+            new ROM('Zero 4 Champ II (Japan) (Track 40).bin', 21897120, '137e2970'),
+            new ROM('Zero 4 Champ II (Japan) (Track 41).bin', 2629536, 'd02a90b7'),
+            new ROM('Zero 4 Champ II (Japan) (Track 42).bin', 14224896, 'aa345590'),
+            new ROM('Zero 4 Champ II (Japan) (Track 43).bin', 25192272, '661f8f8e'),
+          ],
+        }),
+        new Game({
+          name: 'Adventure Quiz - Capcom World + Hatena no Daibouken (Japan)',
+          rom: [
+            new ROM('Adventure Quiz - Capcom World + Hatena no Daibouken (Japan).cue', 984, '54a01e7d'),
+            new ROM('Adventure Quiz - Capcom World + Hatena no Daibouken (Japan) (Track 1).bin', 7916832, '22144d0f'),
+            new ROM('Adventure Quiz - Capcom World + Hatena no Daibouken (Japan) (Track 2).bin', 10536960, '59cc5f3f'),
+            new ROM('Adventure Quiz - Capcom World + Hatena no Daibouken (Japan) (Track 3).bin', 7785120, '877dde76'),
+            new ROM('Adventure Quiz - Capcom World + Hatena no Daibouken (Japan) (Track 4).bin', 18992400, '3236adb9'),
+            new ROM('Adventure Quiz - Capcom World + Hatena no Daibouken (Japan) (Track 5).bin', 9252768, '341cc45f'),
+            new ROM('Adventure Quiz - Capcom World + Hatena no Daibouken (Japan) (Track 6).bin', 27854736, 'e3646dc1'),
+            new ROM('Adventure Quiz - Capcom World + Hatena no Daibouken (Japan) (Track 7).bin', 5216736, 'e3792471'),
+          ],
+        }),
+      ], [
+        'Adventure Quiz - Capcom World + Hatena no Daibouken (Japan).zip',
+        'Zero 4 Champ II (Japan).zip',
+      ]],
+    ]))('%s', async (games, expectedDeletedFilePaths) => {
+      const inputPath = 'input';
+      const options = new Options({
+        commands: ['move', command],
+        input: [inputPath],
+        output: 'output',
+      });
+
+      const dat = new DAT(new Header(), games);
+
+      const rawRomFiles = (await Promise.all(dat.getParents()
+        .flatMap((parent) => parent.getGames())
+        .map(async (game): Promise<File[]> => {
+          // A path that should not exist
+          const zip = new Zip(path.join(inputPath, `${game.getName()}.zip`));
+          return Promise.all(game.getRoms().map(async (rom) => rom.toArchiveEntry(zip)));
+        }))).flatMap((files) => files);
+
+      const indexedRomFiles = await new FileIndexer(options, new ProgressBarFake())
+        .index(rawRomFiles);
+      const parentsToCandidates = await new CandidateGenerator(options, new ProgressBarFake())
+        .generate(dat, indexedRomFiles);
+
+      const inputRoms = rawRomFiles;
+      const movedRoms = [...parentsToCandidates.values()]
+        .flatMap((releaseCandidates) => releaseCandidates)
+        .flatMap((releaseCandidate) => releaseCandidate.getRomsWithFiles())
+        .map((romWithFiles) => romWithFiles.getInputFile());
+
+      const parentsToWrittenRoms = new Map([...parentsToCandidates.entries()]
+        .map(([parent, releaseCandidates]) => {
+          const writtenRoms = releaseCandidates
+            .flatMap((releaseCandidate) => releaseCandidate.getRomsWithFiles())
+            .map((romWithFiles) => romWithFiles.getOutputFile());
+          return [parent, writtenRoms];
+        }));
+      const datsToWrittenRoms = new Map([[dat, parentsToWrittenRoms]]);
+
+      const deletedFilePaths = (
+        await new MovedROMDeleter(new ProgressBarFake())
+          .delete(inputRoms, movedRoms, datsToWrittenRoms)
+      )
+        .map((filePath) => filePath.replace(inputPath + path.sep, ''))
+        .sort();
+
+      expect(deletedFilePaths).toEqual(expectedDeletedFilePaths);
+    });
+  });
+});
+
+it('should not delete files that weren\'t moved', () => {
+  // TODO(cemmer)
+});