fix: replace null-coalescencing and null-conditional operators [MTT-7125]

Assets/Scripts/ApplicationLifecycle/ApplicationController.cs

@@ -98,8 +98,16 @@ private void Start()
 
         protected override void OnDestroy()
         {
-            m_Subscriptions?.Dispose();
-            m_LobbyServiceFacade?.EndTracking();
+            if (m_Subscriptions != null)
+            {
+                m_Subscriptions.Dispose();
+            }
+
+            if (m_LobbyServiceFacade != null)
+            {
+                m_LobbyServiceFacade.EndTracking();
+            }
+
             base.OnDestroy();
         }
 
@@ -127,7 +135,7 @@ private bool OnWantToQuit()
         {
             Application.wantsToQuit -= OnWantToQuit;
 
-            var canQuit = string.IsNullOrEmpty(m_LocalLobby?.LobbyID);
+            var canQuit = m_LocalLobby != null && string.IsNullOrEmpty(m_LocalLobby.LobbyID);
             if (!canQuit)
             {
                 StartCoroutine(LeaveBeforeQuit());

Assets/Scripts/ConnectionManagement/ConnectionState/HostingState.cs

@@ -42,7 +42,19 @@ public override void Exit()
 
         public override void OnClientConnected(ulong clientId)
         {
-            m_ConnectionEventPublisher.Publish(new ConnectionEventMessage() { ConnectStatus = ConnectStatus.Success, PlayerName = SessionManager<SessionPlayerData>.Instance.GetPlayerData(clientId)?.PlayerName });
+            var playerData = SessionManager<SessionPlayerData>.Instance.GetPlayerData(clientId);
+            if (playerData != null)
+            {
+                m_ConnectionEventPublisher.Publish(new ConnectionEventMessage() { ConnectStatus = ConnectStatus.Success, PlayerName = playerData.Value.PlayerName });
+            }
+            else
+            {
+                // This should not happen since player data is assigned during connection approval
+                Debug.LogError($"No player data associated with client {clientId}");
+                var reason = JsonUtility.ToJson(ConnectStatus.GenericDisconnect);
+                m_ConnectionManager.NetworkManager.DisconnectClient(clientId, reason);
+            }
+
         }
 
         public override void OnClientDisconnect(ulong clientId)

Assets/Scripts/Gameplay/GameState/ServerBossRoomState.cs

@@ -88,15 +88,22 @@ void OnNetworkSpawn()
 
         void OnNetworkDespawn()
         {
-            m_LifeStateChangedEventMessageSubscriber?.Unsubscribe(OnLifeStateChangedEventMessage);
+            if (m_LifeStateChangedEventMessageSubscriber != null)
+            {
+                m_LifeStateChangedEventMessageSubscriber.Unsubscribe(OnLifeStateChangedEventMessage);
+            }
+
             NetworkManager.Singleton.OnClientDisconnectCallback -= OnClientDisconnect;
             NetworkManager.Singleton.SceneManager.OnLoadEventCompleted -= OnLoadEventCompleted;
             NetworkManager.Singleton.SceneManager.OnSynchronizeComplete -= OnSynchronizeComplete;
         }
 
         protected override void OnDestroy()
         {
-            m_LifeStateChangedEventMessageSubscriber?.Unsubscribe(OnLifeStateChangedEventMessage);
+            if (m_LifeStateChangedEventMessageSubscriber != null)
+            {
+                m_LifeStateChangedEventMessageSubscriber.Unsubscribe(OnLifeStateChangedEventMessage);
+            }
 
             if (m_NetcodeHooks)
             {

Assets/Scripts/Gameplay/GameplayObjects/Character/ServerCharacterMovement.cs

@@ -141,7 +141,10 @@ public bool IsMoving()
         /// </summary>
         public void CancelMove()
         {
-            m_NavPath?.Clear();
+            if (m_NavPath != null)
+            {
+                m_NavPath.Clear();
+            }
             m_MovementState = MovementState.Idle;
         }
 

Assets/Scripts/Gameplay/GameplayObjects/SwitchedDoor.cs

@@ -100,7 +100,10 @@ void OnDoorStateChanged(bool wasDoorOpen, bool isDoorOpen)
             if (IsClient)
             {
                 m_PhysicsObject.SetActive(!isDoorOpen);
-                m_Publisher?.Publish(new DoorStateChangedEventMessage() { IsDoorOpen = isDoorOpen });
+                if (m_Publisher != null)
+                {
+                    m_Publisher.Publish(new DoorStateChangedEventMessage() { IsDoorOpen = isDoorOpen });
+                }
             }
         }
 

Assets/Scripts/Gameplay/UI/ConnectionStatusMessageUIManager.cs

@@ -30,7 +30,10 @@ void Awake()
 
         void OnDestroy()
         {
-            m_Subscriptions?.Dispose();
+            if (m_Subscriptions != null)
+            {
+                m_Subscriptions.Dispose();
+            }
         }
 
         void OnConnectStatus(ConnectStatus status)

Assets/Scripts/Gameplay/UI/IPConnectionWindow.cs

@@ -36,7 +36,10 @@ void Awake()
 
         void OnDestroy()
         {
-            m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatusMessage);
+            if (m_ConnectStatusSubscriber != null)
+            {
+                m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatusMessage);
+            }
         }
 
         void OnConnectStatusMessage(ConnectStatus connectStatus)

Assets/Scripts/Gameplay/UI/IPUIMediator.cs

@@ -65,7 +65,10 @@ void Start()
 
         void OnDestroy()
         {
-            m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatusMessage);
+            if (m_ConnectStatusSubscriber != null)
+            {
+                m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatusMessage);
+            }
         }
 
         void OnConnectStatusMessage(ConnectStatus connectStatus)

Assets/Scripts/Gameplay/UI/Lobby/LobbyJoiningUI.cs

@@ -41,13 +41,16 @@ void OnDisable()
         {
             if (m_UpdateRunner != null)
             {
-                m_UpdateRunner?.Unsubscribe(PeriodicRefresh);
+                m_UpdateRunner.Unsubscribe(PeriodicRefresh);
             }
         }
 
         void OnDestroy()
         {
-            m_LocalLobbiesRefreshedSub?.Unsubscribe(UpdateUI);
+            if (m_LocalLobbiesRefreshedSub != null)
+            {
+                m_LocalLobbiesRefreshedSub.Unsubscribe(UpdateUI);
+            }
         }
 
         [Inject]

Assets/Scripts/Gameplay/UI/Lobby/LobbyUIMediator.cs

@@ -66,7 +66,10 @@ void OnConnectStatus(ConnectStatus status)
 
         void OnDestroy()
         {
-            m_ConnectStatusSubscriber?.Unsubscribe(OnConnectStatus);
+            if (m_ConnectStatusSubscriber != null)
+            {
+                m_ConnectStatusSubscriber.Unsubscribe(OnConnectStatus);
+            }
         }
 
         //Lobby and Relay calls done from UI

Assets/Scripts/Gameplay/UI/UIMessageFeed.cs

@@ -124,7 +124,10 @@ UIMessageSlot GetAvailableSlot()
 
         void OnDestroy()
         {
-            m_Subscriptions?.Dispose();
+            if (m_Subscriptions != null)
+            {
+                m_Subscriptions.Dispose();
+            }
         }
 
     }

Assets/Scripts/Gameplay/UI/UnityServicesUIHandler.cs

@@ -87,7 +87,10 @@ void HandleLobbyError(UnityServiceErrorMessage error)
 
         void OnDestroy()
         {
-            m_ServiceErrorSubscription?.Unsubscribe(ServiceErrorHandler);
+            if (m_ServiceErrorSubscription != null)
+            {
+                m_ServiceErrorSubscription.Unsubscribe(ServiceErrorHandler);
+            }
         }
     }
 }

Assets/Scripts/Infrastructure/PubSub/MessageChannel.cs

@@ -42,7 +42,10 @@ public virtual void Publish(T message)
 
             foreach (var messageHandler in m_MessageHandlers)
             {
-                messageHandler?.Invoke(message);
+                if (messageHandler != null)
+                {
+                    messageHandler.Invoke(message);
+                }
             }
         }
 

Assets/Scripts/UnityServices/Auth/AuthenticationServiceFacade.cs

@@ -27,7 +27,7 @@ public InitializationOptions GenerateAuthenticationOptions(string profile)
             }
             catch (Exception e)
             {
-                var reason = $"{e.Message} ({e.InnerException?.Message})";
+                var reason = e.InnerException == null ? e.Message : $"{e.Message} ({e.InnerException.Message})";
                 m_UnityServiceErrorMessagePublisher.Publish(new UnityServiceErrorMessage("Authentication Error", reason, UnityServiceErrorMessage.Service.Authentication, e));
                 throw;
             }
@@ -46,7 +46,7 @@ public async Task InitializeAndSignInAsync(InitializationOptions initializationO
             }
             catch (Exception e)
             {
-                var reason = $"{e.Message} ({e.InnerException?.Message})";
+                var reason = e.InnerException == null ? e.Message : $"{e.Message} ({e.InnerException.Message})";
                 m_UnityServiceErrorMessagePublisher.Publish(new UnityServiceErrorMessage("Authentication Error", reason, UnityServiceErrorMessage.Service.Authentication, e));
                 throw;
             }
@@ -67,7 +67,7 @@ public async Task SwitchProfileAndReSignInAsync(string profile)
             }
             catch (Exception e)
             {
-                var reason = $"{e.Message} ({e.InnerException?.Message})";
+                var reason = e.InnerException == null ? e.Message : $"{e.Message} ({e.InnerException.Message})";
                 m_UnityServiceErrorMessagePublisher.Publish(new UnityServiceErrorMessage("Authentication Error", reason, UnityServiceErrorMessage.Service.Authentication, e));
                 throw;
             }
@@ -87,7 +87,7 @@ public async Task<bool> EnsurePlayerIsAuthorized()
             }
             catch (AuthenticationException e)
             {
-                var reason = $"{e.Message} ({e.InnerException?.Message})";
+                var reason = e.InnerException == null ? e.Message : $"{e.Message} ({e.InnerException.Message})";
                 m_UnityServiceErrorMessagePublisher.Publish(new UnityServiceErrorMessage("Authentication Error", reason, UnityServiceErrorMessage.Service.Authentication, e));
 
                 //not rethrowing for authentication exceptions - any failure to authenticate is considered "handled failure"
@@ -96,7 +96,7 @@ public async Task<bool> EnsurePlayerIsAuthorized()
             catch (Exception e)
             {
                 //all other exceptions should still bubble up as unhandled ones
-                var reason = $"{e.Message} ({e.InnerException?.Message})";
+                var reason = e.InnerException == null ? e.Message : $"{e.Message} ({e.InnerException.Message})";
                 m_UnityServiceErrorMessagePublisher.Publish(new UnityServiceErrorMessage("Authentication Error", reason, UnityServiceErrorMessage.Service.Authentication, e));
                 throw;
             }

Assets/Scripts/UnityServices/Lobbies/LobbyServiceFacade.cs

@@ -228,8 +228,14 @@ public void EndTracking()
         void ResetLobby()
         {
             CurrentUnityLobby = null;
-            m_LocalUser.ResetState();
-            m_LocalLobby?.Reset(m_LocalUser);
+            if (m_LocalUser != null)
+            {
+                m_LocalUser.ResetState();
+            }
+            if (m_LocalLobby != null)
+            {
+                m_LocalLobby.Reset(m_LocalUser);
+            }
 
             // no need to disconnect Netcode, it should already be handled by Netcode's callback to disconnect
         }
@@ -471,7 +477,11 @@ public async Task UpdateLobbyDataAndUnlockAsync()
 
             var localData = m_LocalLobby.GetDataForUnityServices();
 
-            var dataCurr = CurrentUnityLobby.Data ?? new Dictionary<string, DataObject>();
+            var dataCurr = CurrentUnityLobby.Data;
+            if (dataCurr == null)
+            {
+                dataCurr = new Dictionary<string, DataObject>();
+            }
 
             foreach (var dataNew in localData)
             {
@@ -533,7 +543,7 @@ void DoLobbyHeartbeat(float dt)
 
         void PublishError(LobbyServiceException e)
         {
-            var reason = $"{e.Message} ({e.InnerException?.Message})"; // Lobby error type, then HTTP error type.
+            var reason = e.InnerException == null ? e.Message : $"{e.Message} ({e.InnerException.Message})"; // Lobby error type, then HTTP error type.
             m_UnityServiceErrorMessagePub.Publish(new UnityServiceErrorMessage("Lobby Error", reason, UnityServiceErrorMessage.Service.Lobby, e));
         }
     }

Assets/Scripts/UnityServices/Lobbies/LocalLobby.cs

@@ -255,7 +255,7 @@ public void ApplyRemoteData(Lobby lobby)
                 var incomingData = new LocalLobbyUser
                 {
                     IsHost = lobby.HostId.Equals(player.Id),
-                    DisplayName = player.Data?.ContainsKey("DisplayName") == true ? player.Data["DisplayName"].Value : default,
+                    DisplayName = player.Data != null && player.Data.ContainsKey("DisplayName") ? player.Data["DisplayName"].Value : default,
                     ID = player.Id
                 };
 

Assets/Scripts/Utils/NetworkSimulatorUIMediator.cs

@@ -144,7 +144,11 @@ void OnScenarioChanged(int optionIndex)
                     break;
             }
             m_NetworkSimulator.Scenario = scenario;
-            m_NetworkSimulator.Scenario?.Start(m_NetworkSimulator);
+            if (m_NetworkSimulator.Scenario != null)
+            {
+                m_NetworkSimulator.Scenario.Start(m_NetworkSimulator);
+            }
+
             UpdateScenarioButton();
         }
 

Assets/Scripts/Utils/ProfileManager.cs

@@ -21,7 +21,12 @@ public string Profile
         {
             get
             {
-                return m_Profile ??= GetProfile();
+                if (m_Profile == null)
+                {
+                    m_Profile = GetProfile();
+                }
+
+                return m_Profile;
             }
             set
             {

CHANGELOG.md

@@ -11,6 +11,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Changed
 * Upgraded editor version to 2022.3.7f1 (#855)
   * Upgraded Authentication Service package to v2.7.1
+* Replaced usages of null-coalescing and null-conditional operators with regular null checks. (#867) These operators can cause issues when used with types inheriting UnityEngine.Object because that type redefines the == operator to define when an object is null. This redefinition applies to regular null checks (if foo == null) but not to those operators, thus this could lead to unexpected behaviour. While those operators were safely used within Boss Room, only with types that were not inheriting UnityEngine.Object, we decided to remove most usages for consistency. This will also help avoid accidental mistakes, such as a user reusing a part of this code, but modifying it so that one of those operators are used with a UnityEngine.Object.
 * Upgraded Boss Room to Netcode for GameObjects v1.6.0 (#865)
   * A package Version Define has been created for Netcode for GameObjects v.1.5.2 - v1.6.0. Recent refactorings to NetworkManager's shutdown have prevented the ability to invoke CustomMessages when OnClientDisconnected callbacks are invoked during a shutdown as host. This regression has caused one of our runtime tests, namely Unity.BossRoom.Tests.Runtime.ConnectionManagementTests.UnexpectedServerShutdown_ClientsFailToReconnect, to fail and it does not impact gameplay. This is a known issue and will be addressed in a future NGO version.
 * Upgraded to Lobby 1.1.0 (#860).

Packages/com.unity.multiplayer.samples.coop/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## [Unreleased] - yyyy-mm-dd
 
+### Changed
+* Replaced usages of null-coalescing and null-conditional operators with regular null checks. (#867) These operators can cause issues when used with types inheriting UnityEngine.Object because that type redefines the == operator to define when an object is null. This redefinition applies to regular null checks (if foo == null) but not to those operators, thus this could lead to unexpected behaviour. While those operators were safely used within Boss Room, only with types that were not inheriting UnityEngine.Object, we decided to remove most usages for consistency. This will also help avoid accidental mistakes, such as a user reusing a part of this code, but modifying it so that one of those operators are used with a UnityEngine.Object.
+
 ### Fixed
 * Clarified fix used to handle clients reconnecting to the server while keeping the same active scene and having additional scenes additively loaded (#864)
 * Host loading progress bar is now properly updated on all clients during scene loads (#862)

Packages/com.unity.multiplayer.samples.coop/Utilities/EditorChildSceneLoader.cs

@@ -23,8 +23,14 @@ void Update()
 
     public void SaveSceneSetup()
     {
-        ChildScenesToLoadConfig ??= new List<SceneAsset>();
-        ChildScenesToLoadConfig.Clear();
+        if (ChildScenesToLoadConfig == null)
+        {
+            ChildScenesToLoadConfig = new List<SceneAsset>();
+        }
+        else
+        {
+            ChildScenesToLoadConfig.Clear();
+        }
         foreach (var sceneSetup in EditorSceneManager.GetSceneManagerSetup())
         {
             ChildScenesToLoadConfig.Add(AssetDatabase.LoadAssetAtPath<SceneAsset>(sceneSetup.path));