feat: INetworkMessage #1187
Merged
feat: INetworkMessage #1187
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
… some xmldoc comments though)
…(aside from INetworkSerializable, which isn't ready to be tested yet due to lack of BufferSerializer)
- Streamlined the implementations of bit-packing uints and ulongs.
…lacing with byte* allocated directly through UnsafeUtility.Malloc() (same function used under the hood by NativeArray). This is required in order to be able to store pointers to FastBufferReader and FastBufferWriter in order to be able to wrap them with BufferSerializer - NativeArray contains a managed variable in it that disallows taking a pointer to it. And since FBW and FBR are structs, pointers are the only way to wrap them "by reference" in another struct - ref fields aren't allowed even inside ref structs.
…g values in other ref structs in a capture-by-reference style, updated BitReader and BitWriter to use that. Also aggressively inlining properties in FBW and FBR.
…. it's a little slower, but apparently some platforms won't support unaligned memory access (e.g., WEBGL, ARM processors) and there's no compile time way to detect ARM processors since the bytecode is not processor-dependent... the cost of the runtime detection would be more expensive than the cost of just doing the memcpy.
…afeUtility.MemCpy for small values, while still supporting unaligned access.
… restructuring of some of the other tests.
…ork correctly (requesting beyond MaxCapacity and requesting more than double current capacity) -Added support for FastBufferReader to be used in a mode that doesn't copy the input buffer
- Fixed incorrect text in a warning in FastBufferReader
# Conflicts: # com.unity.netcode.gameobjects/Runtime/com.unity.netcode.runtime.asmdef # com.unity.netcode.gameobjects/Tests/Editor/com.unity.netcode.editortests.asmdef
…ConnectionApprovedMessage. A couple of tests are known to be failing... this is to let the work start being divided up.
Several tests are failing... fixing them will be the next checkin.
0xFA11
reviewed
Sep 16, 2021
0xFA11
reviewed
Sep 16, 2021
com.unity.netcode.gameobjects/Runtime/Messaging/Messages/TimeSyncMessage.cs
Show resolved
Hide resolved
0xFA11
reviewed
Sep 17, 2021
There was a problem hiding this comment.
also, do you mind doing an overall pass on all types and APIs to check public types and methods we're exposing?
I reckon you might catch some types/APIs that can be internal, private, protected etc instead of public.
and, if you do that, please also list them in the description of this PR so that we could have that list as a guidance.
finally, please make sure to have xmldocs on publicly exposed types so that we can make docs people's lives a little bit easier and we might also have a plan afterwars about authoring more in-depth docs later.
@MFatihMAR I will try to do this today but please don't let this hold up approval of the PR. Adding docs can be done after feature freeze. If this PR doesn't make it in today, it's out. |
jeffreyrainy
approved these changes
Sep 17, 2021
There was a problem hiding this comment.
I could not go over everything, there is a lot.
However, the parts I looked at are good and overall, it improves things a lot. It makes it easier to enable snapshot, as many tests got more resistant to timings change.
Due to shear size, I'm not sure it's perfect. But I'm sure it moves in the right direction, significantly.
LukeStampfli
approved these changes
Sep 17, 2021
# Conflicts: # com.unity.netcode.gameobjects/Runtime/Configuration/NetworkConfig.cs # com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs # com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs # com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs # com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs # com.unity.netcode.gameobjects/Runtime/Messaging/InternalMessageHandler.cs # com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs # com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs # com.unity.netcode.gameobjects/Tests/Runtime/Metrics/OwnershipChangeMetricsTests.cs
# Conflicts: # com.unity.netcode.adapter.utp/Tests/Runtime/Helpers/RuntimeTestsHelpers.cs
SamuelBellomo
added a commit
that referenced
this pull request
Sep 17, 2021
* develop: feat: INetworkMessage (#1187) fix: Increase timeout for UTP adapter tests (#1199) docs: Update repo and both package readme files (#1180) chore: updating UTP package to pull in DTLS fixes (#1197) fix: network time arguments (#1194) feat: network physics (#1175) # Conflicts: # com.unity.netcode.gameobjects/Components/Interpolator/BufferedLinearInterpolator.cs # com.unity.netcode.gameobjects/Components/NetworkTransform.cs # testproject/Assets/Prefabs/PlayerCube.prefab
SamuelBellomo
added a commit
that referenced
this pull request
Sep 17, 2021
…transform-teleport * sam/feature/client-network-transform: adding proper delta sending feat: INetworkMessage (#1187) fix: Increase timeout for UTP adapter tests (#1199) docs: Update repo and both package readme files (#1180) chore: updating UTP package to pull in DTLS fixes (#1197) fix: network time arguments (#1194) feat: network physics (#1175) # Conflicts: # testproject/Assets/Scenes/ZooSam.unity
mollstam
pushed a commit
to Keepsake-Games/com.unity.netcode.gameobjects
that referenced
this pull request
Feb 13, 2023
* FastBufferWriter implemented and tested (still need to add and update some xmldoc comments though) * A few additional tests to cover the last missed cases I can think of (aside from INetworkSerializable, which isn't ready to be tested yet due to lack of BufferSerializer) * FastBufferReader + tests * - More tests - Streamlined the implementations of bit-packing uints and ulongs. * - Removed NativeArray from FastBufferReader and FastBufferWriter, replacing with byte* allocated directly through UnsafeUtility.Malloc() (same function used under the hood by NativeArray). This is required in order to be able to store pointers to FastBufferReader and FastBufferWriter in order to be able to wrap them with BufferSerializer - NativeArray contains a managed variable in it that disallows taking a pointer to it. And since FBW and FBR are structs, pointers are the only way to wrap them "by reference" in another struct - ref fields aren't allowed even inside ref structs. * Added utility ref struct "Ref<T>" to more generically support wrapping values in other ref structs in a capture-by-reference style, updated BitReader and BitWriter to use that. Also aggressively inlining properties in FBW and FBR. * BufferSerializer and tests. * Removed unnecessary comment. * XMLDocs + cleanup for PR * Replaced possibly unaligned memory access with UnsafeUtility.MemCpy... it's a little slower, but apparently some platforms won't support unaligned memory access (e.g., WEBGL, ARM processors) and there's no compile time way to detect ARM processors since the bytecode is not processor-dependent... the cost of the runtime detection would be more expensive than the cost of just doing the memcpy. * Resurrected BytewiseUtil.FastCopyBytes as a faster alternative to UnsafeUtility.MemCpy for small values, while still supporting unaligned access. * Reverting an accidental change. * Removed files that got accidentally duplicated from before the rename. * Standards fixes * Removed accidentally added files. * Added BuildInfo.json to the .gitignore so I stop accidentally checking it in. * Addressed most of the review feedback. Still need to do a little more restructuring of some of the other tests. * standards.py --fix * standards.py --fix * Fixed incorrect namespaces. * -Fixed a couple of issues where growing a FastBufferWriter wouldn't work correctly (requesting beyond MaxCapacity and requesting more than double current capacity) -Added support for FastBufferReader to be used in a mode that doesn't copy the input buffer * Fix a test failure and better implementation of large growths * - Removed RefArray - Fixed incorrect text in a warning in FastBufferReader * First INetworkMessage WIP - not hooked up to anything yet. * Killed DelayUtil. * Hooked up MessagingSystem and converted ConnectionRequestMessage and ConnectionApprovedMessage. A couple of tests are known to be failing... this is to let the work start being divided up. * More converted messages * Finished converting all messages over and removed all the now-dead code. Several tests are failing... fixing them will be the next checkin. * Removed IMessageHandler as it's no longer needed after DelayUtil went away. * Fixed tests. * standards.py --fix * Corrected some incorrect xmldocs. * -Added some tests for serializing RPC parameters using FastBufferReader/FastBufferWriter extension methods -Fixed invalid IL code generated when using an extension method to serialize an array. * Fixed missing send queue information for the server on StartClient(). * Added ILPP compile-time check for INetworkMessage Receive() function. * Missed the meta file. * Changed generic function lookup in ILPP to use Cecil instead of relying on system types. * Changed scene management delivery type to ReliableFragmentedSequenced to support larger messages. * Fixed DontDestroyWithOwner throwing an exception on disconnect, fixed memory leak if the server forcibly disconnects a client * - Removed DynamicUnmanagedArray in favor of NativeList - Changed ulong[] to IEnumerable<ulong> in ClientRpcParams * standards.py --fix, plus some adjustments based on over-the-zoom-shoulder review with Fatih * Not sure how standards.py --fix missed this the first time. * increasing timeout in hopes it fixes test failure. * Restored ClientConnected on the ServerClientId in StartClient... seems we can't rely on the server connect event to come in before messages get sent to it? * Cherrypick from fast buffer reader/writer branch to fix accidentally reverted stuff. * Applied much review feedback. * Fixed snapshot stuff and also an outdated comment. * More feedback. * Fix standards check. * Fixed an edge case where the temp serialize buffer could be too large to copy into the main one. * Fixed metrics tests. * standards.py --fix * Renamed IBufferSerializerImplementation to IReaderWriter * Ok... actually renamed IBufferSerializerImplementation to IReaderWriter... * Missed a meta file * Fixed missing OnAfterSendMessage hook when sending to localhost. * Take 3 at a rename. * standards.py --fix
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.
Refactor to implement a zero-alloc flow for sending and receiving messages.
A couple of previous comments exist on #1121 - I changed the branch name and that closed the PR.