Skip to content
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

Network RTT calculation significantly inaccurate after ServerChangeScene #3576

Closed
Mr-Oregano opened this issue Aug 3, 2023 · 7 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@Mr-Oregano
Copy link

Mr-Oregano commented Aug 3, 2023

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:

  • Using "Room" example...
  • In OfflineScene, add NetworkPingDisplay component to the NetworkRoomManager game object.
    -> Since the example room scene is simple, it loads really fast, so we need to artificially create a loading delay.
  • In NetworkRoomManagerExt.OnGUI() I added...
...

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)
  • 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

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 MrGadget1024 added the bug Something isn't working label Oct 29, 2023
@MrGadget1024
Copy link
Collaborator

Need to reset RTT after scene loading.

@miwarnec
Copy link
Collaborator

great bug report @Mr-Oregano , I can reproduce it:
2023-11-15 - 09-54-39@2x

@miwarnec
Copy link
Collaborator

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
Copy link
Collaborator

resetting NetworkTime statics isn't enough.
we are still receiving messages with high rtt:
2023-11-15 - 10-22-59@2x

@miwarnec
Copy link
Collaborator

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

@miwarnec
Copy link
Collaborator

fix is ready, see PR linked above

miwarnec added a commit that referenced this issue Nov 15, 2023
…sages before a (potentially long) scene load. fixes a bug where RTT would be very high after a long scene load.
miwarnec added a commit that referenced this issue Nov 17, 2023
…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 c729fe1.
miwarnec added a commit that referenced this issue Nov 17, 2023
…sages 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
@miwarnec miwarnec reopened this Nov 17, 2023
@miwarnec
Copy link
Collaborator

reopening, had to revert the original fix since it breaks tanks/billiards demos with latency sim.
new pr: #3656

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants