From a15788b1d4af0e72de9fa639fb909011f8803388 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 24 Feb 2025 12:35:00 -0600 Subject: [PATCH 1/7] fix Fixing issue where FastBufferReader did not provide constructors to honor an ArraySegment's configuration. --- .../Runtime/Serialization/FastBufferReader.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs b/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs index 67d787fea5..28f8e8f198 100644 --- a/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs +++ b/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs @@ -136,10 +136,61 @@ public unsafe FastBufferReader(ArraySegment buffer, Allocator copyAllocato } fixed (byte* data = buffer.Array) { + Handle = CreateHandle(data, length == -1 ? buffer.Count : length, offset, copyAllocator, Allocator.Temp); } } + /// + /// Create a FastBufferReader from an ArraySegment that uses the ArraySegment.Offset for the reader's offset. + /// + /// A new buffer will be created using the given allocator and the value will be copied in. + /// FastBufferReader will then own the data. + /// + /// Allocator.None is not supported for byte[]. If you need this functionality, use a fixed() block + /// and ensure the FastBufferReader isn't used outside that block. + /// + /// The buffer to copy from + /// 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 + /// The number of bytes to copy (all if this is -1) + /// + public unsafe FastBufferReader(ArraySegment buffer, Allocator copyAllocator, int length = -1) + { + if (copyAllocator == Allocator.None) + { + throw new NotSupportedException("Allocator.None cannot be used with managed source buffers."); + } + fixed (byte* data = buffer.Array) + { + + Handle = CreateHandle(data, length == -1 ? buffer.Count : length, buffer.Offset, copyAllocator, Allocator.Temp); + } + } + + /// + /// Create a FastBufferReader from an ArraySegment that uses the ArraySegment.Offset for the reader's offset and the ArraySegment.Count for the reader's length. + /// + /// A new buffer will be created using the given allocator and the value will be copied in. + /// FastBufferReader will then own the data. + /// + /// Allocator.None is not supported for byte[]. If you need this functionality, use a fixed() block + /// and ensure the FastBufferReader isn't used outside that block. + /// + /// The buffer to copy from + /// 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 + /// + public unsafe FastBufferReader(ArraySegment buffer, Allocator copyAllocator) + { + if (copyAllocator == Allocator.None) + { + throw new NotSupportedException("Allocator.None cannot be used with managed source buffers."); + } + fixed (byte* data = buffer.Array) + { + Handle = CreateHandle(data, buffer.Count, buffer.Offset, copyAllocator, Allocator.Temp); + } + } + /// /// Create a FastBufferReader from an existing byte array. /// From 69739633838efccfb51f52a9b61df134a8669253 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 24 Feb 2025 12:38:18 -0600 Subject: [PATCH 2/7] update adding change log entries. --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index ee8ba331c5..6255511ea5 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -9,6 +9,8 @@ Additional documentation and release notes are available at [Multiplayer Documen ## [Unreleased] ### Added +- Added `FastBufferReader(ArraySegment buffer, Allocator copyAllocator)` constructor that uses the `ArraySegment.Offset` as the `FastBufferReader` offset and the `ArraySegment.Count` as the `FastBufferReader` length. +- Added `FastBufferReader(ArraySegment buffer, Allocator copyAllocator, int length = -1)` constructor that uses the `ArraySegment.Offset` as the `FastBufferReader` offset. ### Fixed From 8633d4a7bdf7743b37b97f5986ad0998844ed934 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 24 Feb 2025 12:48:35 -0600 Subject: [PATCH 3/7] style removing the not supported exception from xml api --- .../Runtime/Serialization/FastBufferReader.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs b/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs index 28f8e8f198..7f87e02240 100644 --- a/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs +++ b/com.unity.netcode.gameobjects/Runtime/Serialization/FastBufferReader.cs @@ -153,7 +153,6 @@ public unsafe FastBufferReader(ArraySegment buffer, Allocator copyAllocato /// The buffer to copy from /// 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 /// The number of bytes to copy (all if this is -1) - /// public unsafe FastBufferReader(ArraySegment buffer, Allocator copyAllocator, int length = -1) { if (copyAllocator == Allocator.None) @@ -178,7 +177,6 @@ public unsafe FastBufferReader(ArraySegment buffer, Allocator copyAllocato /// /// The buffer to copy from /// 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 - /// public unsafe FastBufferReader(ArraySegment buffer, Allocator copyAllocator) { if (copyAllocator == Allocator.None) From c4125918ea6fc481958cb4802500be766c68a3db Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 24 Feb 2025 13:01:18 -0600 Subject: [PATCH 4/7] test Adding unit test to run a simple validation for this update. --- .../Serialization/FastBufferReaderTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs index 2b4c70ebcb..de3251ac12 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs @@ -1579,5 +1579,21 @@ public unsafe void WhenCallingTryBeginReadInternal_AllowedReadPositionDoesNotMov Assert.AreEqual(reader.Handle->AllowedReadMark, 25); } } + + [Test] + public unsafe void WhenUsingArraySegment_ConstructorHonorsArraySegmentConfiguration() + { + var bytes = new byte[] { 0, 1, 2, 3 }; + var segment = new ArraySegment(bytes, 1, 3); + var reader = new FastBufferReader(segment, Allocator.Temp); + + var readerArray = reader.ToArray(); + Assert.True(readerArray.Length == bytes.Length - 1, $"Array of reader should have a length of {bytes.Length - 1} but was {readerArray.Length}!"); + for(int i = 0; i < readerArray.Length; i++) + { + Assert.True(bytes[i + 1] == readerArray[i], $"Value of {nameof(readerArray)} at index {i} is {readerArray[i]} but should be {bytes[i + 1]}!"); + } + reader.Dispose(); + } } } From 4bb0d5b1e3378e3cc1539c45daad2081a88488bf Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 24 Feb 2025 13:03:02 -0600 Subject: [PATCH 5/7] update Adding PR number to change log entries. --- com.unity.netcode.gameobjects/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 6255511ea5..dc5ccfb5bc 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -9,8 +9,8 @@ Additional documentation and release notes are available at [Multiplayer Documen ## [Unreleased] ### Added -- Added `FastBufferReader(ArraySegment buffer, Allocator copyAllocator)` constructor that uses the `ArraySegment.Offset` as the `FastBufferReader` offset and the `ArraySegment.Count` as the `FastBufferReader` length. -- Added `FastBufferReader(ArraySegment buffer, Allocator copyAllocator, int length = -1)` constructor that uses the `ArraySegment.Offset` as the `FastBufferReader` offset. +- Added `FastBufferReader(ArraySegment buffer, Allocator copyAllocator)` constructor that uses the `ArraySegment.Offset` as the `FastBufferReader` offset and the `ArraySegment.Count` as the `FastBufferReader` length. (#3320) +- Added `FastBufferReader(ArraySegment buffer, Allocator copyAllocator, int length = -1)` constructor that uses the `ArraySegment.Offset` as the `FastBufferReader` offset. (#3320) ### Fixed From 8dd8c1db7030e30b0260f2c1555cd6df62b5c9b9 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 24 Feb 2025 13:11:52 -0600 Subject: [PATCH 6/7] update Excluding the PVP-151 exception for this added test. --- pvpExceptions.json | 1 + 1 file changed, 1 insertion(+) diff --git a/pvpExceptions.json b/pvpExceptions.json index bab6a87515..4deefeeb86 100644 --- a/pvpExceptions.json +++ b/pvpExceptions.json @@ -731,6 +731,7 @@ "Unity.Netcode.EditorTests.FastBufferReaderTests: void WhenCallingTryBeginRead_TheAllowedReadPositionIsMarkedRelativeToCurrentPosition(): undocumented", "Unity.Netcode.EditorTests.FastBufferReaderTests: void WhenReadingAfterSeeking_TheNewReadComesFromTheCorrectPosition(): undocumented", "Unity.Netcode.EditorTests.FastBufferReaderTests: void WhenCallingTryBeginReadInternal_AllowedReadPositionDoesNotMoveBackward(): undocumented", + "Unity.Netcode.EditorTests.FastBufferReaderTests: void WhenUsingArraySegment_ConstructorHonorsArraySegmentConfiguration(): undocumented", "Unity.Netcode.EditorTests.FastBufferWriterTests: undocumented", "Unity.Netcode.EditorTests.FastBufferWriterTests: void RunTypeTest(T): undocumented", "Unity.Netcode.EditorTests.FastBufferWriterTests: void RunTypeTestSafe(T): undocumented", From ac7e46d0f66210f132cd878ac72d0388a647e26d Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 24 Feb 2025 13:58:16 -0600 Subject: [PATCH 7/7] style missing white space --- .../Tests/Editor/Serialization/FastBufferReaderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs b/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs index de3251ac12..17d0c979d9 100644 --- a/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs @@ -1589,7 +1589,7 @@ public unsafe void WhenUsingArraySegment_ConstructorHonorsArraySegmentConfigurat var readerArray = reader.ToArray(); Assert.True(readerArray.Length == bytes.Length - 1, $"Array of reader should have a length of {bytes.Length - 1} but was {readerArray.Length}!"); - for(int i = 0; i < readerArray.Length; i++) + for (int i = 0; i < readerArray.Length; i++) { Assert.True(bytes[i + 1] == readerArray[i], $"Value of {nameof(readerArray)} at index {i} is {readerArray[i]} but should be {bytes[i + 1]}!"); }