-
Notifications
You must be signed in to change notification settings - Fork 111
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
Experimental - Cancel-able Menus #69
base: master
Are you sure you want to change the base?
Changes from 3 commits
e62a3ab
4addb9d
064cf8f
47ca092
8b51c5a
d59da84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,9 @@ | |
import java.util.Arrays; | ||
import java.util.HashSet; | ||
import java.util.Set; | ||
import java.util.concurrent.Future; | ||
import java.util.concurrent.TimeUnit; | ||
import java.util.function.Consumer; | ||
|
||
import com.jagrosh.jdautilities.commons.waiter.EventWaiter; | ||
import net.dv8tion.jda.core.entities.*; | ||
|
@@ -60,14 +62,19 @@ public abstract class Menu | |
protected Set<Role> roles; | ||
protected final long timeout; | ||
protected final TimeUnit unit; | ||
|
||
protected Menu(EventWaiter waiter, Set<User> users, Set<Role> roles, long timeout, TimeUnit unit) | ||
|
||
private final Consumer<Message> finalAction; | ||
private Future<Void> cancelFuture; | ||
private Message attachedMessage; | ||
|
||
protected Menu(EventWaiter waiter, Set<User> users, Set<Role> roles, long timeout, TimeUnit unit, Consumer<Message> finalAction) | ||
{ | ||
this.waiter = waiter; | ||
this.users = users; | ||
this.roles = roles; | ||
this.timeout = timeout; | ||
this.unit = unit; | ||
this.finalAction = finalAction; | ||
} | ||
|
||
/** | ||
|
@@ -89,6 +96,95 @@ protected Menu(EventWaiter waiter, Set<User> users, Set<Role> roles, long timeou | |
*/ | ||
public abstract void display(Message message); | ||
|
||
/** | ||
* Returns the message, this menu was attached to. | ||
* This is set <b>asynchronously</b> after a call to a {@code display()} or equivalent method. | ||
* <br>The attached message will be reset, when the menu times out or is cancelled. | ||
* | ||
* @return The message, this menu was attached to or {@code null} if not yet attached or already timed out/cancelled. | ||
*/ | ||
public Message getAttachedMessage() | ||
{ | ||
return attachedMessage; | ||
} | ||
|
||
/** | ||
* Cancels the menu. This stops the EventWaiter from waiting for events and behaves exactly the same way as if the menu timed out. | ||
* That includes calling the final/cancel action if provided. | ||
*/ | ||
public void cancel() | ||
{ | ||
if(cancelFuture == null) | ||
throw new IllegalStateException("Menu was not yet displayed or is already cancelled"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So if I don't set the I think more appropriately this should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably better to change the message to something like "Menu can not be cancelled (not supported, not yet displayed or already ended)" |
||
cancelFuture.cancel(false); | ||
cancelFuture = null; | ||
} | ||
|
||
/** | ||
* Internally used to set the attached message. | ||
* Should be used by the corresponding Menu implementation as soon as the message was created/edited. | ||
* | ||
* @param attachedMessage | ||
* The message, where this menu was attached to | ||
*/ | ||
protected final void setAttachedMessage(Message attachedMessage) | ||
{ | ||
this.attachedMessage = attachedMessage; | ||
} | ||
|
||
/** | ||
* Internally used to set the cancelFuture used to cancel the menu. | ||
* Should be used by the corresponding Menu implementation as soon as {@code EventWaiter#waitForEvent} was used. | ||
* | ||
* @param cancelFuture | ||
* The cancel Future returned from {@code EventWaiter#waitForEvent} | ||
*/ | ||
protected final void setCancelFuture(Future<Void> cancelFuture) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am concerned about the implied functionality through these two methods. What if my menu doesn't need cancellation functionality? Should I still use them? Is it safe for my menu to not have cancel functionality? This needs to be documented, and if it is required I'd appreciate it mentioned in the class level documentation for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a big block in the class-level docs of Menu describing the whole system and its use |
||
{ | ||
this.cancelFuture = cancelFuture; | ||
} | ||
|
||
/** | ||
* Returns the finalAction given via constructor. | ||
* | ||
* @return The finalAction given via constructor. | ||
*/ | ||
protected final Consumer<Message> getFinalAction() | ||
{ | ||
return finalAction; | ||
} | ||
|
||
/** | ||
* Calls the final action and cleans up the attached message and cancelFuture (sets them to {@code null}). | ||
* <br>The actual Menu implementation should should call this method whenever the waiting loop is about to exit | ||
* and the final action should be called (e.g. as timeout action for the EventWaiter). | ||
* | ||
* <p>This method is a shortcut for using {@link #finalizeMenu(boolean) finalizeMenu(true)} | ||
* | ||
* @see #finalizeMenu(boolean) | ||
*/ | ||
protected void finalizeMenu() | ||
{ | ||
finalizeMenu(true); | ||
} | ||
|
||
/** | ||
* Cleans up the attached message and cancelFuture (sets them to {@code null}) and optionally calls the final action. | ||
* <br>The actual Menu implementation should should call this method whenever the waiting loop is about to exit | ||
* and {@link #finalizeMenu() finalizeMenu()} is not applicable. | ||
* | ||
* @see #finalizeMenu() | ||
*/ | ||
protected void finalizeMenu(boolean callFinalAction) | ||
{ | ||
Message tmp = this.attachedMessage; | ||
this.attachedMessage = null; | ||
//also clean up cancelFuture if not already | ||
this.cancelFuture = null; | ||
if(callFinalAction && finalAction != null) | ||
finalAction.accept(tmp); | ||
} | ||
|
||
/** | ||
* Checks to see if the provided {@link net.dv8tion.jda.core.entities.User User} | ||
* is valid to interact with this Menu.<p> | ||
|
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.
Unfortunately, this breaks compatibility with previous versions for people who implemented their own menus.
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.
Will add an overloaded Constructor without finalAction that just calls this one with null finalAction (for old menus, not using the new methods)