feat: enum ordered message versioning

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -22,6 +22,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Changed
 
+- Changed messages are now sorted by enum values as opposed to ordinally sorting the messages by their type name. (#2929)
+
 
 ## [2.0.0-exp.2] - 2024-04-02
 

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

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 #if UNITY_EDITOR
 using UnityEditor;
@@ -12,9 +13,117 @@ internal struct ILPPMessageProvider : INetworkMessageProvider
         internal static readonly List<NetworkMessageManager.MessageWithHandler> __network_message_types = new List<NetworkMessageManager.MessageWithHandler>();
 #pragma warning restore IDE1006 // restore naming rule violation check
 
+        /// <summary>
+        /// Enum representing the different types of messages that can be sent over the network.
+        /// The values cannot be changed, as they are used to serialize and deserialize messages.
+        /// Adding new messages should be done by adding new values to the end of the enum 
+        /// using the next free value.
+        /// </summary>
+        /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+        /// Add any new Message types to this table at the END with incremented index value
+        /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+        internal enum NetworkMessageTypes : uint
+        {
+            ConnectionApproved = 0,
+            ConnectionRequest = 1,
+            ChangeOwnership = 2,
+            ClientConnected = 3,
+            ClientDisconnected = 4,
+            ClientRpc = 5,
+            CreateObject = 6,
+            DestroyObject = 7,
+            DisconnectReason = 8,
+            ForwardClientRpc = 9,
+            ForwardServerRpc = 10,
+            NamedMessage = 11,
+            NetworkTransformMessage = 12,
+            NetworkVariableDelta = 13,
+            ParentSync = 14,
+            Proxy = 15,
+            Rpc = 16,
+            SceneEvent = 17,
+            ServerLog = 18,
+            ServerRpc = 19,
+            TimeSync = 20,
+            Unnamed = 21,
+            SessionOwner = 22
+        }
+
+
+        // Enable this for integration tests that need no message types defined
+        internal static bool IntegrationTestNoMessages;
+
         public List<NetworkMessageManager.MessageWithHandler> GetMessages()
         {
-            return __network_message_types;
+            // return no message types when defined for integration tests
+            if (IntegrationTestNoMessages)
+            {
+                return new List<NetworkMessageManager.MessageWithHandler>();
+            }
+            var messageTypeCount = Enum.GetValues(typeof(NetworkMessageTypes)).Length;
+            // Assure the allowed types count is the same as our NetworkMessageType enum count
+            if (__network_message_types.Count != messageTypeCount)
+            {
+                throw new Exception($"Allowed types is not equal to the number of message type indices! Allowed Count: {__network_message_types.Count} | Index Count: {messageTypeCount}");
+            }
+
+            // Populate with blanks to be replaced later
+            var adjustedMessageTypes = new List<NetworkMessageManager.MessageWithHandler>();
+            var blank = new NetworkMessageManager.MessageWithHandler();
+            for (int i = 0; i < messageTypeCount; i++)
+            {
+                adjustedMessageTypes.Add(blank);
+            }
+
+            // Create a type to enum index lookup table
+            // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+            // Add new Message types to this table paired with its new NetworkMessageTypes enum
+            // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+            var messageTypes = new Dictionary<Type, NetworkMessageTypes>
+            {
+                { typeof(ConnectionApprovedMessage), NetworkMessageTypes.ConnectionApproved }, // This MUST be first
+                { typeof(ConnectionRequestMessage), NetworkMessageTypes.ConnectionRequest }, // This MUST be second
+                { typeof(ChangeOwnershipMessage), NetworkMessageTypes.ChangeOwnership },
+                { typeof(ClientConnectedMessage), NetworkMessageTypes.ClientConnected },
+                { typeof(ClientDisconnectedMessage), NetworkMessageTypes.ClientDisconnected },
+                { typeof(ClientRpcMessage), NetworkMessageTypes.ClientRpc },
+                { typeof(CreateObjectMessage), NetworkMessageTypes.CreateObject },
+                { typeof(DestroyObjectMessage), NetworkMessageTypes.DestroyObject },
+                { typeof(DisconnectReasonMessage), NetworkMessageTypes.DisconnectReason },
+                { typeof(ForwardClientRpcMessage), NetworkMessageTypes.ForwardClientRpc },
+                { typeof(ForwardServerRpcMessage), NetworkMessageTypes.ForwardServerRpc },
+                { typeof(NamedMessage), NetworkMessageTypes.NamedMessage },
+                { typeof(NetworkTransformMessage), NetworkMessageTypes.NetworkTransformMessage },
+                { typeof(NetworkVariableDeltaMessage), NetworkMessageTypes.NetworkVariableDelta },
+                { typeof(ParentSyncMessage), NetworkMessageTypes.ParentSync },
+                { typeof(ProxyMessage), NetworkMessageTypes.Proxy },
+                { typeof(RpcMessage), NetworkMessageTypes.Rpc },
+                { typeof(SceneEventMessage), NetworkMessageTypes.SceneEvent },
+                { typeof(ServerLogMessage), NetworkMessageTypes.ServerLog },
+                { typeof(ServerRpcMessage), NetworkMessageTypes.ServerRpc },
+                { typeof(TimeSyncMessage), NetworkMessageTypes.TimeSync },
+                { typeof(UnnamedMessage), NetworkMessageTypes.Unnamed },
+                { typeof(SessionOwnerMessage), NetworkMessageTypes.SessionOwner }
+            };
+
+            // Assure the type to lookup table count and NetworkMessageType enum count matches (i.e. to catch human error when adding new messages)
+            if (messageTypes.Count != messageTypeCount)
+            {
+                throw new Exception($"Message type to Message type index count mistmatch! Table Count: {messageTypes.Count} | Index Count: {messageTypeCount}");
+            }
+
+            // Now order the allowed types list based on the order of the NetworkMessageType enum
+            foreach (var messageHandler in __network_message_types)
+            {
+                if (!messageTypes.ContainsKey(messageHandler.MessageType))
+                {
+                    throw new Exception($"Missing message type from lookup table: {messageHandler.MessageType}");
+                }
+                adjustedMessageTypes[(int)messageTypes[messageHandler.MessageType]] = messageHandler;
+            }
+
+            // return the NetworkMessageType enum ordered list
+            return adjustedMessageTypes;
         }
 
 #if UNITY_EDITOR

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

@@ -120,49 +120,20 @@ internal struct MessageWithHandler
             public VersionGetter GetVersion;
         }
 
-        internal List<MessageWithHandler> PrioritizeMessageOrder(List<MessageWithHandler> allowedTypes)
-        {
-            var prioritizedTypes = new List<MessageWithHandler>();
-
-            // First pass puts the priority message in the first indices
-            // Those are the messages that must be delivered in order to allow re-ordering the others later
-            foreach (var t in allowedTypes)
-            {
-                if (t.MessageType.FullName == typeof(ConnectionRequestMessage).FullName ||
-                    t.MessageType.FullName == typeof(ConnectionApprovedMessage).FullName)
-                {
-                    prioritizedTypes.Add(t);
-                }
-            }
-
-            foreach (var t in allowedTypes)
-            {
-                if (t.MessageType.FullName != typeof(ConnectionRequestMessage).FullName &&
-                    t.MessageType.FullName != typeof(ConnectionApprovedMessage).FullName)
-                {
-                    prioritizedTypes.Add(t);
-                }
-            }
-
-            return prioritizedTypes;
-        }
-
         public NetworkMessageManager(INetworkMessageSender sender, object owner, INetworkMessageProvider provider = null)
         {
             try
             {
                 m_Sender = sender;
                 m_Owner = owner;
-
                 if (provider == null)
                 {
                     provider = new ILPPMessageProvider();
                 }
 
+                // Get the presorted message types returned by the provider
                 var allowedTypes = provider.GetMessages();
 
-                allowedTypes.Sort((a, b) => string.CompareOrdinal(a.MessageType.FullName, b.MessageType.FullName));
-                allowedTypes = PrioritizeMessageOrder(allowedTypes);
                 foreach (var type in allowedTypes)
                 {
                     RegisterMessageType(type);

com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageRegistrationTests.cs

@@ -191,77 +191,5 @@ public void WhenCreatingMessageSystem_BoundTypeMessageHandlersAreRegistered()
                 Assert.AreEqual(handlerFour, systemThree.MessageHandlers[systemThree.GetMessageType(typeof(TestMessageFour))]);
             }
         }
-
-        internal class AAAEarlyLexicographicNetworkMessage : INetworkMessage
-        {
-            public void Serialize(FastBufferWriter writer, int targetVersion)
-            {
-            }
-
-            public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int receivedMessageVersion)
-            {
-                return true;
-            }
-
-            public void Handle(ref NetworkContext context)
-            {
-            }
-
-            public int Version => 0;
-        }
-
-#pragma warning disable IDE1006
-        internal class zzzLateLexicographicNetworkMessage : AAAEarlyLexicographicNetworkMessage
-        {
-        }
-#pragma warning restore IDE1006
-
-        internal class OrderingMessageProvider : INetworkMessageProvider
-        {
-            public List<NetworkMessageManager.MessageWithHandler> GetMessages()
-            {
-                var listMessages = new List<NetworkMessageManager.MessageWithHandler>();
-
-                var messageWithHandler = new NetworkMessageManager.MessageWithHandler
-                {
-                    MessageType = typeof(zzzLateLexicographicNetworkMessage),
-                    GetVersion = NetworkMessageManager.CreateMessageAndGetVersion<zzzLateLexicographicNetworkMessage>
-                };
-                listMessages.Add(messageWithHandler);
-
-                messageWithHandler.MessageType = typeof(ConnectionRequestMessage);
-                messageWithHandler.GetVersion = NetworkMessageManager.CreateMessageAndGetVersion<ConnectionRequestMessage>;
-                listMessages.Add(messageWithHandler);
-
-                messageWithHandler.MessageType = typeof(ConnectionApprovedMessage);
-                messageWithHandler.GetVersion = NetworkMessageManager.CreateMessageAndGetVersion<ConnectionApprovedMessage>;
-                listMessages.Add(messageWithHandler);
-
-                messageWithHandler.MessageType = typeof(AAAEarlyLexicographicNetworkMessage);
-                messageWithHandler.GetVersion = NetworkMessageManager.CreateMessageAndGetVersion<AAAEarlyLexicographicNetworkMessage>;
-                listMessages.Add(messageWithHandler);
-
-                return listMessages;
-            }
-        }
-
-        [Test]
-        public void MessagesGetPrioritizedCorrectly()
-        {
-            var sender = new NopMessageSender();
-            var provider = new OrderingMessageProvider();
-            using var messageManager = new NetworkMessageManager(sender, null, provider);
-
-            // the 2 priority messages should appear first, in lexicographic order
-            Assert.AreEqual(messageManager.MessageTypes[0], typeof(ConnectionApprovedMessage));
-            Assert.AreEqual(messageManager.MessageTypes[1], typeof(ConnectionRequestMessage));
-
-            // the other should follow after
-            Assert.AreEqual(messageManager.MessageTypes[2], typeof(AAAEarlyLexicographicNetworkMessage));
-            Assert.AreEqual(messageManager.MessageTypes[3], typeof(zzzLateLexicographicNetworkMessage));
-
-            // there should not be any extras
-            Assert.AreEqual(messageManager.MessageHandlerCount, 4);
-        }
     }
 }

com.unity.netcode.gameobjects/Tests/Editor/NetworkManagerConfigurationTests.cs

@@ -11,6 +11,18 @@ namespace Unity.Netcode.EditorTests
 {
     public class NetworkManagerConfigurationTests
     {
+        [SetUp]
+        public void OnSetup()
+        {
+            ILPPMessageProvider.IntegrationTestNoMessages = true;
+        }
+
+        [TearDown]
+        public void OnTearDown()
+        {
+            ILPPMessageProvider.IntegrationTestNoMessages = false;
+        }
+
         /// <summary>
         /// Does a simple check to make sure the nested network manager will
         /// notify the user when in the editor.  This is just a unit test to

com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs

@@ -7,6 +7,18 @@ namespace Unity.Netcode.EditorTests
 {
     public class UnityTransportTests
     {
+        [SetUp]
+        public void OnSetup()
+        {
+            ILPPMessageProvider.IntegrationTestNoMessages = true;
+        }
+
+        [TearDown]
+        public void OnTearDown()
+        {
+            ILPPMessageProvider.IntegrationTestNoMessages = false;
+        }
+
         // Check that starting an IPv4 server succeeds.
         [Test]
         public void UnityTransport_BasicInitServer_IPv4()