SoftwareGuy / Ignorance

Ignorance utilizes the power of ENet to provide a reliable UDP networking transport for Mirror Networking.
Other
248 stars 31 forks source link

Spawn Scene Objects #58

Closed Piflik closed 3 years ago

Piflik commented 4 years ago

I am trying to use Ignorance instead of Telepathy, because we use Dissonance Voice Chat and they advise against using a TCP Transport, but I have run into a problem with spawning scene objects. There are two Functions in Mirror.ClientScene: PrepareToSpawnSceneObjects() and SpawnSceneObject(SpawnMessage). The former has to be called before the latter, but when I use Ignorance (Classic or Threaded), they are not always in the correct order, presumably because of the unreliability of UDP (the ObjectSpawnStartedMessage arrives before the SpawnMessage). This happens only on the client and only sporadically.

Is there a way to fix this or ca I just not use Ignorance/UDP?

SoftwareGuy commented 4 years ago

Hmm, this sounds like a out-of-order problem from the description you've provided.

I have used Dissonance without any problems on Ignorance, so of course it can be fixed. How it can be fixed is the question. If this happens on the client only, then it could be it's higher in the chain and not Ignorance's fault. However, sending the data over the reliable channel should include sequencing, so I'm wondering if it's something higher in the chain.

SoftwareGuy commented 4 years ago

I would definitely check the channels you're sending the data on. Channel 0 should be Reliable, and channel 1 should be Unreliable. Out of the box, I hardcode them to be the mentioned channel types.

Piflik commented 4 years ago

I have a custom function to handle scene loading, since I want players to be able to be in different scenes, but all messages are sent on channel 0. I use a NetworkSceneChecker to make sure players in different scenes don't see each other or see the content of other scenes (since scenes get loaded additively, there might be instances where the old scene is not yet unloaded)

This is the code I am using:

void LoadSceneOnServer(NetworkConnection connection, Message.SwitchCubicle message) {
    Scene cubicle = SceneManager.GetSceneByName(message.cubicleName);

    void MovePlayerToScene() {
    SceneManager.MoveGameObjectToScene(connection.identity.gameObject, SceneManager.GetSceneByName(message.cubicleName));

    if (string.IsNullOrWhiteSpace(message.oldCubicleName)) return;

    Scene oldCubicle = SceneManager.GetSceneByName(message.oldCubicleName);
    if (oldCubicle.isLoaded && _serverPlayersInScene.TryGetValue(message.oldCubicleName, out HashSet<NetworkConnection> playersInOldCubicle)) {
        playersInOldCubicle.Remove(connection);
        if (playersInOldCubicle.Count < 1) {
            SceneManager.UnloadSceneAsync(oldCubicle);
        }
    }

    connection.Send(new SceneMessage {
        sceneName = message.oldCubicleName,
        sceneOperation = SceneOperation.UnloadAdditive,
        customHandling = false,
    });
    }

    connection.Send(new SceneMessage {
    sceneName = message.cubicleName,
    sceneOperation = SceneOperation.LoadAdditive,
    customHandling = false,
    });

    if (!cubicle.isLoaded) {
    AsyncOperation async = SceneManager.LoadSceneAsync(message.cubicleName, LoadSceneMode.Additive);
    async.completed += operation => {
        MovePlayerToScene();
    };
    } else {
    MovePlayerToScene();
    }

    if (!_serverPlayersInScene.TryGetValue(message.cubicleName, out HashSet<NetworkConnection> players)) {
    players = new HashSet<NetworkConnection>();
    _serverPlayersInScene[message.cubicleName] = players;
    }

    players.Add(connection);
}

I could reduce the frequency of the error by sending the SceneMessage before I start loading the scene on the server, but it still happens sometimes.

I was unable to reproduce the error using Telepathy.

SoftwareGuy commented 4 years ago

Can you make a reproduction project so I can test locally? This is an interesting out-of-order issue.

Actually, one thing I can think of is what priority is the Ignorance Transport in the Script Execution Order? Since it's tied to LateUpdate (for now...) it should be running after Default Time, above 1000.

Also, the other thing - don't use Ignorance Classic if you can help it. It's deprecated and will be removed in 1.4. You will get much better performance by using the Threaded version.

Piflik commented 4 years ago

Here is a minimal project where I was able to reproduce it. It happens 100% on client connect (although this might just be too early to call that), and it can be reproduced by switching scenes. The frequency of the error is quite low, I needed to switch the scene hundreds of times before it happened, but it is possible that the low complexity hides the error. I included a function to guarantee the error, albeit by switching scenes very fast, so this might not be the actual cause (in my actual project I don't need to switch scenes fast to provoke the error). The high frequency of scene switching might simply cause the messages to arrive interleaved.

To reproduce:

To switch scenes you can use Alpha1 and Alpha2 (not Numpad) Pressing Alpha0 starts the "Break" Coroutine that switches scenes all 0.02s, until the key is released.

IgnoranceSceneCheckerTest.zip

SoftwareGuy commented 4 years ago

Thanks very much for this - I'll see what I can do to replicate it and step through it in the debugger.

Piflik commented 4 years ago

I just updated Mirror directly from their repository with the release from today, and, at least in my project, the error doesn't seem to happen anymore. So it seems like this was a Mirror issue, not connected to Ignorance.

I also updated Mirror in the minimal project, but the reproductions still fail. So the errors that happen there seem to be unconnected to the actual error and just present the same. Also, as I said above, it is probably a Mirror issue.

Piflik commented 4 years ago

Sadly, I have to correct my previous statement. The error is still present, my test was flawed. It does not happen if no scene has been loaded on the server.

It seems like the client tries to spawn every scene object from previously loaded scenes when he tries to load a networked scene for the first time (I also have instanced scenes where users can't see or interact with each other).

I thought it was caused by me moving the player to the scene currently active on the server, but while that seemed to help, if only caused the error to appear the second time a client connects to a server.

However, I still think this is a Mirror issue, not Ignorance.

SoftwareGuy commented 4 years ago

Hey - sorry for the late response; had a workstation crash that I only just recovered from. 😦

Okay, so from what I'm reading is that this problem does not happen if you haven't loaded any scene on server. But it does start happening when a client loads the network scene that the server is on and other scenes have been previously loaded.

Ignorance is only dispatching whatever it's been told, a reliable does sequencing and all that good stuff, much like TCP would. So I think it could be a Mirror race condition, or a bug that is slowly coming out of the woodwork.

SoftwareGuy commented 4 years ago

Is the example ZIP file you provided still a valid repro? Got an extra set of eyes looking at this; may be a Mirror bugfix.

Piflik commented 4 years ago

The timeline to make the error appear in my project is as follows:

  1. Start the server/host
  2. Start the client and connect to the server
  3. Load a networked scene on the client; this causes the server to spawn the online player object; still no problem
  4. Disconnect the client
  5. Start the client again, without restarting the server
  6. Load a networked scene on the client -> client tries to spawn a scene object from an invalid scene
  7. No error on subsequent scene loads (neither when I try to load the same scene again nor a different one that hasn't been loaded before)

I also tried to load a scene on the host client before connecting the standalone client, and this does not seem to cause the problem. I still can connect to the server and load a scene once, but the second time I try to connect to the server and load a scene, the error occurs. I still have to test more to see if it happens when another standalone client loads a scene first before a second standalone client connects or if it only happens if one client tries to connect more than once.

At the moment I have a delay of 0.1s before I call SceneManager.MoveObjectToScene in the server-side part of the scene loading logic, and this seems to fix or at least suppress the error. I don't really like this kind of fix, but in networking code I can accept it more than in offline code.

I am not sure if the repro is 100% valid, but it shows a similar behavior: it produces the same error when the client tries to load a networked scene for the first time after connecting, but not on subsequent scene loads (unless scenes are switched too fast). I can try to create a new repro when I am back at work next week.

MrGadget1024 commented 4 years ago

@Piflik Do you have a more sensible repro project we can look at. The one linked above is just too silly to be a practical use case. Server should have subscenes loaded in advance and keep them, not load/unload them every frame or every few frames, and the client shouldn't be switching scenes that quickly either in a typical game. I wouldn't bet on Unity keeping up with that either, given the Async loading mechanism.

Piflik commented 4 years ago

Sorry for not answering for so long. I was unable to create a minimal example that displays the error, but in my project it gets progressively worse. I just increased the delay before I move the client to the scene to 1.5s, because 0.5 wasn't enough anymore, and this will get magnitudes worse when we finish implementing our system to load scenes as Addressables from a server, if they are not present or too old. This seems to be a race condition between loading a scene on the server and the client and I am not sure what could even be done about that in the Mirror Framework itself.

My assumption is as follows: I send the SceneMessage to the client, then I start loading the scene on the server, and when this is finished I move the client into the scene. But if the client has not finished loading the scene, it gets the message to activate the new scene too early (via the NetworkSceneChecker), and it breaks.

I will try to fix it with another message that the client sends back to the server when the scene has finished loading so I can delay the MoveGameObjectToScene call until I know the scene is ready on the client, but I have to wait till the Addressable code is done (currently I have no good hook to send the message from. I would have tried OnClientSceneChanged, but I don't have access to the scene name there and I try to not modify code from external Frameworks, so I can upgrade them without problems).

It would be nice, if that activation message would be stored in a buffer until the scene load operation is complete. This would not help in our case, since we have to write the scene loading logic ourselves anyway now to handle Addressables, but it might be helpful for more sensible use cases.

SoftwareGuy commented 3 years ago

Going to close this since it's creeping to be a year and may be Mirror related. A lot has changed from May 8th 2020 to present day (15th March 2021), so if this still shits the bed then feel free to reopen.