dolphin icon indicating copy to clipboard operation
dolphin copied to clipboard

Netplay: Redesign Wiimote data exchange.

Open AdmiralCurtiss opened this issue 3 years ago • 7 comments

Intended result is to get Wiimotes more stable in Netplay.

To accomplish this I've decided to sync a compact representation of the Wiimote state, rather than input reports, and let the input reports be generated client-side as usual. To allow this I had to refactor WiimoteEmu a bit, but I'm pretty happy with most of it. The actual serialization is a bit messy, maybe that can be nicer...

The Netplay functions are based on how the GameCube syncing works, but it's still pretty barebones. I suspect it wouldn't be too hard to add host authority and golf mode to this now. I have not yet done anything about each user having to use local Wiimote N instead of local Wiimote 1, I think that needs to be handled differently here due to how Wiimotes have internal state... You also still need to manually make sure everyone has the correct number of Wiimotes selected.

What is already better is that we no longer completely break when Wiimotes disconnect, and in fact can trigger a reconnect by pressing a button on the disconnected Wiimote! The code also technically supports switching Extensions and Motion Plus mid-session, but in practice that's hard to get to because we disable the Controller mapping window during Netplay... Probably should improve that, too.

This is still a bit WIP-y but feel free to experiment with this.

AdmiralCurtiss avatar Sep 18 '22 02:09 AdmiralCurtiss

What's the big thing stopping Wii Remote configuration working like GameCube controllers in this?

JMC47 avatar Sep 19 '22 20:09 JMC47

The way the GameCube Netplay index re-assignment is implemented is a bit unorthodox; without going into details, it relies on the fact that polling a GameCube controller is fundamentally stateless. The poll at timepoint X+1 is independent of the poll at timepoint X. This is not true for Wiimotes, as they have internal motion simulation state that relies on the actual time point.

Practically speaking, we probably need to split the simulation update from the poll in order to make this work. This should also allow an optimization (that we have for GC) where we pack multiple controller polls into a single packet if multiple players are on the same Dolphin instance.

AdmiralCurtiss avatar Sep 19 '22 21:09 AdmiralCurtiss

This should also allow an optimization (that we have for GC) where we pack multiple controller polls into a single packet if multiple players are on the same Dolphin instance.

~~I already made it do that.~~ Nevermind, I missed the part in parentheses.

Techjar avatar Sep 19 '22 21:09 Techjar

Practically speaking, we probably need to split the simulation update from the poll in order to make this work. This should also allow an optimization (that we have for GC) where we pack multiple controller polls into a single packet if multiple players are on the same Dolphin instance.

Yeah, though might that cause issues for the game itself since it could get data updates at an inconsistent rate?

but in practice that's hard to get to because we disable the Controller mapping window during Netplay

That was done specifically because changing stuff would break NetPlay. If we can fix that then it will no longer be necessary. I think it still breaks if you change what's in the GC controller ports, but I'm not 100% sure.

Techjar avatar Sep 19 '22 21:09 Techjar

Yeah, though might that cause issues for the game itself since it could get data updates at an inconsistent rate?

Well we would still call it at the same time from the game's perspective, I just mean we split it out from the current Update() into two functions that we call one after the other with the netplay sync in-between.

Basically right now I do in this PR:

for (wiimote in wiimotes) {
  target_state = wiimote->UpdateSimulationState();
  Netplay::Sync(target_state);
  wiimote->UpdateEmulatedState(target_state);
}

But what I want to do is:

for (wiimote in wiimotes) {
  target_states.push(wiimote->UpdateSimulationState());
}
Netplay::Sync(target_states);
for (wiimote in wiimotes) {
  wiimote->UpdateEmulatedState(target_states[i]);
}

AdmiralCurtiss avatar Sep 19 '22 21:09 AdmiralCurtiss

Oh okay so it'd still be synchronous, just split into two distinct steps instead of interleaving the simulation update and data push.

Techjar avatar Sep 19 '22 21:09 Techjar

Yeah, exactly.

AdmiralCurtiss avatar Sep 19 '22 21:09 AdmiralCurtiss

Alright, I think this is ready now. Please test and review.

AdmiralCurtiss avatar Sep 25 '22 02:09 AdmiralCurtiss

JMC asked me to look through this PR. Can't say I read every line of code (in particular not the part about controller numbers), but the approach seems sound overall.

JosJuice avatar Oct 02 '22 21:10 JosJuice