fix(NetworkIdentity): Check unreliableBaseline when sending baseline (#4095)

Author: MrGadget1024

Assets/Mirror/Core/NetworkIdentity.cs

@@ -1188,8 +1188,13 @@ internal void SerializeServer_Broadcast(
             if (ownerMaskUnreliableDelta != 0)    Compression.CompressVarUInt(ownerWriterUnreliableDelta, ownerMaskUnreliableDelta);
             if (observerMaskUnreliableDelta != 0) Compression.CompressVarUInt(observersWriterUnreliableDelta, observerMaskUnreliableDelta);
 
-            if (ownerMaskUnreliableBaseline != 0)    Compression.CompressVarUInt(ownerWriterUnreliableBaseline, ownerMaskUnreliableBaseline);
-            if (observerMaskUnreliableBaseline != 0) Compression.CompressVarUInt(observersWriterUnreliableBaseline, observerMaskUnreliableBaseline);
+            // only write baseline mask when actually sending a baseline.
+            // otherwise an orphaned 1-2 byte mask is written with no data following it.
+            if (unreliableBaseline)
+            {
+                if (ownerMaskUnreliableBaseline != 0) Compression.CompressVarUInt(ownerWriterUnreliableBaseline, ownerMaskUnreliableBaseline);
+                if (observerMaskUnreliableBaseline != 0) Compression.CompressVarUInt(observersWriterUnreliableBaseline, observerMaskUnreliableBaseline);
+            }
 
             // serialize all components
             // perf: only iterate if either dirty mask has dirty bits.
@@ -1355,7 +1360,6 @@ internal void SerializeClient(NetworkWriter writerReliable, NetworkWriter writer
                         comp.Serialize(writerUnreliableBaseline, true);
 
                         // for unreliable components, only clear dirty bits after the reliable baseline.
-                        // unreliable deltas aren't guaranteed to be delivered, no point in clearing bits.
                         // -> don't clear sync time: that's for delta syncs.
                         comp.ClearAllDirtyBits(false);
                     }
@@ -1640,7 +1644,7 @@ public void RemoveClientAuthority()
         // Marks the identity for future reset, this is because we cant reset
         // the identity during destroy as people might want to be able to read
         // the members inside OnDestroy(), and we have no way of invoking reset
-        // after OnDestroy is called.
+        // after OnDestroy() is called.
         internal void ResetState()
         {
             hasSpawned = false;

Assets/Mirror/Tests/Editor/NetworkIdentity/NetworkIdentitySerializationTests.cs

@@ -479,6 +479,7 @@ public void SerializeServer_Broadcast_NotDirty_WritesNothing()
             Assert.That(ownerWriterReliable.Position, Is.EqualTo(0));
             Assert.That(observersWriterReliable.Position, Is.EqualTo(0));
 
+            // unreliableBaseline=false: no baseline mask or data should be written
             Assert.That(ownerWriterUnreliableBaseline.Position, Is.EqualTo(0));
             Assert.That(observersWriterUnreliableBaseline.Position, Is.EqualTo(0));
 
@@ -639,19 +640,16 @@ public void SerializeServer_ObserversMode_ServerToClient_UnreliableOnly()
             Assert.That(ownerWriterReliable.Position, Is.EqualTo(0));
             Assert.That(observersWriterReliable.Position, Is.EqualTo(0));
 
-            Assert.That(ownerWriterUnreliableBaseline.Position, Is.GreaterThan(0));
-            Assert.That(observersWriterUnreliableBaseline.Position, Is.GreaterThan(0));
+            // unreliableBaseline=false: no baseline mask or data should be written
+            Assert.That(ownerWriterUnreliableBaseline.Position, Is.EqualTo(0));
+            Assert.That(observersWriterUnreliableBaseline.Position, Is.EqualTo(0));
 
             Assert.That(ownerWriterUnreliableDelta.Position, Is.GreaterThan(0));
             Assert.That(observersWriterUnreliableDelta.Position, Is.GreaterThan(0));
         }
 
         [Test]
-        [Ignore("BUG: NetworkIdentity.SerializeServer_Broadcast() writes baseline dirty mask even when unreliableBaseline=false. " +
-        "This wastes 1-2 bytes per object per frame. " +
-        "Fix: Wrap baseline mask writes in 'if (unreliableBaseline) { ... }' block. " +
-        "See lines ~1150-1153 in NetworkIdentity.cs")]
-        public void UnreliableBaseline_Timing_REVEALS_BUG()
+        public void UnreliableBaseline_Timing()
         {
             CreateNetworkedAndSpawn(
                 out _, out NetworkIdentity serverIdentity, out SerializeTest1NetworkBehaviour serverComp,
@@ -691,11 +689,9 @@ public void UnreliableBaseline_Timing_REVEALS_BUG()
             // Delta should be written
             Assert.That(deltaSize, Is.GreaterThan(0), "Delta should be written");
 
-            // BUG REVEALED: This should be 0 but is actually 1
-            // The baseline mask byte is written even though no data follows
+            // unreliableBaseline=false: only delta is sent, baseline writers stay empty
             Assert.That(deltaOnlyBaselineSize, Is.EqualTo(0),
-                "EXPECTED: No baseline data when unreliableBaseline=false. " +
-                "ACTUAL: Writes 1-byte dirty mask, wasting bandwidth.");
+                "No baseline data when unreliableBaseline=false");
 
             // === SCENARIO 3: Serialize with unreliableBaseline=true ===
             serverComp.value = 200;
@@ -950,10 +946,10 @@ public void SerializeServer_SyncModeAndDirection_Matrix(
                     $"[Broadcast/Hybrid] Reliable must be empty. SyncMode={syncMode}, SyncDirection={syncDirection}");
 
                 // unreliableBaseline=false: no baseline mask or data should be written
-                Assert.That(ownerWriterUnreliableBaseline.Position, NonZeroIf(expectOwner),
-                    $"[Broadcast/Hybrid] Unreliable baseline mask written (BUG). SyncMode={syncMode}, SyncDirection={syncDirection}");
-                Assert.That(observersWriterUnreliableBaseline.Position, NonZeroIf(expectObservers),
-                    $"[Broadcast/Hybrid] Unreliable baseline mask written (BUG). SyncMode={syncMode}, SyncDirection={syncDirection}");
+                Assert.That(ownerWriterUnreliableBaseline.Position, Is.EqualTo(0),
+                    $"[Broadcast/Hybrid] Unreliable baseline must be empty. SyncMode={syncMode}, SyncDirection={syncDirection}");
+                Assert.That(observersWriterUnreliableBaseline.Position, Is.EqualTo(0),
+                    $"[Broadcast/Hybrid] Unreliable baseline must be empty. SyncMode={syncMode}, SyncDirection={syncDirection}");
             }
         }