Skip to content
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

Add paged Cartridge RAM to GameBoy and GameBoy Color, leave MegaDuck unchanged #329

Merged
merged 3 commits into from
May 8, 2024

Conversation

GauChoob
Copy link
Contributor

The GameBoy Color already adds the paged banks of WRAM (System RAM) virtually to the end of the memory map in order to have access to this memory location (which does not exist on the GameBoy). However, the GameBoy and GameBoy Color cartridges can have up to 128 KB (16 pages of 8 KB) of Cartridge Save RAM, of which only the first page (8 KB) is accessible with the current memory map.

This would add the last 15 pages of Cartridge Save RAM to the memory map. Unfortunately, the memory addresses between the GameBoy and GameBoy Color would no longer align perfectly as the GameBoy does not have extra pages of WRAM, and we want the changes to be backwards-compatible.

I did not change the MegaDuck memory map because I couldn't find enough documentation to be sure about any potential paged Cartridge Save RAM.

@CasualPokePlayer
Copy link
Contributor

Why not just place extra cartridge RAM at 0x016000-0x033FFF also for the GB (with some nothing filler in 0x010000-0x015FFF)?

@GauChoob
Copy link
Contributor Author

Sure, we could put nothing filler at 0x010000-0x015FFF, which would help align the two memory maps. I'm happy to change it to that if it is preferred - I guess it would be easier for the core maintainers to have equivalent memory maps?

Fix RC_MEMORY_TYPE_SYSTEM_RAM -> RC_MEMORY_TYPE_SAVE_RAM for GameBoy
@GauChoob
Copy link
Contributor Author

Though upon some reflection, in order to have full backwards-compatibility, the 0xA000-0xC000 memory address area currently changes to reflect the current page. If we didn't want to break any existing functionality, we would have to duplicate and just display all 16 pages at the end, because the current assumption is that the banked current page at 0xA000-0xC000 would become fixed to bank 0, which is not the current implementation. On the other hand, this could lead to duplicated values when searching memory...

@Jamiras
Copy link
Member

Jamiras commented Apr 19, 2024

Can you provide an example where this is logically required? In most cases, if a game has multiple save slots, relying on the save RAM limits the achievements to a specific save slot.

As for the actual implementation, I think having the shared memory map is still preferable. There's a reasonable probability that something doing cartridge RAM banking might also have GBC enhancements, and we wouldn't want it to become incompatible if playing in GBC mode vs GB mode.

@CasualPokePlayer
Copy link
Contributor

CasualPokePlayer commented Apr 19, 2024

It's important to note that SRAM is in fact just RAM slapped on the cartridge that happens to be battery backed. Many games don't just use it to save progress, but also as "extra RAM" (especially for games compatible with the original GB, which can't rely on the GBC banked WRAM always being available for use).

@GauChoob
Copy link
Contributor Author

To illustrate the point, in my specific example (Magi-Nation on GBC), there is only a single save file. The player's active game profile is stored in SRAM in banks 2-3. When the player saves the game, a copy of the active profile is saved into banks 0-1, and 0-1 becomes the active profile. So the backup (save) and active game data alternate between banks 0-1 and 2-3. The data is never copied into WRAM, and WRAM just contains the more transient variables. The player's money, items and game progression vars are stored in the first SRAM bank, and the player's creatures are stored in the second SRAM bank.

With the current setup I cannot directly check the player's items, progression or creatures, so for example it's not possible to creature an achievement for opening all the treasure chests in the game. If I had access to the SRAM, I could easily check where the active game profile is and then check all these variables.

The problem was so bad that the previous retroachievement developers (Delmaru) that tried to create achievements for this game developed (Bartis) a patch that would always save the data into SRAM0 because they were not able to find any useful variables, and they ultimately gave up. I attached an (abridged) copy of the sym file for WRAM and SRAM for this game as an example. WRAM.sym.txt SRAM.sym.txt

I still managed to create achievements for this game because I've been disassembling this specific game for 4 years, so I know how the engine works and so I'm able to read the game engine's bytecode program counter to determine what events are being triggered based on the pointer's value, but this is not really readable for others, and it is not realistic to do without an elaborate disassembly of the game.

Also, to clarify my comment about the 0xA000 current mapping, from what I understand, some cores reflect the current page, whereas others always show bank 0, but I would have to check/confirm that. But it seems like there is a lot of confusion already about this current part of the memory that needs to be mapped because not all of the memory is exposed, and this confusion is resolved by exposing all the memory.

@Jamiras
Copy link
Member

Jamiras commented May 1, 2024

I applied these changes to RALibretro and loaded Magi-nation in all four Gameboy Color cores. All four cores did report 32KB of RETRO_MEMORY_SAVE_RAM. They also all implement RETRO_ENVIRONMENT_SET_MEMORY_MAPS (which is preferred), and they do not export any data at $16000, so all of the cores will have to be updated before this will actually be usable.

Gambatte:

[INFO] [MEM] desc[1]: $00c000 (1000): 
[INFO] [MEM] desc[2]: $00d000 (1000): 
[INFO] [MEM] desc[3]: $00ff80 (0080): 
[INFO] [MEM] desc[4]: $008000 (2000): 
[INFO] [MEM] desc[5]: $00fe00 (00a0): 
[INFO] [MEM] desc[6]: $000000 (4000): 
[INFO] [MEM] desc[7]: $004000 (4000): 
[INFO] [MEM] desc[8]: $00ff00 (0080): 
[INFO] [MEM] desc[9]: $00a000 (8000):
[INFO] [MEM] desc[10]: $010000 (6000):

Gearboy:

[INFO] [MEM] desc[1]: $00ffff (0001): 
[INFO] [MEM] desc[2]: $00ff80 (0080): 
[INFO] [MEM] desc[3]: $00c000 (1000): 
[INFO] [MEM] desc[4]: $00d000 (1000): 
[INFO] [MEM] desc[5]: $00a000 (2000): 
[INFO] [MEM] desc[6]: $008000 (2000): 
[INFO] [MEM] desc[7]: $000000 (4000): 
[INFO] [MEM] desc[8]: $004000 (4000): 
[INFO] [MEM] desc[9]: $00fe00 (00a0): 
[INFO] [MEM] desc[10]: $010000 (6000): 
[INFO] [MEM] desc[11]: $00ff00 (0080): 

VBA-M:

[INFO] [MEM] desc[1]: $000000 (1000): 
[INFO] [MEM] desc[2]: $001000 (1000): 
[INFO] [MEM] desc[3]: $002000 (1000): 
[INFO] [MEM] desc[4]: $003000 (1000): 
[INFO] [MEM] desc[5]: $004000 (1000): 
[INFO] [MEM] desc[6]: $005000 (1000): 
[INFO] [MEM] desc[7]: $006000 (1000): 
[INFO] [MEM] desc[8]: $007000 (1000): 
[INFO] [MEM] desc[9]: $008000 (1000): 
[INFO] [MEM] desc[10]: $009000 (1000): 
[INFO] [MEM] desc[11]: $00a000 (1000): 
[INFO] [MEM] desc[12]: $00b000 (1000): 
[INFO] [MEM] desc[13]: $00c000 (2000): 
[INFO] [MEM] desc[14]: $00f000 (1000): 
[INFO] [MEM] desc[15]: $010000 (6000): 

mGBA:

[INFO] [MEM] desc[1]: $000000 (4000): 
[INFO] [MEM] desc[2]: $004000 (4000): 
[INFO] [MEM] desc[3]: $008000 (2000): 
[INFO] [MEM] desc[4]: $00c000 (1000): 
[INFO] [MEM] desc[5]: $00d000 (1000): 
[INFO] [MEM] desc[6]: $00fe00 (00a0): 
[INFO] [MEM] desc[7]: $00ff00 (0080): 
[INFO] [MEM] desc[8]: $00ff80 (007f): 
[INFO] [MEM] desc[9]: $00ffff (0001): 
[INFO] [MEM] desc[10]: $00a000 (8000): 
[INFO] [MEM] desc[11]: $010000 (6000): 

Interestingly enough, both Gambatte and mGBA expose the full 32KB of SRAM at $A000, but we only capture 0x2000 of it because that's what the regions define. As such, at least for those cores, I don't believe there should be any concerns about them reflecting the current page, and it should be trivial to add a new memory mapped region to the core to expose the remaining memory.

It's harder to say how Gearboy and VBA-M are exposing the save ram because they're only reporting 0x2000 bytes at $A000. My guess is since it's just exposing a pointer, it's always going to just be banks 0 and 1. I wouldn't expect the core to memcpy 4K of data every time the game requested a bank switch.

RAVBA, on the other hand, almost certainly does bank switching. It doesn't map a pointer into the DLL, the DLL calls a function to read an address and it evaluates the pointer in real time. If that pointer is getting updated as the result of bank switching, it would be reading from the alternate banks. Minimally, it would have to support registering a reader for the extended memory regions, but it would probably also have to modify how it's reading the first two SRAM banks as well.

CasualPokePlayer added a commit to TASEmulators/BizHawk that referenced this pull request May 3, 2024
@Jamiras Jamiras merged commit d21ad27 into RetroAchievements:develop May 8, 2024
7 checks passed
@Jamiras Jamiras added this to the 11.3.0 milestone May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants