Skip to content

chore: allowing multiple Connection Approval handlers. [MTT-3496] #1964

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

Closed
jeffreyrainy wants to merge 1 commit into from
Closed

chore: allowing multiple Connection Approval handlers. [MTT-3496] #1964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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,6 +12,9 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Changed

- [breaking] Allow multiple Connection Approval handlers.
`public delegate void ConnectionApprovalDelegate(byte[] payload, ulong clientId, ConnectionApprovalDecision decision);` now receives a `ConnectionApprovalDecision` class that must be filled, instead of calling a second callback. This allows having multiple Connection Approval handlers. (#1941)

### Removed

### Fixed
Expand Down
66 changes: 40 additions & 26 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 @@ -358,21 +358,32 @@ 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 Decision
/// </summary>
/// <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="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);
public class ConnectionApprovalDecision
{
public bool Approved = false;
public bool CreatePlayerObject = false;
public uint? PlayerPrefabHash = null;
public Vector3? Position = null;
public Quaternion? Rotation = null;
};

/// <summary>
/// The callback to invoke during connection approval
/// Delegate type invoke during connection approval.
/// </summary>
public event Action<byte[], ulong, ConnectionApprovedDelegate> ConnectionApprovalCallback = null;
/// <param name="decision">The decision the delegates reach.</param>
public delegate void ConnectionApprovalDelegate(byte[] payload, ulong clientId, ConnectionApprovalDecision decision);

internal void InvokeConnectionApproval(byte[] payload, ulong clientId, ConnectionApprovedDelegate action) => ConnectionApprovalCallback?.Invoke(payload, clientId, action);
/// <summary>
/// The property to set with ConnectionApprovalDelegate if connection approval is to be performed.
/// </summary>
public ConnectionApprovalDelegate ConnectionApprovalCallback;

/// <summary>
/// The current NetworkConfig
Expand Down Expand Up @@ -970,27 +981,30 @@ public bool StartHost()
IsClient = true;
IsListening = true;

var decision = new ConnectionApprovalDecision();

if (NetworkConfig.ConnectionApproval)
{
InvokeConnectionApproval(NetworkConfig.ConnectionData, ServerClientId,
(createPlayerObject, playerPrefabHash, approved, position, rotation) =>
ConnectionApprovalCallback?.Invoke(NetworkConfig.ConnectionData, ServerClientId, decision);

// You cannot decline the local server. Force approved to true
if (!decision.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);
});
decision.Approved = true;
HandleApproval(ServerClientId, decision);
}
else
{
HandleApproval(ServerClientId, NetworkConfig.PlayerPrefab != null, null, true, null, null);
decision.Approved = true;
decision.CreatePlayerObject = NetworkConfig.PlayerPrefab != null;
HandleApproval(ServerClientId, decision);
}

SpawnManager.ServerSpawnSceneObjectsOnStartSweep();
Expand Down Expand Up @@ -1824,9 +1838,9 @@ private void SyncTime()
/// <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)
internal void HandleApproval(ulong ownerClientId, ConnectionApprovalDecision decision)
{
if (approved)
if (decision.Approved)
{
// Inform new client it got approved
PendingClients.Remove(ownerClientId);
Expand All @@ -1836,9 +1850,9 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint?
m_ConnectedClientsList.Add(client);
m_ConnectedClientIds.Add(client.ClientId);

if (createPlayerObject)
if (decision.CreatePlayerObject)
{
var networkObject = SpawnManager.CreateLocalNetworkObject(false, playerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, null, null, position, rotation);
var networkObject = SpawnManager.CreateLocalNetworkObject(false, decision.PlayerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, null, null, decision.Position, decision.Rotation);
SpawnManager.SpawnNetworkObjectLocally(networkObject, SpawnManager.GetNetworkObjectId(), false, true, ownerClientId, false);

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

if (!createPlayerObject || (playerPrefabHash == null && NetworkConfig.PlayerPrefab == null))
if (!decision.CreatePlayerObject || (decision.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, decision.PlayerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash);
}
else
{
Expand Down
15 changes: 8 additions & 7 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 @@ -97,20 +97,21 @@ public void Handle(ref NetworkContext context)
client.ConnectionState = PendingClient.State.PendingApproval;
}

var decision = new NetworkManager.ConnectionApprovalDecision();

if (networkManager.NetworkConfig.ConnectionApproval)
{
// Note: Delegate creation allocates.
// Note: ToArray() also allocates. :(
networkManager.InvokeConnectionApproval(ConnectionData, senderId,
(createPlayerObject, playerPrefabHash, approved, position, rotation) =>
{
var localCreatePlayerObject = createPlayerObject;
networkManager.HandleApproval(senderId, localCreatePlayerObject, playerPrefabHash, approved, position, rotation);
});
networkManager.ConnectionApprovalCallback?.Invoke(ConnectionData, senderId, decision);

networkManager.HandleApproval(senderId, decision);
}
else
{
networkManager.HandleApproval(senderId, networkManager.NetworkConfig.PlayerPrefab != null, null, true, null, null);
decision.Approved = true;
decision.CreatePlayerObject = networkManager.NetworkConfig.PlayerPrefab != null;
networkManager.HandleApproval(senderId, decision);
}
}
}
Expand Down
6 changes: 4 additions & 2 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 @@ -47,14 +47,16 @@ public IEnumerator ConnectionApproval()
Assert.True(m_IsValidated);
}

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

decision.CreatePlayerObject = false;
decision.Approved = m_IsValidated;
}

[TearDown]
Expand Down
11 changes: 7 additions & 4 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 @@ -154,7 +154,7 @@ public void OnDisconnectClient()
/// <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)
private void ConnectionApprovalCallback(byte[] dataToken, ulong clientId, NetworkManager.ConnectionApprovalDecision decision)
{
string approvalToken = Encoding.ASCII.GetString(dataToken);
var isTokenValid = approvalToken == m_ApprovalToken;
Expand All @@ -165,14 +165,17 @@ private void ConnectionApprovalCallback(byte[] dataToken, ulong clientId, Networ

if (m_GlobalObjectIdHashOverride != 0 && m_PlayerPrefabOverride && m_PlayerPrefabOverride.isOn)
{
aprovalCallback.Invoke(true, m_GlobalObjectIdHashOverride, isTokenValid, null, null);
decision.Approved = isTokenValid;
Copy link
Copy Markdown
Contributor Author

@jeffreyrainy jeffreyrainy May 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those three lines should be the first focus. The API changed in this way: instead of asking the client code to call a callback, they fill a class with their decision. All the approval handler get to run before the decision is used. Then, the decision is used just once.

decision.PlayerPrefabHash = m_GlobalObjectIdHashOverride;
decision.CreatePlayerObject = true;
}
else
{
aprovalCallback.Invoke(true, null, isTokenValid, null, null);
decision.Approved = isTokenValid;
decision.PlayerPrefabHash = null;
decision.CreatePlayerObject = true;
}


if (m_ConnectionMessageToDisplay)
{
if (isTokenValid)
Expand Down
9 changes: 6 additions & 3 deletions testproject/Assets/Tests/Runtime/MultiClientConnectionApproval.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ private IEnumerator ConnectionApprovalHandler(int numClients, int failureTestCou
/// <param name="connectionData">the NetworkConfig.ConnectionData sent from the client being approved</param>
/// <param name="clientId">the client id being approved</param>
/// <param name="callback">the callback invoked to handle approval</param>
private void ConnectionApprovalCallback(byte[] connectionData, ulong clientId, NetworkManager.ConnectionApprovedDelegate callback)
private void ConnectionApprovalCallback(byte[] connectionData, ulong clientId, NetworkManager.ConnectionApprovalDecision decision)
{
string approvalToken = Encoding.ASCII.GetString(connectionData);
var isApproved = approvalToken == m_ConnectionToken;
Expand All @@ -189,11 +189,14 @@ private void ConnectionApprovalCallback(byte[] connectionData, ulong clientId, N

if (m_PrefabOverrideGlobalObjectIdHash == 0)
{
callback.Invoke(true, null, isApproved, null, null);
decision.CreatePlayerObject = true;
decision.Approved = isApproved;
}
else
{
callback.Invoke(true, m_PrefabOverrideGlobalObjectIdHash, isApproved, null, null);
decision.CreatePlayerObject = true;
decision.Approved = isApproved;
decision.PlayerPrefabHash = m_PrefabOverrideGlobalObjectIdHash;
}
}

Expand Down