fix: BitWriter bytes lost when TryBeginWriteBits grows the buffer (up-port) (#4226)

* fix: keep BitWriter bytes when TryBeginWriteBits grows the buffer

FastBufferWriter.Grow copied Length bytes, which excludes bytes written in an
open bitwise context, so a later TryBeginWriteBits that grew the buffer dropped
them. Copy the whole old buffer instead. (UUM-158588)

* chore: CHANGELOG PR number

* style: remove redundant cast in BitWriterTests

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -26,6 +26,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - Issue where `NetworkTime` reported a `TickOffset` of almost a whole tick, instead of zero, for some times that land on a tick (for example 0.3 at a tick rate of 10), which put `TickWithPartial` one tick ahead or, in long sessions, `Tick` one tick behind. Times between minus one tick and zero now report tick -1 with a positive `TickOffset`, as other negative times do. (#4228)
+- Issue where bits written with `BitWriter` were lost when a later `BitWriter.TryBeginWriteBits` call in the same bitwise context grew the `FastBufferWriter`. The bytes written earlier in that context were replaced with zeros or uninitialized memory. (#4226)
 - Issue where a growable `FastBufferWriter` created with a size of 0 never returned from its first write or `TryBeginWrite` call that needed the buffer to grow. (#4224)
 - Issue where a `FastBufferReader` created with an offset and the default length of -1 read past the end of its source. The `ArraySegment<byte>` overload that takes an offset now applies it from the start of the segment. (#4222)
 - Issue where a client synchronized while a `NetworkList<T>` still had unsent changes applied those changes twice, resulting in duplicate entries and index-based events landing on the wrong elements on that client. (#4216)

com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferWriter.cs

@@ -204,7 +204,8 @@ internal unsafe void Grow(int additionalSizeRequired)
 #if DEBUG
             UnsafeUtility.MemSet(newBuffer, 0, newSize);
 #endif
-            UnsafeUtility.MemCpy(newBuffer, Handle->BufferPointer, Length);
+            // Not Length: bytes written in an open bitwise context are not committed to Position until it is disposed.
+            UnsafeUtility.MemCpy(newBuffer, Handle->BufferPointer, Handle->Capacity);
             if (Handle->BufferGrew)
             {
                 UnsafeUtility.Free(Handle->BufferPointer, Handle->Allocator);

com.unity.netcode.gameobjects/Tests/Editor/Serialization/BitWriterTests.cs

@@ -329,5 +329,30 @@ public unsafe void TestWritingBitsThrowsIfTryBeginWriteNotCalled()
 
             }
         }
+
+        [Test]
+        public void TestTryBeginWriteBitsGrowingBufferKeepsBitwiseWrites([Values] bool writeLeadingByte)
+        {
+            var writer = new FastBufferWriter(writeLeadingByte ? 2 : 1, Allocator.Temp, 16);
+            using (writer)
+            {
+                if (writeLeadingByte)
+                {
+                    writer.WriteByteSafe(0x11);
+                }
+
+                using (var bitWriter = writer.EnterBitwiseContext())
+                {
+                    Assert.IsTrue(bitWriter.TryBeginWriteBits(8));
+                    bitWriter.WriteBits(0xAB, 8);
+                    // Grows the buffer while 0xAB is still uncommitted to the writer.
+                    Assert.IsTrue(bitWriter.TryBeginWriteBits(16));
+                    bitWriter.WriteBits(0xCDEFUL, 16);
+                }
+
+                var expected = writeLeadingByte ? new byte[] { 0x11, 0xAB, 0xEF, 0xCD } : new byte[] { 0xAB, 0xEF, 0xCD };
+                CollectionAssert.AreEqual(expected, writer.ToArray());
+            }
+        }
     }
 }