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

SI/DeviceGBA: Improve link stability #9582

Merged
merged 1 commit into from Mar 26, 2021
Merged

Conversation

endrift
Copy link
Contributor

@endrift endrift commented Mar 13, 2021

Building on #9550 I found some more issues that could cause the link to get out of sync.

The original flush technique didn't work properly; it didn't work before the first RESET was sent, which generally meant the first RESET would be junk too. This replaces it with flushing before sending any message. This approach isn't perfect but clears out the junk much more reliably.

This also adds a minor wait for the "non-booted" state (polling for status instead of actual data transfer) to improve the stability further.

@JMC47
Copy link
Contributor

JMC47 commented Mar 13, 2021

Fixes connecting to Final Fantasy: Crystal Chronicles.

@@ -258,12 +261,12 @@ int GBASockServer::Receive(u8* si_buffer, u8 bytes)
if (!m_client)
return 0;

sf::SocketSelector selector;
selector.add(*m_client);
if (m_booted)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if its worth documenting why m_booted affects how long we can/should wait for a socket, and if those numbers have any significance (vs. being guesses that just work in practise)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently the number didn't even work on Windows, so I replaced it with redefining what m_booted represents.

@JMC47
Copy link
Contributor

JMC47 commented Mar 22, 2021

This does improve linking with Final Fantasy: Crystal Chronicles. I do not know of any regressions.

Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

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

Code looks good, but untested. Plus, I don't know enough of the protocol to say whether there's other things looming that might pop out later.

@endrift
Copy link
Contributor Author

endrift commented Mar 22, 2021

I know I've been testing it and I think @JMC47 has been too, so "untested" might not be the most accurate description here.

@BhaaLseN
Copy link
Member

BhaaLseN commented Mar 22, 2021

Untested by me ;)
I only looked at the code, I didn't try/run it myself.

@JosJuice JosJuice merged commit 9e21f6f into dolphin-emu:master Mar 26, 2021
10 checks passed
@endrift endrift deleted the gba-sync branch March 26, 2021 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants