discordjs / collection Goto Github PK

Hi,

Currently .filter() is a custom implementation that returns a filtered map with the ID being they key. .map() returns an array, consistent with Javascript documentation. Is it possible to implement a function that allows for the Collection to return a map that can return a map with a different key as for instance such as channel.name -> channel etc.

I wrote and got a reply in the wrong repo about this feature request here discordjs/discord.js#6645

Issue description

Im making a permission handler for my bot and I have this following code in a module:

permissionHandler.js

module.exports = {
    async getStaffPermissionFlags(guild, member){
        if(!member || !guild) return;
        const adminRoles = ['600912549570936833'];
        const moderatorRoles = ['659905403458682937'];

        return{
            admin: member.roles.cache.hasAny(adminRoles),
            moderator: member.roles.cache.hasAny(moderatorRoles)
        }
    },
    execute(client){
        client.permissionsHandler = {}
        client.permissionsHandler.getStaffPermissionFlags = this.getStaffPermissionFlags;
    }
}

When the bot is started it runs this file and sets the permissionHandler.getStaffPermissionFlags object to a function in the module.

I can then later call this by using

await interaction.client.permissionsHandler.getStaffPermissionFlags(interaction.guild, interaction.member.id);

The code is intended to return an object with admin and moderator boolean values but it always returns false for both values even though my test account has both roles with id's 600912549570936833 for admin and 659905403458682937 for moderator.

Am I using it incorrectly or is it a bug within the @discordjs/collection package function?

It appears in the documentation that I'm using it right, pass an array to the function.

I've already asked about this on stackoverflow but to no avail.

Code sample

//Permission Handler File
module.exports = {
    async getStaffPermissionFlags(guild, member){
        if(!member || !guild) return;
        const adminRoles = ['600912549570936833'];
        const moderatorRoles = ['659905403458682937'];

        return{
            admin: member.roles.cache.hasAny(adminRoles),
            moderator: member.roles.cache.hasAny(moderatorRoles)
        }
    },
    execute(client){
        client.permissionsHandler = {}
        client.permissionsHandler.getStaffPermissionFlags = this.getStaffPermissionFlags;
    }
}

//To access the handler from client
await interaction.client.permissionsHandler.getStaffPermissionFlags(interaction.guild, interaction.member);

@discordjs/collection version

0.3.2

Node.js version

16.7.0

Operating system

Running on windows 10/Raspbian

Priority this issue should have

Medium (should be fixed soon)

I run the cmd npm start and it has this:
npm ERR! missing script: start

npm ERR! A complete log of this run can be found in:
npm ERR! C:\Users\QCSie\AppData\Roaming\npm-cache_logs\2020-10-12T14_37_22_148Z-debug.log

Need to add unit tests, setup CI testing, etc.

I don't think the above behavior is really the one wanted unless I'm wrong, so here's a suggested fix:
Last line of both methods: return Array.from({ length: amount }, (): V => arr.splice(Math.floor(Math.random() * arr.length), 1)[0]);
Replacement: return Array.from({ length: Math.min(amount, arr.length) }, (): V => arr.splice(Math.floor(Math.random() * arr.length), 1)[0]);

Feature

Current implementation Quirks

1.) intersect isn't commutative, even though it should be

When taking the intersection (∩) of two sets T and S:

𝑆 ∩ 𝑇 = 𝑇 ∩ 𝑆

Order doesn't affect the final outcome of the intersection. This makes sense since we're dealing with equality of elements within a given set and if elements are equal in both sets, the element is kept. Order has no involvement here.

Now let's see if the Collection#intersect respects this property of set intersection:

Sample code:

const T = new Collection<string, string>();
const S = new Collection<string, string>();

T.set('foo', 'bar');
T.set('left', 'right');

S.set('foo', 'baz');
S.set('left', 'other-left');

console.log(T.intersect(S));
console.log(S.intersect(T));

Output:

Collection(2) [Map] { 'foo' => 'baz', 'left' => 'other-left' }
Collection(2) [Map] { 'foo' => 'bar', 'left' => 'right' }

The output for each intersection is different. This means it isn't a true intersection because proper intersections are commutative. IE T.intersect(S) and S.intersect(T) should yield values that are exactly equal.

We can look at the current implementation to see why this is caused:

public intersect(other: Collection<K, V>) {
	const coll = new this.constructor[Symbol.species]<K, V>();
	for (const [k, v] of other) {
		if (this.has(k)) coll.set(k, v);
	}
	return coll;
}

This method only considers values from the right hand side of the intersection. More specifically it only uses keys as the basis of equality. Which brings me to another design oversight.

2.) Key-Based intersection doesn't actually intersect a Map-like structure.

Key-based intersection is just that, an intersection of keys within the key set of a map. However the issue comes into play once you try to tie values in within those keys. You never intersected the values, you only intersected the keys. The act of associating values in the intersection, that haven't been intersected, means that this is no longer an actual intersection.

Ideal solution or implementation

Compare Maps as a set of entries, not as a key-value store

To fix this, we need to change the way we think of map comparisons. Everything I stated above applies to sets. Well... we have an issue here, Map's aren't sets. Well in terms of K/V access they aren't. However in terms of implementation they are definitely are. Hence we have:

Map#entries

Now we can consider the map as a set rather than just a pure K/V access object. This means we can now apply the proper type of intersection on this map.

Let's try a proper intersection on the previous code example:

const T = new Collection<string, string>();
const S = new Collection<string, string>();

T.set('foo', 'bar');
T.set('left', 'right');

S.set('foo', 'baz');
S.set('left', 'other-left');

console.log(T.intersect(S));
console.log(S.intersect(T));

Output:

Collection(0) [Map] {  }
Collection(0) [Map] {  }

Wait what? The intersect is empty?!

Yup, this is completely intentional. We're now taking values into account which means if a given entry has the same key but differing values, it's no longer in the intersection. This preserves the true function of a proper intersection.

I want to note now intersect is commutative since T.intersect(S) is the same set as S.intersect(T).

But isn't that less useful?

Well what use cases are there for key-based intersection?

Also if the library is deciding which collection values to keep and which ones not to keep, doesn't this make the whole of idea of intersecting collections more confusing.

Usually if you want to intersect maps you have two maps that are homogenous in terms of key types and value types. So in most cases the "proper" intersection won't affect those cases.

For example intersecting roles of a user with one given in a list, has the same effect in both implementations. It just so happens that the algorithm I'm proposing is actually correct for all cases not just a certain cases.

How would this be implemented?

For key equality the same method of using Map#has would be used. Under the hood this method uses the SameValueZero algorithm for testing equality. Object.is is the API-equivalent implementation of SameValueZero. So naturally it would also be used for map-value equality.

public intersect(other: Collection<K, V>) {
	const coll = new this.constructor[Symbol.species]<K, V>();
	for (const [k, v] of other) {
        const thisV = this.get(k)
		if (thisV) {
			if (Object.is(thisV, v) {
             	coll.set(k, v);
             }
		}
	}
	return coll;
}

Now the intersection method does correct intersections, and avoids the pitfalls of the previous implementations.

Ok, but there are some use cases where I find key-based map intersection useful

Understood, however this isn't something the library should be implementing. The is because as stated above the library obfuscated the precedence it uses for intersections. And it would be confusing for a method literally called intersect to not perform a proper intersection.

Instead this functionality should be implemented by the user. Doing so, is quite trivial:

declare const T: Collection<string, string>;
declare const S: Collection<string, string>;

// I as a user can control precedence, in this case T has value precedence over S
// however I can change that freely it doesn't fit my use case.
const customIntersected = new Collection<string, string>();
T.filter((_v, k) => S.has(k)).forEach(customIntersected.set);

With the user constructing they're own key-based intersection it returns control to the user to set precedence, and doesn't rely on an implementation that is opinionated in terms of collection precedence.

Alternative solutions or implementations

We could remove intersect, but I don't think that's very ideal.

Other context

No response

npm ERR! 404 Not Found - GET https://registry.npmjs.org/@discordjs%2fcollection - Not found
npm ERR! 404
npm ERR! 404 '@discordjs/collection@latest' is not in the npm registry.
npm ERR! 404 You should bug the author to publish it (or use the name yourself!)

Collection is missing the toJSON method that depend on a method specific to Discord.js; will need to come up with a solution here or there.

Issue description

The following error can be produced with any two collections that have different key or value types. I understand why you shouldn't be able to use two collections with different key types, as the resulting collection will always be empty, however, I don't understand why you can't do two collections with different value types, as values aren't referenced at all in either of the functions below.

For example, if I had a collection of non-discord related data about users (Collection<Snowflake, RandomUserData>), and wanted to only view the data for users who are in a voice channel (VoiceChannel#members is Collection<Snowflake, GuildMember>), I wouldn't be able to.

Code sample

const collection1 = new Collection<string, string>(...);
const collection2 = new Collection<string, number>(...);

collection1.intersect(collection2);
collection1.difference(collection2);

// Argument of type 'Collection<string, number>' is not assignable to parameter of type 'Collection<string, string>'.

@discordjs/collection version

0.3.2

Node.js version

16.6.1

Operating system

Windows

Priority this issue should have

Low (slightly annoying)

Collection#sorted should be renamed to some sort of verb; "sorted" is an adjective and doesn't fit in with the rest of the method names, which are mostly verbs (I recognize that some is a loose fit, but it's coming from Array so...). Opening this issue to generate some ideas.

Issue description

As stated in the documentation, Collection has a function called hasAny() which accepts an Array as argument.
But the hasAny() function always returns false eventhough the member has one of the given roles 100 %.

When using has() it works perfectly fine.

Code sample

const roles = [ "roleid1", "roleid2" ];

// Not working logic
if (<message>.member.roles.cache.hasAny(roles)) {
  console.log("Member has any of the roles");
}

// Working Logic (pretty inefficient)
let pass = false;
for (let i = 0; i < roles.length; i++) {
  if (message.member.roles.cache.has(ignore[i])) {
    pass = true;
  }
}
if (pass) {
  console.log("Member has any of the roles");
}

discord.js version

13.3.1

Node.js version

v16.13.0

Operating system

Linux

Priority this issue should have

Medium (should be fixed soon)

Which partials do you have configured?

CHANNEL

Which gateway intents are you subscribing to?

GUILDS, GUILD_MEMBERS, GUILD_PRESENCES, GUILD_MESSAGES, DIRECT_MESSAGES

I have tested this issue on a development release

No response

https://discord.js.org/#/docs/main/master/class/Collection says it extends BaseCollection at https://discord.js.org/#/docs/collection, but that URL doesn't work and just redirects to https://discord.js.org/#/docs/main/stable/general/welcome. The only options in the dropdown are Main library, Commando, and RPC; collection is probably supposed to show there.