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

BBA/HLE: Code refactoring #12559

Merged
merged 7 commits into from Apr 16, 2024
Merged

Conversation

sepalani
Copy link
Contributor

@sepalani sepalani commented Feb 3, 2024

This PR is based on #12533 and #12555. I needed to refactor it to deduplicate parts of the code and to allow some parts to be easier to modify in the future.

It shouldn't have any functional change.

class NetworkRef
{
public:
StackRefs& data();
Copy link
Member

@lioncash lioncash Feb 4, 2024

Choose a reason for hiding this comment

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

since these are super basic, it's probably fine to implement these inline.

Also it might be nice to also provide const and non-const begin()/end() member functions so you can iterate without needing to get the iterator off data(), which would be a little more idiomatic.

So rather than:

for (auto& net_ref : m_network_ref.data())

you could just do:

for (auto& net_ref : m_network_ref)

@AdmiralCurtiss
Copy link
Contributor

This can now be rebased, the PRs it's depending on are merged.

@sepalani sepalani marked this pull request as ready for review April 6, 2024 17:59
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.

Looks mostly right to me, the one part that isn't obvious to me if it changes behavior or not is 6e2a081, because it swaps the order of the two operations.

@sepalani
Copy link
Contributor Author

Looks mostly right to me, the one part that isn't obvious to me if it changes behavior or not is 6e2a081, because it swaps the order of the two operations.

The behaviour should be unchanged as the sleeping/enqueued data will be handled later (on the next iteration) after processing the socket data.

@AdmiralCurtiss AdmiralCurtiss merged commit 83b5124 into dolphin-emu:master Apr 16, 2024
11 checks passed
@AdmiralCurtiss
Copy link
Contributor

I'll just believe you on that, it seems fine either way.

@sepalani sepalani deleted the bba-refactor branch April 17, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants