From e62a3abba10cdcf8a98462daaa85ecb43e96fcfe Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Thu, 9 Aug 2018 18:12:48 +0200 Subject: [PATCH 1/6] Add possibility to cancel menus and EventWaiter --- .../commons/waiter/EventWaiter.java | 52 ++++++++++++++----- .../jagrosh/jdautilities/menu/ButtonMenu.java | 9 ++-- .../com/jagrosh/jdautilities/menu/Menu.java | 38 ++++++++++++++ .../jdautilities/menu/OrderedMenu.java | 24 ++++++--- .../jagrosh/jdautilities/menu/Paginator.java | 49 ++++++++--------- .../jdautilities/menu/SelectionDialog.java | 21 ++++---- .../jagrosh/jdautilities/menu/Slideshow.java | 49 ++++++++--------- 7 files changed, 160 insertions(+), 82 deletions(-) diff --git a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java index c70d7716..d019177f 100644 --- a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java +++ b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java @@ -24,9 +24,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Set; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.*; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -157,9 +155,9 @@ public boolean isShutdown() *
  • 2) The internal threadpool is shut down, meaning that no more tasks can be submitted.
    • * */ - public void waitForEvent(Class classType, Predicate condition, Consumer action) + public Future waitForEvent(Class classType, Predicate condition, Consumer action) { - waitForEvent(classType, condition, action, -1, null, null); + return waitForEvent(classType, condition, action, -1, null, null); } /** @@ -198,7 +196,7 @@ public void waitForEvent(Class classType, Predicate cond *
  • 2) The internal threadpool is shut down, meaning that no more tasks can be submitted.
    • * */ - public void waitForEvent(Class classType, Predicate condition, Consumer action, + public Future waitForEvent(Class classType, Predicate condition, Consumer action, long timeout, TimeUnit unit, Runnable timeoutAction) { Checks.check(!isShutdown(), "Attempted to register a WaitingEvent while the EventWaiter's threadpool was already shut down!"); @@ -206,18 +204,18 @@ public void waitForEvent(Class classType, Predicate cond Checks.notNull(condition, "The provided condition predicate"); Checks.notNull(action, "The provided action consumer"); - WaitingEvent we = new WaitingEvent<>(condition, action); - Set set = waitingEvents.computeIfAbsent(classType, c -> new HashSet<>()); - set.add(we); + WaitingEvent we = new WaitingEvent<>(classType, condition, action, timeoutAction); + Future future = we.getFuture(); if(timeout > 0 && unit != null) { threadpool.schedule(() -> { - if(set.remove(we) && timeoutAction != null) - timeoutAction.run(); + future.cancel(false); }, timeout, unit); } + + return future; } @Override @@ -268,16 +266,25 @@ public void shutdown() threadpool.shutdown(); } - + private class WaitingEvent { + final Class classType; final Predicate condition; final Consumer action; + final Runnable cancelAction; + final CompletableFuture future = new WaitingFuture(); - WaitingEvent(Predicate condition, Consumer action) + WaitingEvent(Class classType, Predicate condition, Consumer action, Runnable cancelAction) { + this.classType = classType; this.condition = condition; this.action = action; + this.cancelAction = cancelAction; + + + Set set = waitingEvents.computeIfAbsent(classType, c -> new HashSet<>()); + set.add(this); } boolean attempt(T event) @@ -285,9 +292,28 @@ boolean attempt(T event) if(condition.test(event)) { action.accept(event); + future.complete(null); return true; } return false; } + + Future getFuture() + { + return future; + } + + private class WaitingFuture extends CompletableFuture + { + @Override + public boolean cancel(boolean mayInterruptIfRunning) + { + if(isDone() || isCancelled()) + return isCancelled(); + if(waitingEvents.get(classType).remove(WaitingEvent.this) && cancelAction != null) + cancelAction.run(); + return super.cancel(mayInterruptIfRunning); + } + } } } diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java index f4fa0bee..a2a0c10d 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java @@ -50,7 +50,7 @@ public class ButtonMenu extends Menu private final List choices; private final Consumer action; private final Consumer finalAction; - + ButtonMenu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, Color color, String text, String description, List choices, Consumer action, Consumer finalAction) { @@ -94,6 +94,7 @@ public void display(Message message) private void initialize(RestAction ra) { ra.queue(m -> { + setAttachedMessage(m); for(int i=0; i ra) { // This is the last reaction added. r.queue(v -> { - waiter.waitForEvent(MessageReactionAddEvent.class, event -> { + setCancelFuture(waiter.waitForEvent(MessageReactionAddEvent.class, event -> { // If the message is not the same as the ButtonMenu // currently being displayed. if(!event.getMessageId().equals(m.getId())) @@ -139,8 +140,8 @@ private void initialize(RestAction ra) // Preform the specified action with the ReactionEmote action.accept(event.getReaction().getReactionEmote()); - finalAction.accept(m); - }, timeout, unit, () -> finalAction.accept(m)); + callFinalAction(finalAction); + }, timeout, unit, () -> callFinalAction(finalAction))); }); } } diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java index c3e88b46..52312bcd 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java @@ -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,6 +62,9 @@ public abstract class Menu protected Set roles; protected final long timeout; protected final TimeUnit unit; + + private Future cancelFuture; + private Message attachedMessage; protected Menu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit) { @@ -89,6 +94,39 @@ protected Menu(EventWaiter waiter, Set users, Set roles, long timeou */ public abstract void display(Message message); + public Message getAttachedMessage() + { + return attachedMessage; + } + + public void cancel() + { + if(cancelFuture == null) + throw new IllegalStateException("Menu was not yet displayed or is already cancelled"); + cancelFuture.cancel(false); + cancelFuture = null; + } + + protected final void setAttachedMessage(Message attachedMessage) + { + this.attachedMessage = attachedMessage; + } + + protected final void setCancelFuture(Future cancelFuture) + { + this.cancelFuture = cancelFuture; + } + + protected void callFinalAction(Consumer finalAction) + { + Message tmp = this.attachedMessage; + this.attachedMessage = null; + //also clean up cancelFuture if not already + this.cancelFuture = null; + if(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.

    diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java index c1e6b733..f61061f9 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java @@ -156,6 +156,7 @@ public void display(Message message) private void initialize(RestAction ra) { ra.queue(m -> { + setAttachedMessage(m); try { // From 0 until the number of choices. // The last run of this loop will be used to queue @@ -206,7 +207,7 @@ private void initialize(RestAction ra) private void waitGeneric(Message m) { // Wait for a GenericMessageEvent - waiter.waitForEvent(GenericMessageEvent.class, e -> { + setCancelFuture(waiter.waitForEvent(GenericMessageEvent.class, e -> { // If we're dealing with a message reaction being added we return whether it's valid if(e instanceof MessageReactionAddEvent) return isValidReaction(m, (MessageReactionAddEvent)e); @@ -223,12 +224,15 @@ private void waitGeneric(Message m) MessageReactionAddEvent event = (MessageReactionAddEvent)e; // Process which reaction it is if(event.getReaction().getReactionEmote().getName().equals(CANCEL)) - cancel.accept(m); + callFinalAction(cancel); else + { // The int provided in the success consumer is not indexed from 0 to number of choices - 1, // but from 1 to number of choices. So the first choice will correspond to 1, the second // choice to 2, etc. action.accept(m, getNumber(event.getReaction().getReactionEmote().getName())); + callFinalAction(null); + } } // If it's a valid MessageReceivedEvent else if (e instanceof MessageReceivedEvent) @@ -237,29 +241,35 @@ else if (e instanceof MessageReceivedEvent) // Get the number in the message and process int num = getMessageNumber(event.getMessage().getContentRaw()); if(num<0 || num>choices.size()) - cancel.accept(m); + callFinalAction(cancel); else + { action.accept(m, num); + callFinalAction(null); + } } - }, timeout, unit, () -> cancel.accept(m)); + }, timeout, unit, () -> callFinalAction(cancel))); } // Waits only for reaction input private void waitReactionOnly(Message m) { // This one is only for reactions - waiter.waitForEvent(MessageReactionAddEvent.class, e -> { + setCancelFuture(waiter.waitForEvent(MessageReactionAddEvent.class, e -> { return isValidReaction(m, e); }, e -> { m.delete().queue(); if(e.getReaction().getReactionEmote().getName().equals(CANCEL)) - cancel.accept(m); + callFinalAction(cancel); else + { // The int provided in the success consumer is not indexed from 0 to number of choices - 1, // but from 1 to number of choices. So the first choice will correspond to 1, the second // choice to 2, etc. action.accept(m, getNumber(e.getReaction().getReactionEmote().getName())); - }, timeout, unit, () -> cancel.accept(m)); + callFinalAction(null); + } + }, timeout, unit, () -> callFinalAction(cancel))); } // This is where the displayed message for the OrderedMenu is built. diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java index 386c35b9..304ee33a 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java @@ -176,10 +176,11 @@ else if (pageNum>pages) Message msg = renderPage(pageNum); initialize(message.editMessage(msg), pageNum); } - + private void initialize(RestAction action, int pageNum) { action.queue(m -> { + setAttachedMessage(m); if(pages > 1) { if(bulkSkipNumber > 1) @@ -189,14 +190,14 @@ private void initialize(RestAction action, int pageNum) if(bulkSkipNumber > 1) m.addReaction(RIGHT).queue(); m.addReaction(bulkSkipNumber > 1? BIG_RIGHT : RIGHT) - .queue(v -> pagination(m, pageNum), t -> pagination(m, pageNum)); + .queue(v -> pagination(pageNum), t -> pagination(pageNum)); } else if(waitOnSinglePage) { // Go straight to without text-input because only one page is available m.addReaction(STOP).queue( - v -> paginationWithoutTextInput(m, pageNum), - t -> paginationWithoutTextInput(m, pageNum) + v -> paginationWithoutTextInput(pageNum), + t -> paginationWithoutTextInput(pageNum) ); } else @@ -206,24 +207,24 @@ else if(waitOnSinglePage) }); } - private void pagination(Message message, int pageNum) + private void pagination(int pageNum) { if(allowTextInput || (leftText != null && rightText != null)) - paginationWithTextInput(message, pageNum); + paginationWithTextInput(pageNum); else - paginationWithoutTextInput(message, pageNum); + paginationWithoutTextInput(pageNum); } - private void paginationWithTextInput(Message message, int pageNum) + private void paginationWithTextInput(int pageNum) { - waiter.waitForEvent(GenericMessageEvent.class, event -> { + setCancelFuture(waiter.waitForEvent(GenericMessageEvent.class, event -> { if(event instanceof MessageReactionAddEvent) - return checkReaction((MessageReactionAddEvent) event, message.getIdLong()); + return checkReaction((MessageReactionAddEvent) event); else if(event instanceof MessageReceivedEvent) { MessageReceivedEvent mre = (MessageReceivedEvent) event; // Wrong channel - if(!mre.getChannel().equals(message.getChannel())) + if(!mre.getChannel().equals(getAttachedMessage().getChannel())) return false; String rawContent = mre.getMessage().getContentRaw().trim(); if(leftText != null && rightText != null) @@ -247,7 +248,7 @@ else if(event instanceof MessageReceivedEvent) }, event -> { if(event instanceof MessageReactionAddEvent) { - handleMessageReactionAddAction((MessageReactionAddEvent) event, message, pageNum); + handleMessageReactionAddAction((MessageReactionAddEvent) event, pageNum); } else { @@ -268,24 +269,24 @@ else if(rightText != null && rawContent.equalsIgnoreCase(rightText) && (pageNum targetPage = Integer.parseInt(rawContent); } - message.editMessage(renderPage(targetPage)).queue(m -> pagination(m, targetPage)); + getAttachedMessage().editMessage(renderPage(targetPage)).queue(m -> pagination(targetPage)); mre.getMessage().delete().queue(v -> {}, t -> {}); // delete the calling message so it doesn't get spammy } - }, timeout, unit, () -> finalAction.accept(message)); + }, timeout, unit, () -> callFinalAction(finalAction))); } - private void paginationWithoutTextInput(Message message, int pageNum) + private void paginationWithoutTextInput(int pageNum) { - waiter.waitForEvent(MessageReactionAddEvent.class, - event -> checkReaction(event, message.getIdLong()), // Check Reaction - event -> handleMessageReactionAddAction(event, message, pageNum), // Handle Reaction - timeout, unit, () -> finalAction.accept(message)); + setCancelFuture(waiter.waitForEvent(MessageReactionAddEvent.class, + event -> checkReaction(event), // Check Reaction + event -> handleMessageReactionAddAction(event, pageNum), // Handle Reaction + timeout, unit, () -> callFinalAction(finalAction))); } // Private method that checks MessageReactionAddEvents - private boolean checkReaction(MessageReactionAddEvent event, long messageId) + private boolean checkReaction(MessageReactionAddEvent event) { - if(event.getMessageIdLong() != messageId) + if(event.getMessageIdLong() != getAttachedMessage().getIdLong()) return false; switch(event.getReactionEmote().getName()) { @@ -305,7 +306,7 @@ private boolean checkReaction(MessageReactionAddEvent event, long messageId) } // Private method that handles MessageReactionAddEvents - private void handleMessageReactionAddAction(MessageReactionAddEvent event, Message message, int pageNum) + private void handleMessageReactionAddAction(MessageReactionAddEvent event, int pageNum) { int newPageNum = pageNum; switch(event.getReaction().getReactionEmote().getName()) @@ -345,7 +346,7 @@ private void handleMessageReactionAddAction(MessageReactionAddEvent event, Messa } break; case STOP: - finalAction.accept(message); + callFinalAction(finalAction); return; } @@ -354,7 +355,7 @@ private void handleMessageReactionAddAction(MessageReactionAddEvent event, Messa } catch(PermissionException ignored) {} int n = newPageNum; - message.editMessage(renderPage(newPageNum)).queue(m -> pagination(m, n)); + getAttachedMessage().editMessage(renderPage(newPageNum)).queue(m -> pagination(n)); } private Message renderPage(int pageNum) diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java index 8409bbaa..0d2f1457 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java @@ -144,29 +144,30 @@ else if(selection>choices.size()) Message msg = render(selection); initialize(message.editMessage(msg), selection); } - + private void initialize(RestAction action, int selection) { action.queue(m -> { + setAttachedMessage(m); if(choices.size()>1) { m.addReaction(UP).queue(); m.addReaction(SELECT).queue(); m.addReaction(CANCEL).queue(); - m.addReaction(DOWN).queue(v -> selectionDialog(m, selection), v -> selectionDialog(m, selection)); + m.addReaction(DOWN).queue(v -> selectionDialog(selection), v -> selectionDialog(selection)); } else { m.addReaction(SELECT).queue(); - m.addReaction(CANCEL).queue(v -> selectionDialog(m, selection), v -> selectionDialog(m, selection)); + m.addReaction(CANCEL).queue(v -> selectionDialog(selection), v -> selectionDialog(selection)); } }); } - private void selectionDialog(Message message, int selection) + private void selectionDialog(int selection) { - waiter.waitForEvent(MessageReactionAddEvent.class, event -> { - if(!event.getMessageId().equals(message.getId())) + setCancelFuture(waiter.waitForEvent(MessageReactionAddEvent.class, event -> { + if(event.getMessageIdLong() != getAttachedMessage().getIdLong()) return false; if(!(UP.equals(event.getReaction().getReactionEmote().getName()) || DOWN.equals(event.getReaction().getReactionEmote().getName()) @@ -191,10 +192,10 @@ else if(loop) newSelection = 1; break; case SELECT: - success.accept(message, selection); + success.accept(getAttachedMessage(), selection); break; case CANCEL: - cancel.accept(message); + callFinalAction(cancel); return; } @@ -202,8 +203,8 @@ else if(loop) event.getReaction().removeReaction(event.getUser()).queue(); } catch (PermissionException ignored) {} int n = newSelection; - message.editMessage(render(n)).queue(m -> selectionDialog(m, n)); - }, timeout, unit, () -> cancel.accept(message)); + getAttachedMessage().editMessage(render(n)).queue(m -> selectionDialog(n)); + }, timeout, unit, () -> callFinalAction(cancel))); } private Message render(int selection) diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java index 1c1d62f7..8baf20a5 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java @@ -157,10 +157,11 @@ else if (pageNum>urls.size()) Message msg = renderPage(pageNum); initialize(message.editMessage(msg), pageNum); } - + private void initialize(RestAction action, int pageNum) { action.queue(m->{ + setAttachedMessage(m); if(urls.size()>1) { if(bulkSkipNumber > 1) @@ -170,37 +171,37 @@ private void initialize(RestAction action, int pageNum) if(bulkSkipNumber > 1) m.addReaction(RIGHT).queue(); m.addReaction(bulkSkipNumber > 1? BIG_RIGHT : RIGHT) - .queue(v -> pagination(m, pageNum), t -> pagination(m, pageNum)); + .queue(v -> pagination(pageNum), t -> pagination(pageNum)); } else if(waitOnSinglePage) { - m.addReaction(STOP).queue(v -> pagination(m, pageNum), t -> pagination(m, pageNum)); + m.addReaction(STOP).queue(v -> pagination(pageNum), t -> pagination(pageNum)); } else { - finalAction.accept(m); + callFinalAction(finalAction); } }); } - private void pagination(Message message, int pageNum) + private void pagination(int pageNum) { if(allowTextInput || (leftText != null && rightText != null)) - paginationWithTextInput(message, pageNum); + paginationWithTextInput(pageNum); else - paginationWithoutTextInput(message, pageNum); + paginationWithoutTextInput(pageNum); } - private void paginationWithTextInput(Message message, int pageNum) + private void paginationWithTextInput(int pageNum) { - waiter.waitForEvent(GenericMessageEvent.class, event -> { + setCancelFuture(waiter.waitForEvent(GenericMessageEvent.class, event -> { if(event instanceof MessageReactionAddEvent) - return checkReaction((MessageReactionAddEvent) event, message.getIdLong()); + return checkReaction((MessageReactionAddEvent) event); else if(event instanceof MessageReceivedEvent) { MessageReceivedEvent mre = (MessageReceivedEvent) event; // Wrong channel - if(!mre.getChannel().equals(message.getChannel())) + if(!mre.getChannel().equals(getAttachedMessage().getChannel())) return false; String rawContent = mre.getMessage().getContentRaw().trim(); if(leftText != null && rightText != null) @@ -224,7 +225,7 @@ else if(event instanceof MessageReceivedEvent) }, event -> { if(event instanceof MessageReactionAddEvent) { - handleMessageReactionAddAction((MessageReactionAddEvent) event, message, pageNum); + handleMessageReactionAddAction((MessageReactionAddEvent) event, pageNum); } else { @@ -246,24 +247,24 @@ else if(rightText != null && rawContent.equalsIgnoreCase(rightText) && (pageNum targetPage = Integer.parseInt(rawContent); } - message.editMessage(renderPage(targetPage)).queue(m -> pagination(m, targetPage)); + getAttachedMessage().editMessage(renderPage(targetPage)).queue(m -> pagination(targetPage)); mre.getMessage().delete().queue(v -> {}, t -> {}); // delete the calling message so it doesn't get spammy } - }, timeout, unit, () -> finalAction.accept(message)); + }, timeout, unit, () -> callFinalAction(finalAction))); } - private void paginationWithoutTextInput(Message message, int pageNum) + private void paginationWithoutTextInput(int pageNum) { - waiter.waitForEvent(MessageReactionAddEvent.class, - event -> checkReaction(event, message.getIdLong()), - event -> handleMessageReactionAddAction(event, message, pageNum), - timeout, unit, () -> finalAction.accept(message)); + setCancelFuture(waiter.waitForEvent(MessageReactionAddEvent.class, + event -> checkReaction(event), + event -> handleMessageReactionAddAction(event, pageNum), + timeout, unit, () -> callFinalAction(finalAction))); } // Private method that checks MessageReactionAddEvents - private boolean checkReaction(MessageReactionAddEvent event, long messageId) + private boolean checkReaction(MessageReactionAddEvent event) { - if(event.getMessageIdLong() != messageId) + if(event.getMessageIdLong() != getAttachedMessage().getIdLong()) return false; switch(event.getReactionEmote().getName()) { @@ -283,7 +284,7 @@ private boolean checkReaction(MessageReactionAddEvent event, long messageId) } // Private method that handles MessageReactionAddEvents - private void handleMessageReactionAddAction(MessageReactionAddEvent event, Message message, int pageNum) + private void handleMessageReactionAddAction(MessageReactionAddEvent event, int pageNum) { int newPageNum = pageNum; int pages = urls.size(); @@ -324,7 +325,7 @@ private void handleMessageReactionAddAction(MessageReactionAddEvent event, Messa } break; case STOP: - finalAction.accept(message); + callFinalAction(finalAction); return; } @@ -333,7 +334,7 @@ private void handleMessageReactionAddAction(MessageReactionAddEvent event, Messa } catch(PermissionException ignored) {} int n = newPageNum; - message.editMessage(renderPage(newPageNum)).queue(m -> pagination(m, n)); + getAttachedMessage().editMessage(renderPage(newPageNum)).queue(m -> pagination(n)); } private Message renderPage(int pageNum) From 4addb9d8f5554b135c1abbff8f301b17e922396a Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Fri, 10 Aug 2018 14:38:51 +0200 Subject: [PATCH 2/6] Move finalAction to abstract Menu class --- .../jagrosh/jdautilities/menu/ButtonMenu.java | 8 +++----- .../com/jagrosh/jdautilities/menu/Menu.java | 20 +++++++++++++++---- .../jdautilities/menu/OrderedMenu.java | 20 +++++++++---------- .../jagrosh/jdautilities/menu/Paginator.java | 12 +++++------ .../jdautilities/menu/SelectionDialog.java | 10 ++++------ .../jagrosh/jdautilities/menu/Slideshow.java | 12 +++++------ 6 files changed, 42 insertions(+), 40 deletions(-) diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java index a2a0c10d..9007ad10 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/ButtonMenu.java @@ -49,18 +49,16 @@ public class ButtonMenu extends Menu private final String description; private final List choices; private final Consumer action; - private final Consumer finalAction; ButtonMenu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, Color color, String text, String description, List choices, Consumer action, Consumer finalAction) { - super(waiter, users, roles, timeout, unit); + super(waiter, users, roles, timeout, unit, finalAction); this.color = color; this.text = text; this.description = description; this.choices = choices; this.action = action; - this.finalAction = finalAction; } /** @@ -140,8 +138,8 @@ private void initialize(RestAction ra) // Preform the specified action with the ReactionEmote action.accept(event.getReaction().getReactionEmote()); - callFinalAction(finalAction); - }, timeout, unit, () -> callFinalAction(finalAction))); + finalizeMenu(); + }, timeout, unit, this::finalizeMenu)); }); } } diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java index 52312bcd..c62ec320 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java @@ -63,16 +63,18 @@ public abstract class Menu protected final long timeout; protected final TimeUnit unit; + private final Consumer finalAction; private Future cancelFuture; private Message attachedMessage; - - protected Menu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit) + + protected Menu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, Consumer finalAction) { this.waiter = waiter; this.users = users; this.roles = roles; this.timeout = timeout; this.unit = unit; + this.finalAction = finalAction; } /** @@ -117,13 +119,23 @@ protected final void setCancelFuture(Future cancelFuture) this.cancelFuture = cancelFuture; } - protected void callFinalAction(Consumer finalAction) + protected final Consumer getFinalAction() + { + return finalAction; + } + + protected void finalizeMenu() + { + finalizeMenu(true); + } + + protected void finalizeMenu(boolean callFinalAction) { Message tmp = this.attachedMessage; this.attachedMessage = null; //also clean up cancelFuture if not already this.cancelFuture = null; - if(finalAction != null) + if(callFinalAction && finalAction != null) finalAction.accept(tmp); } diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java index f61061f9..d734b80b 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/OrderedMenu.java @@ -59,7 +59,6 @@ public class OrderedMenu extends Menu private final String description; private final List choices; private final BiConsumer action; - private final Consumer cancel; private final boolean useLetters; private final boolean allowTypedInput; private final boolean useCancel; @@ -76,13 +75,12 @@ public class OrderedMenu extends Menu Color color, String text, String description, List choices, BiConsumer action, Consumer cancel, boolean useLetters, boolean allowTypedInput, boolean useCancel) { - super(waiter, users, roles, timeout, unit); + super(waiter, users, roles, timeout, unit, cancel); this.color = color; this.text = text; this.description = description; this.choices = choices; this.action = action; - this.cancel = cancel; this.useLetters = useLetters; this.allowTypedInput = allowTypedInput; this.useCancel = useCancel; @@ -224,14 +222,14 @@ private void waitGeneric(Message m) MessageReactionAddEvent event = (MessageReactionAddEvent)e; // Process which reaction it is if(event.getReaction().getReactionEmote().getName().equals(CANCEL)) - callFinalAction(cancel); + finalizeMenu(); else { // The int provided in the success consumer is not indexed from 0 to number of choices - 1, // but from 1 to number of choices. So the first choice will correspond to 1, the second // choice to 2, etc. action.accept(m, getNumber(event.getReaction().getReactionEmote().getName())); - callFinalAction(null); + finalizeMenu(false); } } // If it's a valid MessageReceivedEvent @@ -241,14 +239,14 @@ else if (e instanceof MessageReceivedEvent) // Get the number in the message and process int num = getMessageNumber(event.getMessage().getContentRaw()); if(num<0 || num>choices.size()) - callFinalAction(cancel); + finalizeMenu(); else { action.accept(m, num); - callFinalAction(null); + finalizeMenu(false); } } - }, timeout, unit, () -> callFinalAction(cancel))); + }, timeout, unit, this::finalizeMenu)); } // Waits only for reaction input @@ -260,16 +258,16 @@ private void waitReactionOnly(Message m) }, e -> { m.delete().queue(); if(e.getReaction().getReactionEmote().getName().equals(CANCEL)) - callFinalAction(cancel); + finalizeMenu(); else { // The int provided in the success consumer is not indexed from 0 to number of choices - 1, // but from 1 to number of choices. So the first choice will correspond to 1, the second // choice to 2, etc. action.accept(m, getNumber(e.getReaction().getReactionEmote().getName())); - callFinalAction(null); + finalizeMenu(false); } - }, timeout, unit, () -> callFinalAction(cancel))); + }, timeout, unit, this::finalizeMenu)); } // This is where the displayed message for the OrderedMenu is built. diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java index 304ee33a..7c3a7895 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java @@ -66,7 +66,6 @@ public class Paginator extends Menu private final boolean numberItems; private final List strings; private final int pages; - private final Consumer finalAction; private final boolean waitOnSinglePage; private final int bulkSkipNumber; private final boolean wrapPageEnds; @@ -86,7 +85,7 @@ public class Paginator extends Menu boolean numberItems, List items, boolean waitOnSinglePage, int bulkSkipNumber, boolean wrapPageEnds, String leftText, String rightText, boolean allowTextInput) { - super(waiter, users, roles, timeout, unit); + super(waiter, users, roles, timeout, unit, finalAction); this.color = color; this.text = text; this.columns = columns; @@ -95,7 +94,6 @@ public class Paginator extends Menu this.numberItems = numberItems; this.strings = items; this.pages = (int)Math.ceil((double)strings.size()/itemsPerPage); - this.finalAction = finalAction; this.waitOnSinglePage = waitOnSinglePage; this.bulkSkipNumber = bulkSkipNumber; this.wrapPageEnds = wrapPageEnds; @@ -202,7 +200,7 @@ else if(waitOnSinglePage) } else { - finalAction.accept(m); + finalizeMenu(); } }); } @@ -272,7 +270,7 @@ else if(rightText != null && rawContent.equalsIgnoreCase(rightText) && (pageNum getAttachedMessage().editMessage(renderPage(targetPage)).queue(m -> pagination(targetPage)); mre.getMessage().delete().queue(v -> {}, t -> {}); // delete the calling message so it doesn't get spammy } - }, timeout, unit, () -> callFinalAction(finalAction))); + }, timeout, unit, this::finalizeMenu)); } private void paginationWithoutTextInput(int pageNum) @@ -280,7 +278,7 @@ private void paginationWithoutTextInput(int pageNum) setCancelFuture(waiter.waitForEvent(MessageReactionAddEvent.class, event -> checkReaction(event), // Check Reaction event -> handleMessageReactionAddAction(event, pageNum), // Handle Reaction - timeout, unit, () -> callFinalAction(finalAction))); + timeout, unit, this::finalizeMenu)); } // Private method that checks MessageReactionAddEvents @@ -346,7 +344,7 @@ private void handleMessageReactionAddAction(MessageReactionAddEvent event, int p } break; case STOP: - callFinalAction(finalAction); + finalizeMenu(); return; } diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java index 0d2f1457..4bd04240 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/SelectionDialog.java @@ -53,8 +53,7 @@ public class SelectionDialog extends Menu private final boolean loop; private final Function text; private final BiConsumer success; - private final Consumer cancel; - + public static final String UP = "\uD83D\uDD3C"; public static final String DOWN = "\uD83D\uDD3D"; public static final String SELECT = "\u2705"; @@ -65,7 +64,7 @@ public class SelectionDialog extends Menu Function color, boolean loop, BiConsumer success, Consumer cancel, Function text) { - super(waiter, users, roles, timeout, unit); + super(waiter, users, roles, timeout, unit, cancel); this.choices = choices; this.leftEnd = leftEnd; this.rightEnd = rightEnd; @@ -74,7 +73,6 @@ public class SelectionDialog extends Menu this.color = color; this.loop = loop; this.success = success; - this.cancel = cancel; this.text = text; } @@ -195,7 +193,7 @@ else if(loop) success.accept(getAttachedMessage(), selection); break; case CANCEL: - callFinalAction(cancel); + finalizeMenu(); return; } @@ -204,7 +202,7 @@ else if(loop) } catch (PermissionException ignored) {} int n = newSelection; getAttachedMessage().editMessage(render(n)).queue(m -> selectionDialog(n)); - }, timeout, unit, () -> callFinalAction(cancel))); + }, timeout, unit, this::finalizeMenu)); } private Message render(int selection) diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java index 8baf20a5..ef54b2bf 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Slideshow.java @@ -56,7 +56,6 @@ public class Slideshow extends Menu private final BiFunction description; private final boolean showPageNumbers; private final List urls; - private final Consumer finalAction; private final boolean waitOnSinglePage; private final int bulkSkipNumber; private final boolean wrapPageEnds; @@ -77,13 +76,12 @@ public class Slideshow extends Menu int bulkSkipNumber, boolean wrapPageEnds, String leftText, String rightText, boolean allowTextInput) { - super(waiter, users, roles, timeout, unit); + super(waiter, users, roles, timeout, unit, finalAction); this.color = color; this.text = text; this.description = description; this.showPageNumbers = showPageNumbers; this.urls = items; - this.finalAction = finalAction; this.waitOnSinglePage = waitOnSinglePage; this.bulkSkipNumber = bulkSkipNumber; this.wrapPageEnds = wrapPageEnds; @@ -179,7 +177,7 @@ else if(waitOnSinglePage) } else { - callFinalAction(finalAction); + finalizeMenu(); } }); } @@ -250,7 +248,7 @@ else if(rightText != null && rawContent.equalsIgnoreCase(rightText) && (pageNum getAttachedMessage().editMessage(renderPage(targetPage)).queue(m -> pagination(targetPage)); mre.getMessage().delete().queue(v -> {}, t -> {}); // delete the calling message so it doesn't get spammy } - }, timeout, unit, () -> callFinalAction(finalAction))); + }, timeout, unit, this::finalizeMenu)); } private void paginationWithoutTextInput(int pageNum) @@ -258,7 +256,7 @@ private void paginationWithoutTextInput(int pageNum) setCancelFuture(waiter.waitForEvent(MessageReactionAddEvent.class, event -> checkReaction(event), event -> handleMessageReactionAddAction(event, pageNum), - timeout, unit, () -> callFinalAction(finalAction))); + timeout, unit, this::finalizeMenu)); } // Private method that checks MessageReactionAddEvents @@ -325,7 +323,7 @@ private void handleMessageReactionAddAction(MessageReactionAddEvent event, int p } break; case STOP: - callFinalAction(finalAction); + finalizeMenu(); return; } From 064cf8ffb979244b78dbc192406c8ced292b682c Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Mon, 13 Aug 2018 12:59:10 +0200 Subject: [PATCH 3/6] Add some docs to menu changes --- .../commons/waiter/EventWaiter.java | 10 +++- .../com/jagrosh/jdautilities/menu/Menu.java | 46 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java index d019177f..f7a07391 100644 --- a/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java +++ b/commons/src/main/java/com/jagrosh/jdautilities/commons/waiter/EventWaiter.java @@ -148,6 +148,9 @@ public boolean isShutdown() * @param action * The Consumer to perform an action when the condition Predicate returns {@code true}. Never null. * + * @return Future for canceling the waiting. + * Using {@code Future#cancel(boolean)} with {@code mayInterruptIfRunning} set to {@code true} is not supported. + * * @throws IllegalArgumentException * One of two reasons: *

      @@ -189,6 +192,9 @@ public Future waitForEvent(Class classType, Predicate * The Runnable to run if the time runs out before a correct Event is thrown, or * {@code null} if there is no action on timeout. * + * @return Future for canceling the waiting. This will call the timeoutAction if provided. + * Using {@code Future#cancel(boolean)} with {@code mayInterruptIfRunning} set to {@code true} is not supported. + * * @throws IllegalArgumentException * One of two reasons: *
        @@ -308,11 +314,13 @@ private class WaitingFuture extends CompletableFuture @Override public boolean cancel(boolean mayInterruptIfRunning) { + if(mayInterruptIfRunning) + throw new UnsupportedOperationException("EventWaiter#waitForEvent can not be cancelled with mayInterruptIfRunning set to true"); if(isDone() || isCancelled()) return isCancelled(); if(waitingEvents.get(classType).remove(WaitingEvent.this) && cancelAction != null) cancelAction.run(); - return super.cancel(mayInterruptIfRunning); + return super.cancel(false); } } } diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java index c62ec320..e3629429 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java @@ -96,11 +96,22 @@ protected Menu(EventWaiter waiter, Set users, Set roles, long timeou */ public abstract void display(Message message); + /** + * Returns the message, this menu was attached to. + * This is set asynchronously after a call to a {@code display()} or equivalent method. + *
        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) @@ -109,26 +120,61 @@ public void cancel() 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 cancelFuture) { this.cancelFuture = cancelFuture; } + /** + * Returns the finalAction given via constructor. + * + * @return The finalAction given via constructor. + */ protected final Consumer getFinalAction() { return finalAction; } + /** + * Calls the final action and cleans up the attached message and cancelFuture (sets them to {@code null}). + *
        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). + * + *

        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. + *
        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; From 47ca0922c93904ff1f9a59c70d030b8e5417276e Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Thu, 16 Aug 2018 14:55:44 +0200 Subject: [PATCH 4/6] Fix PaginatorBuilder#setText() default and setter --- .../main/java/com/jagrosh/jdautilities/menu/Paginator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java index 7c3a7895..4f01729e 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Paginator.java @@ -399,7 +399,7 @@ private Message renderPage(int pageNum) public static class Builder extends Menu.Builder { private BiFunction color = (page, pages) -> null; - private BiFunction text = (page, pages) -> null; + private BiFunction text = null; private Consumer finalAction = m -> m.delete().queue(); private int columns = 1; private int itemsPerPage = 12; @@ -483,7 +483,7 @@ public Builder setColor(BiFunction colorBiFunction) */ public Builder setText(String text) { - this.text = (i0, i1) -> text; + this.text = text == null ? null : (i0, i1) -> text; return this; } From 8b51c5a51644a449fb93d064a740a7251c3b299f Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Mon, 20 Aug 2018 13:23:03 +0200 Subject: [PATCH 5/6] Add back compatible Menu constructor --- menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java index e3629429..e02e7eef 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java @@ -67,6 +67,11 @@ public abstract class Menu private Future cancelFuture; private Message attachedMessage; + protected Menu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit) + { + this(waiter, users, roles, timeout, unit, null); + } + protected Menu(EventWaiter waiter, Set users, Set roles, long timeout, TimeUnit unit, Consumer finalAction) { this.waiter = waiter; From d59da849e89db58ace9f4d4ec34306490553733f Mon Sep 17 00:00:00 2001 From: Michael Ritter Date: Mon, 20 Aug 2018 14:00:18 +0200 Subject: [PATCH 6/6] Improve Menu docs, exception Message --- .../com/jagrosh/jdautilities/menu/Menu.java | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java index e02e7eef..3e321e28 100644 --- a/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java +++ b/menu/src/main/java/com/jagrosh/jdautilities/menu/Menu.java @@ -40,6 +40,15 @@ * the assistance of things such as {@link net.dv8tion.jda.core.entities.MessageReaction reactions}, * but the actual implementation is only limited to the events provided by Discord and handled through JDA. * + *

        Implementations should all use the stateful system consisting of the Constructor with finalAction parameter + * and the three methods {@link #setAttachedMessage(Message) setAttachedMessage(Message)}, + * {@link #setCancelFuture(Future) setCancelFuture(Future)} + * and {@link #finalizeMenu() finalizeMenu()} (or {@link #finalizeMenu(boolean) finalizeMenu(boolean)} if needed). + *
        By properly calling those methods as described in their docs, the Menu can be cancelled externaly by calling + * {@link #cancel() Menu#cancel()}. + *
        This system is not mandatory, as that would break backwards compatibility with older Menu implementations, + * but it should always be used by new ones. + * *

        For custom implementations, readability of creating and integrating may be improved * by the implementation of a companion builder may be helpful (see the documentation on * {@link Menu.Builder Menu.Builder} for more info). @@ -116,11 +125,15 @@ public Message getAttachedMessage() /** * 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. + * + * @throws IllegalStateException + * If the Menu was not yet displayed, already cancelled (includes timeout or other menu ends) + * or cancel functionality is not supported by the custom implementation */ public void cancel() { if(cancelFuture == null) - throw new IllegalStateException("Menu was not yet displayed or is already cancelled"); + throw new IllegalStateException("Menu can not be cancelled (not yet displayed, already ended or not supported by custom implementation)"); cancelFuture.cancel(false); cancelFuture = null; } @@ -139,7 +152,7 @@ protected final void setAttachedMessage(Message 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. + * Should be used by the corresponding Menu implementation as soon as {@code EventWaiter#waitForEvent} was used (with its return value). * * @param cancelFuture * The cancel Future returned from {@code EventWaiter#waitForEvent} @@ -178,7 +191,10 @@ protected void finalizeMenu() *
        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() + * @param callFinalAction + * Whether or not the final action should be called. + * + * @see #finalizeMenu() */ protected void finalizeMenu(boolean callFinalAction) {