-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Convert queueing from reactions to Discord buttons. #1006
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So... while I like the idea of this change, this implementation is not at all how I would have done it. There's a ton of formatting changes, and a lot of changes to core systems that are not meant to be changed in the way you've changed them. Frankly, the best way to go about adding this functionality is to create a class that extends the normal Paginator, but listens for button events instead of reaction adds.
I'm not going to close this right now, but I'm also not sure if there is anything in this PR that would be salvageable for a PR doing this in a way that more matches how JMusicBot is set up.
@@ -177,7 +178,7 @@ else if(config.getGame().getName().equalsIgnoreCase("none")) | |||
.setActivity(nogame ? null : Activity.playing("loading...")) | |||
.setStatus(config.getStatus()==OnlineStatus.INVISIBLE || config.getStatus()==OnlineStatus.OFFLINE | |||
? OnlineStatus.INVISIBLE : OnlineStatus.DO_NOT_DISTURB) | |||
.addEventListeners(cb.build(), waiter, new Listener(bot)) | |||
.addEventListeners(cb.build(), waiter, new Listener(bot), new QueueButtonListener(settings, cb.build())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not add another listener, it should re-use the EventWaiter
|
||
/** | ||
* | ||
* @author John Grosh <john.a.grosh@gmail.com> | ||
*/ | ||
public class QueueCmd extends MusicCommand | ||
{ | ||
public class QueueCmd extends MusicCommand { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first instance of many where you have converted next-line braces to same-line braces. JMusicBot uses next-line.
void someMethod()
{
// content
}
@@ -86,6 +90,32 @@ public void doCommand(CommandEvent event) | |||
}); | |||
return; | |||
} | |||
|
|||
if(pagenum > ah.getQueue().getNumberOfPages()){ | |||
event.reply("That page does not exist. The maximum number of pages on the queue is " + String.valueOf(ah.getQueue().getNumberOfPages()) + ". Showing last page."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't be sending an extraneous message when we're not changing the behavior and the changed behavior is already visible (in the different page number).
@@ -20,6 +20,8 @@ | |||
import java.util.List; | |||
import java.util.Set; | |||
|
|||
import net.dv8tion.jda.api.entities.Message; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FairQueue is a very generic container and should definitely not contain any references to JDA, JMusicBot, or any music-related systems at all. Do not modify this class unless it is to add generic functionality.
This pull request...
Description
This removes the old reaction buttons for queueing and replaces them with shiny new Discord buttons.
![image](https://user-images.githubusercontent.com/84154786/137815433-edd4c919-c34e-477f-9a7e-da635d91b44f.png)
![image](https://user-images.githubusercontent.com/84154786/137815484-1176024e-44a5-4930-8589-9e8914a8b415.png)
The new playlist UI:
Queue buttons:
The
QueueCmd
object now tracks its last message and will remove its buttons from all but the last queue message. If a user tries to navigate to a now-empty queue, this is also dealt with.As with before, the user is given 30 seconds to accept or decline the addition of a playlist.
Purpose
Discord buttons are so much more flexible than reactions, and it looks nicer.