search

thirdweb-dev / unity-sdk

Best in class Web3 Unity SDK, powered by thirdweb.
https://portal.thirdweb.com/unity
Apache License 2.0
131 stars 73 forks source link

Don't use SynchronizationContext.Current to detect whether sdk are in the main thread. #215

Closed BoysheO closed 2 months ago

BoysheO commented 2 months ago

See file WalletConnect.cs line 72. SDK version=v5.2.1 In fact, our client modifies Unity's default SynchronizationContext to achieve better performance, which is a very common practice in commercial games. Therefore, do not use SynchronizationContext to determine whether the current logic is running on the main thread. A more mature approach is to retrieve the current thread's ID in the Awake function of a MonoBehaviour, and use this to check if other asynchronous contexts are on the main thread.

BoysheO commented 2 months ago

I have also briefly reviewed your code, and it seems that SynchronizationContext is a core part of the SDK, rather than just being used to determine whether the current thread is the Unity thread. Therefore, I hope that your SDK can provide an interface that allows me to add my custom SynchronizationContext to a whitelist, so that the error can be ignored.

BoysheO commented 2 months ago

Or do you need any pull requests?I can make it well.

0xFirekeeper commented 2 months ago

Hey @BoysheO - definitely reasonable to use thread id instead. Outside of WalletConnect, I don't believe our SDK makes use of SynchronizationContext. We do mess with it in WebGL but I don't think this is relevant to this scenario.

I can speak to the WalletConnect Unity team and have them make that change to their core sdk, subsequently adding it to our sdk.

Feel free to make a PR regardless

BoysheO commented 2 months ago

see https://github.com/thirdweb-dev/unity-sdk/pull/216.Keep all things simple.