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

NetPlayClient: Treat power button event as a netplay stop. #11059

Merged

Conversation

AdmiralCurtiss
Copy link
Contributor

Sending the power button event over netplay is completely unsynced, which means more likely than not two instances of Dolphin will start the graceful shutdown sequence at different points in time, desyncing in the process. This wouldn't be so bad -- we're shutting down anyway -- except that inputs may still be queried by the game during the shutdown. This can lead to a situation where one instance has fully shut down, but another instance is still waiting for an input.

By stopping Netplay as soon as the power button event is received, we can work around this; that means it won't wait for input that may never come.

@JMC47 Can you test if this fixes known cases of hangs on shutdown, especially when Wiimote disconnects are involved?

@AdmiralCurtiss
Copy link
Contributor Author

Since JMC asked: Yes, this still lets games gracefully shutdown, which is needed for some games (like VC games) to write their saves. I've tested this in combination with #11047 on a VC SNES copy of FF6 -- though with two local instances of Dolphin, so without any real network latency.

@@ -252,6 +252,8 @@ class NetPlayClient : public TraversalClientClient
void DisplayPlayersPing();
u32 GetPlayersMaxPing() const;

bool WaitForWiimoteBuffer(int _number);
Copy link
Contributor

Choose a reason for hiding this comment

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

const?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

m_wii_pad_event.Wait() is not const.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gah, somehow I glossed over that. Sorry!

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

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

One minor comment aside, the code LGTM. Untested.

@JMC47
Copy link
Contributor

JMC47 commented Sep 17, 2022

This seems to work, at least Wii Netplay hasn't crashed for me on LAN testing. Which, it crashes pretty commonly on master.

@AdmiralCurtiss
Copy link
Contributor Author

Well, in that case...

@AdmiralCurtiss AdmiralCurtiss merged commit 69ad2cc into dolphin-emu:master Sep 17, 2022
11 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the netplay-graceful-shutdown branch September 17, 2022 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants