Add load options for PRG and CHR banks#28
Merged
CBongo merged 1 commit intokylewlacy:mainfrom Jun 14, 2025
Merged
Conversation
The primary feature of this commit is to add load options for PRG and CHR banks. Selecting the Options button in the load dialog will show (if appropriate for the mapper) choices for the base address of each PRG bank, and a choice of how to handle CHR banks. There is a fair bit of refactoring going here to support this, as well as preparing for more generalized mapper support in the future. In particular, there has been some shuffling of code between the NesRom, GhidraNesLoader, and NesMapper classes to better align them with their logical responsibilities. The separate classes for UxROM and MMC4 have been rendered moot as a result, so they have been removed. I've also started adding more informational messages that will appear in the log window when loading a ROM.
kylewlacy
approved these changes
Jun 13, 2025
Owner
kylewlacy
left a comment
There was a problem hiding this comment.
LGTM!
Awesome to see that several custom classes just fully deleted while still adding more mappers! To me that's the clearest sign that this is the right direction to go. Love to see the extra debug output too
(Feel free to merge whenever, I only held off because I saw some TODO comments so I wasn't sure if you were planning on pushing more changes before merging or if those were for future follow-ups)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The primary feature of this commit is to add load options for PRG and CHR banks. Selecting the Options button in the load dialog will show (if appropriate for the mapper) choices for the base address of each PRG bank, and a choice of how to handle CHR banks.
There is a fair bit of refactoring going here to support this, as well as preparing for more generalized mapper support in the future. In particular, there has been some shuffling of code between the NesRom, GhidraNesLoader, and NesMapper classes to better align them with their logical responsibilities. The separate classes for UxROM and MMC4 have been rendered moot as a result, so they have been removed.
I've also started adding more informational messages that will appear in the log window when loading a ROM.
Oh, and just for fun, I also updated the README.md with instructions on how to adjust fallthrough when mappers swap banks out from under you. I didn't have a chance to add screenshots yet, though.
@kylewlacy As this is a bit more significant of a change, I was hoping you'd chime in if you have thoughts on whether this approach seems reasonable.