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

MemoryPatches: Fix instruction patches #11201

Merged
merged 3 commits into from Oct 29, 2022

Conversation

JoshuaMKW
Copy link
Contributor

For a long while, multiple patched instructions at a time could only be restored in reverse order of patching. This PR fixes the oversight, allowing users to restore patched instructions in any order they like.

@dreamsyntax
Copy link
Contributor

Can you please squash these commits to one?

Looks great btw, I've personally been affected by this issue

@JoshuaMKW
Copy link
Contributor Author

JoshuaMKW commented Oct 27, 2022

Sure. It's pretty funny too because I intentionaly split up my changes based on context thinking that was more favored :P

@AdmiralCurtiss
Copy link
Contributor

Separate commits are fine, actually, but we don't like merge commits or 'fixup'-style commits. If you can have separate commits that actually make sensible changes per-commit feel free to leave them.

@dreamsyntax
Copy link
Contributor

Separate commits are fine, actually, but we don't like merge commits or 'fixup'-style commits. If you can have separate commits that actually make sensible changes per-commit feel free to leave them.

I was referring to these:
Merge branch 'master' of https://github.com/JoshuaMKW/dolphin

But I was under the impression Dolphin PRs were 1 commit only.
Good to know that is not the case 👍

@JoshuaMKW
Copy link
Contributor Author

Alright, I figured out squashing and it's been cleaned up!

On another note, what would have me become auto-trusted? It's of course not that important but seeing the "All checks have failed" messes with my head lol.

@Pokechu22
Copy link
Contributor

On another note, what would have me become auto-trusted?

People who have been around for a while can get added once they've submitted a few PRs. Otherwise someone just needs to manually trigger the CI build. I wouldn't worry about it too much for now.

Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

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

Tested by enabling "boot to pause", then launching super mario sunshine and nopping out the first 2 function calls, and then restoring the first one. Before restoring would do nothing, now it works. Code looks good to me too.

@Pokechu22 Pokechu22 merged commit 6dcf8a6 into dolphin-emu:master Oct 29, 2022
11 checks passed
@JoshuaMKW JoshuaMKW deleted the fix-instruction-patches branch October 29, 2022 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants