bsnes-emu / bsnes

bsnes is a Super Nintendo (SNES) emulator focused on performance, features, and ease of use.
Other
1.69k stars 158 forks source link

Prevent Satellaview games smaller than 1024kb from crashing, Hide Bottom Most Scanline When Playing SGB Games & LibRetro: Allow MSU-1 SGB games to work properly #142

Closed ds22x closed 3 years ago

ds22x commented 3 years ago

This change allows for a SGB boot rom with the same name as a SGB rom to be loaded from the same folder, which allows for MSU-1 enhanced SGB games to work without having to rely on the subsystem menu. If it fails to load a SGB boot rom from the same folder, it will first revert to the already implemented method of loading the boot rom from the system directory and etc.

ds22x commented 3 years ago

Also added a small fix for loading Satellaview games smaller than 1024kb.

Screwtapello commented 3 years ago

Thank you for looking into these issues for us!

For directly loading Satellaview games, I think it's a good idea to automatically load the BS-X base cartridge, but for consistency it should probably be a setting like the SGB base cartridge is, rather than hard-coding the name BS-X.bin.

I'm pretty sure hard-coding the size of Satellaview games to 1MiB is not the correct approach, but I'm not sure how that's supposed to work, so I'll have to look into it.

For allowing GB games to load an SGB base cart ROM beside the GB ROM, I think you could make the patch smaller by doing something like:

string game_path = string(game->path).transform("\\", "/");
string sgb_path = game_path.replace(".gbc", ".sfc").replace(".gb", ".sfc");

...then you wouldn't have to treat the .gbc and .gb cases separately even though they share 90% of their logic.

That said, I'm not sure where MSU-1 enters into this. What happens if you load an SGB MSU-1 game without this change? Is it just that you have to configure the path to the SGB base ROM in a menu somewhere? Is that how most RetroArch cores work?

ds22x commented 3 years ago

I'm not really sure why the Satellaview BIOS name should be a separate Libretro setting, seeing as, as far as I know, the core only works with the Satellaview base cart anyway (games that use .bs rom paks for DLC for example will not load the extra data, even when using the subsystem), and the agreed name for the base cart between the Snes9x and Mesen-S cores is "BS-X.bin", so it only makes sense there.

For hard coding the Satellaview game size, a better solution could be found, but I couldn't find a better solution myself, and when testing it, I didn't find any ill effect from it.

I'll definitely look try out to simplify the Gameboy statement code.

And what normally happens when you try to load MSU1 SGB games with the Libretro core without this patch is that they'll hang as soon as an audio track is supposed to play, so this fixes it by allowing it to look for a SGB BIOS file in the same folder as the game before defaulting back to the system folder.

ds22x commented 3 years ago

My apologies for the long title and the seemingly unrelated addition, but I didn't know how to separately create a pull for it with a fork already existing.

near-san commented 3 years ago

On Satellaview: officially the only flash packs released were 1MB in size, always. The planned 2MB/4MB packs never happened. One pack could hold multiple smaller games, so you see groups like No-Intro trim those packs to smaller sizes like 256KB. My BS-X flash cart emulation is very intensely thorough and expects it to actually be 1MB. However, we should just update the code to pad the inputs to 1MB with 0xFF bytes in icarus instead.

There are mask ROM Satellaview carts for SD Gundam G-Next and Same Game that are smaller than 1MB, and those should already work fine without needing the padding.

I don't have any opinions on the rest of it, but I will say Screwtape tends to like patches like this split up ^-^ Thanks for your help with this!

ds22x commented 3 years ago

I see. In that case the code responsible for handling Satellaview flash packs should probably be updated instead, although during testing the heuristics change I didn't encounter any issues regarding the Same Game ROM packs when assuming the game size (although regardless of the change, the ROM packs only work with standalone bsnes anyway), so I'd figure that, unless in the future, bigger Satellaview carts are somehow unearthed, having it assume to be 1MB in size wouldn't make much of a difference.

But I'm somewhat confused about one thing. I thought that icarus was a separate used for higan to import and manage game libraries, whereas bsnes doesn't use it, as it directly loads game roms instead.

And I'm more than willing to separate this pull request into three separate ones if it's more convenient.

ds22x commented 3 years ago

Went and split this pull request into three separate ones.