fix: Removed unnecessary GC Allocation (#3527)

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
The backport of this PR is #3601

---------

Co-authored-by: NoelStephensUnity <[email protected]>

Author: samlucas-unity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -17,6 +17,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed an issue where `UnityTransport` would not accept single words as valid hostnames (notably "localhost"). (#3591)
 - Fixed issue where viewing a `NetworkBehaviour` with one or more `NetworkVariable` fields could throw an exception if running a distributed authority network topology with a local (DAHost) host and viewed on the host when the host is not the authority of the associated `NetworkObject`. (#3578)
 - Fixed issue when using a distributed authority network topology and  viewing a `NetworkBehaviour` with one or more `NetworkVariable` fields in the inspector view would not show editable fields. (#3578)
+- 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`. (#3527)
 
 ### Changed
 

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

@@ -996,7 +996,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))
                         {
@@ -1315,7 +1315,7 @@ private unsafe void SendParametersUpdateServerRpc(ParametersUpdateMessage parame
                 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))
                         {
@@ -1378,7 +1378,7 @@ private void SendAnimStateServerRpc(AnimationMessage animationMessage, ServerRpc
                 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))
                         {
@@ -1452,7 +1452,7 @@ internal void SendAnimTriggerServerRpc(AnimationTriggerMessage animationTriggerM
             InternalSetTrigger(animationTriggerMessage.Hash, animationTriggerMessage.IsTriggerSet);
 
             m_ClientSendList.Clear();
-            foreach (var clientId in NetworkManager.ConnectedClientsIds)
+            foreach (var clientId in NetworkManager.ConnectionManager.ConnectedClientIds)
             {
                 if (clientId == NetworkManager.ServerClientId || !NetworkObject.Observers.Contains(clientId))
                 {

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

@@ -149,12 +149,12 @@ internal void InvokeOnClientConnectedCallback(ulong clientId)
             }
 
             // Invoking connection event on non-authority local client. Need to calculate PeerIds.
-            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

@@ -1616,7 +1616,7 @@ public void NetworkHide(ulong clientId)
                         // Send destroy call
                         size = NetworkManager.ConnectionManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, clientId);
                         // Broadcast the destroy to all clients so they can update their observers list
-                        foreach (var client in NetworkManager.ConnectedClientsIds)
+                        foreach (var client in NetworkManager.ConnectionManager.ConnectedClientIds)
                         {
                             if (client == clientId || client == NetworkManager.LocalClientId)
                             {
@@ -2363,7 +2363,7 @@ private void OnTransformParentChanged()
                 }
                 else
                 {
-                    foreach (var clientId in NetworkManager.ConnectedClientsIds)
+                    foreach (var clientId in NetworkManager.ConnectionManager.ConnectedClientIds)
                     {
                         if (clientId == NetworkManager.ServerClientId)
                         {
@@ -2382,7 +2382,7 @@ private void OnTransformParentChanged()
                     var maxCount = NetworkManager.ConnectedClientsIds.Count;
                     ulong* clientIds = stackalloc ulong[maxCount];
                     int idx = 0;
-                    foreach (var clientId in NetworkManager.ConnectedClientsIds)
+                    foreach (var clientId in NetworkManager.ConnectionManager.ConnectedClientIds)
                     {
                         if (clientId == NetworkManager.ServerClientId)
                         {

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs

@@ -250,8 +250,9 @@ public void Handle(ref NetworkContext context)
                 }
                 else
                 {
-                    foreach (var clientId in clientList)
+                    for (int i = 0; i < clientList.Count; i++)
                     {
+                        var clientId = clientList[i];
                         if (clientId == networkManager.LocalClientId)
                         {
                             continue;

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/CreateObjectMessage.cs

@@ -247,9 +247,9 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
                     var clientList = hasObserverIdList && !networkObject.IsPlayerObject ? observerIds : networkManager.ConnectedClientsIds;
 
                     // Update the observers for this instance
-                    foreach (var clientId in clientList)
+                    for (int i = 0; i < clientList.Count; i++)
                     {
-                        networkObject.Observers.Add(clientId);
+                        networkObject.Observers.Add(clientList[i]);
                     }
 
                     // Mock CMB Service and forward to all clients

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DestroyObjectMessage.cs

@@ -191,7 +191,7 @@ private void HandleDAHostForwardMessage(ulong senderId, ref NetworkManager netwo
                 DeferredDespawnTick = DeferredDespawnTick,
             };
             var ownerClientId = networkObject == null ? senderId : networkObject.OwnerClientId;
-            var clientIds = networkObject == null ? networkManager.ConnectedClientsIds.ToList() : networkObject.Observers.ToList();
+            var clientIds = networkObject == null ? networkManager.ConnectionManager.ConnectedClientIds : networkObject.Observers.ToList();
 
             foreach (var clientId in clientIds)
             {

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs

@@ -228,7 +228,7 @@ public void Handle(ref NetworkContext context)
                     if (isServerAndDeltaForwarding)
                     {
                         m_ForwardUpdates = new Dictionary<ulong, List<int>>();
-                        foreach (var clientId in networkManager.ConnectedClientsIds)
+                        foreach (var clientId in networkManager.ConnectionManager.ConnectedClientIds)
                         {
                             if (clientId == context.SenderId || clientId == networkManager.LocalClientId || !networkObject.Observers.Contains(clientId))
                             {

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

@@ -11,6 +11,8 @@ 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 +28,7 @@ internal void Unlock()
         internal BaseRpcTarget(NetworkManager manager)
         {
             m_NetworkManager = manager;
+            ConnectionManager = m_NetworkManager.ConnectionManager;
         }
 
         /// <summary>

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

@@ -40,7 +40,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 || !networkObject.Observers.Contains(clientId))
                         {