Skip to content

Commit cd6fd5b

Browse files
fix: Exception with disabled GameObjects and NetworkTransforms (#3243)
* fix: Exception with disabled GameObjects and NetworkTransforms * Add CHANGELOG * Update naming and code comments to be clearer * fix Minor adjustment as it looks like DAHost might not have been properly forwarding messages of NetworkTransformMessages when the NetworkObject was hidden from it. * fix Some minor tweaks for extracting the reliability used. * style updating the test LogAssert.Expect string used. * update + style 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) --------- Co-authored-by: NoelStephensUnity <[email protected]>
1 parent 6a4f718 commit cd6fd5b

File tree

4 files changed

+176
-11
lines changed

4 files changed

+176
-11
lines changed

com.unity.netcode.gameobjects/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
1212

1313
### Fixed
1414

15+
- Fixed exception being thrown when a `GameObject` with an associated `NetworkTransform` is disabled. (#3243)
1516
- Fixed `NetworkObject.DeferDespawn` to respect the `DestroyGameObject` parameter. (#3219)
1617
- Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3212)
1718
- Fixed issue where a spawned `NetworkObject` that was registered with a prefab handler and owned by a client would invoke destroy more than once on the host-server side if the client disconnected while the `NetworkObject` was still spawned. (#3200)

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkTransformMessage.cs

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,21 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
7171
if (isSpawnedLocally)
7272
{
7373
networkObject = networkManager.SpawnManager.SpawnedObjects[networkObjectId];
74+
if (networkObject.ChildNetworkBehaviours.Count <= networkBehaviourId || networkObject.ChildNetworkBehaviours[networkBehaviourId] == null)
75+
{
76+
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.");
77+
return false;
78+
}
79+
7480
// Get the target NetworkTransform
75-
NetworkTransform = networkObject.ChildNetworkBehaviours[networkBehaviourId] as NetworkTransform;
81+
var transform = networkObject.ChildNetworkBehaviours[networkBehaviourId] as NetworkTransform;
82+
if (transform == null)
83+
{
84+
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.");
85+
return false;
86+
}
87+
88+
NetworkTransform = transform;
7689
isServerAuthoritative = NetworkTransform.IsServerAuthoritative();
7790
ownerAuthoritativeServerSide = !isServerAuthoritative && networkManager.IsServer;
7891

@@ -81,8 +94,20 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
8194
}
8295
else
8396
{
84-
// Deserialize the state
85-
reader.ReadNetworkSerializableInPlace(ref State);
97+
ownerAuthoritativeServerSide = networkManager.DAHost;
98+
// If we are the DAHost and the NetworkObject is hidden from the host we still need to forward this message.
99+
if (ownerAuthoritativeServerSide)
100+
{
101+
// We need to deserialize the state to our local State property so we can extract the reliability used.
102+
reader.ReadNetworkSerializableInPlace(ref State);
103+
// Fall through to act like a proxy for this message.
104+
}
105+
else
106+
{
107+
// Otherwise we can error out because we either shouldn't be receiving this message.
108+
Debug.LogError($"[{nameof(NetworkTransformMessage)}][Invalid] Target NetworkObject ({networkObjectId}) does not exist!");
109+
return false;
110+
}
86111
}
87112

88113
unsafe
@@ -106,12 +131,6 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
106131
ByteUnpacker.ReadValueBitPacked(reader, out targetId);
107132
targetIds[i] = targetId;
108133
}
109-
110-
if (!isSpawnedLocally)
111-
{
112-
// If we are the DAHost and the NetworkObject is hidden from the host we still need to forward this message
113-
ownerAuthoritativeServerSide = networkManager.DAHost && !isSpawnedLocally;
114-
}
115134
}
116135

117136
var ownerClientId = (ulong)0;
@@ -132,7 +151,10 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
132151
ownerClientId = context.SenderId;
133152
}
134153

135-
var networkDelivery = State.IsReliableStateUpdate() ? NetworkDelivery.ReliableSequenced : NetworkDelivery.UnreliableSequenced;
154+
// Depending upon whether it is spawned locally or not, get the deserialized state
155+
var stateToUse = NetworkTransform != null ? NetworkTransform.InboundState : State;
156+
// Determine the reliability used to send the message
157+
var networkDelivery = stateToUse.IsReliableStateUpdate() ? NetworkDelivery.ReliableSequenced : NetworkDelivery.UnreliableSequenced;
136158

137159
// Forward the state update if there are any remote clients to foward it to
138160
if (networkManager.ConnectionManager.ConnectedClientsList.Count > (networkManager.IsHost ? 2 : 1))
@@ -160,7 +182,6 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
160182
{
161183
continue;
162184
}
163-
164185
networkManager.MessageManager.SendMessage(ref currentMessage, networkDelivery, clientId);
165186
}
166187
// Dispose of the reader used for forwarding
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
using System.Collections;
2+
using Unity.Netcode.Components;
3+
using Unity.Netcode.TestHelpers.Runtime;
4+
using UnityEngine;
5+
using UnityEngine.TestTools;
6+
7+
8+
namespace Unity.Netcode.RuntimeTests
9+
{
10+
internal class NetworkTransformErrorTests : NetcodeIntegrationTest
11+
{
12+
protected override int NumberOfClients => 1;
13+
14+
private GameObject m_AuthorityPrefab;
15+
private GameObject m_NonAuthorityPrefab;
16+
17+
private HostAndClientPrefabHandler m_HostAndClientPrefabHandler;
18+
19+
public class EmptyNetworkBehaviour : NetworkBehaviour { }
20+
21+
/// <summary>
22+
/// PrefabHandler that tracks and separates the client GameObject from the host GameObject.
23+
/// Allows independent management of client and host game world while still instantiating NetworkObjects as expected.
24+
/// </summary>
25+
private class HostAndClientPrefabHandler : INetworkPrefabInstanceHandler
26+
{
27+
/// <summary>
28+
/// The registered prefab is the prefab the networking stack is instantiated with.
29+
/// Registering the prefab simulates the prefab that exists on the authority.
30+
/// </summary>
31+
private readonly GameObject m_RegisteredPrefab;
32+
33+
/// <summary>
34+
/// Mocks the registered prefab changing on the non-authority after registration.
35+
/// Allows testing situations mismatched GameObject state between the authority and non-authority.
36+
/// </summary>
37+
private readonly GameObject m_InstantiatedPrefab;
38+
39+
public HostAndClientPrefabHandler(GameObject authorityPrefab, GameObject nonAuthorityPrefab)
40+
{
41+
m_RegisteredPrefab = authorityPrefab;
42+
m_InstantiatedPrefab = nonAuthorityPrefab;
43+
}
44+
45+
/// <summary>
46+
/// Returns the prefab that will mock the instantiated prefab not matching the registered prefab
47+
/// </summary>
48+
public NetworkObject Instantiate(ulong ownerClientId, Vector3 position, Quaternion rotation)
49+
{
50+
return Object.Instantiate(m_InstantiatedPrefab).GetComponent<NetworkObject>();
51+
}
52+
53+
public void Destroy(NetworkObject networkObject)
54+
{
55+
Object.Destroy(networkObject.gameObject);
56+
}
57+
58+
public void Register(NetworkManager networkManager)
59+
{
60+
// Register the version that will be spawned by the authority (i.e. Host)
61+
networkManager.PrefabHandler.AddHandler(m_RegisteredPrefab, this);
62+
}
63+
}
64+
65+
/// <summary>
66+
/// Creates a GameObject and sets the transform parent to the given transform
67+
/// Adds a component of the given type to the GameObject
68+
/// </summary>
69+
private static void AddChildToNetworkObject<T>(Transform transform) where T : Component
70+
{
71+
var gameObj = new GameObject();
72+
gameObj.transform.parent = transform;
73+
gameObj.AddComponent<T>();
74+
}
75+
76+
protected override void OnServerAndClientsCreated()
77+
{
78+
// Create a prefab that has many child NetworkBehaviours
79+
m_AuthorityPrefab = CreateNetworkObjectPrefab("AuthorityPrefab");
80+
AddChildToNetworkObject<EmptyNetworkBehaviour>(m_AuthorityPrefab.transform);
81+
AddChildToNetworkObject<EmptyNetworkBehaviour>(m_AuthorityPrefab.transform);
82+
AddChildToNetworkObject<NetworkTransform>(m_AuthorityPrefab.transform);
83+
84+
// Create a second prefab with only one NetworkBehaviour
85+
// This simulates the GameObjects on the other NetworkBehaviours being disabled
86+
m_NonAuthorityPrefab = CreateNetworkObjectPrefab("NonAuthorityPrefab");
87+
AddChildToNetworkObject<NetworkTransform>(m_NonAuthorityPrefab.transform);
88+
89+
// Create and register a prefab handler
90+
// The prefab handler will behave as if the GameObjects have been disabled on the non-authority client
91+
m_HostAndClientPrefabHandler = new HostAndClientPrefabHandler(m_AuthorityPrefab, m_NonAuthorityPrefab);
92+
m_HostAndClientPrefabHandler.Register(m_ServerNetworkManager);
93+
foreach (var client in m_ClientNetworkManagers)
94+
{
95+
m_HostAndClientPrefabHandler.Register(client);
96+
}
97+
98+
base.OnServerAndClientsCreated();
99+
}
100+
101+
/// <summary>
102+
/// Validates the fix where <see cref="NetworkTransformMessage"/> would throw an exception
103+
/// if a user sets a <see cref="GameObject"/> with one or more <see cref="NetworkBehaviour"/> components
104+
/// to inactive.
105+
/// </summary>
106+
[UnityTest]
107+
public IEnumerator DisabledGameObjectErrorTest()
108+
{
109+
var instance = SpawnObject(m_AuthorityPrefab, m_ServerNetworkManager);
110+
var networkObjectInstance = instance.GetComponent<NetworkObject>();
111+
var networkTransformInstance = instance.GetComponentInChildren<NetworkTransform>();
112+
113+
yield return WaitForConditionOrTimeOut(() => ObjectSpawnedOnAllClients(networkObjectInstance.NetworkObjectId));
114+
AssertOnTimeout("Timed out waiting for object to spawn!");
115+
116+
var errorMessage = $"[Netcode] {nameof(NetworkBehaviour)} index {networkTransformInstance.NetworkBehaviourId} was out of bounds for {m_NonAuthorityPrefab.name}(Clone). " +
117+
$"{nameof(NetworkBehaviour)}s must be the same, and in the same order, between server and client.";
118+
LogAssert.Expect(LogType.Error, errorMessage);
119+
errorMessage = $"[{nameof(NetworkTransformMessage)}][Invalid] Targeted {nameof(NetworkTransform)}, {nameof(NetworkBehaviour.NetworkBehaviourId)} " +
120+
$"({networkTransformInstance.NetworkBehaviourId}), does not exist! Make sure you are not spawning {nameof(NetworkObject)}s with disabled {nameof(GameObject)}s that have " +
121+
$"{nameof(NetworkBehaviour)} components on them.";
122+
LogAssert.Expect(LogType.Error, errorMessage);
123+
124+
yield return new WaitForSeconds(0.3f);
125+
}
126+
127+
private bool ObjectSpawnedOnAllClients(ulong networkObjectId)
128+
{
129+
foreach (var client in m_ClientNetworkManagers)
130+
{
131+
if (!client.SpawnManager.SpawnedObjects.ContainsKey(networkObjectId))
132+
{
133+
return false;
134+
}
135+
}
136+
return true;
137+
}
138+
}
139+
140+
}

com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformErrorTests.cs.meta

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)