fix: Server was able to be mapped as both ID 0 and ID 1 (#1683)

Author: ShadauxCat

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -14,6 +14,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed an issue where sometimes the first client to connect to the server could see messages from the server as coming from itself. (#1683)
+- Fixed an issue where clients seemed to be able to send messages to ClientId 1, but these messages would actually still go to the server (id 0) instead of that client. (#1683)
+- Improved clarity of error messaging when a client attempts to send a message to a destination other than the server, which isn't allowed. (#1683)
 - Disallowed async keyword in RPCs (#1681)
 - Fixed an issue where Alpha release versions of Unity (version 20202.2.0a5 and later) will not compile due to the UNet Transport no longer existing (#1678)
 - Fixed messages larger than 64k being written with incorrectly truncated message size in header (#1686) (credit: @kaen)

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

@@ -1354,7 +1354,20 @@ private void HandleRawTransportPoll(NetworkEvent networkEvent, ulong clientId, A
                     s_TransportConnect.Begin();
 #endif
 
-                    clientId = m_NextClientId++;
+                    // Assumptions:
+                    // - When server receives a connection, it *must be* a client
+                    // - When client receives one, it *must be* the server
+                    // Client's can't connect to or talk to other clients.
+                    // Server is a sentinel so only one exists, if we are server, we can't be
+                    // connecting to it.
+                    if (IsServer)
+                    {
+                        clientId = m_NextClientId++;
+                    }
+                    else
+                    {
+                        clientId = ServerClientId;
+                    }
                     m_ClientIdToTransportIdMap[clientId] = transportId;
                     m_TransportIdToClientIdMap[transportId] = clientId;
 
@@ -1457,6 +1470,12 @@ internal unsafe int SendMessage<TMessageType, TClientIdListType>(ref TMessageTyp
                 }
                 return MessagingSystem.SendMessage(ref message, delivery, nonServerIds, newIdx);
             }
+            // else
+            if (clientIds.Count != 1 || clientIds[0] != ServerClientId)
+            {
+                throw new ArgumentException($"Clients may only send messages to {nameof(ServerClientId)}");
+            }
+
             return MessagingSystem.SendMessage(ref message, delivery, clientIds);
         }
 
@@ -1485,6 +1504,11 @@ internal unsafe int SendMessage<T>(ref T message, NetworkDelivery delivery,
                 }
                 return MessagingSystem.SendMessage(ref message, delivery, nonServerIds, newIdx);
             }
+            // else
+            if (numClientIds != 1 || clientIds[0] != ServerClientId)
+            {
+                throw new ArgumentException($"Clients may only send messages to {nameof(ServerClientId)}");
+            }
 
             return MessagingSystem.SendMessage(ref message, delivery, clientIds, numClientIds);
         }
@@ -1503,6 +1527,11 @@ internal int SendMessage<T>(ref T message, NetworkDelivery delivery, ulong clien
             {
                 return 0;
             }
+
+            if (!IsServer && clientId != ServerClientId)
+            {
+                throw new ArgumentException($"Clients may only send messages to {nameof(ServerClientId)}");
+            }
             return MessagingSystem.SendMessage(ref message, delivery, clientId);
         }
 

com.unity.netcode.gameobjects/Runtime/Messaging/MessagingSystem.cs

@@ -118,7 +118,7 @@ public unsafe void Dispose()
             for (var queueIndex = 0; queueIndex < m_IncomingMessageQueue.Length; ++queueIndex)
             {
                 // Avoid copies...
-                ref var item = ref m_IncomingMessageQueue.GetUnsafeList()->ElementAt(queueIndex);
+                ref var item = ref m_IncomingMessageQueue.ElementAt(queueIndex);
                 item.Reader.Dispose();
             }
 
@@ -279,7 +279,7 @@ internal unsafe void ProcessIncomingMessageQueue()
             for (var index = 0; index < m_IncomingMessageQueue.Length; ++index)
             {
                 // Avoid copies...
-                ref var item = ref m_IncomingMessageQueue.GetUnsafeList()->ElementAt(index);
+                ref var item = ref m_IncomingMessageQueue.ElementAt(index);
                 HandleMessage(item.Header, item.Reader, item.SenderId, item.Timestamp, item.MessageHeaderSerializedSize);
                 if (m_Disposed)
                 {
@@ -314,7 +314,7 @@ private unsafe void CleanupDisconnectedClient(ulong clientId)
             var queue = m_SendQueues[clientId];
             for (var i = 0; i < queue.Length; ++i)
             {
-                queue.GetUnsafeList()->ElementAt(i).Writer.Dispose();
+                queue.ElementAt(i).Writer.Dispose();
             }
 
             queue.Dispose();
@@ -402,22 +402,22 @@ internal unsafe int SendPreSerializedMessage<TMessageType>(in FastBufferWriter t
                 {
                     sendQueueItem.Add(new SendQueueItem(delivery, NON_FRAGMENTED_MESSAGE_MAX_SIZE, Allocator.TempJob,
                         maxSize));
-                    sendQueueItem.GetUnsafeList()->ElementAt(0).Writer.Seek(sizeof(BatchHeader));
+                    sendQueueItem.ElementAt(0).Writer.Seek(sizeof(BatchHeader));
                 }
                 else
                 {
-                    ref var lastQueueItem = ref sendQueueItem.GetUnsafeList()->ElementAt(sendQueueItem.Length - 1);
+                    ref var lastQueueItem = ref sendQueueItem.ElementAt(sendQueueItem.Length - 1);
                     if (lastQueueItem.NetworkDelivery != delivery ||
                         lastQueueItem.Writer.MaxCapacity - lastQueueItem.Writer.Position
                         < tmpSerializer.Length + headerSerializer.Length)
                     {
                         sendQueueItem.Add(new SendQueueItem(delivery, NON_FRAGMENTED_MESSAGE_MAX_SIZE, Allocator.TempJob,
                             maxSize));
-                        sendQueueItem.GetUnsafeList()->ElementAt(sendQueueItem.Length - 1).Writer.Seek(sizeof(BatchHeader));
+                        sendQueueItem.ElementAt(sendQueueItem.Length - 1).Writer.Seek(sizeof(BatchHeader));
                     }
                 }
 
-                ref var writeQueueItem = ref sendQueueItem.GetUnsafeList()->ElementAt(sendQueueItem.Length - 1);
+                ref var writeQueueItem = ref sendQueueItem.ElementAt(sendQueueItem.Length - 1);
                 writeQueueItem.Writer.TryBeginWrite(tmpSerializer.Length + headerSerializer.Length);
 
                 writeQueueItem.Writer.WriteBytes(headerSerializer.GetUnsafePtr(), headerSerializer.Length);
@@ -502,7 +502,7 @@ internal unsafe void ProcessSendQueues()
                 var sendQueueItem = kvp.Value;
                 for (var i = 0; i < sendQueueItem.Length; ++i)
                 {
-                    ref var queueItem = ref sendQueueItem.GetUnsafeList()->ElementAt(i);
+                    ref var queueItem = ref sendQueueItem.ElementAt(i);
                     if (queueItem.BatchHeader.BatchSize == 0)
                     {
                         queueItem.Writer.Dispose();