Setting UpdatePresenceWhenStopped is ignored and Presence shows impossible times

[Svarr]

Describe the bug
Under certain conditions the Rich Presence is displayed in Discord even though UpdatePresenceWhenStopped is turned off and nothing is currently played back. The presence shows information about the last track that was played.

To Reproduce
I'm not entirely certain about which conditions can cause this bug to appear. Here is one way to reproduce it (that is, how this bug happened to me today):

1. Start MusicBee and play some music (don't run Discord yet, UpdatePresenceWhenStopped should be disabled)
2. Do some housework or waste time in whatever way you prefer, and let the playlist run out (maybe a certain time of being afk is required)
3. Eventually return to the PC (playback has already run out) and start Discord
4. Admire the Rich Presence showing MusicBee and the last song that has been played, while still counting up the playtime (way above the track's duration)

Expected behavior
Nothing shown in Discord except my "About Me" text and other stuff that is usually shown when clicking on my name in the userlist.

Screenshots

(In case you didn't know, that song is far shorter than 3h. 😉 )

Environment (please complete the following information):

• MusicBee Version: 3.4.7805
• Plugin Version: 1.4.2

Additional context
I think the key is to start Discord after MusicBee, while playback is halted but something had been played previously in the same MusicBee session (so that UpdateDiscordPresence had been called with PlayState.Playing at least once).

It may be worth to look into what DiscordRpcClient does on initialisation when no Discord client is currently running and then later, when Discord is run.

[Svarr]

Ok, I did some digging and discord-rpc-csharp looks really sus to me now.

As it turns out, when DiscordRpcClient.Initialize() is called, the return value of RpcConnection.AttemptConnection() is assigned to DiscordRpcClient.IsInitialized. The problem is, that AttemptConnection() just starts a thread and immediately returns true. The only way a value of false is assigned, is when the Thread is already running, or when the RpcConnection is either aborting or not in Disconnected state. That means that DiscordRpcClient.IsInitialized == true does in no way guarantee that a connection to Discord is established.

The Method that runs in the thread started by AttemptConnection() keeps polling until a connection is established. Until then every command that is to be sent to Discord (like setting the presence) is stored in a queue while the oldest entry is dequeued if it has reached its maximum capacity (_maxRtQueueSize). This may have implications that I'm currently not aware of, but unless DiscordBee sends an old presence after the clearing triggered by MusicBee reaching the end of the playlist, the latest command (which should set the presence to null) should be received by Discord last. I don't know with which frequence the commands in the out queue are dispatched, maybe Discord drops some of them?

But even if that's the case, DiscordBee shouldn't try to send anything unless a connection to discord is established. So I guess the main goal should be to find out how the connection state can be retreived or at least deduced.

[sll552]

Thanks for your effort. Without having looked into it much I could think of using the Ready Event to check for initialization, provided the event is fired when the actual connection is established.

[Svarr]

That was my thought as well, but I couldn't find out where it's called.

[Svarr]

The Ready Event works. It is called, once the connection is established. But to get notified when the connection is lost, I had to add a callback for the OnConnectionFailed Event. OnClose doesn't get dispatched ofc...

I pushed my changes to my fork here.

Since initialising a previously deinitialised instance of DiscordRpcClient doesn't seem to be supported currently, I dispose of the current instance and create a new one when required. This seems to work for the most part, except I keep getting a NRE when attempting to ClearPresence(). At some point RpcConnection.ProcessFrame(EventPayload) queues a new PresenceMessage instance which is processed by DiscordRpcClient.ProcessMessage(IMessage). Now, for some reason, CurrentPresence is null (which shouldn't be possible) and RichPresence.Merge(BaseRichPresence) is called with the uninitialised and therefore null presence of the fresh message object. Naturally Merge just assumes that the parameter is not null...

For the sake of completeness the exception details:

Unhandled Exception while processing event: System.NullReferenceException
Object reference not set to an instance of an object.
    at DiscordRPC.RichPresence.Merge(BaseRichPresence presence)
    at DiscordRPC.DiscordRpcClient.ProcessMessage(IMessage message)
    at DiscordRPC.DiscordRpcClient.<.ctor>b__103_0(Object sender, IMessage msg)
    at DiscordRPC.RPC.RpcConnection.EnqueueMessage(IMessage message)

This may be a bug in DiscordRPC or maybe I have screwed up. I don't know yet.

But ignoring this exception appears to have no negative effect. I haven't tested if this issue would be solved with my changes, however.

[Svarr]

I found the cause for the exception and it's indeed a bug in DiscordRPC (see this issue).

But I forgot to make the crucial change of actually not sending the presence updates while not connected to Discord. This is fixed as of the latest commit to the branch feature/lifecycle-fix of my fork, linked in the post above.

I also tested whether the bug is still happening and it does seem to be fixed.

[sll552]

Nice work, would you like to open a pull request for it?

Also you might want to rebase because I pushed some changes today (which also include a cherry-pick from your master).

[Svarr]

Pull Request is up.

Also you might want to rebase because I pushed some changes today (which also include a cherry-pick from your master).

Already done. 🙂