-
Notifications
You must be signed in to change notification settings - Fork 459
feat!: Network Time (RFC #14) #845
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
Changes from all commits
fbef7c9
dbb4dc2
64296bd
c90994f
432171a
10c7842
1120814
e6cedfb
11769a7
e6b8f19
9a3867e
1b43bf7
d545e21
c6d2a3a
aa25482
73de76e
2ce9852
315b961
b6a3ffd
36e5e61
827f590
59ec8a3
f0605a3
251eef6
a61e788
5d5c288
7ba6537
011f34c
853ef3c
cd176a0
4c23586
2ff0d00
97ccf72
87964ba
151e224
b7c1bd7
94b3c8a
9439281
448e974
a40db6c
f1d71b3
85df5a1
0454b5a
9442538
8a76115
9cb765e
58cc9cd
e98c012
01f52fa
de9f4ff
5547e4b
dffbe7f
89f5471
ebb0821
0e21b77
b9667a8
c660e1e
61428a0
cdcd56d
d154779
563945a
f6ab35b
b067350
08d13f0
de4559d
d329675
5e60377
f674100
cdc3024
beb8f50
aab1a27
48c5c5e
865bb3d
7d5215f
9745475
cabdfd8
09222f6
1e55ca0
fbe1825
8391703
fbe7c91
dfa2222
8cc5208
d396024
426c23d
bb80275
7c5ed54
e525050
297aa39
bd82cff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,7 +48,7 @@ private void Awake() | |
| } | ||
|
|
||
| private Vector3 m_LastDestination = Vector3.zero; | ||
| private float m_LastCorrectionTime = 0f; | ||
| private double m_LastCorrectionTime = 0d; | ||
|
|
||
| private void Update() | ||
| { | ||
|
|
@@ -79,7 +79,7 @@ private void Update() | |
| } | ||
| } | ||
|
|
||
| if (NetworkManager.NetworkTime - m_LastCorrectionTime >= CorrectionDelay) | ||
| if (NetworkManager.LocalTime.Time - m_LastCorrectionTime >= CorrectionDelay) // TODO this is not aliased correctly, is this an issue? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC in general PredictedTime was the preferred time for these computations as before; trying to understand the shift to LocalTime here. Is it something special w.r.t. NavMesh?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed this overall to |
||
| { | ||
| if (!EnableProximity) | ||
| { | ||
|
|
@@ -99,7 +99,7 @@ private void Update() | |
| OnNavMeshCorrectionUpdateClientRpc(m_Agent.velocity, transform.position, new ClientRpcParams { Send = new ClientRpcSendParams { TargetClientIds = proximityClients.ToArray() } }); | ||
| } | ||
|
|
||
| m_LastCorrectionTime = NetworkManager.NetworkTime; | ||
| m_LastCorrectionTime = NetworkManager.LocalTime.Time; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,28 +61,10 @@ public class NetworkConfig | |
| internal Dictionary<uint, NetworkPrefab> NetworkPrefabOverrideLinks = new Dictionary<uint, NetworkPrefab>(); | ||
|
|
||
| /// <summary> | ||
| /// Amount of times per second the receive queue is emptied and all messages inside are processed. | ||
| /// The tickrate of network ticks. This value controls how often MLAPI runs user code and sends out data. | ||
| /// </summary> | ||
| [Tooltip("The amount of times per second the receive queue is emptied from pending incoming messages")] | ||
| public int ReceiveTickrate = 64; | ||
|
|
||
| /// <summary> | ||
| /// Duration in seconds between network ticks. | ||
| /// </summary> | ||
| [Tooltip("Duration in seconds between network ticks")] | ||
| public float NetworkTickIntervalSec = 0.050f; | ||
|
|
||
| /// <summary> | ||
| /// The max amount of messages to process per ReceiveTickrate. This is to prevent flooding. | ||
| /// </summary> | ||
| [Tooltip("The maximum amount of Receive events to poll per Receive tick. This is to prevent flooding and freezing on the server")] | ||
| public int MaxReceiveEventsPerTickRate = 500; | ||
|
|
||
| /// <summary> | ||
| /// The amount of times per second internal frame events will occur, e.g. send checking. | ||
| /// </summary> | ||
| [Tooltip("The amount of times per second the internal event loop will run. This includes for example NetworkVariable checking.")] | ||
| public int EventTickrate = 64; | ||
| [Tooltip("The tickrate. This value controls how often MLAPI runs user code and sends out data. The value is in 'ticks per seconds' which means a value of 50 will result in 50 ticks being executed per second or a fixed delta time of 0.02.")] | ||
| public int TickRate = 30; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we confident an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not confident. But floating point tickrate introduces some potential problems. DOTS Netcode also only support integral tickrate. MLAPI also only supported integral tickrate before. I will note it down and mention it to UX. |
||
|
|
||
| /// <summary> | ||
| /// The amount of seconds to wait for handshake to complete before timing out a client | ||
|
|
@@ -206,9 +188,7 @@ public string ToBase64() | |
| writer.WriteString(config.RegisteredScenes[i]); | ||
| } | ||
|
|
||
| writer.WriteInt32Packed(config.ReceiveTickrate); | ||
| writer.WriteInt32Packed(config.MaxReceiveEventsPerTickRate); | ||
| writer.WriteInt32Packed(config.EventTickrate); | ||
| writer.WriteInt32Packed(config.TickRate); | ||
| writer.WriteInt32Packed(config.ClientConnectionBufferTimeout); | ||
| writer.WriteBool(config.ConnectionApproval); | ||
| writer.WriteInt32Packed(config.LoadSceneTimeOut); | ||
|
|
@@ -249,9 +229,7 @@ public void FromBase64(string base64) | |
| config.RegisteredScenes.Add(reader.ReadString().ToString()); | ||
| } | ||
|
|
||
| config.ReceiveTickrate = reader.ReadInt32Packed(); | ||
| config.MaxReceiveEventsPerTickRate = reader.ReadInt32Packed(); | ||
| config.EventTickrate = reader.ReadInt32Packed(); | ||
| config.TickRate = reader.ReadInt32Packed(); | ||
| config.ClientConnectionBufferTimeout = reader.ReadInt32Packed(); | ||
| config.ConnectionApproval = reader.ReadBool(); | ||
| config.LoadSceneTimeOut = reader.ReadInt32Packed(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,27 +7,12 @@ public class NetworkBehaviourUpdater | |
| { | ||
| private HashSet<NetworkObject> m_Touched = new HashSet<NetworkObject>(); | ||
|
|
||
| /// <summary> | ||
| /// Stores the network tick at the NetworkBehaviourUpdate time | ||
| /// This allows sending NetworkVariables not more often than once per network tick, regardless of the update rate | ||
| /// </summary> | ||
| public ushort CurrentTick { get; set; } | ||
|
|
||
| #if DEVELOPMENT_BUILD || UNITY_EDITOR | ||
| private ProfilerMarker m_NetworkBehaviourUpdate = new ProfilerMarker($"{nameof(NetworkBehaviour)}.{nameof(NetworkBehaviourUpdate)}"); | ||
| #endif | ||
|
|
||
| internal void NetworkBehaviourUpdate(NetworkManager networkManager) | ||
| { | ||
| // Do not execute NetworkBehaviourUpdate more than once per network tick | ||
| ushort tick = networkManager.NetworkTickSystem.GetTick(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SamuelBellomo I removed this from NetworkBehaviour and now removing this from here as well. This will be called during the network tick now, (In theory multiple times per frame but only once per network tick). This is because each a network variable could be updated by user code and we don't want to lose that information (in most cases irrelevant unless the frame rate < tick rate). Hope that makes sense.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with it, but you might want to git blame deeper on this, I'm not the original writer of that code, I copy pasted it when I moved this from NetworkBehaviour.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👋
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So @jeffreyrainy would you agree that this is no longer necessary if we call this directly from the tick event of our tick system which only gets called once per tick?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, all good. Was just providing data on the "I'm not the original writer of that code" part. |
||
| if (tick == CurrentTick) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| CurrentTick = tick; | ||
|
|
||
| #if DEVELOPMENT_BUILD || UNITY_EDITOR | ||
| m_NetworkBehaviourUpdate.Begin(); | ||
| #endif | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also kill this too? I asked the same question before in @pdeschain's
NetworkAnimatorPR. Do we really want to controlNetworkAnimator's tickrate separately from network config's tickrate? If so, why? If not, why keepm_SendRateand its implementation around?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most likely yes, we will want to remove this. But I don't think it should be in this PR and I think we first need to better understand how we will do NetworkVariables / priority in the future.