Skip to content

Commit 654e413

Browse files
NoelStephensUnityShadauxCat
NoelStephensUnity
and
ShadauxCat
authored
fix: shutdown and shutdown notification issues [MTT-7791] [MTT-7540] (#2789)
* fix Just return from OnClientDisconnectFromServer when the server is shutting down as there is no point in doing ownership cleanup or client disconnected notification messages at that point. * Fix Approaching this fix a different way since we still should send out client disconnect notifications and such. * fix Server or host was not sending disconnect when it shutdown. * update When a client disconnects itself or is disconnected by the server, it now returns its client identifier. Server or host shutdown sequence now includes a "soft shutdown" where the server-host will send out disconnection messages to all clients with the reason for the client being disconnected. * update the test project exit button script is now a bit more intelligent with how it handles exiting and now logs the reason for disconnection. * fix - testproject fixing exceptions when certain missing properties are not set (primarily buttons or toggles) * test updating tests for changes. * fix Don't allow a host or server to disconnect its local client. * update Adding changelog entries * style removing unused namespace. * fix Fixes issue with NetworkVariable and NetworkList throwing exceptions upon shutdown. * Test Validates that setting NetworkVariables or NetworkList values during OnNetworkDespawn does not throw an exception. * update adding change log entry * style fixing warning message about setting network variable/list values during a shutdown. * test Removing unrequired null check * style remove the cr/lf after some else if statments * Minor formatting changes. * update Adjusting warning message language. Adjusting when the send queue is processed. * update reverting back moving the MessageManager?.ProcessSendQueues(); within Shutdown and removing the migration of that out of OnClientDisconnectFromServer. --------- Co-authored-by: Kitty Draper <kitty.draper@unity3d.com>
1 parent fda32b4 commit 654e413

File tree

12 files changed

+299
-47
lines changed

12 files changed

+299
-47
lines changed

‎com.unity.netcode.gameobjects/CHANGELOG.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,22 @@ Additional documentation and release notes are available at [Multiplayer Documen
2121

2222
### Fixed
2323

24+
- Fixed issue where a client disconnected by a server-host would not receive a local notification. (#2789)
25+
- Fixed issue where a server-host could shutdown during a relay connection but periodically the transport disconnect message sent to any connected clients could be dropped. (#2789)
26+
- Fixed issue where a host could disconnect its local client but remain running as a server. (#2789)
27+
- Fixed issue where `OnClientDisconnectedCallback` was not being invoked under certain conditions. (#2789)
28+
- Fixed issue where `OnClientDisconnectedCallback` was always returning 0 as the client identifier. (#2789)
29+
- Fixed issue where if a host or server shutdown while a client owned NetworkObjects (other than the player) it would throw an exception. (#2789)
30+
- Fixed issue where setting values on a `NetworkVariable` or `NetworkList` within `OnNetworkDespawn` during a shutdown sequence would throw an exception. (#2789)
2431
- Fixed issue where a teleport state could potentially be overridden by a previous unreliable delta state. (#2777)
2532
- Fixed issue where `NetworkTransform` was using the `NetworkManager.ServerTime.Tick` as opposed to `NetworkManager.NetworkTickSystem.ServerTime.Tick` during the authoritative side's tick update where it performed a delta state check. (#2777)
2633
- Fixed issue where a parented in-scene placed NetworkObject would be destroyed upon a client or server exiting a network session but not unloading the original scene in which the NetworkObject was placed. (#2737)
2734
- Fixed issue where during client synchronization and scene loading, when client synchronization or the scene loading mode are set to `LoadSceneMode.Single`, a `CreateObjectMessage` could be received, processed, and the resultant spawned `NetworkObject` could be instantiated in the client's currently active scene that could, towards the end of the client synchronization or loading process, be unloaded and cause the newly created `NetworkObject` to be destroyed (and throw and exception). (#2735)
2835
- Fixed issue where a `NetworkTransform` instance with interpolation enabled would result in wide visual motion gaps (stuttering) under above normal latency conditions and a 1-5% or higher packet are drop rate. (#2713)
2936

3037
### Changed
31-
38+
- Changed the server or host shutdown so it will now perform a "soft shutdown" when `NetworkManager.Shutdown` is invoked. This will send a disconnect notification to all connected clients and the server-host will wait for all connected clients to disconnect or timeout after a 5 second period before completing the shutdown process. (#2789)
39+
- Changed `OnClientDisconnectedCallback` will now return the assigned client identifier on the local client side if the client was approved and assigned one prior to being disconnected. (#2789)
3240
- Changed `NetworkTransform.SetState` (and related methods) now are cumulative during a fractional tick period and sent on the next pending tick. (#2777)
3341
- `NetworkManager.ConnectedClientsIds` is now accessible on the client side and will contain the list of all clients in the session, including the host client if the server is operating in host mode (#2762)
3442
- Changed `NetworkSceneManager` to return a `SceneEventProgress` status and not throw exceptions for methods invoked when scene management is disabled and when a client attempts to access a `NetworkSceneManager` method by a client. (#2735)

‎com.unity.netcode.gameobjects/Components/NetworkAnimator.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -833,8 +833,7 @@ private void CheckForStateChange(int layer)
833833
stateChangeDetected = true;
834834
//Debug.Log($"[Cross-Fade] To-Hash: {nt.fullPathHash} | TI-Duration: ({tt.duration}) | TI-Norm: ({tt.normalizedTime}) | From-Hash: ({m_AnimationHash[layer]}) | SI-FPHash: ({st.fullPathHash}) | SI-Norm: ({st.normalizedTime})");
835835
}
836-
else
837-
if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer])
836+
else if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer])
838837
{
839838
// first time in this transition for this layer
840839
m_TransitionHash[layer] = tt.fullPathHash;
@@ -1053,8 +1052,7 @@ private unsafe void WriteParameters(ref FastBufferWriter writer)
10531052
BytePacker.WriteValuePacked(writer, valueBool);
10541053
}
10551054
}
1056-
else
1057-
if (cacheValue.Type == AnimationParamEnumWrapper.AnimatorControllerParameterFloat)
1055+
else if (cacheValue.Type == AnimationParamEnumWrapper.AnimatorControllerParameterFloat)
10581056
{
10591057
var valueFloat = m_Animator.GetFloat(hash);
10601058
fixed (void* value = cacheValue.Value)

‎com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ internal ulong TransportIdCleanUp(ulong transportId)
347347
// When this happens, the client will not have an entry within the m_TransportIdToClientIdMap or m_ClientIdToTransportIdMap lookup tables so we exit early and just return 0 to be used for the disconnect event.
348348
if (!LocalClient.IsServer && !TransportIdToClientIdMap.ContainsKey(transportId))
349349
{
350-
return 0;
350+
return NetworkManager.LocalClientId;
351351
}
352352

353353
var clientId = TransportIdToClientId(transportId);
@@ -476,12 +476,18 @@ internal void DisconnectEventHandler(ulong transportClientId)
476476
s_TransportDisconnect.Begin();
477477
#endif
478478
var clientId = TransportIdCleanUp(transportClientId);
479-
480479
if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
481480
{
482481
NetworkLog.LogInfo($"Disconnect Event From {clientId}");
483482
}
484483

484+
// If we are a client and we have gotten the ServerClientId back, then use our assigned local id as the client that was
485+
// disconnected (either the user disconnected or the server disconnected, but the client that disconnected is the LocalClientId)
486+
if (!NetworkManager.IsServer && clientId == NetworkManager.ServerClientId)
487+
{
488+
clientId = NetworkManager.LocalClientId;
489+
}
490+
485491
// Process the incoming message queue so that we get everything from the server disconnecting us or, if we are the server, so we got everything from that client.
486492
MessageManager.ProcessIncomingMessageQueue();
487493

@@ -496,9 +502,12 @@ internal void DisconnectEventHandler(ulong transportClientId)
496502
{
497503
OnClientDisconnectFromServer(clientId);
498504
}
499-
else
505+
else // As long as we are not in the middle of a shutdown
506+
if (!NetworkManager.ShutdownInProgress)
500507
{
501-
// We must pass true here and not process any sends messages as we are no longer connected and thus there is no one to send any messages to and this will cause an exception within UnityTransport as the client ID is no longer valid.
508+
// We must pass true here and not process any sends messages as we are no longer connected.
509+
// Otherwise, attempting to process messages here can cause an exception within UnityTransport
510+
// as the client ID is no longer valid.
502511
NetworkManager.Shutdown(true);
503512
}
504513
#if DEVELOPMENT_BUILD || UNITY_EDITOR
@@ -901,6 +910,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
901910

902911
// If we are shutting down and this is the server or host disconnecting, then ignore
903912
// clean up as everything that needs to be destroyed will be during shutdown.
913+
904914
if (NetworkManager.ShutdownInProgress && clientId == NetworkManager.ServerClientId)
905915
{
906916
return;
@@ -924,7 +934,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
924934
NetworkManager.SpawnManager.DespawnObject(playerObject, true);
925935
}
926936
}
927-
else
937+
else if (!NetworkManager.ShutdownInProgress)
928938
{
929939
playerObject.RemoveOwnership();
930940
}
@@ -943,7 +953,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
943953
}
944954
else
945955
{
946-
// Handle changing ownership and prefab handlers
956+
// Handle changing ownership and prefab handlers
947957
for (int i = clientOwnedObjects.Count - 1; i >= 0; i--)
948958
{
949959
var ownedObject = clientOwnedObjects[i];
@@ -960,7 +970,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
960970
Object.Destroy(ownedObject.gameObject);
961971
}
962972
}
963-
else
973+
else if (!NetworkManager.ShutdownInProgress)
964974
{
965975
ownedObject.RemoveOwnership();
966976
}
@@ -982,7 +992,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
982992

983993
ConnectedClientIds.Remove(clientId);
984994
var message = new ClientDisconnectedMessage { ClientId = clientId };
985-
NetworkManager.MessageManager.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ConnectedClientIds);
995+
MessageManager?.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ConnectedClientIds);
986996
}
987997

988998
// If the client ID transport map exists
@@ -1033,6 +1043,12 @@ internal void DisconnectClient(ulong clientId, string reason = null)
10331043
throw new NotServerException($"Only server can disconnect remote clients. Please use `{nameof(Shutdown)}()` instead.");
10341044
}
10351045

1046+
if (clientId == NetworkManager.ServerClientId)
1047+
{
1048+
Debug.LogWarning($"Disconnecting the local server-host client is not allowed. Use NetworkManager.Shutdown instead.");
1049+
return;
1050+
}
1051+
10361052
if (!string.IsNullOrEmpty(reason))
10371053
{
10381054
var disconnectReason = new DisconnectReasonMessage
@@ -1077,16 +1093,8 @@ internal void Initialize(NetworkManager networkManager)
10771093
/// </summary>
10781094
internal void Shutdown()
10791095
{
1080-
LocalClient.IsApproved = false;
1081-
LocalClient.IsConnected = false;
1082-
ConnectedClients.Clear();
1083-
ConnectedClientIds.Clear();
1084-
ConnectedClientsList.Clear();
10851096
if (LocalClient.IsServer)
10861097
{
1087-
// make sure all messages are flushed before transport disconnect clients
1088-
MessageManager?.ProcessSendQueues();
1089-
10901098
// Build a list of all client ids to be disconnected
10911099
var disconnectedIds = new HashSet<ulong>();
10921100

@@ -1122,9 +1130,15 @@ internal void Shutdown()
11221130
{
11231131
DisconnectRemoteClient(clientId);
11241132
}
1133+
1134+
// make sure all messages are flushed before transport disconnects clients
1135+
MessageManager?.ProcessSendQueues();
11251136
}
11261137
else if (NetworkManager != null && NetworkManager.IsListening && LocalClient.IsClient)
11271138
{
1139+
// make sure all messages are flushed before disconnecting
1140+
MessageManager?.ProcessSendQueues();
1141+
11281142
// Client only, send disconnect and if transport throws and exception, log the exception and continue the shutdown sequence (or forever be shutting down)
11291143
try
11301144
{
@@ -1136,6 +1150,12 @@ internal void Shutdown()
11361150
}
11371151
}
11381152

1153+
LocalClient.IsApproved = false;
1154+
LocalClient.IsConnected = false;
1155+
ConnectedClients.Clear();
1156+
ConnectedClientIds.Clear();
1157+
ConnectedClientsList.Clear();
1158+
11391159
if (NetworkManager != null && NetworkManager.NetworkConfig?.NetworkTransport != null)
11401160
{
11411161
NetworkManager.NetworkConfig.NetworkTransport.OnTransportEvent -= HandleNetworkEvent;

‎com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs‎

Lines changed: 83 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,89 @@ public void NetworkUpdate(NetworkUpdateStage updateStage)
7373

7474
if (m_ShuttingDown)
7575
{
76-
ShutdownInternal();
76+
// Host-server will disconnect any connected clients prior to finalizing its shutdown
77+
if (IsServer)
78+
{
79+
ProcessServerShutdown();
80+
}
81+
else
82+
{
83+
// Clients just disconnect immediately
84+
ShutdownInternal();
85+
}
7786
}
7887
}
7988
break;
8089
}
8190
}
8291

92+
/// <summary>
93+
/// Used to provide a graceful shutdown sequence
94+
/// </summary>
95+
internal enum ServerShutdownStates
96+
{
97+
None,
98+
WaitForClientDisconnects,
99+
InternalShutdown,
100+
ShuttingDown,
101+
};
102+
103+
internal ServerShutdownStates ServerShutdownState;
104+
private float m_ShutdownTimeout;
105+
106+
/// <summary>
107+
/// This is a "soft shutdown" where the host or server will disconnect
108+
/// all clients, with a provided reasons, prior to invoking its final
109+
/// internal shutdown.
110+
/// </summary>
111+
internal void ProcessServerShutdown()
112+
{
113+
var minClientCount = IsHost ? 2 : 1;
114+
switch (ServerShutdownState)
115+
{
116+
case ServerShutdownStates.None:
117+
{
118+
if (ConnectedClients.Count >= minClientCount)
119+
{
120+
var hostServer = IsHost ? "host" : "server";
121+
var disconnectReason = $"Disconnected due to {hostServer} shutting down.";
122+
for (int i = ConnectedClientsIds.Count - 1; i >= 0; i--)
123+
{
124+
var clientId = ConnectedClientsIds[i];
125+
if (clientId == ServerClientId)
126+
{
127+
continue;
128+
}
129+
ConnectionManager.DisconnectClient(clientId, disconnectReason);
130+
}
131+
ServerShutdownState = ServerShutdownStates.WaitForClientDisconnects;
132+
m_ShutdownTimeout = Time.realtimeSinceStartup + 5.0f;
133+
}
134+
else
135+
{
136+
ServerShutdownState = ServerShutdownStates.InternalShutdown;
137+
ProcessServerShutdown();
138+
}
139+
break;
140+
}
141+
case ServerShutdownStates.WaitForClientDisconnects:
142+
{
143+
if (ConnectedClients.Count < minClientCount || m_ShutdownTimeout < Time.realtimeSinceStartup)
144+
{
145+
ServerShutdownState = ServerShutdownStates.InternalShutdown;
146+
ProcessServerShutdown();
147+
}
148+
break;
149+
}
150+
case ServerShutdownStates.InternalShutdown:
151+
{
152+
ServerShutdownState = ServerShutdownStates.ShuttingDown;
153+
ShutdownInternal();
154+
break;
155+
}
156+
}
157+
}
158+
83159
/// <summary>
84160
/// The client id used to represent the server
85161
/// </summary>
@@ -704,6 +780,12 @@ public int MaximumFragmentedMessageSize
704780

705781
internal void Initialize(bool server)
706782
{
783+
// Make sure the ServerShutdownState is reset when initializing
784+
if (server)
785+
{
786+
ServerShutdownState = ServerShutdownStates.None;
787+
}
788+
707789
// Don't allow the user to start a network session if the NetworkManager is
708790
// still parented under another GameObject
709791
if (NetworkManagerCheckForParent(true))
@@ -970,7 +1052,6 @@ public bool StartHost()
9701052
private void HostServerInitialize()
9711053
{
9721054
LocalClientId = ServerClientId;
973-
//ConnectionManager.ConnectedClientIds.Add(ServerClientId);
9741055
NetworkMetrics.SetConnectionId(LocalClientId);
9751056
MessageManager.SetLocalClientId(LocalClientId);
9761057

@@ -1051,11 +1132,6 @@ public void Shutdown(bool discardMessageQueue = false)
10511132
MessageManager.StopProcessing = discardMessageQueue;
10521133
}
10531134
}
1054-
1055-
if (NetworkConfig != null && NetworkConfig.NetworkTransport != null)
1056-
{
1057-
NetworkConfig.NetworkTransport.OnTransportEvent -= ConnectionManager.HandleNetworkEvent;
1058-
}
10591135
}
10601136

10611137
// Ensures that the NetworkManager is cleaned up before OnDestroy is run on NetworkObjects and NetworkBehaviours when unloading a scene with a NetworkManager

‎com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,15 @@ protected void MarkNetworkBehaviourDirty()
100100
"Are you modifying a NetworkVariable before the NetworkObject is spawned?");
101101
return;
102102
}
103-
103+
if (m_NetworkBehaviour.NetworkManager.ShutdownInProgress)
104+
{
105+
if (m_NetworkBehaviour.NetworkManager.LogLevel <= LogLevel.Developer)
106+
{
107+
Debug.LogWarning($"NetworkVariable is written to during the NetworkManager shutdown! " +
108+
"Are you modifying a NetworkVariable within a NetworkBehaviour.OnDestroy or NetworkBehaviour.OnDespawn method?");
109+
}
110+
return;
111+
}
104112
m_NetworkBehaviour.NetworkManager.BehaviourUpdater.AddForUpdate(m_NetworkBehaviour.NetworkObject);
105113
}
106114

0 commit comments

Comments
 (0)