fix: Removed unnecessary GC Allocation [3527-Backport] (#3601)

Fix for [MTTB-85](https://jira.unity3d.com/browse/MTTB-85) removed the
use of a foreach causing an unnecessary GC Allocation.

Did a full sweep of internal `NetworkManager.ConnectedClientsIds` and
replaced with either `for` loop or directly referencing the
`NetworkConnectionManager.ConnectedClientIds`.

## Changelog

- Fixed: Issue with unnecessary internal GC Allocations when using the
`IReadOnlyList` `NetworkManager.ConnectedClientsIds` within a `foreach`
statement by either replacing with a `for` loop or directly referencing
the `NetworkConnectionManager.ConnectedClientIds`.

## Documentation

- No documentation updates were required.

## Testing & QA
[//]: #  (
This section is REQUIRED and should describe how the changes were tested
and how should they be tested when Playtesting for the release.
It can range from "edge case covered by unit tests" to "manual testing
required and new sample was added".
Expectation is that PR creator does some manual testing and provides a
summary of it here.)

### Functional Testing
[//]: # (If checked, List manual tests that have been performed.)
_Manual testing :_
- [X] `Manual testing done`
- Tested manually before and after fix to confirm the allocation was
present and then absent.

_Automated tests:_
- [ ] `Covered by existing automated tests`
- [ ] `Covered by new automated tests`

_Does the change require QA team to:_

- [ ] `Review automated tests`?
- [ ] `Execute manual tests`?

If any boxes above are checked, please add QA as a PR reviewer.


## Backport
This is the backport for #3527

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -14,9 +14,11 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
-- Removed allocation to the heap in NetworkBehaviourUpdate. (#3568)
+- Fixed issue with unnecessary internal GC Allocations when using the `IReadOnlyList`  `NetworkManager.ConnectedClientsIds` within a `foreach` statement by either replacing with a `for` loop or directly referencing the `NetworkConnectionManager.ConnectedClientIds`. (#3601)
+- Fixed issue with allocation to the heap in NetworkBehaviourUpdate when there is nothing to be updated. (#3568)
 - Fixed issue where NetworkConfig.ConnectionData could cause the ConnectionRequestMessage to exceed the transport's MTU size and would result in a buffer overflow error. (#3565)
 
+
 ### Changed
 
 

com.unity.netcode.gameobjects/Components/NetworkAnimator.cs

@@ -961,7 +961,7 @@ internal void CheckForAnimatorChanges()
                 {
                     // Just notify all remote clients and not the local server
                     m_ClientSendList.Clear();
-                    foreach (var clientId in NetworkManager.ConnectedClientsIds)
+                    foreach (var clientId in NetworkManager.ConnectionManager.ConnectedClientIds)
                     {
                         if (clientId == NetworkManager.LocalClientId || !NetworkObject.Observers.Contains(clientId))
                         {
@@ -1320,7 +1320,7 @@ private unsafe void SendAnimStateServerRpc(AnimationMessage animationMessage, Se
                 if (NetworkManager.ConnectedClientsIds.Count > (IsHost ? 2 : 1))
                 {
                     m_ClientSendList.Clear();
-                    foreach (var clientId in NetworkManager.ConnectedClientsIds)
+                    foreach (var clientId in NetworkManager.ConnectionManager.ConnectedClientIds)
                     {
                         if (clientId == serverRpcParams.Receive.SenderClientId || clientId == NetworkManager.ServerClientId || !NetworkObject.Observers.Contains(clientId))
                         {

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -134,12 +134,12 @@ internal void InvokeOnClientConnectedCallback(ulong clientId)
 
             if (!NetworkManager.IsServer)
             {
-                var peerClientIds = new NativeArray<ulong>(Math.Max(NetworkManager.ConnectedClientsIds.Count - 1, 0), Allocator.Temp);
+                var peerClientIds = new NativeArray<ulong>(Math.Max(ConnectedClientIds.Count - 1, 0), Allocator.Temp);
                 // `using var peerClientIds` or `using(peerClientIds)` renders it immutable...
                 using var sentinel = peerClientIds;
 
                 var idx = 0;
-                foreach (var peerId in NetworkManager.ConnectedClientsIds)
+                foreach (var peerId in ConnectedClientIds)
                 {
                     if (peerId == NetworkManager.LocalClientId)
                     {

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

@@ -1252,10 +1252,10 @@ private void OnTransformParentChanged()
 
             unsafe
             {
-                var maxCount = NetworkManager.ConnectedClientsIds.Count;
+                var maxCount = NetworkManager.ConnectionManager.ConnectedClientIds.Count;
                 ulong* clientIds = stackalloc ulong[maxCount];
                 int idx = 0;
-                foreach (var clientId in NetworkManager.ConnectedClientsIds)
+                foreach (var clientId in NetworkManager.ConnectionManager.ConnectedClientIds)
                 {
                     if (Observers.Contains(clientId))
                     {

com.unity.netcode.gameobjects/Runtime/Messaging/RpcTargets/BaseRpcTarget.cs

@@ -11,6 +11,7 @@ public abstract class BaseRpcTarget : IDisposable
         /// The <see cref="NetworkManager"/> instance which can be used to handle sending and receiving the specific target(s)
         /// </summary>
         protected NetworkManager m_NetworkManager;
+        internal NetworkConnectionManager ConnectionManager;
         private bool m_Locked;
 
         internal void Lock()
@@ -26,6 +27,7 @@ internal void Unlock()
         internal BaseRpcTarget(NetworkManager manager)
         {
             m_NetworkManager = manager;
+            ConnectionManager = m_NetworkManager.ConnectionManager;
         }
 
         /// <summary>

com.unity.netcode.gameobjects/Runtime/Messaging/RpcTargets/NotMeRpcTarget.cs

@@ -43,7 +43,7 @@ internal override void Send(NetworkBehaviour behaviour, ref RpcMessage message,
             }
             else
             {
-                foreach (var clientId in m_NetworkManager.ConnectedClientsIds)
+                foreach (var clientId in ConnectionManager.ConnectedClientIds)
                 {
                     if (clientId == behaviour.NetworkManager.LocalClientId)
                     {

com.unity.netcode.gameobjects/Runtime/Messaging/RpcTargets/NotOwnerRpcTarget.cs

@@ -55,7 +55,7 @@ internal override void Send(NetworkBehaviour behaviour, ref RpcMessage message,
             }
             else
             {
-                foreach (var clientId in m_NetworkManager.ConnectedClientsIds)
+                foreach (var clientId in ConnectionManager.ConnectedClientIds)
                 {
                     if (clientId == behaviour.OwnerClientId)
                     {

com.unity.netcode.gameobjects/Runtime/Messaging/RpcTargets/NotServerRpcTarget.cs

@@ -44,7 +44,7 @@ internal override void Send(NetworkBehaviour behaviour, ref RpcMessage message,
             }
             else
             {
-                foreach (var clientId in m_NetworkManager.ConnectedClientsIds)
+                foreach (var clientId in ConnectionManager.ConnectedClientIds)
                 {
                     if (clientId == NetworkManager.ServerClientId)
                     {

com.unity.netcode.gameobjects/Runtime/Messaging/RpcTargets/RpcTarget.cs

@@ -107,9 +107,11 @@ public enum RpcTargetUse
     public class RpcTarget
     {
         private NetworkManager m_NetworkManager;
+        private NetworkConnectionManager m_ConnectionManager;
         internal RpcTarget(NetworkManager manager)
         {
             m_NetworkManager = manager;
+            m_ConnectionManager = manager.ConnectionManager;
 
             Everyone = new EveryoneRpcTarget(manager);
             Owner = new OwnerRpcTarget(manager);
@@ -284,8 +286,9 @@ public BaseRpcTarget Not(ulong excludedClientId, RpcTargetUse use)
                     target = m_CachedProxyRpcTargetGroup;
                 }
             }
+
             target.Clear();
-            foreach (var clientId in m_NetworkManager.ConnectedClientsIds)
+            foreach (var clientId in m_ConnectionManager.ConnectedClientIds)
             {
                 if (clientId != excludedClientId)
                 {
@@ -468,7 +471,7 @@ public BaseRpcTarget Not(NativeArray<ulong> excludedClientIds, RpcTargetUse use)
                 asASet.Add(clientId);
             }
 
-            foreach (var clientId in m_NetworkManager.ConnectedClientsIds)
+            foreach (var clientId in m_ConnectionManager.ConnectedClientIds)
             {
                 if (!asASet.Contains(clientId))
                 {
@@ -557,13 +560,13 @@ public BaseRpcTarget Not<T>(T excludedClientIds, RpcTargetUse use) where T : IEn
             }
             target.Clear();
 
-            using var asASet = new NativeHashSet<ulong>(m_NetworkManager.ConnectedClientsIds.Count, Allocator.Temp);
+            using var asASet = new NativeHashSet<ulong>(m_ConnectionManager.ConnectedClientIds.Count, Allocator.Temp);
             foreach (var clientId in excludedClientIds)
             {
                 asASet.Add(clientId);
             }
 
-            foreach (var clientId in m_NetworkManager.ConnectedClientsIds)
+            foreach (var clientId in m_ConnectionManager.ConnectedClientIds)
             {
                 if (!asASet.Contains(clientId))
                 {

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

@@ -829,7 +829,7 @@ internal NetworkSceneManager(NetworkManager networkManager)
         private void SceneManager_ActiveSceneChanged(Scene current, Scene next)
         {
             // If no clients are connected, then don't worry about notifications
-            if (!(NetworkManager.ConnectedClientsIds.Count > (NetworkManager.IsHost ? 1 : 0)))
+            if (!(NetworkManager.ConnectionManager.ConnectedClientIds.Count > (NetworkManager.IsHost ? 1 : 0)))
             {
                 return;
             }
@@ -850,7 +850,7 @@ private void SceneManager_ActiveSceneChanged(Scene current, Scene next)
                 var sceneEvent = BeginSceneEvent();
                 sceneEvent.SceneEventType = SceneEventType.ActiveSceneChanged;
                 sceneEvent.ActiveSceneHash = BuildIndexToHash[next.buildIndex];
-                SendSceneEventData(sceneEvent.SceneEventId, NetworkManager.ConnectedClientsIds.Where(c => c != NetworkManager.ServerClientId).ToArray());
+                SendSceneEventData(sceneEvent.SceneEventId, NetworkManager.ConnectionManager.ConnectedClientIds.Where(c => c != NetworkManager.ServerClientId).ToArray());
                 EndSceneEvent(sceneEvent.SceneEventId);
             }
         }
@@ -1139,10 +1139,10 @@ private bool OnSceneEventProgressCompleted(SceneEventProgress sceneEventProgress
             {
                 EventData = sceneEventData
             };
-            var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, NetworkManager.ConnectedClientsIds);
+            var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, NetworkManager.ConnectionManager.ConnectedClientIds);
 
             NetworkManager.NetworkMetrics.TrackSceneEventSent(
-                NetworkManager.ConnectedClientsIds,
+                NetworkManager.ConnectionManager.ConnectedClientIds,
                 (uint)sceneEventProgress.SceneEventType,
                 SceneNameFromHash(sceneEventProgress.SceneHash),
                 size);
@@ -1291,7 +1291,7 @@ private void OnSceneUnloaded(uint sceneEventId)
                 // Server sends the unload scene notification after unloading because it will despawn all scene relative in-scene NetworkObjects
                 // If we send this event to all clients before the server is finished unloading they will get warning about an object being
                 // despawned that no longer exists
-                SendSceneEventData(sceneEventId, NetworkManager.ConnectedClientsIds.Where(c => c != NetworkManager.ServerClientId).ToArray());
+                SendSceneEventData(sceneEventId, NetworkManager.ConnectionManager.ConnectedClientIds.Where(c => c != NetworkManager.ServerClientId).ToArray());
 
                 //Only if we are a host do we want register having loaded for the associated SceneEventProgress
                 if (SceneEventProgressTracking.ContainsKey(sceneEventData.SceneEventProgressId) && NetworkManager.IsHost)
@@ -2515,7 +2515,7 @@ internal void CheckForAndSendNetworkObjectSceneChanged()
             // Some NetworkObjects still exist, send the message
             var sceneEvent = BeginSceneEvent();
             sceneEvent.SceneEventType = SceneEventType.ObjectSceneChanged;
-            SendSceneEventData(sceneEvent.SceneEventId, NetworkManager.ConnectedClientsIds.Where(c => c != NetworkManager.ServerClientId).ToArray());
+            SendSceneEventData(sceneEvent.SceneEventId, NetworkManager.ConnectionManager.ConnectedClientIds.Where(c => c != NetworkManager.ServerClientId).ToArray());
             EndSceneEvent(sceneEvent.SceneEventId);
         }