From 01e2ca5c0dfb90cbf181fe472aa2daf32e92a2fe Mon Sep 17 00:00:00 2001 From: MrGadget <9826063+MrGadget1024@users.noreply.github.com> Date: Thu, 2 Apr 2026 17:40:27 -0400 Subject: [PATCH 1/8] fix: SyncVar hooks only fire on host client respecting AOI - Weaver generation also preserves oldValue for the hooks. --- Assets/Mirror/Core/Mirror.asmdef | 3 +- Assets/Mirror/Core/NetworkBehaviour.cs | 157 ++++++++++++------ Assets/Mirror/Core/NetworkClient.cs | 6 + Assets/Mirror/Core/NetworkIdentity.cs | 3 + .../Processors/NetworkBehaviourProcessor.cs | 31 +++- .../Processors/SyncVarAttributeProcessor.cs | 63 +++++++ Assets/Mirror/Editor/Weaver/WeaverTypes.cs | 12 ++ 7 files changed, 220 insertions(+), 55 deletions(-) diff --git a/Assets/Mirror/Core/Mirror.asmdef b/Assets/Mirror/Core/Mirror.asmdef index 2fa8d952260..da4ed13bf65 100644 --- a/Assets/Mirror/Core/Mirror.asmdef +++ b/Assets/Mirror/Core/Mirror.asmdef @@ -2,7 +2,8 @@ "name": "Mirror", "rootNamespace": "", "references": [ - "GUID:325984b52e4128546bc7558552f8b1d2" + "GUID:325984b52e4128546bc7558552f8b1d2", + "GUID:3b5390adca4e2bb4791cb930316d6f3e" ], "includePlatforms": [], "excludePlatforms": [], diff --git a/Assets/Mirror/Core/NetworkBehaviour.cs b/Assets/Mirror/Core/NetworkBehaviour.cs index 1f885f0176b..49cb79829d0 100644 --- a/Assets/Mirror/Core/NetworkBehaviour.cs +++ b/Assets/Mirror/Core/NetworkBehaviour.cs @@ -173,6 +173,10 @@ public bool authority // Actions are queued during deserialization and invoked in OnObjectSpawnFinished. internal readonly List deferredSyncCollectionActions = new List(); + // Dictionary to store original SyncVar values before host mode initialization + // This allows us to provide correct oldValue parameters to hooks during deserialization + internal Dictionary hostModeOriginalValues = new Dictionary(); + protected virtual void OnValidate() { // Skip if Editor is in Play mode @@ -202,6 +206,16 @@ protected virtual void OnValidate() #endif } + /// Capture all current SyncVar values before server initialization in host mode. + internal void CaptureHostModeOriginalValues() + { + if (NetworkServer.activeHost) + { + hostModeOriginalValues.Clear(); + // This will be populated by individual SyncVar accessors as needed + } + } + // USED BY WEAVER to set syncvars in host mode without deadlocking protected bool GetSyncVarHookGuard(ulong dirtyBit) => (syncVarHookGuard & dirtyBit) != 0UL; @@ -566,20 +580,22 @@ protected void SendTargetRPCInternal(NetworkConnection conn, string functionFull [MethodImpl(MethodImplOptions.AggressiveInlining)] public void GeneratedSyncVarSetter(T value, ref T field, ulong dirtyBit, Action OnChanged) { + // In host mode, capture original value before any changes + if (NetworkServer.activeHost && !hostModeOriginalValues.ContainsKey(dirtyBit)) + { + hostModeOriginalValues[dirtyBit] = field; + } + if (!SyncVarEqual(value, ref field)) { T oldValue = field; SetSyncVar(value, ref field, dirtyBit); - // call hook (if any) + // call hook (if any) if (OnChanged != null) { - // in host mode, setting a SyncVar calls the hook directly. - // in client-only mode, OnDeserialize would call it. - // we use hook guard to protect against deadlock where hook - // changes syncvar, calling hook again. - // IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned). - // This prevents hooks from firing at spawn for objects out of AOI range. + // Don't fire in host mode during server-side setting + // Hooks will fire later during client deserialization when AOI is known if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId)) { SetSyncVarHookGuard(dirtyBit, true); @@ -810,34 +826,39 @@ public static bool SyncVarNetworkIdentityEqual(NetworkIdentity newIdentity, uint // GeneratedSyncVarDeserialize(reader, ref health, null, reader.ReadInt()); // } // } - public void GeneratedSyncVarDeserialize(ref T field, Action OnChanged, T value) + public void GeneratedSyncVarDeserialize(ref T field, Action OnChanged, T value, ulong dirtyBit) { T previous = field; field = value; - // any hook? then call if changed. - // in host mode initial spawn, also call hook even if value hasn't changed, - // because the field was already set on server but hook wasn't called yet. if (OnChanged != null) { bool changed = !SyncVarEqual(previous, ref field); bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn; + if (changed || hostInitialSpawnInHostMode) { - // Defer hooks during initial spawn on pure client to eliminate - // cross-object reference race conditions. All objects will be in - // NetworkClient.spawned before any hooks fire. - if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) + // Use captured original value for correct old value in host mode + T actualPrevious = previous; + if (hostInitialSpawnInHostMode && hostModeOriginalValues.TryGetValue(dirtyBit, out object original)) { - // Capture values in closure for deferred execution - T capturedPrevious = previous; - T capturedNew = field; - deferredSyncVarHooks.Add(() => OnChanged(capturedPrevious, capturedNew)); + actualPrevious = (T)original; + changed = !SyncVarEqual(actualPrevious, ref field); // Re-check with correct old value } - else + + // Only fire if actually changed and visible per AOI + if (changed && NetworkClient.spawned.ContainsKey(netIdentity.netId)) { - // Normal: invoke immediately (host mode, server, or after spawn finished) - OnChanged(previous, field); + if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) + { + T capturedPrevious = actualPrevious; + T capturedNew = field; + deferredSyncVarHooks.Add(() => OnChanged(capturedPrevious, capturedNew)); + } + else + { + OnChanged(actualPrevious, field); + } } } } @@ -888,7 +909,7 @@ public void GeneratedSyncVarDeserialize(ref T field, Action OnChanged, // GeneratedSyncVarDeserialize_GameObject(reader, ref target, OnChangedNB, ref ___targetNetId); // } // } - public void GeneratedSyncVarDeserialize_GameObject(ref GameObject field, Action OnChanged, NetworkReader reader, ref uint netIdField) + public void GeneratedSyncVarDeserialize_GameObject(ref GameObject field, Action OnChanged, NetworkReader reader, ref uint netIdField, ulong dirtyBit) { uint previousNetId = netIdField; GameObject previousGameObject = field; @@ -898,23 +919,35 @@ public void GeneratedSyncVarDeserialize_GameObject(ref GameObject field, Action< field = GetSyncVarGameObject(netIdField, ref field); // any hook? then call if changed. - // in host mode initial spawn, also call hook even if value hasn't changed, - // because the field was already set on server but hook wasn't called yet. if (OnChanged != null) { bool changed = !SyncVarEqual(previousNetId, ref netIdField); bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn; + if (changed || hostInitialSpawnInHostMode) { - if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) + // Use captured original value for correct old value in host mode + GameObject actualPrevious = previousGameObject; + if (hostInitialSpawnInHostMode && hostModeOriginalValues.TryGetValue(dirtyBit, out object original)) { - GameObject capturedPrevious = previousGameObject; - GameObject capturedNew = field; - deferredSyncVarHooks.Add(() => OnChanged(capturedPrevious, capturedNew)); + actualPrevious = (GameObject)original; + // Re-check with correct old value by comparing original GameObject to new one + changed = !SyncVarEqual(actualPrevious, ref field); } - else + + // Only fire if actually changed and visible per AOI + if (changed && NetworkClient.spawned.ContainsKey(netIdentity.netId)) { - OnChanged(previousGameObject, field); + if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) + { + GameObject capturedPrevious = actualPrevious; + GameObject capturedNew = field; + deferredSyncVarHooks.Add(() => OnChanged(capturedPrevious, capturedNew)); + } + else + { + OnChanged(actualPrevious, field); + } } } } @@ -966,7 +999,7 @@ public void GeneratedSyncVarDeserialize_GameObject(ref GameObject field, Action< // GeneratedSyncVarDeserialize_NetworkIdentity(reader, ref target, OnChangedNI, ref ___targetNetId); // } // } - public void GeneratedSyncVarDeserialize_NetworkIdentity(ref NetworkIdentity field, Action OnChanged, NetworkReader reader, ref uint netIdField) + public void GeneratedSyncVarDeserialize_NetworkIdentity(ref NetworkIdentity field, Action OnChanged, NetworkReader reader, ref uint netIdField, ulong dirtyBit) { uint previousNetId = netIdField; NetworkIdentity previousIdentity = field; @@ -976,23 +1009,35 @@ public void GeneratedSyncVarDeserialize_NetworkIdentity(ref NetworkIdentity fiel field = GetSyncVarNetworkIdentity(netIdField, ref field); // any hook? then call if changed. - // in host mode initial spawn, also call hook even if value hasn't changed, - // because the field was already set on server but hook wasn't called yet. if (OnChanged != null) { bool changed = !SyncVarEqual(previousNetId, ref netIdField); bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn; + if (changed || hostInitialSpawnInHostMode) { - if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) + // Use captured original value for correct old value in host mode + NetworkIdentity actualPrevious = previousIdentity; + if (hostInitialSpawnInHostMode && hostModeOriginalValues.TryGetValue(dirtyBit, out object original)) { - NetworkIdentity capturedPrevious = previousIdentity; - NetworkIdentity capturedNew = field; - deferredSyncVarHooks.Add(() => OnChanged(capturedPrevious, capturedNew)); + actualPrevious = (NetworkIdentity)original; + // Re-check with correct old value by comparing original NetworkIdentity to new one + changed = !SyncVarEqual(actualPrevious, ref field); } - else + + // Only fire if actually changed and visible per AOI + if (changed && NetworkClient.spawned.ContainsKey(netIdentity.netId)) { - OnChanged(previousIdentity, field); + if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) + { + NetworkIdentity capturedPrevious = actualPrevious; + NetworkIdentity capturedNew = field; + deferredSyncVarHooks.Add(() => OnChanged(capturedPrevious, capturedNew)); + } + else + { + OnChanged(actualPrevious, field); + } } } } @@ -1045,8 +1090,8 @@ public void GeneratedSyncVarDeserialize_NetworkIdentity(ref NetworkIdentity fiel // GeneratedSyncVarDeserialize_NetworkBehaviour(reader, ref target, OnChangedNB, ref ___targetNetId); // } // } - public void GeneratedSyncVarDeserialize_NetworkBehaviour(ref T field, Action OnChanged, NetworkReader reader, ref NetworkBehaviourSyncVar netIdField) - where T : NetworkBehaviour + public void GeneratedSyncVarDeserialize_NetworkBehaviour(ref T field, Action OnChanged, NetworkReader reader, ref NetworkBehaviourSyncVar netIdField, ulong dirtyBit) + where T : NetworkBehaviour { NetworkBehaviourSyncVar previousNetId = netIdField; T previousBehaviour = field; @@ -1056,23 +1101,35 @@ public void GeneratedSyncVarDeserialize_NetworkBehaviour(ref T field, Action< field = GetSyncVarNetworkBehaviour(netIdField, ref field); // any hook? then call if changed. - // in host mode initial spawn, also call hook even if value hasn't changed, - // because the field was already set on server but hook wasn't called yet. if (OnChanged != null) { bool changed = !SyncVarEqual(previousNetId, ref netIdField); bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn; + if (changed || hostInitialSpawnInHostMode) { - if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) + // Use captured original value for correct old value in host mode + T actualPrevious = previousBehaviour; + if (hostInitialSpawnInHostMode && hostModeOriginalValues.TryGetValue(dirtyBit, out object original)) { - T capturedPrevious = previousBehaviour; - T capturedNew = field; - deferredSyncVarHooks.Add(() => OnChanged(capturedPrevious, capturedNew)); + actualPrevious = (T)original; + // Re-check with correct old value by comparing original NetworkBehaviour to new one + changed = !SyncVarEqual(actualPrevious, ref field); } - else + + // Only fire if actually changed and visible per AOI + if (changed && NetworkClient.spawned.ContainsKey(netIdentity.netId)) { - OnChanged(previousBehaviour, field); + if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) + { + T capturedPrevious = actualPrevious; + T capturedNew = field; + deferredSyncVarHooks.Add(() => OnChanged(capturedPrevious, capturedNew)); + } + else + { + OnChanged(actualPrevious, field); + } } } } diff --git a/Assets/Mirror/Core/NetworkClient.cs b/Assets/Mirror/Core/NetworkClient.cs index fd2bf465123..a867f50d8e6 100644 --- a/Assets/Mirror/Core/NetworkClient.cs +++ b/Assets/Mirror/Core/NetworkClient.cs @@ -1425,6 +1425,12 @@ internal static void OnHostClientSpawn(SpawnMessage message) // Invoke callbacks after deserializing InvokeIdentityCallbacks(identity); + + // Clear stored original SyncVar values + foreach (NetworkBehaviour comp in identity.NetworkBehaviours) + { + comp.hostModeOriginalValues.Clear(); + } } } diff --git a/Assets/Mirror/Core/NetworkIdentity.cs b/Assets/Mirror/Core/NetworkIdentity.cs index 9ffe04d0a97..c444c75d272 100644 --- a/Assets/Mirror/Core/NetworkIdentity.cs +++ b/Assets/Mirror/Core/NetworkIdentity.cs @@ -743,6 +743,9 @@ internal void OnStartServer() // one exception doesn't stop all the other Start() calls! try { + if (NetworkServer.activeHost) + comp.CaptureHostModeOriginalValues(); + comp.OnStartServer(); } catch (Exception e) diff --git a/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs index 47f1b940487..83a16c548b4 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs @@ -78,6 +78,12 @@ public bool Process(ref bool WeavingFailed) syncObjects = SyncObjectProcessor.FindSyncObjectsFields(writers, readers, Log, netBehaviourSubclass, ref WeavingFailed); + // Generate CaptureHostModeOriginalValues method + if (syncVars.Count > 0) + { + syncVarAttributeProcessor.GenerateCaptureHostModeOriginalValues(netBehaviourSubclass, syncVars, ref WeavingFailed); + } + ProcessMethods(ref WeavingFailed); if (WeavingFailed) { @@ -576,7 +582,7 @@ void GenerateSerialization(ref bool WeavingFailed) netBehaviourSubclass.Methods.Add(serialize); } - void DeserializeField(FieldDefinition syncVar, ILProcessor worker, ref bool WeavingFailed) + void DeserializeField(FieldDefinition syncVar, ILProcessor worker, int dirtyBit, ref bool WeavingFailed) { // put 'this.' onto stack for 'this.syncvar' below worker.Append(worker.Create(OpCodes.Ldarg_0)); @@ -620,6 +626,10 @@ void DeserializeField(FieldDefinition syncVar, ILProcessor worker, ref bool Weav FieldDefinition netIdField = syncVarNetIds[syncVar]; worker.Emit(OpCodes.Ldarg_0); worker.Emit(OpCodes.Ldflda, netIdField); + + // NEW: push dirtyBit + worker.Emit(OpCodes.Ldc_I8, 1L << dirtyBit); + worker.Emit(OpCodes.Call, weaverTypes.generatedSyncVarDeserialize_GameObject); } else if (syncVar.FieldType.Is()) @@ -631,6 +641,10 @@ void DeserializeField(FieldDefinition syncVar, ILProcessor worker, ref bool Weav FieldDefinition netIdField = syncVarNetIds[syncVar]; worker.Emit(OpCodes.Ldarg_0); worker.Emit(OpCodes.Ldflda, netIdField); + + // NEW: push dirtyBit + worker.Emit(OpCodes.Ldc_I8, 1L << dirtyBit); + worker.Emit(OpCodes.Call, weaverTypes.generatedSyncVarDeserialize_NetworkIdentity); } // handle both NetworkBehaviour and inheritors. @@ -645,6 +659,10 @@ void DeserializeField(FieldDefinition syncVar, ILProcessor worker, ref bool Weav FieldDefinition netIdField = syncVarNetIds[syncVar]; worker.Emit(OpCodes.Ldarg_0); worker.Emit(OpCodes.Ldflda, netIdField); + + // NEW: push dirtyBit + worker.Emit(OpCodes.Ldc_I8, 1L << dirtyBit); + // make generic version of GeneratedSyncVarSetter_NetworkBehaviour MethodReference getFunc = weaverTypes.generatedSyncVarDeserialize_NetworkBehaviour_T.MakeGeneric(assembly.MainModule, syncVar.FieldType); worker.Emit(OpCodes.Call, getFunc); @@ -667,6 +685,9 @@ void DeserializeField(FieldDefinition syncVar, ILProcessor worker, ref bool Weav // reader.Read() worker.Emit(OpCodes.Call, readFunc); + // NEW: push dirtyBit + worker.Emit(OpCodes.Ldc_I8, 1L << dirtyBit); + // make generic version of GeneratedSyncVarDeserialize MethodReference generic = weaverTypes.generatedSyncVarDeserialize.MakeGeneric(assembly.MainModule, syncVar.FieldType); worker.Emit(OpCodes.Call, generic); @@ -715,9 +736,11 @@ void GenerateDeSerialization(ref bool WeavingFailed) serWorker.Append(serWorker.Create(OpCodes.Ldarg_2)); serWorker.Append(serWorker.Create(OpCodes.Brfalse, initialStateLabel)); + int dirtyBit = syncVarAccessLists.GetSyncVarStart(netBehaviourSubclass.BaseType.FullName); foreach (FieldDefinition syncVar in syncVars) { - DeserializeField(syncVar, serWorker, ref WeavingFailed); + DeserializeField(syncVar, serWorker, dirtyBit, ref WeavingFailed); // Pass dirtyBit + dirtyBit += 1; } serWorker.Append(serWorker.Create(OpCodes.Ret)); @@ -732,7 +755,7 @@ void GenerateDeSerialization(ref bool WeavingFailed) // conditionally read each syncvar // start at number of syncvars in parent - int dirtyBit = syncVarAccessLists.GetSyncVarStart(netBehaviourSubclass.BaseType.FullName); + dirtyBit = syncVarAccessLists.GetSyncVarStart(netBehaviourSubclass.BaseType.FullName); foreach (FieldDefinition syncVar in syncVars) { Instruction varLabel = serWorker.Create(OpCodes.Nop); @@ -743,7 +766,7 @@ void GenerateDeSerialization(ref bool WeavingFailed) serWorker.Append(serWorker.Create(OpCodes.And)); serWorker.Append(serWorker.Create(OpCodes.Brfalse, varLabel)); - DeserializeField(syncVar, serWorker, ref WeavingFailed); + DeserializeField(syncVar, serWorker, dirtyBit, ref WeavingFailed); // Pass dirtyBit serWorker.Append(varLabel); dirtyBit += 1; diff --git a/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs index 76e3ac7ed5c..8fff664b371 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs @@ -252,6 +252,69 @@ public MethodDefinition GenerateSyncVarGetter(FieldDefinition fd, string origina return get; } + // Generates the CaptureHostModeOriginalValues method that captures original SyncVar field values + // before OnStartServer runs in host mode. This fixes the issue where SyncVar hooks would fire + // with incorrect oldValue parameters (oldValue == newValue) because the server had already + // modified the fields before client deserialization occurred. + public void GenerateCaptureHostModeOriginalValues(TypeDefinition td, List syncVars, ref bool WeavingFailed) + { + // Override the empty CaptureHostModeOriginalValues method from NetworkBehaviour base class + const string MethodName = "CaptureHostModeOriginalValues"; + + MethodDefinition method = new MethodDefinition(MethodName, + MethodAttributes.Family | MethodAttributes.Virtual | MethodAttributes.HideBySig | MethodAttributes.ReuseSlot, + weaverTypes.Import(typeof(void))); + + ILProcessor worker = method.Body.GetILProcessor(); + method.Body.InitLocals = true; + + // Generate early return if not in host mode: if (!NetworkServer.activeHost) return; + // Only capture values in host mode where both server and client are active + Instruction returnLabel = worker.Create(OpCodes.Ret); + worker.Emit(OpCodes.Call, weaverTypes.NetworkServerGetActive); + worker.Emit(OpCodes.Brfalse, returnLabel); + worker.Emit(OpCodes.Call, weaverTypes.NetworkClientGetActive); + worker.Emit(OpCodes.Brfalse, returnLabel); + + // Generate: hostModeOriginalValues.Clear(); + // Clear the dictionary to start fresh each time + worker.Emit(OpCodes.Ldarg_0); + worker.Emit(OpCodes.Ldfld, weaverTypes.hostModeOriginalValuesReference); + worker.Emit(OpCodes.Callvirt, weaverTypes.dictionaryClearReference); + + // Generate capture code for each SyncVar: hostModeOriginalValues[dirtyBit] = field; + // Start dirtyBit counting from parent class SyncVar count to avoid conflicts + int dirtyBit = syncVarAccessLists.GetSyncVarStart(td.BaseType.FullName); + foreach (FieldDefinition syncVar in syncVars) + { + // Load the dictionary for indexer access: hostModeOriginalValues + worker.Emit(OpCodes.Ldarg_0); + worker.Emit(OpCodes.Ldfld, weaverTypes.hostModeOriginalValuesReference); + + // Load the dirtyBit key as ulong (1L << dirtyBit creates unique bit masks: 1, 2, 4, 8...) + // This key will be used later during deserialization to look up the original value + worker.Emit(OpCodes.Ldc_I8, 1L << dirtyBit); + + // Load the current field value (this captures the original value before OnStartServer changes it) + worker.Emit(OpCodes.Ldarg_0); + worker.Emit(OpCodes.Ldfld, syncVar); + + // Box value types since dictionary stores object values + if (syncVar.FieldType.IsValueType) + { + worker.Emit(OpCodes.Box, syncVar.FieldType); + } + + // Call dictionary setter: dictionary[key] = value + worker.Emit(OpCodes.Callvirt, weaverTypes.dictionarySetItemReference); + + dirtyBit += 1; + } + + worker.Append(returnLabel); + td.Methods.Add(method); + } + // for [SyncVar] health, weaver generates // // NetworkHealth diff --git a/Assets/Mirror/Editor/Weaver/WeaverTypes.cs b/Assets/Mirror/Editor/Weaver/WeaverTypes.cs index 27c3b7ba304..9696986d33a 100644 --- a/Assets/Mirror/Editor/Weaver/WeaverTypes.cs +++ b/Assets/Mirror/Editor/Weaver/WeaverTypes.cs @@ -1,5 +1,7 @@ using System; +using System.Collections.Generic; using Mono.CecilX; +using Mono.CecilX.Rocks; using UnityEditor; using UnityEngine; @@ -34,6 +36,10 @@ public class WeaverTypes // Action for SyncVar Hooks public MethodReference ActionT_T; + public FieldReference hostModeOriginalValuesReference; + public MethodReference dictionaryClearReference; + public MethodReference dictionarySetItemReference; + // syncvar public MethodReference generatedSyncVarSetter; public MethodReference generatedSyncVarSetter_GameObject; @@ -95,6 +101,12 @@ public WeaverTypes(AssemblyDefinition assembly, Logger Log, ref bool WeavingFail TypeReference NetworkBehaviourType = Import(); + hostModeOriginalValuesReference = Resolvers.ResolveField(NetworkBehaviourType, assembly, Log, "hostModeOriginalValues", ref WeavingFailed); + + TypeReference dictionaryType = Import(typeof(Dictionary<,>)).MakeGenericInstanceType(Import(), Import()); + dictionaryClearReference = Resolvers.ResolveMethod(dictionaryType, assembly, Log, "Clear", ref WeavingFailed); + dictionarySetItemReference = Resolvers.ResolveMethod(dictionaryType, assembly, Log, "set_Item", ref WeavingFailed); + NetworkBehaviourIsClientReference = Resolvers.ResolveMethod(NetworkBehaviourType, assembly, Log, "get_isClient", ref WeavingFailed); NetworkBehaviourIsServerReference = Resolvers.ResolveMethod(NetworkBehaviourType, assembly, Log, "get_isServer", ref WeavingFailed); From b2a7173616eac7914e1db78d748e62d112053113 Mon Sep 17 00:00:00 2001 From: MrGadget <9826063+MrGadget1024@users.noreply.github.com> Date: Thu, 2 Apr 2026 17:49:03 -0400 Subject: [PATCH 2/8] Update Assets/Mirror/Core/Mirror.asmdef --- Assets/Mirror/Core/Mirror.asmdef | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Assets/Mirror/Core/Mirror.asmdef b/Assets/Mirror/Core/Mirror.asmdef index da4ed13bf65..2fa8d952260 100644 --- a/Assets/Mirror/Core/Mirror.asmdef +++ b/Assets/Mirror/Core/Mirror.asmdef @@ -2,8 +2,7 @@ "name": "Mirror", "rootNamespace": "", "references": [ - "GUID:325984b52e4128546bc7558552f8b1d2", - "GUID:3b5390adca4e2bb4791cb930316d6f3e" + "GUID:325984b52e4128546bc7558552f8b1d2" ], "includePlatforms": [], "excludePlatforms": [], From 04199cdd4efa0a3b986a98b74cb0f64a9c58bbb2 Mon Sep 17 00:00:00 2001 From: MrGadget <9826063+MrGadget1024@users.noreply.github.com> Date: Thu, 2 Apr 2026 18:12:18 -0400 Subject: [PATCH 3/8] Updated Test --- .../SyncVars/SyncVarHookDeferralTests.cs | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/Assets/Mirror/Tests/Editor/SyncVars/SyncVarHookDeferralTests.cs b/Assets/Mirror/Tests/Editor/SyncVars/SyncVarHookDeferralTests.cs index a83a3701fe7..1191e8d7696 100644 --- a/Assets/Mirror/Tests/Editor/SyncVars/SyncVarHookDeferralTests.cs +++ b/Assets/Mirror/Tests/Editor/SyncVars/SyncVarHookDeferralTests.cs @@ -107,7 +107,7 @@ public void GameObject_CrossReferences_HooksCanAccessTargets() } [Test] - public void HostMode_CrossReferences_HooksFireFromSetter_AndDuringDeserialize() + public void HostMode_CrossReferences_HooksFireOnceWithCorrectOldValues() { // Start in HOST mode (server + client together) NetworkServer.Listen(10); @@ -126,31 +126,35 @@ public void HostMode_CrossReferences_HooksFireFromSetter_AndDuringDeserialize() comp2.target = identity3; comp3.target = comp1.netIdentity; - // In host mode, hooks fire TWICE: - // 1. From setter when value is assigned - // 2. From DeserializeClient during OnHostClientSpawn (because hostInitialSpawn=true) - Assert.That(comp1.callCount, Is.EqualTo(2), - "Host mode: Hook should fire from setter immediately"); - Assert.That(comp2.callCount, Is.EqualTo(2), - "Host mode: Hook should fire from setter immediately"); - Assert.That(comp3.callCount, Is.EqualTo(2), - "Host mode: Hook should fire from setter immediately"); - - // All targets should be accessible (host mode, everything is local) + // After our fix: hooks fire ONCE during client deserialization, not from setters + // This provides correct old values (captured originals) and respects AOI + Assert.That(comp1.callCount, Is.EqualTo(1), + "Host mode: Hook should fire once during deserialization with correct old value"); + Assert.That(comp2.callCount, Is.EqualTo(1), + "Host mode: Hook should fire once during deserialization with correct old value"); + Assert.That(comp3.callCount, Is.EqualTo(1), + "Host mode: Hook should fire once during deserialization with correct old value"); + + // All targets should be accessible (host mode, everything is local and visible per AOI) Assert.That(comp1.targetWasInSpawnedWhenHookFired, Is.True, - "Host mode: Targets always accessible"); + "Host mode: Targets accessible when hooks fire during deserialization"); Assert.That(comp2.targetWasInSpawnedWhenHookFired, Is.True, - "Host mode: Targets always accessible"); + "Host mode: Targets accessible when hooks fire during deserialization"); Assert.That(comp3.targetWasInSpawnedWhenHookFired, Is.True, - "Host mode: Targets always accessible"); + "Host mode: Targets accessible when hooks fire during deserialization"); - // Verify no deferred hooks queued (host mode doesn't defer) + // Verify no deferred hooks queued (host mode fires immediately during deserialization) Assert.That(comp1.deferredSyncVarHooks.Count, Is.EqualTo(0), "Host mode should not defer hooks"); Assert.That(comp2.deferredSyncVarHooks.Count, Is.EqualTo(0), "Host mode should not defer hooks"); Assert.That(comp3.deferredSyncVarHooks.Count, Is.EqualTo(0), "Host mode should not defer hooks"); + + // Verify references are correct + Assert.That(comp1.target, Is.EqualTo(identity2)); + Assert.That(comp2.target, Is.EqualTo(identity3)); + Assert.That(comp3.target, Is.EqualTo(comp1.netIdentity)); } [Test] From 7da5a3002653ba9f378df926f27e08e6c76a6178 Mon Sep 17 00:00:00 2001 From: MrGadget <9826063+MrGadget1024@users.noreply.github.com> Date: Fri, 3 Apr 2026 02:51:58 -0400 Subject: [PATCH 4/8] NetworkClient::OnHostClientObjectHide remove from spawned --- Assets/Mirror/Core/NetworkClient.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Assets/Mirror/Core/NetworkClient.cs b/Assets/Mirror/Core/NetworkClient.cs index a867f50d8e6..1b769498f4b 100644 --- a/Assets/Mirror/Core/NetworkClient.cs +++ b/Assets/Mirror/Core/NetworkClient.cs @@ -1374,12 +1374,13 @@ internal static void OnObjectSpawnFinished(ObjectSpawnFinishedMessage _) // host mode callbacks ///////////////////////////////////////////////// static void OnHostClientObjectHide(ObjectHideMessage message) { - //Debug.Log($"ClientScene::OnLocalObjectObjHide netId:{message.netId}"); - if (spawned.TryGetValue(message.netId, out NetworkIdentity identity) && - identity != null) + //Debug.Log($"NetworkClient::OnHostClientObjectHide netId:{message.netId}"); + if (spawned.TryGetValue(message.netId, out NetworkIdentity identity) && identity != null) { if (aoi != null) aoi.SetHostVisibility(identity, false); + + spawned.Remove(message.netId); } } From 79b267e2b6cc36e8f50034c50cc88cda5bed4745 Mon Sep 17 00:00:00 2001 From: MrGadget <9826063+MrGadget1024@users.noreply.github.com> Date: Fri, 3 Apr 2026 03:11:35 -0400 Subject: [PATCH 5/8] Code formatting --- Assets/Mirror/Core/NetworkClient.cs | 2 -- .../Editor/Weaver/Processors/NetworkBehaviourProcessor.cs | 5 ----- .../Editor/Weaver/Processors/SyncVarAttributeProcessor.cs | 2 -- 3 files changed, 9 deletions(-) diff --git a/Assets/Mirror/Core/NetworkClient.cs b/Assets/Mirror/Core/NetworkClient.cs index 1b769498f4b..11e6094cc22 100644 --- a/Assets/Mirror/Core/NetworkClient.cs +++ b/Assets/Mirror/Core/NetworkClient.cs @@ -1429,9 +1429,7 @@ internal static void OnHostClientSpawn(SpawnMessage message) // Clear stored original SyncVar values foreach (NetworkBehaviour comp in identity.NetworkBehaviours) - { comp.hostModeOriginalValues.Clear(); - } } } diff --git a/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs index 83a16c548b4..b44786c314d 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs @@ -67,10 +67,7 @@ public bool Process(ref bool WeavingFailed) { // only process once if (WasProcessed(netBehaviourSubclass)) - { return false; - } - MarkAsProcessed(netBehaviourSubclass); // deconstruct tuple and set fields @@ -80,9 +77,7 @@ public bool Process(ref bool WeavingFailed) // Generate CaptureHostModeOriginalValues method if (syncVars.Count > 0) - { syncVarAttributeProcessor.GenerateCaptureHostModeOriginalValues(netBehaviourSubclass, syncVars, ref WeavingFailed); - } ProcessMethods(ref WeavingFailed); if (WeavingFailed) diff --git a/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs index 8fff664b371..1dda4cdfeda 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs @@ -301,9 +301,7 @@ public void GenerateCaptureHostModeOriginalValues(TypeDefinition td, List Date: Fri, 3 Apr 2026 03:18:31 -0400 Subject: [PATCH 6/8] Code formatting --- Assets/Mirror/Core/NetworkBehaviour.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Assets/Mirror/Core/NetworkBehaviour.cs b/Assets/Mirror/Core/NetworkBehaviour.cs index 49cb79829d0..c9aba192abf 100644 --- a/Assets/Mirror/Core/NetworkBehaviour.cs +++ b/Assets/Mirror/Core/NetworkBehaviour.cs @@ -206,14 +206,11 @@ protected virtual void OnValidate() #endif } - /// Capture all current SyncVar values before server initialization in host mode. + // USED BY WEAVER to capture all original SyncVar values before server initialization in host mode. internal void CaptureHostModeOriginalValues() { if (NetworkServer.activeHost) - { hostModeOriginalValues.Clear(); - // This will be populated by individual SyncVar accessors as needed - } } // USED BY WEAVER to set syncvars in host mode without deadlocking @@ -582,9 +579,7 @@ public void GeneratedSyncVarSetter(T value, ref T field, ulong dirtyBit, Acti { // In host mode, capture original value before any changes if (NetworkServer.activeHost && !hostModeOriginalValues.ContainsKey(dirtyBit)) - { hostModeOriginalValues[dirtyBit] = field; - } if (!SyncVarEqual(value, ref field)) { From 01f55e17eaf97044aab9f9dd25898639d4a6746a Mon Sep 17 00:00:00 2001 From: MrGadget <9826063+MrGadget1024@users.noreply.github.com> Date: Fri, 3 Apr 2026 03:21:05 -0400 Subject: [PATCH 7/8] Code formatting --- Assets/Mirror/Core/NetworkBehaviour.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Assets/Mirror/Core/NetworkBehaviour.cs b/Assets/Mirror/Core/NetworkBehaviour.cs index c9aba192abf..0baa605d65e 100644 --- a/Assets/Mirror/Core/NetworkBehaviour.cs +++ b/Assets/Mirror/Core/NetworkBehaviour.cs @@ -298,7 +298,7 @@ protected void InitSyncObject(SyncObject syncObject) // Store back-reference to this NetworkBehaviour syncObject.networkBehaviour = this; - + // add it, remember the index in list (if Count=0, index=0 etc.) int index = syncObjects.Count; syncObjects.Add(syncObject); @@ -586,7 +586,7 @@ public void GeneratedSyncVarSetter(T value, ref T field, ulong dirtyBit, Acti T oldValue = field; SetSyncVar(value, ref field, dirtyBit); - // call hook (if any) + // call hook (if any) if (OnChanged != null) { // Don't fire in host mode during server-side setting From 506808a8628cf65863d47b4f55c7a5ab94d8941c Mon Sep 17 00:00:00 2001 From: MrGadget <9826063+MrGadget1024@users.noreply.github.com> Date: Mon, 13 Apr 2026 12:03:23 -0400 Subject: [PATCH 8/8] Hooks now fire when set before or after .Spawn in same frame --- Assets/Mirror/Core/NetworkBehaviour.cs | 70 +++-- Assets/Mirror/Core/NetworkIdentity.cs | 8 +- Assets/Mirror/Core/NetworkServer.cs | 33 +++ .../Processors/SyncVarAttributeProcessor.cs | 34 +-- Assets/Mirror/Editor/Weaver/WeaverTypes.cs | 9 +- .../SyncVars/SyncVarHookDeferralTests.cs | 264 ++++++++++++++++++ 6 files changed, 371 insertions(+), 47 deletions(-) diff --git a/Assets/Mirror/Core/NetworkBehaviour.cs b/Assets/Mirror/Core/NetworkBehaviour.cs index 0baa605d65e..d01eab23b7a 100644 --- a/Assets/Mirror/Core/NetworkBehaviour.cs +++ b/Assets/Mirror/Core/NetworkBehaviour.cs @@ -175,7 +175,7 @@ public bool authority // Dictionary to store original SyncVar values before host mode initialization // This allows us to provide correct oldValue parameters to hooks during deserialization - internal Dictionary hostModeOriginalValues = new Dictionary(); + protected internal Dictionary hostModeOriginalValues = new Dictionary(); protected virtual void OnValidate() { @@ -207,10 +207,17 @@ protected virtual void OnValidate() } // USED BY WEAVER to capture all original SyncVar values before server initialization in host mode. - internal void CaptureHostModeOriginalValues() + internal virtual void CaptureHostModeOriginalValues() { } + + // Helper methods for weaver-generated CaptureHostModeOriginalValues + protected void ClearHostModeOriginalValues() { - if (NetworkServer.activeHost) - hostModeOriginalValues.Clear(); + hostModeOriginalValues.Clear(); + } + + protected void StoreHostModeOriginalValue(ulong dirtyBit, object value) + { + hostModeOriginalValues[dirtyBit] = value; } // USED BY WEAVER to set syncvars in host mode without deadlocking @@ -577,20 +584,23 @@ protected void SendTargetRPCInternal(NetworkConnection conn, string functionFull [MethodImpl(MethodImplOptions.AggressiveInlining)] public void GeneratedSyncVarSetter(T value, ref T field, ulong dirtyBit, Action OnChanged) { - // In host mode, capture original value before any changes - if (NetworkServer.activeHost && !hostModeOriginalValues.ContainsKey(dirtyBit)) - hostModeOriginalValues[dirtyBit] = field; + //Debug.Log($"SERVER SETTER: {gameObject.name} setting field from '{field}' to '{value}', dirtyBit={dirtyBit}, isServer={isServer}"); if (!SyncVarEqual(value, ref field)) { T oldValue = field; SetSyncVar(value, ref field, dirtyBit); + //Debug.Log($"SERVER SETTER: Field changed, dirty bit set. syncVarDirtyBits={syncVarDirtyBits}"); + // call hook (if any) if (OnChanged != null) { // Don't fire in host mode during server-side setting // Hooks will fire later during client deserialization when AOI is known + // We use hook guard to protect against deadlock where hook changes syncvar, calling hook again. + // IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned). + // This prevents hooks from firing at spawn for objects out of AOI range. if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId)) { SetSyncVarHookGuard(dirtyBit, true); @@ -599,6 +609,10 @@ public void GeneratedSyncVarSetter(T value, ref T field, ulong dirtyBit, Acti } } } + //else + //{ + // Debug.Log($"SERVER SETTER: No change, not marking dirty"); + //} } // GameObject needs custom handling for persistence via netId. @@ -614,10 +628,9 @@ public void GeneratedSyncVarSetter_GameObject(GameObject value, ref GameObject f // call hook (if any) if (OnChanged != null) { - // in host mode, setting a SyncVar calls the hook directly. - // in client-only mode, OnDeserialize would call it. - // we use hook guard to protect against deadlock where hook - // changes syncvar, calling hook again. + // Don't fire in host mode during server-side setting + // Hooks will fire later during client deserialization when AOI is known + // We use hook guard to protect against deadlock where hook changes syncvar, calling hook again. // IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned). // This prevents hooks from firing at spawn for objects out of AOI range. if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId)) @@ -643,10 +656,9 @@ public void GeneratedSyncVarSetter_NetworkIdentity(NetworkIdentity value, ref Ne // call hook (if any) if (OnChanged != null) { - // in host mode, setting a SyncVar calls the hook directly. - // in client-only mode, OnDeserialize would call it. - // we use hook guard to protect against deadlock where hook - // changes syncvar, calling hook again. + // Don't fire in host mode during server-side setting + // Hooks will fire later during client deserialization when AOI is known + // We use hook guard to protect against deadlock where hook changes syncvar, calling hook again. // IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned). // This prevents hooks from firing at spawn for objects out of AOI range. if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId)) @@ -673,10 +685,9 @@ public void GeneratedSyncVarSetter_NetworkBehaviour(T value, ref T field, ulo // call hook (if any) if (OnChanged != null) { - // in host mode, setting a SyncVar calls the hook directly. - // in client-only mode, OnDeserialize would call it. - // we use hook guard to protect against deadlock where hook - // changes syncvar, calling hook again. + // Don't fire in host mode during server-side setting + // Hooks will fire later during client deserialization when AOI is known + // We use hook guard to protect against deadlock where hook changes syncvar, calling hook again. // IMPORTANT: only call hook if object is visible to host client (in NetworkClient.spawned). // This prevents hooks from firing at spawn for objects out of AOI range. if (NetworkServer.activeHost && !GetSyncVarHookGuard(dirtyBit) && NetworkClient.spawned.ContainsKey(netIdentity.netId)) @@ -823,6 +834,8 @@ public static bool SyncVarNetworkIdentityEqual(NetworkIdentity newIdentity, uint // } public void GeneratedSyncVarDeserialize(ref T field, Action OnChanged, T value, ulong dirtyBit) { + //Debug.Log($"Deserialize called for {gameObject.name}: field={field}, value={value}, dirtyBit={dirtyBit}"); + T previous = field; field = value; @@ -831,19 +844,31 @@ public void GeneratedSyncVarDeserialize(ref T field, Action OnChanged, bool changed = !SyncVarEqual(previous, ref field); bool hostInitialSpawnInHostMode = NetworkServer.activeHost && netIdentity.hostInitialSpawn; + //Debug.Log($" changed={changed}, hostInitialSpawn={hostInitialSpawnInHostMode}"); + if (changed || hostInitialSpawnInHostMode) { // Use captured original value for correct old value in host mode T actualPrevious = previous; if (hostInitialSpawnInHostMode && hostModeOriginalValues.TryGetValue(dirtyBit, out object original)) { + //Debug.Log($" Found captured original: {original}"); actualPrevious = (T)original; changed = !SyncVarEqual(actualPrevious, ref field); // Re-check with correct old value + //Debug.Log($" After using captured: changed={changed}, actualPrevious={actualPrevious}"); } + //else if (hostInitialSpawnInHostMode) + //{ + // Debug.Log($" No captured value found for dirtyBit {dirtyBit}"); + //} + + bool inSpawned = NetworkClient.spawned.ContainsKey(netIdentity.netId); + //Debug.Log($" Final check: changed={changed}, inSpawned={inSpawned}"); // Only fire if actually changed and visible per AOI - if (changed && NetworkClient.spawned.ContainsKey(netIdentity.netId)) + if (changed && inSpawned) { + //Debug.Log($" FIRING HOOK: {actualPrevious} -> {field}"); if (NetworkClient.active && !NetworkServer.active && !NetworkClient.isSpawnFinished) { T capturedPrevious = actualPrevious; @@ -857,6 +882,10 @@ public void GeneratedSyncVarDeserialize(ref T field, Action OnChanged, } } } + //else + //{ + // Debug.Log($" OnChanged is NULL for {gameObject.name}"); + //} } // move the [SyncVar] generated OnDeserialize C# to avoid much IL. @@ -1276,6 +1305,7 @@ protected void SetSyncVar(T value, ref T fieldValue, ulong dirtyBit) // note: SyncVar hooks are only called when inital=false public virtual void OnSerialize(NetworkWriter writer, bool initialState) { + //Debug.Log($"OnSerialize called for {gameObject.name}: initialState={initialState}, syncVarDirtyBits={syncVarDirtyBits}"); SerializeSyncObjects(writer, initialState); SerializeSyncVars(writer, initialState); } diff --git a/Assets/Mirror/Core/NetworkIdentity.cs b/Assets/Mirror/Core/NetworkIdentity.cs index c444c75d272..7f037676ea0 100644 --- a/Assets/Mirror/Core/NetworkIdentity.cs +++ b/Assets/Mirror/Core/NetworkIdentity.cs @@ -123,6 +123,10 @@ public sealed class NetworkIdentity : MonoBehaviour // only set temporarily during OnHostClientSpawn deserialization. internal bool hostInitialSpawn; + // flag to indicate this object's spawn messages should be deferred + // until the next frame, allowing user code to set SyncVars after NetworkServer.Spawn() + internal bool deferSpawnMessages; + /// The set of network connections (players) that can see this object. public readonly Dictionary observers = new Dictionary(); @@ -345,6 +349,7 @@ internal void InitializeNetworkBehaviours() NetworkBehaviour component = NetworkBehaviours[i]; component.netIdentity = this; component.ComponentIndex = (byte)i; + component.CaptureHostModeOriginalValues(); } } @@ -743,9 +748,6 @@ internal void OnStartServer() // one exception doesn't stop all the other Start() calls! try { - if (NetworkServer.activeHost) - comp.CaptureHostModeOriginalValues(); - comp.OnStartServer(); } catch (Exception e) diff --git a/Assets/Mirror/Core/NetworkServer.cs b/Assets/Mirror/Core/NetworkServer.cs index 0ba7cfcb070..76a87b19165 100644 --- a/Assets/Mirror/Core/NetworkServer.cs +++ b/Assets/Mirror/Core/NetworkServer.cs @@ -81,6 +81,10 @@ public static partial class NetworkServer public static readonly Dictionary spawned = new Dictionary(); + // deferred spawning to allow post-spawn SyncVar modifications + // objects in this list will have their spawn messages sent in the next NetworkEarlyUpdate + static readonly HashSet deferredSpawns = new HashSet(); + /// Single player mode can set listen=false to not accept incoming connections. public static bool listen; @@ -274,6 +278,7 @@ public static void Shutdown() connections.Clear(); connectionsCopy.Clear(); handlers.Clear(); + deferredSpawns.Clear(); // destroy all spawned objects, _then_ set inactive. // make sure .active is still true before calling this. @@ -1794,6 +1799,9 @@ static void SpawnObject(GameObject obj, NetworkConnectionToClient ownerConnectio identity.OnStartServer(); } + // defer spawn messages to allow post-spawn SyncVar modifications + identity.deferSpawnMessages = true; + // Debug.Log($"SpawnObject instance ID {identity.netId} asset ID {identity.assetId}"); if (aoi) @@ -2030,6 +2038,13 @@ internal static void AddAllReadyServerConnectionsToObservers(NetworkIdentity ide // both worlds without any worrying now! public static void RebuildObservers(NetworkIdentity identity, bool initialize) { + // if spawn messages are deferred, add to deferred list and skip rebuild + if (identity.deferSpawnMessages) + { + deferredSpawns.Add(identity); + return; + } + // if there is no interest management system, // or if 'force shown' then add all connections if (aoi == null || identity.visibility == Visibility.ForceShown) @@ -2293,6 +2308,21 @@ static void Broadcast(bool unreliableBaselineElapsed) } } + static void ProcessDeferredSpawns() + { + // process all deferred spawns + foreach (NetworkIdentity identity in deferredSpawns) + { + if (identity != null) + { + // clear the defer flag and rebuild observers to send spawn messages + identity.deferSpawnMessages = false; + RebuildObservers(identity, true); + } + } + deferredSpawns.Clear(); + } + // update ////////////////////////////////////////////////////////////// // NetworkEarlyUpdate called before any Update/FixedUpdate // (we add this to the UnityEngine in NetworkLoop) @@ -2305,6 +2335,9 @@ internal static void NetworkEarlyUpdate() fullUpdateDuration.Begin(); } + // process deferred spawns first to allow post-spawn SyncVar modifications + ProcessDeferredSpawns(); + // process all incoming messages first before updating the world if (Transport.active != null) Transport.active.ServerEarlyUpdate(); diff --git a/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs index 1dda4cdfeda..cc4da692889 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/SyncVarAttributeProcessor.cs @@ -266,7 +266,6 @@ public void GenerateCaptureHostModeOriginalValues(TypeDefinition td, List)).MakeGenericInstanceType(Import(), Import()); - dictionaryClearReference = Resolvers.ResolveMethod(dictionaryType, assembly, Log, "Clear", ref WeavingFailed); - dictionarySetItemReference = Resolvers.ResolveMethod(dictionaryType, assembly, Log, "set_Item", ref WeavingFailed); + clearHostModeOriginalValuesReference = Resolvers.ResolveMethod(NetworkBehaviourType, assembly, Log, "ClearHostModeOriginalValues", ref WeavingFailed); + storeHostModeOriginalValueReference = Resolvers.ResolveMethod(NetworkBehaviourType, assembly, Log, "StoreHostModeOriginalValue", ref WeavingFailed); NetworkBehaviourIsClientReference = Resolvers.ResolveMethod(NetworkBehaviourType, assembly, Log, "get_isClient", ref WeavingFailed); NetworkBehaviourIsServerReference = Resolvers.ResolveMethod(NetworkBehaviourType, assembly, Log, "get_isServer", ref WeavingFailed); diff --git a/Assets/Mirror/Tests/Editor/SyncVars/SyncVarHookDeferralTests.cs b/Assets/Mirror/Tests/Editor/SyncVars/SyncVarHookDeferralTests.cs index 1191e8d7696..5950385f8d4 100644 --- a/Assets/Mirror/Tests/Editor/SyncVars/SyncVarHookDeferralTests.cs +++ b/Assets/Mirror/Tests/Editor/SyncVars/SyncVarHookDeferralTests.cs @@ -261,6 +261,212 @@ public void UpdateAfterSpawn_HooksFireImmediately() Assert.That(clientComp1.callCount, Is.EqualTo(2)); Assert.That(clientComp1.target, Is.Null); } + + [Test] + public void PostSpawnModification_HooksFireWithFinalValues() + { + // This tests the core deferred spawn fix: post-spawn SyncVar modifications work + NetworkServer.Listen(10); + ConnectClientBlockingAuthenticatedAndReady(out _); + + // Simulate SpawnerTest scenario: modify SyncVar immediately after spawn + CreateNetworkedAndSpawn( + out GameObject serverGO, out _, out PostSpawnTestBehaviour serverComp, + out GameObject clientGO, out _, out PostSpawnTestBehaviour clientComp); + + // Simulate the exact SpawnerTest pattern: + serverComp.testValue = "Before Spawn"; // This would be lost without deferred spawn + // NetworkServer.Spawn() already called by CreateNetworkedAndSpawn + serverComp.testValue = "After Spawn"; // Final value that should sync + + ProcessMessages(); + + // Critical: Hook should fire with final post-spawn value + Assert.That(clientComp.callCount, Is.EqualTo(1), + "Hook should fire once with final value - DEFERRED SPAWN FIX"); + Assert.That(clientComp.receivedOldValue, Is.EqualTo(""), + "Hook oldValue should be empty string (original) - HOST MODE FIX"); + Assert.That(clientComp.receivedNewValue, Is.EqualTo("After Spawn"), + "Hook newValue should be final post-spawn value - DEFERRED SPAWN FIX"); + } + + [Test] + public void HostMode_PostSpawnModification_CorrectOldValues() + { + // This tests BOTH fixes together in host mode + NetworkServer.Listen(10); + ConnectHostClientBlockingAuthenticatedAndReady(); + + Assert.That(NetworkServer.activeHost, Is.True, "Should be in host mode"); + + // In host mode, there's only one object (not separate server/client) + CreateNetworkedAndSpawn(out _, out _, out PostSpawnTestBehaviour comp); + + // Simulate post-spawn modification in host mode + comp.testValue = "Before Spawn"; // This sets the field directly + comp.testValue = "After Spawn"; // This should trigger proper hook with correct old value + + // In host mode after our fixes: + // 1. Original value ("") was captured before OnStartServer + // 2. Hook fires during deserialization with correct old/new values + // 3. No duplicate hook calls from setter + Assert.That(comp.callCount, Is.EqualTo(1), + "Host mode: Hook should fire once during deserialization"); + Assert.That(comp.receivedOldValue, Is.EqualTo(""), + "Host mode: oldValue should be captured original ('') - HOST MODE FIX"); + Assert.That(comp.receivedNewValue, Is.EqualTo("After Spawn"), + "Host mode: newValue should be final value - DEFERRED SPAWN FIX"); + } + + [Test] + public void MultiplePostSpawnChanges_OnlyFinalValueSyncs() + { + // Test that intermediate values are lost (correct SyncVar behavior) + NetworkServer.Listen(10); + ConnectClientBlockingAuthenticatedAndReady(out _); + + CreateNetworkedAndSpawn( + out _, out _, out PostSpawnTestBehaviour serverComp, + out _, out _, out PostSpawnTestBehaviour clientComp); + + // Multiple rapid changes after spawn + serverComp.testValue = "Change 1"; + serverComp.testValue = "Change 2"; + serverComp.testValue = "Change 3"; + serverComp.testValue = "Final Value"; + + ProcessMessages(); + + // Only final value should sync (intermediate values lost - expected behavior) + Assert.That(clientComp.callCount, Is.EqualTo(1), + "Only one hook call for final value"); + Assert.That(clientComp.receivedNewValue, Is.EqualTo("Final Value"), + "Only final value should be synchronized"); + } + + [Test] + public void SpawnerTestExactScenario_WorksForAllClients() + { + // Exact reproduction of SpawnerTest.cs to prevent regression + NetworkServer.Listen(10); + ConnectClientBlockingAuthenticatedAndReady(out _); + + // Simulate spawning multiple objects like SpawnerTest does + for (int i = 0; i < 3; i++) + { + CreateNetworkedAndSpawn( + out _, out _, out PostSpawnTestBehaviour serverComp, + out _, out _, out PostSpawnTestBehaviour clientComp); + + // Exact SpawnerTest pattern + serverComp.testValue = "Before Spawn"; + // NetworkServer.Spawn() already called + serverComp.testValue = "After Spawn"; + + ProcessMessages(); + + // Each spawned object should work correctly + Assert.That(clientComp.callCount, Is.EqualTo(1), + $"Object {i}: Hook should fire"); + Assert.That(clientComp.receivedOldValue, Is.EqualTo(""), + $"Object {i}: Correct oldValue"); + Assert.That(clientComp.receivedNewValue, Is.EqualTo("After Spawn"), + $"Object {i}: Correct newValue"); + } + } + + [Test] + public void GameObjectPostSpawn_ModificationWorks() + { + // Test post-spawn modification with GameObject SyncVars + NetworkServer.Listen(10); + ConnectClientBlockingAuthenticatedAndReady(out _); + + CreateNetworkedAndSpawn( + out _, out _, out GameObjectPostSpawnBehaviour serverComp, + out _, out _, out GameObjectPostSpawnBehaviour clientComp); + + CreateNetworkedAndSpawn( + out GameObject serverTarget, out _, + out GameObject clientTarget, out _); + + // Post-spawn modification of GameObject SyncVar + serverComp.targetObject = serverTarget; + + ProcessMessages(); + + Assert.That(clientComp.callCount, Is.EqualTo(1)); + Assert.That(clientComp.receivedOldValue, Is.Null, "GameObject oldValue should be null"); + Assert.That(clientComp.receivedNewValue, Is.EqualTo(clientTarget), "GameObject newValue should be correct"); + } + + [Test] + public void StructSyncVar_PostSpawnModification() + { + // Test post-spawn modification with struct SyncVars + NetworkServer.Listen(10); + ConnectClientBlockingAuthenticatedAndReady(out _); + + CreateNetworkedAndSpawn( + out _, out _, out StructPostSpawnBehaviour serverComp, + out _, out _, out StructPostSpawnBehaviour clientComp); + + // Post-spawn modification of struct SyncVar + serverComp.structValue = new TestStruct { value = 42, name = "Test" }; + + ProcessMessages(); + + Assert.That(clientComp.callCount, Is.EqualTo(1)); + Assert.That(clientComp.receivedOldValue.value, Is.EqualTo(0), "Struct oldValue should be default"); + Assert.That(clientComp.receivedNewValue.value, Is.EqualTo(42), "Struct newValue should be correct"); + Assert.That(clientComp.receivedNewValue.name, Is.EqualTo("Test"), "Struct string field should be correct"); + } + + [Test] + public void HostAndRemoteClients_BothReceiveCorrectHooks() + { + // Test that both host and remote clients get correct hook behavior + NetworkServer.Listen(10); + ConnectHostClientBlockingAuthenticatedAndReady(); + ConnectClientBlockingAuthenticatedAndReady(out _); + + Assert.That(NetworkServer.activeHost, Is.True); + + // Create object visible to both host and remote client + CreateNetworkedAndSpawn(out _, out _, out PostSpawnTestBehaviour comp); + + // Post-spawn modification + comp.testValue = "Modified After Spawn"; + + ProcessMessages(); + + // Both host and remote should receive correct hook + Assert.That(comp.callCount, Is.EqualTo(1), "Host should receive hook"); + Assert.That(comp.receivedOldValue, Is.EqualTo(""), "Host should have correct oldValue"); + Assert.That(comp.receivedNewValue, Is.EqualTo("Modified After Spawn"), "Host should have correct newValue"); + + // Note: In this test setup, host and remote point to same objects + // In real scenarios, you'd have separate client objects to test + } + + [Test] + public void NoPostSpawnChanges_HooksDoNotFire() + { + // Verify hooks don't fire unnecessarily when no post-spawn changes occur + NetworkServer.Listen(10); + ConnectClientBlockingAuthenticatedAndReady(out _); + + CreateNetworkedAndSpawn( + out _, out _, out PostSpawnTestBehaviour serverComp, + out _, out _, out PostSpawnTestBehaviour clientComp); + + // Don't modify SyncVar after spawn + ProcessMessages(); + + // No hook should fire since value never changed from default + Assert.That(clientComp.callCount, Is.EqualTo(0), + "Hook should not fire when value never changes from default"); + } } public class CrossRefBehaviour : NetworkBehaviour @@ -330,4 +536,62 @@ void OnTargetChanged(NBCrossRefBehaviour oldValue, NBCrossRefBehaviour newValue) targetWasInSpawnedWhenHookFired = NetworkClient.spawned.ContainsKey(newValue.netId); } } + + public class PostSpawnTestBehaviour : NetworkBehaviour + { + [SyncVar(hook = nameof(OnValueChanged))] + public string testValue = ""; + + public int callCount; + public string receivedOldValue; + public string receivedNewValue; + + void OnValueChanged(string oldValue, string newValue) + { + callCount++; + receivedOldValue = oldValue; + receivedNewValue = newValue; + } + } + + public class GameObjectPostSpawnBehaviour : NetworkBehaviour + { + [SyncVar(hook = nameof(OnTargetChanged))] + public GameObject targetObject; + + public int callCount; + public GameObject receivedOldValue; + public GameObject receivedNewValue; + + void OnTargetChanged(GameObject oldValue, GameObject newValue) + { + callCount++; + receivedOldValue = oldValue; + receivedNewValue = newValue; + } + } + + [System.Serializable] + public struct TestStruct + { + public int value; + public string name; + } + + public class StructPostSpawnBehaviour : NetworkBehaviour + { + [SyncVar(hook = nameof(OnStructChanged))] + public TestStruct structValue; + + public int callCount; + public TestStruct receivedOldValue; + public TestStruct receivedNewValue; + + void OnStructChanged(TestStruct oldValue, TestStruct newValue) + { + callCount++; + receivedOldValue = oldValue; + receivedNewValue = newValue; + } + } }