From 1e5e79d492b535accd7106003fc07d8fed98dc9d Mon Sep 17 00:00:00 2001 From: Jaedyn Draper Date: Tue, 7 Dec 2021 11:22:42 -0600 Subject: [PATCH 1/4] fix: error when serializing ConnectionApprovalMessage with scene management disabled when one or more objects is hidden via the CheckObjectVisibility delegate --- .../Runtime/Core/NetworkManager.cs | 1 - .../Messages/ConnectionApprovedMessage.cs | 20 ++++-- .../Components/NetworkVisibilityComponent.cs | 13 ++++ .../NetworkVisibilityComponent.cs.meta | 11 +++ .../Tests/Runtime/MultiInstanceHelpers.cs | 8 ++- .../Tests/Runtime/NetworkVisibilityTests.cs | 72 +++++++++++++++++++ .../Runtime/NetworkVisibilityTests.cs.meta | 3 + 7 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs.meta create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs.meta diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs index 91fdab0d50..c308ef538b 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs @@ -1655,7 +1655,6 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint? { if (SpawnManager.SpawnedObjectsList.Count != 0) { - message.SceneObjectCount = SpawnManager.SpawnedObjectsList.Count; message.SpawnedObjectsList = SpawnManager.SpawnedObjectsList; } } diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs index 1fd9beec54..68bd584d1a 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs @@ -7,7 +7,6 @@ internal struct ConnectionApprovedMessage : INetworkMessage { public ulong OwnerClientId; public int NetworkTick; - public int SceneObjectCount; // Not serialized, held as references to serialize NetworkVariable data public HashSet SpawnedObjectsList; @@ -23,10 +22,13 @@ public void Serialize(FastBufferWriter writer) } writer.WriteValue(OwnerClientId); writer.WriteValue(NetworkTick); - writer.WriteValue(SceneObjectCount); - if (SceneObjectCount != 0) + uint sceneObjectCount = 0; + if (SpawnedObjectsList != null) { + var pos = writer.Position; + writer.Seek(writer.Position + FastBufferWriter.GetWriteSize(sceneObjectCount)); + // Serialize NetworkVariable data foreach (var sobj in SpawnedObjectsList) { @@ -35,8 +37,16 @@ public void Serialize(FastBufferWriter writer) sobj.Observers.Add(OwnerClientId); var sceneObject = sobj.GetMessageSceneObject(OwnerClientId); sceneObject.Serialize(writer); + ++sceneObjectCount; } } + writer.Seek(pos); + writer.WriteValue(sceneObjectCount); + writer.Seek(writer.Length); + } + else + { + writer.WriteValue(sceneObjectCount); } } @@ -56,7 +66,6 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context) reader.ReadValue(out OwnerClientId); reader.ReadValue(out NetworkTick); - reader.ReadValue(out SceneObjectCount); m_ReceivedSceneObjectData = reader; return true; } @@ -77,10 +86,11 @@ public void Handle(ref NetworkContext context) if (!networkManager.NetworkConfig.EnableSceneManagement) { networkManager.SpawnManager.DestroySceneObjects(); + m_ReceivedSceneObjectData.ReadValue(out uint sceneObjectCount); // Deserializing NetworkVariable data is deferred from Receive() to Handle to avoid needing // to create a list to hold the data. This is a breach of convention for performance reasons. - for (ushort i = 0; i < SceneObjectCount; i++) + for (ushort i = 0; i < sceneObjectCount; i++) { var sceneObject = new NetworkObject.SceneObject(); sceneObject.Deserialize(m_ReceivedSceneObjectData); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs new file mode 100644 index 0000000000..2abf2c8243 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs @@ -0,0 +1,13 @@ +namespace Unity.Netcode.RuntimeTests +{ + public class NetworkVisibilityComponent : NetworkBehaviour + { + public void Hide() + { + GetComponent().CheckObjectVisibility += HandleCheckObjectVisibility; + } + + protected virtual bool HandleCheckObjectVisibility(ulong clientId) => false; + + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs.meta new file mode 100644 index 0000000000..e25fdeca76 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Components/NetworkVisibilityComponent.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 7c84178a212f3cf4a8818bfbcbc92eef +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/MultiInstanceHelpers.cs b/com.unity.netcode.gameobjects/Tests/Runtime/MultiInstanceHelpers.cs index 1067414ce5..01d01ff24a 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/MultiInstanceHelpers.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/MultiInstanceHelpers.cs @@ -230,15 +230,17 @@ private static void SceneManagerValidationAndTestRunnerInitialization(NetworkMan } } + public delegate void BeforeClientStartCallback(); + /// /// Starts NetworkManager instances created by the Create method. /// /// Whether or not to create a Host instead of Server /// The Server NetworkManager /// The Clients NetworkManager - /// called immediately after server and client(s) are started + /// called immediately after server is started and before client(s) are started /// - public static bool Start(bool host, NetworkManager server, NetworkManager[] clients) + public static bool Start(bool host, NetworkManager server, NetworkManager[] clients, BeforeClientStartCallback callback = null) { if (s_IsStarted) { @@ -260,6 +262,8 @@ public static bool Start(bool host, NetworkManager server, NetworkManager[] clie // if set, then invoke this for the server RegisterHandlers(server); + callback?.Invoke(); + if (ClientSceneHandler != null) { throw new Exception("Some how ClientSceneHandler did not get disposed when Destroy was called?"); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs new file mode 100644 index 0000000000..7c4138d33a --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs @@ -0,0 +1,72 @@ +using System.Collections; +using NUnit.Framework; +using UnityEngine; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + public class NetworkVisibilityTests + { + private NetworkObject m_NetSpawnedObject; + private GameObject m_TestNetworkPrefab; + + [TearDown] + public void TearDown() + { + MultiInstanceHelpers.Destroy(); + if (m_TestNetworkPrefab) + { + GameObject.Destroy(m_TestNetworkPrefab); + m_TestNetworkPrefab = null; + } + } + + [UnityTest] + public IEnumerator HiddenObjectsTest() + { + + const int numClients = 1; + Assert.True(MultiInstanceHelpers.Create(numClients, out NetworkManager server, out NetworkManager[] clients)); + m_TestNetworkPrefab = new GameObject("Object"); + var networkObject = m_TestNetworkPrefab.AddComponent(); + m_TestNetworkPrefab.AddComponent(); + + // Make it a prefab + MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObject); + + var validNetworkPrefab = new NetworkPrefab(); + validNetworkPrefab.Prefab = m_TestNetworkPrefab; + server.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab); + server.NetworkConfig.EnableSceneManagement = false; + foreach (var client in clients) + { + client.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab); + client.NetworkConfig.EnableSceneManagement = false; + } + + // Start the instances + if (!MultiInstanceHelpers.Start(true, server, clients, () => + { + var serverObject = Object.Instantiate(m_TestNetworkPrefab, Vector3.zero, Quaternion.identity); + NetworkObject serverNetworkObject = serverObject.GetComponent(); + serverNetworkObject.NetworkManagerOwner = server; + serverNetworkObject.Spawn(); + serverObject.GetComponent().Hide(); + })) + { + Debug.LogError("Failed to start instances"); + Assert.Fail("Failed to start instances"); + } + + // [Client-Side] Wait for a connection to the server + yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForClientsConnected(clients, null, 512)); + + // [Host-Side] Check to make sure all clients are connected + yield return MultiInstanceHelpers.Run( + MultiInstanceHelpers.WaitForClientsConnectedToServer(server, clients.Length + 1, null, 512)); + + Assert.AreEqual(2, GameObject.FindObjectsOfType().Length); + } + } +} + diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs.meta new file mode 100644 index 0000000000..aa1302e108 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: e053d627444648908b32a1bc22778d37 +timeCreated: 1638402116 \ No newline at end of file From 332761665fcb906a7e711daa632662b6b5c87881 Mon Sep 17 00:00:00 2001 From: Jaedyn Draper Date: Tue, 7 Dec 2021 11:25:02 -0600 Subject: [PATCH 2/4] Changelog --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 400b6ab707..8b8be95828 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -29,6 +29,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed NetworkObjects not being despawned before they are destroyed during shutdown for client, host, and server instances. (#1390) - Fixed client player object being destroyed on server when the client's player object has DontDestroyWithOwner set. (#1433) - Fixed: NetworkVariables containing more than 1300 bytes of data (such as large NetworkLists) no longer cause an OverflowException (the limit on data size is now whatever limit the chosen transport imposes on fragmented NetworkDelivery mechanisms) (#1481) +- Fixed: Fixed: Fixed error when serializing ConnectionApprovalMessage with scene management disabled when one or more objects is hidden via the CheckObjectVisibility delegate (#1509) ### Changed From aeb747d926681981a3778025cef8bb381f60d871 Mon Sep 17 00:00:00 2001 From: Jaedyn Draper Date: Tue, 7 Dec 2021 11:42:36 -0600 Subject: [PATCH 3/4] standards --- .../Tests/Runtime/NetworkVisibilityTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs index 7c4138d33a..a69fb7ec60 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVisibilityTests.cs @@ -16,7 +16,7 @@ public void TearDown() MultiInstanceHelpers.Destroy(); if (m_TestNetworkPrefab) { - GameObject.Destroy(m_TestNetworkPrefab); + Object.Destroy(m_TestNetworkPrefab); m_TestNetworkPrefab = null; } } @@ -65,7 +65,7 @@ public IEnumerator HiddenObjectsTest() yield return MultiInstanceHelpers.Run( MultiInstanceHelpers.WaitForClientsConnectedToServer(server, clients.Length + 1, null, 512)); - Assert.AreEqual(2, GameObject.FindObjectsOfType().Length); + Assert.AreEqual(2, Object.FindObjectsOfType().Length); } } } From b8feaef220282904ab6a5ddc9a718b367b3066c2 Mon Sep 17 00:00:00 2001 From: Jaedyn Draper Date: Tue, 7 Dec 2021 13:00:57 -0600 Subject: [PATCH 4/4] Fixed the "Fixed: Fixed: Fixed", fixing the "fixed" count so it's fixed at 2 fixeds. --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 8b8be95828..a40d3297df 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -29,7 +29,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed NetworkObjects not being despawned before they are destroyed during shutdown for client, host, and server instances. (#1390) - Fixed client player object being destroyed on server when the client's player object has DontDestroyWithOwner set. (#1433) - Fixed: NetworkVariables containing more than 1300 bytes of data (such as large NetworkLists) no longer cause an OverflowException (the limit on data size is now whatever limit the chosen transport imposes on fragmented NetworkDelivery mechanisms) (#1481) -- Fixed: Fixed: Fixed error when serializing ConnectionApprovalMessage with scene management disabled when one or more objects is hidden via the CheckObjectVisibility delegate (#1509) +- Fixed: Fixed error when serializing ConnectionApprovalMessage with scene management disabled when one or more objects is hidden via the CheckObjectVisibility delegate (#1509) ### Changed