bsnes-emu / bsnes

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

Mega Man II SNES port - broken graphics #278

Closed nadiaholmquist closed 1 year ago

nadiaholmquist commented 1 year ago

On the latest commit of bsnes as well as the stable version, the homebrew Mega Man II port seems to have completely broken graphics.

Screenshot is of the title screen: image In-game, sprites appear to be displayed correctly, but backgrounds are completely broken: image

ROM:

b2acff9da4af3bdab59ecbad808f10eb4f8d4dfc  Mega Man II SNES (Rev A - infidelity).sfc

I've verified on my sd2snes and Super Famicom that this ROM does work correctly on real hardware, and it also seems to be working in Snes9x as well.

carmiker commented 1 year ago

It's an heuristics failure. The board string should be LOROM-RAM vs LOROM-RAM#A. If I comment the relevant code, it suddenly works, but I have not had time to verify what else that will break, or how to better avoid appending the #A in this case.

orbea commented 1 year ago

The relevant line is:

https://github.com/bsnes-emu/bsnes/blob/5cefce5c08f74cfc80eee3f82a32d846144e5277/bsnes/heuristics/super-famicom.cpp#L310

Commenting it or changing 0x200000 to 0x100000 will allow the game to run, but as explained above this may not be the correct fix.

orbea commented 1 year ago

This version works out of the box. What revision is it?

2415d5d69600c3d7d0ce5738761a0bc0b2e60d9d  Mega Man II SNES (infidelity).sfc

Also this Manifest could be added to bsnes/Database/Super Famicom.bml.

game
  sha256:   91fca82b086b9e5e2b5191c969c25d47d50f6b1a9c1452ddb7bc3d8947210ad9
  label:    Mega Man II SNES (Rev A - infidelity)
  name:     Mega Man II SNES (Rev A - infidelity)
  title:    MEGA MAN II SNES
  region:   NTSC
  revision: 1.0
  board:    LOROM-RAM
    memory
      type: ROM
      size: 0x200000
      content: Program
    memory
      type: RAM
      size: 0x800
      content: Save
orbea commented 1 year ago

@Screwtapello Do you happen to know what the #A in LOROM-RAM#A means?

euan-forrester commented 1 year ago

Here's a comment that https://github.com/retrostage made in the in the retroblaster discord (they're a series of reprogrammable snes etc carts) when a similar issue appeared trying to get this game running on there:

looks like you just need to double-up the ROM data with a hex editor.

Long story short, the game's code erroneously jumps to a higher bank in the LoROM address space, but there is no data there. That's not saying his code is broken, but flash carts/emulators auto-mask off any address bigger than the original game should be using so if the bank doesn't initialize to the lower half of the address space then it glitches. The SNES Blaster doesn't have a big FPGA in there so it doesn't handle all addresses, so it can't do auto-masking.

I've tested it working on a SNES Blaster here, so just double-up the data in the ROM file and you'll be good to go.

Screwtapello commented 1 year ago

I can no longer find a copy of the original release of the Mega Man II SNES port, but I found a copy of Revision B, and I can confirm that the same broken graphics appear.

0feb1d157ec36964441b0bf143976884b6fe7bdb8a613ab8a9259cfd19f21e77  Mega Man II SNES (Rev B - infidelity).sfc

bsnes autogenerates the following manifest for it:

game
  sha256:   0feb1d157ec36964441b0bf143976884b6fe7bdb8a613ab8a9259cfd19f21e77
  label:    Mega Man II SNES (Rev B - infidelity)
  name:     Mega Man II SNES (Rev B - infidelity)
  title:    MEGA MAN II SNES
  region:   NTSC
  revision: 1.0
  board:    LOROM-RAM#A
    memory
      type: ROM
      size: 0x200000
      content: Program
    memory
      type: RAM
      size: 0x800
      content: Save

board: LOROM-RAM#A
  memory
    type:ROM
    content:Program
    map
      address:00-3f,80-bf:8000-ffff
      mask:0x8000
  memory
    type:RAM
    content:Save
    map
      address:70-7d,f0-ff:0000-ffff
      mask:0x8000

If I save the following as Mega Man II SNES (Rev B - infidelity).bml beside the ROM:

game
  sha256:   0feb1d157ec36964441b0bf143976884b6fe7bdb8a613ab8a9259cfd19f21e77
  label:    Mega Man II SNES (Rev B - infidelity)
  name:     Mega Man II SNES (Rev B - infidelity)
  title:    MEGA MAN II SNES
  region:   NTSC
  revision: 1.0
  board:    LOROM-RAM
    memory
      type: ROM
      size: 0x200000
      content: Program
    memory
      type: RAM
      size: 0x800
      content: Save

then the game works... not perfectly but at least better:

Mega Man II SNES (Rev B - infidelity)-001

(note the glitch tiles at the top of the screen).

Screwtapello commented 1 year ago

The difference between LOROM-RAM and LOROM-RAM#A is how the ROM data is mapped into the SNES memory map.

LOROM-RAM:     map address=00-7d,80-ff:8000-ffff mask=0x8000
LOROM-RAM#A:   map address=00-3f,80-bf:8000-ffff mask=0x8000

Both mappings fill the top 32KiB in each bank with data from the ROM. This ROM is 0x200000 bytes or 2MiB, which means it will be spread across 64 banks.

LOROM-RAM#A fills the 64 banks 00-3F with the data from the ROM, and then (since there's no more data in the ROM) the remaining 64 banks 80-BF mirror the first 64 banks. The banks 40-7D and C0-FF are open bus, I assume.

LOROM-RAM puts the data into every bank except 7E and 7F, the RAM banks. The 64 banks 00-3F are filled with the data from the ROM as before, the 62 banks 40-4D mirror the first 62 banks. I'm not sure whether bank 80 mirrors bank 00 or bank 3D, but I'm sure it does something sensible.

bsnes' heuristics basically say that if a ROM can fit within first 64 banks, it should only be there.

Screwtapello commented 1 year ago

I should investigate the history of these mappings. So far as I can tell, the one called LOROM-RAM used to be LOROM-RAM#B and before that, EXLOROM-RAM.

orbea commented 1 year ago

I can no longer find a copy of the original release of the Mega Man II SNES port, but I found a copy of Revision B, and I can confirm that the same broken graphics appear.

So I presume the unknown version with the sha256sum of e9c5d6db7451af6c3bb86fddcf8e78176a5cf26b1a0fafc806f2e12e85e5d017 which does work is Revision C.

Screwtapello commented 1 year ago

I have another file here named "Mega Man II MSU-1 (Rev C 4-26-23 - infidelity)" whose sha256sum is 47f257ae7e863382f11f760eb5b790a21be5abae33c2f6674aa76ecbc85084fe. It's still 2MiB large and doesn't work properly, however.

Perhaps there's a Rev D out there somewhere with that sha256sum? Is the file 4MiB large (being the result of "doubling up" the data, as described above?

orbea commented 1 year ago

The overscan issue may be unrelated and it appears to only happen if the intro is not skipped, I was able to reproduce it with all 3 revisions I have.

e9c5d6db7451af6c3bb86fddcf8e78176a5cf26b1a0fafc806f2e12e85e5d017  Mega Man II SNES (infidelity).sfc
91fca82b086b9e5e2b5191c969c25d47d50f6b1a9c1452ddb7bc3d8947210ad9  Mega Man II SNES (Rev A - infidelity).sfc
0feb1d157ec36964441b0bf143976884b6fe7bdb8a613ab8a9259cfd19f21e77  Mega Man II SNES (Rev B - infidelity).sfc

Only Rev B identifies itself within the game without relying on the filename, perhaps the unidentified version is a hacked version of Revision A? The Revision C MSU-1 version like you said has issues for me too.

Screwtapello commented 1 year ago

The Revision C MSU-1 version I have also identifies itself as "Rev C":

Mega Man II MSU1-001

I assume the unknown version is, as you suggest, a modified version of Rev A. Can you tell me the filesize?

orbea commented 1 year ago

I assume the unknown version is, as you suggest, a modified version of Rev A. Can you tell me the filesize?

All three roms I have are the same size, 2048 bytes.

Screwtapello commented 1 year ago

So, here's a thing.

The auto-generated manifest uses LOROM-RAM#A because the "RAM size" byte in the ROM header (offset $7FD8 if I'm reading this correctly) is set to 1, which means 2048 bytes of save RAM. However, Mega Man II famously uses a password system instead of save RAM. I booted this game inside Mesen-S, which logs whether save RAM is read or written, and after starting a game and running out of lives save RAM was still untouched. Perhaps it's used elsewhere for some other thing, I don't know.

If the game didn't request save RAM, bsnes would guess the LOROM mapping, which is just like LOROM-RAM except without the RAM (logically enough). I tried editing the manifest to force the LOROM mapping and it still seemed to run just fine. So perhaps the mystery version just has the RAM size set to 0 instead of 1?

Looking at LOROM-RAM vs. LOROM-RAM#A again, it seems the RAM layout is different as well as the ROM layout.

So the most crucial difference is: is the address $708000 mapped to ROM (LOROM-RAM) or RAM (LOROM-RAM#A)?

This collision is only possible if the game has more than 3.5MiB of ROM, or more than 32KiB of RAM, or the game is playing silly games with mirroring. This game has only 2MiB of ROM and (allegedly) 2KiB of RAM, but it does depend on a particular mirroring configuration.

There's a number of possible ways to fix this game:

That last one is the only one we can do here, however I'm terrified of making any changes to the heuristics because there's no actual reason or logic behind them, they're just heuristics tweaked over decades to get games working. Any change should be followed by a full-library regression test to make sure we haven't broken anything, and I'm not sure I want to do that for a single homebrew game, notable though it may be.

Perhaps I could develop a tool to generate heuristics for every game in the No-Intro set under the old rules, and also under the new rules, and see how many commercial games would be affected by the change. Those games would still need to be tested, and that wouldn't tell us anything about homebrew and romhacks that might be affected (imagine if it broke a popular fan translation!), but it would be a start.

orbea commented 1 year ago

Thanks for spelling out all of the details, but I would suspect the least disruptive way to fix this is to ship the manifest in Super Famicom.bml. Of course other then contacting infidelity and getting him to fix it in the game, I don't use twitter so someone else would need to tell him.

Screwtapello commented 1 year ago

...the least disruptive way to fix this is to ship the manifest in Super Famicom.bml.

Yeah, that'd solve this problem neatly now, but I very much don't want to establish a precedent for adding romhacks to the overrides database.

More research:

ares has the exact same heuristics and the exact same behaviour. Ideally we should solve this problem in the same way.

higan v106 (the last stable version before the heuristics rewrite, the bsnes fork, and the ares fork) works just fine. It produces this manifest for this ROM:

board region=ntsc
  rom name=program.rom size=0x200000
    map address=00-7d,80-ff:8000-ffff mask=0x8000
    map address=40-6f,c0-ef:0000-7fff mask=0x8000
  ram name=save.ram size=0x800
    map address=70-7d,f0-ff:0000-ffff

This is quite different to the mapping bsnes and ares use. It's the "LoROM" mapping for ROM + RAM; the "ExLoROM" mapping looks like this:

    markup.append(
      "  rom name=program.rom size=0x", hex(rom_size), "\n"
      "    map address=00-3f,80-bf:8000-ffff mask=0x8000\n"
      "    map address=40-7d:0000-ffff\n"
    );
    if(ram_size > 0) markup.append(
      "  ram name=save.ram size=0x", hex(ram_size), "\n"
      "    map address=20-3f,a0-bf:6000-7fff mask=0xe000\n"
      "    map address=70-7d:0000-7fff mask=0x8000\n"
    );

...and the choice between them looks like this:

    if(index == 0x7fc0 && size >= 0x401000) {
      type = Type::ExLoROM;
    }

In addition, nocash's fullsnes docs have a detailed list of what (commercial game) LoROM mappings exist, which clearly fall into a few categories:

  1. games with no RAM, which just map ROM to the top 32KiB of every possible bank
  2. 1MiB games with RAM, that map RAM into full 64KiB banks
  3. 2MiB games with RAM, that map RAM into the bottom 32KiB of the relevant banks
  4. larger games with RAM, that map RAM into the bottom 32KiB of the relevant banks

The first category is what (current) bsnes calls LOROM.

The second and third categories, bsnes calls both of them LOROM-RAM#A. In theory it could get away with reserving that name just for the second category, but I suspect Near wanted to support fan translations of category-1 games that expanded the ROM to obtain more storage space - if the original game happened to touch RAM in the upper 32KiB of a bank, it could be a pain to fix, and there's room in the memory map for RAM in the upper portion of those banks, so why not?

The fourth category is what bsnes calls LOROM-RAM.

higan v106's mapping seems to map ROM and RAM to the top half of banks 70-7D. I wonder which wins. I suppose it doesn't matter for this particular game (as mentioned, it doesn't seem to even look at RAM), but at least it mirrors more of the ROM. I wonder if I patch the manifest to map banks 00-6F, not just 00-3F, whether that will get this game working.

Screwtapello commented 1 year ago

Yep, this manifest gets the game working:

board: LOROM-RAM-hacked
  memory type=ROM content=Program
    map address=00-6f,80-ef:8000-ffff mask=0x8000
  memory type=RAM content=Save
    map address=70-7d,f0-ff:0000-ffff mask=0x8000

I'm tempted to just change the definition of LOROM-RAM#A to the above, leaving the logic about "size<=2MiB" intact. That seems a closer match to what higan v106 (probably) does, and gets this game working, while maintaining the heuristic that larger ROMs probably want RAM only mapped in the lower 32KiB of each bank.

Screwtapello commented 1 year ago

It looks like Mesen uses the same rules as higan v106, including RAM mapping overwriting ROM mapping:

https://github.com/SourMesen/Mesen2/blob/a16a4e27be98d8ffd6e3dc2de668d79052019827/Core/SNES/BaseCartridge.cpp#L446-L460

snes9x is a bit less structured, but it seems to be doing something similar.

Screwtapello commented 1 year ago

Falling asleep last night, it occurred to me that although it had been reported that it was a mapping problem, and although changing the mapping seemed to fix the problem, I hadn't actually seen any evidence of that.

Luckily, the problem reproduces in the first frame or two, so I could turn on ares' tracing mode and log every CPU instruction and DMA transfer to look for any access to memory outside the mapped 00-3f,80-bf banks. It turns out, yeah:

CPU  8083cb  lda #$bf                         A:0018 X:0000 Y:00c2 S:01ff D:0000 B:00 nvMXdIzC  V: 99 H: 814 I:1
CPU  8083cd  sta $4302              [004302]  A:00bf X:0000 Y:00c2 S:01ff D:0000 B:00 NvMXdIzC  V: 99 H: 826 I:1
CPU  8083d0  lda #$83                         A:00bf X:0000 Y:00c2 S:01ff D:0000 B:00 NvMXdIzC  V: 99 H: 850 I:1
CPU  8083d2  sta $4303              [004303]  A:0083 X:0000 Y:00c2 S:01ff D:0000 B:00 NvMXdIzC  V: 99 H: 862 I:1
CPU  8083d5  lda #$c0                         A:0083 X:0000 Y:00c2 S:01ff D:0000 B:00 NvMXdIzC  V: 99 H: 886 I:1
CPU  8083d7  sta $4304              [004304]  A:00c0 X:0000 Y:00c2 S:01ff D:0000 B:00 NvMXdIzC  V: 99 H: 898 I:1
[...]
CPU DMA: Channel 0: $c083bf ($a9) => $2118 V: 99 H:1080 I:1

As you can see, the game writes $bf, $83, $c0 to $4302..4, the DMA setup registers. In particular, $c0 is written to the HDMA Start Address (bank) register, and a few instructions later, a DMA transfer begins from bank $c0.

I hypothesise that if that $c0 were changed to $80 (where that data is actually mapped), the game would run fine, but I don't need that much verification - I see the problem, I have a solution, I understand why the solution solves the problem.

ShadowXeen commented 3 months ago

I have a Version of Megaman 2, which works on bsnes-mt. I think it was the first Version which was released. But unfortunately it has no msu-1 Audio and i can´t find a patch for that. I have the same graphic issues with the newer Versions.