diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 305e08edc2..3829a2f098 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/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 diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/ILPPMessageProvider.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/ILPPMessageProvider.cs index 4424550e9a..609cadf3ec 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/ILPPMessageProvider.cs +++ b/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 __network_message_types = new List(); #pragma warning restore IDE1006 // restore naming rule violation check + /// + /// 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. + /// + /// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + /// 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 GetMessages() { - return __network_message_types; + // return no message types when defined for integration tests + if (IntegrationTestNoMessages) + { + return new List(); + } + 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(); + 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 + { + { 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 diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs index 7dd4dae2f0..b455c9bfa1 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/NetworkMessageManager.cs @@ -120,49 +120,20 @@ internal struct MessageWithHandler public VersionGetter GetVersion; } - internal List PrioritizeMessageOrder(List allowedTypes) - { - var prioritizedTypes = new List(); - - // 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); diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageRegistrationTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageRegistrationTests.cs index bb7ab651d7..2b601a16fd 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Messaging/MessageRegistrationTests.cs +++ b/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 GetMessages() - { - var listMessages = new List(); - - var messageWithHandler = new NetworkMessageManager.MessageWithHandler - { - MessageType = typeof(zzzLateLexicographicNetworkMessage), - GetVersion = NetworkMessageManager.CreateMessageAndGetVersion - }; - listMessages.Add(messageWithHandler); - - messageWithHandler.MessageType = typeof(ConnectionRequestMessage); - messageWithHandler.GetVersion = NetworkMessageManager.CreateMessageAndGetVersion; - listMessages.Add(messageWithHandler); - - messageWithHandler.MessageType = typeof(ConnectionApprovedMessage); - messageWithHandler.GetVersion = NetworkMessageManager.CreateMessageAndGetVersion; - listMessages.Add(messageWithHandler); - - messageWithHandler.MessageType = typeof(AAAEarlyLexicographicNetworkMessage); - messageWithHandler.GetVersion = NetworkMessageManager.CreateMessageAndGetVersion; - 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); - } } } diff --git a/com.unity.netcode.gameobjects/Tests/Editor/NetworkManagerConfigurationTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/NetworkManagerConfigurationTests.cs index 45ac91ce57..b57a13c974 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/NetworkManagerConfigurationTests.cs +++ b/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; + } + /// /// 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 diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs index fb61c053a1..f5caa6195e 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs +++ b/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()