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.

Sign up for GitHub

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

Jump to bottom

fix for audio disconnect command not checking perms #6385

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

fix for audio disconnect command not checking perms #6385

wants to merge 19 commits into from

Conversation

BenCos17
Copy link

@BenCos17 BenCos17 commented May 31, 2024 •
edited by Jackenmen
Loading

Description of the changes

fixes #6365

Have the changes in this PR been tested?

Yes

@github-actions github-actions bot added the Category: Cogs - Audio This is related to the Audio cog. label May 31, 2024
@BenCos17 BenCos17 marked this pull request as ready for review May 31, 2024 21:26
@Flame442
Copy link
Member

I think in cases where nobody is in the voice channel, it would make sense to allow someone not in the voice channel to disconnect.
Additionally, you are currently failing the style checks. Please format your PR with black -l 99 using the instructions in our CONTRIBUTING.MD file.

@BenCos17
Copy link
Author

I think in cases where nobody is in the voice channel, it would make sense to allow someone not in the voice channel to disconnect. Additionally, you are currently failing the style checks. Please format your PR with black -l 99 using the instructions in our CONTRIBUTING.MD file.

fixed the style stuff btw

@Flame442 Flame442 added the Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. label May 31, 2024
@Flame442 Flame442 linked an issue May 31, 2024 that may be closed by this pull request
[Audio] disconnect command should check if the user is actually in the VC #6365
Open
BenCos17 added 5 commits June 1, 2024 10:44
@BenCos17
fix double code
bae4e41
@BenCos17
Update controller.py
1fec6d1 
seeing if this will allow someone to disconnect without being in vc if no one is there
@BenCos17
Update controller.py
31ee06d 
try 2 sorry
@BenCos17
Revert "Update controller.py"
8917bea 
This reverts commit 31ee06d.
@BenCos17
Merge pull request #2 from Cog-Creators/V3/develop
b1669ee 
working on audio disconnect again -ben (update from upstream)
@BenCos17 BenCos17 marked this pull request as draft August 1, 2024 15:50
@BenCos17 BenCos17 marked this pull request as ready for review August 7, 2024 21:15
BenCos17 added a commit to JARVIS-discordbot/Red-DiscordBot-jarvis that referenced this pull request Aug 7, 2024
@BenCos17
Update controller.py from PR Cog-Creators#6385 in the main red repo
a0c1cd4
@BenCos17
Copy link
Author

And that should all work
I've tested it all on my red instance for the past while with no issues

@Flame442
Copy link
Member

I'm not certain if the current implementation needs to be so complicated. self._can_instaskip seems to be the utility method used by Audio to account for all of the kinds of permissions a user may need to take actions on the player. It would likely be better to use that method instead.

@BenCos17
Copy link
Author

I'm not certain if the current implementation needs to be so complicated. self._can_instaskip seems to be the utility method used by Audio to account for all of the kinds of permissions a user may need to take actions on the player. It would likely be better to use that method instead.

true
I'll have a look at that at some point then
thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aikaterna aikaterna Awaiting requested review from aikaterna aikaterna is a code owner

@PredaaA PredaaA Awaiting requested review from PredaaA PredaaA is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Category: Cogs - Audio This is related to the Audio cog. Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Audio] disconnect command should check if the user is actually in the VC
2 participants
@BenCos17 @Flame442