cleaned up code, inlined senderHasPerm, condensed help command output

src/ca/gibstick/discosheep/BaaBaaBlockSheepEvents.java

@@ -1,6 +1,7 @@
 /*
- * To change this template, choose Tools | Templates
- * and open the template in the editor.
+ * BaaBaaBlockSheep have you any wool?
+ * Nope, event got cancelled.
+ * Also listens to other events, not just sheep events
  */
 package ca.gibstick.discosheep;
 
@@ -42,7 +43,7 @@ public class BaaBaaBlockSheepEvents implements Listener {
 			for (DiscoParty party : parent.getParties()) {
 				if (party.getSheep().contains((Sheep) e.getEntity())) {
 					{
-						party.jumpSheep((Sheep) e.getEntity());
+						party.jumpSheep((Sheep) e.getEntity()); // for kicks
 						e.setCancelled(true);
 					}
 				}
@@ -53,7 +54,7 @@ public class BaaBaaBlockSheepEvents implements Listener {
 	@EventHandler
 	public void onPlayerQuitEvent(PlayerQuitEvent e) {
 		String name = e.getPlayer().getName();
-
 		parent.stopParty(name);
+		// stop party on player quit or else it will CONTINUE FOR ETERNITY
 	}
 }

src/ca/gibstick/discosheep/DiscoParty.java

@@ -39,7 +39,7 @@ public class DiscoParty {
 	static int maxPeriod = 40;	// 2.0 seconds
 	private boolean doFireworks = false;
 	private boolean doJump = true;
-	private int state = 0;
+	private int state = 0; // basically our own tick system
 	private DiscoUpdater updater;
 	private static final DyeColor[] discoColours = {
 		DyeColor.RED,
@@ -75,11 +75,7 @@ public class DiscoParty {
 	}
 
 	// copy but with new player
-	/**
-	 *
-	 * @param player The new player to be stored
-	 * @return A copy of the class with the new player
-	 */
+	// used for /ds other and /ds all
 	public DiscoParty DiscoParty(Player player) {
 		DiscoParty newParty = new DiscoParty(this.ds, player);
 		newParty.setDoFireworks(this.doFireworks);
@@ -154,15 +150,16 @@ public class DiscoParty {
 			double z = player.getLocation().getZ();
 			double y;
 
-			// random point on circle with polar coordinates
-			double r = Math.sqrt(Math.random()) * sheepSpawnRadius; // sqrt for uniform distribution
+			/* random point on circle with polar coordinates
+			 * random number must be square rooted to obtain uniform distribution
+			 * otherwise the sheep are biased toward the centre */
+			double r = Math.sqrt(Math.random()) * sheepSpawnRadius;
 			double azimuth = Math.random() * 2 * Math.PI; // radians
 			x += r * Math.cos(azimuth);
 			z += r * Math.sin(azimuth);
-
 			y = world.getHighestBlockYAt((int) x, (int) z);
+
 			loc = new Location(world, x, y, z);
-			//loc.setPitch((float) ((180f / Math.PI) * Math.atan((loc.getX() - player.getLocation().getX()) / (loc.getZ() - player.getLocation().getZ()))));
 			loc.setPitch((float) Math.random() * 360 - 180);
 			loc.setYaw(0);
 			spawnSheep(world, loc);
@@ -172,15 +169,15 @@ public class DiscoParty {
 	void spawnSheep(World world, Location loc) {
 		Sheep newSheep = (Sheep) world.spawnEntity(loc, EntityType.SHEEP);
 		newSheep.setColor(discoColours[(int) (Math.random() * (discoColours.length - 1))]);
-		newSheep.setBreed(false);
-		newSheep.teleport(loc); // for sheep orientation
+		newSheep.setBreed(false); // this prevents breeding - no event listener required
+		newSheep.teleport(loc); // teleport is needed to set orientation
 		getSheep().add(newSheep);
 	}
 
 	// Mark all sheep in the sheep array for removal, then clear the array
 	void removeAllSheep() {
 		for (Sheep sheeple : getSheep()) {
-			sheeple.setHealth(0);
+			//sheeple.setHealth(0); // removed to make it more resilient to updates
 			sheeple.remove();
 		}
 		getSheep().clear();
@@ -198,6 +195,7 @@ public class DiscoParty {
 		sheep.setVelocity(newVel);
 	}
 
+	// WHY ISN'T THERE A Color.getValue() ?!?!?!?!
 	private Color getColor(int i) {
 		Color c = null;
 		if (i == 1) {
@@ -306,7 +304,7 @@ public class DiscoParty {
 
 		// set random effect and randomize power
 		meta.addEffect(effect.build());
-		meta.setPower(r.nextInt(2)+1);
+		meta.setPower(r.nextInt(2) + 1);
 
 		// apply it to the given firework
 		firework.setFireworkMeta(meta);
@@ -323,7 +321,11 @@ public class DiscoParty {
 			playSounds();
 			duration -= period;
 			this.scheduleUpdate();
-			this.state++;
+			if (state < 10000) {
+				this.state++;
+			} else {
+				state = 1; // prevent overflow
+			}
 		} else {
 			this.stopDisco();
 		}

src/ca/gibstick/discosheep/DiscoSheep.java

@@ -57,7 +57,7 @@ public final class DiscoSheep extends JavaPlugin {
 
 	@Override
 	public void onDisable() {
-		this.stopAllParties();
+		this.stopAllParties(); // or else the parties will continue FOREVER
 		this.config = null;
 	}
 
@@ -106,16 +106,4 @@ public final class DiscoSheep extends JavaPlugin {
 			this.getPartyMap().remove(name);
 		}
 	}
-
-	/*public void startParty(Player player, int duration, int sheepAmount, int radius, int period, boolean fireworksEnabled) {
-	 * if (!hasParty(player.getName())) {
-	 * DiscoParty ds = new DiscoParty(this, player);
-	 * ds.setDuration(duration);
-	 * ds.setSheep(sheepAmount);
-	 * ds.setRadius(radius);
-	 * ds.setPeriod(period);
-	 * ds.setDoFireworks(fireworksEnabled);
-	 * ds.startDisco();
-	 * }
-	 * }*/
 }

src/ca/gibstick/discosheep/DiscoSheepCommandExecutor.java

@@ -23,11 +23,6 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 	private static final String PERMISSION_OTHER = "discosheep.partyother";
 	private static final String PERMISSION_CHANGEPERIOD = "discosheep.changeperiod";
 
-	//private static final String DELIM = "[ ]+";
-	private boolean senderHasPerm(CommandSender sender, String permission) {
-		return sender.hasPermission(permission);
-	}
-
 	private boolean noPermsMessage(CommandSender sender, String permission) {
 		sender.sendMessage(ChatColor.RED + "You do not have the permission node " + ChatColor.GRAY + permission);
 		return false;
@@ -45,20 +40,24 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 			try {
 				return Integer.parseInt(args[i + 1]);
 			} catch (NumberFormatException e) {
-				return -1;
+				return -1; // so that it fails limit checks elsewhere
 			}
 		}
-		return -1;
+		return -1; // ibid
 	}
 
 	private Double parseNextDoubleArg(String[] args, int i) {
 		if (i < args.length - 1) {
-			return Double.parseDouble(args[i + 1]);
+			try {
+				return Double.parseDouble(args[i + 1]);
+			} catch (NumberFormatException e) {
+				return -1.0d; // so that it fais limit checks elsewhere
+			}
 		}
-		return -1.0d;
+		return -1.0d; // ibid
 	}
 
-	// extract a list of players from a list of arguments
+	// return portion of the array that contains the list of players
 	private String[] parsePlayerList(String[] args, int i) {
 		int j = i;
 		while (j < args.length && !args[j].startsWith("-")) {
@@ -71,10 +70,8 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 	private boolean helpCommand(CommandSender sender) {
 		sender.sendMessage(ChatColor.YELLOW + "DiscoSheep Help\n"
 				+ ChatColor.GRAY + "  Subcommands\n" + ChatColor.WHITE
-				+ "me: start a party for yourself\n"
-				+ "stop: stop your own party\n"
-				+ "all: start a party for all players on the server\n"
-				+ "stopall: stop all parties (takes no arguments)\n"
+				+ "me, stop, all, stopall\n"
+				+ "You do not need permission to use the \"stop\" command\n"
 				+ "other <players>: start a party for the space-delimited list of players\n"
 				+ ChatColor.GRAY + "  Arguments\n" + ChatColor.WHITE
 				+ "-n <integer>: set the number of sheep per player that spawn\n"
@@ -86,7 +83,7 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 	}
 
 	private boolean reloadCommand(CommandSender sender) {
-		if (senderHasPerm(sender, PERMISSION_RELOAD)) {
+		if (sender.hasPermission(PERMISSION_RELOAD)) {
 			parent.reloadConfigFromDisk();
 			sender.sendMessage(ChatColor.GREEN + "DiscoSheep config reloaded from disk");
 			return true;
@@ -96,12 +93,12 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 	}
 
 	private boolean partyCommand(Player player, DiscoParty party) {
-		if (senderHasPerm(player, PERMISSION_PARTY)) {
+		if (player.hasPermission(PERMISSION_PARTY)) {
 			if (!parent.hasParty(player.getName())) {
 				party.setPlayer(player);
 				party.startDisco();
 			} else {
-				player.sendMessage("You already have a party. Are you underground?");
+				player.sendMessage(ChatColor.RED + "You already have a party. Are you underground?");
 			}
 			return true;
 		} else {
@@ -110,7 +107,7 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 	}
 
 	private boolean partyAllCommand(CommandSender sender, DiscoParty party) {
-		if (senderHasPerm(sender, PERMISSION_ALL)) {
+		if (sender.hasPermission(PERMISSION_ALL)) {
 			for (Player p : Bukkit.getServer().getOnlinePlayers()) {
 				if (!parent.hasParty(p.getName())) {
 					DiscoParty individualParty = party.DiscoParty(p);
@@ -125,7 +122,7 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 	}
 
 	private boolean stopAllCommand(CommandSender sender) {
-		if (senderHasPerm(sender, PERMISSION_STOPALL)) {
+		if (sender.hasPermission(PERMISSION_STOPALL)) {
 			parent.stopAllParties();
 			return true;
 		} else {
@@ -139,7 +136,7 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 	}
 
 	private boolean partyOtherCommand(String[] players, CommandSender sender, DiscoParty party) {
-		if (senderHasPerm(sender, PERMISSION_OTHER)) {
+		if (sender.hasPermission(PERMISSION_OTHER)) {
 			Player p;
 			for (String playerName : players) {
 				p = Bukkit.getServer().getPlayer(playerName);
@@ -167,9 +164,11 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 		if (sender instanceof Player) {
 			player = (Player) sender;
 			isPlayer = true;
-		}
+		} // check isPlayer before "stop" and "me" commands
 
 		// check for commands that don't need a party
+		// so that we get them out of the way, and 
+		// prevent needless construction of parties
 		if (args.length == 1) {
 			if (args[0].equalsIgnoreCase("stopall")) {
 				return stopAllCommand(sender);
@@ -182,19 +181,20 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 			}
 		}
 
-		DiscoParty parentParty = new DiscoParty(parent);
+		// construct a main party; all other parties will copy from this
+		DiscoParty mainParty = new DiscoParty(parent);
 
+		// omg I love argument parsing and I know the best way!
 		for (int i = 1; i < args.length; i++) {
 			if (args[i].equalsIgnoreCase("-fw")) {
-				if (senderHasPerm(sender, PERMISSION_FIREWORKS)) {
-					parentParty.setDoFireworks(true);
+				if (sender.hasPermission(PERMISSION_FIREWORKS)) {
+					mainParty.setDoFireworks(true);
 				} else {
 					return noPermsMessage(sender, PERMISSION_FIREWORKS);
 				}
 			} else if (args[i].equalsIgnoreCase("-r")) {
 				try {
-					parentParty.setRadius(parseNextIntArg(args, i));
-					//sender.sendMessage("RADIUS OK");
+					mainParty.setRadius(parseNextIntArg(args, i));
 				} catch (IllegalArgumentException e) {
 					sender.sendMessage("Radius must be an integer within the range [1, "
 							+ DiscoParty.maxRadius + "]");
@@ -202,8 +202,7 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 				}
 			} else if (args[i].equalsIgnoreCase("-n")) {
 				try {
-					parentParty.setSheep(parseNextIntArg(args, i));
-					//sender.sendMessage("SHEEP OK");
+					mainParty.setSheep(parseNextIntArg(args, i));
 				} catch (IllegalArgumentException e) {
 					sender.sendMessage("The number of sheep must be an integer within the range [1, "
 							+ DiscoParty.maxSheep + "]");
@@ -211,20 +210,18 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 				}
 			} else if (args[i].equalsIgnoreCase("-t")) {
 				try {
-					parentParty.setDuration(parent.toTicks(parseNextIntArg(args, i)));
-					//sender.sendMessage("DURATION OK");
+					mainParty.setDuration(parent.toTicks(parseNextIntArg(args, i)));
 				} catch (IllegalArgumentException e) {
 					sender.sendMessage("The duration in seconds must be an integer within the range [1, "
 							+ parent.toSeconds(DiscoParty.maxDuration) + "]");
 					return false;
 				}
 			} else if (args[i].equalsIgnoreCase("-p")) {
-				if (!senderHasPerm(sender, PERMISSION_CHANGEPERIOD)) {
+				if (!sender.hasPermission(PERMISSION_CHANGEPERIOD)) {
 					return noPermsMessage(sender, PERMISSION_CHANGEPERIOD);
 				}
 				try {
-					parentParty.setPeriod(parseNextIntArg(args, i));
-					//sender.sendMessage("PERIOD OK");
+					mainParty.setPeriod(parseNextIntArg(args, i));
 				} catch (IllegalArgumentException e) {
 					sender.sendMessage(
 							"The period in ticks must be within the range ["
@@ -237,11 +234,11 @@ public class DiscoSheepCommandExecutor implements CommandExecutor {
 
 		if (args.length > 0) {
 			if (args[0].equalsIgnoreCase("all")) {
-				return partyAllCommand(sender, parentParty);
+				return partyAllCommand(sender, mainParty);
 			} else if (args[0].equalsIgnoreCase("me") && isPlayer) {
-				return partyCommand(player, parentParty);
+				return partyCommand(player, mainParty);
 			} else if (args[0].equalsIgnoreCase("other")) {
-				return partyOtherCommand(parsePlayerList(args, 1), sender, parentParty);
+				return partyOtherCommand(parsePlayerList(args, 1), sender, mainParty);
 			} else {
 				sender.sendMessage(ChatColor.RED + "Invalid argument (certain commands do not work from console).");
 				return false;

src/plugin.yml

@@ -10,6 +10,7 @@ commands:
         Use /ds help for more information
         To stop your party, use /ds stop.
 permissions:
+  # If default is set to false, console will not have permission!
   discosheep.*:
     description: Permission node for all DiscoSheep commands
     default: op