fix: Wrong MTU calculations in UnityTransport#2731
Merged
ShadauxCat merged 14 commits intorelease/1.7.0from Oct 10, 2023
Merged
fix: Wrong MTU calculations in UnityTransport#2731ShadauxCat merged 14 commits intorelease/1.7.0from
ShadauxCat merged 14 commits intorelease/1.7.0from
Conversation
…ed prefab instances [MTT-7055] (#2707) * update Resolves MTT-7055. Split apart the NetworkObjectRefreshTool from NetworkObject. Made some updates that don't require any form of editor application update. Added script in NetworkObject.RefreshAllPrefabInstances context menu method that handles refreshing the currently active scene and all enabled scenes in the build list. Added dialog notification when attempting to do a NetworkObject Refresh on an in-scene placed prefab instance as opposed to a prefab instance. * test Made some minor updates for the manual test * update adding change log entry * style Added object type identifier constants for code clarity purposes.
…GenerateSerializationForTypeAttribute` (#2694) This enables users to control the generation of serialization code through codegen, making it easier to create their own network variable subtypes, and also making it possible for them to easily generate serialization for specific types they know they need to use. NetworkVariable and NetworkList have been changed to use these attributes so they are no longer special cases in our codegen. This PR also exposes methods in `NetworkVariableSerialization<T>` to further support this type of serialization need. resolves #2686 --------- Co-authored-by: Noel Stephens <noel.stephens@unity3d.com>
…hronization issues [MTT-7271] (#2712) * update Adding OnOwnershipChanged(ulong previous, ulong current) * fix This resolves the issue with client authoritative network transforms and the random "noise" that would occur when transitioning ownership from a remote client back to the host-server. - Latent messages from the client would still be received and processed after ownership changed. - Ownership changed messages would proceed the NetworkTransform initialization state update message. Now ownership changed messages precede the NetworkTransform initialization state update message. - Clients could sometimes have the same network tick value even when the tick event had triggered, which for NetworkDeltaPosition would cause dropped state updates.
* update Removing the invocation of process scene from the OnValidate. * update Just merging GenerateGlobalObjectIdHash into OnValidate (no reason to have the method call a method).
…h domain reload/scene reload being disabled (#2720) Adds support for generic network behaviours, as well as for serializing generic types in RPCs (so long as the type is defined on the behaviour and not on the RPC itself; which is to say, this is supported: ```csharp public class Foo<T> : NetworkBehaviour { [ServerRpc] public void MyServerRpc(T val) {} } ``` But this is not: ```csharp public class Foo : NetworkBehaviour { [ServerRpc] public void MyServerRpc<T>(T val) {} } ``` As in the former case, when the class is instantiated, it knows what T is and there is only one version of the RPC per class, while the latter case would require much more significant plumbing for runtime type identification of RPC parameters in order to be able to instantiate the RPC correctly on the receiving side.) In fixing this, the majority of the issues with domain reload were also fixed coincidentally, so I went ahead and fixed the couple of remaining issues as well.
Co-authored-by: Noel Stephens <noel.stephens@unity3d.com>
Also fixes NetworkVariables with NonSerializedAttribute showing in editor when they should not.
…scenes loaded [MTT-7537] (#2723) * fix This fixes the issue where the currently active scene, on the server side, is not taken into consideration as being a valid "loaded scene" which can cause issues as outlined in GitHub issue #2722. * fix: TestHelpers This resolves an issue with integration testing where: - the scene handler registration was being registered multiple times - the server registration was not passing in true to NetcodeIntegrationTestHelper.RegisterHandlers - the IntegrationTestSceneHandler.CoroutineRunner could get destroyed if the active scene it was instantiated within was unloaded (now it is migrated to the DDOL) - Registration of the currently active scene during the scene handler registration was adjusted to no longer use NetworkSceneManager.GetAndAddNewlyLoadedSceneByName (but still registers the scene). * test Added an integration test: `NetworkSceneManagerFixValidationTests.InitialActiveSceneUnload` This validates the fix for #2722. * update Adding change log entry for this fix.
ShadauxCat
approved these changes
Oct 5, 2023
chrispope
approved these changes
Oct 9, 2023
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
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.
This PR fixes an issue I introduced in my previous change to add support for peer MTUs in
UnityTransport.My change would use the peer MTU plus the transport headers as the maximum amount of data that can be written to a
DataStreamWriterinSendBatchedMessages. But that's wrong. TheDataStreamWritercapacity already accounts for transport headers. That was not an issue when testing with direct IP connections, because the default peer MTU and transport headers are small enough that this miscalculation still ends up with a value that's less than theDataStreamWriter's capacity. But if using Unity Relay (which requires much larger headers), then it was enough to cause the value to be higher than the writer's capacity. We'd then (attempt to) write more than possible to the writer, which ended up generating an error.I also added an extra protection in
BatchedSendQueueto never ever attempt to write more than the writer's capacity, no matter what the peer MTU says. That should prevent the same issue from happening (Relay or not) if the user increases a peer's MTU.Changelog
N/A
Testing and Documentation