Fixes comments for the portal activator

src/main/java/net/knarcraft/stargate/portal/PortalActivator.java

@@ -11,46 +11,47 @@ import java.util.List;
 import java.util.Random;
 
 /**
- * The portal destinations contain information about a portal's destinations, and is responsible for cycling destinations
+ * The portal activator activates/de-activates portals and keeps track of a portal's destinations
  *
- * <p>The activator is responsible for activating/de-activating the portal and contains information about
+ * <p>The portal activator is responsible for activating/de-activating the portal and contains information about
  * available destinations and which player activated the portal.</p>
  */
 public class PortalActivator {
 
+    private final Portal portal;
+    private final PortalOpener opener;
+
+    private List<String> destinations = new ArrayList<>();
     private String destination;
     private String lastDestination = "";
-    private List<String> destinations = new ArrayList<>();
-    private final Portal portal;
-    private final PortalOpener activator;
     private Player activePlayer;
 
     /**
      * Instantiates a new portal destinations object
      *
-     * @param portal      <p>The portal which this this object stores destinations for</p>
+     * @param portal       <p>The portal which this this object stores destinations for</p>
-     * @param activator   <p>The activator to use when a player activates a portal</p>
+     * @param portalOpener <p>The portal opener to trigger when the activation causes the portal to open</p>
-     * @param destination <p></p>
+     * @param destination  <p>The fixed destination specified on the portal's sign</p>
      */
-    public PortalActivator(Portal portal, PortalOpener activator, String destination) {
+    public PortalActivator(Portal portal, PortalOpener portalOpener, String destination) {
         this.portal = portal;
-        this.activator = activator;
+        this.opener = portalOpener;
         this.destination = destination;
     }
 
     /**
-     * Gets the player currently using this portal activator's portal
+     * Gets the player which this activator's portal is currently activated for
      *
-     * @return <p>The player currently using this portal activator's portal</p>
+     * @return <p>The player this activator's portal is currently activated for</p>
      */
     public Player getActivePlayer() {
         return activePlayer;
     }
 
     /**
-     * Gets the destinations of this portal
+     * Gets the available portal destinations
      *
-     * @return <p>The destinations of this portal</p>
+     * @return <p>The available portal destinations</p>
      */
     public List<String> getDestinations() {
         return new ArrayList<>(this.destinations);
@@ -63,44 +64,112 @@ public class PortalActivator {
      * @return <p>The destination portal the player should teleport to</p>
      */
     public Portal getDestination(Player player) {
+        String portalNetwork = portal.getNetwork();
         if (portal.getOptions().isRandom()) {
-            destinations = PortalHandler.getDestinations(portal, player, portal.getNetwork());
+            //Find possible destinations
+            List<String> destinations = PortalHandler.getDestinations(portal, player, portalNetwork);
             if (destinations.size() == 0) {
                 return null;
             }
+            //Get one random destination
             String destination = destinations.get((new Random()).nextInt(destinations.size()));
-            destinations.clear();
+            return PortalHandler.getByName(destination, portalNetwork);
-            return PortalHandler.getByName(destination, portal.getNetwork());
+        } else {
+            //Just return the normal fixed destination
+            return PortalHandler.getByName(destination, portalNetwork);
         }
-        return PortalHandler.getByName(destination, portal.getNetwork());
     }
 
     /**
-     * Activates this portal for the given player
+     * Gets the portal's destination
+     *
+     * <p>For random portals, getDestination must be given a player to decide which destinations are valid. Without a
+     * player, or with a null player, behavior is only defined for a non-random gate.</p>
+     *
+     * @return <p>The portal destination</p>
+     */
+    public Portal getDestination() {
+        return getDestination(null);
+    }
+
+    /**
+     * Sets the destination of this portal activator's portal
+     *
+     * @param destination <p>The new destination of this portal activator's portal</p>
+     */
+    public void setDestination(Portal destination) {
+        setDestination(destination.getName());
+    }
+
+    /**
+     * Sets the destination of this portal activator's portal
+     *
+     * @param destination <p>The new destination of this portal activator's portal</p>
+     */
+    public void setDestination(String destination) {
+        this.destination = destination;
+    }
+
+    /**
+     * Gets the name of the selected destination
+     *
+     * @return <p>The name of the selected destination</p>
+     */
+    public String getDestinationName() {
+        return destination;
+    }
+
+    /**
+     * Activates this activator's portal for the given player
      *
      * @param player <p>The player to activate the portal for</p>
      * @return <p>True if the portal was activated</p>
      */
     boolean activate(Player player) {
+        //Clear previous destination data
         this.destination = "";
         this.destinations.clear();
+
+        //Adds the active gate to the active queue to allow it to be remotely deactivated
         Stargate.activePortalsQueue.add(portal);
+
+        //Set the given player as the active player
         activePlayer = player;
+
         String network = portal.getNetwork();
         destinations = PortalHandler.getDestinations(portal, player, network);
+
+        //Sort destinations if enabled
         if (Stargate.sortNetworkDestinations) {
             Collections.sort(destinations);
         }
+
+        //Select last used destination if remember destination is enabled
         if (Stargate.rememberDestination && !lastDestination.isEmpty() && destinations.contains(lastDestination)) {
             destination = lastDestination;
         }
 
+        //Trigger an activation event to allow the cancellation to be cancelled
+        return triggerStargateActivationEvent(player);
+    }
+
+    /**
+     * Triggers a stargate activation event to allow other plugins to cancel the activation
+     *
+     * <p>The event may also end up changing destinations.</p>
+     *
+     * @param player <p>The player trying to activate this activator's portal</p>
+     * @return <p>True if the portal was activated. False otherwise</p>
+     */
+    private boolean triggerStargateActivationEvent(Player player) {
         StargateActivateEvent event = new StargateActivateEvent(portal, player, destinations, destination);
         Stargate.server.getPluginManager().callEvent(event);
         if (event.isCancelled()) {
             Stargate.activePortalsQueue.remove(portal);
             return false;
         }
+
+        //Update destinations in case they changed, and update the sign
         destination = event.getDestination();
         destinations = event.getDestinations();
         portal.drawSign();
@@ -111,16 +180,24 @@ public class PortalActivator {
      * Deactivates this portal
      */
     public void deactivate() {
+        //Trigger a stargate deactivate event to allow other plugins to cancel the event
         StargateDeactivateEvent event = new StargateDeactivateEvent(portal);
         Stargate.server.getPluginManager().callEvent(event);
         if (event.isCancelled()) {
             return;
         }
 
+        //Un-mark the portal as activated
         Stargate.activePortalsQueue.remove(portal);
+
+        //For a fixed gate, the destinations and the sign never really change, but at the same time, fixed gates are
+        // never really activated, so in theory, this check should be redundant.
+        //TODO: Decide if this check is really useless
         if (portal.getOptions().isFixed()) {
             return;
         }
+
+        //Clear destinations and the active player before re-drawing the sign to show that it's deactivated
         destinations.clear();
         destination = "";
         activePlayer = null;
@@ -128,54 +205,16 @@ public class PortalActivator {
     }
 
     /**
-     * Gets whether this portal is active
+     * Gets whether this portal activator's portal is active
      *
-     * @return <p>Whether this portal is active</p>
+     * @return <p>Whether this portal activator's portal is active</p>
      */
     public boolean isActive() {
         return portal.getOptions().isFixed() || (destinations.size() > 0);
     }
 
     /**
-     * Gets the portal destination
+     * Cycles destination for a non-fixed gate by one forwards step
-     *
-     * <p>If this portal is random, a player should be given to get correct destinations.</p>
-     *
-     * @return <p>The portal destination</p>
-     */
-    public Portal getDestination() {
-        return getDestination(null);
-    }
-
-    /**
-     * Sets the destination of this portal
-     *
-     * @param destination <p>The new destination of this portal</p>
-     */
-    public void setDestination(Portal destination) {
-        setDestination(destination.getName());
-    }
-
-    /**
-     * Sets the destination of this portal
-     *
-     * @param destination <p>The new destination of this portal</p>
-     */
-    public void setDestination(String destination) {
-        this.destination = destination;
-    }
-
-    /**
-     * Gets the name of the destination of this portal
-     *
-     * @return <p>The name of this portal's destination</p>
-     */
-    public String getDestinationName() {
-        return destination;
-    }
-
-    /**
-     * Cycles destination for a network gate forwards
      *
      * @param player <p>The player to cycle the gate for</p>
      */
@@ -184,37 +223,44 @@ public class PortalActivator {
     }
 
     /**
-     * Cycles destination for a network gate
+     * Cycles destination for a non-fixed gate
      *
      * @param player    <p>The player cycling destinations</p>
      * @param direction <p>The direction of the cycle (+1 for next, -1 for previous)</p>
      */
     public void cycleDestination(Player player, int direction) {
+        //Only allow going exactly one step in either direction
         if (direction != 1 && direction != -1) {
             throw new IllegalArgumentException("The destination direction must be 1 or -1.");
         }
 
         boolean activate = false;
         if (!isActive() || getActivePlayer() != player) {
-            //If the stargate activate event is cancelled, return
+            //If not active or not active for the given player, and the activation is denied, just abort
             if (!activate(player)) {
                 return;
             }
+            activate = true;
+
             Stargate.debug("cycleDestination", "Network Size: " +
                     PortalHandler.getNetwork(portal.getNetwork()).size());
             Stargate.debug("cycleDestination", "Player has access to: " + destinations.size());
-            activate = true;
         }
 
+        //If no destinations are available, just tell the player and quit
         if (destinations.size() == 0) {
             Stargate.sendErrorMessage(player, Stargate.getString("destEmpty"));
             return;
         }
 
+        //Cycle if destination remembering is disabled, if the portal was already active, or it has no last destination
         if (!Stargate.rememberDestination || !activate || lastDestination.isEmpty()) {
             cycleDestination(direction);
         }
-        activator.setOpenTime(System.currentTimeMillis() / 1000);
+
+        //Update the activated time to allow it to be deactivated after a timeout, and re-draw the sign to show the
+        // selected destination
+        opener.setActivatedTime(System.currentTimeMillis() / 1000);
         portal.drawSign();
     }