Lachee / discord-rpc-csharp

C# custom implementation for Discord Rich Presence. Not deprecated and still available!
MIT License
560 stars 94 forks source link

[FR] Readme section for UnityEngine API calls for thread-aware concerns? #138

Closed Kein closed 3 years ago

Kein commented 3 years ago

Does the lib or any of its dependencies use any of the Unity's API for its functionality? It is not clear from the readme, the Invoking part does not mention any of the relevant info. If the lib does not require any Unity API to function I'd rather run it on a separate thread from the get-go and just feed the volatile data through Update loop manually to avoid any possible bottleneck or performance issues.

Lachee commented 3 years ago

The library uses a separate thread to handle the IO operations between discord and your application as to avoid blocking the main thread. The events are put into a queue and are invoked with the Invoke

The issue arises with the default behaviour of the library:

This results in the events being invoked from the IO thread. This is fine for most applications and is how other libraries generally handle events.

However, this means if you are trying to say, update a TextMeshPro or a GameObject's texture to match the result of an event (like onReady), you will get a cross thread issue from the default behaviour of the library since the events were emitted from the IO thread automatically.

The solution is simple:

  1. Handle thread safety yourself with locks and other shenanigans to safely copy the data across.
  2. Disable the automatic invocation of events, and call Invoke yourself on the main thread.

Solution B is what is described in the documentation for Unity. You are letting the library queue the events and then manually dequing those events using the Invoke().

Its important to note that since the events are queued, there is no blocking concerned when invoking them and it has been designed so that dequing the events is a thread safe operation.

TL:DR

Events are emitted on the thread that calls Invoke(). By default this is the IO Thread. The documentation shows how to invoke it on Unity's main thread instead.

Kein commented 3 years ago

I see, appreciate the clear up! I looked at the code some more and I'm a bit concerned with excessive locks, I guess they make sense for full RPC support like joining and lobbies, but for Rich presence I want, a simple swap-buffer like instance cache + volatile member would be more than enough to update it completely on a separate thread with 0 issues.

Lachee commented 3 years ago

The locks are not too much of a concern and don't really impact performance. The reality of it is that you really only are only locking when you are attempting to dequeue events and when you receive events. Events are rarely received, so its only the dequeuing that might be concern, but since events are rare there is not much chance of it actually blocking.

If you actually don't care about events from Discord at all, you can disable event queues by setting DiscordRpcClient.MaxQueueSize to 0, then just dont do any Invoke() calls.

Games like OSU use this library and they have not reported any significant issues with performance. For the most use cases, its negligible, but if you have issues let me know and I can see what can be done 😄