fix: NCCBUG-175: BitReader/BitWriter Reading/writing more than 8 bits was producing the wrong result

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

@@ -20,6 +20,8 @@ public ref struct BitReader
 #endif
 
         private const int k_BitsPerByte = 8;
+
+        private int BytePosition => m_BitPosition >> 3;
 
         /// <summary>
         /// Whether or not the current BitPosition is evenly divisible by 8. I.e. whether or not the BitPosition is at a byte boundary.
@@ -98,11 +100,6 @@ public unsafe void ReadBits(out ulong value, uint bitCount)
                 throw new ArgumentOutOfRangeException(nameof(bitCount), "Cannot read more than 64 bits from a 64-bit value!");
             }
 
-            if (bitCount < 0)
-            {
-                throw new ArgumentOutOfRangeException(nameof(bitCount), "Cannot read fewer than 0 bits!");
-            }
-
             int checkPos = (int)(m_BitPosition + bitCount);
             if (checkPos > m_AllowedBitwiseReadMark)
             {
@@ -165,7 +162,7 @@ public unsafe void ReadBit(out bool bit)
 #endif
 
             int offset = m_BitPosition & 7;
-            int pos = m_BitPosition >> 3;
+            int pos = BytePosition;
             bit = (m_BufferPointer[pos] & (1 << offset)) != 0;
             ++m_BitPosition;
         }
@@ -175,7 +172,7 @@ private unsafe void ReadPartialValue<T>(out T value, int bytesToRead, int offset
         {
             var val = new T();
             byte* ptr = ((byte*)&val) + offsetBytes;
-            byte* bufferPointer = m_BufferPointer + m_Position;
+            byte* bufferPointer = m_BufferPointer + BytePosition;
             UnsafeUtility.MemCpy(ptr, bufferPointer, bytesToRead);
 
             m_BitPosition += bytesToRead * k_BitsPerByte;

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

@@ -1,6 +1,7 @@
 using System;
 using System.Runtime.CompilerServices;
 using Unity.Collections.LowLevel.Unsafe;
+using UnityEngine;
 
 namespace Unity.Netcode
 {
@@ -29,6 +30,8 @@ public bool BitAligned
             get => (m_BitPosition & 7) == 0;
         }
 
+        private int BytePosition => m_BitPosition >> 3;
+
         internal unsafe BitWriter(FastBufferWriter writer)
         {
             m_Writer = writer;
@@ -137,7 +140,7 @@ public unsafe void WriteBits(ulong value, uint bitCount)
                     WriteMisaligned(asBytes[i]);
                 }
             }
-
+            
             for (var count = wholeBytes * k_BitsPerByte; count < bitCount; ++count)
             {
                 WriteBit((value & (1UL << count)) != 0);
@@ -181,7 +184,7 @@ public unsafe void WriteBit(bool bit)
 #endif
 
             int offset = m_BitPosition & 7;
-            int pos = m_BitPosition >> 3;
+            int pos = BytePosition;
             ++m_BitPosition;
             m_BufferPointer[pos] = (byte)(bit ? (m_BufferPointer[pos] & ~(1 << offset)) | (1 << offset) : (m_BufferPointer[pos] & ~(1 << offset)));
         }
@@ -190,7 +193,7 @@ public unsafe void WriteBit(bool bit)
         private unsafe void WritePartialValue<T>(T value, int bytesToWrite, int offsetBytes = 0) where T : unmanaged
         {
             byte* ptr = ((byte*)&value) + offsetBytes;
-            byte* bufferPointer = m_BufferPointer + m_Position;
+            byte* bufferPointer = m_BufferPointer + BytePosition;
             UnsafeUtility.MemCpy(bufferPointer, ptr, bytesToWrite);
 
             m_BitPosition += bytesToWrite * k_BitsPerByte;

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

@@ -0,0 +1,97 @@
+using NUnit.Framework;
+using Unity.Collections;
+
+namespace Unity.Netcode.EditorTests
+{
+    public class UserBitReaderAndBitWriterTests_NCCBUG175
+    {
+
+        [Test]
+	    public void WhenBitwiseWritingMoreThan8Bits_ValuesAreCorrect()
+	    {
+		    using FastBufferWriter writer = new FastBufferWriter(1024, Allocator.Temp);
+		    ulong inVal = 123456789;
+
+		    for (int i = 0; i < 100; ++i)
+		    {
+			    writer.WriteValueSafe(i);
+		    }
+
+		    using (var bitWriter = writer.EnterBitwiseContext())
+		    {
+			    for (int i = 0; i < 16; ++i)
+			    {
+				    Assert.IsTrue((bitWriter.TryBeginWriteBits(32)));
+				    bitWriter.WriteBits(inVal, 31);
+				    bitWriter.WriteBit(true);
+			    }
+		    }
+
+		    using FastBufferReader reader = new FastBufferReader(writer, Allocator.Temp);
+
+		    for (int i = 0; i < 100; ++i)
+		    {
+			    reader.ReadValueSafe(out int outVal);
+			    Assert.AreEqual(i, outVal);
+		    }
+
+		    using (var bitReader = reader.EnterBitwiseContext())
+		    {
+			    for (int i = 0; i < 16; ++i)
+			    {
+				    Assert.IsTrue(bitReader.TryBeginReadBits(32));
+				    bitReader.ReadBits(out ulong outVal, 31);
+				    bitReader.ReadBit(out bool bit);
+				    Assert.AreEqual(inVal, outVal);
+				    Assert.AreEqual(true, bit);
+			    }
+		    }
+	    }
+
+	    [Test]
+	    public void WhenBitwiseReadingMoreThan8Bits_ValuesAreCorrect()
+	    {
+		    using FastBufferWriter writer = new FastBufferWriter(1024, Allocator.Temp);
+		    ulong inVal = 123456789;
+
+		    for (int i = 0; i < 100; ++i)
+		    {
+			    writer.WriteValueSafe(i);
+		    }
+
+		    uint combined = (uint)inVal | (1u << 31);
+		    writer.WriteValueSafe( combined );
+		    writer.WriteValueSafe( combined );
+		    writer.WriteValueSafe( combined );
+
+		    using FastBufferReader reader = new FastBufferReader(writer, Allocator.Temp);
+
+		    for (int i = 0; i < 100; ++i)
+		    {
+			    reader.ReadValueSafe(out int outVal);
+			    Assert.AreEqual(i, outVal);
+		    }
+
+		    using (var bitReader = reader.EnterBitwiseContext())
+		    {
+			    Assert.IsTrue(bitReader.TryBeginReadBits(32));
+			    bitReader.ReadBits(out ulong outVal, 31 );
+			    bitReader.ReadBit(out bool bit);
+			    Assert.AreEqual( inVal, outVal );
+			    Assert.AreEqual( true, bit );
+
+			    Assert.IsTrue(bitReader.TryBeginReadBits(32));
+			    bitReader.ReadBits(out outVal, 31 );
+			    bitReader.ReadBit(out bit);
+			    Assert.AreEqual( inVal, outVal );
+			    Assert.AreEqual( true, bit );
+
+			    Assert.IsTrue(bitReader.TryBeginReadBits(32));
+			    bitReader.ReadBits(out outVal, 31 );
+			    bitReader.ReadBit(out bit);
+			    Assert.AreEqual( inVal, outVal );
+			    Assert.AreEqual( true, bit );
+		    }
+	    }
+    }
+}