BeardedManStudios / ForgeNetworkingRemastered

See various forks, also please join the Forge Community fork of Forge Alloy! -> https://github.com/ForgeAlloyCommunity/ForgeAlloy
https://twitter.com/FarrisFaulds
Apache License 2.0
1.49k stars 306 forks source link

Changed the network object registration timing #270

Closed PyeonggukLee closed 5 years ago

PyeonggukLee commented 5 years ago

Why pushed this PR? I had a problem that the client A doesn't get synced a network object of the client B sometimes when they try to connect to a server at the same time.

Sequences when new client connects to a server(AFAIK):

  1. A client connects to the server.
  2. The server sends the data containing currently existed 'NetworkObjects' to the client.
  3. A networkObject is created for the client and send the data to other players.
  4. On the main thread, the networkObject is initialized and added to 'NetworkObjects'.

In the case that two clients try to connect at the same time, sometimes the client connected later than another doesn't get the networkObject data for another client. There are two clients: A, B. A3-B1-B2-A4 => a networkObject of the client A doesn't get synced on the client B side, because A4 happens later than B2.

so, to solve these problem, I merged the step 3 and 4 in to one step.

Merged Sequences:

  1. A client connects to the server. (same)
  2. The server sends the data containing currently existed 'NetworkObjects' to the client. (same)
  3. A networkObject is created and initialize & added to 'NetworkObjects'. for the client and send the data to other players.

so, A3-B1-B2 (NetworkObjects is updated at A3 timing, there's no omitted data at B2.)

Please, review this PR and give me any opinons to discuss or questions about this.

phalasz commented 5 years ago

I assume CompleteInitialization then doesn't need to be called anywhere else right? Did you remove those calls as well if there were any?

PyeonggukLee commented 5 years ago

@phalasz The purpose of this PR is that the server should register the networkObject to NetworkObjects collection as soon as the networkObject is created. so, I've added the CompleteInitialization in there(RegisterNetworkObject). As you said, CompleteInitialization also gets called when NetworkStart is called for the first time, but it checks the networkObject is in the NetworkObjects collection or not. so it seems fine, doesn't it?

ref:

public void CompleteInitialization(NetworkObject networkObject)
{
    lock (NetworkObjects)
    {
        if (NetworkObjects.ContainsKey(networkObject.NetworkId))
            return;

        NetworkObjects.Add(networkObject.NetworkId, networkObject);
        NetworkObjectList.Add(networkObject);
    }
}
PyeonggukLee commented 5 years ago

or... more clearly, I could do adding a networkObject to the NetworkObjects collection if the NetWorker is IServer.

phalasz commented 5 years ago

Ah I see, makes sense. I think using CompleteInitialization is fine.