Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(GuildMemberManager): allow moving members to any non-text channel #5681

Merged
merged 1 commit into from Jun 2, 2021

Conversation

AndyClausen
Copy link
Contributor

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

This will allow moving members to channels with type stage. Closes #5673 馃檪

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

@tipakA
Copy link
Contributor

tipakA commented May 27, 2021

Technically, this way category channels are going to be an issue, since they return false from isText()
Is this a call for isVoice()? 馃憖

@AndyClausen
Copy link
Contributor Author

Technically, this way category channels are going to be an issue, since they return false from isText()
Is this a call for isVoice()? 馃憖

Touch茅. I could change this to just add stage to the list of checks, but if an isVoice method is going to be added, I'd much rather wait for that 馃槄

@PraveshKunwar
Copy link
Contributor

Technically, this way category channels are going to be an issue, since they return false from isText()
Is this a call for isVoice()? 馃憖

would an isVoice method be needed?

@ThaTiemsz
Copy link
Contributor

Technically, this way category channels are going to be an issue, since they return false from isText()
Is this a call for isVoice()? 馃憖

Touch茅. I could change this to just add stage to the list of checks, but if an isVoice method is going to be added, I'd much rather wait for that 馃槄

Store channels will also be an issue.

@tipakA
Copy link
Contributor

tipakA commented May 27, 2021

Well, yes, i wasn't really trying to provide an extensive list of every single channel type this check will not work with, but simply give an example of why isText() isn't exactly the way here.

And my mention of isVoice() was made to avoid this issue in future in case discord would make new voice channel type. I'm definitely not saying whether it should be added or not, since it's a overall niche use case.

@ckohen
Copy link
Member

ckohen commented May 27, 2021

Since all applicable voice channels (dm channels technically have voice, but they don't count and not for bots anyways) extend BaseGuildVoiceChannel, and any future channel you could move people in a guild should probably follow suit, you can use instanceof BaseGuildVoiceChannel I think.

@AndyClausen
Copy link
Contributor Author

I'll try that out tomorrow.

Now that I have you all here, should I rather make a new commit or amend the existing when it's such small changes? I've heard many different opinions from other repos.

@AndyClausen
Copy link
Contributor Author

Tested with instanceof, seems to work. Rebased and amended my commit.

src/managers/GuildMemberManager.js Outdated Show resolved Hide resolved
change check on channel type to allow stage channel as well

close discordjs#5673
@kyranet kyranet requested a review from iCrawl May 29, 2021 17:41
@iCrawl iCrawl merged commit d21e6af into discordjs:master Jun 2, 2021
@iCrawl iCrawl added this to the Version 13 milestone Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot move members to a stage channel
9 participants