fix: Unify SyncVar and SyncCollection replay across host visibility, scene re-observation, and pooled respawn

[Copilot]

SyncVar hooks and SyncCollection callbacks could diverge between host and remote clients when visibility, scene-object reuse, or pooled respawn got involved. This PR brings those paths back into alignment so observation-driven replay behaves consistently across host mode, remote clients, AOI hide/show, and runtime UnSpawn/respawn reuse.

• Preserve correct SyncVar baselines

  • Weaver-generated hooked SyncVars now capture the real pre-visibility field value, including declaration initializers and non-primitive types.
  • Host replay uses that captured baseline so the first visible hook sees the same oldValue a fresh observer would.

• Defer host callbacks until actual observation

  • Host-mode SyncVar hooks and SyncCollection actions now stay deferred until the host client has a local player and the object is actually observed.
  • Re-entering AOI or observing after a delayed player spawn replays state once, instead of firing early or double-firing during host spawn transitions.

• Replay re-observation as a fresh observation

  • Host and remote clients now treat scene-object hide/show as a new observation for hooked SyncVars.
  • SyncCollections replay the current visible state as Add actions again on re-observation, instead of remembering prior observation state.

• Handle pooled runtime-object reuse in host mode

  • Host-mode NetworkServer.UnSpawn/respawn of pooled runtime objects now marks SyncVar hooks and SyncCollection replays pending before visibility is lost.
  • Respawning the same object replays one-time-assigned SyncVars (such as RandomColor) and current SyncCollection contents again, even without AOI.

• Preserve server delta queues and avoid duplicate callbacks

  • Host visibility replay no longer deserializes the shared server/host SyncObject state or consumes pending remote deltas.
  • Replay-pending tracking is now scoped per hooked SyncVar / SyncObject, preventing unrelated members from replaying twice.
  • SyncObject callback clearing is limited to reused client scene-object paths so client re-observation does not accumulate handlers and server pooling keeps its subscriptions.

• Regression coverage

  • Added and corrected focused editor tests for host visibility deferral, AOI leave/re-enter, remote scene-object re-observation, ready-without-player flows, AddPlayer/spawn boundary cases, pooled runtime respawn, and double-fire prevention for both SyncVars and SyncCollections.

Example behavior after this change:

[SyncVar(hook = nameof(OnValueChanged))]
public int value = 42;

// Mutated before the host can observe it
value = 100;
value = 200;

// When the host first observes or re-observes the object:
// OnValueChanged(42, 200)

[MrGadget1024]

@copilot Error: Assets/Mirror/Core/SyncDictionary.cs(371,42): error CS0103: The name 'newItem' does not exist in the current context
Should be TValue capturedNew = item;

[Copilot]

@copilot Error: Assets/Mirror/Core/SyncDictionary.cs(371,42): error CS0103: The name 'newItem' does not exist in the current context
Should be TValue capturedNew = item;

Addressed in 485ce90.

[MrGadget1024]

@copilot Fix errors related to failed tests

[Copilot]

@copilot Fix errors related to failed tests

Addressed in 7ee058b.

[MrGadget1024]

@copilot Fix failed test
https://github.com/MirrorNetworking/Mirror/actions/runs/28763688725/job/85352608919

[Copilot]

@copilot Fix failed test
https://github.com/MirrorNetworking/Mirror/actions/runs/28763688725/job/85352608919

Addressed in 52f22a7.

[codecov]

Codecov Report

❌ Patch coverage is 74.94357% with 111 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.84%. Comparing base (31163ab) to head (310425d).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
Assets/Mirror/Core/NetworkBehaviour.cs	65.33%	104 Missing ⚠️
Assets/Mirror/Core/SyncObject.cs	75.00%	3 Missing ⚠️
Assets/Mirror/Core/NetworkClient.cs	95.00%	2 Missing ⚠️
Assets/Mirror/Core/SyncDictionary.cs	96.42%	1 Missing ⚠️
Assets/Mirror/Core/SyncSet.cs	96.15%	1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (74.94%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4118      +/-   ##
==========================================
+ Coverage   42.22%   42.84%   +0.61%     
==========================================
  Files         156      156              
  Lines       15100    15496     +396     
==========================================
+ Hits         6376     6639     +263     
- Misses       8724     8857     +133     

Flag	Coverage Δ
unittests	42.84% <74.94%> (+0.61%)	⬆️
unity-6000.5.1f1	42.84% <74.94%> (+0.61%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
Assets/Mirror/Core/NetworkIdentity.cs	91.07% <100.00%> (+0.05%)	⬆️
Assets/Mirror/Core/NetworkServer.cs	69.00% <100.00%> (+0.53%)	⬆️
Assets/Mirror/Core/SyncList.cs	91.91% <100.00%> (+0.77%)	⬆️
Assets/Mirror/Core/SyncDictionary.cs	89.83% <96.42%> (+2.09%)	⬆️
Assets/Mirror/Core/SyncSet.cs	92.37% <96.15%> (+1.82%)	⬆️
Assets/Mirror/Core/NetworkClient.cs	94.14% <95.00%> (+0.34%)	⬆️
Assets/Mirror/Core/SyncObject.cs	78.57% <75.00%> (-21.43%)	⬇️
Assets/Mirror/Core/NetworkBehaviour.cs	74.37% <65.33%> (-11.35%)	⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[MrGadget1024]

@copilot Extend the new tests to move host client player out of AOI range, Assert that the test object is no longer observed, change the SyncVar value or make a change to the Sync Collection, Assert no hooks / actions, move host client player into range again, Assert hooks / actions invoked again the same as previously, e.g. same original oldValue as if the host client is seeing it for the first time again.

[Copilot]

@copilot
Hook_DoesNotReplayHiddenChangesAfterLeavingAOI (0.003s)

...

Fixed in d83f910. The SyncCollection replay-pending scoping change stopped host AOI re-observation from marking SyncVar-only behaviours for replay, so that test could miss the second in-range hook. MarkAllSyncVarHostVisibilityReplayPending() now also marks the owning behaviour for replay.

[MrGadget1024]

@Lymdun I think we've covered everything now.

[Lymdun]

@MrGadget1024 re-ran evals:

Cross-member duplication really is fixed now. I verified be2f07c (per-hook pending), 956a0f6 (per-SyncObject pending), and d83f910 (AOI replay regression) in code, and the earlier pooling fix in ae5c2fd still holds. But re-running the evaluation at d83f910 leaves two transition cases, and one is exactly what you asked copilot to ensure against: a SyncCollection can still double-run its Add actions.

1. [P1] Two changes to the same member straddling AddPlayerForConnection are still mishandled

The new fixes and tests all cover one pending member plus a different immediately-delivered member (SyncVar test, collection test). The same-member ordering behaves differently for the two kinds:

• SyncList — actual double-fire. list.Add("first") while unobserved marks the list pending. Add the player without processing messages, then list.Add("second"): IsHostClientObserved() is now true, so AddOperation fires it immediately — but nothing clears the list's pending flag. Spawn-message processing then consumes the pending replay and replays the full list → Add: second runs twice. (The SyncVar fix clears its per-hook flag in the live branch at NetworkBehaviour.cs#L690; AddOperation has no equivalent.)
• SyncVar — single fire, but wrong values delivered early. 42 → 50 while unobserved (pending, baseline 42), add player, 50 → 100: the live branch clears the pending flag and fires (50, 100) immediately. The replay is then skipped, so the host client never gets the fresh-observation (42, 100) — it sees a hidden intermediate value as oldValue, before it has even processed the spawn message.

Suggested shape for both: if the member is already replay-pending, stay deferred instead of live-firing during the observer-entry → spawn-processing window. The replay then delivers exactly once with correct values ((42, 100); first + second once each).

2. [P1] Pending replays are still consumed without a local player

FlushHostVisibilityDeferredCallbacks correctly gates on localPlayer != null, but OnHostClientSpawn invokes InvokeHostVisibilityDeferredCallbacks unconditionally, and that method consumes all pending state with no eligibility check. With no AOI, a ready host connection observes runtime spawns before AddPlayer — so ready the host, delay AddPlayer, spawn + populate an object, process its spawn message: the pending hooks/actions fire while NetworkClient.localPlayer == null, and the AddPlayer flush later finds nothing. That contradicts the "hooks are deferred until the host client has a player" rule. (Your delayed-AddPlayer test passes because its changes happen after spawn processing, so they land on the correctly-gated flush path — the hole only affects changes made before the spawn message is processed.)

Suggested regressions: two same-member changes (one SyncVar, one list Add) straddling AddPlayerForConnection before ProcessMessages, and a no-AOI ready-without-player runtime spawn asserting nothing fires until AddPlayer. Codecov's 74.8% patch-hit failure on d83f910 points at the same untested windows.

Everything else from my earlier reviews is resolved on my side.

Every claim and line anchor in it is verified against the actual code at d83f910 — the lhes described (the setter's pending-clear at line 690, AddOperation's ungated immediatefire, the full-list replay, and the gated vs. ungated flush paths).

[MrGadget1024]

@copilot Review this comment

[Copilot]

@copilot Review this comment

Addressed in d41bf4c and 0a75f09. The linked review was actionable: host replay now stays deferred until the host can actually observe, and same-member SyncVar/SyncCollection changes no longer live-fire during the AddPlayer/spawn transition. I also added focused regressions for both the straddle-AddPlayer case and the ready-without-player runtime-spawn case.

[MrGadget1024]

@Lymdun We good now?

[MrGadget1024]

@copilot In the Room example, there is a Spawner static class Assets/Mirror/Examples/Room/Scripts/Spawner.cs that uses a Pool Assets/Mirror/Core/Tools/Pool.cs, and a Reward script Assets/Mirror/Examples/Room/Scripts/Reward.cs. The Reward prefab has RandomColor, and the color is set once when the Reward is initially created by the Spawner and never changes. OnTriggerEnter in Reward script recycles the reward item by Unspawning it into the pool to be respawned after a delay at a new position. The host client should fire the SyncVar hook of RandomColor when it's respawned, but it's not firing. There is no AOI in this example. If the reward had a SyncCollection, the Add actions should be invoked again too.

[Copilot]

@copilot In the Room example, there is a Spawner static class Assets/Mirror/Examples/Room/Scripts/Spawner.cs that uses a Pool Assets/Mirror/Core/Tools/Pool.cs, and a Reward script `Assets/Mirror/E...

Addressed in 1fab2fc with the test follow-up in 310425d. Host-mode UnSpawn now marks SyncVar and SyncCollection replay pending before the shared object leaves observation, so respawning the same runtime object replays the RandomColor hook again, and SyncCollection Add actions replay again too. I also added focused host/no-AOI regression tests for both runtime-object SyncVar hook replay and SyncCollection replay after unspawn/respawn.

[Lymdun]

@Lymdun We good now?

@MrGadget1024 re-ran all evals, all good now ✅

[MrGadget1024]

@copilot Update the Title and Description of this PR reflecting the goals we achieved and describing the work completed.

[Copilot]

@copilot Update the Title and Description of this PR reflecting the goals we achieved and describing the work completed.

Use this updated PR metadata:

Title
Unify SyncVar and SyncCollection replay across host visibility, scene re-observation, and pooled respawn

Description
SyncVar hooks and SyncCollection callbacks could diverge between host and remote clients when visibility, scene-object reuse, or pooled respawn got involved. This PR brings those paths back into alignment so observation-driven replay behaves consistently across host mode, remote clients, AOI hide/show, and runtime UnSpawn/respawn reuse.

• Preserve correct SyncVar baselines

  • Weaver-generated hooked SyncVars now capture the real pre-visibility field value, including declaration initializers and non-primitive types.
  • Host replay uses that captured baseline so the first visible hook sees the same oldValue a fresh observer would.

• Defer host callbacks until actual observation

  • Host-mode SyncVar hooks and SyncCollection actions now stay deferred until the host client has a local player and the object is actually observed.
  • Re-entering AOI or observing after a delayed player spawn replays state once, instead of firing early or double-firing during host spawn transitions.

• Replay re-observation as a fresh observation

  • Host and remote clients now treat scene-object hide/show as a new observation for hooked SyncVars.
  • SyncCollections replay the current visible state as Add actions again on re-observation, instead of remembering prior observation state.

• Handle pooled runtime-object reuse in host mode

  • Host-mode NetworkServer.UnSpawn/respawn of pooled runtime objects now marks SyncVar hooks and SyncCollection replays pending before visibility is lost.
  • Respawning the same object replays one-time-assigned SyncVars (such as RandomColor) and current SyncCollection contents again, even without AOI.

• Preserve server delta queues and avoid duplicate callbacks

  • Host visibility replay no longer deserializes the shared server/host SyncObject state or consumes pending remote deltas.
  • Replay-pending tracking is now scoped per hooked SyncVar / SyncObject, preventing unrelated members from replaying twice.
  • SyncObject callback clearing is limited to reused client scene-object paths so client re-observation does not accumulate handlers and server pooling keeps its subscriptions.

• Regression coverage

  • Added and corrected focused editor tests for host visibility deferral, AOI leave/re-enter, remote scene-object re-observation, ready-without-player flows, AddPlayer/spawn boundary cases, pooled runtime respawn, and double-fire prevention for both SyncVars and SyncCollections.

Example behavior after this change:

[SyncVar(hook = nameof(OnValueChanged))]
public int value = 42;

// Mutated before the host can observe it
value = 100;
value = 200;

// When the host first observes or re-observes the object:
// OnValueChanged(42, 200)

That wording matches the behavior covered by the final test set and the host/remote example scenarios fixed in this PR.