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.

Sign up for GitHub

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

Jump to bottom

PPCCache: Avoid Global System Accessor #12646

Merged
merged 1 commit into from Apr 9, 2024
Merged

PPCCache: Avoid Global System Accessor #12646

merged 1 commit into from Apr 9, 2024

Conversation

mitaclaw
Copy link
Contributor

No description provided.

@mitaclaw mitaclaw force-pushed the ppc-cache-global-system branch 2 times, most recently from 119f132 to caf6240 Compare March 18, 2024 03:46
@mitaclaw
Copy link
Contributor Author

@dolphin-emu-bot rebuild

AdmiralCurtiss
AdmiralCurtiss reviewed Apr 4, 2024
View reviewed changes
Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss left a comment

Choose a reason for hiding this comment

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

I'm unsure if it really makes sense that each Cache instance keeps its own references to the System/Memory stuff when it can just be passed in when needed, but ymmv. Otherwise this looks fine.

@mitaclaw mitaclaw force-pushed the ppc-cache-global-system branch 2 times, most recently from 7591a47 to d21d7fe Compare April 8, 2024 23:12
@mitaclaw
Copy link
Contributor Author

mitaclaw commented Apr 8, 2024

@AdmiralCurtiss I have put together a second commit following your suggestion. If it is found to be preferable, I will move forward with a squash to resume the PR. Most places were able to pass in references like you said.

@AdmiralCurtiss
Copy link
Contributor

Yeah, I do like this better, but it's definitely a personal preference thing so either way is probably fine.

@mitaclaw
Copy link
Contributor Author

mitaclaw commented Apr 9, 2024

I'll trust your judgement.

@AdmiralCurtiss AdmiralCurtiss merged commit 3583622 into dolphin-emu:master Apr 9, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdmiralCurtiss AdmiralCurtiss AdmiralCurtiss left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
2 participants
@mitaclaw @AdmiralCurtiss