Skip to content

Commit

Permalink
Fix: writing headered files instead of unheadered (#998)
Browse files Browse the repository at this point in the history
  • Loading branch information
@emmercm
emmercm committed Mar 8, 2024
1 parent 1f4ac62 commit 176d7ef
Show file tree
Hide file tree
Showing 13 changed files with 198 additions and 89 deletions.
72 changes: 47 additions & 25 deletions src/modules/candidateGenerator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,49 +111,73 @@ export default class CandidateGenerator extends Module {
// For each Game's ROM, find the matching File
const romFiles = await Promise.all(
game.getRoms().map(async (rom) => {
// NOTE(cemmer): if the ROM's CRC includes a header, then this will only find headered
// files. If the ROM's CRC excludes a header, this can find either a headered or non-
// headered file.
const originalInputFile = romsToInputFiles.get(rom);
if (!originalInputFile) {
if (!romsToInputFiles.has(rom)) {
return [rom, undefined];
}
let inputFile = romsToInputFiles.get(rom) as File;
/**
* WARN(cemmer): {@link inputFile} may not be an exact match for {@link rom}. There are two
* situations we can be in:
* - {@link rom} is headered and so is {@link inputFile}, so we have an exact match
* - {@link rom} is unheadered but {@link inputFile} is headered, because we know how to
* remove headers from ROMs - but we can't remove headers in all writing modes!
*/

// If we're not writing (report only) then just use the input file for the output file
if (!this.options.shouldWrite()) {
return [rom, new ROMWithFiles(rom, originalInputFile, originalInputFile)];
return [rom, new ROMWithFiles(rom, inputFile, inputFile)];
}

// If the input file is headered...
if (inputFile.getFileHeader()
// ..and we want a headered ROM
&& (inputFile.getCrc32() === rom.getCrc32()
|| inputFile.getMd5() === rom.getMd5()
|| inputFile.getSha1() === rom.getSha1())
// ...and we shouldn't remove the header
&& !this.options.canRemoveHeader(
dat,
path.extname(inputFile.getExtractedFilePath()),
)
) {
// ...then forget the input file's header, so that we don't later remove it
inputFile = inputFile.withoutFileHeader();
}

// If the input file is headered...
if (inputFile.getFileHeader()
// ...and we DON'T want a headered ROM
&& !(inputFile.getCrc32() === rom.getCrc32()
|| inputFile.getMd5() === rom.getMd5()
|| inputFile.getSha1() === rom.getSha1())
// ...and we're writing file links
&& this.options.shouldLink()
) {
// ...then we can't use this file
return [rom, undefined];
}

/**
* If the matched input file is from an archive, and we're not zipping or extracting, then
* treat the file as "raw" so it can be copied/moved as-is.
* Matches {@link ROMHeaderProcessor.getFileWithHeader}
*/
let finalInputFile = originalInputFile;
if (originalInputFile instanceof ArchiveEntry
&& !this.options.shouldZip(rom.getName())
if (inputFile instanceof ArchiveEntry
&& !this.options.shouldZipFile(rom.getName())
&& !this.options.shouldExtract()
) {
// No automatic header removal will be performed when raw-copying an archive, so return no
// match if we wanted a headerless ROM but got a headered one.
if (rom.hashCode() !== originalInputFile.hashCodeWithHeader()
&& rom.hashCode() === originalInputFile.hashCodeWithoutHeader()
) {
return [rom, undefined];
}

if (this.options.shouldTest() || this.options.getOverwriteInvalid()) {
// If we're testing, then we need to calculate the archive's CRC
finalInputFile = await originalInputFile.getArchive().asRawFile();
inputFile = await inputFile.getArchive().asRawFile();
} else {
// Otherwise, we can skip calculating the CRC for efficiency
finalInputFile = await originalInputFile.getArchive().asRawFileWithoutCrc();
inputFile = await inputFile.getArchive().asRawFileWithoutCrc();
}
}

try {
const outputFile = await this.getOutputFile(dat, game, release, rom, finalInputFile);
const romWithFiles = new ROMWithFiles(rom, finalInputFile, outputFile);
const outputFile = await this.getOutputFile(dat, game, release, rom, inputFile);
const romWithFiles = new ROMWithFiles(rom, inputFile, outputFile);
return [rom, romWithFiles];
} catch (error) {
this.progressBar.logWarn(`${dat.getNameShort()}: ${game.getName()}: ${error}`);
Expand Down Expand Up @@ -278,15 +302,13 @@ export default class CandidateGenerator extends Module {
// Determine the output CRC of the file
let outputFileCrc = inputFile.getCrc32();
let outputFileSize = inputFile.getSize();
if (inputFile.getFileHeader()
&& this.options.canRemoveHeader(dat, path.extname(outputPathParsed.entryPath))
) {
if (inputFile.getFileHeader()) {
outputFileCrc = inputFile.getCrc32WithoutHeader();
outputFileSize = inputFile.getSizeWithoutHeader();
}

// Determine the output file type
if (this.options.shouldZip(rom.getName())) {
if (this.options.shouldZipFile(rom.getName())) {
// Should zip, return an archive entry within an output zip
return ArchiveEntry.entryOf(
new Zip(outputFilePath),
Expand Down
7 changes: 1 addition & 6 deletions src/modules/candidateWriter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,17 +405,12 @@ export default class CandidateWriter extends Module {
inputRomFile: File,
outputFilePath: string,
): Promise<boolean> {
const removeHeader = this.options.canRemoveHeader(
dat,
path.extname(inputRomFile.getExtractedFilePath()),
);

this.progressBar.logInfo(`${dat.getNameShort()}: ${releaseCandidate.getName()}: copying file '${inputRomFile.toString()}' (${fsPoly.sizeReadable(inputRomFile.getSize())}) -> '${outputFilePath}'`);

try {
await CandidateWriter.ensureOutputDirExists(outputFilePath);
const tempRawFile = await fsPoly.mktemp(outputFilePath);
await inputRomFile.extractAndPatchToFile(tempRawFile, removeHeader);
await inputRomFile.extractAndPatchToFile(tempRawFile);
await fsPoly.mv(tempRawFile, outputFilePath);
return true;
} catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion src/modules/movedRomDeleter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export default class MovedROMDeleter extends Module {
}

// Otherwise, the entry needs to have been explicitly moved
return entry.hashCodes().some((hashCode) => !movedEntryHashCodes.has(hashCode));
return !entry.hashCodes().some((hashCode) => movedEntryHashCodes.has(hashCode));
});
if (unmovedEntries.length > 0) {
this.progressBar.logWarn(`${filePath}: not deleting moved file, ${unmovedEntries.length.toLocaleString()} archive entr${unmovedEntries.length !== 1 ? 'ies were' : 'y was'} unmatched:\n${unmovedEntries.sort().map((entry) => ` ${entry}`).join('\n')}`);
Expand Down
2 changes: 1 addition & 1 deletion src/modules/romHeaderProcessor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export default class ROMHeaderProcessor extends Module {
* Matches {@link CandidateGenerator.buildReleaseCandidateForRelease}
*/
if (inputFile instanceof ArchiveEntry
&& !this.options.canZip()
&& !this.options.shouldZip()
&& !this.options.shouldExtract()
) {
return inputFile;
Expand Down
10 changes: 10 additions & 0 deletions src/types/files/archives/archiveEntry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,16 @@ export default class ArchiveEntry<A extends Archive> extends File implements Arc
);
}

withoutFileHeader(): ArchiveEntry<A> {
return new ArchiveEntry({
...this,
fileHeader: undefined,
crc32WithoutHeader: this.getCrc32(),
md5WithoutHeader: this.getMd5(),
sha1WithoutHeader: this.getSha1(),
});
}

withPatch(patch: Patch): ArchiveEntry<A> {
if (patch.getCrcBefore() !== this.getCrc32()) {
return this;
Expand Down
41 changes: 17 additions & 24 deletions src/types/files/archives/zip.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,32 +202,25 @@ export default class Zip extends Archive {
3,
async.asyncify(async (
[inputFile, outputArchiveEntry]: [File, ArchiveEntry<Zip>],
): Promise<void> => {
const removeHeader = options.canRemoveHeader(
dat,
path.extname(inputFile.getExtractedFilePath()),
);
): Promise<void> => inputFile.createPatchedReadStream(async (stream) => {
// Catch stream errors such as `ENOENT: no such file or directory`
stream.on('error', catchError);

return inputFile.createPatchedReadStream(removeHeader, async (stream) => {
// Catch stream errors such as `ENOENT: no such file or directory`
stream.on('error', catchError);

const entryName = outputArchiveEntry.getEntryPath().replace(/[\\/]/g, '/');
zipFile.append(stream, {
name: entryName,
});

// Leave the input stream open until we're done writing it
await new Promise<void>((resolve) => {
const interval = setInterval(() => {
if (writtenEntries.has(entryName) || zipFileError) {
clearInterval(interval);
resolve();
}
}, 10);
});
const entryName = outputArchiveEntry.getEntryPath().replace(/[\\/]/g, '/');
zipFile.append(stream, {
name: entryName,
});
}),

// Leave the input stream open until we're done writing it
await new Promise<void>((resolve) => {
const interval = setInterval(() => {
if (writtenEntries.has(entryName) || zipFileError) {
clearInterval(interval);
resolve();
}
}, 10);
});
})),
);

if (zipFileError) {
Expand Down
20 changes: 13 additions & 7 deletions src/types/files/file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,8 @@ export default class File implements FileProps {
});
}

async extractAndPatchToFile(
destinationPath: string,
removeHeader: boolean,
): Promise<void> {
const start = removeHeader && this.getFileHeader()
async extractAndPatchToFile(destinationPath: string): Promise<void> {
const start = this.getFileHeader()
? this.getFileHeader()?.getDataOffsetBytes() ?? 0
: 0;
const patch = this.getPatch();
Expand Down Expand Up @@ -319,10 +316,9 @@ export default class File implements FileProps {
}

async createPatchedReadStream<T>(
removeHeader: boolean,
callback: (stream: Readable) => (T | Promise<T>),
): Promise<T> {
const start = removeHeader && this.getFileHeader()
const start = this.getFileHeader()
? this.getFileHeader()?.getDataOffsetBytes() ?? 0
: 0;
const patch = this.getPatch();
Expand Down Expand Up @@ -417,6 +413,16 @@ export default class File implements FileProps {
);
}

withoutFileHeader(): File {
return new File({
...this,
fileHeader: undefined,
crc32WithoutHeader: this.getCrc32(),
md5WithoutHeader: this.getMd5(),
sha1WithoutHeader: this.getSha1(),
});
}

withPatch(patch: Patch): File {
if (patch.getCrcBefore() !== this.getCrc32()) {
return this;
Expand Down
6 changes: 3 additions & 3 deletions src/types/options.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,15 +523,15 @@ export default class Options implements OptionsProps {
/**
* Was the `zip` command provided?
*/
canZip(): boolean {
shouldZip(): boolean {
return this.getCommands().has('zip');
}

/**
* Should a given output file path be zipped?
*/
shouldZip(filePath: string): boolean {
return this.canZip()
shouldZipFile(filePath: string): boolean {
return this.shouldZip()
&& (!this.getZipExclude() || !micromatch.isMatch(
filePath.replace(/^.[\\/]/, ''),
this.getZipExclude(),
Expand Down
10 changes: 3 additions & 7 deletions src/types/outputFactory.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ export default class OutputFactory {
inputFile: File,
): string {
// Determine the output path of the file
if (options.shouldZip(rom.getName())) {
if (options.shouldZipFile(rom.getName())) {
// Should zip, generate the zip name from the game name
return `${game.getName()}.zip`;
}
Expand Down Expand Up @@ -513,7 +513,7 @@ export default class OutputFactory {
inputFile: File,
): string {
const romBasename = this.getRomBasename(options, dat, rom, inputFile);
if (!options.shouldZip(rom.getName())) {
if (!options.shouldZipFile(rom.getName())) {
return romBasename;
}

Expand Down Expand Up @@ -543,11 +543,7 @@ export default class OutputFactory {
const fileHeader = inputFile.getFileHeader();
if (parsedRomPath.ext && fileHeader) {
// If the ROM has a header, then we're going to ignore the file extension from the DAT
if (options.canRemoveHeader(dat, parsedRomPath.ext)) {
parsedRomPath.ext = fileHeader.getUnheaderedFileExtension();
} else {
parsedRomPath.ext = fileHeader.getHeaderedFileExtension();
}
parsedRomPath.ext = fileHeader.getUnheaderedFileExtension();
}

return path.format(parsedRomPath);
Expand Down
42 changes: 42 additions & 0 deletions test/fixtures/dats/unheadered.dat
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
clrmamepro (
name "Unheadered"
description "Unheadered"
version 20240307
author emmercm
)

game (
name "allpads"
description "allpads"
rom ( name allpads.nes size 32768 crc 6339abe6 md5 d632fd16189bd32f574b0b12c10ede47 sha1 f181104ca6ea30fa166260ab29395ce1fcdaa48e )
)

game (
name "color_test"
description "color_test"
rom ( name color_test.nes size 40976 crc c9c1b7aa md5 0c09743a41d469c1da119316a23f15ba sha1 7f5a5f590c17b8178350f79883caa18d8df305a1 )
)

game (
name "diagnostic_test_cartridge"
description "diagnostic_test_cartridge"
rom ( name diagnostic_test_cartridge.a78 size 32768 crc a1eaa7c1 md5 91041aadd1700a7a4076f4005f2c362f sha1 76ec76c423d88bdf739e673c051c5b9c174881c6 )
)

game (
name "fds_joypad_test"
description "fds_joypad_test"
rom ( name fds_joypad_test.fds size 65484 crc 3ecbac61 md5 26df56a7e5b096577338bcc4c334ec7d sha1 7b6bd1a69bbc5d8121c72dd1eedfb6752fe11787 )
)

game (
name "LCDTestROM"
description "LCDTestROM"
rom ( name LCDTestROM.lyx size 5302 crc 42583855 md5 294a07b07ce67a9b492e4b6e77d6d2f7 sha1 e2901046126153b318a09cc1476eec8afff0b698 )
)

game (
name "speed_test_v51"
description "speed_test_v51"
rom ( name speed_test_v51.sfc size 65536 crc 8beffd94 md5 cf62b6d186954e6f5d9feec3adc8ffbc sha1 2f2f061ee81bafb4c48b83993a56969012162d68 )
)
Loading

0 comments on commit 176d7ef

Please sign in to comment.