SourMesen / Mesen-S

Mesen-S is a cross-platform (Windows & Linux) SNES emulator built in C++ and C#
GNU General Public License v3.0
415 stars 100 forks source link

SRAM storing incorrect data #131

Open sukuigo opened 4 years ago

sukuigo commented 4 years ago

doing various tests on different emulators to see the boundaries of how well they mapped higher sram values at $FFD8 in the rom header, I noticed a bug in which mesen-s was incorrectly grabbing data from the top half of the bank instead of going to the next bank (doing lorom tests)

sram in this case of my test should be: $70:0000-$70:7FFFF $71:0000-$71:7FFFF $72:0000-$72:7FFFF $73:0000-$73:7FFFF $74:0000-$74:7FFFF $75:0000-$75:7FFFF $76:0000-$76:7FFFF $77:0000-$77:7FFFF for a total of 256kbs using a value of $08 in $FFD8 of the rom header the behavior I got however was sram saving values between: $70:0000-$70:FFFF $71:0000-$71:FFFF $72:0000-$72:FFFF $73:0000-$73:FFFF causing garbage data to be saved to sram instead of the intended data this behavior was seen doing tests with other values in $FFD8 as well, any values that made sram higher than $8000 bytes gave me this behavior ($06-$08, 64,128,and 256kbs respectively, and values higher than $08 behaving as $08)

this behavior was not seen in other emulators I tested: bsnes, snes9x, and zsnes even, nor does it line up with knowledge that I have about memory mapping, and therefore seems like a bug

most emulators I tested allowed up to 128kbs of sram to be saved correctly (minus snes9x not properly mirroring sram at higher banks, thus discarding values saved at those addresses instead of overwriting them) and bsnes plus (which was not accepting higher values in $FFD8 correctly oddly enough, considering the normal branch of bsnes was working correctly, and allocating up to 256kbs)

I will go ahead and leave the homebrew code I wrote as an attachment for testing on your end if you feel so inclined to do so (there is a define !ram near the top for setting $FFD8)

test.txt

(edit: made a slight correction in relation to how the addresses given correlated to the sram value)

SourMesen commented 4 years ago

I think you're probably getting this due to this heuristic that's used to get some specific games to work properly:

if(_prgRomSize >= 1024 * 1024 * 2) {
    //For games >= 2mb in size, put save RAM at 70-7D/F0-FF:0000-7FFF (e.g: Fire Emblem: Thracia 776) 
    mm.RegisterHandler(0x70, 0x7D, 0x0000, 0x7FFF, _saveRamHandlers);
    mm.RegisterHandler(0xF0, 0xFF, 0x0000, 0x7FFF, _saveRamHandlers);
} else {
    //For games < 2mb in size, put save RAM at 70-7D/F0-FF:0000-FFFF (e.g: Wanderers from Ys) 
    mm.RegisterHandler(0x70, 0x7D, 0x0000, 0xFFFF, _saveRamHandlers);
    mm.RegisterHandler(0xF0, 0xFF, 0x0000, 0xFFFF, _saveRamHandlers);
}

Basically, save ram is being mapped over the full 0000-FFFF range of each bank, rather than only being in the first half, which is probably what is causing the results you're getting.

As far as I know, this works correctly for licensed games (very few games even have over 32kb of save ram, so the exact mappings probably make no difference in most scenarios.)

Unfortunately, despite it being typically split between "lorom" and "hirom" (and that's all the information the header provides), things are actually not that simple and cartridges do have some differences within those 2 categories, which makes it somewhat hard to have a solution that works properly for all scenarios.

sukuigo commented 4 years ago

I expected to run into some unusual behavior with my sram tests between various emulators (I dont have the capacity to do my tests on actual hardware I am afraid) since that much sram is never used in commercial games and was surprised it worked as well as it did honestly, I figured it was worth bringing to your attention

I dont know much about the hardware side of things, but I assume the issue you mentioned is with not knowing the exact pinouts of the cartridge rom and only working with the limited data given by the rom's header? which I guess there is no elegant solution for without having some sort of exemption list

if I am reading what you wrote correctly though, I wouldnt have this issue if I was using a rom image that is greater than 2MB, so I wont run into any data conflicts if I was using a 4MB rom image with rom data at $F0:8000-$FF:8000? and in that case its checking the actual length of the rom image itself not $FFD7 (rom size) in the header? that was my main concern honestly, so if its a non issue I suppose its not a big deal

SourMesen commented 4 years ago

Exactly, the rom header does not give enough information to properly recreate the memory mappings of each cart. The code sometimes relies on the title or the "game code" to hardcode some specific mappings that some titles need, and other times resorts to heuristics like this to try and use something that works for all known licensed titles - which is obviously a problem for homebrew at times. I think higan/bsnes has some sort of manifest file that can be used to specify the mappings for a rom, but I'm not quite sure how this works, and I think it needs to be a separate file, which is not super convenient.

But yes, in this case, if your ROM is 2mb or larger, the heuristics should pick the other mapping for SRAM and you should get the same behavior as bsnes/snes9x/etc.