Skip to content

fix: handle null or empty named message registration [MTT-7771] #2807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2763,7 +2763,7 @@ public override void OnNetworkSpawn()
public override void OnNetworkDespawn()
{
// During destroy, use NetworkBehaviour.NetworkManager as opposed to m_CachedNetworkManager
if (!NetworkManager.ShutdownInProgress && NetworkManager.CustomMessagingManager != null)
if (!NetworkManager.ShutdownInProgress && NetworkManager.CustomMessagingManager != null && !string.IsNullOrEmpty(m_MessageName))
{
NetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(m_MessageName);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using Unity.Collections;
using UnityEngine;

namespace Unity.Netcode
{
Expand Down Expand Up @@ -199,6 +200,14 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read
/// <param name="callback">The callback to run when a named message is received.</param>
public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate callback)
{
if (string.IsNullOrEmpty(name))
{
if (m_NetworkManager.LogLevel <= LogLevel.Error)
{
Debug.LogError($"[{nameof(CustomMessagingManager.RegisterNamedMessageHandler)}] Cannot register a named message of type null or empty!");
}
return;
}
var hash32 = XXHash.Hash32(name);
var hash64 = XXHash.Hash64(name);

Expand All @@ -215,6 +224,15 @@ public void RegisterNamedMessageHandler(string name, HandleNamedMessageDelegate
/// <param name="name">The name of the message.</param>
public void UnregisterNamedMessageHandler(string name)
{
if (string.IsNullOrEmpty(name))
{
if (m_NetworkManager.LogLevel <= LogLevel.Error)
{
Debug.LogError($"[{nameof(CustomMessagingManager.UnregisterNamedMessageHandler)}] Cannot unregister a named message of type null or empty!");
}
return;
}

var hash32 = XXHash.Hash32(name);
var hash64 = XXHash.Hash64(name);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,24 @@ public void NamedMessageIsReceivedOnHostWithContent()
Assert.AreEqual(m_ServerNetworkManager.LocalClientId, receivedMessageSender);
}

private void MockNamedMessageCallback(ulong sender, FastBufferReader reader)
{

}

[Test]
public void NullOrEmptyNamedMessageDoesNotThrowException()
{
LogAssert.Expect(UnityEngine.LogType.Error, $"[{nameof(CustomMessagingManager.RegisterNamedMessageHandler)}] Cannot register a named message of type null or empty!");
m_ServerNetworkManager.CustomMessagingManager.RegisterNamedMessageHandler(string.Empty, MockNamedMessageCallback);
LogAssert.Expect(UnityEngine.LogType.Error, $"[{nameof(CustomMessagingManager.RegisterNamedMessageHandler)}] Cannot register a named message of type null or empty!");
m_ServerNetworkManager.CustomMessagingManager.RegisterNamedMessageHandler(null, MockNamedMessageCallback);
LogAssert.Expect(UnityEngine.LogType.Error, $"[{nameof(CustomMessagingManager.UnregisterNamedMessageHandler)}] Cannot unregister a named message of type null or empty!");
m_ServerNetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(string.Empty);
LogAssert.Expect(UnityEngine.LogType.Error, $"[{nameof(CustomMessagingManager.UnregisterNamedMessageHandler)}] Cannot unregister a named message of type null or empty!");
m_ServerNetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(null);
}

[UnityTest]
public IEnumerator NamedMessageIsReceivedOnMultipleClientsWithContent()
{
Expand Down