MirrorNetworking / Mirror

#1 Open Source Unity Networking Library
https://mirror-networking.com
MIT License
5.23k stars 772 forks source link

Network RTT calculation significantly inaccurate after ServerChangeScene #3576

Closed Mr-Oregano closed 1 year ago

Mr-Oregano commented 1 year ago

Describe the bug The Network RTT calculation becomes highly inaccurate after an online scene is loaded with ServerChangeScene. This is due to all messages being blocked by server and client whenever a scene is being loaded. If the scene takes a long time to load, EMA for NetworkTime.cs will be thrown off.

How can we reproduce the issue, step by step:

ServerChangeScene(GameplayScene);

// ↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓ static async void foo() { loadingSceneAsync.allowSceneActivation = false; await System.Threading.Tasks.Task.Delay(5000); loadingSceneAsync.allowSceneActivation = true; };

foo(); // ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑


 -  Open project on new instance of Unity editor (could use [ParrelSync](https://github.com/VeriorPies/ParrelSync))
 -  Run game on both Unity editors (start host on one, start client on the other)
 -  Ready players on both clients, and start game on host client
 -  Scene should take 5 seconds to load after which you should see the RTT on the client rise significantly (usually in the 1000s)

**Expected behavior**
Ideally, `NetworkPingMessage` should still go through while loading a scene so that server/client RTT calculation always stays as accurate as possible. 

**Screenshots**
![image](https://github.com/MirrorNetworking/Mirror/assets/33503562/98197506-17ec-4192-a391-82b2568aa2b3)

**Desktop:**
* OS: Windows
* Build target: Android, Windows
* Unity version: `v2021.3.8f1`
* Mirror branch: master

**Additional context**
After a while, the RTT will settle back down to a more accurate number, usually this takes a few seconds to happen.
MrGadget1024 commented 1 year ago

Need to reset RTT after scene loading.

miwarnec commented 1 year ago

great bug report @Mr-Oregano , I can reproduce it:

2023-11-15 - 09-54-39@2x
miwarnec commented 1 year ago

okay so the above repro is not valid because it sleeps after the scene was loaded. in other words, that's just a server freezing randomly - which is expected to increase RTT.

however, this is more fitting repro for the issue: NetworkManager.ServerChangeScene gets an artificial 5s delay right after LoadSceneAsync(), simulating a slow scene load:

        public virtual void ServerChangeScene(string newSceneName)
        {
            if (string.IsNullOrWhiteSpace(newSceneName))
            {
                Debug.LogError("ServerChangeScene empty scene name");
                return;
            }

            if (NetworkServer.isLoadingScene && newSceneName == networkSceneName)
            {
                Debug.LogError($"Scene change is already in progress for {newSceneName}");
                return;
            }

            // Debug.Log($"ServerChangeScene {newSceneName}");
            NetworkServer.SetAllClientsNotReady();
            networkSceneName = newSceneName;

            // Let server prepare for scene change
            OnServerChangeScene(newSceneName);

            // set server flag to stop processing messages while changing scenes
            // it will be re-enabled in FinishLoadScene.
            NetworkServer.isLoadingScene = true;

            loadingSceneAsync = SceneManager.LoadSceneAsync(newSceneName);

            // ARTIFICIAL DELAY
            Thread.Sleep(5000);

            // ServerChangeScene can be called when stopping the server
            // when this happens the server is not active so does not need to tell clients about the change
            if (NetworkServer.active)
            {
                // notify all clients about the new scene
                NetworkServer.SendToAll(new SceneMessage
                {
                    sceneName = newSceneName
                });
            }

            startPositionIndex = 0;
            startPositions.Clear();
        }

this is a better repro, so the client doesn't immediately enter the scene either. working on the fix now.

miwarnec commented 1 year ago

resetting NetworkTime statics isn't enough. we are still receiving messages with high rtt:

2023-11-15 - 10-22-59@2x
miwarnec commented 1 year ago

client is still receiving about 50 messages from the previous scene with the previous old time. we need to disregard them somehow.

miwarnec commented 1 year ago

fix is ready, see PR linked above

miwarnec commented 1 year ago

reopening, had to revert the original fix since it breaks tanks/billiards demos with latency sim. new pr: https://github.com/MirrorNetworking/Mirror/pull/3656