Skip to content

Commit ba2cb5f

Browse files
EmandMEmandM
committed
fix: NullReferenceExceptions in spawn path
1 parent 7800803 commit ba2cb5f

10 files changed

Lines changed: 234 additions & 186 deletions

File tree

‎com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1218,7 +1218,7 @@ internal void ApprovedPlayerSpawn(ulong clientId, uint playerPrefabHash)
12181218

12191219
var message = new CreateObjectMessage
12201220
{
1221-
ObjectInfo = ConnectedClients[clientId].PlayerObject.Serialize(clientPair.Key),
1221+
ObjectInfo = ConnectedClients[clientId].PlayerObject.SerializeSpawnedObject(clientPair.Key),
12221222
IncludesSerializedObject = true,
12231223
};
12241224

‎com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Diagnostics.CodeAnalysis;
34
using System.Linq;
45
using System.Runtime.CompilerServices;
56
using System.Text;
@@ -3277,6 +3278,7 @@ public void Deserialize(FastBufferReader reader)
32773278
var readSize = 0;
32783279
readSize += HasTransform ? FastBufferWriter.GetWriteSize<TransformData>() : 0;
32793280
readSize += FastBufferWriter.GetWriteSize<int>();
3281+
readSize += FastBufferWriter.GetWriteSize<int>();
32803282

32813283
// Try to begin reading the remaining bytes
32823284
if (!reader.TryBeginRead(readSize))
@@ -3295,7 +3297,7 @@ public void Deserialize(FastBufferReader reader)
32953297

32963298
// Read the size of the remaining synchronization data
32973299
// This data will be read in AddSceneObject()
3298-
reader.ReadValueSafe(out SynchronizationDataSize);
3300+
reader.ReadValue(out SynchronizationDataSize);
32993301
}
33003302
}
33013303

@@ -3369,7 +3371,12 @@ internal void SynchronizeNetworkBehaviours<T>(ref BufferSerializer<T> serializer
33693371
}
33703372
}
33713373

3372-
internal SerializedObject Serialize(ulong targetClientId = NetworkManager.ServerClientId, bool syncObservers = false)
3374+
/// <summary>
3375+
/// Creates a <see cref="SerializedObject"/> on an authority client.
3376+
/// Used to synchronize <see cref="NetworkObject"/> state to a non-authority client.
3377+
/// </summary>
3378+
/// <remarks>This function is the authority mirror of <see cref="DeserializeAndSpawnObject"/></remarks>
3379+
internal SerializedObject SerializeSpawnedObject(ulong targetClientId = NetworkManager.ServerClientId, bool syncObservers = false)
33733380
{
33743381
var obj = new SerializedObject
33753382
{
@@ -3444,15 +3451,17 @@ internal SerializedObject Serialize(ulong targetClientId = NetworkManager.Server
34443451
}
34453452

34463453
/// <summary>
3447-
/// Used to deserialize a serialized <see cref="SerializedObject"/> which occurs
3448-
/// when the client is approved or during a scene transition
3454+
/// Does a non-authority local spawn of a given <see cref="SerializedObject"/>.
3455+
/// This occurs when the client is approved, a new object is spawned by an authority, or during a scene transition.
34493456
/// </summary>
3450-
/// <param name="serializedObject">Deserialized scene object data</param>
3451-
/// <param name="reader">FastBufferReader for the NetworkVariable data</param>
3452-
/// <param name="networkManager">NetworkManager instance</param>
3457+
/// <remarks>This function is the non-authority mirror of <see cref="SerializeSpawnedObject"/></remarks>
3458+
/// <param name="serializedObject">Deserialized data received from the authority for this <see cref="NetworkObject"/></param>
3459+
/// <param name="reader">FastBufferReader for any additional data sent with this object on spawn.</param>
3460+
/// <param name="networkManager">NetworkManager instance.</param>
34533461
/// <param name="invokedByMessage">will be true if invoked by CreateObjectMessage</param>
34543462
/// <returns>The deserialized NetworkObject or null if deserialization failed</returns>
3455-
internal static NetworkObject Deserialize(in SerializedObject serializedObject, FastBufferReader reader, NetworkManager networkManager, bool invokedByMessage = false)
3463+
[return: MaybeNull]
3464+
internal static NetworkObject DeserializeAndSpawnObject(in SerializedObject serializedObject, FastBufferReader reader, NetworkManager networkManager, bool invokedByMessage = false)
34563465
{
34573466
var endOfSynchronizationData = reader.Position + serializedObject.SynchronizationDataSize;
34583467

@@ -3479,7 +3488,8 @@ internal static NetworkObject Deserialize(in SerializedObject serializedObject,
34793488
{
34803489
if (networkManager.LogLevel <= LogLevel.Normal)
34813490
{
3482-
NetworkLog.LogWarning($"[{networkObject.name}][Deserialize][{nameof(NetworkBehaviour)}Synchronization][Size mismatch] Expected: {endOfSynchronizationData} Currently At: {reader.Position}!");
3491+
var networkObjectName = networkObject != null ? networkObject.name : "null";
3492+
NetworkLog.LogWarning($"[{networkObjectName}][Deserialize][{nameof(NetworkBehaviour)}Synchronization][Size mismatch] Expected: {endOfSynchronizationData} Currently At: {reader.Position}!");
34833493
}
34843494
reader.Seek(endOfSynchronizationData);
34853495
}

‎com.unity.netcode.gameobjects/Runtime/Messaging/ILPPMessageProvider.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ internal static Dictionary<Type, NetworkMessageTypes> GetMessageTypesMap()
148148
[InitializeOnLoadMethod]
149149
public static void NotifyOnPlayStateChange()
150150
{
151+
EditorApplication.playModeStateChanged -= OnPlayModeStateChanged;
151152
EditorApplication.playModeStateChanged += OnPlayModeStateChanged;
152153
}
153154

‎com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ public void Serialize(FastBufferWriter writer, int targetVersion)
160160
{
161161
sobj.AddObserver(OwnerClientId);
162162
// In distributed authority mode, we send the currently known observers of each NetworkObject to the client being synchronized.
163-
var serializedObject = sobj.Serialize(OwnerClientId, IsDistributedAuthority);
163+
var serializedObject = sobj.SerializeSpawnedObject(OwnerClientId, IsDistributedAuthority);
164164
serializedObject.Serialize(writer);
165165
++sceneObjectCount;
166166
}
@@ -344,7 +344,7 @@ public void Handle(ref NetworkContext context)
344344
{
345345
var serializedObject = new NetworkObject.SerializedObject();
346346
serializedObject.Deserialize(m_ReceivedSceneObjectData);
347-
NetworkObject.Deserialize(serializedObject, m_ReceivedSceneObjectData, networkManager);
347+
NetworkObject.DeserializeAndSpawnObject(serializedObject, m_ReceivedSceneObjectData, networkManager);
348348
}
349349

350350
if (networkManager.DistributedAuthorityMode && networkManager.AutoSpawnPlayerPrefabClientSide)

‎com.unity.netcode.gameobjects/Runtime/Messaging/Messages/CreateObjectMessage.cs‎

Lines changed: 21 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Linq;
22
using System.Runtime.CompilerServices;
3+
using Unity.Netcode.Logging;
34

45
namespace Unity.Netcode
56
{
@@ -171,33 +172,40 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
171172
{
172173
if (!networkManager.DistributedAuthorityMode)
173174
{
174-
networkObject = NetworkObject.Deserialize(serializedObject, networkVariableData, networkManager);
175+
networkObject = NetworkObject.DeserializeAndSpawnObject(serializedObject, networkVariableData, networkManager);
176+
if (networkObject == null)
177+
{
178+
networkManager.Log.ErrorServer(new Context(LogLevel.Developer, $"Failed to deserialize {nameof(NetworkObject)}.").AddInfo(nameof(NetworkObject.GlobalObjectIdHash), serializedObject.Hash).AddInfo(nameof(NetworkObject.NetworkObjectId), serializedObject.NetworkObjectId));
179+
return;
180+
}
175181
}
176182
else
177183
{
178184
var hasObserverIdList = observerIds != null && observerIds.Length > 0;
179185
var hasNewObserverIdList = newObserverIds != null && newObserverIds.Length > 0;
180186
// Depending upon visibility of the NetworkObject and the client in question, it could be that
181187
// this client already has visibility of this NetworkObject
182-
if (networkManager.SpawnManager.SpawnedObjects.ContainsKey(serializedObject.NetworkObjectId))
188+
if (networkManager.SpawnManager.SpawnedObjects.TryGetValue(serializedObject.NetworkObjectId, out networkObject))
183189
{
184-
// If so, then just get the local instance
185-
networkObject = networkManager.SpawnManager.SpawnedObjects[serializedObject.NetworkObjectId];
186-
187190
// This should not happen, logging error just in case
188191
if (hasNewObserverIdList && newObserverIds.Contains(networkManager.LocalClientId))
189192
{
190193
NetworkLog.LogErrorServer($"[{nameof(CreateObjectMessage)}][Duplicate-Broadcast] Detected duplicated object creation for {serializedObject.NetworkObjectId}!");
191194
}
192-
else // Trap to make sure the owner is not receiving any messages it sent
193-
if (networkManager.CMBServiceConnection && networkManager.LocalClientId == networkObject.OwnerClientId)
194-
{
195-
NetworkLog.LogWarning($"[{nameof(CreateObjectMessage)}][Client-{networkManager.LocalClientId}][Duplicate-CreateObjectMessage][Client Is Owner] Detected duplicated object creation for {networkObject.name}-{serializedObject.NetworkObjectId}!");
196-
}
195+
// Trap to make sure the owner is not receiving any messages it sent
196+
else if (networkManager.CMBServiceConnection && networkManager.LocalClientId == networkObject.OwnerClientId)
197+
{
198+
NetworkLog.LogWarning($"[{nameof(CreateObjectMessage)}][Client-{networkManager.LocalClientId}][Duplicate-CreateObjectMessage][Client Is Owner] Detected duplicated object creation for {networkObject.name}-{serializedObject.NetworkObjectId}!");
199+
}
197200
}
198201
else
199202
{
200-
networkObject = NetworkObject.Deserialize(serializedObject, networkVariableData, networkManager, true);
203+
networkObject = NetworkObject.DeserializeAndSpawnObject(serializedObject, networkVariableData, networkManager, true);
204+
if (networkObject == null)
205+
{
206+
networkManager.Log.ErrorServer(new Context(LogLevel.Developer, $"Failed to deserialize {nameof(NetworkObject)}.").AddInfo(nameof(NetworkObject.GlobalObjectIdHash), serializedObject.Hash).AddInfo(nameof(NetworkObject.NetworkObjectId), serializedObject.NetworkObjectId));
207+
return;
208+
}
201209
}
202210

203211
// DA - NGO CMB SERVICE NOTES:
@@ -214,27 +222,6 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
214222
// Mock CMB Service and forward to all clients
215223
if (networkManager.DAHost)
216224
{
217-
// DA - NGO CMB SERVICE NOTES:
218-
// (*** See above notes fist ***)
219-
// If it is a player object freshly spawning and one or more clients all connect at the exact same time (i.e. received on effectively
220-
// the same frame), then we need to check the observers list to make sure all players are visible upon first spawning. At a later date,
221-
// for area of interest we will need to have some form of follow up "observer update" message to cull out players not within each
222-
// player's AOI.
223-
if (networkObject.IsPlayerObject && hasNewObserverIdList && clientList.Count != observerIds.Length)
224-
{
225-
// For same-frame newly spawned players that might not be aware of all other players, update the player's observer
226-
// list.
227-
observerIds = clientList.ToArray();
228-
}
229-
230-
var createObjectMessage = new CreateObjectMessage()
231-
{
232-
ObjectInfo = serializedObject,
233-
m_ReceivedNetworkVariableData = networkVariableData,
234-
ObserverIds = hasObserverIdList ? observerIds : null,
235-
NetworkObjectId = networkObject.NetworkObjectId,
236-
IncludesSerializedObject = true,
237-
};
238225
foreach (var clientId in clientList)
239226
{
240227
// DA - NGO CMB SERVICE NOTES:
@@ -250,16 +237,12 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
250237
// If this included a list of new observers and the targeted clientId is one of the observers, then send the serialized data.
251238
// Otherwise, the targeted clientId has already has visibility (i.e. it is already spawned) and so just send the updated
252239
// observers list to that client's instance.
253-
createObjectMessage.IncludesSerializedObject = hasNewObserverIdList && newObserverIds.Contains(clientId);
254-
255240
networkManager.SpawnManager.SendSpawnCallForObject(clientId, networkObject);
256241
}
257242
}
258243
}
259-
if (networkObject != null)
260-
{
261-
networkManager.NetworkMetrics.TrackObjectSpawnReceived(senderId, networkObject, messageSize);
262-
}
244+
245+
networkManager.NetworkMetrics.TrackObjectSpawnReceived(senderId, networkObject, messageSize);
263246
}
264247
catch (System.Exception ex)
265248
{

‎com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventData.cs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ private void WriteSceneSynchronizationData(FastBufferWriter writer)
622622
{
623623
var noStart = writer.Position;
624624
// In distributed authority mode, we send the currently known observers of each NetworkObject to the client being synchronized.
625-
var serializedObject = networkObject.Serialize(TargetClientId, distributedAuthority);
625+
var serializedObject = networkObject.SerializeSpawnedObject(TargetClientId, distributedAuthority);
626626

627627
serializedObject.Serialize(writer);
628628
var noStop = writer.Position;
@@ -698,7 +698,7 @@ private void SerializeScenePlacedObjects(FastBufferWriter writer)
698698
foreach (var objectToSync in m_NetworkObjectsSync)
699699
{
700700
// Serialize the NetworkObject
701-
var serializedObject = objectToSync.Serialize(TargetClientId, distributedAuthority);
701+
var serializedObject = objectToSync.SerializeSpawnedObject(TargetClientId, distributedAuthority);
702702
serializedObject.Serialize(writer);
703703
numberOfObjects++;
704704
}
@@ -875,9 +875,9 @@ internal void DeserializeScenePlacedObjects()
875875
m_NetworkManager.SceneManager.SetTheSceneBeingSynchronized(serializedObject.NetworkSceneHandle);
876876
}
877877

878-
var networkObject = NetworkObject.Deserialize(serializedObject, m_InternalBuffer, m_NetworkManager);
878+
var networkObject = NetworkObject.DeserializeAndSpawnObject(serializedObject, m_InternalBuffer, m_NetworkManager);
879879

880-
if (serializedObject.IsSceneObject)
880+
if (serializedObject.IsSceneObject && networkObject != null)
881881
{
882882
sceneObjects.Add(networkObject);
883883
}
@@ -1101,7 +1101,11 @@ internal void SynchronizeSceneNetworkObjects(NetworkManager networkManager)
11011101
{
11021102
m_NetworkManager.SceneManager.SetTheSceneBeingSynchronized(serializedObject.NetworkSceneHandle);
11031103
}
1104-
var spawnedNetworkObject = NetworkObject.Deserialize(serializedObject, m_InternalBuffer, networkManager);
1104+
var spawnedNetworkObject = NetworkObject.DeserializeAndSpawnObject(serializedObject, m_InternalBuffer, networkManager);
1105+
if (spawnedNetworkObject == null)
1106+
{
1107+
continue;
1108+
}
11051109

11061110
var noStop = m_InternalBuffer.Position;
11071111
if (EnableSerializationLogs)
@@ -1110,12 +1114,9 @@ internal void SynchronizeSceneNetworkObjects(NetworkManager networkManager)
11101114
LogArray(m_InternalBuffer.ToArray(), noStart, noStop, builder);
11111115
}
11121116
// If we failed to deserialize the NetworkObject then don't add null to the list
1113-
if (spawnedNetworkObject != null)
1117+
if (!m_NetworkObjectsSync.Contains(spawnedNetworkObject))
11141118
{
1115-
if (!m_NetworkObjectsSync.Contains(spawnedNetworkObject))
1116-
{
1117-
m_NetworkObjectsSync.Add(spawnedNetworkObject);
1118-
}
1119+
m_NetworkObjectsSync.Add(spawnedNetworkObject);
11191120
}
11201121
}
11211122
if (EnableSerializationLogs)

‎com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,13 +1165,13 @@ internal bool AuthorityLocalSpawn([NotNull] NetworkObject networkObject, ulong n
11651165
/// <summary>
11661166
/// Only spawn non-authority <see cref="NetworkManager"/> instances should invoke this.
11671167
/// This is invoked to instantiate an authority spawned <see cref="NetworkObject"/>, and
1168-
/// is only invoked by: <see cref="NetworkObject.AddSceneObject(in NetworkObject.SceneObject, FastBufferReader, NetworkManager, bool)"/>
1168+
/// is only invoked by: <see cref="NetworkObject.DeserializeAndSpawnObject"/>
11691169
/// </summary>
11701170
/// <remarks>
1171-
/// IMPORTANT: Pre spawn methods need to be invoked from within <see cref="NetworkObject.Deserialize"/>.
1171+
/// IMPORTANT: Pre spawn methods need to be invoked from within <see cref="NetworkObject.DeserializeAndSpawnObject"/>.
11721172
/// </remarks>
11731173
/// <returns>boolean indicating whether the spawn succeeded</returns>
1174-
internal bool NonAuthorityLocalSpawn(in NetworkObject.SerializedObject serializedObject, out NetworkObject networkObject, FastBufferReader reader, bool destroyWithScene)
1174+
internal bool NonAuthorityLocalSpawn(in NetworkObject.SerializedObject serializedObject, [MaybeNullWhen(false)] out NetworkObject networkObject, FastBufferReader reader, bool destroyWithScene)
11751175
{
11761176
if (SpawnedObjects.ContainsKey(serializedObject.NetworkObjectId))
11771177
{
@@ -1368,7 +1368,7 @@ internal void SendSpawnCallForObject(ulong clientId, NetworkObject networkObject
13681368
}
13691369
var message = new CreateObjectMessage
13701370
{
1371-
ObjectInfo = networkObject.Serialize(clientId, NetworkManager.DistributedAuthorityMode),
1371+
ObjectInfo = networkObject.SerializeSpawnedObject(clientId, NetworkManager.DistributedAuthorityMode),
13721372
IncludesSerializedObject = true,
13731373
UpdateObservers = NetworkManager.DistributedAuthorityMode,
13741374
ObserverIds = NetworkManager.DistributedAuthorityMode ? networkObject.Observers.ToArray() : null,
@@ -1394,7 +1394,7 @@ internal void SendSpawnCallForObserverUpdate(ulong[] newObservers, NetworkObject
13941394

13951395
var message = new CreateObjectMessage
13961396
{
1397-
ObjectInfo = networkObject.Serialize(),
1397+
ObjectInfo = networkObject.SerializeSpawnedObject(),
13981398
ObserverIds = networkObject.Observers.ToArray(),
13991399
NewObserverIds = newObservers.ToArray(),
14001400
IncludesSerializedObject = true,

0 commit comments

Comments
 (0)