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

#42: Command /autopause #98

Merged
merged 8 commits into from
Jan 22, 2022
Merged

#42: Command /autopause #98

merged 8 commits into from
Jan 22, 2022

Conversation

joao-conde
Copy link
Collaborator

@joao-conde joao-conde commented Jan 21, 2022

- -
Issue Closes #42
Dependencies --
Decisions Usage /autopause as a toggle, similar to /repeat.
Make use of serenity's context cache to store GuildSettings such as the autopause flag. @aquelemiguel will be using it for other Guild-related cache.
Tested the edge cases of skipping and seeking to the end of the track with autopause on.

@joao-conde joao-conde added the ✨ feature New feature or request label Jan 21, 2022
Copy link
Collaborator

@afonsojramos afonsojramos left a comment

Choose a reason for hiding this comment

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

Super clean, love it 👌

Copy link
Owner

@aquelemiguel aquelemiguel left a comment

Choose a reason for hiding this comment

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

Looks great, I like your refactoring of the /events dir.

A behaviour went against my intuition: skipping an autopaused song also pauses the next one. From what I've investigated, there's no simple way to check whether the track was paused before being skipped. Let's revisit this later if after using it for a while we find it that frustrating.

@aquelemiguel aquelemiguel merged commit 39fb017 into main Jan 22, 2022
@aquelemiguel aquelemiguel deleted the jc/42-autopause branch January 24, 2022 02:11
@aquelemiguel aquelemiguel mentioned this pull request Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command !autopause
3 participants