Skip to content

fix: preventing issues with game code registering multiple ConnectionApprovalCallbacks [MTT-3496] #1972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 17, 2022
Merged

fix: preventing issues with game code registering multiple ConnectionApprovalCallbacks [MTT-3496] #1972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5e1330b
fix: preventing issues with game code registering multiple Connection…
jeffreyrainy May 16, 2022
3a935c0
fix: preventing issues with game code registering multiple Connection…
jeffreyrainy May 16, 2022
8d70bdb
fix: preventing issues with game code registering multiple Connection…
jeffreyrainy May 17, 2022
a2e2593
some refactorings
0xFA11 May 17, 2022
22a10f0
update changelog
0xFA11 May 17, 2022
92b5fc9
standards fix
0xFA11 May 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Changed

- (API Breaking) `ConnectionApprovalCallback` is no longer an `event` and will not allow more than 1 handler registered at a time. Also, `ConnectionApprovalCallback` is now a `Func<>` taking `ConnectionApprovalRequest` in and returning `ConnectionApprovalResponse` back out (#1972)

### Removed

### Fixed

- Fixed issues when multiple `ConnectionApprovalCallback`s were registered (#1972)
- Fixed endless dialog boxes when adding a NetworkBehaviour to a NetworkManager or vice-versa (#1947)

## [1.0.0-pre.9] - 2022-05-10
Expand Down
127 changes: 86 additions & 41 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,14 @@ public GameObject GetNetworkPrefabOverride(GameObject gameObject)
/// <summary>
/// Gets or sets if the application should be set to run in background
/// </summary>
[HideInInspector] public bool RunInBackground = true;
[HideInInspector]
public bool RunInBackground = true;

/// <summary>
/// The log level to use
/// </summary>
[HideInInspector] public LogLevel LogLevel = LogLevel.Normal;
[HideInInspector]
public LogLevel LogLevel = LogLevel.Normal;

/// <summary>
/// The singleton instance of the NetworkManager
Expand Down Expand Up @@ -358,26 +360,59 @@ public IReadOnlyList<ulong> ConnectedClientsIds
public event Action OnServerStarted = null;

/// <summary>
/// Delegate type called when connection has been approved. This only has to be set on the server.
/// Connection Approval Response
/// </summary>
/// <param name="createPlayerObject">If true, a player object will be created. Otherwise the client will have no object.</param>
/// <param name="playerPrefabHash">The prefabHash to use for the client. If createPlayerObject is false, this is ignored. If playerPrefabHash is null, the default player prefab is used.</param>
/// <param name="approved">Whether or not the client was approved</param>
/// <param name="position">The position to spawn the client at. If null, the prefab position is used.</param>
/// <param name="rotation">The rotation to spawn the client with. If null, the prefab position is used.</param>
public delegate void ConnectionApprovedDelegate(bool createPlayerObject, uint? playerPrefabHash, bool approved, Vector3? position, Quaternion? rotation);
/// <param name="Approved">Whether or not the client was approved</param>
/// <param name="CreatePlayerObject">If true, a player object will be created. Otherwise the client will have no object.</param>
/// <param name="PlayerPrefabHash">The prefabHash to use for the client. If createPlayerObject is false, this is ignored. If playerPrefabHash is null, the default player prefab is used.</param>
/// <param name="Position">The position to spawn the client at. If null, the prefab position is used.</param>
/// <param name="Rotation">The rotation to spawn the client with. If null, the prefab position is used.</param>
public struct ConnectionApprovalResponse
{
public bool Approved;
public bool CreatePlayerObject;
public uint? PlayerPrefabHash;
public Vector3? Position;
public Quaternion? Rotation;
}

/// <summary>
/// Connection Approval Request
/// </summary>
/// <param name="Payload">The connection data payload</param>
/// <param name="ClientNetworkId">The Network Id of the client we are about to handle</param>
public struct ConnectionApprovalRequest
{
public byte[] Payload;
public ulong ClientNetworkId;
}

/// <summary>
/// The callback to invoke during connection approval
/// The callback to invoke during connection approval. Allows client code to decide whether or not to allow incoming client connection
/// </summary>
public event Action<byte[], ulong, ConnectionApprovedDelegate> ConnectionApprovalCallback = null;
public Func<ConnectionApprovalRequest, ConnectionApprovalResponse> ConnectionApprovalCallback
{
get => m_ConnectionApprovalCallback;
set
{
if (value != null && value.GetInvocationList().Length > 1)
{
throw new InvalidOperationException($"Only one {nameof(ConnectionApprovalCallback)} can be registered at a time.");
}
else
{
m_ConnectionApprovalCallback = value;
}
}
}

internal void InvokeConnectionApproval(byte[] payload, ulong clientId, ConnectionApprovedDelegate action) => ConnectionApprovalCallback?.Invoke(payload, clientId, action);
private Func<ConnectionApprovalRequest, ConnectionApprovalResponse> m_ConnectionApprovalCallback;

/// <summary>
/// The current NetworkConfig
/// </summary>
[HideInInspector] public NetworkConfig NetworkConfig;
[HideInInspector]
public NetworkConfig NetworkConfig;

/// <summary>
/// The current host name we are connected to, used to validate certificate
Expand Down Expand Up @@ -970,27 +1005,28 @@ public bool StartHost()
IsClient = true;
IsListening = true;

if (NetworkConfig.ConnectionApproval)
if (NetworkConfig.ConnectionApproval && ConnectionApprovalCallback != null)
{
InvokeConnectionApproval(NetworkConfig.ConnectionData, ServerClientId,
(createPlayerObject, playerPrefabHash, approved, position, rotation) =>
var response = ConnectionApprovalCallback(new ConnectionApprovalRequest { Payload = NetworkConfig.ConnectionData, ClientNetworkId = ServerClientId });
if (!response.Approved)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
{
// You cannot decline the local server. Force approved to true
if (!approved)
{
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
{
NetworkLog.LogWarning(
"You cannot decline the host connection. The connection was automatically approved.");
}
}
NetworkLog.LogWarning("You cannot decline the host connection. The connection was automatically approved.");
}
}

HandleApproval(ServerClientId, createPlayerObject, playerPrefabHash, true, position, rotation);
});
response.Approved = true;
HandleConnectionApproval(ServerClientId, response);
}
else
{
HandleApproval(ServerClientId, NetworkConfig.PlayerPrefab != null, null, true, null, null);
var response = new ConnectionApprovalResponse
{
Approved = true,
CreatePlayerObject = NetworkConfig.PlayerPrefab != null
};
HandleConnectionApproval(ServerClientId, response);
}

SpawnManager.ServerSpawnSceneObjectsOnStartSweep();
Expand Down Expand Up @@ -1818,15 +1854,11 @@ private void SyncTime()
/// <summary>
/// Server Side: Handles the approval of a client
/// </summary>
/// <param name="ownerClientId">client being approved</param>
/// <param name="createPlayerObject">whether we want to create a player or not</param>
/// <param name="playerPrefabHash">the GlobalObjectIdHash value for the Network Prefab to create as the player</param>
/// <param name="approved">Is the player approved or not?</param>
/// <param name="position">Used when createPlayerObject is true, position of the player when spawned </param>
/// <param name="rotation">Used when createPlayerObject is true, rotation of the player when spawned</param>
internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint? playerPrefabHash, bool approved, Vector3? position, Quaternion? rotation)
/// <param name="ownerClientId">The Network Id of the client being approved</param>
/// <param name="response">The response to allow the player in or not, with its parameters</param>
internal void HandleConnectionApproval(ulong ownerClientId, ConnectionApprovalResponse response)
{
if (approved)
if (response.Approved)
{
// Inform new client it got approved
PendingClients.Remove(ownerClientId);
Expand All @@ -1836,10 +1868,23 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint?
m_ConnectedClientsList.Add(client);
m_ConnectedClientIds.Add(client.ClientId);

if (createPlayerObject)
if (response.CreatePlayerObject)
{
var networkObject = SpawnManager.CreateLocalNetworkObject(false, playerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, null, null, position, rotation);
SpawnManager.SpawnNetworkObjectLocally(networkObject, SpawnManager.GetNetworkObjectId(), false, true, ownerClientId, false);
var networkObject = SpawnManager.CreateLocalNetworkObject(
isSceneObject: false,
response.PlayerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash,
ownerClientId,
parentNetworkId: null,
networkSceneHandle: null,
response.Position,
response.Rotation);
SpawnManager.SpawnNetworkObjectLocally(
networkObject,
SpawnManager.GetNetworkObjectId(),
sceneObject: false,
playerObject: true,
ownerClientId,
destroyWithScene: false);

ConnectedClients[ownerClientId].PlayerObject = networkObject;
}
Expand Down Expand Up @@ -1879,13 +1924,13 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint?
InvokeOnClientConnectedCallback(ownerClientId);
}

if (!createPlayerObject || (playerPrefabHash == null && NetworkConfig.PlayerPrefab == null))
if (!response.CreatePlayerObject || (response.PlayerPrefabHash == null && NetworkConfig.PlayerPrefab == null))
{
return;
}

// Separating this into a contained function call for potential further future separation of when this notification is sent.
ApprovedPlayerSpawn(ownerClientId, playerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash);
ApprovedPlayerSpawn(ownerClientId, response.PlayerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash);
}
else
{
Expand Down
16 changes: 11 additions & 5 deletions com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionRequestMessage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,22 @@ public void Handle(ref NetworkContext context)
{
// Note: Delegate creation allocates.
// Note: ToArray() also allocates. :(
networkManager.InvokeConnectionApproval(ConnectionData, senderId,
(createPlayerObject, playerPrefabHash, approved, position, rotation) =>
var response = networkManager.ConnectionApprovalCallback(
new NetworkManager.ConnectionApprovalRequest
{
var localCreatePlayerObject = createPlayerObject;
networkManager.HandleApproval(senderId, localCreatePlayerObject, playerPrefabHash, approved, position, rotation);
Payload = ConnectionData,
ClientNetworkId = senderId
});
networkManager.HandleConnectionApproval(senderId, response);
}
else
{
networkManager.HandleApproval(senderId, networkManager.NetworkConfig.PlayerPrefab != null, null, true, null, null);
var response = new NetworkManager.ConnectionApprovalResponse
{
Approved = true,
CreatePlayerObject = networkManager.NetworkConfig.PlayerPrefab != null
};
networkManager.HandleConnectionApproval(senderId, response);
}
}
}
Expand Down
16 changes: 12 additions & 4 deletions com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApproval.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public void Setup()
[UnityTest]
public IEnumerator ConnectionApproval()
{
NetworkManagerHelper.NetworkManagerObject.ConnectionApprovalCallback += NetworkManagerObject_ConnectionApprovalCallback;
NetworkManagerHelper.NetworkManagerObject.ConnectionApprovalCallback = NetworkManagerObject_ConnectionApprovalCallback;
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.ConnectionApproval = true;
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.PlayerPrefab = null;
NetworkManagerHelper.NetworkManagerObject.NetworkConfig.ConnectionData = Encoding.UTF8.GetBytes(m_ValidationToken.ToString());
Expand All @@ -47,14 +47,22 @@ public IEnumerator ConnectionApproval()
Assert.True(m_IsValidated);
}

private void NetworkManagerObject_ConnectionApprovalCallback(byte[] connectionData, ulong clientId, NetworkManager.ConnectionApprovedDelegate callback)
private NetworkManager.ConnectionApprovalResponse NetworkManagerObject_ConnectionApprovalCallback(NetworkManager.ConnectionApprovalRequest request)
{
var stringGuid = Encoding.UTF8.GetString(connectionData);
var response = new NetworkManager.ConnectionApprovalResponse();
var stringGuid = Encoding.UTF8.GetString(request.Payload);
if (m_ValidationToken.ToString() == stringGuid)
{
m_IsValidated = true;
}
callback(false, null, m_IsValidated, null, null);

response.Approved = m_IsValidated;
response.CreatePlayerObject = false;
response.Position = null;
response.Rotation = null;
response.PlayerPrefabHash = null;

return response;
}

[TearDown]
Expand Down
33 changes: 21 additions & 12 deletions testproject/Assets/Tests/Manual/Scripts/ConnectionApprovalComponent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private void Start()

if (NetworkManager != null && NetworkManager.NetworkConfig.ConnectionApproval)
{
NetworkManager.ConnectionApprovalCallback += ConnectionApprovalCallback;
NetworkManager.ConnectionApprovalCallback = ConnectionApprovalCallback;

if (m_ApprovalToken != string.Empty)
{
Expand Down Expand Up @@ -151,41 +151,50 @@ public void OnDisconnectClient()
/// <summary>
/// Invoked only on the server, this will handle the various connection approval combinations
/// </summary>
/// <param name="dataToken">key or password to get approval</param>
/// <param name="clientId">client identifier being approved</param>
/// <param name="aprovalCallback">callback that should be invoked once it is determined if client is approved or not</param>
private void ConnectionApprovalCallback(byte[] dataToken, ulong clientId, NetworkManager.ConnectionApprovedDelegate aprovalCallback)
/// <param name="request">The connection approval request</param>
/// <returns>ConnectionApprovalResult with the approval decision, with parameters</returns>
private NetworkManager.ConnectionApprovalResponse ConnectionApprovalCallback(NetworkManager.ConnectionApprovalRequest request)
{
string approvalToken = Encoding.ASCII.GetString(dataToken);
var response = new NetworkManager.ConnectionApprovalResponse();
string approvalToken = Encoding.ASCII.GetString(request.Payload);
var isTokenValid = approvalToken == m_ApprovalToken;
if (m_SimulateFailure && m_SimulateFailure.isOn && IsServer && clientId != NetworkManager.LocalClientId)
if (m_SimulateFailure && m_SimulateFailure.isOn && IsServer && request.ClientNetworkId != NetworkManager.LocalClientId)
{
isTokenValid = false;
}

if (m_GlobalObjectIdHashOverride != 0 && m_PlayerPrefabOverride && m_PlayerPrefabOverride.isOn)
{
aprovalCallback.Invoke(true, m_GlobalObjectIdHashOverride, isTokenValid, null, null);
response.Approved = isTokenValid;
response.PlayerPrefabHash = m_GlobalObjectIdHashOverride;
response.Position = null;
response.Rotation = null;
response.CreatePlayerObject = true;
}
else
{
aprovalCallback.Invoke(true, null, isTokenValid, null, null);
response.Approved = isTokenValid;
response.PlayerPrefabHash = null;
response.Position = null;
response.Rotation = null;
response.CreatePlayerObject = true;
}


if (m_ConnectionMessageToDisplay)
{
if (isTokenValid)
{
AddNewMessage($"Client id {clientId} is authorized!");
AddNewMessage($"Client id {request.ClientNetworkId} is authorized!");
}
else
{
AddNewMessage($"Client id {clientId} failed authorization!");
AddNewMessage($"Client id {request.ClientNetworkId} failed authorization!");
}

m_ConnectionMessageToDisplay.gameObject.SetActive(true);
}

return response;
}

/// <summary>
Expand Down
Loading