Skip to content

fix: Creating a zero-length FastBufferReader no longer reports its size as 1 (backport) #1724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
- Fixed error when serializing ConnectionApprovalMessage with scene management disabled when one or more objects is hidden via the CheckObjectVisibility delegate (#1720)
- Fixed CheckObjectVisibility delegate not being properly invoked for connecting clients when Scene Management is enabled. (#1680)
- Fixed NetworkList to properly call INetworkSerializable's NetworkSerialize() method (#1682)
- Fixed FastBufferReader being created with a length of 1 if provided an input of length 0 (#1724)
- Fixed The NetworkConfig's checksum hash includes the NetworkTick so that clients with a different tickrate than the server are identified and not allowed to connect (#1728)
- Fixed OwnedObjects not being properly modified when using ChangeOwnership (#1731)
- Improved performance in NetworkAnimator (#1735)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ internal void __endSendClientRpc(ref FastBufferWriter bufferWriter, uint rpcMeth
};
clientRpcMessage.ReadBuffer = tempBuffer;
clientRpcMessage.Handle(ref context);
rpcWriteSize = tempBuffer.Length;
}

bufferWriter.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ internal unsafe void CommitBitwiseReads(int amount)

/// <summary>
/// Create a FastBufferReader from a NativeArray.
///
///
/// A new buffer will be created using the given allocator and the value will be copied in.
/// FastBufferReader will then own the data.
///
Expand All @@ -93,12 +93,12 @@ internal unsafe void CommitBitwiseReads(int amount)
/// <param name="offset"></param>
public unsafe FastBufferReader(NativeArray<byte> buffer, Allocator allocator, int length = -1, int offset = 0)
{
Handle = CreateHandle((byte*)buffer.GetUnsafePtr(), Math.Max(1, length == -1 ? buffer.Length : length), offset, allocator);
Handle = CreateHandle((byte*)buffer.GetUnsafePtr(), length == -1 ? buffer.Length : length, offset, allocator);
}

/// <summary>
/// Create a FastBufferReader from an ArraySegment.
///
///
/// A new buffer will be created using the given allocator and the value will be copied in.
/// FastBufferReader will then own the data.
///
Expand All @@ -117,13 +117,13 @@ public unsafe FastBufferReader(ArraySegment<byte> buffer, Allocator allocator, i
}
fixed (byte* data = buffer.Array)
{
Handle = CreateHandle(data, Math.Max(1, length == -1 ? buffer.Count : length), offset, allocator);
Handle = CreateHandle(data, length == -1 ? buffer.Count : length, offset, allocator);
}
}

/// <summary>
/// Create a FastBufferReader from an existing byte array.
///
///
/// A new buffer will be created using the given allocator and the value will be copied in.
/// FastBufferReader will then own the data.
///
Expand All @@ -142,13 +142,13 @@ public unsafe FastBufferReader(byte[] buffer, Allocator allocator, int length =
}
fixed (byte* data = buffer)
{
Handle = CreateHandle(data, Math.Max(1, length == -1 ? buffer.Length : length), offset, allocator);
Handle = CreateHandle(data, length == -1 ? buffer.Length : length, offset, allocator);
}
}

/// <summary>
/// Create a FastBufferReader from an existing byte buffer.
///
///
/// A new buffer will be created using the given allocator and the value will be copied in.
/// FastBufferReader will then own the data.
///
Expand All @@ -165,12 +165,12 @@ public unsafe FastBufferReader(byte[] buffer, Allocator allocator, int length =
/// <param name="offset">The offset of the buffer to start copying from</param>
public unsafe FastBufferReader(byte* buffer, Allocator allocator, int length, int offset = 0)
{
Handle = CreateHandle(buffer, Math.Max(1, length), offset, allocator);
Handle = CreateHandle(buffer, length, offset, allocator);
}

/// <summary>
/// Create a FastBufferReader from a FastBufferWriter.
///
///
/// A new buffer will be created using the given allocator and the value will be copied in.
/// FastBufferReader will then own the data.
///
Expand All @@ -187,7 +187,7 @@ public unsafe FastBufferReader(byte* buffer, Allocator allocator, int length, in
/// <param name="offset">The offset of the buffer to start copying from</param>
public unsafe FastBufferReader(FastBufferWriter writer, Allocator allocator, int length = -1, int offset = 0)
{
Handle = CreateHandle(writer.GetUnsafePtr(), Math.Max(1, length == -1 ? writer.Length : length), offset, allocator);
Handle = CreateHandle(writer.GetUnsafePtr(), length == -1 ? writer.Length : length, offset, allocator);
}

/// <summary>
Expand Down Expand Up @@ -250,7 +250,7 @@ public unsafe BitReader EnterBitwiseContext()
/// When you know you will be reading multiple fields back-to-back and you know the total size,
/// you can call TryBeginRead() once on the total size, and then follow it with calls to
/// ReadValue() instead of ReadValueSafe() for faster serialization.
///
///
/// Unsafe read operations will throw OverflowException in editor and development builds if you
/// go past the point you've marked using TryBeginRead(). In release builds, OverflowException will not be thrown
/// for performance reasons, since the point of using TryBeginRead is to avoid bounds checking in the following
Expand Down Expand Up @@ -284,7 +284,7 @@ public unsafe bool TryBeginRead(int bytes)
/// When you know you will be reading multiple fields back-to-back and you know the total size,
/// you can call TryBeginRead() once on the total size, and then follow it with calls to
/// ReadValue() instead of ReadValueSafe() for faster serialization.
///
///
/// Unsafe read operations will throw OverflowException in editor and development builds if you
/// go past the point you've marked using TryBeginRead(). In release builds, OverflowException will not be thrown
/// for performance reasons, since the point of using TryBeginRead is to avoid bounds checking in the following
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,52 @@ public unsafe void GivenFastBufferReaderInitializedFromFastBufferWriterContainin
}
}

[Test]
public void WhenCreatingAReaderFromAnEmptyArraySegment_LengthIsZero()
{
var bytes = new byte[] { };
var input = new ArraySegment<byte>(bytes, 0, 0);
using var reader = new FastBufferReader(input, Allocator.Temp);
Assert.AreEqual(0, reader.Length);
}

[Test]
public void WhenCreatingAReaderFromAnEmptyArray_LengthIsZero()
{
var input = new byte[] { };
using var reader = new FastBufferReader(input, Allocator.Temp);
Assert.AreEqual(0, reader.Length);
}

[Test]
public void WhenCreatingAReaderFromAnEmptyNativeArray_LengthIsZero()
{
var input = new NativeArray<byte>(0, Allocator.Temp);
using var reader = new FastBufferReader(input, Allocator.Temp);
Assert.AreEqual(0, reader.Length);
}

[Test]
public void WhenCreatingAReaderFromAnEmptyFastBufferWriter_LengthIsZero()
{
var input = new FastBufferWriter(0, Allocator.Temp);
using var reader = new FastBufferReader(input, Allocator.Temp);
Assert.AreEqual(0, reader.Length);
}

[Test]
public void WhenCreatingAReaderFromAnEmptyBuffer_LengthIsZero()
{
var input = new byte[] { };
unsafe
{
fixed (byte* ptr = input)
{
using var reader = new FastBufferReader(ptr, Allocator.Temp, 0);
Assert.AreEqual(0, reader.Length);
}
}
}

[Test]
public void WhenCallingReadByteWithoutCallingTryBeingReadFirst_OverflowExceptionIsThrown()
Expand Down