EionRobb / purple-discord

A libpurple/Pidgin plugin for Discord
GNU General Public License v3.0
381 stars 43 forks source link

I'm not dead --- code cleanup and really real ratelimit bucket(s) #425

Closed dlmarquis closed 2 years ago

dlmarquis commented 2 years ago

I had a ton of IRL things going on this summer and disappeared, but these are the odds and ends I've managed to find time for since Spring. I'm submitting this as a PR instead of pushing directly to master because it feels a bit rude to drop off the face of the Earth for months and then push several commits that rearrange a bunch of code without warning.

Items contained in this PR:

alexolog commented 2 years ago

Welcome back @dlmarquis!

Thank you for your work, it is much appreciated!

When you have the time and the inclination, can you please look at the following issues:

Those, in my opinion, are the most significant issues as they affect the core functionality. There are multiple tickets logged about them (by me and by others).

dlmarquis commented 2 years ago

@alexolog Thanks! And I absolutely will.

The stuff in this PR that touches the guild member fetch and the gateway limiting was actually intended to fix a few causes of the incoming/outgoing messages problem. And I'd planned on the next phase of that work to be rearranging the discord_fetch_url* and related functions so that we could display useful errors to the user (e.g., "request timeout, message was not sent" or "You are being ratelimited!") to aid in troubleshooting the rest of them.

The conversations viewed elsewhere not being fetched is partly due to how we check for unread messages before we fully load in the buddy list. The buddy list is where we keep pidgin's "last seen message" for each channel. I've been meaning on rearranging the startup code for a while, and I did have a version where we loaded the buddy list before handing unreads in #376. Doing this will also let us streamline the way we handle those unread messages at startup.

For the message duplicates, I think I know exactly what's causing that. The way the different history fetching cases work is a bit of a convoluted mess, and the way they all connect into discord_join_chat and discord_open_chat needs to be gutted and rewritten (which is why it hasn't happened yet). Fixing and rearranging the startup code to fix the unreads problem is going to significantly impact how much work has to be done here, so this one should be tackled last.

alexolog commented 2 years ago

Thank you so much for working on Making Pidgin Great Again! I kid! I kid! But the appreciation is genuine!