Skip to content

fix: SyncVar hooks only fire on host client respecting AOI#4107

Open
MrGadget1024 wants to merge 8 commits intomasterfrom
FixHostSyncVarHooks
Open

fix: SyncVar hooks only fire on host client respecting AOI#4107
MrGadget1024 wants to merge 8 commits intomasterfrom
FixHostSyncVarHooks

Conversation

@MrGadget1024
Copy link
Copy Markdown
Collaborator

@MrGadget1024 MrGadget1024 commented Apr 2, 2026

WIP - Do Not Merge

Fixes host mode SyncVar hooks to provide correct old values and respect AOI properly.

Problem

SyncVar hooks in host mode had two critical issues:

  1. Wrong old values: Hooks fired with oldValue == newValue instead of the true original value
  2. AOI violations: Hooks fired for objects outside the host client's Area of Interest (AOI) range
    Root ### Cause
    In host mode, the sequence was:
  3. Server spawns object and calls OnStartServer()
  4. OnStartServer() modifies SyncVars (e.g., color = Random.ColorHSV())
  5. Later, host client deserialization occurs
  6. By this time, the "previous" value was already the server-set value
  7. Hooks fired with incorrect parameters and without AOI consideration

Solution

Core Changes

• Capture original values: Added mechanism to capture SyncVar values before OnStartServer() runs
• Use captured values in hooks: Modified deserialize methods to use captured originals as oldValue
• Respect AOI: Only fire hooks for objects visible to the host client (spawned check)
• Proper timing: Hooks fire during client deserialization, not server setters

Implementation Details

NetworkBehaviour.cs
• Added hostModeOriginalValues dictionary to store original values by dirtyBit
• Updated all 4 GeneratedSyncVarDeserialize* methods to accept ulong dirtyBit parameter
• Added logic to use captured values as actualPrevious in host mode
• Added AOI check: NetworkClient.spawned.ContainsKey(netIdentity.netId)

NetworkIdentity.cs
• Added CaptureHostModeOriginalValues() call before each comp.OnStartServer() in host mode
• Perfect timing ensures values are captured before server modifications

NetworkClient.cs
• Clear hostModeOriginalValues after spawn completed
• Remove from spawned in OnHostClientObjectHide

Weaver Changes
• SyncVarAttributeProcessor: Generated CaptureHostModeOriginalValues() method that captures each SyncVar with its dirtyBit as key
• NetworkBehaviourProcessor: Updated DeserializeField() to pass dirtyBit to deserialize methods
• WeaverTypes: Added references for hostModeOriginalValues dictionary operations

Test Changes
• Replaced test that expected double-firing of SyncVar hooks for host

Test Results

[17:01:09] SetColor RGBA(0, 0, 0, 255) -> RGBA(145, 130, 0, 255)  // Player
[17:01:09] SetColor RGBA(0, 0, 0, 255) -> RGBA(0, 247, 198, 255)  // Near cube
[17:01:26] SetColor RGBA(0, 0, 0, 255) -> RGBA(148, 235, 0, 255)  // Far cube (when in AOI range)

✅ Correct old values: All hooks show oldValue = Color.black (true original)
✅ AOI respected: Far cube hook only fires when player moves into range (17 seconds later)
✅ Proper timing: Hooks fire when objects become visible to host client

Files Modified

• NetworkBehaviour.cs
• NetworkIdentity.cs
• NetworkClient.cs
• SyncVarAttributeProcessor.cs
• NetworkBehaviourProcessor.cs
• WeaverTypes.cs
• SyncVarHookDeferralTests.cs

- Weaver generation also preserves oldValue for the hooks.
@MrGadget1024 MrGadget1024 requested a review from miwarnec April 2, 2026 21:46
@MrGadget1024 MrGadget1024 added bug Something isn't working Awaiting Review labels Apr 2, 2026
Copy link
Copy Markdown
Collaborator Author

@MrGadget1024 MrGadget1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert asmdef change

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 59.42029% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.00%. Comparing base (28ee96d) to head (01f55e1).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
Assets/Mirror/Core/NetworkBehaviour.cs 55.55% 28 Missing ⚠️

❌ Your patch check has failed because the patch coverage (59.42%) 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    #4107      +/-   ##
==========================================
+ Coverage   42.92%   43.00%   +0.08%     
==========================================
  Files         156      156              
  Lines       14859    14907      +48     
==========================================
+ Hits         6378     6411      +33     
- Misses       8481     8496      +15     
Flag Coverage Δ
unittests 43.00% <59.42%> (+0.08%) ⬆️
unity-6000.4.0f1 ?
unity-6000.4.1f1 43.00% <59.42%> (?)

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

Files with missing lines Coverage Δ
Assets/Mirror/Core/NetworkClient.cs 94.49% <100.00%> (+0.01%) ⬆️
Assets/Mirror/Core/NetworkIdentity.cs 91.04% <100.00%> (+0.02%) ⬆️
Assets/Mirror/Core/NetworkBehaviour.cs 84.09% <55.55%> (-1.62%) ⬇️

... and 1 file 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working work in progress Need more time to decide. Nothing to do here for now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant