emmercm / igir

๐Ÿ•น A zero-setup ROM collection manager that sorts, filters, extracts or archives, patches, and reports on collections of any size on any OS.
https://igir.io/
GNU General Public License v3.0
364 stars 18 forks source link

No-Intro NES Headered and Headerless DATs matching same headered ROMs #281

Closed chainsawsalad closed 1 year ago

chainsawsalad commented 1 year ago

Paste the command

npx igir@latest -vv copy report \
--dat ./Nintendo\ -\ Nintendo\ Entertainment\ System\ \(Headered\)\ \(Parent-Clone\)\ \(20230202-191513\).dat \
--dat ./Nintendo\ -\ Nintendo\ Entertainment\ System\ \(Headerless\)\ \(Parent-Clone\)\ \(20230202-191513\).dat \
--input ./ROMs/nes/Nintendo\ -\ Nintendo\ Entertainment\ System\ \(20220418-114958\)\ \[headered_NES2.0\]/Contra\ \(USA\).zip* \
--output ./No-Intro/ \
--dir-dat-name

Describe the bug

Running igir on most of my headered NES ROMs seems to match the DAT files for both the NES headered and headerless versions. The various sizes, CRC, MD5, SHA1 attributes are all different in the DAT files, but yet the result of the command is as if the ROMs are identical between headerless and headered versions.

The outcome of the command results in the same ROM file being copied to No-Intro/Nintendo - Nintendo Entertainment System (Headered) as well as No-Intro/Nintendo - Nintendo Entertainment System (Headerless).

The resulting CSV report has FOUND for both DAT entries.

Expected behavior

When running igir with both headered and headerless DATs, a headered ROM would only match the headered DAT entry, and a headerless ROM would only match the headerless DAT entry.

Debug logs

INFO:  DATScanner: Scanning DAT files
DEBUG: DATScanner: Found 2 DAT files
DEBUG: DATScanner: Enumerating DAT archives
DEBUG: DATScanner: Deserializing DAT XML to objects
DEBUG: DATScanner: Parsing DAT files
INFO:  DATScanner: Done scanning DAT files
โœ“ Scanning for DATs ยทยทยทยทยทยทยทยทยทยทยทยท | 2 unique DATs found
INFO:  ROMScanner: Scanning ROM files
DEBUG: ROMScanner: Found 1 ROM file
INFO:  ROMScanner: Done scanning ROM files
โœ“ Scanning for ROMs ยทยทยทยทยทยทยทยทยทยทยทยท | 1 unique ROM found
INFO:  HeaderProcessor: Processing file headers
INFO:  HeaderProcessor: Done processing file headers
โœ“ Detecting ROM headers ยทยทยทยทยทยทยทยท | 1 ROM processed
INFO:  Processing 2 DATs
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Generating candidates
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Generating candidates
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): 2 unique ROMs found
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): 2 unique ROMs found
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): generated 128KiB of 1 candidate for 2,501 parents
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Done generating candidates
INFO:  PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Generating patched candidates
DEBUG: PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): 0 unique patches found
INFO:  PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Done generating patched candidates
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Filtering candidates
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): filtered to 128KiB of 1 candidate for 2,501 parents
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Done filtering candidates
INFO:  ROMWriter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Writing candidates
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): generated 128KiB of 1 candidate for 2,501 parents
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done generating candidates
INFO:  PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Generating patched candidates
DEBUG: PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): 0 unique patches found
INFO:  PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done generating patched candidates
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Filtering candidates
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): filtered to 128KiB of 1 candidate for 2,501 parents
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done filtering candidates
INFO:  ROMWriter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Writing candidates
INFO:  ROMWriter: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Done writing candidates
INFO:  StatusGenerator: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Generating ROM statuses
INFO:  StatusGenerator: Nintendo - Nintendo Entertainment System (Headered) (Parent-Clone): Done generating ROM statuses
โœ“ Nintendo - Nintendo Entertainm | 1/4,602 games, 0/5 BIOSes, 1/2,905 retail releases written
INFO:  ROMWriter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done writing candidates
INFO:  StatusGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Generating ROM statuses
INFO:  StatusGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done generating ROM statuses
โœ“ Nintendo - Nintendo Entertainm | 1/4,602 games, 0/5 BIOSes, 1/2,905 retail releases written
INFO:  Done processing 2 DATs
INFO:  ReportGenerator: Generating report
DEBUG: ReportGenerator: No-Intro/igir_2023-02-07T23;46;48-05;00.csv: wrote 2 statuses
INFO:  ReportGenerator: Done generating report
โœ“ Generating report ยทยทยทยทยทยทยทยทยทยทยทยท | No-Intro/igir_2023-02-07T23;46;48-05;00.csv

igir version

1.2.0

Node.js version

12.22.12

Operating system

Linux

Additional context

No response

chainsawsalad commented 1 year ago

Worth noting that running the command with only the headerless DAT against the headered ROM results in an incorrect match:

npx igir@latest -vv copy report \
--dat ./Nintendo\ -\ Nintendo\ Entertainment\ System\ \(Headerless\)\ \(Parent-Clone\)\ \(20230202-191513\).dat \
--input ./ROMs/nes/Nintendo\ -\ Nintendo\ Entertainment\ System\ \(20220418-114958\)\ \[headered_NES2.0\]/Contra\ \(USA\).zip* \
--output ./No-Intro/ \
--dir-dat-name
INFO:  DATScanner: Scanning DAT files
DEBUG: DATScanner: Found 1 DAT file
DEBUG: DATScanner: Enumerating DAT archives
DEBUG: DATScanner: Deserializing DAT XML to objects
DEBUG: DATScanner: Parsing DAT files
INFO:  DATScanner: Done scanning DAT files
โœ“ Scanning for DATs ยทยทยทยทยทยทยทยทยทยทยทยท | 1 unique DAT found
INFO:  ROMScanner: Scanning ROM files
DEBUG: ROMScanner: Found 1 ROM file
INFO:  ROMScanner: Done scanning ROM files
โœ“ Scanning for ROMs ยทยทยทยทยทยทยทยทยทยทยทยท | 1 unique ROM found
INFO:  HeaderProcessor: Processing file headers
INFO:  HeaderProcessor: Done processing file headers
โœ“ Detecting ROM headers ยทยทยทยทยทยทยทยท | 1 ROM processed
INFO:  Processing 1 DAT
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Generating candidates
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): 2 unique ROMs found
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): generated 128KiB of 1 candidate for 2,501 parents
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done generating candidates
INFO:  PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Generating patched candidates
DEBUG: PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): 0 unique patches found
INFO:  PatchCandidateGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done generating patched candidates
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Filtering candidates
DEBUG: CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): filtered to 128KiB of 1 candidate for 2,501 parents
INFO:  CandidateFilter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done filtering candidates
INFO:  ROMWriter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Writing candidates
INFO:  ROMWriter: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done writing candidates
INFO:  StatusGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Generating ROM statuses
INFO:  StatusGenerator: Nintendo - Nintendo Entertainment System (Headerless) (Parent-Clone): Done generating ROM statuses
โœ“ Nintendo - Nintendo Entertainm | 1/4,602 games, 0/5 BIOSes, 1/2,905 retail releases written
INFO:  Done processing 1 DAT
INFO:  ReportGenerator: Generating report
DEBUG: ReportGenerator: No-Intro/igir_2023-02-07T23;57;27-05;00.csv: wrote 1 status
INFO:  ReportGenerator: Done generating report
โœ“ Generating report ยทยทยทยทยทยทยทยทยทยทยทยท | No-Intro/igir_2023-02-07T23;57;27-05;00.csv
emmercm commented 1 year ago

This was an intentional design choice from a while ago because I was thinking the average user of the tool probably doesn't know or care about the difference between headered and headerless ROMs. It's probably good to revisit that assumption.

With the headerless DAT, igir can/will match headered ROMs, and if you specify a writing command (copy or move) then the header will be removed on write. I didn't consider this report-only case.

Maybe the solution is to only do exact checksum matching when in report-only mode? I'm curious about what your thoughts are.

chainsawsalad commented 1 year ago

I am actually seeing this behavior with copy report, but I haven't tried it with move or with omitting report. Happy to test that if it's helpful.

I assumed this wasn't an intentional choice, given N64 DATs for BigEndian and ByteSwapped work as I expect (#282). Is that situation not very similar to the NES headered/headerless DATs?

emmercm commented 1 year ago

I'm less familiar with N64 ROMs, but I think the header indicates the structure of the rest of the bytes in the file. So unlike NES and other ROMs, it's not as simple to skip/remove some amount of beginning bytes.

Source: http://n64dev.org/romformats.html


I missed the copy command in your posts. Given the second post that only specified the headerless DAT, it should have automatically removed the header on those ROMs during write, could you compare the CRCs?

But to solve the original ask, would #282 excluding the headerless DAT fit your needs?

chainsawsalad commented 1 year ago

I just confirmed, copy DOES turn the headered version of the ROM in a headerless version. However, not if the ROM being scanned is in an archive. So using Contra (USA).nes as an input actually does work as you describe, but Contra (USA).zip (which contains Contra (USA).nes) results in identical copies of Contra (USA).zip in my output directory.

Maybe this is more of a niche use case than I originally assumed, as it only affects a collection of zipped ROMs.

emmercm commented 1 year ago

For that ROM you would need to specify igir copy zip to create a new zip, or igir copy extract to extract - either should apply the ROM manipulation.

But let me see if I can make all of this more obvious in the behavior.

emmercm commented 1 year ago

I think #284 will address this:

github-actions[bot] commented 12 months ago

:lock: Inactive issue lock

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.