Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Most users not getting fetched #65

Open
ToxicFrog opened this issue Feb 18, 2020 · 12 comments
Open

Most users not getting fetched #65

ToxicFrog opened this issue Feb 18, 2020 · 12 comments
Labels
bug Something isn't working

Comments

@ToxicFrog
Copy link

When weechat-discord connects, it gets only a subset of the users on each channel (e.g. 16 users for a channel with 32 online and 80+ total users). This seems to be a change in the way Discord handles GUILD_MEMBERS_CHUNK, since it also affects discord.js and cordless.

Ideally there would be a way to fetch the entire user list without relying on this; there must be some API call that returns that, after all, or the discord web client wouldn't be able to. Failing that, a mostly-workable alternative would be to incrementally build the user list for each channel by adding anyone who speaks in that channel and anyone who's @mentioned to the user list as they show up.

@ToxicFrog
Copy link
Author

This also means that other people's @-mentions of unloaded users show up as "@invalid-user", because we can't rely on guild.read().members being fully populated, but in [1] Serenity does just that. This will likely need to be fixed in terminal-discord's fork of serenity to fetch users on demand because AIUI it's a problem that only affects user tokens, which upstream doesn't support.

[1] https://github.com/terminal-discord/serenity/blob/2e8784905b2f73826efcfb4cb14f9981de75bb57/src/utils/mod.rs#L711

@Noskcaj19
Copy link
Member

So, a bit of background, (mostly for me so I don't forget again), a little while ago the Discord client migrated to a lazy-loading approach for guilds and users (from opcode 12 which used to load everything but now it does nothing, to using opcode 14 for fetching guilds: f70173c).

The core of the lazy-loading seems to be opcode 14 (which responds with GUILD_MEMBER_LIST_UPDATE) which I actually implemented a while ago in the serenity fork.

This was stress tested with a test account in 55+ very large guilds with 10s of thousands of members, alas, it seems something has been changed.
I took a quick look at the response, and it doesn't look like anything has been changed... it needs to be looked into, but I don't have the time right now.

@virus-found
Copy link

Yeah, all mentions look like:
@invalid-user message

Kinda problematic to chat :)

@ToxicFrog
Copy link
Author

I have a not-even-half-baked patch that attempts to fetch user information on demand when they're first referenced (i.e. when they first speak in a channel you've joined or when they're first @ed by a message you see) via the HTTPCache, but it's not even at the point where it builds yet, let alone runs.

@Noskcaj19
Copy link
Member

Noskcaj19 commented May 19, 2020

Ok, so I've had some time to look into this and have good news and bad news. Good news is it looks like the primary source of this problem was my quick hack to process GUILD_MEMBER_LIST_UPDATEs did not actually add those users to the guilds, I never actually got around to adding that part. Well, I was able to fix that (I'll commit after it's cleaned up a bit), however it seems it doesn't actually load all the users. And, from looking at the official client, it looks like the remaining users are loaded asynchronously with "Request Guild Members" (8).
Asynchronous loading is great for performance, but isn't ideal in weechat.
I'll look into it further and see if theres an easy solution I'm missing.

I hope that I can implement it using hdata line editing, but I'm starting to feel like completely reimplementing the buffer render would be best.

@Noskcaj19
Copy link
Member

I've started some (really hacky) experiments in the async-user-fetching-tests branch

@Noskcaj19
Copy link
Member

Async user fetching has been committed to master, although it seems some users are still not fetched.

@virus-found
Copy link

Now I see mentions like these:
<@!100454794015014912> Hey
Although in original client it's:
@Meat Man Hey

@Noskcaj19
Copy link
Member

For some reason, not all members are being fetched (or received). I haven't been able to look into it, and I currently don't have to time to work on it. But it is a known issue that I will be looking into.

@Noskcaj19 Noskcaj19 added the bug Something isn't working label Sep 2, 2020
@Noskcaj19
Copy link
Member

The new message renderer implementation should now show unloaded users as @unknown-user until they load.
Although it seems there is still more I don't understand about async loading, since it seems some responses only include some users.

@ToxicFrog
Copy link
Author

I actually preferred the IDs to unknown-user, since at least that way I could tell when people were atting different people.

@Noskcaj19
Copy link
Member

That's a fair point, it should probably be configurable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants