Prune counts others messages when deleting

[abyss]

In the scenario:

Me: Thing 1
Me: Thing 2
Someone Else: Other Thing
Me: Thing 3

Running //prune 3, you would expect to delete all 3 things, but instead only deletes Thing 2 and 3 (due to counting Someone Else's Other Thing as well).

[rayzr522]

Almost impossible to fix this due to the nature of how the messages are fetched. It really is a pain in the butt, but the only way would be to re-write how prune/purge work altogether. I plan on doing this at some point anyways (to allow for pruning/purging more than 100 messages).

EDIT: Summary? Not gonna fix it yet.

[abyss]

If/when this does ever change, it will need to be advertised pretty heavily to people using it for a bit of time. Anyone that's learned the current prune behavior will .. well, get less than ideal results if they go with the old style ;)

[yonilerner]

Part of the difficulty with this is that since discord.js's fetchMessages doesnt allow you to filter by user, you would have to continually fetch messages until the number of messages from that user has been met. Something like:

let count = args.options.numberOfMessages
let lastMessage
while (count > 0) {
    const messages = channel.fetchMessages({limit: Math.min(100, count), after: lastMessage ? lastMessage : undefined})
    // iterate over messages, if belong to user, decrement count and delete message. check count and break if its 0
    lastMessage = messages.slice(-1)[0].id
}

At some point there would have to be an arbitrary cutoff at which we stop searching for messages, in the event that the user doesnt have that many messages to delete, otherwise we would loop indefinitely. Depending on the use case, the performance could be pretty terrible.

[abyss]

I think we should consider doing this and just locking it to the last 100 messages. Maybe make a note in the help/information for the command that it only deletes X messages within the last 100 of the current channel? If we even consider this worth fixing, of course.

[yonilerner]

That would definitely be a reasonable approach.

You mentioned the fact that we would have to make clear that the implementation has changed - but like you said, it might not be worth it: If the command deletes fewer messages than expected, it can be run again, but if it deletes more than expected, that could be really bad.

Might be better off to just leave it as is.

[rayzr522]

I can always slap my recursion method in there and allow it to delete unlimited messages. Why not? We could still limit it to 1000 or something.

function purge(channel, amount, deleted, before) {
    if (!deleted) deleted = 0;
    if (amount > 5000) amount = 5000;
    if (amount < 1) return deleted;

    var options = {
        limit: Math.min(amount, 100)
    };

    if (before) options['before'] = before;

    return channel.fetchMessages(options).then(messages => {
        var size = messages.size;
        deleted += size;
        var id;
        if (size < 1) {
            return deleted;
        } else {
            id = messages.last().id;
            if (size < 2) {
                messages.first().delete();
            } else {
                channel.bulkDelete(messages);
            }
        }

        if (size >= 100) {
            var value = purge(channel, amount - 100, deleted, id);
            return typeof value === 'number' ? value : value.then(amount => amount);
        }
    });
}

This is the method I use in BaconBot (a very old project) and it could be easily adapted to work for SharpBot. This, however, brings up another topic:

async/await

When do we want to introduce async/await? It would make methods such as this far easier to write, and I believe it would even be possible to do what the method does without recursion if we had async/await.

[yonilerner]

While I love the idea of introducing async/await, we dont need it to avoid recursion. Any loop can be written with promises in the following way:

let p = Promise.resolve()
for (let i = 0; i < someNumber; i++) {
    p = p.then(() => someLoopingOperation())
}
return p

[rayzr522]

Except we have to wait for all promises to complete before we know how many have successfully completed, and whether or not we need to delete more.

[yonilerner]

Thats not really true. You could do something like this:

const someLoopingOperation = deleteMore => {
    ....
    if (someCondition) {
        deleteMore = false
    }
    ...
    return deleteMore
}

let p = Promise.resolve(true)
let keepLooping = true
while (keepLooping) {
    p = p
        .then(someLoopingOperation)
        .then(deleteMore => {
            if (!deleteMore) {
                keepLooping = false
            }
            return deleteMore
        })
}
return p

Its not pretty, but very doable.

Again, I would love to upgrade Node and use async/await. But its also good to recognize that pretty much anything that can be accomplished with async/await can also be accomplished with vanilla promises

[rayzr522]

You can't use the (sync) while loop with the (async) Promises, it would cause the while loop to be called several times before even the first message was deleted.

[yonilerner]

Youre 10000% percent correct. This is what I get for trying to write code 10 minutes after rolling out of bed

[y21]

<Channel>.fetchMessages({limit:<amount>}).then(messageCollection => messageCollection.forEach(msg => {if(msg.author.tag == <Client>.user.tag){msg.delete()}}

[rayzr522]

@y21 yes, we know how to fetch messages and delete them if they are the user’s own messages. What is the purpose of your comment?

[archturtle]

why not just filter the retrieved messages and filter through them where the author id is the specific user id

[LucasPMagno]

@TurtleGamingFTW This is exactly what it does now, the problem is, you can't fetch more than 100 messages, so if you do //prune 100 and 50 of those 100 messages are sent by someone else, it will count those, but won't delete it, the result is you told it to delete 100 of your messages, and it deleted 50 of your messages.

[rayzr522]

If you're referring to the ChannelLogsQueryOptions, they have no option for retrieving the last X messages sorted by a specific author. To my knowledge, there is no way to do that, but if you are aware of a way to do so then I would be glad to hear it.