fix: disconnect issue with wifi turned off

[SamuelBellomo]

Description (*)

Disconnecting wifi caused an issue where clients wouldn't be able to reconnect.
This is due to a bug in NGO's Shutdown, which happens when it's called when NGO isn't Listening.

This PR adds a check around each Shutdown to make sure we're listing before calling it. --> workaround
This PR also cleans up our disconnect handling, to make sure we're using the same connection flow each time. --> cherrypick
This also fixes an issue where we didn't sent the appropriate allocation Id to lobby. --> cherrypick

Related Pull Requests

Issue Number(s) (*)

Fixes issue(s): MTT-2833

Manual testing scenarios

client on local machine, host on different machine. Connect client and host together using relay. Disconnect client's wifi. Wait 10-15 seconds (screen should go black with some curl error messages saying unable to reach services). Reconnect wifi, client should be brought back to main menu. Host should show only 1 connected player. Try to connect client to host again. There will be a silent fail and client won't be able to connect.

In boss room, we try to reconnect you once if connection drops. In our case, since lobby requests fail, we never reach the "StartClient" part of our reconnection process. We then go to the "reconnection failed" part, which calls Shutdown on our network manager.

[SamuelBellomo]

Setting both cherrypick and workaround label, I'll need to extract some stuff to be cherrypicked.

[fernando-cortez]

So since this resolves issues with reconnections, shall we increase the reconnection attempts? It's at 1 currently.

[SamuelBellomo]

So since this resolves issues with reconnections, shall we increase the reconnection attempts? It's at 1 currently.

@fernando-cortez since disconnect timeout is at 10 seconds, I feel like only 1 reconnect attempt is fine? We'd spend 20 seconds trying to connect in total. More than that feels like too much

[fernando-cortez]

Removing my approval as I've encountered a bug. Repeat reconnects are not possible. I've highlighted where it makes sense to re-initiate the reconnecting coroutine (which also doesn't self consume on successful reconnect).

[fernando-cortez]

@fernando-cortez since disconnect timeout is at 10 seconds, I feel like only 1 reconnect attempt is fine? We'd spend 20 seconds trying to connect in total. More than that feels like too much

@SamuelBellomo Why not bump that up to 2/3 and reduce the time between reconnects to say 5s? Currently we do it as soon as you disconnect and just wait ~10s. In that time someone might actually fix their connection.

[LPLafontaineB]

@fernando-cortez since disconnect timeout is at 10 seconds, I feel like only 1 reconnect attempt is fine? We'd spend 20 seconds trying to connect in total. More than that feels like too much

@SamuelBellomo Why not bump that up to 2/3 and reduce the time between reconnects to say 5s? Currently we do it as soon as you disconnect and just wait ~10s. In that time someone might actually fix their connection.

I agree we could bump the number of attempts, but I'm not sure we can simply reduce the time between the attempts, because we need to make sure that an attempt times out before trying another. However, we could make it so we only wait when actually trying to attempt to connect through NGO, so when we succesfully joined a lobby. In that case, we could have 2/3 attempts with the delay between attempt being small if not able to join lobby and ~10s when successful and trying to connect through NGO

[SamuelBellomo]

@fernando-cortez since disconnect timeout is at 10 seconds, I feel like only 1 reconnect attempt is fine? We'd spend 20 seconds trying to connect in total. More than that feels like too much

@SamuelBellomo Why not bump that up to 2/3 and reduce the time between reconnects to say 5s? Currently we do it as soon as you disconnect and just wait ~10s. In that time someone might actually fix their connection.

I agree we could bump the number of attempts, but I'm not sure we can simply reduce the time between the attempts, because we need to make sure that an attempt times out before trying another. However, we could make it so we only wait when actually trying to attempt to connect through NGO, so when we succesfully joined a lobby. In that case, we could have 2/3 attempts with the delay between attempt being small if not able to join lobby and ~10s when successful and trying to connect through NGO

@LPLafontaineB timeout is set in transport settings, (thinking of it, we should actually be using this config instead of a const k_TimeoutDuration

[LPLafontaineB]

@SamuelBellomo Wouldn't that amount to the same thing, since we are setting that value to k_TimeoutDuration in the ConnectClient method? (m_Portal.NetManager.NetworkConfig.ClientConnectionBufferTimeout = k_TimeoutDuration;)

[SamuelBellomo]

@SamuelBellomo Wouldn't that amount to the same thing, since we are setting that value to k_TimeoutDuration in the ConnectClient method? (m_Portal.NetManager.NetworkConfig.ClientConnectionBufferTimeout = k_TimeoutDuration;)

good point, there seems to be two configs, one in networkManager and one in unityTransport. Raised this with the teams, let's see. There might be some overlap...

[SamuelBellomo]

@SamuelBellomo Wouldn't that amount to the same thing, since we are setting that value to k_TimeoutDuration in the ConnectClient method? (m_Portal.NetManager.NetworkConfig.ClientConnectionBufferTimeout = k_TimeoutDuration;)

good point, there seems to be two configs, one in networkManager and one in unityTransport. Raised this with the teams, let's see. There might be some overlap...

One is for approval timeout, the other is for low level transport timeouts. Updated both and set 2 reconnect attempts at 5 seconds timeouts.