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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat(VoiceState): self mute/deaf methods #3243

Merged
merged 8 commits into from May 3, 2019

Conversation

Projects
None yet
5 participants
@MrJacz
Copy link
Contributor

commented Apr 30, 2019

Please describe the changes this PR makes and why it should be merged:
In this PR I've added setSelfMute & setSelfDeaf methods to the VoiceState class, I've also managed to add typings for implemented methods and VoiceConnection#voice.
Also managed to fix a slight bug where Error wasn't imported in VoiceState so some errors weren't being thrown with the proper string.

replaces #3171

Status

  • 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

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.
Show resolved Hide resolved src/errors/Messages.js Outdated
Show resolved Hide resolved src/structures/VoiceState.js
Show resolved Hide resolved src/structures/VoiceState.js
@appellation

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

Not a huge fan of separating self muting/deafening from regular muting/deafening. This creates extra errors and methods that can simply be taken care of in the existing mute and deafen methods.

Edit: I guess there's plenty of pushback on this idea, so scratch that.

MrJacz added some commits Apr 30, 2019

@amishshah
Copy link
Member

left a comment

Needs testing, I might do it in the next few days but if anyone else is able to test feel free!

/**
* Self-mutes/unmutes the bot for this voice state.
* @param {boolean} mute Whether or not the bot should be self-muted
* @returns {Promise<boolean>}

This comment has been minimized.

Copy link
@SpaceEEC

SpaceEEC May 1, 2019

Member

You might be able to infer the meaning of the return value, but a doc-string would certainly not hurt.

/**
* Self-deafens/undeafens the bot for this voice state.
* @param {boolean} deaf Whether or not the bot should be self-deafened
* @returns {Promise<boolean>}

This comment has been minimized.

Copy link
@SpaceEEC

SpaceEEC May 1, 2019

Member

See above.

@@ -1185,6 +1185,7 @@ declare module 'discord.js' {
public channel: VoiceChannel;
public readonly client: Client;
public readonly dispatcher: StreamDispatcher;
public readonly voice: VoiceState;

This comment has been minimized.

Copy link
@SpaceEEC

SpaceEEC May 1, 2019

Member

I think those should be sorted alphabetically.

* @private
*/
sendVoiceStateUpdate(options = {}) {
options = Util.mergeDefault({
guild_id: this.channel.guild.id,
channel_id: this.channel.id,
self_mute: false,
self_deaf: false,
self_mute: Boolean(this.voice.selfMute),

This comment has been minimized.

Copy link
@SpaceEEC

SpaceEEC May 1, 2019

Member

There is no voice state when the client initially joins.

Show resolved Hide resolved typings/index.d.ts

MrJacz added some commits May 2, 2019

@MrJacz

This comment has been minimized.

Copy link
Contributor Author

commented May 2, 2019

I have tested this, as you can see in the screenshots below.

Seems to work fine and as intended. type checking works as well.
Self deaf/muted
Not self deaf/muted
Proof it is self and not server

Show resolved Hide resolved src/structures/VoiceState.js Outdated

amishshah and others added some commits May 3, 2019

Update src/structures/VoiceState.js
Co-Authored-By: MrJacz <23615291+MrJacz@users.noreply.github.com>
fix

@SpaceEEC SpaceEEC merged commit 692494d into discordjs:master May 3, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MrJacz MrJacz deleted the MrJacz:self-deaf-mute branch May 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.