Skip to content
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

chore: update MessageVersionData for service compatibility #3037

Draft
wants to merge 2 commits into
base: develop-2.0.0
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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 @@ -544,10 +544,9 @@ internal void TransportFailureEventHandler(bool duringStart = false)
}

/// <summary>
/// Client-Side:
/// Upon transport connecting, the client will send a connection request
/// Generates the connection request message
/// </summary>
private void SendConnectionRequest()
internal ConnectionRequestMessage GenerateConnectionRequestMessage()
{
var message = new ConnectionRequestMessage
{
Expand All @@ -567,14 +566,29 @@ private void SendConnectionRequest()
if (MessageManager.MessageTypes[index] != null)
{
var type = MessageManager.MessageTypes[index];
message.MessageVersions[index] = new MessageVersionData
var messageVersionData = new MessageVersionData
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole group of logic feels like it maybe would make sense on the data structure?

Something like two constructors, one that is for DistributedAuthorityMode == false and takes the hash and the version, and one that is for DistributedAuthorityMode == true that takes the type and the version, and then calculates the hash internally. That way if the type is important, the hash can never be wrong. That would also mean the logic of do we or do we not send the type is better encapsulated and consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... let me fiddle with that idea...seems a bit cleaner indeed.

{
Hash = XXHash.Hash32(type.FullName),
Version = MessageManager.GetLocalVersion(type)
Version = MessageManager.GetLocalVersion(type),
SendMessageType = NetworkManager.DistributedAuthorityMode,
};
if (messageVersionData.SendMessageType)
{
messageVersionData.NetworkMessageType = (uint)ILPPMessageProvider.TypeToNetworkMessageType[type];
}
message.MessageVersions[index] = messageVersionData;
}
}
return message;
}

/// <summary>
/// Client-Side:
/// Upon transport connecting, the client will send a connection request
/// </summary>
private void SendConnectionRequest()
{
var message = GenerateConnectionRequestMessage();
SendMessage(ref message, NetworkDelivery.ReliableSequenced, NetworkManager.ServerClientId);
message.MessageVersions.Dispose();
}
Expand Down Expand Up @@ -807,11 +821,17 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
if (MessageManager.MessageTypes[index] != null)
{
var type = MessageManager.MessageTypes[index];
message.MessageVersions[index] = new MessageVersionData
var messageVersionData = new MessageVersionData
{
Hash = XXHash.Hash32(type.FullName),
Version = MessageManager.GetLocalVersion(type)
Version = MessageManager.GetLocalVersion(type),
SendMessageType = NetworkManager.DistributedAuthorityMode,
};
if (messageVersionData.SendMessageType)
{
messageVersionData.NetworkMessageType = (uint)ILPPMessageProvider.TypeToNetworkMessageType[type];
}
message.MessageVersions[index] = messageVersionData;
}
}
if (!MockSkippingApproval)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ internal enum NetworkMessageTypes : uint

// Enable this for integration tests that need no message types defined
internal static bool IntegrationTestNoMessages;
internal static Dictionary<Type, NetworkMessageTypes> TypeToNetworkMessageType;

public List<NetworkMessageManager.MessageWithHandler> GetMessages()
{
Expand Down Expand Up @@ -81,7 +82,7 @@ internal enum NetworkMessageTypes : uint
// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
// Add new Message types to this table paired with its new NetworkMessageTypes enum
// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
var messageTypes = new Dictionary<Type, NetworkMessageTypes>
TypeToNetworkMessageType = new Dictionary<Type, NetworkMessageTypes>
{
{ typeof(ConnectionApprovedMessage), NetworkMessageTypes.ConnectionApproved }, // This MUST be first
{ typeof(ConnectionRequestMessage), NetworkMessageTypes.ConnectionRequest }, // This MUST be second
Expand Down Expand Up @@ -111,19 +112,19 @@ internal enum NetworkMessageTypes : uint
};

// 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)
if (TypeToNetworkMessageType.Count != messageTypeCount)
{
throw new Exception($"Message type to Message type index count mistmatch! Table Count: {messageTypes.Count} | Index Count: {messageTypeCount}");
throw new Exception($"Message type to Message type index count mistmatch! Table Count: {TypeToNetworkMessageType.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))
if (!TypeToNetworkMessageType.ContainsKey(messageHandler.MessageType))
{
throw new Exception($"Missing message type from lookup table: {messageHandler.MessageType}");
}
adjustedMessageTypes[(int)messageTypes[messageHandler.MessageType]] = messageHandler;
adjustedMessageTypes[(int)TypeToNetworkMessageType[messageHandler.MessageType]] = messageHandler;
}

// return the NetworkMessageType enum ordered list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
var messageHashesInOrder = new NativeArray<uint>(length, Allocator.Temp);
for (var i = 0; i < length; ++i)
{
var messageVersion = new MessageVersionData();
var messageVersion = new MessageVersionData()
{
SendMessageType = networkManager.DistributedAuthorityMode,
};
messageVersion.Deserialize(reader);
networkManager.ConnectionManager.MessageManager.SetVersion(context.SenderId, messageVersion.Hash, messageVersion.Version);
messageHashesInOrder[i] = messageVersion.Hash;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public void Serialize(FastBufferWriter writer, int targetVersion)
{
messageVersion.Serialize(writer);
}

// ============================================================
// END FORBIDDEN SEGMENT
// ============================================================
Expand Down Expand Up @@ -65,9 +66,13 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
// must go AFTER the message version header.
// ============================================================
ByteUnpacker.ReadValueBitPacked(reader, out int length);

for (var i = 0; i < length; ++i)
{
var messageVersion = new MessageVersionData();
var messageVersion = new MessageVersionData()
{
SendMessageType = networkManager.DistributedAuthorityMode,
};
messageVersion.Deserialize(reader);
networkManager.ConnectionManager.MessageManager.SetVersion(context.SenderId, messageVersion.Hash, messageVersion.Version);

Expand All @@ -79,6 +84,7 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
receivedMessageVersion = messageVersion.Version;
}
}

// ============================================================
// END FORBIDDEN SEGMENT
// ============================================================
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,27 @@ internal struct MessageVersionData
{
public uint Hash;
public int Version;
public uint NetworkMessageType;

internal bool SendMessageType;
public void Serialize(FastBufferWriter writer)
{
writer.WriteValueSafe(Hash);
BytePacker.WriteValueBitPacked(writer, Version);
if (SendMessageType)
{
BytePacker.WriteValueBitPacked(writer, NetworkMessageType);
}
}

public void Deserialize(FastBufferReader reader)
{
reader.ReadValueSafe(out Hash);
ByteUnpacker.ReadValueBitPacked(reader, out Version);
if (SendMessageType)
{
ByteUnpacker.ReadValueBitPacked(reader, out NetworkMessageType);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ public SendQueueItem(NetworkDelivery delivery, int writerSize, Allocator writerA
private NativeList<ReceiveQueueItem> m_IncomingMessageQueue = new NativeList<ReceiveQueueItem>(16, Allocator.Persistent);

// These array will grow as we need more message handlers. 4 is just a starting size.
private MessageHandler[] m_MessageHandlers = new MessageHandler[4];
private Type[] m_ReverseTypeMap = new Type[4];
private MessageHandler[] m_MessageHandlers = new MessageHandler[Enum.GetValues(typeof(ILPPMessageProvider.NetworkMessageTypes)).Length];
private Type[] m_ReverseTypeMap = new Type[Enum.GetValues(typeof(ILPPMessageProvider.NetworkMessageTypes)).Length];

private Dictionary<Type, uint> m_MessageTypes = new Dictionary<Type, uint>();
private Dictionary<ulong, NativeList<SendQueueItem>> m_SendQueues = new Dictionary<ulong, NativeList<SendQueueItem>>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
using System.Collections;
using NUnit.Framework;
using Unity.Netcode.TestHelpers.Runtime;
using UnityEngine.TestTools;

namespace Unity.Netcode.RuntimeTests
{
[TestFixture(HostOrServer.DAHost)]
[TestFixture(HostOrServer.Host)]
[TestFixture(HostOrServer.Server)]
internal class MessageVersionDataTests : NetcodeIntegrationTest
{
protected override int NumberOfClients => 2;

private NetworkManager m_SessionOwner;

public MessageVersionDataTests(HostOrServer hostOrServer) : base(hostOrServer) { }

private void ValidateConnectionRequest(NetworkManager networkManager)
{
var message = networkManager.ConnectionManager.GenerateConnectionRequestMessage();

foreach (var messageVersionData in message.MessageVersions)
{
Assert.True(messageVersionData.SendMessageType == m_DistributedAuthority, $"Include {nameof(MessageVersionData.SendMessageType)} is {messageVersionData.SendMessageType} and distributed authority is {m_DistributedAuthority}!");
if (m_DistributedAuthority)
{
var type = networkManager.ConnectionManager.MessageManager.GetMessageForHash(messageVersionData.Hash);
var networkMessageType = ILPPMessageProvider.TypeToNetworkMessageType[type];
Assert.True(messageVersionData.NetworkMessageType == (uint)networkMessageType, $"{nameof(MessageVersionData.NetworkMessageType)} is {messageVersionData.NetworkMessageType} but the hash type derived value is {networkMessageType}!");
}
}
}

[UnityTest]
public IEnumerator MessageVersionDataTest()
{
m_SessionOwner = UseCMBService() ? m_ClientNetworkManagers[0] : m_ServerNetworkManager;

ValidateConnectionRequest(m_SessionOwner);

foreach (var client in m_ClientNetworkManagers)
{
ValidateConnectionRequest(client);
}

yield return null;
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading