Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error where ISO files are prioritised over CUE files #794

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

michael-j-green
Copy link
Contributor

Closes #792

@michael-j-green michael-j-green self-assigned this Mar 5, 2024
@michael-j-green michael-j-green added the bug Something isn't working label Mar 5, 2024
if (isoFile !== null && (supportsExt('iso') || supportsExt('cso') || supportsExt('chd') || supportsExt('elf'))) {
this.fileName = isoFile;
} else if (supportsExt('cue') || supportsExt('ccd') || supportsExt('toc') || supportsExt('m3u')) {
if (supportsExt('cue') || supportsExt('ccd') || supportsExt('toc') || supportsExt('m3u')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When core supports both cue and iso and iso file provided, it will try to load (or create) cue and will not load iso.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the goal.

Ideally a user would only be providing a cue file if they intend to use the cue image loading method, so I don’t think this should be an issue.

Certainly better than those ROMs that have both a cue and iso and require the image to be loaded via the cue but end up trying to load the iso instead resulting in a failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh wait... that's not what you wrote :D

You meant core supports both core and iso and only an iso is provided, it'll try and create a cue and fail to load the iso.

Apologies. I'll dig back into this again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ethanaobrien; this issue has reared it's head again - this time with Amiga. It's trying to use cue files which simply don't exist.

I'm wondering if we shouldn't re-evalute the logic for the selection of files...

Could we use something like the following psuedo code?

if game file extension ends in not [ zip, 7z ] then start emulator with provided file name
else

extract file to /game (so files aren’t mixed with others)
determine if common files exist [ m3u, iso, chd, cue ]
if (m3u) start emulator with the m3u file
if (cue) start emulator with the cue file
if (chd) start emulator with the chd file
if (iso) create cue file and start emulator with new cue file

Copy link
Member

Choose a reason for hiding this comment

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

this time with Amiga.

How is this issue platform specific?

It's trying to use cue files which simply don't exist.

Any clue why? I’d rather fix that then change the file name extensions since not all cores support compressed files

Could we use something like the following psuedo code?

For specific systems, it should work

As a note - the only reason cue generation exists is because of the mednafen psx core. Back when the core was first pushed to stable, the amount of people that couldn’t configure the file properly was huge, and would open an issue/complain about a bug when it was just a very common user error, so I wrote the generate cue function to fix users mistakes

(as well as needing a cue, the core is also very specific about the paths of the files in the cue file)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah... this makes sense. I didn't know the reasoning behind creating the CUE files.

I'll have to ruminate some more on it.

BTW: I found the Amiga fault, I was sure that I had added puae to

if (['pcsx_rearmed', 'genesis_plus_gx', 'picodrive', 'mednafen_pce', 'smsplus', 'vice_x64', 'vice_x64sc', 'vice_x128', 'vice_xvic', 'vice_xplus4', 'vice_xpet'].includes(this.getCore()) && this.config.disableCue === undefined) {

This is my fault. I'll submit a PR to fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ISO files are prioritised over CUE files
3 participants