From d34a04626408091bdd4c7607a5b1d9045c66a16f Mon Sep 17 00:00:00 2001 From: mischa Date: Fri, 17 Nov 2023 11:07:53 +0100 Subject: [PATCH 1/2] Revert "fix: #3576 Pings are now stamped with a scene hash so we can drop messages before a (potentially long) scene load. fixes a bug where RTT would be very high after a long scene load. (#3650)" This reverts commit c729fe119c871e21b4f11658ec8476e90ecb3b7b. --- Assets/Mirror/Core/Messages.cs | 20 ++----------------- .../Mirror/Core/NetworkConnectionToClient.cs | 4 +--- Assets/Mirror/Core/NetworkTime.cs | 13 ------------ 3 files changed, 3 insertions(+), 34 deletions(-) diff --git a/Assets/Mirror/Core/Messages.cs b/Assets/Mirror/Core/Messages.cs index 5a5c57afdd0..62888c07e62 100644 --- a/Assets/Mirror/Core/Messages.cs +++ b/Assets/Mirror/Core/Messages.cs @@ -105,13 +105,6 @@ public struct EntityStateMessage : NetworkMessage // whoever wants to measure rtt, sends this to the other end. public struct NetworkPingMessage : NetworkMessage { - // ping messages are stamped with scene name (as hash). - // this way we can disregard messages from before a scene change. - // otherwise a 30s loading pause would cause super high RTT after: - // https://github.com/MirrorNetworking/Mirror/issues/3576 - // (2 byte hash instead of N byte string to minimize bandwidth) - public ushort sceneHash; - // local time is used to calculate round trip time, // and to calculate the predicted time offset. public double localTime; @@ -119,9 +112,8 @@ public struct NetworkPingMessage : NetworkMessage // predicted time is sent to compare the final error, for debugging only public double predictedTimeAdjusted; - public NetworkPingMessage(ushort sceneHash, double localTime, double predictedTimeAdjusted) + public NetworkPingMessage(double localTime, double predictedTimeAdjusted) { - this.sceneHash = sceneHash; this.localTime = localTime; this.predictedTimeAdjusted = predictedTimeAdjusted; } @@ -131,13 +123,6 @@ public NetworkPingMessage(ushort sceneHash, double localTime, double predictedTi // we can use this to calculate rtt. public struct NetworkPongMessage : NetworkMessage { - // ping messages are stamped with scene name (as hash). - // this way we can disregard messages from before a scene change. - // otherwise a 30s loading pause would cause super high RTT after: - // https://github.com/MirrorNetworking/Mirror/issues/3576 - // (2 byte hash instead of N byte string to minimize bandwidth) - public ushort sceneHash; - // local time is used to calculate round trip time. public double localTime; @@ -145,9 +130,8 @@ public struct NetworkPongMessage : NetworkMessage public double predictionErrorUnadjusted; public double predictionErrorAdjusted; // for debug purposes - public NetworkPongMessage(ushort sceneHash, double localTime, double predictionErrorUnadjusted, double predictionErrorAdjusted) + public NetworkPongMessage(double localTime, double predictionErrorUnadjusted, double predictionErrorAdjusted) { - this.sceneHash = sceneHash; this.localTime = localTime; this.predictionErrorUnadjusted = predictionErrorUnadjusted; this.predictionErrorAdjusted = predictionErrorAdjusted; diff --git a/Assets/Mirror/Core/NetworkConnectionToClient.cs b/Assets/Mirror/Core/NetworkConnectionToClient.cs index 52207188a97..361b00c34e6 100644 --- a/Assets/Mirror/Core/NetworkConnectionToClient.cs +++ b/Assets/Mirror/Core/NetworkConnectionToClient.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Runtime.CompilerServices; using UnityEngine; -using UnityEngine.SceneManagement; namespace Mirror { @@ -130,8 +129,7 @@ protected virtual void UpdatePing() // messages' timestamp and only send a message number. // This way client's can't just modify the timestamp. // predictedTime parameter is 0 because the server doesn't predict. - ushort sceneHash = SceneManager.GetActiveScene().name.GetStableHashCode16(); - NetworkPingMessage pingMessage = new NetworkPingMessage(sceneHash, NetworkTime.localTime, 0); + NetworkPingMessage pingMessage = new NetworkPingMessage(NetworkTime.localTime, 0); Send(pingMessage, Channels.Unreliable); lastPingTime = NetworkTime.localTime; } diff --git a/Assets/Mirror/Core/NetworkTime.cs b/Assets/Mirror/Core/NetworkTime.cs index f9efcb89cc5..cb1f7776279 100644 --- a/Assets/Mirror/Core/NetworkTime.cs +++ b/Assets/Mirror/Core/NetworkTime.cs @@ -7,7 +7,6 @@ using System; using System.Runtime.CompilerServices; using UnityEngine; -using UnityEngine.SceneManagement; #if !UNITY_2020_3_OR_NEWER using Stopwatch = System.Diagnostics.Stopwatch; #endif @@ -145,11 +144,8 @@ internal static void UpdateClient() { // send raw predicted time without the offset applied yet. // we then apply the offset to it after. - // include scene name (as hash) - ushort sceneHash = SceneManager.GetActiveScene().name.GetStableHashCode16(); NetworkPingMessage pingMessage = new NetworkPingMessage ( - sceneHash, localTime, predictedTime ); @@ -179,7 +175,6 @@ internal static void OnServerPing(NetworkConnectionToClient conn, NetworkPingMes // Debug.Log($"OnServerPing conn:{conn}"); NetworkPongMessage pongMessage = new NetworkPongMessage ( - message.sceneHash, message.localTime, unadjustedError, adjustedError @@ -195,13 +190,6 @@ internal static void OnClientPong(NetworkPongMessage message) // prevent attackers from sending timestamps which are in the future if (message.localTime > localTime) return; - // ping messages are stamped with scene name (as hash). - // this way we can disregard messages from before a scene change. - // otherwise a 30s loading pause would cause super high RTT after: - // https://github.com/MirrorNetworking/Mirror/issues/3576 - int sceneHash = SceneManager.GetActiveScene().name.GetStableHashCode(); - if (message.sceneHash != sceneHash) return; - // how long did this message take to come back double newRtt = localTime - message.localTime; _rtt.Add(newRtt); @@ -222,7 +210,6 @@ internal static void OnClientPing(NetworkPingMessage message) // Debug.Log($"OnClientPing conn:{conn}"); NetworkPongMessage pongMessage = new NetworkPongMessage ( - message.sceneHash, message.localTime, 0, 0 // server doesn't predict ); From 26ae2becc2261852285ac5365a67578af4145470 Mon Sep 17 00:00:00 2001 From: mischa <16416509+miwarnec@users.noreply.github.com> Date: Wed, 15 Nov 2023 11:25:37 +0100 Subject: [PATCH 2/2] fix: #3576 Pings are now stamped with a scene hash so we can drop messages before a (potentially long) scene load. fixes a bug where RTT would be very high after a long scene load. (#3650) * fix: #3576 Pings are now stamped with a scene hash so we can drop messages before a (potentially long) scene load. fixes a bug where RTT would be very high after a long scene load. * 16 bit hash fakebyte --- Assets/Mirror/Core/Messages.cs | 20 +++++++++++++++++-- .../Mirror/Core/NetworkConnectionToClient.cs | 4 +++- Assets/Mirror/Core/NetworkTime.cs | 13 ++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/Assets/Mirror/Core/Messages.cs b/Assets/Mirror/Core/Messages.cs index 62888c07e62..5a5c57afdd0 100644 --- a/Assets/Mirror/Core/Messages.cs +++ b/Assets/Mirror/Core/Messages.cs @@ -105,6 +105,13 @@ public struct EntityStateMessage : NetworkMessage // whoever wants to measure rtt, sends this to the other end. public struct NetworkPingMessage : NetworkMessage { + // ping messages are stamped with scene name (as hash). + // this way we can disregard messages from before a scene change. + // otherwise a 30s loading pause would cause super high RTT after: + // https://github.com/MirrorNetworking/Mirror/issues/3576 + // (2 byte hash instead of N byte string to minimize bandwidth) + public ushort sceneHash; + // local time is used to calculate round trip time, // and to calculate the predicted time offset. public double localTime; @@ -112,8 +119,9 @@ public struct NetworkPingMessage : NetworkMessage // predicted time is sent to compare the final error, for debugging only public double predictedTimeAdjusted; - public NetworkPingMessage(double localTime, double predictedTimeAdjusted) + public NetworkPingMessage(ushort sceneHash, double localTime, double predictedTimeAdjusted) { + this.sceneHash = sceneHash; this.localTime = localTime; this.predictedTimeAdjusted = predictedTimeAdjusted; } @@ -123,6 +131,13 @@ public NetworkPingMessage(double localTime, double predictedTimeAdjusted) // we can use this to calculate rtt. public struct NetworkPongMessage : NetworkMessage { + // ping messages are stamped with scene name (as hash). + // this way we can disregard messages from before a scene change. + // otherwise a 30s loading pause would cause super high RTT after: + // https://github.com/MirrorNetworking/Mirror/issues/3576 + // (2 byte hash instead of N byte string to minimize bandwidth) + public ushort sceneHash; + // local time is used to calculate round trip time. public double localTime; @@ -130,8 +145,9 @@ public struct NetworkPongMessage : NetworkMessage public double predictionErrorUnadjusted; public double predictionErrorAdjusted; // for debug purposes - public NetworkPongMessage(double localTime, double predictionErrorUnadjusted, double predictionErrorAdjusted) + public NetworkPongMessage(ushort sceneHash, double localTime, double predictionErrorUnadjusted, double predictionErrorAdjusted) { + this.sceneHash = sceneHash; this.localTime = localTime; this.predictionErrorUnadjusted = predictionErrorUnadjusted; this.predictionErrorAdjusted = predictionErrorAdjusted; diff --git a/Assets/Mirror/Core/NetworkConnectionToClient.cs b/Assets/Mirror/Core/NetworkConnectionToClient.cs index 361b00c34e6..52207188a97 100644 --- a/Assets/Mirror/Core/NetworkConnectionToClient.cs +++ b/Assets/Mirror/Core/NetworkConnectionToClient.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Runtime.CompilerServices; using UnityEngine; +using UnityEngine.SceneManagement; namespace Mirror { @@ -129,7 +130,8 @@ protected virtual void UpdatePing() // messages' timestamp and only send a message number. // This way client's can't just modify the timestamp. // predictedTime parameter is 0 because the server doesn't predict. - NetworkPingMessage pingMessage = new NetworkPingMessage(NetworkTime.localTime, 0); + ushort sceneHash = SceneManager.GetActiveScene().name.GetStableHashCode16(); + NetworkPingMessage pingMessage = new NetworkPingMessage(sceneHash, NetworkTime.localTime, 0); Send(pingMessage, Channels.Unreliable); lastPingTime = NetworkTime.localTime; } diff --git a/Assets/Mirror/Core/NetworkTime.cs b/Assets/Mirror/Core/NetworkTime.cs index cb1f7776279..f9efcb89cc5 100644 --- a/Assets/Mirror/Core/NetworkTime.cs +++ b/Assets/Mirror/Core/NetworkTime.cs @@ -7,6 +7,7 @@ using System; using System.Runtime.CompilerServices; using UnityEngine; +using UnityEngine.SceneManagement; #if !UNITY_2020_3_OR_NEWER using Stopwatch = System.Diagnostics.Stopwatch; #endif @@ -144,8 +145,11 @@ internal static void UpdateClient() { // send raw predicted time without the offset applied yet. // we then apply the offset to it after. + // include scene name (as hash) + ushort sceneHash = SceneManager.GetActiveScene().name.GetStableHashCode16(); NetworkPingMessage pingMessage = new NetworkPingMessage ( + sceneHash, localTime, predictedTime ); @@ -175,6 +179,7 @@ internal static void OnServerPing(NetworkConnectionToClient conn, NetworkPingMes // Debug.Log($"OnServerPing conn:{conn}"); NetworkPongMessage pongMessage = new NetworkPongMessage ( + message.sceneHash, message.localTime, unadjustedError, adjustedError @@ -190,6 +195,13 @@ internal static void OnClientPong(NetworkPongMessage message) // prevent attackers from sending timestamps which are in the future if (message.localTime > localTime) return; + // ping messages are stamped with scene name (as hash). + // this way we can disregard messages from before a scene change. + // otherwise a 30s loading pause would cause super high RTT after: + // https://github.com/MirrorNetworking/Mirror/issues/3576 + int sceneHash = SceneManager.GetActiveScene().name.GetStableHashCode(); + if (message.sceneHash != sceneHash) return; + // how long did this message take to come back double newRtt = localTime - message.localTime; _rtt.Add(newRtt); @@ -210,6 +222,7 @@ internal static void OnClientPing(NetworkPingMessage message) // Debug.Log($"OnClientPing conn:{conn}"); NetworkPongMessage pongMessage = new NetworkPongMessage ( + message.sceneHash, message.localTime, 0, 0 // server doesn't predict );