feat: Message Ordering #948
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,19 @@ internal enum __RpcExecStage | |
| Client = 2 | ||
| } | ||
|
|
||
| private static void SetUpdateStage<T>(ref T param) where T: IHasUpdateStage | ||
ShadauxCat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| if (param.UpdateStage == NetworkUpdateStage.Unset) | ||
| { | ||
| param.UpdateStage = NetworkUpdateLoop.UpdateStage; | ||
|
|
||
| if (param.UpdateStage == NetworkUpdateStage.Initialization) | ||
| { | ||
| param.UpdateStage = NetworkUpdateStage.EarlyUpdate; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #pragma warning disable 414 // disable assigned but its value is never used | ||
| #pragma warning disable IDE1006 // disable naming rule violation check | ||
| [NonSerialized] | ||
|
|
@@ -47,34 +60,35 @@ internal NetworkSerializer __beginSendServerRpc(uint rpcMethodId, ServerRpcParam | |
| { | ||
| PooledNetworkWriter writer; | ||
|
|
||
| SetUpdateStage(ref serverRpcParams.Send); | ||
|
|
||
| if (serverRpcParams.Send.UpdateStage == NetworkUpdateStage.Initialization) | ||
| { | ||
| throw new NotSupportedException( | ||
| $"{nameof(NetworkUpdateStage.Initialization)} cannot be used as a target for processing RPCs."); | ||
mattwalsh-unity marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
ShadauxCat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| var messageQueueContainer = NetworkManager.MessageQueueContainer; | ||
| var transportChannel = rpcDelivery == RpcDelivery.Reliable ? NetworkChannel.ReliableRpc : NetworkChannel.UnreliableRpc; | ||
|
|
||
| if (IsHost) | ||
| { | ||
ShadauxCat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| writer = messageQueueContainer.BeginAddQueueItemToFrame(MessageQueueContainer.MessageType.ServerRpc, Time.realtimeSinceStartup, transportChannel, | ||
| NetworkManager.ServerClientId, null, MessageQueueHistoryFrame.QueueFrameType.Inbound, serverRpcParams.Send.UpdateStage); | ||
| } | ||
| else | ||
| { | ||
| writer = messageQueueContainer.BeginAddQueueItemToFrame(MessageQueueContainer.MessageType.ServerRpc, Time.realtimeSinceStartup, transportChannel, | ||
| NetworkManager.ServerClientId, null, MessageQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate); | ||
|
|
||
| writer.WriteByte((byte)MessageQueueContainer.MessageType.ServerRpc); | ||
| writer.WriteByte((byte)serverRpcParams.Send.UpdateStage); // NetworkUpdateStage | ||
| } | ||
|
|
||
| writer.WriteUInt64Packed(NetworkObjectId); // NetworkObjectId | ||
| writer.WriteUInt16Packed(NetworkBehaviourId); // NetworkBehaviourId | ||
| writer.WriteUInt32Packed(rpcMethodId); // NetworkRpcMethodId | ||
|
|
||
|
There was a problem hiding this comment. Ok, so in the old code we wrote out the There was a problem hiding this comment. I think that is the "host loopback" and so it doesn't need to write it out because it is being directly inserted into the next frame's inbound local MessageQueue. There was a problem hiding this comment. UpdateStage is part of the unified header now. It's no longer part of the RPC message. The loopback code doesn't read the header for some reason... the fact that the behavior when pushing to the inbound queue is different than when pushing to the outbound is one of the things I'm addressing in my serialization RFC. There was a problem hiding this comment. To elaborate a bit further... The message header (including UpdateStage) is only used on the receiving end during HandleIncomingData() in order to properly add the item to the correct queue. When sending to loopback, the header isn't written because HandleIncomingData() is bypassed, so we skip it when the only destination is the inbound queue because skipping it here emulates the behavior of HandleIncomingData in reading it (and thus pushing the read head past it). There was a problem hiding this comment. Ok, brilliant @ShadauxCat , can we hoist this last comment into a comment in the code? This is something subtle and worth placing here for future devs / users. |
||
|
|
||
| return writer.Serializer; | ||
| } | ||
|
|
@@ -89,14 +103,16 @@ internal void __endSendServerRpc(NetworkSerializer serializer, uint rpcMethodId, | |
| return; | ||
| } | ||
|
|
||
| SetUpdateStage(ref serverRpcParams.Send); | ||
|
|
||
| var messageQueueContainer = NetworkManager.MessageQueueContainer; | ||
| if (IsHost) | ||
| { | ||
| messageQueueContainer.EndAddQueueItemToFrame(serializer.Writer, MessageQueueHistoryFrame.QueueFrameType.Inbound, serverRpcParams.Send.UpdateStage); | ||
ShadauxCat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| else | ||
| { | ||
| messageQueueContainer.EndAddQueueItemToFrame(serializer.Writer, MessageQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -107,66 +123,66 @@ internal NetworkSerializer __beginSendClientRpc(uint rpcMethodId, ClientRpcParam | |
| { | ||
| PooledNetworkWriter writer; | ||
|
|
||
| SetUpdateStage(ref clientRpcParams.Send); | ||
|
|
||
| if (clientRpcParams.Send.UpdateStage == NetworkUpdateStage.Initialization) | ||
| { | ||
| throw new NotSupportedException( | ||
| $"{nameof(NetworkUpdateStage.Initialization)} cannot be used as a target for processing RPCs."); | ||
| } | ||
ShadauxCat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // This will start a new queue item entry and will then return the writer to the current frame's stream | ||
| var transportChannel = rpcDelivery == RpcDelivery.Reliable ? NetworkChannel.ReliableRpc : NetworkChannel.UnreliableRpc; | ||
|
|
||
| ulong[] clientIds = clientRpcParams.Send.TargetClientIds ?? NetworkManager.ConnectedClientsIds; | ||
ShadauxCat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (clientRpcParams.Send.TargetClientIds != null && clientRpcParams.Send.TargetClientIds.Length == 0) | ||
| { | ||
| clientIds = NetworkManager.ConnectedClientsIds; | ||
| } | ||
|
|
||
| //NOTES ON BELOW CHANGES: | ||
| //The following checks for IsHost and whether the host client id is part of the clients to recieve the RPC | ||
| //Is part of a patch-fix to handle looping back RPCs into the next frame's inbound queue. | ||
| //!!! This code is temporary and will change (soon) when NetworkSerializer can be configured for mutliple NetworkWriters!!! | ||
| var containsServerClientId = clientIds.Contains(NetworkManager.ServerClientId); | ||
| bool addHeader = true; | ||
| var messageQueueContainer = NetworkManager.MessageQueueContainer; | ||
| if (IsHost && containsServerClientId) | ||
| { | ||
| //Always write to the next frame's inbound queue | ||
| writer = messageQueueContainer.BeginAddQueueItemToFrame(MessageQueueContainer.MessageType.ClientRpc, Time.realtimeSinceStartup, transportChannel, | ||
| NetworkManager.ServerClientId, null, MessageQueueHistoryFrame.QueueFrameType.Inbound, clientRpcParams.Send.UpdateStage); | ||
|
|
||
| //Handle sending to the other clients, if so the above notes explain why this code is here (a temporary patch-fix) | ||
| if (clientIds.Length > 1) | ||
| { | ||
| //Set the loopback frame | ||
| messageQueueContainer.SetLoopBackFrameItem(clientRpcParams.Send.UpdateStage); | ||
|
|
||
| //Switch to the outbound queue | ||
| writer = messageQueueContainer.BeginAddQueueItemToFrame(MessageQueueContainer.MessageType.ClientRpc, Time.realtimeSinceStartup, transportChannel, NetworkObjectId, | ||
| clientIds, MessageQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate); | ||
| } | ||
| else | ||
| { | ||
| addHeader = false; | ||
ShadauxCat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| else | ||
| { | ||
| writer = messageQueueContainer.BeginAddQueueItemToFrame(MessageQueueContainer.MessageType.ClientRpc, Time.realtimeSinceStartup, transportChannel, NetworkObjectId, | ||
| clientIds, MessageQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate); | ||
| } | ||
|
|
||
| if (addHeader) | ||
|
There was a problem hiding this comment. Ultra nit, I would probably rename this / invert to 'skipHeader' and (even though it's pretty obvious when you look at the code) have a comment: "don't send a header for 1 client" There was a problem hiding this comment. I generally dislike using negative terms for variables and then inverting them. I find "if(do thing)" to be more intuitive and less prone to being misread than "if(not don't do thing)". But I won't die on that hill if you disagree. There was a problem hiding this comment. Ok, I can see that...but let's add the |
||
| { | ||
| writer.WriteByte((byte)MessageQueueContainer.MessageType.ClientRpc); | ||
| writer.WriteByte((byte)clientRpcParams.Send.UpdateStage); // NetworkUpdateStage | ||
| } | ||
| writer.WriteUInt64Packed(NetworkObjectId); // NetworkObjectId | ||
| writer.WriteUInt16Packed(NetworkBehaviourId); // NetworkBehaviourId | ||
| writer.WriteUInt32Packed(rpcMethodId); // NetworkRpcMethodId | ||
|
|
||
|
|
||
| return writer.Serializer; | ||
| } | ||
|
|
@@ -181,25 +197,27 @@ internal void __endSendClientRpc(NetworkSerializer serializer, uint rpcMethodId, | |
| return; | ||
| } | ||
|
|
||
| SetUpdateStage(ref clientRpcParams.Send); | ||
|
|
||
| var messageQueueContainer = NetworkManager.MessageQueueContainer; | ||
|
|
||
| if (IsHost) | ||
| { | ||
| ulong[] clientIds = clientRpcParams.Send.TargetClientIds ?? NetworkManager.ConnectedClientsIds; | ||
| if (clientRpcParams.Send.TargetClientIds != null && clientRpcParams.Send.TargetClientIds.Length == 0) | ||
| { | ||
| clientIds = NetworkManager.ConnectedClientsIds; | ||
| } | ||
|
|
||
| var containsServerClientId = clientIds.Contains(NetworkManager.ServerClientId); | ||
| if (containsServerClientId && clientIds.Length == 1) | ||
| { | ||
| messageQueueContainer.EndAddQueueItemToFrame(serializer.Writer, MessageQueueHistoryFrame.QueueFrameType.Inbound, clientRpcParams.Send.UpdateStage); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| messageQueueContainer.EndAddQueueItemToFrame(serializer.Writer, MessageQueueHistoryFrame.QueueFrameType.Outbound, NetworkUpdateStage.PostLateUpdate); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -564,8 +582,16 @@ private void NetworkVariableUpdate(ulong clientId, int behaviourIndex) | |
|
|
||
| if (writtenAny) | ||
| { | ||
| var context = NetworkManager.MessageQueueContainer.EnterInternalCommandContext( | ||
mattwalsh-unity marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| MessageQueueContainer.MessageType.NetworkVariableDelta, m_ChannelsForNetworkVariableGroups[j], | ||
| new[] {clientId}, NetworkUpdateLoop.UpdateStage); | ||
| if (context != null) | ||
| { | ||
| using (var nonNullContext = (InternalCommandContext)context) | ||
| { | ||
| nonNullContext.NetworkWriter.WriteBytes(buffer.GetBuffer(), buffer.Length); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.