search

Lachee / discord-rpc-csharp

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

[BUG] Calling ClearPresence logs an error #155

Open IncPlusPlus opened 3 years ago

IncPlusPlus commented 3 years ago

Describe the bug When calling ClearPresence on an active client. An error is logged. This is definitely a bug. Interestingly, the rich presence status is successfully cleared from Discord despite this error.

To Reproduce Steps to reproduce the behavior:

  1. Set a presence with client.SetPresence(new RichPresence(){})
  2. Clear this presence with client.ClearPresence()
  3. Observe the discord status clearing but an error also being logged

This error only seems to happen if there's an existing presence. If one was never set, ClearPresence. Also, if one was cleared already, calling ClearPresence again doesn't log the error a second time.

Expected behavior The status is cleared but without logging an error.

Desktop:

Additional context In case it matters, it's worth noting that my project is using the null safety feature of C# by having <Nullable>enable</Nullable> inside of the main PropertyGroup of the .csproj file.

Logs

2021-08-28 23:27:57,385 [Discord IPC Thread] ERROR - Unhandled Exception while processing event: System.NullReferenceException
2021-08-28 23:27:57,387 [Discord IPC Thread] ERROR - Object reference not set to an instance of an object.
2021-08-28 23:27:57,387 [Discord IPC Thread] ERROR -    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)
Svarr commented 3 years ago

I was just about to open an issue for that.

The exception is caused by the if statements (starting at DiscordRpcClient.cs:303) not accounting for CurrentPresence == null && pm.Presence == null. This leads to RichPresence.Merge(BaseRichPresence) being called, which just assumes that the argument is not null. Exchanging the if and else if checks (i.e. testing for pm.Presence == null first) should prevent the NRE from happening.

Though, the fact that line 486 nulls CurrentPresence, which results in CurrentPresence being set to null twice (first in DiscordRpcClient.SetPresence(RichPresence) and then in DiscordRpcClient.ProcessMessage(IMessage)), suggests, that maybe there was some kind of planning mistake and the whole algorithm should be revised. 🤷