Skip to content

Add description for out of bounds MBC ram access #594

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alloncm
Copy link
Contributor

@alloncm alloncm commented Mar 22, 2025

This is based on:

Since there are many types of MBC's I added that this is for most of the MBC's

This is based on:
- Source code of the SameBoy emulator
- Dissassembling Pokemon Pinball which depends on this behavior
- Mbc1 schematics - https://www.devrs.com/gb/files/mbc1.gif
@nummacway
Copy link

Use "modulo max_ram_size" instead of "and (max_ram_size - 1)"?
Why does Pinball depend on it? It doesn't even have banked RAM.

@alloncm
Copy link
Contributor Author

alloncm commented Apr 5, 2025

Perhaps "depends" is not the best way to phrase it, what I actually saw is that Pokemon Pinball sets the ram bank to a non zero value and since it does not have banked ram, all accesses are clipped (according to the formula).
Failing to emulate this exact behavior causes the game to freeze at some point.

@nummacway
Copy link

What point? I want to try with a 32 KiB cart.

@alloncm
Copy link
Contributor Author

alloncm commented Apr 5, 2025

It depends on the way you are handling this case, originally I just ignored those out of bounds writes, (and returned 0 on reads) with this way it froze after trying to reload a save from sram (boot up -> save -> restart -> load save -> freeze).
I dont remember exactly if the load save is also done with the same non zero bank (I think it was bank 4) if it is it supposed to work with a 32kb cart, if not I think it will freeze.

@alloncm alloncm requested a review from ISSOtm April 13, 2025 18:09
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.

5 participants