fix: Fixed NetworkVariables being truncated to short.MaxValue when NetworkConfig.EnsureNetworkVariableLengthSafety is enabled. #1811
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tworkConfig.EnsureNetworkVariableLengthSafety is enabled. In the process it was discovered that FRAGMENTED_MESSAGE_MAX_SIZE was actually larger than BytePacker.WriteValueBitPacked can support, so that was reduced to the actual correct max value.
| @@ -18,6 +18,7 @@ Additional documentation and release notes are available at [Multiplayer Documen | |||
| - Fixed issue where NetworkManager would continue starting even if the NetworkTransport selected failed. (#1780) | |||
| - Fixed issue when spawning new player if an already existing player exists it does not remove IsPlayer from the previous player (#1779) | |||
| - Fixed lack of notification that NetworkManager and NetworkObject cannot be added to the same GameObject with in-editor notifications (#1777) | |||
| - Network variable updates are no longer limited to 32,768 bytes when NetworkConfig.EnsureNetworkVariableLengthSafety is enabled. The limits are now determined by what the transport can send in a message. (#1811) | |||
There was a problem hiding this comment.
(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?
| @@ -56,7 +56,7 @@ public void Serialize(FastBufferWriter writer) | |||
| { | |||
| if (!shouldWrite) | |||
| { | |||
| writer.WriteValueSafe((ushort)0); | |||
| BytePacker.WriteValueBitPacked(writer, 0); | |||
|
|
||
| if (networkManager.NetworkConfig.EnsureNetworkVariableLengthSafety) | ||
| { | ||
| m_ReceivedNetworkVariableData.ReadValueSafe(out varSize); | ||
| ByteUnpacker.ReadValueBitPacked(m_ReceivedNetworkVariableData, out varSize); |
| [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; |
NoelStephensUnity
approved these changes
Mar 17, 2022
0xFA11
approved these changes
Mar 18, 2022
mattwalsh-unity
approved these changes
Mar 18, 2022
| 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.
Nit, can still use the networkVariable temporary here. More consistent if we create the temporary to use it throughout
There was a problem hiding this comment.
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.
0xFA11
deleted the
fix/networkvars_truncated_to_32k_when_lengthsafety_enabled
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixed NetworkVariables being truncated to short.MaxValue when NetworkConfig.EnsureNetworkVariableLengthSafety is enabled.
In the process it was discovered that FRAGMENTED_MESSAGE_MAX_SIZE was actually larger than BytePacker.WriteValueBitPacked can support, so that was reduced to the actual correct max value.
MTT-2434
Changelog
com.unity.netcode.gameobjects
Testing and Documentation