Unity-Technologies / com.unity.netcode.gameobjects

Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.
MIT License
2.15k stars 435 forks source link

Native callback on NetworkObjectSpawned #1325

Open SorenSaket opened 3 years ago

SorenSaket commented 3 years ago

Is your feature request related to a problem? Please describe. I have a lot of asnyc operations that rely on NetworkedObjects (Client side). Therefore I require a reference to the specific NetworkObject when it spawns. Right now there's no easy way to know when an object is spawned and no way for other objects to react to spawned NetworkObjects.

Current workarounds include (Not limited to):

Describe the solution you'd like I would however suggest to have a built-in solution, that you can easily subscribe to. On invocation it might return the NetworkObject ID of the spawned object, or a reference to it. That way you can get the prefabhash from it and more. This makes sense since all other networked states have easily accessible OnChange events. RPC in themselfes notify of a state change and Networked Variables have an OnValueChangedDelegate.

will-mearns commented 3 years ago

Added into our holistic backlog for discussion - MTTH-122

JesseOlmer commented 2 years ago

Is this still relevant? If so, can you please provide more information? We have OnNetworkSpawn on the objects themselves. Is there a reason you can't call whatever systems you have locally from this callback?

zachstronaut commented 1 year ago

The OnNetworkSpawn virtual method is called as each individual NetworkObject in your scene comes online. This means that some of your other NetworkObjects that were in the scene when it loaded will be IsSpawned and some of them will not be IsSpawned. There are certain operations you cannot do (such as re-parenting) until the objects involved are all IsSpawned. This kind of reduces the utility of OnNetworkSpawn. I imagine in almost every project, us programmers are having to solve this problem.

It would be very useful to have another built-in virtual method on NetworkBehaviour that folks could use that would be called on every NetworkObject AFTER ALL NetworkObjects have reached their IsSpawned state.

Kind of similar to Awake vs Start in a way.

JesseOlmer commented 1 year ago

@JesseOlmer The OnNetworkSpawn virtual method is called as each individual NetworkObject in your scene comes online. This means that some of your other NetworkObjects that were in the scene when it loaded will be IsSpawned and some of them will not be IsSpawned. There are certain operations you cannot do (such as re-parenting) until the objects involved are all IsSpawned. This kind of reduces the utility of OnNetworkSpawn. I imagine in almost every project, us programmers are having to solve this problem.

It would be very useful to have another built-in virtual method on NetworkBehaviour that folks could use that would be called on every NetworkObject AFTER ALL NetworkObjects have reached their IsSpawned state.

Kind of similar to Awake vs Start in a way.

@zachstronaut sorry but I don't work at Unity any longer. This ticket is over a year old, and you're asking for what sounds like a different feature than the original person was. You might have better luck opening your own issue, but again, I haven't worked on this project since last year, so I won't be much help to you, and considering this ticket hasn't been closed in that time they may not even triage these issues any longer. Good luck.

NoelStephensUnity commented 1 year ago

@zachstronaut If you are referring to client synchronization, then there is actually a way to know when a client has finished loading all scenes required and spawned all NetworkObjects via the SceneEventType.Synchronize scene event.

You can either subscribe to the NetworkSceneManager.OnSynchronizeComplete event or trap for it using the more verbose NetworkSceneManager.OnSceneEvent.

So, you could subscribe to either event in a NetworkBehaviour component assigned to one or more in-scene placed NetworkObjects, and when it is invoked you know that all NetworkObjects are spawned (and you can handling parenting or any other action that requires certain NetworkObjects to be spawned).

Always keep in mind that the server-host does all of the dynamic spawning and under the context of dynamically spawning several NetworkObjects the server-host would handle parenting (and such) in the same region of code (same stack really) that spawned the NetworkObjects.

Let me know if you have a scenario where you couldn't use the SynchronizeComplete event or if it provides you the functionality you were looking for?

zachstronaut commented 1 year ago

@NoelStephensUnity We're using SceneEventType.LoadEventCompleted to solve this problem at the moment in most cases where we needed something like this.

That said, the requested feature here would still be a built in public virtual method that can be overridden that is called AFTER ALL NetworkObjects have reached their IsSpawned state.

OnNetworkSpawn

OnNetworkSynchronized ?

It's less about missing functionality and more about useful base functionality in NetworkBehaviour that offers convenience. It's less exciting to include a subscription to this event in many classes. I suppose we could subclass NetworkBehaviour.

NoelStephensUnity commented 1 year ago

@zachstronaut Totally understandable. The catch is knowing when "all" NetworkObjects needed to be spawned...are spawned. I could see where adding an additional protected virtual method to NetworkBehaviour (i.e. OnNetworkSynchronized) that was invoked immediately after the OnSynchronizeComplete method is invoked, but then that would be only in the context of a client's initial synchronization when scene management is enabled or after a client connection had been approved and the client-side had spawned all NetworkObjects in order to be synchronized with the server-host.

In order to invoke that for a set of NetworkObjects dynamically spawned by the server-host, we would need to add additional NetworkObjectId "associative" meta data that would require pre-or-post-spawn user input that associated the dynamically spawned NetworkObjects which then would make this it a bit more confusing as you would need to "associate" NetworkObjects with one another only under the context of dynamically spawned but not have to associate NetworkObjects when in-scene placed. So, would it be safe to assume you would only want this additional method invoked during the client's initial synchronization?

If we added this method here would be all of the different ways to know when all NetworkObjects are spawned during the initial client synchronization:

In the end, you still have to either subscribe or override.

If you can think of any other scenarios, other than client synchronization, where this would be useful please do post them here.

đź‘Ť

zachstronaut commented 1 year ago

@NoelStephensUnity Related to this, I'm kind of shocked to find that I cannot call a ClientRPC from a NetworkBehaviour until after SceneEventType.LoadEventCompleted. Specifically, this is when loading a scene and I want to call a ClientRPC on a NetworkBehaviour that exists on a NetworkObject that is in the scene, not spawned.

Now I'm really questioning the utility of OnNetworkSpawn / IsSpawned and I'm wondering where in the documentation I missed this rather important bit of info about RPCs when loading scenes??

NoelStephensUnity commented 1 year ago

@zachstronaut

Specifically, this is when loading a scene and I want to call a ClientRPC on a NetworkBehaviour that exists on a NetworkObject that is in the scene, not spawned.

If a NetworkObject is not spawned, then that means the associated NetworkBehaviours have no way of knowing how to route RPCs. Since you can have (n) instances of a NetworkBehaviour component, each instance is tied to their respective NetworkObject. If the NetworkObject is not spawned, then there is no associated NetworkObjectId.

So, the routing of an RPC is handled by: NetworkObjectId-->NetworkBehaviourId

If there is no NetworkObjectId (i.e. not spawned) then there is no way to know which NetworkBehaviour instance is the "right" one to invoke the RPC. Really, if your NetworkBehaviour.IsSpawned is set then you should be able to invoke an RPC...but we often recommend waiting until the client has completely synchronized to avoid the scenario where an RPC method might try to perform some netcode related behaviour (i.e. setting a NetworkVariable for instance) on a completely different NetworkBehaviour who's NetworkObject is not yet spawned.

However, with that being said... it looks like we might have some missing text on RPCs (in general) that discuss when it is "ok" to invoke them. Most likely, there should be something within the Ways to Synchronize that clearly states NetworkVariables and RPCs both require the associated NetworkObject to be spawned.

zachstronaut commented 1 year ago

@NoelStephensUnity Forgive the ambiguity of my previous statement. What I meant was:

Specifically, this is when loading a scene and I want to call a ClientRPC on a NetworkBehaviour that exists on a NetworkObject that is in the scene, not dynamically spawned.

I'm loading a scene, and in that scene exist NetworkObjects that are not dynamically spawned... they just exist in the scene.

When OnNetworkSpawn fires for the NetworkBehaviour of one of those objects, if I immediately call a ClientRPC on the host it never fires on the remote client.

public class Example : NetworkBehaviour
{
    public override void OnNetworkSpawn()
    {
        base.OnNetworkSpawn();
        Test_ClientRpc(); // ** see below
    }

    [ClientRpc]
    void Test_ClientRpc()
    {
        // never executes on remote client, only host
    }
}

The only way I'm able to get it to actually fire on the remote client too is if I wait for SceneEventType.LoadEventCompleted on the host before calling any RPCs.

To me this feels like a bug. If locally my object is in the IsSpawned state then I feel the Netcode internals should work out buffering and delivering the RPC to all clients once they finally sync the scene, too.

** sidenote, but why does this ClientRpc call not generate any error or warnings in the console when executed on the non-host client?

NoelStephensUnity commented 1 year ago

@zachstronaut Ahhh... ok that makes sense. So, if you look at the Loading Scenes section on the timing considerations page you will see that the server first loads the scene and then sends the notification.

However, you might try adjusting the NetworkManager.NetworkConfig.SpawnTimeout value as its default is 1 second... which I think if you just move that up to like 10-20 seconds (or a time reasonable to hang on to messages for "yet to be spawned" objects) you might find that your code above will start working as it is. It looks like you can make adjustments to that value during runtime... so you could tweak that value based on the level you are loading... so... you could theoretically use like a named message to broadcast a new "spawn time out" value just prior to loading a scene and have unique timeout values per scene (if needed... and I suggested a named message as that doesn't require a NetworkObject or NetworkBehaviour to work...so you could create a component and drop that onto your NetworkManager's GameObject that handles "global settings").

Let me know if this adjustment works for you?

zachstronaut commented 1 year ago

@NoelStephensUnity thanks for the helpful note on the configurable SpawnTimeout

I've got a related question now about the Loading Scenes section in the docs you sent me. It seems to raise another issue with OnNetworkSpawn. The docs say:

:::caution While a client can start sending the server messages (including NetworkVariable changes) upon local SceneEventType.LoadComplete event notifications, under more controlled testing environments where the network being used has little to no latency (that is, using loopback with multiple instances running on the same system or using your LAN), this approach won't expose latency related issues. Even though the timing might "work out" under controlled low latency conditions you can still run into edge case scenarios where if a client approaches or exceeds a 500ms RTT latency you can potentially run into issues. ::: TIP It is recommended that if your project's design requires that one or more NetworkBehaviours immediately send any form of client to server message (that is, changing a NetworkVariable, sending an RPC, sending a custom message, etc.) upon a client being locally notified of a SceneEventType.LoadComplete then you should test with artificial/simulated network conditions.

Is this a super long winded way of saying "you'll have problems with our default SpawnTimeout = 1 setting"? This is basically describing another scenario where you can't use the NetworkVariable or RPC features just based off OnNetworkSpawn alone.

My expectation as a Netcode API user is that the core "ready state" that is exposed in NetworkBehaviours via the IsSpawned bool and by the virtual method OnNetworkSpawn would be what is giving me the green light to use all features of Netcode. It's weird to have to reconfigure system defaults and/or having to add a handler for scene events in order to reliably use RPC and NetworkVariable features.

NoelStephensUnity commented 1 year ago

@zachstronaut Well, actually I believe that documentation was written prior to the SpawnTimeout capabilities... which means that is another good point about the documentation not being properly updated with things like that. Will add that section to my list of areas that need to be updated (along with RPCs etc).

The default values are typically set with "minimum overhead" in mind with the ability to adjust... but as we have come to determine in several areas it would seem the "minimum overhead" was too "minimum" indeed. Most likely a fair value for that would be like 4-5 seconds with the ability to adjust up or down.

zachstronaut commented 1 year ago

@NoelStephensUnity it's kind of untenable for me unless we set the timeout to 9999 or some effective "forever" ... otherwise it would still be possible to load into a scene where some initial setup silently fails, and not a failure that necessarily prevents the game from being played... just one that makes it break in unexpected ways later.

NoelStephensUnity commented 1 year ago

@zachstronaut So, the way SpawnTimeout works is it will hold onto messages (client-side) until the targeted NetworkObject (and associated NetworkBehaviour) is spawned and then process the messages. If you never spawn the NetworkObject that one or more messages are waiting for and the SpawnTimeout is reached, then those messages are dropped/ignored. As I understood it, you just wanted the ability to send RPCs immediately from an in-scene placed NetworkObject on the server side without having to wait for the scene(s) to finish loading on the client side?

It wouldn't make sense to hold onto messages indefinitely or for some unrealistic period of time (i.e. 1 hour)...especially if something caused an unexpected failure that caused the NetworkObject(s) in question to never be spawned...and definitely if they are in-scene placed NetworkObjects (i.e. level didn't finish processing or even load). Under those scenarios, it would make more sense to drop the pending/deferred messages since there is no point in having a client hold onto messages (indefinitely) for a never-to-be-instantiated-or-spawned NetworkObject.

Perhaps I am missing something...

zachstronaut commented 1 year ago

As I understood it, you just wanted the ability to send RPCs immediately from an in-scene placed NetworkObject on the server side without having to wait for the scene(s) to finish loading on the client side?

Yes! Which, as we've been discussing, in dependent on the SpawnTimeout value (or waiting for a scene load message).

especially if something caused an unexpected failure that caused the NetworkObject(s) in question to never be spawned...and definitely if they are in-scene placed NetworkObjects (i.e. level didn't finish processing or even load).

Another scenario: level is large and takes SpawnTimeout+1 seconds to load... which only happens sometimes depending on network. Now you've got intermittent silent failure.

NoelStephensUnity commented 1 year ago

Another scenario: level is large and takes SpawnTimeout+1 seconds to load... which only happens sometimes depending on network. Now you've got intermittent silent failure.

Right, so not knowing your project's boundaries (i.e. average maximum time it could take to load a scene) you could pick an abnormally high SpanTimeout of say 300 seconds/5 minutes (or an adjusted value relative to your project's maximum scene loading time)...

Although, if you are using scene management then you most likely would want to make your SpawnTimeout equal to the Load Scene Time Out value: image Since it would consider any client that took longer than that to load a scene as having "failed to load the scene" (typically it means the client has dropped or as you mentioned some other issue occurred).

If you are handling scene management yourself, then whatever time you have determined to be the "failed to load" period of time would be a pretty good setting for SpawnTimeout.

NoTuxNoBux commented 2 weeks ago

To get this back to the original post as I would also like what is described in the OP and to answer this question:

Is this still relevant? If so, can you please provide more information? We have OnNetworkSpawn on the objects themselves. Is there a reason you can't call whatever systems you have locally from this callback?

Yes. We do dependency injection using a container on MonoBehaviours. This is easy to do when spawning GameObjects since all you need to do is inject on them after spawning them, but with netcode this is only possible on the host currently as it is the one doing the spawning; clients also need to be able to inject (after the objects are replicated/spawned automatically by Netcode there), which currently requires e.g. resorting to static access to the container or similar in OnNetworkSpawn, or use something like GameObject.Find there to locate the container in the scene, both of which are suboptimal compared to a simple NetworkManager.OnNetworkObjectSpawned += (no) => container.InjectOn(no);.

I see that a list of network objects is already being maintained in the spawn manager, there is just no event that is fired when something is added to this list, which is all I need I think. Creating a MonoBehaviour that scans this list for changes through Update would probably also work but it's rather cumbersome and tacking things that run every frame on is not optimal for performance.