skmp / reicast-emulator

Reicast was a multiplatform Sega Dreamcast emulator
https://reicast.emudev.org
Other
1.1k stars 343 forks source link

Linux: cannot open gdi files that contain special characters #1006

Closed danboid closed 6 years ago

danboid commented 7 years ago

Linux reicast fails to open gdi files if any of the file names contained within contain special characters such as parentheses or commas.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/41666896-linux-cannot-open-gdi-files-that-contain-special-characters?utm_campaign=plugin&utm_content=tracker%2F500311&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F500311&utm_medium=issues&utm_source=github).
goigle commented 7 years ago

Which Linux are you on? I'm on Raspbian and my filenames have spaces and it works fine.

danboid commented 7 years ago

That's strange - I doubt the distro would matter but I run Arch.

danboid commented 6 years ago

Hi goigle

I have returned to try reicast again and I'm trying to run the same roms as last time. I noticed that the filenames of the files that make up the ROM I'm trying to run contain not only spaces but commas and parentheses so it is more likely one of those that reicast is choking on hence I have edited the title of this issue to reflect that.

Sarkie commented 6 years ago

Give an example filename...

danboid commented 6 years ago

One such problematic gdi attached, albeit with an added .txt extension

Gauntlet Legends (Europe) (En,Fr,De).gdi.txt

It reads:

5 1 0 4 2352 "Gauntlet Legends (Europe) (En,Fr,De) (Track 1).bin" 0 2 607 0 2352 "Gauntlet Legends (Europe) (En,Fr,De) (Track 2).bin" 0 3 45000 4 2352 "Gauntlet Legends (Europe) (En,Fr,De) (Track 3).bin" 0 4 245774 0 2352 "Gauntlet Legends (Europe) (En,Fr,De) (Track 4).bin" 0 5 246300 4 2352 "Gauntlet Legends (Europe) (En,Fr,De) (Track 5).bin" 0

AbandonedCart commented 6 years ago

Is this still an issue?

danboid commented 6 years ago

Yes, this is still an issue. I've just tried Sonic Adventure under the latest git version of reicast under Ubuntu 18.04. The gdi file as downloaded off archive.org looked like this, which didn't load:

3
1     0 4 2352 "Sonic Adventure (Europe) (En,Ja,Fr,De,Es) (Track 1).bin" 0
2 11361 0 2352 "Sonic Adventure (Europe) (En,Ja,Fr,De,Es) (Track 2).bin" 0
3 45000 4 2352 "Sonic Adventure (Europe) (En,Ja,Fr,De,Es) (Track 3).bin" 0

So I renamed the files and edited the .gdi file to look like this:

3
1     0 4 2352 SonicAdventureTrack1.bin 0
2 11361 0 2352 SonicAdventureTrack2.bin 0
3 45000 4 2352 SonicAdventureTrack3.bin 0

Then it loaded.

First I tried removing the brackets from the filenames, that didn't fix it. Next I tried removing the spaces with no luck. It wasn't until I removed the quotes surrounding the filenames within the gdi file that reicast successfully found and loaded all three bin files. With the quotes around the .bin filenames it only seemed to load the first .bin file so I would only see the DC BIOS menu and the game failed to load.

baka0815 commented 6 years ago

Looks like the issue could be line 112 in gdi.cpp: https://github.com/reicast/reicast-emulator/blob/1f088f211d273b86875b91b567bd08a6a9fce0fe/core/imgread/gdi.cpp#L112

baka0815 commented 6 years ago

Maybe I'll have time tomorrow to look into this.

baka0815 commented 6 years ago

It's all because the skipws-flag is not reset...

AbandonedCart commented 6 years ago

Back when this was ported based on claims that RA could handle whitepsace and reicast couldn't, it was a simple oversight.

            gdi >> std::noskipws;
            for(;;) {
                gdi >> last;
                if (last == '"')
                    break;
                track_filename += last;
            }
            gdi >> std::skipws;

So rather than a lot of changes to initialize variables and then explicitly define whitespace outside the actual usage, couldn't you just add the gdi >> std::skipws; after track_filename += last; and be done with it? Or even duplicate the original implementation by moving it around the for?

baka0815 commented 6 years ago

Ok, I resubmitted the fix in #1191.