"loadCartridge" always checks board incorrectly

[Morilli]

In loadCartridge, the current board is always checked via node["board"], which always fails, because it should be retrieved via node["game/board"] (or just by using game.board): https://github.com/bsnes-emu/bsnes/blob/e71da4d8c86d561445781cc00b6ac83de12e7d2d/bsnes/sfc/cartridge/load.cpp#L28-L30

This is not harmful in most cases, because the following loadBoard call is then always called and usually works, but for one (hacked) game I was just debugging this was precisely the error condition, as the heuristically determined board value for that game is HITACHI-LOROM-RAM, but loadBoard("HITACHI-LOROM-RAM") returns nothing. I'm not entirely sure what the intention here is in the first place, because the loadBoard call seems to always just return game.board itself again... in any case, it must be wrong that the board that was previously loaded in a game.load is only used in the loadBoard call and the node["board"] cannot ever work.

[Morilli]

Looking at the commit history, it seems to me that this may have actually been a leftover change that was never meant to be commited (see 5961ea9c0):

@@ -25,7 +26,8 @@ auto Cartridge::loadBoard(string board) -> Markup::Node {
 }

 auto Cartridge::loadCartridge(Markup::Node node) -> void {
-  board = loadBoard(game.board);
+  board = node["board"];
+  if(!board) board = loadBoard(game.board);

   if(region() == "Auto") {
     auto region = game.region;

The previous logic makes sense, and the only remaining issue would be that HITACHI- is not correctly replaced with SHVC-.

[orbea]

Are there any test cases showing where this is broken?

[Morilli]

I don't have anything test-like ready, but this should be an issue for all roms ever loaded, so it should be very easy to verify.

[orbea]

There are some roms that didn't boot correctly, but as far as I am aware they were all fixed in our fork. While this repo seems more historical reference at this point.

https://gitlab.com/jgemu/bsnes

A lot of theses issues were exposed when switching from nall:BML to byuuml (https://github.com/SolraBizna/byuuML), but I'm not sure if this was one or not. Of course if there are any that we missed we will fix them if possible.

[Morilli]

The rom in question is a patched mega man x3 rom from https://github.com/justin3009/MMX3-ZeroProject, however I cannot get this rom to load even with my mentioned fix in place, so I'm not sure how trustworthy it is. The rom works in previously versions of bsnes, so either something broke in newer versions of bsnes, or accuracy improvements happens and the rom got exposed as broken.

[orbea]

I can confirm neither of these work in this repo or in our fork. Do you know if they work on real hardware?

https://www.romhacking.net/hacks/888/ https://www.romhacking.net/hacks/4086/

[Morilli]

Do you know if they work on real hardware?

No, but I was wondering that as well. From personal experience I would assume no, but I can also not say what would be wrong about it.

[Morilli]

Okay, I've looked at it again and was able to figure out what causes this specific rom to fail and how to make it work: As I've mentioned in the original issue description, bsnes determines the board to be HITACHI-LOROM-RAM, which does not exist in the boards.bml file. Therefor, creating any .bml file without the specific needed board for it is wasted effort, because it cannot ever work. Adding an entry to boards.bml like the following:

board: HITACHI-LOROM-RAM
  processor architecture=HG51BS169
    map address=00-3f,80-bf:6c00-6fff,7c00-7fff
    memory type=ROM content=Program
      map address=00-7d,80-ff:8000-ffff mask=0x8000
    memory type=RAM content=Save
      map address=70-7d,f0-ff:0000-7fff mask=0x8000
    memory type=ROM content=Data architecture=HG51BS169
    memory type=RAM content=Data architecture=HG51BS169
      map address=00-3f,80-bf:6000-6bff,7000-7bff mask=0xf000
    oscillator

works, and makes the rom load without issues. The above board is basically just a copy of HITACHI-LOROM, with the program.rom and save.ram entries replaced with the ones from the LOROM-RAM board.

I am not sure what exactly the boards.bml file is trying to achieve and what not, but perhaps adding this board would be good? Or should just "officially-used" mappings be in there?

As I was writing this, I also just realized what the logic in question is actually accomplishing: A .bml file can contain a board node in addition to the game node, which means the board above can actually be put in the [gamename].bml file and make the rom work without the boards.bml file needing to be modified. Still, I'd like to know whether adding this would be good or not.

[Kawa-oneechan]

I vote to add it.

[orbea]

@Morilli Thanks!

On my end that only works for the following patch.

https://www.romhacking.net/hacks/4086/

And not the other one, can you confirm?

https://www.romhacking.net/hacks/888/

[carmiker]

In my view, the threshold to add entries to boards.bml should be whether or not the board exists as a physical implementation. This would include real game cartridges, or implementations on hardware such as the SD2SNES -- whether it already exists or is possible to add support for easily.

[Morilli]

@Morilli Thanks!

On my end that only works for the following patch.

https://www.romhacking.net/hacks/4086/

And not the other one, can you confirm?

https://www.romhacking.net/hacks/888/

Can confirm. It seem that the other one is using the HITACHI-LOROM board, however it expects a mapping like in the normal LOROM board, just with the cx4 support on top. Here's a patch that makes it work:

diff --git a/bsnes/target-bsnes/resource/system/boards.bml b/bsnes/target-bsnes/resource/system/boards.bml
index d0cbe9ec..3937a09b 100644
--- a/bsnes/target-bsnes/resource/system/boards.bml
+++ b/bsnes/target-bsnes/resource/system/boards.bml
@@ -807,7 +807,7 @@ board: HITACHI-LOROM
   processor architecture=HG51BS169
     map address=00-3f,80-bf:6c00-6fff,7c00-7fff
     memory type=ROM content=Program
-      map address=00-3f,80-bf:8000-ffff mask=0x8000
+      map address=00-7d,80-ff:8000-ffff mask=0x8000
     memory type=RAM content=Save
       map address=70-77:0000-7fff
     memory type=ROM content=Data architecture=HG51BS169

Unsure what exactly this means. As far as I can see at least, this board is not used by any game in the Super Famicom.bml database, so I cannot verify correctness of either of these.

[carmiker]

Could it be that this was a theoretical, generic HITACHI-LOROM board where the game's manifest can only be generated via heuristics? The only Cx4 games in the database have more specific board names. I would guess that making this change to HITACHI-LOROM would affect only hacks and homebrew, which is the subject here.

[Screwtapello]

Yes. Board names that mention LOROM or HIROM are heuristically generated; you can see the code in bsnes/heuristics/super-famicom.cpp. Although no board in the Super Famicom.bml database uses the HITACHI-LOROM board type, it's identical to the boards SHVC-1DC0N-01 and SHVC-2DC0N-01, the boards used by Mega Man X2 and Mega Man X3. Those games are both less than 2MB, so they don't need expanded LOROM mapping; just banks 00-3f is enough.

From my limited understanding of the SNES memory map, there's probably nothing stopping somebody making a physical board with a CX4 chip and more than 2MB of ROM in a LOROM mapping. If they did, I guess there's a decent chance it might wind up with the memory map this romhack assumes. On the other hand, I don't think we want to run around making custom heuristic boards for all the crazy memory maps romhackers can trick emulators into creating by tweaking internal header fields.

Ideally, romhacks that require a memory-map not used by any commercial game should include a memory-map .bml file so that emulators can just implement the memory-map the game wants instead of having to guess. Given that bsnes probably isn't going to have a release any time soon, that's probably the quickest way forward.

Maybe if there's some popular fictional-but-practical memory maps, they should be added. bsnes already has special handling for he FEoEZ translation, after all. I'm reluctant to propose this as a solution without some clear rule about which fictional memory maps to support, though — it's not going to be practical to hard-code support for every custom memory map people come up with.

[Morilli]

Ideally, romhacks that require a memory-map not used by any commercial game should include a memory-map .bml file so that emulators can just implement the memory-map the game wants instead of having to guess. Given that bsnes probably isn't going to have a release any time soon, that's probably the quickest way forward.

Generally a fair point, which is why I have made a PR for this specific romhack. However, it does feel a bit impractical for a hack that seems to be using sane rom mappings that bsnes also correctly guesses heuristically to not work, because a board is missing (a bsnes internal thing that users won't and probably shouldn't know about). Other emulators, like snes9x, and even older bsnes version (can name v087 and v086 by hand) run this hack out-of-the-box, so it definitely feels like a step backwards for (new) bsnes to just fail on it without any indication as to what is missing, at least from a user-perspective.

[carmiker]

I've just tested both hacks on my SD2SNES and they both work. I think this justifies adding at the very least, the HITACHI-LOROM-RAM board entry to boards.bml.