-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update consoleinfo.c for SA-1 Support #371
Update consoleinfo.c for SA-1 Support #371
Conversation
Code suggestion by Souzooka. This has not been tested or verified. Maybe this works right out of the gate, maybe this doesn't. Just wanted this out there.
You should go through and test to see if the mapping properly shows up at the very least. |
I can do that when I get home. I was under a time crunch when I made the PR. |
The tests aren't failing with this change, so that's good.
{ 0x020000U, 0x03FFFFU, 0xFE0000U, RC_MEMORY_TYPE_SAVE_RAM, "Cartridge RAM" } | ||
{ 0x020000U, 0x03FFFFU, 0xFE0000U, RC_MEMORY_TYPE_SAVE_RAM, "Cartridge RAM" }, | ||
{ 0x040000U, 0x05FFFFU, 0x400000U, RC_MEMORY_TYPE_SYSTEM_RAM, "BW-RAM (SA-1)" }, | ||
{ 0x060000U, 0x0607FFU, 0x003000U, RC_MEMORY_TYPE_SYSTEM_RAM, "I-RAM (SA-1)" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a reasonable mapping based on the documentation available here.
I've verified that snes9x
and mesen-s
cores do not expose this memory currently, but it looks like bsnes-mercury
does:
[INFO] [MEM] desc[15]: $003000 (0800):
[INFO] [MEM] desc[16]: $400000 (1000000):
However, it appears to be exposing 16MB of BW-RAM, which is significantly more than the 256KB documented max, or the 128KB expected real amount used.
Please build a version of RAlibretro with these changes and make sure the data being exposed by the bsnes-mercury is what you're expecting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently running into issues with a missing Git.cpp
file and a lack of XP toolset. At least I got the submodule part figured out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Git.cpp should be created by the etc\MakeGitCpp.bat batch file, which should automatically be run as a pre-build step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the XP toolset, just change it to something you have installed. It shouldn't really matter if you're just testing things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking a bit more specifically into how cores are mapping SRAM here:
BizHawk Snes9x and libretro Snes9x: Snes9x's Memory.SRAM
is exposed entirely at the end. Except not, there's some odd internal 128KiB restraint here (yes, this is apparently in both BizHawk's and libretro's Snes9x's cores). This appears to just contain all of BW-RAM here anyways (up to 128KiB, at least, the latter not being accessible, though removing that odd restraint would be trivial anyways). Snes9x's I-RAM is put into its Memory.FillRAM
, at offset 0x3000
(everything before seems to be a cache of I/O regs, interestingly enough). Should be trivial to expose in BizHawk, and probably easy for libretro to expose in a memory map?
libretro mesen-s: Appears to just expose all of BW-RAM as RETRO_MEMORY_SAVE_RAM. I-RAM is some other buffer (internal api for touching it appears to be DebugGetInternalRam()/DebugGetInternalRamSize()
?), not exposed with libretro's API (would probably be easy to expose in a memory map anyways?)
libretro bsnes-mercury: I have no idea what it's doing here. There's some mess of code I assume trying to make it match how the core is mapping memory internally? Although considering the weird 16MiBs I assume it's going horribly wrong at some points? (I assume the example was actually checking an SA-1 game at least)
BizHawk Faust: Appears to also just expose all of BW-RAM as CARTRAM
(so all plopped at the end for cheevos). I-RAM is exposed with another memory domain, so would be a trivial change on the C# side to expose it for cheevos.
BizHawk BSNESv115+: Appears to also just expose all of BW-RAM as CARTRAM
(so all plopped at the end for cheevos). I-RAM is exposed with another memory domain, so would be a trivial change on the C# side to expose it for cheevos.
BizHawk (old) BSNES: Appears to also just expose all of BW-RAM as CARTRAM
(so all plopped at the end for cheevos). I-RAM isn't exposed as another memory domain, it's probably trivial to do so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering how libretro cores all appear to expose BW-RAM as RETRO_MEMORY_SAVE_RAM
, and that behavior is also effectively done by BizHawk (what gets plopped into .SaveRAM
), it might make more sense to change the current Cartridge RAM field to expose 256KiB of SRAM (or maybe more if there's actually another higher limit on some other cartridge type's SRAM? I'm not quite sure what's actually the upper limit here on SRAM size: the 128KiB upper limit Snes9x puts in might be more placed in there intentionally due to the ROM header byte in theory potentially specifying nonsense, so 128KiB is some sanity limit. If I look at Faust too, I see the ROM header byte for SRAM limited to 512KiB, so it decides that's its sanity limit? Snes9x too seems to actually have some support for 256KiB BW-RAM anyways, it's just the latter half is not normally exposed for saveram purposes). It could be that, or just having some specification that anything past 128KiB has to go through some other magic rcheevos memory mapping quirk.
EDIT: The 128KiB limit in Snes9x seems to be an old Snes9x limit, it's been increased to 512KiB since snes9xgit/snes9x@a70aa17 and BizHawk has this increase (well, kind of, it's still enforced in SaveRAM saving / memory exposure, this should be easy to remove). Not sure if libretro is out of date or I'm looking at the wrong repo or what. Nevermind, it also internally has the 512KiB limit, it's just memory exposure / SRAM saving is still locked to 128KiB due to some kind of legacy? 512KiB I guess is more the upper limit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My hack uses these addresses here to save (dl is address and dw is the length in hex of that area used):
bw_ram_table:
dl $003019 : dw $0001
dl $003140 : dw $000D
dl $400DB4 : dw $000C
dl $400DC1 : dw $0002
dl $400F48 : dw $0002
dl $4013C7 : dw $0001
dl $40187A : dw $0001
dl $401F2F : dw $000C
dl $401F3B : dw $0001
dl $401FEE : dw $000C
dl $41C35A : dw $0001
dl $7F9FB0 : dw $0009
dl $0035F0 : dw $0002
dl $40148B : dw $0002
dl $40A660 : dw $0600
dl $40AC60 : dw $0010
dl $400DDC : dw $0001
.end
I don't think i go past any limit. $40A660 is what keeps track of every single dragon coin in the game.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: The 128KiB limit in Snes9x seems to be an old Snes9x limit, it's been increased to 512KiB since snes9xgit/snes9x@a70aa17 and BizHawk has this increase (well, kind of, it's still enforced in SaveRAM saving / memory exposure, this should be easy to remove).
Not sure if libretro is out of date or I'm looking at the wrong repo or what.Nevermind, it also internally has the 512KiB limit, it's just memory exposure / SRAM saving is still locked to 128KiB due to some kind of legacy? 512KiB I guess is more the upper limit?
Going towards the original issue which had the size increased to 512KiB seems to point to https://en.wikibooks.org/wiki/Super_NES_Programming/SNES_memory_map indicating that 512KiB would just the maximum you could possibly map with normal SNES cart hardware, so Cartridge RAM probably should just be bumped upwards to 512KiB (then afterwards you can have SA1 IRAM)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done an initial implementation exposing SA1 memory here: TASEmulators/BizHawk@e71cdf8
Rather than do exactly as this PR does, it simply increases the Cartridge RAM exposed over to 512KiB (the theoretical maximum?) which also would be BW-RAM (which is RC_MEMORY_TYPE_SAVE_RAM and RETRO_MEMORY_SAVE_RAM anyways). Afterwards, SA1 IRAM would be exposed. It should work on all 4 BizHawk SNES cores (Snes9x, Faust, BSNES, and BSNESv115+). Dev builds can be grabbed in the readme to verify it has what you need.
I think #375 is a better implementation for this. Since BW-RAM is already exposed as SRAM, expanding SRAM to support the maximum amount available (which is actually larger than the 256KB allowed by BW-RAM) maintains backwards compatibility with the any achievements already looking for BW-RAM there. Then the I-RAM can be appended after that. Note that no cores currently expose more than 128KB of SRAM/BW-RAM, and none of them expose any I-RAM. |
To be clear, Snes9x libretro currently does have an internal limit of 512KiB, but there's a 128KiB limit placed in for exposing memory due to that being the old limit and libretro core not being updated (also likewise, upstream Snes9x would just produce 512KiB SRAM for whatever romhacks have that). It would be easy enough to fix that on their side. |
Code suggestion by Souzooka. This has not been tested or verified. Maybe this works right out of the gate, maybe this doesn't. Just wanted this out there.