fix: leaving lobby when connection denied (#551)

* client now leaves lobby if their connection gets denied

* host now removes client from lobby when denying connection

* Moved beginning of lobby tracking to after successful connection through ngo

(cherry picked from commit 7ec53d1)

Author: LPLafontaineB

Assets/BossRoom/Scripts/Client/UI/Lobby/LobbyUIMediator.cs

@@ -130,7 +130,7 @@ void OnFailedLobbyCreateOrJoin()
         void OnCreatedLobby(Lobby lobby)
         {
             m_LocalUser.IsHost = true;
-            m_LobbyServiceFacade.BeginTracking(lobby);
+            m_LobbyServiceFacade.SetRemoteLobby(lobby);
 
             m_GameNetPortal.PlayerName = m_LocalUser.DisplayName;
 
@@ -150,7 +150,7 @@ void OnCreatedLobby(Lobby lobby)
 
         void OnJoinedLobby(Lobby remoteLobby)
         {
-            m_LobbyServiceFacade.BeginTracking(remoteLobby);
+            m_LobbyServiceFacade.SetRemoteLobby(remoteLobby);
             m_GameNetPortal.PlayerName = m_LocalUser.DisplayName;
 
             switch (m_LocalLobby.OnlineMode)
@@ -243,6 +243,7 @@ void UnblockUIAfterLoadingIsComplete()
         /// </summary>
         void OnNetworkTimeout()
         {
+            m_LobbyServiceFacade.EndTracking();
             UnblockUIAfterLoadingIsComplete();
         }
     }

Assets/BossRoom/Scripts/Shared/Net/ConnectionManagement/ClientGameNetPortal.cs

@@ -112,6 +112,10 @@ public void OnConnectFinished(ConnectStatus status)
             else
             {
                 m_ConnectStatusPub.Publish(status);
+                if (m_LobbyServiceFacade.CurrentUnityLobby != null)
+                {
+                    m_LobbyServiceFacade.BeginTracking();
+                }
             }
         }
 
@@ -145,9 +149,8 @@ private void OnDisconnectOrTimeout(ulong clientID)
                     }
                     m_ApplicationController.LeaveSession();
                 }
-                else if (DisconnectReason.Reason == ConnectStatus.GenericDisconnect || DisconnectReason.Reason == ConnectStatus.Undefined)
+                else
                 {
-                    // only call this if generic disconnect. Else if there's a reason, there's already code handling that popup
                     NetworkTimedOut?.Invoke();
                 }
                 m_ConnectStatusPub.Publish(DisconnectReason.Reason);

Assets/BossRoom/Scripts/Shared/Net/ConnectionManagement/ServerGameNetPortal.cs

@@ -178,6 +178,9 @@ void ApprovalCheck(byte[] connectionData, ulong clientId, NetworkManager.Connect
                 return;
             }
 
+            string payload = System.Text.Encoding.UTF8.GetString(connectionData);
+            var connectionPayload = JsonUtility.FromJson<ConnectionPayload>(payload); // https://docs.unity3d.com/2020.2/Documentation/Manual/JSONSerialization.html
+
             ConnectStatus gameReturnStatus;
 
             // Test for over-capacity connection. This needs to be done asap, to make sure we refuse connections asap and don't spend useless time server side
@@ -186,53 +189,42 @@ void ApprovalCheck(byte[] connectionData, ulong clientId, NetworkManager.Connect
             if (m_Portal.NetManager.ConnectedClientsIds.Count >= CharSelectData.k_MaxLobbyPlayers)
             {
                 gameReturnStatus = ConnectStatus.ServerFull;
-                //TODO-FIXME:Netcode Issue #796. We should be able to send a reason and disconnect without a coroutine delay.
-                //TODO:Netcode: In the future we expect Netcode to allow us to return more information as part of
-                //the approval callback, so that we can provide more context on a reject. In the meantime we must provide the extra information ourselves,
-                //and then manually close down the connection.
-                SendServerToClientConnectResult(clientId, gameReturnStatus);
-                SendServerToClientSetDisconnectReason(clientId, gameReturnStatus);
-                StartCoroutine(WaitToDisconnect(clientId));
-                return;
             }
-
-            string payload = System.Text.Encoding.UTF8.GetString(connectionData);
-            var connectionPayload = JsonUtility.FromJson<ConnectionPayload>(payload); // https://docs.unity3d.com/2020.2/Documentation/Manual/JSONSerialization.html
-
-            int clientScene = connectionPayload.clientScene;
-
-            Debug.Log("Host ApprovalCheck: connecting client with player ID: " + connectionPayload.playerId);
-
-            gameReturnStatus = SessionManager<SessionPlayerData>.Instance.SetupConnectingPlayerSessionData(clientId, connectionPayload.playerId,
-                new SessionPlayerData(clientId, connectionPayload.playerName, m_Portal.AvatarRegistry.GetRandomAvatar().Guid.ToNetworkGuid(), 0, true))
-                ? ConnectStatus.Success
-                : ConnectStatus.LoggedInAgain;
-
-            //Test for Duplicate Login.
-            if (gameReturnStatus == ConnectStatus.LoggedInAgain)
+            else
             {
-                SessionPlayerData? sessionPlayerData =
-                    SessionManager<SessionPlayerData>.Instance.GetPlayerData(connectionPayload.playerId);
-
-                ulong oldClientId = sessionPlayerData?.ClientID ?? 0;
-                // kicking old client to leave only current
-                SendServerToClientSetDisconnectReason(oldClientId, ConnectStatus.LoggedInAgain);
+                Debug.Log("Host ApprovalCheck: connecting client with player ID: " + connectionPayload.playerId);
 
-                StartCoroutine(WaitToDisconnect(clientId));
-                return;
+                gameReturnStatus = SessionManager<SessionPlayerData>.Instance.SetupConnectingPlayerSessionData(clientId, connectionPayload.playerId,
+                    new SessionPlayerData(clientId, connectionPayload.playerName, m_Portal.AvatarRegistry.GetRandomAvatar().Guid.ToNetworkGuid(), 0, true))
+                    ? ConnectStatus.Success
+                    : ConnectStatus.LoggedInAgain;
             }
 
             if (gameReturnStatus == ConnectStatus.Success)
             {
+                int clientScene = connectionPayload.clientScene;
                 SendServerToClientConnectResult(clientId, gameReturnStatus);
 
                 //Populate our dictionaries with the playerData
                 m_ClientSceneMap[clientId] = clientScene;
 
                 connectionApprovedCallback(true, null, true, Vector3.zero, Quaternion.identity);
-
                 // connection approval will create a player object for you
             }
+            else
+            {
+                //TODO-FIXME:Netcode Issue #796. We should be able to send a reason and disconnect without a coroutine delay.
+                //TODO:Netcode: In the future we expect Netcode to allow us to return more information as part of
+                //the approval callback, so that we can provide more context on a reject. In the meantime we must provide the extra information ourselves,
+                //and then manually close down the connection.
+                SendServerToClientConnectResult(clientId, gameReturnStatus);
+                SendServerToClientSetDisconnectReason(clientId, gameReturnStatus);
+                StartCoroutine(WaitToDisconnect(clientId));
+                if (m_LobbyServiceFacade.CurrentUnityLobby != null)
+                {
+                    m_LobbyServiceFacade.RemovePlayerFromLobbyAsync(connectionPayload.playerId, m_LobbyServiceFacade.CurrentUnityLobby.Id, null, null);
+                }
+            }
         }
 
         IEnumerator WaitToDisconnect(ulong clientId)

Assets/BossRoom/Scripts/Shared/Net/UnityServices/Lobbies/LobbyServiceFacade.cs

@@ -74,13 +74,17 @@ public void Dispose()
             m_ServiceScope?.Dispose();
         }
 
-        public void BeginTracking(Lobby lobby)
+        public void SetRemoteLobby(Lobby lobby)
+        {
+            CurrentUnityLobby = lobby;
+            m_LocalLobby.ApplyRemoteData(lobby);
+        }
+
+        public void BeginTracking()
         {
             if(!m_IsTracking)
             {
                 m_IsTracking = true;
-                CurrentUnityLobby = lobby;
-                m_LocalLobby.ApplyRemoteData(lobby);
                 // 2s update cadence is arbitrary and is here to demonstrate the fact that this update can be rather infrequent
                 // the actual rate limits are tracked via the RateLimitCooldown objects defined above
                 m_UpdateRunner.Subscribe(UpdateLobby, 2f);
@@ -91,12 +95,8 @@ public void BeginTracking(Lobby lobby)
         public Task EndTracking()
         {
             var task = Task.CompletedTask;
-            if (m_IsTracking)
+            if (CurrentUnityLobby != null)
             {
-                m_UpdateRunner.Unsubscribe(UpdateLobby);
-                m_IsTracking = false;
-                m_HeartbeatTime = 0;
-                m_JoinedLobbyContentHeartbeat.EndTracking();
                 CurrentUnityLobby = null;
 
                 if (!string.IsNullOrEmpty(m_LocalLobby?.LobbyID))
@@ -108,6 +108,13 @@ public Task EndTracking()
                 m_LocalLobby?.Reset(m_LocalUser);
             }
 
+            if (m_IsTracking)
+            {
+                m_UpdateRunner.Unsubscribe(UpdateLobby);
+                m_IsTracking = false;
+                m_HeartbeatTime = 0;
+                m_JoinedLobbyContentHeartbeat.EndTracking();
+            }
             return task;
         }