fix: NetworkManager always sent to the DDOL and cannot be nested

[NoelStephensUnity]

Networkmanage 's gameObject can't be a child of another gameObject. otherwise will show this issue:
Exception: Failed to find any loaded scene named "XXXX"
and everything related to the network will not working.

This addresses Github-Issue #1417 (described above) where users will now be provided notifications that the NetworkManager cannot be parented under another GameObject (nested). This PR also removes the DontDestroy property from the NetworkManager so that no NetworkManager instance can be destroyed by unloading a scene.

Development Standalone Build: The in-engine debug console displays the message that they cannot nest a NetworkManager
Editor Edit-Mode: A dialog box notifies the user that they cannot nest a NetworkManager and provides the option for the user to auto-fix or manually fix it
Editor Play-Mode: A dialog box just notifies the user that they cannot nest a NetworkManager

MTT-1732

PR Checklist

• Have you added a backport label (if needed)? For example, the type:backport-release-* label. After you backport the PR, the label changes to stat:backported-release-*.
• Have you updated the changelog? Each package has a CHANGELOG.md file.

Changelog

com.unity.netcode.gameobjects

• NetworkManager's GameObject is no longer allowed to be nested under one or more GameObject(s).
• NetworkManager DontDestroy property was removed and now NetworkManager always is migrated into the DontDestroyOnLoad scene.

Testing and Documentation

• Includes editor and runtime tests to validate the fix

[mattwalsh-unity]

Poor NetworkManager is so large already....could this live in its own file? Preferably in the Editor folder?

[NoelStephensUnity]

I will see what I can do about removing the editor related code from NetworkManager into some other class within the NGO editor assembly.

[mattwalsh-unity]

Ok, I see the update, but originally imagined like all this code from line 1705 to 1785 would be hoisted to NetworkManagerHelper.cs

[mattwalsh-unity]

Looks good if we can hoist that new editor checking code into another file per my comment. Thoughts?

[WhippetsAintDogs]

@NoelStephensUnity

The pop-up warning logic doesn't scale in bigger projects. The version 1.0.0-pre.5 added this check for nested NetworkManager that performs a costly Resources.FindObjectsOfTypeAll<NetworkManager>(); on EditorApplication.hierarchyChanged which slows down our editor to 5 FPS and cause annoying 30-40s Inspector.WindowRepaint intermittent editor freezes.

Please consider to remove / change it ! 🙏

[vklubkov]

Just encountered that this check clashes with Zenject(Extenject) prefab instantiation in Editor. It happens because in Editor Zenject temporarily reparents instantiated prefabs. There are probably some workarounds but instantiation of prefabs via Zenject is a common use case. So far the only bad thing is that there's an error message in the console - luckily Zenject doesn't reparent instantiated objects in builds to anything except null, and luckily the popup message is avoided by runtime instantiation.

What really happens under the hood is:

1. Running in Editor
   • Zenject creates inactive parent to initially parent instantiated gameobjects under it
   • Zenject instantiates prefab under that parent
   • Zenject deactivates the instantiated GO (SetActive(false))
   • Zenject reparents the prefab to ContextTransform
   • NetworkManager OnTransformParentChanged is triggered by that and the message ("NetworkManager(Clone) is nested under ProjectContext(Clone). NetworkManager cannot be nested.") is printed
   • Zenject reparents the GO to null
   • NetworkManager OnTransformParentChanged is triggered by that and NetworkManager isn't parented anymore.
   • Zenject activates the instantiated GO (SetActive(true))
   • NetworkManager Awakes
   • NetworkManager OnEnable is triggered
   • NetworkManagerCheckForParent is called from there and NetworkManager isn't parented at this time
   • NetworkManager successfully calls DontDestroyOnLoad
   • NetworkManager Start is called
2. Running in Build
   • Zenject deactivates the prefab (SetActive(false))
   • Zenject instantiates prefab under ContextTransform
   • Zenject activates the instantiated GO (SetActive(true))
   • Zenject reparents the GO to null
   • NetworkManager OnTransformParentChanged is triggered by that and NetworkManager isn't parented at this time
   • NetworkManager Awakes
   • NetworkManager OnEnable is triggered
   • NetworkManagerCheckForParent is called from there and NetworkManager isn't parented at this time
   • NetworkManager successfully calls DontDestroyOnLoad
   • NetworkManager Start is called

I am not sure why Zenject does that in two steps in Editor. But checking for parent before the object is fully constructed looks somewhat strange to me either.

Repro setup:

ProjectContext:

or SceneContext:

Installer:

Prefab:

Both ProjectContext and SceneContext result in error message because of temporary reparenting:

But NetworkManager is properly parented in the end:

[NoelStephensUnity]

@MARKED-ONE
It does indeed look like the issue is the reparenting of the NetworkManager while in the editor which is directly tied to the NetworkManager being in a prefab. I am guessing that you have specific properties that you want to preserve and as such you are doing that within the prefab itself?
Just out of curiosity, have you tried using FromNewComponentOnNewGameObject and InstantiatedCallback to then configure the NetworkManager? So, you could still have a reference to your prefab in your NetworkInstaller and upon the NetworkManager being instantiate just copy the prefab NetworkManager.NetworkConfig to your newly instantiated NetworkManager. For the NetworkTransport component (i.e. UnityTransport), you might think about making that a separate prefab that NetworkInstaller has a reference to so that you can instantiate and place it under the newly instantiated NetworkManager and then assign the NetworkTransport component (i.e. UnityTransport) to: NetworkManager.NetworkConfig.NetworkTransport. (of the newly instantiated NetworkManager)

I only am suggesting something along those lines as a work around to prevent the editor only check for reparenting NetworkManager. While we are looking at ways to improve NetworkManager in general, most likely this editor only check will persist for the next few updates and so you might look at alternate approaches other than using the FromComponentInPrefab.

As a reminder, we have not tested NGO on all third party solutions/packages and with Zenject you could run into more complexities if using dependency injection with things like NetworkBehaviour derived components. NGO v1.x uses ILPP to build internal tables which provide maps to various things (i.e. RPCs, Network Serialization, NetworkVariables, etc) and methods which depending upon the order of operations of Zenject relative to NGO it could result in other complexities not accounted for in NGO v1.x versions.

[WhippetsAintDogs]

Hi @MARKED-ONE !

In our project, we call this method in our installer as a workaround to prevent the error :

private void BindNetworkManagerWithoutReparenting()
    {
        networkManagerPrefab.gameObject.SetActive(false);

        Container.Bind<NetworkManager>().FromComponentInNewPrefab(networkManagerPrefab).AsSingle().OnInstantiated<NetworkManager>((_, networkManager) =>
        {
            networkManager.gameObject.SetActive(true);
            networkManagerPrefab.gameObject.SetActive(true);
        }).NonLazy();
    }

Hope it can help you 😄

[vklubkov]

@NoelStephensUnity yes, my idea was to have a prefab with configured network setup, then simply spawn that prefab via Zenject and inject the NetworkManager wherever it is needed.

I'm still new to Netcode, so only exploring what is the best way to pair it with Zenject. This problem just seemed like a potential bug to me.

Also, I tried your workaround and can confirm that it works without issues.

@WhippetsAintDogs clever solution! 😄I will probably choose this one as it doesn't require multiple prefabs.

I personally was considering keeping everything as is because I can definitely live with the error in console, at least for now. But I really prefer 0 errors 0 warnings practice.

And thank you both for the helpful tips!

[NoelStephensUnity]

@WhippetsAintDogs
Nice work around!
Will have to remember this one.
I haven't had the chance to test various third party solutions like Zenject and so I was trying to avoid the issue entirely. However, it is clever to disable the GameObject of the prefab prior to making the clone and injecting it and upon being injected re-enabling the clone and the prefab itself. It definitely avoids the check that way and thus the error. Nice! 💯