Skip to content

Commit eaf54a1

Browse files
EmandMEmandM
committed
further cleanup
1 parent a658445 commit eaf54a1

File tree

11 files changed

+70
-94
lines changed

11 files changed

+70
-94
lines changed

‎com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2229,14 +2229,6 @@ internal void SynchronizeObjectsToNewlyJoinedClient(ulong newClientId)
22292229
{
22302230
if (networkObject.Observers.Contains(newClientId))
22312231
{
2232-
// if (NetworkManager.LogLevel <= LogLevel.Developer)
2233-
// {
2234-
// // Temporary tracking to make sure we are not showing something already visibile (should never be the case for this)
2235-
// Debug.LogWarning($"[{nameof(SynchronizeObjectsToNewlyJoinedClient)}][{networkObject.name}] New client as already an observer!");
2236-
// }
2237-
// // For now, remove the client (impossible for the new client to have an instance since the session owner doesn't) to make sure newly added
2238-
// // code to handle this edge case works.
2239-
// networkObject.Observers.Remove(newClientId);
22402232
continue;
22412233
}
22422234
networkObject.NetworkShow(newClientId);

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs‎

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,16 @@ public static void DeregisterNetworkObject(ulong localClientId, ulong networkObj
103103

104104
protected const uint k_DefaultTickRate = 30;
105105

106-
private int m_NumberOfClients;
106+
/// <summary>
107+
/// Specifies the number of client instances to be created for the integration test.
108+
/// </summary>
109+
/// <remarks>
110+
/// When running with a hosted CMB Service, NumberOfClients + 1 clients are created.
111+
/// This holds assumptions throughout the test suite that when running with a host, there is an extra client.
112+
/// For example, see the calculation for <see cref="TotalClients"/>.
113+
/// </remarks>
107114
protected abstract int NumberOfClients { get; }
115+
private int m_NumberOfClients;
108116

109117
/// <summary>
110118
/// Set this to false to create the clients first.
@@ -131,11 +139,11 @@ public enum HostOrServer
131139

132140
protected GameObject m_PlayerPrefab;
133141

134-
/// <summary>The Server <see cref="NetworkManager"/> instance</summary>
142+
/// <summary>The Server <see cref="NetworkManager"/> instance instantiated and tracked within the current test</summary>
135143
protected NetworkManager m_ServerNetworkManager;
136-
/// <summary>All the client <see cref="NetworkManager"/> instances</summary>
144+
/// <summary>All the client <see cref="NetworkManager"/> instances instantiated and tracked within the current test</summary>
137145
protected NetworkManager[] m_ClientNetworkManagers;
138-
/// <summary>All the <see cref="NetworkManager"/> instances</summary>
146+
/// <summary>All the <see cref="NetworkManager"/> instances instantiated and tracked within the current test</summary>
139147
protected NetworkManager[] m_NetworkManagers;
140148

141149
/// <summary>
@@ -595,6 +603,17 @@ protected IEnumerator CreateAndStartNewClient()
595603
// in the event any modifications need to be made before starting the client
596604
OnNewClientCreated(networkManager);
597605

606+
yield return StartClient(networkManager);
607+
}
608+
609+
/// <summary>
610+
/// Starts and connects the given networkManager as a client while in the middle of an
611+
/// integration test.
612+
/// </summary>
613+
/// <param name="networkManager">The network manager to start and connect</param>
614+
/// <returns>An IEnumerator to be used in a coroutine for asynchronous execution.</returns>
615+
protected IEnumerator StartClient(NetworkManager networkManager)
616+
{
598617
NetcodeIntegrationTestHelpers.StartOneClient(networkManager);
599618

600619
if (LogAllMessages)
@@ -706,21 +725,33 @@ protected void CreateAndStartNewClientWithTimeTravel()
706725
}
707726

708727
/// <summary>
709-
/// This will stop a client while in the middle of an integration test
728+
/// This will stop the given <see cref="NetworkManager"/> instance while in the middle of an integration test.
729+
/// The instance is then removed from the lists of managed instances (<see cref="m_NetworkManagers"/>, <see cref="m_ClientNetworkManagers"/>).
710730
/// </summary>
731+
/// <remarks>
732+
/// If there are no other references to the managed instance, it will be destroyed regardless of the destroy parameter.
733+
/// To avoid this, save a reference to the <see cref="NetworkManager"/> instance before calling this method.
734+
/// </remarks>
735+
/// <param name="networkManager">The <see cref="NetworkManager"/> instance of the client to stop.</param>
736+
/// <param name="destroy">Whether the <see cref="NetworkManager"/> instance should be destroyed after stopping. Defaults to false.</param>
737+
/// <returns>An <see cref="IEnumerator"/> to be used in a coroutine for asynchronous execution.</returns>
711738
protected IEnumerator StopOneClient(NetworkManager networkManager, bool destroy = false)
712739
{
713740
NetcodeIntegrationTestHelpers.StopOneClient(networkManager, destroy);
714-
if (destroy)
715-
{
716-
AddRemoveNetworkManager(networkManager, false);
717-
}
741+
AddRemoveNetworkManager(networkManager, false);
718742
yield return WaitForConditionOrTimeOut(() => !networkManager.IsConnectedClient);
719743
}
720744

721745
/// <summary>
722-
/// This will stop a client while in the middle of an integration test
746+
/// This will stop the given <see cref="NetworkManager"/> instance while in the middle of a time travel integration test.
747+
/// The instance is then removed from the lists of managed instances (<see cref="m_NetworkManagers"/>, <see cref="m_ClientNetworkManagers"/>).
723748
/// </summary>
749+
/// <remarks>
750+
/// If there are no other references to the managed instance, it will be destroyed regardless of the destroy parameter.
751+
/// To avoid this, save a reference to the <see cref="NetworkManager"/> instance before calling this method.
752+
/// </remarks>
753+
/// <param name="networkManager">The <see cref="NetworkManager"/> instance of the client to stop.</param>
754+
/// <param name="destroy">Whether the <see cref="NetworkManager"/> instance should be destroyed after stopping. Defaults to false.</param>
724755
protected void StopOneClientWithTimeTravel(NetworkManager networkManager, bool destroy = false)
725756
{
726757
NetcodeIntegrationTestHelpers.StopOneClient(networkManager, destroy);

‎com.unity.netcode.gameobjects/Tests/Runtime/DisconnectTests.cs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ private bool DoesServerStillHaveSpawnedPlayerObject()
139139
[UnityTest]
140140
public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType clientDisconnectType)
141141
{
142-
m_ClientId = m_ClientNetworkManagers[0].LocalClientId;
142+
var clientNetworkManager = m_ClientNetworkManagers[0];
143+
m_ClientId = clientNetworkManager.LocalClientId;
143144
m_ClientDisconnectType = clientDisconnectType;
144145

145146
var serverSideClientPlayer = m_ServerNetworkManager.ConnectionManager.ConnectedClients[m_ClientId].PlayerObject;
@@ -148,18 +149,18 @@ public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType client
148149

149150
if (clientDisconnectType == ClientDisconnectType.ServerDisconnectsClient)
150151
{
151-
m_ClientNetworkManagers[0].OnClientDisconnectCallback += OnClientDisconnectCallback;
152-
m_ClientNetworkManagers[0].OnConnectionEvent += OnConnectionEvent;
152+
clientNetworkManager.OnClientDisconnectCallback += OnClientDisconnectCallback;
153+
clientNetworkManager.OnConnectionEvent += OnConnectionEvent;
153154
m_ServerNetworkManager.OnConnectionEvent += OnConnectionEvent;
154155
m_ServerNetworkManager.DisconnectClient(m_ClientId);
155156
}
156157
else
157158
{
158159
m_ServerNetworkManager.OnClientDisconnectCallback += OnClientDisconnectCallback;
159160
m_ServerNetworkManager.OnConnectionEvent += OnConnectionEvent;
160-
m_ClientNetworkManagers[0].OnConnectionEvent += OnConnectionEvent;
161+
clientNetworkManager.OnConnectionEvent += OnConnectionEvent;
161162

162-
yield return StopOneClient(m_ClientNetworkManagers[0]);
163+
yield return StopOneClient(clientNetworkManager);
163164
}
164165

165166
yield return WaitForConditionOrTimeOut(() => m_ClientDisconnected);
@@ -169,17 +170,17 @@ public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType client
169170
{
170171
Assert.IsTrue(m_DisconnectedEvent.ContainsKey(m_ServerNetworkManager), $"Could not find the server {nameof(NetworkManager)} disconnect event entry!");
171172
Assert.IsTrue(m_DisconnectedEvent[m_ServerNetworkManager].ClientId == m_ClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the server {nameof(NetworkManager)} disconnect event entry!");
172-
Assert.IsTrue(m_DisconnectedEvent.ContainsKey(m_ClientNetworkManagers[0]), $"Could not find the client {nameof(NetworkManager)} disconnect event entry!");
173-
Assert.IsTrue(m_DisconnectedEvent[m_ClientNetworkManagers[0]].ClientId == m_ClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the client {nameof(NetworkManager)} disconnect event entry!");
173+
Assert.IsTrue(m_DisconnectedEvent.ContainsKey(clientNetworkManager), $"Could not find the client {nameof(NetworkManager)} disconnect event entry!");
174+
Assert.IsTrue(m_DisconnectedEvent[clientNetworkManager].ClientId == m_ClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the client {nameof(NetworkManager)} disconnect event entry!");
174175
// Unregister for this event otherwise it will be invoked during teardown
175176
m_ServerNetworkManager.OnConnectionEvent -= OnConnectionEvent;
176177
}
177178
else
178179
{
179180
Assert.IsTrue(m_DisconnectedEvent.ContainsKey(m_ServerNetworkManager), $"Could not find the server {nameof(NetworkManager)} disconnect event entry!");
180181
Assert.IsTrue(m_DisconnectedEvent[m_ServerNetworkManager].ClientId == m_ClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the server {nameof(NetworkManager)} disconnect event entry!");
181-
Assert.IsTrue(m_DisconnectedEvent.ContainsKey(m_ClientNetworkManagers[0]), $"Could not find the client {nameof(NetworkManager)} disconnect event entry!");
182-
Assert.IsTrue(m_DisconnectedEvent[m_ClientNetworkManagers[0]].ClientId == m_ClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the client {nameof(NetworkManager)} disconnect event entry!");
182+
Assert.IsTrue(m_DisconnectedEvent.ContainsKey(clientNetworkManager), $"Could not find the client {nameof(NetworkManager)} disconnect event entry!");
183+
Assert.IsTrue(m_DisconnectedEvent[clientNetworkManager].ClientId == m_ClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the client {nameof(NetworkManager)} disconnect event entry!");
183184
Assert.IsTrue(m_ServerNetworkManager.ConnectedClientsIds.Count == 1, $"Expected connected client identifiers count to be 1 but it was {m_ServerNetworkManager.ConnectedClientsIds.Count}!");
184185
Assert.IsTrue(m_ServerNetworkManager.ConnectedClients.Count == 1, $"Expected connected client identifiers count to be 1 but it was {m_ServerNetworkManager.ConnectedClients.Count}!");
185186
Assert.IsTrue(m_ServerNetworkManager.ConnectedClientsList.Count == 1, $"Expected connected client identifiers count to be 1 but it was {m_ServerNetworkManager.ConnectedClientsList.Count}!");

‎com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/DistributeObjectsTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ internal class DistributeObjectsTests : IntegrationTestWithApproximation
2626
private const int k_LateJoinClientCount = 4;
2727
protected override int NumberOfClients => 0;
2828

29-
3029
public DistributeObjectsTests() : base(HostOrServer.DAHost)
3130
{
3231
}

‎com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ParentChildDistibutionTests.cs‎

Lines changed: 16 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88

99
namespace Unity.Netcode.RuntimeTests
1010
{
11-
[TestFixture(DistributionTypes.UponConnect)]
12-
[TestFixture(DistributionTypes.UponDisconnect)]
13-
internal class ParentChildDistibutionTests : IntegrationTestWithApproximation
11+
internal class ParentChildDistributionTests : IntegrationTestWithApproximation
1412
{
1513
protected override int NumberOfClients => 4;
1614

@@ -20,17 +18,15 @@ internal class ParentChildDistibutionTests : IntegrationTestWithApproximation
2018
private List<NetworkObject> m_AltTargetSpawnedObjects = new List<NetworkObject>();
2119
private Dictionary<ulong, List<NetworkObject>> m_AncillarySpawnedObjects = new Dictionary<ulong, List<NetworkObject>>();
2220
private StringBuilder m_ErrorMsg = new StringBuilder();
23-
private DistributionTypes m_DistributionType;
2421

2522
// TODO: [CmbServiceTesting] Update UponDisconnect tests to work with the rust service
2623
protected override bool UseCMBService()
2724
{
2825
return false;
2926
}
3027

31-
public ParentChildDistibutionTests(DistributionTypes distributionType) : base(HostOrServer.DAHost)
28+
public ParentChildDistributionTests() : base(HostOrServer.DAHost)
3229
{
33-
m_DistributionType = distributionType;
3430
}
3531

3632
protected override IEnumerator OnTearDown()
@@ -54,10 +50,6 @@ private bool AllTargetedInstancesSpawned()
5450
m_ErrorMsg.Clear();
5551
foreach (var client in m_NetworkManagers)
5652
{
57-
if (!client.IsConnectedClient)
58-
{
59-
continue;
60-
}
6153
foreach (var spawnedObject in m_TargetSpawnedObjects)
6254
{
6355
if (!client.SpawnManager.SpawnedObjects.ContainsKey(spawnedObject.NetworkObjectId))
@@ -75,10 +67,6 @@ private bool AllAncillaryInstancesSpawned()
7567
m_ErrorMsg.Clear();
7668
foreach (var client in m_NetworkManagers)
7769
{
78-
if (!client.IsConnectedClient)
79-
{
80-
continue;
81-
}
8270
foreach (var clientObjects in m_AncillarySpawnedObjects)
8371
{
8472
foreach (var spawnedObject in clientObjects.Value)
@@ -163,19 +151,20 @@ public enum OwnershipLocking
163151

164152

165153
[UnityTest]
166-
public IEnumerator DistributeOwnerHierarchy([Values] OwnershipLocking ownershipLock)
154+
public IEnumerator DistributeOwnerHierarchy([Values] DistributionTypes distributionType, [Values] OwnershipLocking ownershipLock)
167155
{
168156
m_TargetSpawnedObjects.Clear();
169157
m_AncillarySpawnedObjects.Clear();
170158
m_AltTargetSpawnedObjects.Clear();
171159

172-
if (m_DistributionType == DistributionTypes.UponConnect)
160+
var clientToReconnect = m_ClientNetworkManagers[3];
161+
if (distributionType == DistributionTypes.UponConnect)
173162
{
174-
m_ClientNetworkManagers[3].Shutdown();
163+
yield return StopOneClient(clientToReconnect);
175164
}
176165

177166
// When testing connect redistribution,
178-
var instances = m_DistributionType == DistributionTypes.UponDisconnect ? 1 : 2;
167+
var instances = distributionType == DistributionTypes.UponDisconnect ? 1 : 2;
179168
var rootObject = (GameObject)null;
180169
var childOne = (GameObject)null;
181170
var childTwo = (GameObject)null;
@@ -186,7 +175,7 @@ public IEnumerator DistributeOwnerHierarchy([Values] OwnershipLocking ownershipL
186175
rootObject = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[0]);
187176
networkObject = rootObject.GetComponent<NetworkObject>();
188177
networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable);
189-
if (ownershipLock == OwnershipLocking.LockRootParent && m_DistributionType == DistributionTypes.UponConnect)
178+
if (ownershipLock == OwnershipLocking.LockRootParent && distributionType == DistributionTypes.UponConnect)
190179
{
191180
networkObject.SetOwnershipLock(true);
192181
}
@@ -202,7 +191,7 @@ public IEnumerator DistributeOwnerHierarchy([Values] OwnershipLocking ownershipL
202191
childTwo = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[0]);
203192
networkObject = childTwo.GetComponent<NetworkObject>();
204193
networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable);
205-
if (ownershipLock == OwnershipLocking.LockTargetChild && m_DistributionType == DistributionTypes.UponConnect)
194+
if (ownershipLock == OwnershipLocking.LockTargetChild && distributionType == DistributionTypes.UponConnect)
206195
{
207196
networkObject.SetOwnershipLock(true);
208197
}
@@ -249,7 +238,7 @@ public IEnumerator DistributeOwnerHierarchy([Values] OwnershipLocking ownershipL
249238
// Get the original clientId
250239
m_OriginalOwnerId = m_ClientNetworkManagers[0].LocalClientId;
251240

252-
if (m_DistributionType == DistributionTypes.UponDisconnect)
241+
if (distributionType == DistributionTypes.UponDisconnect)
253242
{
254243
// Swap out the original owner's NetworkObject with one of the other client's since those instances will
255244
// be destroyed when the client disconnects.
@@ -258,13 +247,13 @@ public IEnumerator DistributeOwnerHierarchy([Values] OwnershipLocking ownershipL
258247
m_AltTargetSpawnedObjects.Add(m_ClientNetworkManagers[1].SpawnManager.SpawnedObjects[entry.NetworkObjectId]);
259248
}
260249
// Disconnect the client to trigger object redistribution
261-
m_ClientNetworkManagers[0].Shutdown();
250+
yield return StopOneClient(m_ClientNetworkManagers[0]);
262251
}
263252
else
264253
{
265-
m_ClientNetworkManagers[3].StartClient();
266-
yield return WaitForConditionOrTimeOut(() => m_ClientNetworkManagers[3].IsConnectedClient);
267-
AssertOnTimeout($"{m_ClientNetworkManagers[3].name} failed to reconnect!");
254+
yield return StartClient(clientToReconnect);
255+
yield return WaitForConditionOrTimeOut(() => clientToReconnect.IsConnectedClient);
256+
AssertOnTimeout($"{clientToReconnect.name} failed to reconnect!");
268257
}
269258

270259
// Verify all of the targeted objects changed ownership to the same client
@@ -273,7 +262,7 @@ public IEnumerator DistributeOwnerHierarchy([Values] OwnershipLocking ownershipL
273262

274263
// When enabled, you should see one of the two root instances that have children get distributed to
275264
// the reconnected client.
276-
if (m_EnableVerboseDebug && m_DistributionType == DistributionTypes.UponConnect)
265+
if (m_EnableVerboseDebug && distributionType == DistributionTypes.UponConnect)
277266
{
278267
m_ErrorMsg.Clear();
279268
m_ErrorMsg.AppendLine($"Original targeted objects owner: {m_OriginalOwnerId}");
@@ -285,7 +274,7 @@ public IEnumerator DistributeOwnerHierarchy([Values] OwnershipLocking ownershipL
285274
}
286275

287276
// We only want to make sure no other children owned by still connected clients change ownership
288-
if (m_DistributionType == DistributionTypes.UponDisconnect)
277+
if (distributionType == DistributionTypes.UponDisconnect)
289278
{
290279
// Verify the ancillary objects kept the same ownership
291280
yield return WaitForConditionOrTimeOut(AncillaryObjectsKeptOwnership);

0 commit comments

Comments
 (0)