Skip to content

Commit 15166f7

Browse files
fix: hybrid prefab component gating and distributed authority rejection (#4184)
* fix: gate hybrid prefab components instead of destroying them A hybrid prefab's NetworkRigidbodyBase derivatives were destroyed at runtime so a user could try hybrid mode without first stripping components from the prefab. Removing an entry from ChildNetworkBehaviours is not safe: InitializeChildNetworkBehaviours rebuilds unconditionally and has five call sites, two of them the public GetNetworkBehaviourOrderIndex and GetNetworkBehaviourAtOrderIndex, while Destroy is deferred to end of frame. A rebuild in a later frame therefore renumbers every behaviour after the removed one, and the NetworkVariable half of object synchronization is positional with no id, so a one-entry disagreement corrupts the whole buffer for that object. NetworkTransform was already handled by early-returning on HasGhost rather than being destroyed - the removal behind UNIFIED_NETCODE_DESTROY was dead, that define exists nowhere. This extends the same treatment to NetworkRigidbodyBase and closes four NetworkTransform paths that had no gate: OnSynchronize, InternalOnNetworkPostSpawn, InternalOnNetworkSessionSynchronized and InternalOnNetworkObjectParentChanged. The first of those means a hybrid object no longer writes a full teleport state to every joining client. The rigidbody is now forced kinematic on non-server peers rather than destroyed, restored from m_OriginalKinematicState on destroy as the class already provided for. That decision is made at spawn, not during Awake, because Initialize runs from Awake where an in-scene placed instance has no session to ask whether it is the server. UnregisterRigidbody had no call sites and is now called from the gate, which is what keeps a second Initialize from leaving a registration pointing at a component that no longer drives anything. [RequireComponent(typeof(Rigidbody))] is unconditional again. Behind #if !UNIFIED_NETCODE it was stripped from every NetworkRigidbody in the project the moment N4E was installed, hybrid or not, after which NetworkRigidbodyBase dereferences a null rigidbody in Awake. * fix: reject hybrid prefabs in distributed authority sessions Distributed authority does not support hybrid prefabs. A start with any GhostObject prefab registered now fails in CanStart with an error naming each prefab, before Initialize replaces the transport with UnifiedNetcodeTransport. A hybrid prefab added during a distributed authority session is refused, so HasGhostPrefabs cannot switch the running session into hybrid mode and stop its send queue. * fix: address u-pr review and reject hybrid prefab overrides - NetworkRigidbodyBase no longer re-applies the authority kinematic rule on an ownership change for a hybrid prefab. - NetworkPrefabs recomputes HasGhostPrefabs on initialize, and keeps a single subscription to each NetworkPrefabsList (a prefab added to a list during a session was registered more than once). - RejectGhostPrefabs is set in Initialize, after SetRole accepts the start, so a failed start cannot leave it set. - A NetworkPrefab override with a hybrid source or target prefab is detected as a hybrid registration. N4E instantiates the server's prefab from the ghost type, so overrides are not supported for hybrid prefabs yet: a start with one registered fails in any topology, and one added during a session is rejected. * fix: address review on hybrid prefab validation - Move the distributed authority hybrid prefab check and its session flag into NetworkPrefabs, called through NetworkConfig.InitializePrefabsForStart. - NetworkPrefab.Validate treats an override with a hybrid source or target as invalid, so it is logged and ignored in any topology. - Add NetworkPrefab.GetDebugName, used by the prefab logs and the debugger. - Add NetcodeIntegrationTest.CreateNetworkObjectPrefab(string, bool). - Trim the hybrid prefab tests to the behaviour id and kinematic checks, and rework the validation tests onto the harness helpers. * test: remove BehaviourIdsMatchOnAllPeers KinematicStateSurvivesOwnershipChange already runs the same behaviour table and gated component checks.
1 parent c55eebf commit 15166f7

16 files changed

Lines changed: 619 additions & 108 deletions

‎com.unity.netcode.gameobjects/CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
1818

1919
### Fixed
2020

21+
- Fixed issue where a prefab added to a `NetworkPrefabsList` while a session was running was registered more than once and logged a duplicate `GlobalObjectIdHash` error. (#4184)
22+
2123
### Security
2224

2325
### Obsolete

‎com.unity.netcode.gameobjects/Runtime/Components/NetworkRigidBodyBase.cs‎

Lines changed: 85 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,6 @@ public abstract class NetworkRigidbodyBase : NetworkBehaviour
2121
internal bool NetworkRigidbodyBaseExpanded;
2222
#endif
2323

24-
// TODO-UNIFIED:
25-
// Provide an option to automatically remove the NetworkRigidbodyBase component at runtime if it is a hybrid prefab that is spawned since this
26-
// component is primarily used for the Rigidbody interpolation and extrapolation features of NetworkTransform which are not relevant for a hybrid
27-
// prefab that is spawned since it will be using N4E's built in interpolation and extrapolation features. This greatly improves performance on
28-
// the client side. If using N4E prediction or distributed authority mode, then Rigibody and any component derived from this should always be used.
29-
30-
3124
/// <summary>
3225
/// When enabled, the associated <see cref="NetworkTransform"/> will use the Rigidbody/Rigidbody2D to apply and synchronize changes in position, rotation, and
3326
/// allows for the use of Rigidbody interpolation/extrapolation.
@@ -133,6 +126,13 @@ protected void Initialize(RigidbodyTypes rigidbodyType, NetworkTransform network
133126
NetworkTransform = GetComponent<NetworkTransform>();
134127
}
135128

129+
#if UNIFIED_NETCODE
130+
if (InitializeHybrid())
131+
{
132+
return;
133+
}
134+
#endif
135+
136136
if (NetworkTransform != null)
137137
{
138138
NetworkTransform.RegisterRigidbody(this);
@@ -184,6 +184,13 @@ protected void Initialize(RigidbodyTypes rigidbodyType, NetworkTransform network
184184
NetworkTransform = GetComponent<NetworkTransform>();
185185
}
186186

187+
#if UNIFIED_NETCODE
188+
if (InitializeHybrid())
189+
{
190+
return;
191+
}
192+
#endif
193+
187194
if (NetworkTransform != null)
188195
{
189196
NetworkTransform.RegisterRigidbody(this);
@@ -201,11 +208,6 @@ protected void Initialize(RigidbodyTypes rigidbodyType, NetworkTransform network
201208
#endif
202209

203210
#if COM_UNITY_MODULES_PHYSICS && COM_UNITY_MODULES_PHYSICS2D
204-
#if UNIFIED_NETCODE
205-
// Used to keep track of the original kinematic state upon awake.
206-
// (see OnDestroy below)
207-
private bool m_OriginalKinematicState;
208-
#endif
209211
/// <summary>
210212
/// Initializes the networked Rigidbody based on the <see cref="RigidbodyTypes"/>
211213
/// passed in as a parameter.
@@ -248,6 +250,13 @@ protected void Initialize(RigidbodyTypes rigidbodyType, NetworkTransform network
248250
NetworkTransform = GetComponent<NetworkTransform>();
249251
}
250252

253+
#if UNIFIED_NETCODE
254+
if (InitializeHybrid())
255+
{
256+
return;
257+
}
258+
#endif
259+
251260
if (NetworkTransform != null)
252261
{
253262
NetworkTransform.RegisterRigidbody(this);
@@ -259,31 +268,68 @@ protected void Initialize(RigidbodyTypes rigidbodyType, NetworkTransform network
259268

260269
if (AutoUpdateKinematicState)
261270
{
262-
#if UNIFIED_NETCODE
263-
// Keep track of the original kinematic state. (see OnDestroy)
264-
m_OriginalKinematicState = IsKinematic();
265-
#endif
266271
SetIsKinematic(true);
267272
}
268273
}
274+
#endif
269275

270276
#if UNIFIED_NETCODE
277+
// The authored kinematic state, restored in OnDestroy.
278+
private bool m_OriginalKinematicState;
279+
280+
/// <summary>
281+
/// Skips initialization on a hybrid prefab, whose GhostObject drives its motion. The component stays so <see cref="NetworkBehaviour.NetworkBehaviourId"/> values match on every peer.
282+
/// </summary>
283+
/// <returns>true for a hybrid prefab</returns>
284+
private bool InitializeHybrid()
285+
{
286+
if (NetworkObject == null || !NetworkObject.HasGhost)
287+
{
288+
return false;
289+
}
290+
291+
// Clears any registration left behind by a prior Initialize call.
292+
if (NetworkTransform != null)
293+
{
294+
NetworkTransform.UnregisterRigidbody();
295+
}
296+
297+
m_OriginalKinematicState = IsKinematic();
298+
return true;
299+
}
300+
301+
/// <summary>
302+
/// Kinematic on every peer except the server. Called at spawn because an in-scene placed instance has no session during Awake.
303+
/// </summary>
304+
private void SetHybridKinematicState()
305+
{
306+
if (!m_LocalNetworkManager.IsServer)
307+
{
308+
SetIsKinematic(true);
309+
}
310+
}
311+
312+
/// <inheritdoc/>
271313
public override void OnDestroy()
272314
{
273315
base.OnDestroy();
274-
// If the user has left this component on their prefab and this is a hybrid prefab,
275-
// then we want to set the rigid body back to its original kinematic settings since
276-
// we are automatically destroying these components at runtime when it is a hybrid
277-
// prefab that is spawned.
278-
if (NetworkObject && NetworkObject.HasGhost)
316+
if (!NetworkObject || !NetworkObject.HasGhost)
279317
{
280-
if (m_InternalRigidbody || m_InternalRigidbody2D)
281-
{
282-
SetIsKinematic(m_OriginalKinematicState);
283-
}
318+
return;
284319
}
285-
}
320+
#if COM_UNITY_MODULES_PHYSICS && COM_UNITY_MODULES_PHYSICS2D
321+
if (m_InternalRigidbody || m_InternalRigidbody2D)
286322
#endif
323+
#if COM_UNITY_MODULES_PHYSICS && !COM_UNITY_MODULES_PHYSICS2D
324+
if (m_InternalRigidbody)
325+
#endif
326+
#if !COM_UNITY_MODULES_PHYSICS && COM_UNITY_MODULES_PHYSICS2D
327+
if (m_InternalRigidbody2D)
328+
#endif
329+
{
330+
SetIsKinematic(m_OriginalKinematicState);
331+
}
332+
}
287333
#endif
288334
internal Vector3 GetAdjustedPositionThreshold()
289335
{
@@ -1019,7 +1065,12 @@ public void ResetInterpolation()
10191065

10201066
protected override void OnOwnershipChanged(ulong previous, ulong current)
10211067
{
1022-
UpdateOwnershipAuthority();
1068+
#if UNIFIED_NETCODE
1069+
if (!NetworkObject.HasGhost)
1070+
#endif
1071+
{
1072+
UpdateOwnershipAuthority();
1073+
}
10231074
base.OnOwnershipChanged(previous, current);
10241075
}
10251076

@@ -1062,6 +1113,13 @@ internal override void InternalOnNetworkPreSpawn(ref NetworkManager networkManag
10621113
/// <inheritdoc />
10631114
public override void OnNetworkSpawn()
10641115
{
1116+
#if UNIFIED_NETCODE
1117+
if (NetworkObject.HasGhost)
1118+
{
1119+
SetHybridKinematicState();
1120+
return;
1121+
}
1122+
#endif
10651123
m_TickFrequency = 1.0f / m_LocalNetworkManager.NetworkConfig.TickRate;
10661124
m_TickRate = m_LocalNetworkManager.NetworkConfig.TickRate;
10671125
UpdateOwnershipAuthority();

‎com.unity.netcode.gameobjects/Runtime/Components/NetworkRigidbody.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ namespace Unity.Netcode.Components
99
/// mode of the <see cref="Rigidbody"/> and disabling it on all peers but the authoritative one.
1010
/// </summary>
1111
[RequireComponent(typeof(NetworkTransform))]
12-
// TODO-UNIFIED: We should not require this for unified and come up with a different way of handling the dependency
13-
#if !UNIFIED_NETCODE
1412
[RequireComponent(typeof(Rigidbody))]
15-
#endif
1613
[AddComponentMenu("Netcode/Network Rigidbody")]
1714
[HelpURL(HelpUrls.NetworkRigidbody)]
1815
public class NetworkRigidbody : NetworkRigidbodyBase

‎com.unity.netcode.gameobjects/Runtime/Components/NetworkRigidbody2D.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ namespace Unity.Netcode.Components
99
/// mode of the rigidbody and disabling it on all peers but the authoritative one.
1010
/// </summary>
1111
[RequireComponent(typeof(NetworkTransform))]
12-
// TODO-UNIFIED: We should not require this for unified and come up with a different way of handling the dependency
13-
#if !UNIFIED_NETCODE
1412
[RequireComponent(typeof(Rigidbody2D))]
15-
#endif
1613
[AddComponentMenu("Netcode/Network Rigidbody 2D")]
1714
[HelpURL(HelpUrls.NetworkRigidbody2D)]
1815
public class NetworkRigidbody2D : NetworkRigidbodyBase

‎com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1908,6 +1908,13 @@ private bool ShouldSynchronizeHalfFloat(ulong targetClientId)
19081908
/// </remarks>
19091909
protected override void OnSynchronize<T>(ref BufferSerializer<T> serializer)
19101910
{
1911+
#if UNIFIED_NETCODE
1912+
// No transform state is synchronized for a hybrid prefab.
1913+
if (NetworkObject.HasGhost)
1914+
{
1915+
return;
1916+
}
1917+
#endif
19111918
var targetClientId = m_TargetIdBeingSynchronized;
19121919
SynchronizeState = new NetworkTransformState()
19131920
{
@@ -3527,6 +3534,13 @@ private void NonAuthorityFinalizeSynchronization()
35273534
/// </summary>
35283535
protected internal override void InternalOnNetworkSessionSynchronized()
35293536
{
3537+
#if UNIFIED_NETCODE
3538+
// Nothing was synchronized for a hybrid prefab.
3539+
if (NetworkObject.HasGhost)
3540+
{
3541+
return;
3542+
}
3543+
#endif
35303544
NonAuthorityFinalizeSynchronization();
35313545

35323546
base.InternalOnNetworkSessionSynchronized();
@@ -3548,6 +3562,12 @@ private void ApplyPlayerTransformState()
35483562
/// </summary>
35493563
protected internal override void InternalOnNetworkPostSpawn()
35503564
{
3565+
#if UNIFIED_NETCODE
3566+
if (NetworkObject.HasGhost)
3567+
{
3568+
return;
3569+
}
3570+
#endif
35513571
// This is a special case for client-server where a server is spawning an owner authoritative NetworkObject but has yet to serialize anything.
35523572
// When the server detects that:
35533573
// - We are not in a distributed authority session (DAHost check).
@@ -3643,9 +3663,6 @@ internal override void InternalOnNetworkPreSpawn(ref NetworkManager networkManag
36433663
public override void OnNetworkSpawn()
36443664
{
36453665
#if UNIFIED_NETCODE
3646-
// TODO-UNIFIED:
3647-
// Provide a notification to users that NetworkTransform component will be removed at runtime if it is a hybrid prefab that is spawned since
3648-
// it will be using N4E's built in interpolation and extrapolation features.
36493666
if (NetworkObject.HasGhost)
36503667
{
36513668
return;
@@ -3743,9 +3760,7 @@ private void ResetInterpolatedStateToCurrentAuthoritativeState()
37433760
internal virtual void InternalInitialization(bool isOwnershipChange = false)
37443761
{
37453762
#if UNIFIED_NETCODE
3746-
// TODO-UNIFIED:
3747-
// Provide a notification to users that NetworkTransform component will be removed at runtime if it is a hybrid prefab that is spawned since
3748-
// it will be using N4E's built in interpolation and extrapolation features.
3763+
// Inert on a hybrid prefab, but kept so NetworkBehaviourId values match on every peer.
37493764
if (NetworkObject.HasGhost)
37503765
{
37513766
return;
@@ -3932,6 +3947,13 @@ private void DefaultParentChanged()
39323947

39333948
internal override void InternalOnNetworkObjectParentChanged(NetworkObject parentNetworkObject)
39343949
{
3950+
#if UNIFIED_NETCODE
3951+
// A hybrid prefab's transform space is driven by its GhostObject.
3952+
if (NetworkObject.HasGhost)
3953+
{
3954+
return;
3955+
}
3956+
#endif
39353957
if (!SwitchTransformSpaceWhenParented)
39363958
{
39373959
// Motion authority doesn't need to adjust anything

‎com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,17 @@ internal void InitializePrefabs()
388388
Prefabs.Initialize();
389389
}
390390

391+
#if UNIFIED_NETCODE
392+
/// <summary>
393+
/// Registers any prefab list assigned after Awake, and returns false for a distributed authority session with a hybrid prefab registered.
394+
/// </summary>
395+
internal bool InitializePrefabsForStart()
396+
{
397+
InitializePrefabs();
398+
return NetworkTopology != NetworkTopologyTypes.DistributedAuthority || Prefabs.ValidateForDistributedAuthority();
399+
}
400+
#endif
401+
391402
[NonSerialized]
392403
private bool m_DidWarnOldPrefabList = false;
393404

‎com.unity.netcode.gameobjects/Runtime/Configuration/NetworkPrefab.cs‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public enum NetworkPrefabOverride
2929
/// Class that represents a NetworkPrefab
3030
/// </summary>
3131
[Serializable]
32+
[System.Diagnostics.DebuggerDisplay("{GetDebugName()}")]
3233
public class NetworkPrefab
3334
{
3435
/// <summary>
@@ -154,7 +155,8 @@ public uint TargetPrefabGlobalObjectIdHash
154155
/// <returns>True if the NetworkPrefab is valid and ready for use, false otherwise</returns>
155156
public bool Validate(int index = -1)
156157
{
157-
NetworkObject networkObject;
158+
// Null for a hash override.
159+
NetworkObject networkObject = null;
158160
if (Override == NetworkPrefabOverride.None)
159161
{
160162
if (Prefab == null)
@@ -270,9 +272,43 @@ public bool Validate(int index = -1)
270272
return false;
271273
}
272274

275+
#if UNIFIED_NETCODE
276+
// N4E spawns the ghost's own prefab on every client, so the override would never be applied.
277+
if ((networkObject != null && networkObject.HasGhost)
278+
|| (OverridingTargetPrefab.TryGetComponent(out NetworkObject targetNetworkObject) && targetNetworkObject.HasGhost))
279+
{
280+
NetworkLog.LogError($"{HybridPrefabOverrideError} {GetDebugName()} (entry will be ignored).");
281+
return false;
282+
}
283+
#endif
273284
return true;
274285
}
275286

287+
#if UNIFIED_NETCODE
288+
internal const string HybridPrefabOverrideError = "NetworkPrefab overrides are not supported for hybrid prefabs yet.";
289+
#endif
290+
291+
/// <summary>
292+
/// Names the prefab, including its override target, for logs and the debugger.
293+
/// </summary>
294+
internal string GetDebugName()
295+
{
296+
switch (Override)
297+
{
298+
case NetworkPrefabOverride.Prefab:
299+
return $"{GetName(SourcePrefabToOverride)} (overridden by {GetName(OverridingTargetPrefab)})";
300+
case NetworkPrefabOverride.Hash:
301+
return $"{SourceHashToOverride} (overridden by {GetName(OverridingTargetPrefab)})";
302+
default:
303+
return GetName(Prefab);
304+
}
305+
}
306+
307+
private static string GetName(GameObject prefab)
308+
{
309+
return prefab != null ? prefab.name : "null";
310+
}
311+
276312
/// <summary>
277313
/// Returns a string representation of this NetworkPrefab's source and target hash values
278314
/// </summary>

0 commit comments

Comments
 (0)