fix(Util): fix sorting for GuildChannels

[That-Guy977]

Please describe the changes this PR makes and why it should be merged:

Util#discordSort sorts Ids descending when the rawPosition is the same, which is consistent with Role sorting, but is not for GuildChannel sorting, which sorts Ids ascending

Status and versioning classification:

• Typings don't need updating

Note: I've been unable to test the changes nor some similar versions, due to issues as I mentioned here; I'm not sure what I'm doing wrong, as I've only modified the required files and this method, but it seems to break the entire Util exports
I've just decided to create this PR in the hopes that someone else with more experience can fix these mistakes
Issues resolved, changes tested

[kyranet]

Come think of it, can't we read the first value in the collection and determine whether or not it's a collection of GuildChannels? That way we do 1 instanceof check rather than N * log(N) times (being N the collection's size), which can be quite expensive as instanceof is an O(N) operation (being N the amount of prototypes it takes to get to null), resulting on an O(N^2) cost?

I'm thinking more along the lines of:

// Nothing to sort if it has 0 or 1 entry:
if (collection.size < 2) return collection.clone();

// Check whether or not the first value is a GuildChannel, if it is, assume the rest
// of entries also are:
const isGuildChannel = collection.first() instanceof GuildChannel;
return collection.sorted(isGuildChannel ?
	(a, b) => a.rawPosition - b.rawPosition || Number(BigInt(a.id) - BigInt(b.id)) :
	(a, b) => a.rawPosition - b.rawPosition || Number(BigInt(b.id) - BigInt(a.id)));

This way it keeps being O(N * log N) while also fixes the bug.

[That-Guy977]

Would it really be necessary to add a case specifically for 0 or 1 elements in the collection? Wouldn't the internal use of Array#sort just not do much at all?

[kyranet]

That's because with this trick, we're reading the first element, so we need to make sure the collection is not empty (if (collection.size === 0) return collection.clone();), however, since we cannot sort a collection with one entry (it's always sorted), then we change the condition from === 0 to < 2, that's why the suggested code is like that.

[That-Guy977]

But if there aren't elements, the sort function isn't run anyways, so I don't understand why we would need to make sure that it isn't empty
It'd just be undefined instanceof GuildChannel, no? and that would be false but it wouldn't really matter

[kyranet]

Fair enough, feel free to remove that size check.

[That-Guy977]

This is what i got to after a few style iterations

static discordSort(collection) {
  const isGuildChannel = collection.first() instanceof GuildChannel;
  return collection.sorted(
    (a, b) =>
      a.rawPosition - b.rawPosition ||
      Number(isGuildChannel ? BigInt(a.id) - BigInt(b.id) : BigInt(b.id) - BigInt(a.id)),
  );
}

Does this look good to you?

[kyranet]

You're back to making an easily avoidable ternary check N * log(N) times.

[That-Guy977]

Ah, i missed the purpose of the seperate functions, I'll just go with your method minus the size check then

[kyranet]

As long as this is tested, looks good to me.

[That-Guy977]

The issues i mentioned in the initial comment seem to have fixed themselves, so I was able to test it, and the position is consistent with the UI for both channels and roles with this version
However, I'm now getting these warnings/errors from sending the message output, not sure if this is related/significant? I tested this in the node repl

(node:24339) Warning: Accessing non-existent property 'idToBinary' of module exports inside circular dependency
Uncaught TypeError: Util.idToBinary is not a function
    at Function.deconstruct (/Users/that_guy977/Desktop/Stuff/Github/test/node_modules/discord.js/src/util/SnowflakeUtil.js:63:25)
    at Message._patch (/Users/that_guy977/Desktop/Stuff/Github/test/node_modules/discord.js/src/structures/Message.js:62:43)
    at new Message (/Users/that_guy977/Desktop/Stuff/Github/test/node_modules/discord.js/src/structures/Message.js:48:10)
    at MessageManager._add (/Users/that_guy977/Desktop/Stuff/Github/test/node_modules/discord.js/src/managers/CachedManager.js:58:32)
    at MessageManager._add (/Users/that_guy977/Desktop/Stuff/Github/test/node_modules/discord.js/src/managers/MessageManager.js:31:18)
    at TextChannel.send (/Users/that_guy977/Desktop/Stuff/Github/test/node_modules/discord.js/src/structures/interfaces/TextBasedChannel.js:177:59)

Seems to occur with the other Util methods as well

Solved with 5dddd92