Skip to content

Commit 81d53df

Browse files
fix: host client SyncVar hooks ignoring AOI visibility (#4079)
* Initial plan * Fix host client SyncVar hook AOI bugs - Modified GeneratedSyncVarSetter methods to only call hooks in host mode if object is visible (in NetworkClient.spawned) - Modified OnHostClientSpawn to deserialize spawn payload, triggering SyncVar hooks when objects enter AOI range - This fixes both issues: hooks no longer fire at spawn for out-of-range objects, and hooks now fire when entering range Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com> * Replace placeholder issue numbers with descriptive text Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com> * Fix comment capitalization per code review Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com> * Fix regression: restore SyncVar hooks in host mode with AOI Previous fix prevented all SyncVar hooks from firing by checking NetworkClient.spawned too early. The check happened when the value was set on the server, before the object was added to spawned. New approach: - Revert setter checks to allow hooks during server-side value assignment - Add hostInitialSpawn flag to NetworkIdentity - Set flag in OnHostClientSpawn before deserializing - Modify all GeneratedSyncVarDeserialize methods to force hooks during initial spawn even if value unchanged - Clear flag after deserialization This ensures hooks fire when objects enter AOI range in host mode, addressing the original issue without breaking normal hook execution. Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com> * Fix issue 1: prevent hooks from firing at spawn for out-of-range objects Added NetworkClient.spawned.ContainsKey() check to all four GeneratedSyncVarSetter methods. This prevents hooks from firing when SyncVars are set during OnStartServer() for objects not yet visible to host client. Combined with the hostInitialSpawn flag (which forces hooks during deserialization when entering AOI), this now fixes both issues: - Issue 1: No hooks fire at spawn for out-of-range objects - Issue 2: Hooks fire when objects enter AOI range The check works because objects are added to NetworkClient.spawned in OnHostClientSpawn BEFORE deserialization, so the deserialize path can fire hooks while the setter path is blocked for invisible objects. Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com> * Fix unit tests: only check hostInitialSpawn in actual host mode Added NetworkServer.activeHost check before using hostInitialSpawn flag in all four GeneratedSyncVarDeserialize methods. This prevents the flag from being checked in non-host mode tests, which was causing NullReferenceExceptions when hooks were called during deserialization even though values hadn't changed. The hostInitialSpawn flag should only force hooks in host mode. In regular client mode, hooks should only fire when values actually change. Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com> * Fix host mode tests: only force hooks when AOI is active Changed hostInitialSpawn flag to only be set when aoi != null in OnHostClientSpawn. This ensures hooks are only forced during deserialization when AOI is actually hiding objects. In regular host mode without AOI, hooks fire normally via setters when values change. This fixes the host mode unit tests while maintaining the AOI fix for the original issue. Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com> * perf: optimize GeneratedSyncVarDeserialize methods to skip work when no hook Check OnChanged != null before calculating changed and hostInitialSpawnInHostMode booleans. No point doing that work if there's no hook to call. Applied to all 4 GeneratedSyncVarDeserialize methods: - GeneratedSyncVarDeserialize<T> - GeneratedSyncVarDeserialize_GameObject - GeneratedSyncVarDeserialize_NetworkIdentity - GeneratedSyncVarDeserialize_NetworkBehaviour<T> Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MrGadget1024 <9826063+MrGadget1024@users.noreply.github.com>
1 parent caaf7e1 commit 81d53df

File tree

3 files changed

+70
-13
lines changed

3 files changed

+70
-13
lines changed

Assets/Mirror/Core/NetworkBehaviour.cs

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,9 @@ public void GeneratedSyncVarSetter<T>(T value, ref T field, ulong dirtyBit, Acti
565565
// in client-only mode, OnDeserialize would call it.
566566
// we use hook guard to protect against deadlock where hook
567567
// changes syncvar, calling hook again.
568-
if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit))
568+
// IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned).
569+
// This prevents hooks from firing at spawn for objects out of AOI range.
570+
if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId))
569571
{
570572
SetSyncVarHookGuard(dirtyBit, true);
571573
OnChanged(oldValue, value);
@@ -592,7 +594,9 @@ public void GeneratedSyncVarSetter_GameObject(GameObject value, ref GameObject f
592594
// in client-only mode, OnDeserialize would call it.
593595
// we use hook guard to protect against deadlock where hook
594596
// changes syncvar, calling hook again.
595-
if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit))
597+
// IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned).
598+
// This prevents hooks from firing at spawn for objects out of AOI range.
599+
if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId))
596600
{
597601
SetSyncVarHookGuard(dirtyBit, true);
598602
OnChanged(oldValue, value);
@@ -619,7 +623,9 @@ public void GeneratedSyncVarSetter_NetworkIdentity(NetworkIdentity value, ref Ne
619623
// in client-only mode, OnDeserialize would call it.
620624
// we use hook guard to protect against deadlock where hook
621625
// changes syncvar, calling hook again.
622-
if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit))
626+
// IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned).
627+
// This prevents hooks from firing at spawn for objects out of AOI range.
628+
if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId))
623629
{
624630
SetSyncVarHookGuard(dirtyBit, true);
625631
OnChanged(oldValue, value);
@@ -647,7 +653,9 @@ public void GeneratedSyncVarSetter_NetworkBehaviour<T>(T value, ref T field, ulo
647653
// in client-only mode, OnDeserialize would call it.
648654
// we use hook guard to protect against deadlock where hook
649655
// changes syncvar, calling hook again.
650-
if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit))
656+
// IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned).
657+
// This prevents hooks from firing at spawn for objects out of AOI range.
658+
if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId))
651659
{
652660
SetSyncVarHookGuard(dirtyBit, true);
653661
OnChanged(oldValue, value);
@@ -795,9 +803,14 @@ public void GeneratedSyncVarDeserialize<T>(ref T field, Action<T, T> OnChanged,
795803
field = value;
796804

797805
// any hook? then call if changed.
798-
if (OnChanged != null && !SyncVarEqual(previous, ref field))
806+
// in host mode initial spawn, also call hook even if value hasn't changed,
807+
// because the field was already set on server but hook wasn't called yet.
808+
if (OnChanged != null)
799809
{
800-
OnChanged(previous, field);
810+
bool changed = !SyncVarEqual(previous, ref field);
811+
bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn;
812+
if (changed || hostInitialSpawnInHostMode)
813+
OnChanged(previous, field);
801814
}
802815
}
803816

@@ -856,9 +869,14 @@ public void GeneratedSyncVarDeserialize_GameObject(ref GameObject field, Action<
856869
field = GetSyncVarGameObject(netIdField, ref field);
857870

858871
// any hook? then call if changed.
859-
if (OnChanged != null && !SyncVarEqual(previousNetId, ref netIdField))
872+
// in host mode initial spawn, also call hook even if value hasn't changed,
873+
// because the field was already set on server but hook wasn't called yet.
874+
if (OnChanged != null)
860875
{
861-
OnChanged(previousGameObject, field);
876+
bool changed = !SyncVarEqual(previousNetId, ref netIdField);
877+
bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn;
878+
if (changed || hostInitialSpawnInHostMode)
879+
OnChanged(previousGameObject, field);
862880
}
863881
}
864882

@@ -918,9 +936,14 @@ public void GeneratedSyncVarDeserialize_NetworkIdentity(ref NetworkIdentity fiel
918936
field = GetSyncVarNetworkIdentity(netIdField, ref field);
919937

920938
// any hook? then call if changed.
921-
if (OnChanged != null && !SyncVarEqual(previousNetId, ref netIdField))
939+
// in host mode initial spawn, also call hook even if value hasn't changed,
940+
// because the field was already set on server but hook wasn't called yet.
941+
if (OnChanged != null)
922942
{
923-
OnChanged(previousIdentity, field);
943+
bool changed = !SyncVarEqual(previousNetId, ref netIdField);
944+
bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn;
945+
if (changed || hostInitialSpawnInHostMode)
946+
OnChanged(previousIdentity, field);
924947
}
925948
}
926949

@@ -982,9 +1005,14 @@ public void GeneratedSyncVarDeserialize_NetworkBehaviour<T>(ref T field, Action<
9821005
field = GetSyncVarNetworkBehaviour(netIdField, ref field);
9831006

9841007
// any hook? then call if changed.
985-
if (OnChanged != null && !SyncVarEqual(previousNetId, ref netIdField))
1008+
// in host mode initial spawn, also call hook even if value hasn't changed,
1009+
// because the field was already set on server but hook wasn't called yet.
1010+
if (OnChanged != null)
9861011
{
987-
OnChanged(previousBehaviour, field);
1012+
bool changed = !SyncVarEqual(previousNetId, ref netIdField);
1013+
bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn;
1014+
if (changed || hostInitialSpawnInHostMode)
1015+
OnChanged(previousBehaviour, field);
9881016
}
9891017
}
9901018

Assets/Mirror/Core/NetworkClient.cs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1384,7 +1384,31 @@ internal static void OnHostClientSpawn(SpawnMessage message)
13841384
aoi.SetHostVisibility(identity, true);
13851385

13861386
identity.isOwned = message.isOwner;
1387-
BootstrapIdentity(identity);
1387+
1388+
// Set flag to indicate initial spawn deserialization in host mode WITH AOI.
1389+
// This forces SyncVar hooks to fire even if values haven't changed,
1390+
// because the field was already set on server but hook wasn't called yet (object was out of range).
1391+
// Only set this flag when AOI is active, otherwise hooks should fire normally via setters.
1392+
identity.hostInitialSpawn = aoi != null;
1393+
1394+
// Configure flags before deserializing
1395+
InitializeIdentityFlags(identity);
1396+
1397+
// Deserialize components if any payload.
1398+
// This will trigger SyncVar hooks via GeneratedSyncVarDeserialize.
1399+
if (message.payload.Count > 0)
1400+
{
1401+
using (NetworkReaderPooled payloadReader = NetworkReaderPool.Get(message.payload))
1402+
{
1403+
identity.DeserializeClient(payloadReader, true);
1404+
}
1405+
}
1406+
1407+
// Clear flag after deserialization
1408+
identity.hostInitialSpawn = false;
1409+
1410+
// Invoke callbacks after deserializing
1411+
InvokeIdentityCallbacks(identity);
13881412
}
13891413
}
13901414

Assets/Mirror/Core/NetworkIdentity.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ public sealed class NetworkIdentity : MonoBehaviour
118118
// internal so NetworkManager can reset it from StopClient.
119119
internal bool clientStarted;
120120

121+
// flag to indicate we're deserializing initial spawn in host mode.
122+
// used to force SyncVar hooks to fire even if value hasn't changed.
123+
// only set temporarily during OnHostClientSpawn deserialization.
124+
internal bool hostInitialSpawn;
125+
121126
/// <summary>The set of network connections (players) that can see this object.</summary>
122127
public readonly Dictionary<int, NetworkConnectionToClient> observers =
123128
new Dictionary<int, NetworkConnectionToClient>();

0 commit comments

Comments
 (0)