PretendoNetwork / nex-protocols-common-go

Reusable implementations of NEX methods found in many servers
GNU Affero General Public License v3.0
19 stars 8 forks source link

Lots of changes on common protocols #12

Closed DaniElectra closed 1 year ago

DaniElectra commented 1 year ago

Depends on https://github.com/PretendoNetwork/nex-protocols-go/pull/24

DaniElectra commented 1 year ago

While I was testing this on MK7, everything seemed to work fine, except that one of the clients would randomly disconnect in-game from the session (also causing the other client to leave) and from NEX for seemingly no reason (there aren't any requests sent to NEX before this happening, just the Leaving message).

Luckily, the NEX matchmaking sessions themselves seem to work fine. I can't test nat traversal however, since both machines are connected to the same network

jonbarrow commented 1 year ago

(there aren't any requests sent to NEX before this happening, just the Leaving message).

About how long does this take to happen? It sounds like a ping timeout issue

DaniElectra commented 1 year ago

(there aren't any requests sent to NEX before this happening, just the Leaving message).

About how long does this take to happen? It sounds like a ping timeout issue

It would happen some seconds after the game had started (the time varied on each test). Actually, that may be it, since the server is set with ping after 999 seconds and I hadn't changed that parameter

I will try again tomorrow

DaniElectra commented 1 year ago

Interestingly, the issue above appears on Race mode, but on Battle mode almost everything works just fine, and I can play as many rounds as I want. However, when one of the participants leaves, the EndParticipation isn't sent for some reason (but the client doesn't disconnect from the server), and the other player will show a connection error.

I wonder if this is a me issue, because of having the server and consoles on the same network? idk

jonbarrow commented 1 year ago

It's possible that's causing issues, yeah. I've had a few issues when hosting the servers on the same network as the clients in the past. That is weird though

jonbarrow commented 1 year ago

I see this was marked as open, no longer a draft. Are we still waiting for more changes or is this ready for review now?

DaniElectra commented 1 year ago

Yes, it's ready for review

shutterbug2000 commented 1 year ago

Looks good to me

jonbarrow commented 1 year ago

Relating to a comment we talked about the other day, nex-protocols-go version 1.0.30 now has a GatheringFlags structure in the match making protocol to hold all the possible flags a gathering can have. I added 0x20 and 0x200 as well despite them being labeled as "unknown"

jonbarrow commented 1 year ago

Carrying over from Discord, @shutterbug2000 has confirmed that the matchmaking changes here work fine in MK7 during his testing. So this PR should be good to merge once the Go modules are updated (I merged your other PR https://github.com/PretendoNetwork/nex-protocols-go/pull/25 and made a new release) and once the other issues are looked at

DaniElectra commented 1 year ago

I changed invalid values to be zero, as it can't be a valid gathering ID nor a valid connection ID. To get a gathering ID, I changed the counter to the function GetSessionIndex(), which will check for empty sessions starting from ID 1. I tested this implementation in isolation (with maps with a million values) with minimal performance impact. Considering a PID can only create one session, it's impossible that we reach the max uint32 limit, so if we get an error, it should be an issue of not erasing empty sessions.

I also implemented time validation on Kerberos tickets. If the ticket is expired, we kick the client for timeout, as on Mario Kart 7 the client was still sending data packets when using a graceful kick (using graceful kicks aren't really an issue though, as we delete the client and the server can't decrypt those data packets anymore [PRUDPv0] Error decoding packet data: [PRUDPv0] Error parsing RMC request: RMC Request size does not match length of buffer, but the client would get spammy with those packets).