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

clear mapped RAM on subsong change #125

Merged
merged 2 commits into from
Apr 16, 2024
Merged

clear mapped RAM on subsong change #125

merged 2 commits into from
Apr 16, 2024

Conversation

mmitch
Copy link
Owner

@mmitch mmitch commented Apr 12, 2024

This should fix #124.

I know @mrehkopf has already fixed the bug locally, but there was no pull request yet and it's only 6 lines of code ;-)
He did most of the debugging, that's not my achievement.

Please have a good look at my commit message and HISTORY entry, I'm not sure if they are comprehensible and if I have used the correct technical terms. I'm open for better wording :)

@mmitch mmitch added the bug label Apr 12, 2024
@mmitch mmitch added this to the 0.0.97 milestone Apr 12, 2024
@mmitch mmitch requested review from mrehkopf and ranma April 12, 2024 22:12
Copy link
Collaborator

@mrehkopf mrehkopf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, basically a cleaner version of my local edit.

mapper.c Outdated
@@ -246,3 +254,7 @@ struct mapper *mapper_gb(struct gbcpu *gbcpu, const uint8_t *rom, size_t size, u
void mapper_free(struct mapper *m) {
free(m);
}

void mapper_init(struct mapper *m) {
memset(m->ram, 0, MAPPER_MAX_EXTRAM_SIZE);
Copy link
Collaborator

@mrehkopf mrehkopf Apr 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe assert(sizeof(m->ram) == MAPPER_MAX_EXTRAM_SIZE) beforehand, or use sizeof(m->ram) as the count?

@mrehkopf
Copy link
Collaborator

"mapped external cartridge mapped" in the message for 2f6a309 should probably be "mapped external cartridge RAM" ;-)

based on git history
On a subsong change the player has to reset the CPU, registers, RAM
etc.

Until now the mapped external cartridge RAM was not reset, which could
make state from one subsong spill over into the next.  Depending on
the player routine and subsong sequence this could lead to playback
errors.

This fixes issue #124.
@mmitch
Copy link
Owner Author

mmitch commented Apr 15, 2024

I have added both changes (went with the sizeof(m->ram) variant) and found another typo in the commit message (onto -> into).
Because I had to edit the commit message anyway, I simply did a rebase :)

@mmitch mmitch merged commit 6b4e953 into master Apr 16, 2024
20 checks passed
@mmitch mmitch deleted the reset-mapped-ram branch April 16, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong timing in certain Super Mario Land 2 subtunes
2 participants