feat: Nested NetworkBehaviour Synchronization [MTT-5024] #2298
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This includes the changes to provide users with a way to synchronize NetworkBehaviours with custom data prior to their associated NetworkObject is spawned. This also includes some fixes that allows for NetworkObjects to fail, NetworkVariables to fail, and NetworkBehaviour synchronization to fail without impacting the rest of the synchronization process.
An updated version of this test that includes additional tests related to the refactoring. This test was not properly testing the entire serialization pipeline and it only tested for one NetworkVariable. If you added another NetworkVariable to the NetworkBehaviourWithNetworkVariables component the synchronization process would halt. There was also a bug in SceneEventData that would occur if a NetworkObject failed to be instantiated. The updates included resolve all of these issues for the original intention behind the test.
This include a nested NetworkTransform manual test to visually validate the nested NetworkTransform update.
minor camera related updates to the nested NetworkTransform manual test.
jeffreyrainy
reviewed
Nov 7, 2022
jeffreyrainy
reviewed
Nov 7, 2022
Minor updates to some manual test assets.
fixing some minor merge issues with the byte packing updates.
NoelStephensUnity
changed the title
NoelStephensUnity
changed the title
NoelStephensUnity
changed the title
NoelStephensUnity
changed the title
jeffreyrainy
approved these changes
Nov 16, 2022
There was a problem hiding this comment.
For the record, I have reservation around the direction and the NetworkObject nested restriction. I feel we walked pretty far away from the MLAPI design relying on NetworkVariables and RPC in a pretty uniform fashion.
That said, the PR shows thorough code, properly tested and addresses specific shortcomings. So, let's go with this, but consider marking a bit more clearly "OnSynchronize is optional and only meant to address complex cases where NetworkVariables and RPC fall short"
NoelStephensUnity
deleted the
fix/nested-networktransform-synchronization
branch
jakobbbb
pushed a commit
to GooseGirlGames/com.unity.netcode.gameobjects
that referenced
this pull request
Feb 22, 2023
…ologies#2298) * feat This includes the changes to provide users with a way to synchronize NetworkBehaviours with custom data prior to their associated NetworkObject is spawned. * fix and refactor This updates NetworkTransform so that it will properly synchronize when placed on nested NetworkBehaviours (i.e. their GameObject has no NetworkObject component). This also includes some fixes that allows for NetworkObjects to fail, NetworkVariables to fail, and NetworkBehaviour synchronization to fail without impacting the rest of the synchronization process. Minor fix for in-scene placed parenting under a non-NetworkObject to prevent from being added to the orphaned child list. * test Renamed NetworkObjectSceneSerializationTests to NetworkObjectSynchronizationTests. Added a more robust/wider range of tests, also added running host or server as well as added a basic OnSynchronize test. Added integration test to validate nested network transforms synchronize properly with nested GameObjects' transforms. Fixed an issue with late joined clients not registering all players in NetcodeIntegrationTest. * test manual This include a nested NetworkTransform manual test to visually validate the nested NetworkTransform update.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This refactor provides users a way for synchronizing nested NetworkBehaviours.
It also fixes an issue with the serialization pipeline where if a NetworkObject fails to synchronize it will not halt the entire synchronization process.
MTT-5024
Associated reports of this issue:
https://discordapp.com/channels/449263083769036810/1033497351475699712/1033497351475699712
https://discordapp.com/channels/449263083769036810/1034802372880375828/1034802372880375828
Changelog
NetworkBehaviour.OnSynchronizewhich is invoked during the initialNetworkObjectsynchronization process. This provides users the ability to include custom serialization information that will be applied to theNetworkBehaviourprior to theNetworkObjectbeing spawned.NetworkTransformcomponents nested under a parent with aNetworkObectcomponent (i.e. network prefab) would not have their associatedGameObject's transform synchronized.NetworkObjects that failed to instantiate could cause the entire synchronization pipeline to be disrupted/halted for a connecting client.NetworkObjects nested under aGameObjectwould be added to the orphaned children list causing continual console warning log messages.NetworkConfig.EnsureNetworkVariableLengthSafetyis disabledNetworkVariablefields do not write the additionalushortsize value (which helps to reduce the total synchronization message size), but when enabled it still writes the additionalushortvalue.Testing and Documentation