From 2958644b29abf27baa3da6be578cfb39b8c682e6 Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 22 Apr 2025 15:47:21 -0400 Subject: [PATCH 1/4] fix: Custom messaging manager exception --- .../Runtime/Messaging/CustomMessageManager.cs | 16 ++++++++--- .../Runtime/Messaging/NamedMessageTests.cs | 28 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs index 815e38d35c..15e30205f8 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs @@ -180,14 +180,18 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read // We dont know what size to use. Try every (more collision prone) if (m_NamedMessageHandlers32.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler32)) { + // handler can remove itself, cache the name for metrics + var messageName = m_MessageHandlerNameLookup32[hash]; messageHandler32(sender, reader); - m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup32[hash], bytesCount); + m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, messageName, bytesCount); } if (m_NamedMessageHandlers64.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler64)) { + // handler can remove itself, cache the name for metrics + var messageName = m_MessageHandlerNameLookup64[hash]; messageHandler64(sender, reader); - m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup64[hash], bytesCount); + m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, messageName, bytesCount); } } else @@ -198,15 +202,19 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read case HashSize.VarIntFourBytes: if (m_NamedMessageHandlers32.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler32)) { + // handler can remove itself, cache the name for metrics + var messageName = m_MessageHandlerNameLookup32[hash]; messageHandler32(sender, reader); - m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup32[hash], bytesCount); + m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, messageName, bytesCount); } break; case HashSize.VarIntEightBytes: if (m_NamedMessageHandlers64.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler64)) { + // handler can remove itself, cache the name for metrics + var messageName = m_MessageHandlerNameLookup64[hash]; messageHandler64(sender, reader); - m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup64[hash], bytesCount); + m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, messageName, bytesCount); } break; } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs index 4c2d0d8cfd..ec1a0e2b64 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs @@ -279,5 +279,33 @@ public unsafe void ErrorMessageIsPrintedWhenAttemptingToSendNamedMessageWithTooB Assert.IsTrue(message.Contains($"Given message size ({msgSize} bytes) is greater than the maximum"), $"Unexpected exception: {message}"); } } + + [Test] + public void NamedMessageHandlerIsUnregisteredWithoutException() + { + var messageName = Guid.NewGuid().ToString(); + + var receivedMessageContent = new ForceNetworkSerializeByMemcpy(new Guid()); + m_ServerNetworkManager.CustomMessagingManager.RegisterNamedMessageHandler( + messageName, + (_, reader) => + { + reader.ReadValueSafe(out receivedMessageContent); + m_ServerNetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(messageName); + }); + + var messageContent = new ForceNetworkSerializeByMemcpy(Guid.NewGuid()); + var writer = new FastBufferWriter(1300, Allocator.Temp); + using (writer) + { + writer.WriteValueSafe(messageContent); + m_ServerNetworkManager.CustomMessagingManager.SendNamedMessage( + messageName, + m_ServerNetworkManager.LocalClientId, + writer); + } + + Assert.AreEqual(messageContent.Value, receivedMessageContent.Value); + } } } From a5bb3c67574a8b823d6882ef1c161bd1181b7954 Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 22 Apr 2025 15:57:06 -0400 Subject: [PATCH 2/4] update CHANGELOG --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 8cb9af6bd9..a1621b3150 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -21,6 +21,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed an exception being thrown when unregistering a custom message handler from within the registered callback. (#3417) - Fixed issue where the authority instance of NetworkTransform could check for state updates more than one time in a frame if the frame rate is greater than the tick frequency. (#3413) - Fixed issue where the new interpolator types were blocking after the first consumption of a sequence of buffered state updates. (#3413) - Fixed issue where root level in-scene placed `NetworkObject`s would only allow the ownership permission to be no less than distributable or sessionowner. (#3407) From 822e3a2770ddcfeaa8b7f45b891441f48c1e87f9 Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 22 Apr 2025 15:59:58 -0400 Subject: [PATCH 3/4] update the update of CHANGELOG --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index a1621b3150..85ad8ff0f6 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -13,6 +13,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed an exception being thrown when unregistering a custom message handler from within the registered callback. (#3417) ### Changed @@ -21,7 +22,6 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed -- Fixed an exception being thrown when unregistering a custom message handler from within the registered callback. (#3417) - Fixed issue where the authority instance of NetworkTransform could check for state updates more than one time in a frame if the frame rate is greater than the tick frequency. (#3413) - Fixed issue where the new interpolator types were blocking after the first consumption of a sequence of buffered state updates. (#3413) - Fixed issue where root level in-scene placed `NetworkObject`s would only allow the ownership permission to be no less than distributable or sessionowner. (#3407) From dd1a844b4bb347576664ddda561df6ebaf23da03 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 23 Apr 2025 14:26:29 -0400 Subject: [PATCH 4/4] Update test to be more comprehensive --- .../Runtime/Messaging/NamedMessageTests.cs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs index ec1a0e2b64..28ab33223e 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs @@ -284,13 +284,15 @@ public unsafe void ErrorMessageIsPrintedWhenAttemptingToSendNamedMessageWithTooB public void NamedMessageHandlerIsUnregisteredWithoutException() { var messageName = Guid.NewGuid().ToString(); + const int numMessagesToSend = 3; + const int expectedMessageHandlerCallCount = 1; - var receivedMessageContent = new ForceNetworkSerializeByMemcpy(new Guid()); + var messageHandlerCalled = 0; m_ServerNetworkManager.CustomMessagingManager.RegisterNamedMessageHandler( messageName, - (_, reader) => + (_, _) => { - reader.ReadValueSafe(out receivedMessageContent); + messageHandlerCalled++; m_ServerNetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(messageName); }); @@ -299,13 +301,16 @@ public void NamedMessageHandlerIsUnregisteredWithoutException() using (writer) { writer.WriteValueSafe(messageContent); - m_ServerNetworkManager.CustomMessagingManager.SendNamedMessage( - messageName, - m_ServerNetworkManager.LocalClientId, - writer); + for (var i = 0; i < numMessagesToSend; i++) + { + m_ServerNetworkManager.CustomMessagingManager.SendNamedMessage( + messageName, + m_ServerNetworkManager.LocalClientId, + writer); + } } - Assert.AreEqual(messageContent.Value, receivedMessageContent.Value); + Assert.AreEqual(expectedMessageHandlerCallCount, messageHandlerCalled); } } }