discordjs / discord.js

A powerful JavaScript library for interacting with the Discord API
https://discord.js.org
Apache License 2.0
25.36k stars 3.97k forks source link

discussion: Emit an event or change the emitted object from events that depend on cached data when said data isn't cached #2920

Closed vladfrangu closed 5 years ago

vladfrangu commented 5 years ago

Is your feature request related to a problem? Please describe.

A few places in discord.js have some if checks in place to emit events only if the data is cached, a good one to show would be in src/client/actions/GuildMemberRemove

https://github.com/discordjs/discord.js/blob/ab3a43919890f0a45812fd91788ea911a106f937/src/client/actions/GuildMemberRemove.js#L8-L18

This depends on the member (the data) being cached, which isn't always true (fetchAllMembers off, user who wasn't online so the bot didn't have it cached, or just swept away by people).

This would mean that, as an example, in order to properly handle leave events for uncached members, we'd have to tap into the RAW event and handle it there (which isn't hard per-see, but for beginner devs it's confusing)

Describe the ideal solution

Possibly have an event that would get emitted for uncached data (something like

client.on('uncachedEvent', (event) => {
    const { name, data } = event;
    /**
     * Name could be the client event, like guildMemberRemove,
     * Data could be an array or an object with parsed data, for example, with guildMemberRemove
     * { guild, user }, where guild is the guild where this event happened, and the user is the user who left
     */
});

)

Describe alternatives you've considered

Another thing that could be done is change what data gets emitted if the data is uncached (for example, partial GuildMember or Message objects containing the data we received and that we were able to parse (say, channel, guild, user, etc)), which would be alright, but it can also cause a lot more issues imho.

iCrawl commented 5 years ago

We already somewhat discussed this internally, emitting uncached events is on the list somewhere down the line. However what you proposed here is just the raw event. The data you get might be a bit differently structured. Overall I think this is not what we should aim for here.

amishshah commented 5 years ago

Addressed in partials PR

Dark1z commented 5 years ago

@amishshah , Is this fixed in version 11.5.0 ??? especially for event guildMemberRemove ??? Thanking you , Best regards πŸ‘

amishshah commented 5 years ago

@dark-1 sadly this isn't present in 11.5.0 nor will it be backported. The only way to be able to use this functionality is by using v12 (the master branch)

Dark1z commented 5 years ago

@amishshah , Thanks for insta-reply πŸ˜ƒ is there any code that can use raw ?? I tried inside raw :

if (client.guilds.get(data.guild_id).members.has(data.user.id)) return;
const member = client.guilds.get(data.guild_id).members.get(data.user.id);
client.emit(events[event.t], member);

But i guess first line won't work ... I am a little noob here...

Thanking you , Best regards πŸ‘

Dark1z commented 5 years ago

@amishshah ,

Will this work :

client.on('raw', async event => {
    if (event.t == 'GUILD_MEMBER_REMOVE') {
        if (!client.guilds.get(data.guild_id).members.has(data.user.id)) return;
        const member = await client.guilds.get(data.guild_id).members.get(data.user.id);
        if (member.deleted == true) return;
        client.emit(events[event.t], member);
    }
});

Your Kind help would be appreciated.

Thanking you , Best regards πŸ‘