Skip to content

fix: resolve cue FILE refs by basename against cue dir#2403

Merged
Hachi-R merged 5 commits into
mainfrom
fix/sku-bin-path
Jul 2, 2026
Merged

fix: resolve cue FILE refs by basename against cue dir#2403
Hachi-R merged 5 commits into
mainfrom
fix/sku-bin-path

Conversation

@Moyasee

@Moyasee Moyasee commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

When submitting this pull request, I confirm the following (please check the boxes):

  • I have read the Hydra documentation.
  • I have checked that there are no duplicate pull requests related to this request.
  • I have considered, and confirm that this submission is valuable to others.
  • I accept that this submission may not be used and the pull request may be closed at the discretion of the maintainers.

Fill in the PR content:

PS1/PS2 disc SKU extraction during LaunchBox ROM import returned null whenever a .cue sheet's FILE entry pointed somewhere other than a sibling filename. Some discs were mastered with a stale absolute path, e.g.:

FILE "C:\WINDOWS\DESKTOP\CASTLEVANIA.BIN" BINARY

parseCueReferencedFiles fed that value straight into path.resolve, which discards the cue's own directory for an absolute/\-separated path and hands back the bogus location. fs.open then throws ENOENT, the generic catch swallows it, and the real sibling .bin is never scanned.

What this does

  • Strips the FILE value to its basename (normalizing \ to / first) and resolves it against the cue's own directory.
  • If the resolved path is missing on a case-sensitive filesystem (cue says CASTLEVANIA.BIN, file is Castlevania.bin), it scans the directory and matches the basename case-insensitively, returning the actual on-disk name.
  • Adds an explicit logger.warn in extractPs12Sku when the resolved sniff target does not exist, so future cases are diagnosable instead of failing silently.

Testing

Verified end-to-end against a real affected ROM (Castlevania.cue + Castlevania.bin): the cue's bad C:\WINDOWS\DESKTOP\CASTLEVANIA.BIN now resolves to the sibling Castlevania.bin, and the scan yields the correct SKU SLUS-00067. tsc typecheck passes.

PS1/PS2 SKU extraction returned null when a .cue FILE entry pointed to
an absolute or foreign path (e.g. C:\WINDOWS\DESKTOP\CASTLEVANIA.BIN).
path.resolve kept the bogus path and the sibling .bin was never scanned.

Strip the FILE value to its basename (normalizing \ to /) and resolve it
against the cue's own directory. When the resolved path is missing on a
case-sensitive filesystem, scan the directory and match the basename
case-insensitively, returning the real on-disk name.

Also warn explicitly in extractPs12Sku when the resolved sniff target
does not exist, instead of failing silently in the generic catch.
@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes CUE sheet FILE reference resolution in parseCueReferencedFiles so that stale absolute paths (e.g. C:\\WINDOWS\\DESKTOP\\CASTLEVANIA.BIN) no longer prevent sibling .bin files from being found during PS1/PS2 SKU extraction. It also adds an explicit existence-check with a logger.warn in extractPs12Sku so that unresolvable targets surface in logs rather than failing silently.

  • Introduces resolveCueRef: normalises backslashes, reduces any FILE path to its path.basename, resolves against the cue's own directory, and falls back to a case-insensitive readdir scan when the exact name is absent on a case-sensitive filesystem.
  • Guards the scan loop in extractPs12Sku with a pre-flight fs.access check and a warn-level log entry when the resolved target does not exist on disk.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to CUE path resolution and adds no new failure modes for the primary PS1/PS2 use case.

Both changed functions are self-contained and fail gracefully (returning null) on every error path. The stale-absolute-path bug is correctly fixed, the case-insensitive fallback is well-guarded, and the new existence check in extractPs12Sku improves diagnosability without altering control flow for the success path.

sniff-disc-platform.ts — the unconditional path.basename strip is worth revisiting if parseCueReferencedFiles is ever used for disc formats whose CUE sheets reference audio tracks in subdirectories.

Important Files Changed

Filename Overview
src/main/services/emulators/sniff-disc-platform.ts Adds resolveCueRef to normalize CUE FILE refs: strips backslashes, takes basename, resolves against cue dir, and falls back to a case-insensitive readdir scan. Correctly fixes the stale-absolute-path bug, but unconditionally discarding directory components via path.basename would silently break any legitimate relative sub-path refs (e.g. Audio/track02.wav).
src/main/services/emulators/extract-disc-sku.ts Adds an explicit fs.access guard with logger.warn before the file-open loop in extractPs12Sku. Straightforward defensive addition; no logic issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["parseCueReferencedFiles(cuePath)"] --> B["readFile + matchAll FILE refs"]
    B --> C["resolveCueRef(dir, ref) × N"]
    C --> D["path.basename — normalize \\ to /"]
    D --> E["path.resolve(dir, base)"]
    E --> F{"fs.access(resolved)?"}
    F -- yes --> G["return resolved"]
    F -- no --> H["fs.readdir(dir)"]
    H --> I{"case-insensitive match?"}
    I -- yes --> J["return path.resolve(dir, match)"]
    I -- no --> K["return null"]
    H -- throws --> K
    G --> L["filter nulls → string[]"]
    J --> L
    K --> L
    L --> M["resolveSniffTarget → refs[0]"]
    M --> N["extractPs12Sku"]
    N --> O{"fs.access(target)?"}
    O -- missing --> P["logger.warn + return null"]
    O -- exists --> Q["fs.open + scan for SKU"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["parseCueReferencedFiles(cuePath)"] --> B["readFile + matchAll FILE refs"]
    B --> C["resolveCueRef(dir, ref) × N"]
    C --> D["path.basename — normalize \\ to /"]
    D --> E["path.resolve(dir, base)"]
    E --> F{"fs.access(resolved)?"}
    F -- yes --> G["return resolved"]
    F -- no --> H["fs.readdir(dir)"]
    H --> I{"case-insensitive match?"}
    I -- yes --> J["return path.resolve(dir, match)"]
    I -- no --> K["return null"]
    H -- throws --> K
    G --> L["filter nulls → string[]"]
    J --> L
    K --> L
    L --> M["resolveSniffTarget → refs[0]"]
    M --> N["extractPs12Sku"]
    N --> O{"fs.access(target)?"}
    O -- missing --> P["logger.warn + return null"]
    O -- exists --> Q["fs.open + scan for SKU"]
Loading

Reviews (2): Last reviewed commit: "refactor: return null from resolveCueRef..." | Re-trigger Greptile

Comment thread src/main/services/emulators/sniff-disc-platform.ts
@Moyasee Moyasee added bug Something isn't working help wanted Extra attention is needed labels Jun 24, 2026
So the exported parseCueReferencedFiles never hands a non-existent
path to callers; unresolvable refs are filtered out instead.
@Moyasee

Moyasee commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

@greptile review

Move the file open/scan loop into scanTargetForSku so extractPs12Sku
stays under the cognitive-complexity threshold, and switch the cue ref
backslash normalization to String#replaceAll.

@Hachi-R Hachi-R left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing this.

I reviewed the change and didn't find any blocker. The fix is nicely scoped, handles the bad absolute CUE refs case, and the case-insensitive fallback should help on Linux too.

@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

@Hachi-R Hachi-R merged commit 78796ad into main Jul 2, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants