From e6886e8a841054b81ea5daec9eeb5fb7f0c6df51 Mon Sep 17 00:00:00 2001 From: vis2k Date: Wed, 12 Oct 2022 21:00:32 +0200 Subject: [PATCH] fix: ensure Serialize writes nothing at all if not dirty and add test so it's never missed again(!). fixes unchanged objects sending an unnecessary 1 byte dirty mask after recent dirtybits improvement. --- Assets/Mirror/Core/NetworkIdentity.cs | 7 ++++-- .../NetworkIdentitySerializationTests.cs | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Assets/Mirror/Core/NetworkIdentity.cs b/Assets/Mirror/Core/NetworkIdentity.cs index 284b1156e..4d02ba10d 100644 --- a/Assets/Mirror/Core/NetworkIdentity.cs +++ b/Assets/Mirror/Core/NetworkIdentity.cs @@ -947,8 +947,11 @@ internal void Serialize(bool initialState, NetworkWriter ownerWriter, NetworkWri // 11 components fit into 2 bytes. (previously 11 bytes) // 16 components fit into 3 bytes. (previously 16 bytes) // TODO imer: client knows amount of comps, write N bytes instead - Compression.CompressVarUInt(ownerWriter, ownerMask); - Compression.CompressVarUInt(observersWriter, observerMask); + + // if nothing dirty, then don't even write the mask. + // otherwise, every unchanged object would send a 1 byte dirty mask! + if (ownerMask != 0) Compression.CompressVarUInt(ownerWriter, ownerMask); + if (observerMask != 0) Compression.CompressVarUInt(observersWriter, observerMask); // serialize all components for (int i = 0; i < components.Length; ++i) diff --git a/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs b/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs index 50fd04826..bb15f0d87 100644 --- a/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs +++ b/Assets/Mirror/Tests/Editor/NetworkIdentitySerializationTests.cs @@ -199,5 +199,28 @@ public void SerializationMismatch() // should still work fine. that's the whole point. Assert.That(clientComp.value, Is.EqualTo("42")); } + + // ensure Serialize writes nothing if not dirty. + // previously after the dirty mask improvement, it would write a 1 byte + // 0-dirty-mask. instead, we need to ensure it writes nothing. + // too easy to miss, with too significant bandwidth implications. + [Test] + public void Serialize_NotInitial_NotDirty_WritesNothing() + { + // create spawned so that isServer/isClient is set properly + CreateNetworkedAndSpawn( + out _, out NetworkIdentity serverIdentity, out SerializeTest1NetworkBehaviour serverComp1, out SerializeTest2NetworkBehaviour serverComp2, + out _, out NetworkIdentity clientIdentity, out SerializeTest1NetworkBehaviour clientComp1, out SerializeTest2NetworkBehaviour clientComp2); + + // change nothing + // serverComp.value = "42"; + + // serialize server object. + // 'initial' would write everything. + // instead, try 'not initial' with 0 dirty bits + serverIdentity.Serialize(false, ownerWriter, observersWriter); + Assert.That(ownerWriter.Position, Is.EqualTo(0)); + Assert.That(observersWriter.Position, Is.EqualTo(0)); + } } }