Skip to content

refactor: channel internally represented with bytes vs. strings [MTT-… #444

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.

Sign up for GitHub

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
mattwalsh-unity merged 5 commits into from
Jan 15, 2021
Merged

refactor: channel internally represented with bytes vs. strings [MTT-… #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
acffa02
refactor: channel internally represented with bytes vs. strings [MTT-…
mattwalsh-unity Jan 14, 2021
460300b
proper channel default in custom message manager
mattwalsh-unity Jan 14, 2021
2b745c3
cleaner and friendlier comment
mattwalsh-unity Jan 14, 2021
3b6bfa7
develop merge conflict fix
mattwalsh-unity Jan 15, 2021
7475aca
added jira comment
mattwalsh-unity Jan 15, 2021
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
2 changes: 1 addition & 1 deletion com.unity.multiplayer.mlapi/Runtime/Core/MessageBatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class MessageBatcher
{
public class SendStream
{
public string channel;
public byte channel;
Copy link
Copy Markdown
Contributor

@jeffreyrainy jeffreyrainy Jan 14, 2021

Choose a reason for hiding this comment

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

"256 channels ought to be enough for everyone" ? 640K

I'm not arguing for more, I think 256 is enough. But just want to point out the limit and ask whether we stopped to consider the impact

Copy link
Copy Markdown
Contributor Author

@mattwalsh-unity mattwalsh-unity Jan 14, 2021

Choose a reason for hiding this comment

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

Yeah, I wondered about the same thing, and even thought about putting dup channel detection in the registration function just in case. I thought from our discussion we wanted to stick with 'byte'. Happy to go all the way to a QWORD too :)

Copy link
Copy Markdown
Member

@NoelStephensUnity NoelStephensUnity Jan 15, 2021

Choose a reason for hiding this comment

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

I think byte should be fine for the next few milestones, but it could be that when we get into the MMO side of things this value could have potential to be increased to a ushort or even a uint.

public PooledBitStream Stream = PooledBitStream.Get();
public PooledBitWriter Writer;
public bool Empty = true;
Expand Down
20 changes: 10 additions & 10 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkedBehaviour.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using MLAPI.Serialization;
using MLAPI.Serialization.Pooled;
using MLAPI.Spawning;
using MLAPI.Transports;
using BitStream = MLAPI.Serialization.BitStream;
using Unity.Profiling;

Expand All @@ -38,11 +39,6 @@ internal enum NExec
Client = 2
}

/// <summary>
/// This is a temporary solution for channel names and the below hardcoded value might not be mandatory in the future.
/// </summary>
private const string StandardRpc_ChannelName = "STDRPC";
Comment on lines -41 to -44
Copy link
Copy Markdown
Contributor

@0xFA11 0xFA11 Jan 14, 2021

Choose a reason for hiding this comment

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

I like seeing this go :)


#pragma warning disable 414
// RuntimeAccessModifiersILPP will make this `protected`
internal NExec __nexec = NExec.None;
Expand All @@ -53,7 +49,7 @@ internal BitWriter BeginSendServerRpc(ServerRpcSendParams sendParams, bool isRel
{
var rpcQueueContainer = NetworkingManager.Singleton.rpcQueueContainer;

var writer = rpcQueueContainer.BeginAddQueueItemToOutboundFrame(RpcQueueContainer.QueueItemType.ServerRpc, Time.realtimeSinceStartup, StandardRpc_ChannelName, 0, NetworkingManager.Singleton.ServerClientId, null);
var writer = rpcQueueContainer.BeginAddQueueItemToOutboundFrame(RpcQueueContainer.QueueItemType.ServerRpc, Time.realtimeSinceStartup, Transport.MLAPI_STDRPC_CHANNEL, 0, NetworkingManager.Singleton.ServerClientId, null);

if(!rpcQueueContainer.IsUsingBatching())
{
Expand Down Expand Up @@ -93,7 +89,7 @@ internal BitWriter BeginSendClientRpc(ClientRpcSendParams sendParams, bool isRel
//This will start a new queue item entry and will then return the writer to the current frame's stream
var rpcQueueContainer = NetworkingManager.Singleton.rpcQueueContainer;

var writer = rpcQueueContainer.BeginAddQueueItemToOutboundFrame(RpcQueueContainer.QueueItemType.ClientRpc, Time.realtimeSinceStartup, StandardRpc_ChannelName, 0, NetworkId, sendParams.TargetClientIds ?? NetworkingManager.Singleton.ConnectedClientsList.Select(c => c.ClientId).ToArray());
var writer = rpcQueueContainer.BeginAddQueueItemToOutboundFrame(RpcQueueContainer.QueueItemType.ClientRpc, Time.realtimeSinceStartup, Transport.MLAPI_STDRPC_CHANNEL, 0, NetworkId, sendParams.TargetClientIds ?? NetworkingManager.Singleton.ConnectedClientsList.Select(c => c.ClientId).ToArray());

if(!rpcQueueContainer.IsUsingBatching())
{
Expand Down Expand Up @@ -309,7 +305,7 @@ protected NetworkedBehaviour GetBehaviour(ushort id)
private bool varInit = false;

private readonly List<HashSet<int>> channelMappedNetworkedVarIndexes = new List<HashSet<int>>();
private readonly List<string> channelsForNetworkedVarGroups = new List<string>();
private readonly List<byte> channelsForNetworkedVarGroups = new List<byte>();
internal readonly List<INetworkedVar> networkedVarFields = new List<INetworkedVar>();

private static HashSet<MLAPI.NetworkedObject> touched = new HashSet<MLAPI.NetworkedObject>();
Expand Down Expand Up @@ -374,12 +370,16 @@ internal void InitializeVars()

{
// Create index map for channels
Dictionary<string, int> firstLevelIndex = new Dictionary<string, int>();
Dictionary<byte, int> firstLevelIndex = new Dictionary<byte, int>();
int secondLevelCounter = 0;

for (int i = 0; i < networkedVarFields.Count; i++)
{
string channel = networkedVarFields[i].GetChannel(); // Cache this here. Some developers are stupid. You don't know what shit they will do in their methods
// this could be cleaner. The GetChannel() methods look for the SendChannel string channel name
// from the settings file, which could be easily misconfigured. If a bogus channel is specified,
// GetChannelByte() will return the default, MLAPI_INTERNAL_CHANNEL
string channelName = networkedVarFields[i].GetChannel();
byte channel = Transport.GetChannelByte(channelName);

if (!firstLevelIndex.ContainsKey(channel))
{
Expand Down
7 changes: 4 additions & 3 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkedObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using MLAPI.Security;
using MLAPI.Serialization.Pooled;
using MLAPI.Spawning;
using MLAPI.Transports;
using Unity.Profiling;
using UnityEngine;

Expand Down Expand Up @@ -277,7 +278,7 @@ public static void NetworkShow(List<NetworkedObject> networkedObjects, ulong cli
SpawnManager.WriteSpawnCallForObject(stream, clientId, networkedObjects[i], payload);
}

InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_ADD_OBJECTS, "MLAPI_INTERNAL", stream, SecuritySendFlags.None);
InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_ADD_OBJECTS, Transport.MLAPI_INTERNAL_CHANNEL, stream, SecuritySendFlags.None);
}
}

Expand Down Expand Up @@ -317,7 +318,7 @@ public void NetworkHide(ulong clientId)
{
writer.WriteUInt64Packed(NetworkId);

InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_DESTROY_OBJECT, "MLAPI_INTERNAL", stream, SecuritySendFlags.None);
InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_DESTROY_OBJECT, Transport.MLAPI_INTERNAL_CHANNEL, stream, SecuritySendFlags.None);
}
}
}
Expand Down Expand Up @@ -369,7 +370,7 @@ public static void NetworkHide(List<NetworkedObject> networkedObjects, ulong cli
}
}

InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_DESTROY_OBJECTS, "MLAPI_INTERNAL", stream, SecuritySendFlags.None);
InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_DESTROY_OBJECTS, Transport.MLAPI_INTERNAL_CHANNEL, stream, SecuritySendFlags.None);
}
}

Expand Down
43 changes: 23 additions & 20 deletions com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,15 +261,15 @@ internal void InvokeOnIncomingCustomMessage(ulong clientId, Stream stream)
/// <param name="channel">The channel to send the data on</param>
/// <param name="security">The security settings to apply to the message</param>
[Obsolete("Use CustomMessagingManager.SendUnnamedMessage instead")]
public void SendCustomMessage(List<ulong> clientIds, BitStream stream, string channel = null, SecuritySendFlags security = SecuritySendFlags.None)
public void SendCustomMessage(List<ulong> clientIds, BitStream stream, byte channel = Transport.MLAPI_INTERNAL_CHANNEL, SecuritySendFlags security = SecuritySendFlags.None)
{
if (!IsServer)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Error) NetworkLog.LogWarning("Can not send unnamed message to multiple users as a client");
return;
}

InternalMessageSender.Send(MLAPIConstants.MLAPI_UNNAMED_MESSAGE, string.IsNullOrEmpty(channel) ? "MLAPI_DEFAULT_MESSAGE" : channel, clientIds, stream, security);
InternalMessageSender.Send(MLAPIConstants.MLAPI_UNNAMED_MESSAGE, channel, clientIds, stream, security);
}

/// <summary>
Expand All @@ -280,9 +280,9 @@ public void SendCustomMessage(List<ulong> clientIds, BitStream stream, string ch
/// <param name="channel">The channel tos end the data on</param>
/// <param name="security">The security settings to apply to the message</param>
[Obsolete("Use CustomMessagingManager.SendUnnamedMessage instead")]
public void SendCustomMessage(ulong clientId, BitStream stream, string channel = null, SecuritySendFlags security = SecuritySendFlags.None)
public void SendCustomMessage(ulong clientId, BitStream stream, byte channel = Transport.MLAPI_INTERNAL_CHANNEL, SecuritySendFlags security = SecuritySendFlags.None)
{
InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_UNNAMED_MESSAGE, string.IsNullOrEmpty(channel) ? "MLAPI_DEFAULT_MESSAGE" : channel, stream, security);
InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_UNNAMED_MESSAGE, channel, stream, security);
}

private void OnValidate()
Expand Down Expand Up @@ -759,7 +759,7 @@ private void NetworkPreUpdate()
while (__loopbackRpcQueue.Count > 0)
{
var (payload, receiveTime) = __loopbackRpcQueue.Dequeue();
HandleRawTransportPoll(NetEventType.Data, ServerClientId, "STDRPC", payload, receiveTime);
HandleRawTransportPoll(NetEventType.Data, ServerClientId, Transport.MLAPI_STDRPC_CHANNEL, payload, receiveTime);
}
}
// @mfatihmar (Unity) End: Temporary, inbound RPC queue will replace this workaround
Expand All @@ -775,7 +775,10 @@ private void NetworkPreUpdate()
{
processedEvents++;
eventType = NetworkConfig.NetworkTransport.PollEvent(out ulong clientId, out string channelName, out ArraySegment<byte> payload, out float receiveTime);
HandleRawTransportPoll(eventType, clientId, channelName, payload, receiveTime);
// [MTT-443] This can be improved if the Transport implementations return the channel as a byte vs. string
// Holding off on this; refactoring the Transport package will be a separate step
byte channel = Transport.GetChannelByte(channelName);
HandleRawTransportPoll(eventType, clientId, channel, payload, receiveTime);

// Only do another iteration if: there are no more messages AND (there is no limit to max events or we have processed less than the maximum)
} while (IsListening && (eventType != NetEventType.Nothing) && (NetworkConfig.MaxReceiveEventsPerTickRate <= 0 || processedEvents < NetworkConfig.MaxReceiveEventsPerTickRate));
Expand Down Expand Up @@ -895,7 +898,7 @@ internal void SendConnectionRequest()
writer.WriteByteArray(NetworkConfig.ConnectionData);
}

InternalMessageSender.Send(ServerClientId, MLAPIConstants.MLAPI_CONNECTION_REQUEST, "MLAPI_INTERNAL", stream, SecuritySendFlags.Authenticated | SecuritySendFlags.Encrypted);
InternalMessageSender.Send(ServerClientId, MLAPIConstants.MLAPI_CONNECTION_REQUEST, Transport.MLAPI_INTERNAL_CHANNEL, stream, SecuritySendFlags.Authenticated | SecuritySendFlags.Encrypted);
}
}

Expand All @@ -922,7 +925,7 @@ internal IEnumerator TimeOutSwitchSceneProgress(SceneSwitchProgress switchSceneP
switchSceneProgress.SetTimedOut();
}

private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, string channelName, ArraySegment<byte> payload, float receiveTime)
private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, byte channel, ArraySegment<byte> payload, float receiveTime)
{
ProfilerStatManager.bytesRcvd.Record(payload.Count);
switch (eventType)
Expand All @@ -931,7 +934,7 @@ private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, stri
#if DEVELOPMENT_BUILD || UNITY_EDITOR
s_TransportConnect.Begin();
#endif
NetworkProfiler.StartEvent(TickType.Receive, (uint)payload.Count, channelName, "TRANSPORT_CONNECT");
NetworkProfiler.StartEvent(TickType.Receive, (uint)payload.Count, channel, "TRANSPORT_CONNECT");
if (IsServer)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) NetworkLog.LogInfo("Client Connected");
Expand Down Expand Up @@ -998,7 +1001,7 @@ private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, stri
}
}
// Send the hail
InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_CERTIFICATE_HAIL, "MLAPI_INTERNAL", hailStream, SecuritySendFlags.None, null);
InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_CERTIFICATE_HAIL, Transport.MLAPI_INTERNAL_CHANNEL, hailStream, SecuritySendFlags.None);
}
}
else
Expand Down Expand Up @@ -1030,14 +1033,14 @@ private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, stri
case NetEventType.Data:
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) NetworkLog.LogInfo($"Incoming Data From {clientId} : {payload.Count} bytes");
HandleIncomingData(clientId, channelName, payload, receiveTime, true);
HandleIncomingData(clientId, channel, payload, receiveTime, true);
break;
}
case NetEventType.Disconnect:
#if DEVELOPMENT_BUILD || UNITY_EDITOR
s_TransportDisconnect.Begin();
#endif
NetworkProfiler.StartEvent(TickType.Receive, 0, "NONE", "TRANSPORT_DISCONNECT");
NetworkProfiler.StartEvent(TickType.Receive, 0, Transport.MLAPI_INTERNAL_CHANNEL, "TRANSPORT_DISCONNECT");

if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) NetworkLog.LogInfo("Disconnect Event From " + clientId);

Expand All @@ -1062,7 +1065,7 @@ private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, stri
private readonly BitStream inputStreamWrapper = new BitStream(new byte[0]);
private MessageBatcher batcher = new MessageBatcher();

internal void HandleIncomingData(ulong clientId, string channelName, ArraySegment<byte> data, float receiveTime, bool allowBuffer)
internal void HandleIncomingData(ulong clientId, byte channel, ArraySegment<byte> data, float receiveTime, bool allowBuffer)
{
#if DEVELOPMENT_BUILD || UNITY_EDITOR
s_HandleIncomingData.Begin();
Expand All @@ -1087,7 +1090,7 @@ internal void HandleIncomingData(ulong clientId, string channelName, ArraySegmen
}

uint headerByteSize = (uint)Arithmetic.VarIntSize(messageType);
NetworkProfiler.StartEvent(TickType.Receive, (uint)(data.Count - headerByteSize), channelName, messageType);
NetworkProfiler.StartEvent(TickType.Receive, (uint)(data.Count - headerByteSize), channel, messageType);

if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) NetworkLog.LogInfo("Data Header: messageType=" + messageType);

Expand Down Expand Up @@ -1134,7 +1137,7 @@ internal void HandleIncomingData(ulong clientId, string channelName, ArraySegmen
InternalMessageHandler.HandleNetworkedVarDelta(clientId, messageStream, BufferCallback, new PreBufferPreset()
{
AllowBuffer = allowBuffer,
ChannelName = channelName,
Channel = channel,
ClientId = clientId,
Data = data,
MessageType = messageType,
Expand All @@ -1145,7 +1148,7 @@ internal void HandleIncomingData(ulong clientId, string channelName, ArraySegmen
InternalMessageHandler.HandleNetworkedVarUpdate(clientId, messageStream, BufferCallback, new PreBufferPreset()
{
AllowBuffer = allowBuffer,
ChannelName = channelName,
Channel = channel,
ClientId = clientId,
Data = data,
MessageType = messageType,
Expand Down Expand Up @@ -1308,7 +1311,7 @@ private void BufferCallback(ulong networkId, PreBufferPreset preset)
throw new InvalidOperationException("Cannot buffer on server.");
}

BufferManager.BufferMessageForNetworkId(networkId, preset.ClientId, preset.ChannelName, preset.ReceiveTime, preset.Data);
BufferManager.BufferMessageForNetworkId(networkId, preset.ClientId, preset.Channel, preset.ReceiveTime, preset.Data);
}

/// <summary>
Expand Down Expand Up @@ -1417,7 +1420,7 @@ private void SyncTime()
using (PooledBitWriter writer = PooledBitWriter.Get(stream))
{
writer.WriteSinglePacked(Time.realtimeSinceStartup);
InternalMessageSender.Send(MLAPIConstants.MLAPI_TIME_SYNC, "MLAPI_TIME_SYNC", stream, SecuritySendFlags.None);
InternalMessageSender.Send(MLAPIConstants.MLAPI_TIME_SYNC, Transport.MLAPI_TIME_SYNC_CHANNEL, stream, SecuritySendFlags.None);
}
}
#if DEVELOPMENT_BUILD || UNITY_EDITOR
Expand Down Expand Up @@ -1551,7 +1554,7 @@ internal void HandleApproval(ulong clientId, bool createPlayerObject, ulong? pla
}
}

InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_CONNECTION_APPROVED, "MLAPI_INTERNAL", stream, SecuritySendFlags.Encrypted | SecuritySendFlags.Authenticated);
InternalMessageSender.Send(clientId, MLAPIConstants.MLAPI_CONNECTION_APPROVED, Transport.MLAPI_INTERNAL_CHANNEL, stream, SecuritySendFlags.Encrypted | SecuritySendFlags.Authenticated);

if (OnClientConnectedCallback != null)
OnClientConnectedCallback.Invoke(clientId);
Expand Down Expand Up @@ -1613,7 +1616,7 @@ internal void HandleApproval(ulong clientId, bool createPlayerObject, ulong? pla
ConnectedClients[clientId].PlayerObject.WriteNetworkedVarData(stream, clientPair.Key);
}

InternalMessageSender.Send(clientPair.Key, MLAPIConstants.MLAPI_ADD_OBJECT, "MLAPI_INTERNAL", stream, SecuritySendFlags.None);
InternalMessageSender.Send(clientPair.Key, MLAPIConstants.MLAPI_ADD_OBJECT, Transport.MLAPI_INTERNAL_CHANNEL, stream, SecuritySendFlags.None);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion com.unity.multiplayer.mlapi/Runtime/Logging/NetworkLog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using MLAPI.Messaging;
using MLAPI.Security;
using MLAPI.Serialization.Pooled;
using MLAPI.Transports;
using UnityEngine;

namespace MLAPI.Logging
Expand Down Expand Up @@ -75,7 +76,7 @@ private static void LogServer(string message, LogType logType)

writer.WriteStringPacked(message);

InternalMessageSender.Send(NetworkingManager.Singleton.ServerClientId, MLAPIConstants.MLAPI_SERVER_LOG, "MLAPI_INTERNAL", stream, SecuritySendFlags.None);
InternalMessageSender.Send(NetworkingManager.Singleton.ServerClientId, MLAPIConstants.MLAPI_SERVER_LOG, Transport.MLAPI_INTERNAL_CHANNEL, stream, SecuritySendFlags.None);
}
}
}
Expand Down
Loading