search

Unity-Technologies / com.unity.netcode.gameobjects

Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.
MIT License
2.13k stars 430 forks source link

New Approval Func<ConnectionApprovalRequest, ConnectionApprovalResponse> restricts approval to main thread #1992

Closed JayPeet closed 2 years ago

JayPeet commented 2 years ago

Feedback The new changes to the NetworkManager.ConnectionApprovalCallback seems to restrict the usage of Approval to anything main thread related (since its waiting in the function to return the ConnectionApprovalResponse). The documentation gives an example of doing a Steam API ticket check on client connection, which would be a web request that could take seconds to complete. In my current code, I do a web request to the authentication API to check if the client is authenticated, which now requires me to lock the main thread, whereas before I would just call the callback later once my web request had finished.

Suggested Changes Not sure what to suggest as this change was intentionally made by the NGO team. However it seems like the Approval callback is now useless for doing any actual authentication / approval since it seems to block the main thread

ashwinimurt commented 2 years ago

Thanks for reporting. We are working on some refactor on Connection Approval logic and will take this into consideration. After the refactor, it will be possible to use any thread to perform approval.

ashwinimurt commented 2 years ago

Will be addressed by MTT-2567

jeffreyrainy commented 2 years ago

Hi JayPeet,

Thanks, this is the kind of great feedback we need. We were a bit hasty in the changes. https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/2002 would address the concerns you raised, I believe.

Please let me know if you think otherwise.

Thanks!

JayPeet commented 2 years ago

Much better, thank you for taking the time to make these changes!