Skip to content

Commit 9c9826a

Browse files
fix: BitWriter bytes lost when TryBeginWriteBits grows the buffer (#4225)
* 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 --------- Co-authored-by: Emma <emma.mcmillan@unity3d.com>
1 parent fe7cab5 commit 9c9826a

3 files changed

Lines changed: 28 additions & 1 deletion

File tree

‎com.unity.netcode.gameobjects/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
2222

2323
### Fixed
2424

25+
- 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. (#4225)
2526
- 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. (#4227)
2627
- 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. (#4223)
2728
- 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. (#4221)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,8 @@ internal unsafe void Grow(int additionalSizeRequired)
204204
#if DEBUG
205205
UnsafeUtility.MemSet(newBuffer, 0, newSize);
206206
#endif
207-
UnsafeUtility.MemCpy(newBuffer, Handle->BufferPointer, Length);
207+
// Not Length: bytes written in an open bitwise context are not committed to Position until it is disposed.
208+
UnsafeUtility.MemCpy(newBuffer, Handle->BufferPointer, Handle->Capacity);
208209
if (Handle->BufferGrew)
209210
{
210211
UnsafeUtility.Free(Handle->BufferPointer, Handle->Allocator);

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,5 +329,30 @@ public unsafe void TestWritingBitsThrowsIfTryBeginWriteNotCalled()
329329

330330
}
331331
}
332+
333+
[Test]
334+
public void TestTryBeginWriteBitsGrowingBufferKeepsBitwiseWrites([Values] bool writeLeadingByte)
335+
{
336+
var writer = new FastBufferWriter(writeLeadingByte ? 2 : 1, Allocator.Temp, 16);
337+
using (writer)
338+
{
339+
if (writeLeadingByte)
340+
{
341+
writer.WriteByteSafe(0x11);
342+
}
343+
344+
using (var bitWriter = writer.EnterBitwiseContext())
345+
{
346+
Assert.IsTrue(bitWriter.TryBeginWriteBits(8));
347+
bitWriter.WriteBits(0xAB, 8);
348+
// Grows the buffer while 0xAB is still uncommitted to the writer.
349+
Assert.IsTrue(bitWriter.TryBeginWriteBits(16));
350+
bitWriter.WriteBits(0xCDEFUL, 16);
351+
}
352+
353+
var expected = writeLeadingByte ? new byte[] { 0x11, 0xAB, 0xEF, 0xCD } : new byte[] { 0xAB, 0xEF, 0xCD };
354+
CollectionAssert.AreEqual(expected, writer.ToArray());
355+
}
356+
}
332357
}
333358
}

0 commit comments

Comments
 (0)