fix: notify networkmanager and networkobject not allowed #1777
Conversation
Renamed the method being invoked to NotifyUserNetworkObjectRemoved in order to be more explicit in what was happening. Updated comments
NoelStephensUnity
changed the title
adding a test to validate that checking for a NetworkObject, removing the NetworkObject, and notifying the user that a NetworkManager cannot exist if a NetworkObject resides on the GameObject. This also now checks children containing a NetworkObject. It does not need to check for a parent as this is checked after to invoking NetworkManagerCheckForParent.
| if (!EditorApplication.isUpdating) | ||
| { | ||
| Object.DestroyImmediate(networkObject); | ||
| var message = $"A {nameof(GameObject)} cannot have both a {nameof(NetworkManager)} and {nameof(NetworkObject)} assigned to it."; |
There was a problem hiding this comment.
This is cool, but what we don't tell users here is that you not only can't have a GO that has a NO and a NM, but you also cannot have children of a NM that have NOs. I would like the message to convey that, else people can have a setup that matches your error message but is still broken.
Bonus points if you flag the "children" scenario and then have a dedicated message for that one (vs. one catchall message)
There was a problem hiding this comment.
Didn't need to add additional code for the child checking. Just adjusted the language of the message.
com.unity.netcode.gameobjects/Tests/Editor/NetworkManagerConfigurationTests.cs
Show resolved
Hide resolved
| var networkManager = gameObject.AddComponent<NetworkManager>(); | ||
|
|
||
| // The error message we should expect | ||
| var messageToCheck = $"A {nameof(GameObject)} cannot have both a {nameof(NetworkManager)} and {nameof(NetworkObject)} assigned to it."; |
There was a problem hiding this comment.
IIRC we have banned the use of "checking for specific error messages", because it's so easy to forget that if you fix a message in one spot then the tests won't pass.
There was a problem hiding this comment.
Added method to return the error message.
com.unity.netcode.gameobjects/Tests/Editor/NetworkManagerConfigurationTests.cs
Show resolved
Hide resolved
NoelStephensUnity
deleted the
fix/notify-networkmanager-and-networkobject-not-allowed
branch
Adding a GameObject with a NetworkObject component as a child to the GameObject that is assigned the NetworkManager component will cause a soft synchronization error. It is advised to avoid assigning NetworkObjects to the parent GameObject with the NetworkManager component or to any child GameObject of the parent GameObject. This only applies to a GameObject with the NetworkManager component assigned to it.
Users should be notified of this while within the editor and should not allow them to proceed with any configuration as described above.
MTT-1598
MTT-1634
Changelog
com.unity.netcode.gameobjects
Testing and Documentation
Notification Displayed:
NetworkObject Removed