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

[Audio] Allow a custom voice room to be specified in the arguments to the summon command. #5536

Open
wants to merge 20 commits into
base: V3/develop
Choose a base branch
from

Conversation

AAA3A-AAA3A
Copy link
Contributor

@AAA3A-AAA3A AAA3A-AAA3A commented Jan 8, 2022

Description of the changes

Hello,
With this code, a user will be able to use the summon command like this: [p]summon id_or_mention.
Some users will be able to put the bot in when it starts up, without having to join a chat room.
Of course, users should be careful with "empty disconnect". Otherwise, it will work.
Thank you in advance.

@github-actions github-actions bot added the Category: Cogs - Audio This is related to the Audio cog. label Jan 8, 2022
Copy link
Contributor

@Drapersniper Drapersniper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is not standard - usually discord clients allow users to reference text channels this is not the case for Voice channels which means users are required to explicitly use the channel ID.

Functionaly the PR is sound.
image

You need to address the changes I suggested - Optional without an a default hides the arguments in our Help.
image

When making PRs make sure to run make reformat in the root folder of the project to apply black -l 99 to all files changed.

In addition to the above you will need to update the docstrings for this command (Currently just """Summon the bot to a voice channel.""") to mention that users will need to use the channel ID - with a link to the documentation of where/how to get that.
And the difference in behaviour if a channel ID is not used.

@aikaterna please request changes (I'm unable to do so)

redbot/cogs/audio/core/commands/controller.py Outdated Show resolved Hide resolved
redbot/cogs/audio/core/commands/controller.py Outdated Show resolved Hide resolved
redbot/cogs/audio/core/commands/controller.py Outdated Show resolved Hide resolved
AAA3A-AAA3A and others added 2 commits March 20, 2022 18:37
Co-authored-by: Draper <27962761+Drapersniper@users.noreply.github.com>
Co-authored-by: Draper <27962761+Drapersniper@users.noreply.github.com>
@github-actions github-actions bot added Category: Cogs - Admin This is related to the Admin cog. Category: Cogs - Alias This is related to the Alias cog. Category: Bot Core Category: Cogs - Cleanup This is related to the Cleanup cog. Category: Cogs - Downloader This is related to the Downloader cog. Category: Cogs - Filter This is related to the Filter cog. Category: Cogs - General This is related to the General cog. Category: Core - Help This is related to our help command and/or HelpFormatter API. Category: Cogs - Image This is related to the Image cog. Category: Cogs - Modlog This is related to the Modlog cog. Category: Cogs - Streams This is related to the Streams cog. labels Mar 20, 2022
@Jackenmen Jackenmen removed Category: Cogs - Streams This is related to the Streams cog. Category: Bot Core Category: Cogs - Downloader This is related to the Downloader cog. Category: Cogs - Alias This is related to the Alias cog. Category: Core - Help This is related to our help command and/or HelpFormatter API. labels Mar 20, 2022
@AAA3A-AAA3A
Copy link
Contributor Author

This is good. The change has been made.

@AAA3A-AAA3A
Copy link
Contributor Author

The extra empty line has been added.

@Drapersniper
Copy link
Contributor

image
LGTM

redbot/cogs/audio/core/commands/controller.py Outdated Show resolved Hide resolved
redbot/cogs/audio/core/commands/controller.py Outdated Show resolved Hide resolved
redbot/cogs/audio/core/commands/controller.py Outdated Show resolved Hide resolved
@AAA3A-AAA3A
Copy link
Contributor Author

I did what you said.

@AAA3A-AAA3A AAA3A-AAA3A changed the title [Audio] Allow a custom voice room to be specified in the arguments to the "[p]summon" command [Audio] Allow a custom voice room to be specified in the arguments to the summon command. Jul 12, 2022
@Kreusada Kreusada mentioned this pull request Oct 30, 2022
"""Summon the bot to a voice channel.

You can specify a personalised voice room with an id. Otherwise, the bot will join the voice room you are in.
To learn how to get a channel ID please read the following help article: <https://support.discord.com/hc/en-us/articles/206346498-Where-can-I-find-my-User-Server-Message-ID>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To learn how to get a channel ID please read the following help article: <https://support.discord.com/hc/en-us/articles/206346498-Where-can-I-find-my-User-Server-Message-ID>
To learn how to get a channel ID please read [this help article](https://support.discord.com/hc/en-us/articles/206346498-Where-can-I-find-my-User-Server-Message-ID).

In this case, it might look more presentable to use hyperlink syntax instead of just having the link as it is (see my suggestion)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one issue with hyperlinks is that in non-embed help it renders without formatting. I can't remember if we've accepted this limitation in other commands with links in core, but if we have not I'd say to continue to not use them.

@github-actions github-actions bot added the Category: Docs - Other This is related to documentation that doesn't have its dedicated label. label Nov 12, 2022
@AAA3A-AAA3A AAA3A-AAA3A requested review from Kreusada and removed request for PredaaA and aikaterna February 23, 2023 18:34
@github-actions github-actions bot removed the Category: Docs - Other This is related to documentation that doesn't have its dedicated label. label Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Cogs - Audio This is related to the Audio cog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants