fix: preventing issues with game code registering multiple ConnectionApprovalCallbacks [MTT-3496]
#1972
Conversation
…ApprovalCallback. Refactored the entire connection approval
| { | ||
| if (value != null && value.GetInvocationList().Length > 1) | ||
| { | ||
| Debug.LogError( |
There was a problem hiding this comment.
throw an InvalidOperationException instead?
There was a problem hiding this comment.
s/instead/additionally ?
There was a problem hiding this comment.
an unhandled exception is eventually an error/exception log in Unity console anyway
There was a problem hiding this comment.
What I mean to say is that throw (new InvalidOperationException()); will be less informational than the log. See updated code. But ok, no big deal, I can remove the log too.
There was a problem hiding this comment.
what about throw new InvalidOperationException($"Only one connection approval handler is supported in {nameof(m_ConnectionApprovalCallback)}. Rejecting further adds."); ? :)
There was a problem hiding this comment.
Ok, good, we should be good to go now. Please let me know if I missed any bits or parts.
com.unity.netcode.gameobjects/Tests/Runtime/ConnectionApproval.cs
Outdated
Show resolved
Hide resolved
testproject/Assets/Tests/Manual/Scripts/ConnectionApprovalComponent.cs
Outdated
Show resolved
Hide resolved
…ApprovalCallback. Refactored the entire connection approval [code review]
…ApprovalCallback. Refactored the entire connection approval [code review 2]
0xFA11
changed the title
ConnectionApprovalCallbacks [MTT-3496]
fix: preventing issues with game code registering multiple ConnectionApprovalCallback. Refactored the entire connection approval
Changelog
[breaking]
ConnectionApprovalCallbackis nowConnectionApprovalHandler. Receives aConnectionApprovalRequest, and returns aConnectionApprovalResult. By virtue of being aFunc<>, there cannot be multiple handlers registered.Fixed issues when multiple Connection Approval callbacks were registered (Exception when there are multiple connectional approval callbacks #1941)