-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GuildChannel#permissionsLocked incorrect when parent category updated #6004
Comments
could this be related to #6082 and accordingly be fixed now? |
Nope, this still happens as of |
Hey @MattIPv4, from what I understood in this issue, in the child category the Also, what is |
That is one consequence of the issue, yes, but it is not the root cause. The root cause is that the parent receives an update first, so when the child update comes through the child is perceived to be out of sync with the parent, when in reality the child is being updated because it is in sync with the parent.
https://discord.js.org/#/docs/collection/master/class/Collection?scrollTo=some (This is now actually |
Do you think it is possible that the child is updated first and then the parent channel, which could be a possible reason why this is happening? I'm testing trying to debug and this issue and had this thought. Thoughts @MattIPv4 ? |
No, the parent is updated first. If you run my repro code locally you will be able to observe the parent update event arrives first, which updates the d.js cache for both the parent and therefore the child. The child update event then arrives after, at which point the parent has already been updated and so the child is perceived as out of sync. |
I'm trying to run your code and getting this error |
Yup, as I noticed two replies above, with recent changes to master I believe it is now |
From what I understood and debugged only we can see channel category being updated for |
Hey @MattIPv4 so I tried this: client.on('channelUpdate', (oldChannel, newChannel) => {
console.log(newChannel.type);
if (newChannel.type === 'GUILD_CATEGORY') {
console.log('Category permissions updated');
return;
}
if (newChannel.type === 'GUILD_TEXT') {
console.log('Text channel permissions updated');
console.log('Synced before update:', oldChannel.permissionsLocked);
console.log('Synced after update:', newChannel.permissionsLocked);
return;
}
if (newChannel.type === 'GUILD_VOICE') {
console.log('Voice channel permissions updated');
console.log('Synced before update:', oldChannel.permissionsLocked);
console.log('Synced after update:', newChannel.permissionsLocked);
return;
}
}); Output: Is this the output you were expecting? Node: 14.6.0 |
Yeah, that looks like it also demonstrates the issue here -- |
Hey, Matt isn't it how it is supposed to be? |
Yes, based on the current logic, it is the expected behaviour. However, given that |
What should be the desired output here instead, how can we change the handling here? Can you give me an example? |
I would expect |
console.log('Synced before update:', oldChannel.permissionsLocked);
console.log('Synced after update:', newChannel.permissionsLocked); So basically both should return true? |
Yes 👍, because they were still set to be synced, and were updated in exactly the same way the parent was, as they are synced. |
This would then defeat the purpose, would it be better if we just had |
No, it does not defeat the purpose... the whole idea here is that I want to be able to accurately report when a channel has its permissions updated in such a way that means the permissions are no longer set to sync, or have been updated when they were previously not in sync and are now in sync. This current behaviour results in continual false positives, as the value of |
Oh, got it! I was not able to understand the sync thingy for a while now I get it. Ok, so we can actually do that by check the permissionsLocked of I hope this would solve the issues here? |
Yup, that sounds like the right theory. |
So I went a bit into the folders and pointed where it permissions were getting allocated.
Original const channelVal = this.permissionOverwrites.cache.get(key);
const parentVal = this.parent.permissionOverwrites.cache.get(key); After Changes made const channelVal = this.parent.permissionOverwrites.cache.get(key);
const parentVal = this.parent.permissionOverwrites.cache.get(key); This keeps it sync always |
To put it in simple terms, no, this doesn't sound like it solves the issue. It sounds like you're removing the concept of permissions overwrites on channels, which makes no sense at all. |
I recommend creating a PR though so that proposed changes to solve this issue can be discussed there, directly on your proposed changes, rather than in the issue :) |
Ohh ok, I think I'll wait for someone to add more comments on how to solve this. |
The closest thing I can think of to a solution would be some mess where when the Category is updated it checks all its children, sees if they're currently synced, and creates some sort of Side note - since this isn't a proper API-provided boolean doesnt this mean it can return a false positive if a parent and child have the same overwrites without being in sync? |
From my quick testing, no -- Discord seems to potentially apply the same logic as D.js does. If you have a channel & parent sync'ed, and then update the channel to be out of sync, and then match that update in the parent the channel will show as sync'ed again. |
Please describe the problem you are having in as much detail as possible:
Assume a situation where a text channel is in a category, with the channel's permissions set in Discord to sync with the category.
Accessing
GuildChannel#permissionsLocked
for that channel will confirm this.Now, assume that the permissions overwrites in the category are updated, and you have an event listener set for channel updates.
First, you will see the update come through for the category permissions overwrites being updated, which is all fine (or is it).
Then, you will see the update come through for the channel's permissions overwrites being updated. At this point though, if you access
GuildChannel#permissionsLocked
on the old channel, it will report that the permissions were not locked, even though they were, andGuildChannel#permissionsLocked
on the new channel will report that the permissions are not locked.This happens (from my understanding) because we've already received the update event for the category overwrites changing, so both the old and new channel in the channel update event have the same, already updated, category as their parent which is used for the
GuildChannel#permissionsLocked
calculation.Include a reproducible code sample here, if possible:
Further details:
Relevant client options:
partials: channel (though both
GuildChannel
andChannel
do not have apartial
prop)gateway intents: guilds
other: none
I have also tested the issue on latest master, commit hash:
fe6cc0c15dde99caa1049d35f75b9335ace1721d
The text was updated successfully, but these errors were encountered: