Skip to content

Fix disconnect allowing DC from VC that user has no access to#6768

Open
Jackenmen wants to merge 5 commits into
Cog-Creators:V3/developfrom
Jackenmen:fix_disconnect_perm_behavior
Open

Fix disconnect allowing DC from VC that user has no access to#6768
Jackenmen wants to merge 5 commits into
Cog-Creators:V3/developfrom
Jackenmen:fix_disconnect_perm_behavior

Conversation

@Jackenmen
Copy link
Copy Markdown
Member

@Jackenmen Jackenmen commented May 21, 2026

Description of the changes

Fixes #6365, (supersedes) fixes #6385
I added a new condition disallowing disconnect when the user:

  • is not in a VC that the bot is in,
  • cannot connect to that VC,
  • and the VC has non-bot users in it (i.e. someone is actually listening to music in there)

Note that this PR might seem larger than it is - the first commit is a formatting-only change (removed else and dedented all lines), see how it looks with whitespaces hidden: cf6055f

The actual changeset is the 2 commits that come afterwards, see diff:
https://github.com/Cog-Creators/Red-DiscordBot/pull/6768/changes/cf6055f92ed84eff62526a7e54a176c71f65043f..456b93e6559e59447c245072566b4208744ee955

Have the changes in this PR been tested?

Yes

@Jackenmen Jackenmen added this to the 3.5.25 milestone May 21, 2026
@Jackenmen Jackenmen added the Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing. label May 21, 2026
@github-actions github-actions Bot added the Category: Cogs - Audio This is related to the Audio cog. label May 21, 2026
@Flame442 Flame442 self-assigned this May 24, 2026
Flame442
Flame442 previously approved these changes May 24, 2026
Copy link
Copy Markdown
Member

@Flame442 Flame442 left a comment

Choose a reason for hiding this comment

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

Nothing wrong with your changes (unless the way you refactored caused any of these issues I didn't really look closely), but some feedback on the existing implementation of the checks in this command while I am looking at them.

Comment thread redbot/cogs/audio/core/commands/controller.py Outdated
Comment thread redbot/cogs/audio/core/commands/controller.py Outdated
@Jackenmen
Copy link
Copy Markdown
Member Author

I've decided to rework the if statements further with some actual documentation in the code comments, which should actually address both of the review comments:

  • future readers will no longer be confused because the expected behaviour is clearly stated, meaning I can remove (vote_enabled or (vote_enabled and dj_enabled))
  • is_alone is checked, along with can_skip - if either of these is true, the user should be allowed to DC the bot; if not, we have elif conditions that check the specific early-return conditions that we previously had
    • as you might notice, this is a slight change in behaviour - when can_instaskip is True, we do not early return regardless of whether the user can connect to/is in the player's current VC; this is because instaskip is only True if one of the following is true:
      • the user has a DJ role
      • the user is a mod/guild owner/bot owner
      • the player's channel is equal to the user's channel (i.e. the maybe_move_player() function returned True, meaning the player moved to the user's channel)

@Jackenmen Jackenmen requested a review from Flame442 May 26, 2026 17:52
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. Type: Bug Unexpected behavior, result, or exception. In case of PRs, it is a fix for the foregoing.

Projects

None yet

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