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

Restore wiimote netplay #3949

Merged
merged 3 commits into from Jul 8, 2016

Conversation

mimimi085181
Copy link
Contributor

@mimimi085181 mimimi085181 commented Jun 26, 2016

In its broken state.

Reverts prs #3802, #3691 and #3660

The broken state will be useful when trying to fix wiimote netplay.


This change is Reviewable

@mimimi085181 mimimi085181 force-pushed the restore-wiimote-netplay branch 4 times, most recently from ede25fc to df27213 Compare June 26, 2016 09:20
@Jofzar
Copy link

Jofzar commented Jun 26, 2016

The broken state also breaks testing for netplay, there are 3 netplay changes waiting to be merged, could it be possible to merge and test those before this is broken again?

@mimimi085181
Copy link
Contributor Author

Are you sure it breaks netplay? You should just have to disable all wimotes manually.

@Jofzar
Copy link

Jofzar commented Jun 26, 2016

I would have to retest but at one point netplay was straight breaking due to one of these, also #3691 changes the text of the netplay window, which should probably be noted for consistency when these are reverted.

@mimimi085181
Copy link
Contributor Author

I need this pr merged, before i can really contine to work on it. I have one follow up pr ready, which should fix starting a 2nd netplay session. The code static u8 previousSize[4] = {4, 4, 4, 4}; only sets the size for the 7 dummy inputs to 4 for the 1st session, but on the 2nd start, it has the size where the previous session ended.

@Jofzar: Please check how i changed the texts. I reverted it, and added a line "Manually set the extensions for each Wiimote".

And also, netplay with gamecube controllers should work fine with this merged, if you manually set all wiimotes to none on all clients. @JMC47: Can you confirm this?

@JMC47
Copy link
Contributor

JMC47 commented Jul 7, 2016

@lioncash can you review this? I don't know who else to bother, but this is becoming important.

@Jofzar
Copy link

Jofzar commented Jul 8, 2016

Seems to be fine, would prefer to have a second opinion, my game library is smaller then jmc's so its probably best for his opinion.

As for the wording, I think there could be a better non technical way of saying that.

Maybe something like this?

"If you are using Wii Motes, they will have to be manually set"

@mimimi085181
Copy link
Contributor Author

mimimi085181 commented Jul 8, 2016

@Jofzar: One big problem with the current code is that there's no difference between the setup remote and local wiimotes. If player 2 uses a nunchuck for example, the 2nd controller needs to have the nunchuck extension set on both clients. The problem is that the input is taken from the 1st slot from player 2, so that has to be set to nunchuck as well.

So maybe something like: "All players need to use the same Wiimote extension?"?

@Jofzar
Copy link

Jofzar commented Jul 8, 2016

Add on the end "to avoid desyncing" and then it will be perfect

@mathieui
Copy link
Member

mathieui commented Jul 8, 2016

Reviewed 2 of 5 files at r1, 3 of 3 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@mathieui
Copy link
Member

mathieui commented Jul 8, 2016

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@mimimi085181
Copy link
Contributor Author

For the record, before the 3 prs, Dolphin properly disabled all wiimotes when there's no mapped wiimote for netplay. I got confused because somewhen it was necessary to disable wiimote manually, which is not the case after reverting all 3 prs.

@delroth delroth merged commit bfe8b11 into dolphin-emu:master Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants