project-slippi / Ishiiruka

GNU General Public License v2.0
330 stars 142 forks source link

Refactor SlippiNetplayClient to enable attempting connections to multiple endpoints per player #389

Open jmlee337 opened 1 year ago

jmlee337 commented 1 year ago

(at this time, the only possible endpoints are external and local)

Because this rewrites much of the connection logic, I tested this in the following cases

In the immediate term this will fix connections for the case where

Before #387 is merged this will also fix connections for the case where

In the future there is the possibility of using this functionality to

JLaferri commented 1 year ago

Oh boy. Yeah this changes a lot of critical things. Some of the stuff that got changed were things that I added because there were issues with people disconnecting from each other 30 seconds into a match, for example.

Would be good to do some decent testing and also have a decent revert plan if something breaks.

jmlee337 commented 1 year ago

I'll have to think more about a testing plan. The main problem I'm thinking of is that this PR alone doesn't offer any improved functionality to entice testers (in 99.9% of cases).

In the meantime can you tell me about the 30 second disconnect? What parts of the original code address that? I can at least try to make sure there's no regressions.