Merge branch 'develop-3.x.x' into fix/fastbufferwriter-grow-from-zero-up-port

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -25,6 +25,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - 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 promoted to session owner could throw an exception when synchronizing a newly joining client. (#4214)
 - Issue where `NetworkManager.OnNetCodeDisconnect` was not invoked when a connection closed during a hybrid mode session, only when the session shut down. (#4209)
 - Issue where a rejected parent change, for example one made by a client without parenting authority, restored the `NetworkObject`'s parent but left it at the moved position when the change was made with `worldPositionStays` disabled. The original position, rotation and scale are now restored. (#4202)

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

@@ -90,6 +90,16 @@ internal unsafe void CommitBitwiseReads(int amount)
             return readerHandle;
         }
 
+        // A copy starts at offset, so -1 copies what follows it. Allocator.None keeps the whole source and starts reading at offset.
+        private static int ResolveLength(int length, int sourceLength, int offset, Allocator copyAllocator)
+        {
+            if (length != -1)
+            {
+                return length;
+            }
+            return copyAllocator == Allocator.None ? sourceLength : sourceLength - offset;
+        }
+
         /// <summary>
         /// Create a FastBufferReader from a NativeArray.
         ///
@@ -107,12 +117,12 @@ internal unsafe void CommitBitwiseReads(int amount)
         /// </summary>
         /// <param name="buffer">The NativeArray to create the reader from</param>
         /// <param name="copyAllocator">The allocator type used for internal data when copying an existing buffer if other than Allocator.None is specified, that memory will be owned by this FastBufferReader instance</param>
-        /// <param name="length">The number of bytes to read from the buffer. If set to -1, the entire buffer length will be used</param>
+        /// <param name="length">The number of bytes to read from the buffer. If set to -1, every byte from offset to the end of the buffer will be used</param>
         /// <param name="offset">The offset into the buffer to start reading from</param>
         /// <param name="internalAllocator">The allocator type used for internal data when this reader points directly at a buffer owned by someone else</param>
         public unsafe FastBufferReader(NativeArray<byte> buffer, Allocator copyAllocator, int length = -1, int offset = 0, Allocator internalAllocator = Allocator.Temp)
         {
-            Handle = CreateHandle((byte*)buffer.GetUnsafePtr(), length == -1 ? buffer.Length : length, offset, copyAllocator, internalAllocator);
+            Handle = CreateHandle((byte*)buffer.GetUnsafePtr(), ResolveLength(length, buffer.Length, offset, copyAllocator), offset, copyAllocator, internalAllocator);
         }
 
         /// <summary>
@@ -127,7 +137,7 @@ public unsafe FastBufferReader(NativeArray<byte> buffer, Allocator copyAllocator
         /// <param name="buffer">The buffer to copy from</param>
         /// <param name="copyAllocator">The allocator type used for internal data when copying an existing buffer if other than Allocator.None is specified, that memory will be owned by this FastBufferReader instance</param>
         /// <param name="length">The number of bytes to copy (all if this is -1)</param>
-        /// <param name="offset">The offset of the buffer to start copying from</param>
+        /// <param name="offset">The offset into the segment to start copying from</param>
         public unsafe FastBufferReader(ArraySegment<byte> buffer, Allocator copyAllocator, int length = -1, int offset = 0)
         {
             if (copyAllocator == Allocator.None)
@@ -136,7 +146,7 @@ public unsafe FastBufferReader(ArraySegment<byte> buffer, Allocator copyAllocato
             }
             fixed (byte* data = buffer.Array)
             {
-                Handle = CreateHandle(data, length == -1 ? buffer.Count : length, offset, copyAllocator, Allocator.Temp);
+                Handle = CreateHandle(data + buffer.Offset, ResolveLength(length, buffer.Count, offset, copyAllocator), offset, copyAllocator, Allocator.Temp);
             }
         }
 
@@ -209,7 +219,7 @@ public unsafe FastBufferReader(byte[] buffer, Allocator copyAllocator, int lengt
             }
             fixed (byte* data = buffer)
             {
-                Handle = CreateHandle(data, length == -1 ? buffer.Length : length, offset, copyAllocator, Allocator.Temp);
+                Handle = CreateHandle(data, ResolveLength(length, buffer.Length, offset, copyAllocator), offset, copyAllocator, Allocator.Temp);
             }
         }
 
@@ -260,7 +270,7 @@ public unsafe FastBufferReader(byte* buffer, Allocator copyAllocator, int length
         /// <param name="internalAllocator">The allocator type used for internal data when this reader points directly at a buffer owned by someone else</param>
         public unsafe FastBufferReader(FastBufferWriter writer, Allocator copyAllocator, int length = -1, int offset = 0, Allocator internalAllocator = Allocator.Temp)
         {
-            Handle = CreateHandle(writer.GetUnsafePtr(), length == -1 ? writer.Length : length, offset, copyAllocator, internalAllocator);
+            Handle = CreateHandle(writer.GetUnsafePtr(), ResolveLength(length, writer.Length, offset, copyAllocator), offset, copyAllocator, internalAllocator);
         }
 
         /// <summary>
@@ -285,7 +295,7 @@ public unsafe FastBufferReader(FastBufferWriter writer, Allocator copyAllocator,
         /// <param name="internalAllocator">The allocator type used for internal data when this reader points directly at a buffer owned by someone else</param>
         public unsafe FastBufferReader(FastBufferReader reader, Allocator copyAllocator, int length = -1, int offset = 0, Allocator internalAllocator = Allocator.Temp)
         {
-            Handle = CreateHandle(reader.GetUnsafePtr(), length == -1 ? reader.Length : length, offset, copyAllocator, internalAllocator);
+            Handle = CreateHandle(reader.GetUnsafePtr(), ResolveLength(length, reader.Length, offset, copyAllocator), offset, copyAllocator, internalAllocator);
         }
 
         /// <summary>

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

@@ -1118,6 +1118,78 @@ public void WhenCreatingAReaderFromAnEmptyBuffer_LengthIsZero()
             }
         }
 
+        private const int k_SourceLength = 10;
+        private const int k_ReaderOffset = 2;
+
+        private static byte[] CreateSequentialBytes()
+        {
+            var bytes = new byte[k_SourceLength];
+            for (var i = 0; i < bytes.Length; ++i)
+            {
+                bytes[i] = (byte)i;
+            }
+            return bytes;
+        }
+
+        private static void AssertReadsToEndFrom(FastBufferReader reader, int firstValue)
+        {
+            var expectedCount = k_SourceLength - firstValue;
+            Assert.AreEqual(expectedCount, reader.Length - reader.Position);
+            for (var i = 0; i < expectedCount; ++i)
+            {
+                reader.ReadByteSafe(out byte value);
+                Assert.AreEqual((byte)(firstValue + i), value);
+            }
+        }
+
+        [Test]
+        public void WhenCreatingAReaderFromAnArrayWithAnOffsetAndNoLength_TheRestOfTheArrayIsCopied()
+        {
+            using var reader = new FastBufferReader(CreateSequentialBytes(), Allocator.Temp, -1, k_ReaderOffset);
+            AssertReadsToEndFrom(reader, k_ReaderOffset);
+        }
+
+        [Test]
+        public void WhenCreatingAReaderFromAnArraySegmentWithAnOffsetAndNoLength_TheRestOfTheSegmentIsCopied()
+        {
+            using var reader = new FastBufferReader(new ArraySegment<byte>(CreateSequentialBytes()), Allocator.Temp, -1, k_ReaderOffset);
+            AssertReadsToEndFrom(reader, k_ReaderOffset);
+        }
+
+        [Test]
+        public void WhenCreatingAReaderFromAnOffsetArraySegmentWithAnOffset_TheOffsetIsRelativeToTheSegment()
+        {
+            var segmentOffset = 3;
+            var segment = new ArraySegment<byte>(CreateSequentialBytes(), segmentOffset, k_SourceLength - segmentOffset);
+            using var reader = new FastBufferReader(segment, Allocator.Temp, -1, k_ReaderOffset);
+            AssertReadsToEndFrom(reader, segmentOffset + k_ReaderOffset);
+        }
+
+        [Test]
+        public void WhenCreatingAReaderFromANativeArrayWithAnOffsetAndNoLength_TheRestOfTheArrayIsRead([Values(Allocator.Temp, Allocator.None)] Allocator copyAllocator)
+        {
+            using var source = new NativeArray<byte>(CreateSequentialBytes(), Allocator.Temp);
+            using var reader = new FastBufferReader(source, copyAllocator, -1, k_ReaderOffset);
+            AssertReadsToEndFrom(reader, k_ReaderOffset);
+        }
+
+        [Test]
+        public void WhenCreatingAReaderFromAWriterWithAnOffsetAndNoLength_TheRestOfTheWriterIsRead([Values(Allocator.Temp, Allocator.None)] Allocator copyAllocator)
+        {
+            using var writer = new FastBufferWriter(k_SourceLength, Allocator.Temp);
+            writer.WriteBytesSafe(CreateSequentialBytes());
+            using var reader = new FastBufferReader(writer, copyAllocator, -1, k_ReaderOffset);
+            AssertReadsToEndFrom(reader, k_ReaderOffset);
+        }
+
+        [Test]
+        public void WhenCreatingAReaderFromAReaderWithAnOffsetAndNoLength_TheRestOfTheReaderIsRead([Values(Allocator.Temp, Allocator.None)] Allocator copyAllocator)
+        {
+            using var source = new FastBufferReader(CreateSequentialBytes(), Allocator.Temp);
+            using var reader = new FastBufferReader(source, copyAllocator, -1, k_ReaderOffset);
+            AssertReadsToEndFrom(reader, k_ReaderOffset);
+        }
+
         [Test]
         public void WhenCreatingNewFastBufferReader_IsInitializedIsTrue()
         {