Skip to content

Commit e9481cf

Browse files
NoelStephensUnityNoelStephensUnity
authored
feat: hybrid interop tests (#4176)
* test: add hybrid prefab prediction and interop tests CreateHybridPrefab takes an optional GhostMode. Predicted modes are applied after NetworkObjectBridge is added, because its editor OnValidate resets the supported ghost modes to interpolated. HybridPredictionTests: an owner-predicted hybrid prefab predicts and re-simulates on the owning client. HybridInteropTests combines N4E and NGO features on one hybrid prefab: - An NGO RPC sent from PredictionUpdate repeats for re-simulated ticks, and sends each tick once when gated on IsFirstTimeFullyPredictingTick. - Unified remote to NGO RPC to unified remote, and NGO RPC to unified remote to NGO RPC. - A NetworkVariable read during prediction is not tick-aligned, while a tick-stamped value applied from its stamp tick is consistent across re-simulation. * test: cover NetworkVariable writes from prediction and NGO ownership on hybrid prefabs - A NetworkVariable written from PredictionUpdate moves backwards when older ticks re-simulate, and only moves forward when gated on IsFirstTimeFullyPredictingTick. - An NGO ownership change does not change the ghost's N4E owner. * fix: hybrid prefab issues found by the interop tests - UnifiedBootstrap registers the worlds it created for other NetworkManagers again after each bootstrap, since every ClientServerBootstrap constructor clears N4E's ServerWorlds and ClientWorlds. Remote methods send through those lists, so with several NetworkManagers in one process server-to-client remotes were dropped. - NetworkObjectBridge defaults a GhostObject to interpolation only when the bridge is first added, instead of on every OnValidate, so prediction enabled on a hybrid prefab is kept. - NGO spawn ownership and ownership changes set the ghost's owner, so an owner-predicted ghost is predicted by its NGO owner. - NetworkManager shutdown disposes only its own world instead of every world in the process. NetworkObjectDontDestroyWithOwnerTests and NetworkSpawnManagerTests now run in hybrid prefab mode. NetworkShowThenClientDisconnects is ignored for hybrid prefabs: a scene migration update can reach a client before the object's ghost has spawned there. * fix: set the ghost owner before OnNetworkSpawn - The ghost owner is now assigned right after SetupOnSpawn, so OnNetworkSpawn sees the same owner on the NetworkObject and its ghost. The interop test records the ghost owner in OnNetworkSpawn and asserts it. - NetworkVariableReadDuringPredictionIsNotTickAligned asserts that a re-simulated tick reads a different value than its first prediction, rather than counting reads before the stamp tick. - NetworkShowThenClientDisconnects is only ignored for UnifiedServer, the fixture where the scene migration failure occurs. * fix: guard the host world in UpdateGhostOwner and wait for a re-simulation in the backwards-write test - UpdateGhostOwner leaves the ghost unowned if a host NetworkManager has no NetcodeWorld. - NetworkVariableWrittenFromPredictionUpdateMovesBackwards keeps writing until a backwards move has happened, instead of stopping after 20 changes, which could come before any re-simulation. * fix: raise the client's own disconnect event when a hybrid client disconnects UnifiedNetcodeTransport.DisconnectLocalClient only requested the N4E disconnect. N4E reports it later, after NGO's shutdown has stopped listening, so the disconnecting client never received its ClientDisconnected event. It now notifies immediately, as UnityTransport does, and ignores N4E's later event. A server-initiated disconnect removes the connection before notifying, so the shutdown it triggers does not notify a second time. DisconnectTests and PeerDisconnectCallbackTests now run in hybrid prefab mode. * fix: only register hybrid instances as pending ghosts on clients in integration tests The integration test hook that replaces GhostSpawnManager.RegisterGhostBridge also registered the server's own instance. On a dedicated server, which is never a connected client, that moved the instance into the DontDestroyOnLoad scene. The server then sent a scene migration for it to every client, and none of them had spawned it. The hook now skips the server, as the runtime does. NetworkShowThenClientDisconnects no longer needs to be ignored for UnifiedServer. * fix: only send scene migrations to observers, and synchronize late hybrid spawns into their scene ObjectSceneChanged was sent to every connected client, including clients that did not observe the migrated NetworkObjects and had never spawned them, which logged "Trying to synchronize NetworkObjectId but it was not spawned". In client-server mode it is now only sent to clients that observe at least one migrated NetworkObject, and each client is only sent the NetworkObjects it observes. Distributed authority is unchanged. A hybrid prefab instance that is part of a client's initial synchronization can spawn after the synchronization has completed, once its ghost arrives. It missed the step that migrates synchronized NetworkObjects into their server-side scene, and stayed in the active scene. It now runs that step when it spawns. * fix: spawn a NetworkObject shown after a scene migration in the server's scene A client that starts observing a NetworkObject with NetworkShow after the NetworkObject migrated into another scene was never sent that migration, and instantiated the NetworkObject in its active scene. The client now moves a spawned NetworkObject into its server-side scene, as it already does for NetworkObjects that are part of its initial synchronization. Adds CHANGELOG entries for this and for scene migrations being sent to clients that do not observe the NetworkObject. * test: collapse the DisconnectTests owner persistence fixtures into the test Owner persistence is now set per client on the server-side player object instead of on the player prefab, so the fixtures are only Host and UnifiedHost. The host-client shutdown check runs once after both clients disconnect; before, it ran after the first client and the second client was skipped on the ClientDisconnectsFromServer path. * fix: adapt the hybrid tests to the N4E 7.1.0 GameObject API renames N4E 7.1.0 (unity/unity PR 126612) renamed [Remote(Directionality)] to [RPC(SendDirection)], GhostObject.IsPredictedGhost to CanWriteState, and changed GhostBehaviour.PredictionUpdate(float) to PredictionUpdate(PredictionUpdateContext). * chore: remove stale comments from NetworkObjectBridge The execution order TODO is already handled by DefaultExecutionOrder(GhostObject.ExecutionOrder + 1). * style Removing more comments. * feat: warn when a hybrid instance sends an RPC or writes a NetworkVariable inside the prediction loop RPCs and NetworkVariables are not part of Netcode for Entities prediction: they are not rolled back or tick aligned, so a re-simulated tick sends or writes them again. Using them from inside the prediction loop is not supported. A hybrid instance now logs one warning per NetworkBehaviour when it sends an RPC or writes a NetworkVariable while its ghost is in the prediction loop. Replaces the HybridInteropTests cases that measured those unsupported patterns with tests for the warning, and that the same RPC and write made outside the prediction loop do not warn. * update 1:1 With Emma. Further simplified disconnect tests. Made CreateHybridPrefab private. Removed reference to unused enum. * update 1:1 With Emma. We both agreed this test doesn't really test anything that the HybridInteropTests validate. It is removed. * test Enabling the CMB service for this test. * update Enabling CMB server on some tests. Making the CreateHybridPrefab internal access only for now (it is used in our hybrid prefab prediction tests). * fix: apply the scene migration fixes in distributed authority mode Distributed authority still sent a scene migration to every client, so clients that did not observe a migrated NetworkObject logged "Trying to synchronize NetworkObjectId but it was not spawned", and a NetworkObject shown after it migrated spawned in the client's active scene. The owner now only sends a migration to the clients that observe a migrated NetworkObject, as in client-server mode. The CMB service's own copy still includes every migrated NetworkObject, and a migration the DAHost forwards is not filtered again. A NetworkObject a distributed authority client spawns when it is shown is moved into its owner's scene. * chore: correct the comment on when the CMB service is sent a scene migration Only the session owner sends the CMB service its own copy of a migration; a client that is not the session owner does not tell the service about objects only it observes. * test Rust server needs to be updated
1 parent 3afa703 commit e9481cf

23 files changed

Lines changed: 889 additions & 158 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+
- Issue where moving a `NetworkObject` into another scene made the clients that did not observe it log "Trying to synchronize NetworkObjectId but it was not spawned". The scene migration is now only sent to the clients that observe the `NetworkObject`. (#4176)
22+
- Issue where a `NetworkObject` that was moved into another scene while hidden from a client spawned in that client's active scene when it was shown with `NetworkShow`, instead of the scene it is in on the server. (#4176)
2123
- 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)
2224

2325
### Security

‎com.unity.netcode.gameobjects/Runtime/Components/Helpers/NetworkObjectBridge.cs‎

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,27 @@
99
namespace Unity.Netcode
1010
{
1111
/// <summary>
12-
/// TODO-UNIFIED: Needs further peer review and exploring alternate ways of handling this.
1312
/// This is a component that is added to the root of all N4E-spawned hybrid prefab instances. It is used to link
1413
/// <see cref="NetworkObject.SerializedObject"/> the N4E-spawned hybrid prefab instances to the incoming <see cref="CreateObjectMessage"/>
1514
/// specific to the N4E-spawned hybrid prefab instance that has the matching <see cref="NetworkObjectId"/>.
1615
/// </summary>
1716

1817
[DefaultExecutionOrder(GhostObject.ExecutionOrder + 1)]
19-
//BREAK --- Fix this on UNIFIED side 1st
20-
// Internal: GhostBehaviour is only public when NETCODE_GAMEOBJECT_BRIDGE_EXPERIMENTAL is defined, and a public
21-
// type cannot derive from an internal one.
2218
internal partial class NetworkObjectBridge : GhostBehaviour
2319
{
24-
// DefaultExecutionOrder
25-
// TODO: Define a const for the value used on GhostObject and use that value
26-
// to set the execution order so if it changes on GhostObject it updates here.
2720
#if UNITY_EDITOR
21+
private void Reset()
22+
{
23+
// Start users with just interpolation (they can adjust this if they want prediction)
24+
// to make the initial transition less problematic for users.
25+
// Only set when the bridge is first added, so a user's prediction setting is kept.
26+
var ghostAdapter = GetComponent<GhostObject>();
27+
if (ghostAdapter != null)
28+
{
29+
ghostAdapter.SupportedGhostModes = GhostModeMask.Interpolated;
30+
}
31+
}
32+
2833
private void OnValidate()
2934
{
3035
hideFlags = HideFlags.HideInInspector;
@@ -35,10 +40,6 @@ private void OnValidate()
3540
return;
3641
}
3742

38-
// Start users with just interpolation (they can adjust this if they want prediction)
39-
// to make the initial transition less problematic for users.
40-
ghostAdapter.SupportedGhostModes = GhostModeMask.Interpolated;
41-
4243
#if COM_UNITY_MODULES_PHYSICS
4344
var rigidBody = GetComponent<Rigidbody>();
4445
var ghostRigidBody = GetComponent<GhostRigidbody>();
@@ -88,6 +89,41 @@ internal void ApplyScale(Vector3 scale)
8889
{
8990
Ghost.ApplyPostTransformMatrixScale(scale);
9091
}
92+
93+
/// <summary>
94+
/// Keeps the ghost's owner in step with the <see cref="NetworkObject"/> owner, so an owner-predicted
95+
/// ghost is predicted by the NGO owner.
96+
/// </summary>
97+
/// <remarks>Only the server can assign a ghost owner; N4E replicates it to clients.</remarks>
98+
/// <param name="networkManager">The server's <see cref="NetworkManager"/>.</param>
99+
/// <param name="ownerClientId">The new NGO owner.</param>
100+
internal void UpdateGhostOwner(NetworkManager networkManager, ulong ownerClientId)
101+
{
102+
if (!networkManager.IsServer || !Ghost.HasOwner)
103+
{
104+
return;
105+
}
106+
107+
// Owned by the server with no local client means no client owns the ghost.
108+
var ownerNetworkId = default(NetworkId);
109+
if (ownerClientId == networkManager.LocalClientId)
110+
{
111+
if (networkManager.IsHost && networkManager.NetcodeWorld != null)
112+
{
113+
ownerNetworkId = networkManager.NetcodeWorld.LocalConnection.NetworkId;
114+
}
115+
}
116+
else
117+
{
118+
// The unified transport id is the client's N4E network id.
119+
var (transportId, found) = networkManager.ConnectionManager.ClientIdToTransportId(ownerClientId);
120+
if (found)
121+
{
122+
ownerNetworkId = new NetworkId { Value = (int)transportId };
123+
}
124+
}
125+
Ghost.OwnerNetworkId = ownerNetworkId;
126+
}
91127
}
92128

93129
#if UNIFIED_NETCODE_7_0_0

‎com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedBootstrap.cs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#endif
44
#if UNIFIED_NETCODE
55
using System;
6+
using System.Collections.Generic;
67
using Unity.Entities;
78
#if !UNIFIED_NETCODE_7_0_0
89
using Unity.NetCode;
@@ -27,6 +28,32 @@ internal class UnifiedBootstrap : ClientServerBootstrap
2728

2829
private static int s_WorldCounter = 0;
2930

31+
// Every ClientServerBootstrap constructor clears N4E's ServerWorlds and ClientWorlds, and each NetworkManager
32+
// creates its own bootstrap, so the worlds created for other NetworkManagers are registered again.
33+
private static readonly List<NetcodeWorld> s_CreatedWorlds = new List<NetcodeWorld>();
34+
35+
private static void RegisterCreatedWorlds()
36+
{
37+
for (int i = s_CreatedWorlds.Count - 1; i >= 0; i--)
38+
{
39+
var world = s_CreatedWorlds[i];
40+
if (!world.IsCreated)
41+
{
42+
s_CreatedWorlds.RemoveAt(i);
43+
continue;
44+
}
45+
// A single world host is registered as both a server and a client world, the same as N4E does.
46+
if (world.IsServer() && !ServerWorlds.Contains(world))
47+
{
48+
ServerWorlds.Add(world);
49+
}
50+
if (world.IsClient() && !ClientWorlds.Contains(world))
51+
{
52+
ClientWorlds.Add(world);
53+
}
54+
}
55+
}
56+
3057
public override bool Initialize(string defaultWorldName)
3158
{
3259
var networkManager = CurrentNetworkManagerForInitialization;
@@ -44,6 +71,7 @@ public override bool Initialize(string defaultWorldName)
4471

4572
if (networkManager != null)
4673
{
74+
RegisterCreatedWorlds();
4775
Debug.Log($"Starting a world for {(networkManager.IsServer ? "Host" : "Client")}");
4876
s_WorldCounter++;
4977
LastCreatedWorld = networkManager.IsServer ? CreateSingleWorldHost($"HostSingleWorld-{s_WorldCounter}")
@@ -69,6 +97,7 @@ public override bool Initialize(string defaultWorldName)
6997
}
7098

7199
networkManager.NetcodeWorld = (NetcodeWorld)LastCreatedWorld;
100+
s_CreatedWorlds.Add(networkManager.NetcodeWorld);
72101
}
73102
else
74103
{

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,54 @@ internal enum __RpcExecStage
7474
private const int k_RpcMessageDefaultSize = 1024; // 1k
7575
private const int k_RpcMessageMaximumSize = 1024 * 64; // 64k
7676

77+
#if UNIFIED_NETCODE
78+
private bool m_WarnedRpcInPredictionLoop;
79+
private bool m_WarnedNetworkVariableInPredictionLoop;
80+
81+
/// <summary>
82+
/// Whether this hybrid instance's ghost is currently inside the Netcode for Entities prediction loop.
83+
/// </summary>
84+
private bool IsInGhostPredictionLoop()
85+
{
86+
if (m_NetworkObject == null || !m_NetworkObject.HasGhost)
87+
{
88+
return false;
89+
}
90+
var ghostObject = m_NetworkObject.GhostObject;
91+
return ghostObject != null && ghostObject.IsSpawned && ghostObject.NetworkTime.IsInPredictionLoop;
92+
}
93+
94+
// RPCs and NetworkVariables are not rolled back or tick aligned, so a re-simulated tick sends or writes them again.
95+
// Warned once per NetworkBehaviour.
96+
private void WarnIfRpcSentInPredictionLoop()
97+
{
98+
if (m_WarnedRpcInPredictionLoop || !IsInGhostPredictionLoop())
99+
{
100+
return;
101+
}
102+
m_WarnedRpcInPredictionLoop = true;
103+
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
104+
{
105+
NetworkLog.LogWarning($"[{name}][{GetType().Name}] An RPC was sent from inside the Netcode for Entities prediction loop, which is not supported. " +
106+
"RPCs are not part of prediction and are sent again for every re-simulated tick.");
107+
}
108+
}
109+
110+
internal void WarnIfNetworkVariableWrittenInPredictionLoop(string variableName)
111+
{
112+
if (m_WarnedNetworkVariableInPredictionLoop || !IsInGhostPredictionLoop())
113+
{
114+
return;
115+
}
116+
m_WarnedNetworkVariableInPredictionLoop = true;
117+
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
118+
{
119+
NetworkLog.LogWarning($"[{name}][{GetType().Name}][{variableName}] A NetworkVariable was written from inside the Netcode for Entities prediction loop, which is not supported. " +
120+
"NetworkVariables are not rolled back, so re-simulated ticks write it again and its value can move backwards.");
121+
}
122+
}
123+
#endif
124+
77125
#pragma warning disable IDE1006 // disable naming rule violation check
78126
// RuntimeAccessModifiersILPP will make this `protected`
79127
internal FastBufferWriter __beginSendServerRpc(uint rpcMethodId, ServerRpcParams serverRpcParams, RpcDelivery rpcDelivery)
@@ -83,6 +131,9 @@ internal FastBufferWriter __beginSendServerRpc(uint rpcMethodId, ServerRpcParams
83131
{
84132
throw new RpcException("The NetworkBehaviour must be spawned before calling this method.");
85133
}
134+
#if UNIFIED_NETCODE
135+
WarnIfRpcSentInPredictionLoop();
136+
#endif
86137

87138
return new FastBufferWriter(k_RpcMessageDefaultSize, Allocator.Temp, k_RpcMessageMaximumSize);
88139
}
@@ -161,6 +212,9 @@ internal FastBufferWriter __beginSendClientRpc(uint rpcMethodId, ClientRpcParams
161212
{
162213
throw new RpcException("The NetworkBehaviour must be spawned before calling this method.");
163214
}
215+
#if UNIFIED_NETCODE
216+
WarnIfRpcSentInPredictionLoop();
217+
#endif
164218

165219
return new FastBufferWriter(k_RpcMessageDefaultSize, Allocator.Temp, k_RpcMessageMaximumSize);
166220
}
@@ -339,6 +393,9 @@ internal FastBufferWriter __beginSendRpc(uint rpcMethodId, RpcParams rpcParams,
339393
{
340394
throw new RpcException("This RPC can only be sent by its owner.");
341395
}
396+
#if UNIFIED_NETCODE
397+
WarnIfRpcSentInPredictionLoop();
398+
#endif
342399
return new FastBufferWriter(k_RpcMessageDefaultSize, Allocator.Temp, k_RpcMessageMaximumSize);
343400
}
344401

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1843,8 +1843,11 @@ internal void ShutdownInternal()
18431843
{
18441844
try
18451845
{
1846-
// Dispose of all worlds
1847-
World.DisposeAllWorlds();
1846+
// Dispose only this NetworkManager's world. Other NetworkManagers in the same process keep theirs.
1847+
if (NetcodeWorld != null && NetcodeWorld.IsCreated)
1848+
{
1849+
NetcodeWorld.Dispose();
1850+
}
18481851
// Clear the world assigned from previous session
18491852
NetcodeWorld = null;
18501853
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,13 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
197197
networkManager.Log.ErrorServer(new Context(LogLevel.Developer, $"Failed to deserialize {nameof(NetworkObject)}.").AddInfo(nameof(NetworkObject.GlobalObjectIdHash), serializedObject.Hash).AddInfo(nameof(NetworkObject.NetworkObjectId), serializedObject.NetworkObjectId));
198198
return;
199199
}
200+
201+
// A NetworkObject shown to this client after it migrated into another scene was instantiated in the
202+
// active scene, and the migration was only sent to the clients observing it at the time.
203+
if (networkManager.NetworkConfig.EnableSceneManagement)
204+
{
205+
networkManager.SceneManager.SynchronizeNetworkObjectScene(networkObject);
206+
}
200207
}
201208
else
202209
{
@@ -225,6 +232,12 @@ internal static void CreateObject(ref NetworkManager networkManager, ulong sende
225232
networkManager.Log.ErrorServer(new Context(LogLevel.Developer, $"Failed to deserialize {nameof(NetworkObject)}.").AddInfo(nameof(NetworkObject.GlobalObjectIdHash), serializedObject.Hash).AddInfo(nameof(NetworkObject.NetworkObjectId), serializedObject.NetworkObjectId));
226233
return;
227234
}
235+
236+
// Same as client-server: the migration was only sent to the clients observing it at the time
237+
if (networkManager.NetworkConfig.EnableSceneManagement)
238+
{
239+
networkManager.SceneManager.SynchronizeNetworkObjectScene(networkObject);
240+
}
228241
}
229242

230243
// DA - NGO CMB SERVICE NOTES:

‎com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,12 @@ public virtual void SetDirty(bool isDirty)
281281

282282
if (m_IsDirty)
283283
{
284+
#if UNIFIED_NETCODE
285+
if (m_NetworkBehaviour != null)
286+
{
287+
m_NetworkBehaviour.WarnIfNetworkVariableWrittenInPredictionLoop(Name);
288+
}
289+
#endif
284290
MarkNetworkBehaviourDirty();
285291
}
286292
}

0 commit comments

Comments
 (0)