-
Notifications
You must be signed in to change notification settings - Fork 450
fix: Fixed NetworkVariables being truncated to short.MaxValue when NetworkConfig.EnsureNetworkVariableLengthSafety is enabled. #1811
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
Changes from all commits
fb5df79
87470b0
744b130
4c4ab63
998a85f
5b416b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,7 +57,7 @@ public void Serialize(FastBufferWriter writer) | |
{ | ||
if (!shouldWrite) | ||
{ | ||
writer.WriteValueSafe((ushort)0); | ||
BytePacker.WriteValueBitPacked(writer, 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😻 |
||
} | ||
} | ||
else | ||
|
@@ -69,15 +69,15 @@ public void Serialize(FastBufferWriter writer) | |
{ | ||
if (NetworkBehaviour.NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety) | ||
{ | ||
var tempWriter = new FastBufferWriter(MessagingSystem.NON_FRAGMENTED_MESSAGE_MAX_SIZE, Allocator.Temp, short.MaxValue); | ||
networkVariable.WriteDelta(tempWriter); | ||
var tempWriter = new FastBufferWriter(MessagingSystem.NON_FRAGMENTED_MESSAGE_MAX_SIZE, Allocator.Temp, MessagingSystem.FRAGMENTED_MESSAGE_MAX_SIZE); | ||
NetworkBehaviour.NetworkVariableFields[i].WriteDelta(tempWriter); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit, can still use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was actually a merge thing. That local variable was introduced in another change that was made to develop while I was working on this and the merge naturally didn't update this line. I'll update it. |
||
BytePacker.WriteValueBitPacked(writer, tempWriter.Length); | ||
|
||
if (!writer.TryBeginWrite(FastBufferWriter.GetWriteSize<ushort>() + tempWriter.Length)) | ||
if (!writer.TryBeginWrite(tempWriter.Length)) | ||
{ | ||
throw new OverflowException($"Not enough space in the buffer to write {nameof(NetworkVariableDeltaMessage)}"); | ||
} | ||
|
||
writer.WriteValue((ushort)tempWriter.Length); | ||
tempWriter.CopyTo(writer); | ||
} | ||
else | ||
|
@@ -135,10 +135,10 @@ public void Handle(ref NetworkContext context) | |
{ | ||
for (int i = 0; i < networkBehaviour.NetworkVariableFields.Count; i++) | ||
{ | ||
ushort varSize = 0; | ||
int varSize = 0; | ||
if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety) | ||
{ | ||
m_ReceivedNetworkVariableData.ReadValueSafe(out varSize); | ||
ByteUnpacker.ReadValueBitPacked(m_ReceivedNetworkVariableData, out varSize); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😻 |
||
|
||
if (varSize == 0) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ namespace Unity.Netcode | |
public static class BytePacker | ||
{ | ||
#if UNITY_NETCODE_DEBUG_NO_PACKING | ||
|
||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
public void WriteValuePacked<T>(FastBufferWriter writer, T value) where T: unmanaged => writer.WriteValueSafe(value); | ||
#else | ||
|
@@ -277,10 +277,21 @@ public static void WriteValuePacked(FastBufferWriter writer, string s) | |
|
||
|
||
#if UNITY_NETCODE_DEBUG_NO_PACKING | ||
|
||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
public void WriteValueBitPacked<T>(FastBufferWriter writer, T value) where T: unmanaged => writer.WriteValueSafe(value); | ||
#else | ||
|
||
public const ushort BitPackedUshortMax = (1 << 15) - 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😻 |
||
public const short BitPackedShortMax = (1 << 14) - 1; | ||
public const short BitPackedShortMin = -(1 << 14); | ||
public const uint BitPackedUintMax = (1 << 30) - 1; | ||
public const int BitPackedIntMax = (1 << 29) - 1; | ||
public const int BitPackedIntMin = -(1 << 29); | ||
public const ulong BitPackedULongMax = (1L << 61) - 1; | ||
public const long BitPackedLongMax = (1L << 60) - 1; | ||
public const long BitPackedLongMin = -(1L << 60); | ||
|
||
/// <summary> | ||
/// Writes a 14-bit signed short to the buffer in a bit-encoded packed format. | ||
/// The first bit indicates whether the value is 1 byte or 2. | ||
|
@@ -307,7 +318,7 @@ public static void WriteValuePacked(FastBufferWriter writer, string s) | |
public static void WriteValueBitPacked(FastBufferWriter writer, ushort value) | ||
{ | ||
#if DEVELOPMENT_BUILD || UNITY_EDITOR | ||
if (value >= 0b1000_0000_0000_0000) | ||
if (value >= BitPackedUshortMax) | ||
{ | ||
throw new ArgumentException("BitPacked ushorts must be <= 15 bits"); | ||
} | ||
|
@@ -356,7 +367,7 @@ public static void WriteValueBitPacked(FastBufferWriter writer, ushort value) | |
public static void WriteValueBitPacked(FastBufferWriter writer, uint value) | ||
{ | ||
#if DEVELOPMENT_BUILD || UNITY_EDITOR | ||
if (value >= 0b0100_0000_0000_0000_0000_0000_0000_0000) | ||
if (value > BitPackedUintMax) | ||
{ | ||
throw new ArgumentException("BitPacked uints must be <= 30 bits"); | ||
} | ||
|
@@ -396,7 +407,7 @@ public static void WriteValueBitPacked(FastBufferWriter writer, uint value) | |
public static void WriteValueBitPacked(FastBufferWriter writer, ulong value) | ||
{ | ||
#if DEVELOPMENT_BUILD || UNITY_EDITOR | ||
if (value >= 0b0010_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000_0000) | ||
if (value > BitPackedULongMax) | ||
{ | ||
throw new ArgumentException("BitPacked ulongs must be <= 61 bits"); | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit-pick)
Could you update your entry to this: "- Fixed network..." and move this entry to the top of the list so we keep a descending order on the PR numbers?