Fix IRC quit messages sending to all channels by tracking users #214
Conversation
Keep a list of users in each channel, using the names event in addition to the join/part events, so as to prevent spamming unnecessary channels when the quit event is raised with all channels from a server. Fixes reactiflux#213.
|
I can't think of a better way to fix this either - seems like it's a TODO in the node-irc codebase: https://github.com/martynsmith/node-irc/blob/master/lib/irc.js#L587-L592 I'm not sure if it's strictly worth it, but I think the implementation here is clean enough that we should include it. Maybe use a set instead of an array for the user lists though? That way you can use |
|
It doesn't even look like in their code they're trying to match if the user is in that channel – they just delete the user from the channel (whether they were there or not) and proceed to add it to the list, when I think they should be guarding by whether the object's key exists? I've updated this to use sets – thanks, didn't realize they existed in JavaScript. (Or, well, ECMAScript 6?) |
| this.sendExactToDiscord(channel, `*${nick}* has left the channel (${reason})`); | ||
| }); | ||
|
|
||
| this.ircClient.on('quit', (nick, reason, channels) => { | ||
| if (!this.ircStatusNotices || nick === this.nickname) return; | ||
| channels.forEach((channel) => { | ||
| if (!this.channelUsers[channel].delete(nick)) return; |
There was a problem hiding this comment.
clever use of delete's return value :)
|
Ye, it indeed doesn't look like it attempts to find the correct username. If they ever get merged we can remove this I guess. |
|
Released in 2.3.1 - thanks! |
Keep a list of users in each channel, using the
namesevent in addition to thejoin/partevents, so as to prevent spamming unnecessary channels when thequitevent is raised with all channels from a server.Fixes #213.
I'm not sure if this is a bug with node-irc (the quit event shouldn't be sent for all channels on that server, just the channels the user is in) or if it's intended behavior (the quit event is sent 'for the server', and so for all channels on it). Either way this is a workaround to make the new functionality… less spammy. I might report it there but they don't seem to have very active development.
This also rewrites a test name to make it fit into the syntax ("Bot Events should…") and reflect the new functionality: "should be possible to get the bot to announce itself joining" → "should announce the bot itself when config enabled".