Skip to content

fix: Wrong MTU calculations in UnityTransport#2731

Merged
ShadauxCat merged 14 commits intorelease/1.7.0from
fix/mtu-calculations-utp
Oct 10, 2023
Merged

fix: Wrong MTU calculations in UnityTransport#2731
ShadauxCat merged 14 commits intorelease/1.7.0from
fix/mtu-calculations-utp

Conversation

@simon-lemay-unity
Copy link
Contributor

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 DataStreamWriter in SendBatchedMessages. But that's wrong. The DataStreamWriter capacity 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 the DataStreamWriter'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 BatchedSendQueue to 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

  • Includes unit test.
  • No documentation changes or additions were necessary.

NoelStephensUnity and others added 12 commits October 2, 2023 14:41
…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.
@simon-lemay-unity simon-lemay-unity changed the base branch from develop to release/1.7.0 October 5, 2023 20:06
@ShadauxCat ShadauxCat merged commit 702875d into release/1.7.0 Oct 10, 2023
@ShadauxCat ShadauxCat deleted the fix/mtu-calculations-utp branch October 10, 2023 19:28
ShadauxCat added a commit that referenced this pull request Oct 26, 2023
* chore: merge develop into release 1.7.0 (#2729)

* fix: Wrong MTU calculations in UnityTransport (#2731)

* chore: Tagging release 1.7.0 (#2736)

---------

Co-authored-by: Simon Lemay <simon.lemay@unity3d.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants