-
Notifications
You must be signed in to change notification settings - Fork 438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Exception with disabled GameObjects and NetworkTransforms #3243
fix: Exception with disabled GameObjects and NetworkTransforms #3243
Conversation
…/com.unity.netcode.gameobjects into fix/mttb-959-exception-with-disabled-gameobjects
Just keeping the established pattern of using "nameof" within a string. Also added the NetworkTransform's NetworkBehaviourId value as opposed to hard coding the value. (minor tweaks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to add a fix for another issue I noticed while reviewing your PR (not due to your PR but that already existed).
Did a minor cosmetic update to the error message generation for LogAssert.Expect.
Other than that, it looks good to me!
As long as my fix didn't break something else! 😝
@@ -71,8 +71,21 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int | |||
if (isSpawnedLocally) | |||
{ | |||
networkObject = networkManager.SpawnManager.SpawnedObjects[networkObjectId]; | |||
if (networkObject.ChildNetworkBehaviours.Count <= networkBehaviourId || networkObject.ChildNetworkBehaviours[networkBehaviourId] == null) | |||
{ | |||
Debug.LogError($"[{nameof(NetworkTransformMessage)}][Invalid] Targeted {nameof(NetworkTransform)}, {nameof(NetworkBehaviour.NetworkBehaviourId)} ({networkBehaviourId}), does not exist! Make sure you are not spawning {nameof(NetworkObject)}s with disabled {nameof(GameObject)}s that have {nameof(NetworkBehaviour)} components on them."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice wordsmithing...
var transform = networkObject.ChildNetworkBehaviours[networkBehaviourId] as NetworkTransform; | ||
if (transform == null) | ||
{ | ||
Debug.LogError($"[{nameof(NetworkTransformMessage)}][Invalid] Targeted {nameof(NetworkTransform)}, {nameof(NetworkBehaviour.NetworkBehaviourId)} ({networkBehaviourId}), does not exist! Make sure you are not spawning {nameof(NetworkObject)}s with disabled {nameof(GameObject)}s that have {nameof(NetworkBehaviour)} components on them."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➕
@@ -81,8 +94,20 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int | |||
} | |||
else | |||
{ | |||
// Deserialize the state | |||
reader.ReadNetworkSerializableInPlace(ref State); | |||
ownerAuthoritativeServerSide = networkManager.DAHost; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While reviewing your PR, I noticed a bug in DAHost mode.
Just including the fix for that here.
@@ -132,7 +151,10 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int | |||
ownerClientId = context.SenderId; | |||
} | |||
|
|||
var networkDelivery = State.IsReliableStateUpdate() ? NetworkDelivery.ReliableSequenced : NetworkDelivery.UnreliableSequenced; | |||
// Depending upon whether it is spawned locally or not, get the deserialized state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the 2nd part of the fix where State was never being set and it wasn't adjusting based on whether the host, server, or DAHost had a NetworkTransform instance.
yield return WaitForConditionOrTimeOut(() => ObjectSpawnedOnAllClients(networkObjectInstance.NetworkObjectId)); | ||
AssertOnTimeout("Timed out waiting for object to spawn!"); | ||
|
||
var errorMessage = $"[Netcode] {nameof(NetworkBehaviour)} index {networkTransformInstance.NetworkBehaviourId} was out of bounds for {m_NonAuthorityPrefab.name}(Clone). " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor adjustments.
MTTB-959
Changelog
GameObject
associated with aNetworkBehaviour
. Do not throw an excpetion.Testing and Documentation