GuildMemberUpdated event is not working as I expected.

[eyagovbusiness]

Hey there! 👋

So, I was diving into the DiscordClient.GuildMemberUpdated AsyncEventHandler just the other day. Decided to test it out by changing my avatar image. Funny thing, I noticed that GuildMemberUpdateEventArgs.MemberAfter was the same as...well, GuildMemberUpdateEventArgs.MemberAfter itself. 😅 Here's what I was looking at:

Thought I'd pull the repo and snoop around a bit. Found the quirk in the "internal async Task OnGuildMemberUpdateEventAsync(TransportMember member, DiscordGuild guild)" inside DiscordClient.Dispatch.cs. Looks like this method is doing double duty - updating the cached member and users. Cool, but it makes MemberBefore and MemberAfter end up being identical after the UserCache and guild.Members update.

Got some thoughts on how to tweak this, but wanted to touch base with you first. Were you guys aiming to track every change when a member's updated in this handler? Or was it more about just keeping the cached items fresh?

Thank you for your work and your time

[OoLunar]

Oh dear. This is definitely a bug. Just to confirm, you're on the latest nightly? @Plerx2493 has been doing phenomenal work rewriting the core parts of the lib, but it looks like we haven't gotten around to that area yet!

Ideally the GuildMemberUpdate event should work like this: Any *Before properties should contain the "old" cached information that we received previously. Any *After properties should contain the new information copied straight from Discord, with minimal (ideally none!) modification.

Thank you for bringing this to our attention! I'll create an issue for this and I'm sure it'll be resolved soon.

[eyagovbusiness]

Thanks for the response. Just to add a bit more detail, I've encountered this issue using the latest NuGet package (version 4.4.2), and the same problem persists even with the current main branch from the cloned repo.

If @Plerx2493 is already tackling this, I'm eager to check out their solution once it's integrated. I'm curious to see how he is handling what seems for me like a tricky problem. On the off chance that a fix isn't in the works yet, I'd propose considering a double caching strategy for the UserCache. Imagine each entry as a pair: one being the current user and the other, their previous state. This could be implemented by abstracting the existing dictionary to function with value pairs, and tweaking the concurrent dictionary so that updates to current values simultaneously swap out the old with the current, and the current with the new. Granted, this might double the worst-case size of the UserCache, so perhaps introducing a lifespan for the "old" values could help manage this by periodically clearing them out.

In case you need more information or details, I am open for discussion.

Big thanks for all the fantastic work on this library. Your efforts really shine through!

[Plerx2493]

My pr tackles a full rewrite of our cache. The fix to this problem could be a simple problem of when cache gets updated. I assume atm the cache gets updated and then the "old cached" entity is taken from cache. We have other similar events where before and after fields work fine