Skip to content

Commit

Permalink
Refactor: breaking: combine prefer revision older & newer options (#1308
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@emmercm
emmercm authored Aug 26, 2024
1 parent 6f80e28 commit 2811035
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 92 deletions.
19 changes: 17 additions & 2 deletions docs/roms/filtering-preferences.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,10 +475,10 @@ For example, to prefer games from: USA (highest priority), "world," and then Eur
### Prefer revision

```text
--prefer-revision-newer, --prefer-revision-older
--prefer-revision <older|newer>
```

Prefer newer or older revisions of a game.
Prefer newer or older revisions, versions, or ring codes of a game.

Revisions can be numeric:

Expand All @@ -496,6 +496,21 @@ MSR - Metropolis Street Racer (Europe) (En,Fr,De,Es) (Rev A)
MSR - Metropolis Street Racer (Europe) (En,Fr,De,Es) (Rev B)
```

Versions can be semantic:

```text
F1 World Grand Prix for Dreamcast v1.011 (1999)(Video System)(JP)(en)[!]
F1 World Grand Prix for Dreamcast v1.000 (1999)(Video System)(PAL)(M4)[!]
F1 World Grand Prix v1.006 (2000)(Video System)(US)(M4)[!]
```

Ring codes can be numeric:

```text
Sonic CD (USA) (RE125)
Sonic CD (USA) (RE125) (Alt)
```

### Prefer retail

```text
Expand Down
20 changes: 8 additions & 12 deletions src/modules/argumentsParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import Options, {
FixExtension,
GameSubdirMode,
InputChecksumArchivesMode,
MergeMode,
MergeMode, PreferRevision,
} from '../types/options.js';
import PatchFactory from '../types/patches/patchFactory.js';

Expand Down Expand Up @@ -730,18 +730,14 @@ export default class ArgumentsParser {
}
return true;
})
.option('prefer-revision-newer', {
.option('prefer-revision', {
group: groupRomPriority,
description: 'Prefer newer ROM revisions over older',
type: 'boolean',
conflicts: ['prefer-revision-older'],
implies: 'single',
})
.option('prefer-revision-older', {
group: groupRomPriority,
description: 'Prefer older ROM revisions over newer',
type: 'boolean',
conflicts: ['prefer-revision-newer'],
description: 'Prefer older or newer revisions, versions, or ring codes',
choices: Object.keys(PreferRevision)
.filter((mode) => Number.isNaN(Number(mode)))
.map((mode) => mode.toLowerCase()),
coerce: ArgumentsParser.getLastValue, // don't allow string[] values
requiresArg: true,
implies: 'single',
})
.option('prefer-retail', {
Expand Down
6 changes: 3 additions & 3 deletions src/modules/candidatePreferer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import ProgressBar, { ProgressBarSymbol } from '../console/progressBar.js';
import fsPoly from '../polyfill/fsPoly.js';
import DAT from '../types/dats/dat.js';
import Parent from '../types/dats/parent.js';
import Options from '../types/options.js';
import Options, { PreferRevision } from '../types/options.js';
import ReleaseCandidate from '../types/releaseCandidate.js';
import Module from './module.js';

Expand Down Expand Up @@ -188,9 +188,9 @@ export default class CandidatePreferer extends Module {
}

private preferRevisionSort(a: ReleaseCandidate, b: ReleaseCandidate): number {
if (this.options.getPreferRevisionNewer()) {
if (this.options.getPreferRevision() === PreferRevision.NEWER) {
return b.getGame().getRevision() - a.getGame().getRevision();
} if (this.options.getPreferRevisionOlder()) {
} if (this.options.getPreferRevision() === PreferRevision.OLDER) {
return a.getGame().getRevision() - b.getGame().getRevision();
}
return 0;
Expand Down
28 changes: 15 additions & 13 deletions src/types/options.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ export enum FixExtension {
ALWAYS = 3,
}

export enum PreferRevision {
OLDER = 1,
NEWER = 2,
}

export interface OptionsProps {
readonly commands?: string[],

Expand Down Expand Up @@ -149,8 +154,7 @@ export interface OptionsProps {
readonly preferGood?: boolean,
readonly preferLanguage?: string[],
readonly preferRegion?: string[],
readonly preferRevisionNewer?: boolean,
readonly preferRevisionOlder?: boolean,
readonly preferRevision?: string,
readonly preferRetail?: boolean,
readonly preferParent?: boolean,

Expand Down Expand Up @@ -332,9 +336,7 @@ export default class Options implements OptionsProps {

readonly preferRegion: string[];

readonly preferRevisionNewer: boolean;

readonly preferRevisionOlder: boolean;
readonly preferRevision?: string;

readonly preferRetail: boolean;

Expand Down Expand Up @@ -453,8 +455,7 @@ export default class Options implements OptionsProps {
this.preferGood = options?.preferGood ?? false;
this.preferLanguage = options?.preferLanguage ?? [];
this.preferRegion = options?.preferRegion ?? [];
this.preferRevisionNewer = options?.preferRevisionNewer ?? false;
this.preferRevisionOlder = options?.preferRevisionOlder ?? false;
this.preferRevision = options?.preferRevision;
this.preferRetail = options?.preferRetail ?? false;
this.preferParent = options?.preferParent ?? false;

Expand Down Expand Up @@ -1198,12 +1199,13 @@ export default class Options implements OptionsProps {
return Options.filterUniqueUpper(this.preferRegion);
}

getPreferRevisionNewer(): boolean {
return this.preferRevisionNewer;
}

getPreferRevisionOlder(): boolean {
return this.preferRevisionOlder;
getPreferRevision(): PreferRevision | undefined {
const preferRevision = Object.keys(PreferRevision)
.find((mode) => mode.toLowerCase() === this.preferRevision?.toLowerCase());
if (!preferRevision) {
return undefined;
}
return PreferRevision[preferRevision as keyof typeof PreferRevision];
}

getPreferRetail(): boolean {
Expand Down
32 changes: 9 additions & 23 deletions test/modules/argumentsParser.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
FixExtension,
GameSubdirMode,
InputChecksumArchivesMode,
MergeMode,
MergeMode, PreferRevision,
} from '../../src/types/options.js';

const dummyRequiredArgs = ['--input', os.devNull, '--output', os.devNull];
Expand Down Expand Up @@ -199,8 +199,7 @@ describe('options', () => {
expect(options.getPreferGood()).toEqual(false);
expect(options.getPreferLanguages()).toHaveLength(0);
expect(options.getPreferRegions()).toHaveLength(0);
expect(options.getPreferRevisionNewer()).toEqual(false);
expect(options.getPreferRevisionOlder()).toEqual(false);
expect(options.getPreferRevision()).toBeUndefined();
expect(options.getPreferRetail()).toEqual(false);
expect(options.getPreferParent()).toEqual(false);

Expand Down Expand Up @@ -753,26 +752,13 @@ describe('options', () => {
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--single', '--prefer-region', 'USA,usa']).getPreferRegions()).toEqual(['USA']);
});

it('should parse "prefer-revision-newer"', () => {
expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-newer', '--prefer-revision-older', '--single'])).toThrow(/mutually exclusive/i);
expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-newer'])).toThrow(/dependent|implication/i);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-newer', '--single']).getPreferRevisionNewer()).toEqual(true);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-newer', 'true', '--single']).getPreferRevisionNewer()).toEqual(true);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-newer', 'false', '--single']).getPreferRevisionNewer()).toEqual(false);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-newer', '--prefer-revision-newer', '--single']).getPreferRevisionNewer()).toEqual(true);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-newer', 'false', '--prefer-revision-newer', 'true', '--single']).getPreferRevisionNewer()).toEqual(true);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-newer', 'true', '--prefer-revision-newer', 'false', '--single']).getPreferRevisionNewer()).toEqual(false);
});

it('should parse "prefer-revision-older"', () => {
expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-older', '--prefer-revision-newer', '--single'])).toThrow(/mutually exclusive/i);
expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-older'])).toThrow(/dependent|implication/i);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-older', '--single']).getPreferRevisionOlder()).toEqual(true);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-older', 'true', '--single']).getPreferRevisionOlder()).toEqual(true);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-older', 'false', '--single']).getPreferRevisionOlder()).toEqual(false);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-older', '--prefer-revision-older', '--single']).getPreferRevisionOlder()).toEqual(true);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-older', 'false', '--prefer-revision-older', 'true', '--single']).getPreferRevisionOlder()).toEqual(true);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision-older', 'true', '--prefer-revision-older', 'false', '--single']).getPreferRevisionOlder()).toEqual(false);
it('should parse "prefer-revision"', () => {
expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision', 'newer'])).toThrow(/dependent|implication/i);
expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--prefer-revision', 'foobar']).getMergeRoms()).toThrow(/invalid values/i);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--single', '--prefer-revision', 'older']).getPreferRevision()).toEqual(PreferRevision.OLDER);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--single', '--prefer-revision', 'older', '--prefer-revision', 'newer']).getPreferRevision()).toEqual(PreferRevision.NEWER);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--single', '--prefer-revision', 'newer']).getPreferRevision()).toEqual(PreferRevision.NEWER);
expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--single', '--prefer-revision', 'newer', '--prefer-revision', 'older']).getPreferRevision()).toEqual(PreferRevision.OLDER);
});

it('should parse "prefer-retail"', () => {
Expand Down
46 changes: 7 additions & 39 deletions test/modules/candidatePreferer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import LogiqxDAT from '../../src/types/dats/logiqx/logiqxDat.js';
import Parent from '../../src/types/dats/parent.js';
import Release from '../../src/types/dats/release.js';
import ROM from '../../src/types/dats/rom.js';
import Options, { OptionsProps } from '../../src/types/options.js';
import Options, { OptionsProps, PreferRevision } from '../../src/types/options.js';
import ReleaseCandidate from '../../src/types/releaseCandidate.js';
import ROMWithFiles from '../../src/types/romWithFiles.js';
import ProgressBarFake from '../console/progressBarFake.js';
Expand Down Expand Up @@ -421,28 +421,12 @@ describe('sort', () => {
});

describe('prefer revision newer', () => {
test.each([
[['one'], 'one'],
[['two', 'two (Rev 1)'], 'two'],
[['three', 'three (Rev 1)', 'three (Rev2)'], 'three'],
[['four (Rev 1.1)', 'four (Rev 1.2)'], 'four (Rev 1.1)'],
[['five (Rev 13.37)'], 'five (Rev 13.37)'],
[['six (Rev B)', 'six (Rev A)', 'six (Rev C)'], 'six (Rev B)'],
[['seven (RE2)', 'seven (RE3)', 'seven'], 'seven (RE2)'],
])('should return the first candidate when option is false: %s', async (names, expectedName) => {
expectPreferredCandidates(
{ preferRevisionNewer: false, single: true },
[await buildReleaseCandidatesWithRegionLanguage(names)],
[expectedName],
);
});

test.each([
[['one'], 'one'],
[['two', 'two two'], 'two'],
])('should return the first candidate when none matching: %s', async (names, expectedName) => {
expectPreferredCandidates(
{ preferRevisionNewer: true, single: true },
{ preferRevision: PreferRevision[PreferRevision.NEWER].toLowerCase(), single: true },
[await buildReleaseCandidatesWithRegionLanguage(names)],
[expectedName],
);
Expand All @@ -458,7 +442,7 @@ describe('sort', () => {
[['seven (RE2)', 'seven (RE3)', 'seven'], 'seven (RE3)'],
])('should return the first matching candidate when some matching: %s', async (names, expectedName) => {
expectPreferredCandidates(
{ preferRevisionNewer: true, single: true },
{ preferRevision: PreferRevision[PreferRevision.NEWER].toLowerCase(), single: true },
[await buildReleaseCandidatesWithRegionLanguage(names)],
[expectedName],
);
Expand All @@ -474,36 +458,20 @@ describe('sort', () => {
], 'ChuChu Rocket! v1.014 (2000)(Sega)(PAL)(M5)[!]'],
])('should return the first candidate when all matching: %s', async (names, expectedName) => {
expectPreferredCandidates(
{ preferRevisionNewer: true, single: true },
{ preferRevision: PreferRevision[PreferRevision.NEWER].toLowerCase(), single: true },
[await buildReleaseCandidatesWithRegionLanguage(names)],
[expectedName],
);
});
});

describe('prefer revision older', () => {
test.each([
[['one'], 'one'],
[['two', 'two (Rev 1)'], 'two'],
[['three', 'three (Rev 1)', 'three (Rev2)'], 'three'],
[['four (Rev 1.1)', 'four (Rev 1.2)'], 'four (Rev 1.1)'],
[['five (Rev 13.37)'], 'five (Rev 13.37)'],
[['six (Rev B)', 'six (Rev A)', 'six (Rev C)'], 'six (Rev B)'],
[['seven (RE2)', 'seven (RE3)', 'seven'], 'seven (RE2)'],
])('should return the first candidate when option is false: %s', async (names, expectedName) => {
expectPreferredCandidates(
{ preferRevisionOlder: false, single: true },
[await buildReleaseCandidatesWithRegionLanguage(names)],
[expectedName],
);
});

test.each([
[['one'], 'one'],
[['two', 'two two'], 'two'],
])('should return the first candidate when none matching: %s', async (names, expectedName) => {
expectPreferredCandidates(
{ preferRevisionOlder: true, single: true },
{ preferRevision: PreferRevision[PreferRevision.OLDER].toLowerCase(), single: true },
[await buildReleaseCandidatesWithRegionLanguage(names)],
[expectedName],
);
Expand All @@ -519,7 +487,7 @@ describe('sort', () => {
[['seven (RE2)', 'seven (RE3)', 'seven'], 'seven'],
])('should return the first matching candidate when some matching: %s', async (names, expectedName) => {
expectPreferredCandidates(
{ preferRevisionOlder: true, single: true },
{ preferRevision: PreferRevision[PreferRevision.OLDER].toLowerCase(), single: true },
[await buildReleaseCandidatesWithRegionLanguage(names)],
[expectedName],
);
Expand All @@ -530,7 +498,7 @@ describe('sort', () => {
[['two (Rev 13.37)'], 'two (Rev 13.37)'],
])('should return the first candidate when all matching: %s', async (names, expectedName) => {
expectPreferredCandidates(
{ preferRevisionOlder: true, single: true },
{ preferRevision: PreferRevision[PreferRevision.OLDER].toLowerCase(), single: true },
[await buildReleaseCandidatesWithRegionLanguage(names)],
[expectedName],
);
Expand Down

0 comments on commit 2811035

Please sign in to comment.