Change the way region setting works.

The region now contains two location references per region point for
the bounding boxes where p1, p2, l1 and l2 remain the same (min/max),
but also additional locations for the player's actual location, such
that e.g. moving back and re-setting a point has the more intuitive
behavior of expanding regardless of the optimized points.

This also (hopefully) finally fixes the previous issues with the setup
process *knock on wood*.

resources/plugin.yml

@@ -1,7 +1,7 @@
 name: MobArena
 author: garbagemule
 main: com.garbagemule.MobArena.MobArena
-version: 0.95.2.1
+version: 0.95.2.2
 softdepend: [Spout,Towny,Heroes,MagicSpells,Vault]
 commands:
     ma:

src/com/garbagemule/MobArena/ArenaImpl.java

@@ -188,6 +188,7 @@ public class ArenaImpl implements Arena
     @Override
     public void setWorld(World world) {
         this.world = world;
+        if (region != null) region.refreshWorld();
     }
 
     @Override

src/com/garbagemule/MobArena/commands/setup/AddSpawnpointCommand.java

@@ -1,5 +1,7 @@
 package com.garbagemule.MobArena.commands.setup;
 
+import com.garbagemule.MobArena.framework.Arena;
+import org.bukkit.World;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
 
@@ -34,17 +36,39 @@ public class AddSpawnpointCommand implements Command
             return true;
         }
 
+        // Make a world check first
+        Arena arena = am.getSelectedArena();
+        World aw = arena.getWorld();
+        World pw = p.getLocation().getWorld();
+        boolean changeWorld = !aw.getName().equals(pw.getName());
+
+        // Change worlds to make sure the region check doesn't fail
+        if (changeWorld) arena.setWorld(pw);
+        
         // Make sure we're inside the region
-        if (am.getSelectedArena().getRegion().contains(p.getLocation())) {
+        if (!am.getSelectedArena().getRegion().contains(p.getLocation())) {
-            am.getSelectedArena().getRegion().addSpawn(arg1, p.getLocation());
+            if (arena.getRegion().isDefined()) {
-            Messenger.tellPlayer(sender, "Spawnpoint " + arg1 + " added for arena \"" + am.getSelectedArena().configName() + "\"");
-        } else {
-            // If not, make sure the region is defined
-            if (am.getSelectedArena().getRegion().isDefined()) {
                 Messenger.tellPlayer(sender, "You must be inside the arena region!");
             } else {
                 Messenger.tellPlayer(sender, "You must first set the region points p1 and p2");
             }
+
+            // Restore the world reference in the arena 
+            if (changeWorld) arena.setWorld(aw);
+        } else {
+            // Add the spawnpoint
+            am.getSelectedArena().getRegion().addSpawn(arg1, p.getLocation());
+
+            // Notify the player if world changed
+            if (changeWorld) {
+                Messenger.tellPlayer(sender, "Changed world of arena '" + arena.configName() +
+                        "' from '" + aw.getName() +
+                        "' to '" + pw.getName() + "'");
+            }
+            
+            // Then notify about point set
+            Messenger.tellPlayer(sender, "Spawnpoint '" + arg1 + "' added for arena '" + am.getSelectedArena().configName() + "'");
+            arena.getRegion().checkData(am.getPlugin(), sender, false, false, false, true);
         }
         return true;
     }

src/com/garbagemule/MobArena/commands/setup/CheckDataCommand.java

@@ -27,7 +27,7 @@ public class CheckDataCommand implements Command
             return false;
         }
         
-        arena.getRegion().checkData(am.getPlugin(), sender);
+        arena.getRegion().checkData(am.getPlugin(), sender, true, true, true, true);
         return true;
     }
 }

src/com/garbagemule/MobArena/commands/setup/SetRegionCommand.java

@@ -36,6 +36,7 @@ public class SetRegionCommand implements Command
         
         am.getSelectedArena().getRegion().set(arg1, p.getLocation());
         Messenger.tellPlayer(sender, "Region point " + arg1 + " for arena '" + am.getSelectedArena().configName() + "' set.");
+        am.getSelectedArena().getRegion().checkData(am.getPlugin(), sender, true, true, false, false);
         return true;
     }
 }

src/com/garbagemule/MobArena/commands/setup/SetWarpCommand.java

@@ -1,5 +1,7 @@
 package com.garbagemule.MobArena.commands.setup;
 
+import com.garbagemule.MobArena.framework.Arena;
+import org.bukkit.World;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
 
@@ -34,17 +36,39 @@ public class SetWarpCommand implements Command
             return true;
         }
         
+        // Make a world check first
+        Arena arena = am.getSelectedArena();
+        World aw = arena.getWorld();
+        World pw = p.getLocation().getWorld();
+        boolean changeWorld = !aw.getName().equals(pw.getName());
+        
+        // Change worlds to make sure the region check doesn't fail
+        if (changeWorld) arena.setWorld(pw);
+        
         // Make sure the arena warp is inside the region
-        if (arg1.equals("arena") && !am.getSelectedArena().getRegion().contains(p.getLocation())) {
+        if (arg1.equals("arena") && !arena.getRegion().contains(p.getLocation())) {
-            if (am.getSelectedArena().getRegion().isDefined()) {
+            if (arena.getRegion().isDefined()) {
                 Messenger.tellPlayer(sender, "You must be inside the arena region!");
             } else {
                 Messenger.tellPlayer(sender, "You must first set the region points p1 and p2");
             }
+            
+            // Restore the world reference in the arena 
+            if (changeWorld) arena.setWorld(aw);
         } else {
-            am.getSelectedArena().getRegion().set(arg1, p.getLocation());
+            // Set the region point
-            Messenger.tellPlayer(sender, "Warp point " + arg1 + " was set for arena '" + am.getSelectedArena().configName() + "'");
+            arena.getRegion().set(arg1, p.getLocation());
-            Messenger.tellPlayer(sender, "Type /ma checkdata to see if you're missing anything...");
+            
+            // Notify the player if world changed
+            if (changeWorld) {
+                Messenger.tellPlayer(sender, "Changed world of arena '" + arena.configName() +
+                        "' from '" + aw.getName() +
+                        "' to '" + pw.getName() + "'");
+            }
+            
+            // Then notify about point set
+            Messenger.tellPlayer(sender, "Warp point '" + arg1 + "' was set for arena '" + am.getSelectedArena().configName() + "'");
+            arena.getRegion().checkData(am.getPlugin(), sender, true, false, true, false);
         }
         return true;
     }

src/com/garbagemule/MobArena/commands/setup/ShowRegionCommand.java

@@ -52,6 +52,14 @@ public class ShowRegionCommand implements Command
             }
         }
         
+        // Show an error message if we aren't in the right world
+        if (!arena.getWorld().getName().equals(arena.getWorld().getName())) {
+            Messenger.tellPlayer(sender, "Arena '" + arena.configName() +
+                    "' is in world '" + arena.getWorld().getName() +
+                    "' and you are in world '" + p.getWorld().getName() + "'");
+            return false;
+        }
+        
         arena.getRegion().showRegion(p);
         
         return true;

src/com/garbagemule/MobArena/region/ArenaRegion.java

@@ -7,6 +7,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import com.garbagemule.MobArena.MAUtils;
 import com.garbagemule.MobArena.util.Enums;
 import org.bukkit.ChatColor;
 import org.bukkit.Chunk;
@@ -27,6 +28,7 @@ public class ArenaRegion
     private Arena arena;
     private World world;
     
+    private Location lastP1, lastP2, lastL1, lastL2;
     private Location p1, p2, l1, l2, arenaWarp, lobbyWarp, specWarp, leaderboard;
     private Map<String,Location> spawnpoints, containers;
     
@@ -38,7 +40,7 @@ public class ArenaRegion
     
     public ArenaRegion(ConfigSection coords, Arena arena) {
         this.arena  = arena;
-        this.world  = arena.getWorld();
+        refreshWorld();
         
         this.coords = coords;
         this.spawns = coords.getConfigSection("spawnpoints");
@@ -47,6 +49,10 @@ public class ArenaRegion
         reloadAll();
     }
     
+    public void refreshWorld() {
+        this.world = arena.getWorld();
+    }
+    
     public void reloadAll() {
         reloadRegion();
         reloadWarps();
@@ -60,11 +66,11 @@ public class ArenaRegion
     public void reloadRegion() {
         p1 = coords.getLocation("p1", world);
         p2 = coords.getLocation("p2", world);
-        fixRegion();
+        //fixRegion();
         
         l1 = coords.getLocation("l1", world);
         l2 = coords.getLocation("l2", world);
-        fixLobbyRegion();
+        //fixLobbyRegion();
     }
     
     public void reloadWarps() {
@@ -111,23 +117,45 @@ public class ArenaRegion
                       l2 != null);
     }
     
-    public void checkData(MobArena plugin, CommandSender s) {
+    public void checkData(MobArena plugin, CommandSender s, boolean ready, boolean region, boolean warps, boolean spawns) {
+        // Verify data first
         verifyData();
         
-        if (arenaWarp == null)
+        // Prepare the list
-            Messenger.tellPlayer(s, "Missing warp: arena");
+        List<String> list = new ArrayList<String>();
-        if (lobbyWarp == null)
+
-            Messenger.tellPlayer(s, "Missing warp: lobby");
+        // Region points
-        if (specWarp == null)
+        if (region) {
-            Messenger.tellPlayer(s, "Missing warp: spectator");
+            if (p1 == null) list.add("p1");
-        if (p1 == null)
+            if (p2 == null) list.add("p2");
-            Messenger.tellPlayer(s, "Missing region point: p1");
+            if (!list.isEmpty()) {
-        if (p2 == null)
+                Messenger.tellPlayer(s, "Missing region points: " + MAUtils.listToString(list, plugin));
-            Messenger.tellPlayer(s, "Missing region point: p2");
+                list.clear();
-        if (spawnpoints.isEmpty())
+            }
-            Messenger.tellPlayer(s, "Missing spawnpoints");
+        }
-        if (setup)
+        
+        // Warps
+        if (warps) {
+            if (arenaWarp == null) list.add("arena");
+            if (lobbyWarp == null) list.add("lobby");
+            if (specWarp  == null) list.add("spectator");
+            if (!list.isEmpty()) {
+                Messenger.tellPlayer(s, "Missing warps: " + MAUtils.listToString(list, plugin));
+                list.clear();
+            }
+        }
+        
+        // Spawnpoints
+        if (spawns) {
+            if (spawnpoints.isEmpty()) {
+                Messenger.tellPlayer(s, "Missing spawnpoints");
+            }
+        }
+        
+        // Ready?
+        if (ready && setup) {
             Messenger.tellPlayer(s, "Arena is ready to be used!");
+        }
     }
     
     public boolean isDefined() {
@@ -359,17 +387,101 @@ public class ArenaRegion
     public void set(RegionPoint point, Location loc) {
         // Act based on the point
         switch (point) {
-            case P1: setP1(loc); return;
+            case P1:
-            case P2: setP2(loc); return;
+            case P2:
+            case L1:
+            case L2: setPoint(point, loc); return;
             case ARENA:
             case LOBBY:
-            case SPECTATOR: setWarp(point, loc); return;
+            case SPECTATOR:   setWarp(point, loc); return;
             case LEADERBOARD: setLeaderboard(loc); return;
         }
         
         throw new IllegalArgumentException("Invalid region point!");
     }
     
+    private void setPoint(RegionPoint point, Location l) {
+        // Lower and upper locations
+        RegionPoint r1, r2;
+        Location lower, upper;
+
+        /* Initialize the bounds.
+         *
+         * To allow users to set a region point without paying attention to
+         * the 'fixed' points, we continuously store the previously stored
+         * location for the given point. These location references are only
+         * ever overwritten when using the set commands, and remain fully
+         * decoupled from the 'fixed' points.
+         * 
+         * Effectively, the config-file and region store 'fixed' locations
+         * that allow fast membership tests, but the region also stores the
+         * 'unfixed' locations for a more intuitive setup process.
+         */
+        switch (point) {
+            case P1:
+                lastP1 = l.clone();
+                lower = lastP1.clone();
+                upper = (lastP2 != null ? lastP2.clone() : p2);
+                r1 = RegionPoint.P1; r2 = RegionPoint.P2;
+                break;
+            case P2:
+                lastP2 = l.clone();
+                lower = (lastP1 != null ? lastP1.clone() : p1);
+                upper = lastP2.clone();
+                r1 = RegionPoint.P1; r2 = RegionPoint.P2;
+                break;
+            case L1:
+                lastL1 = l.clone();
+                lower = lastL1.clone();
+                upper = (lastL2 != null ? lastL2.clone() : l2);
+                r1 = RegionPoint.L1; r2 = RegionPoint.L2;
+                break;
+            case L2:
+                lastL2 = l.clone();
+                lower = (lastL1 != null ? lastL1.clone() : l1);
+                upper = lastL2.clone();
+                r1 = RegionPoint.L1; r2 = RegionPoint.L2;
+                break;
+            default:
+                lower = upper = null;
+                r1    = r2    = null;
+        }
+        
+        // Grab the far corner
+        
+        // Min-max if both locations are non-null
+        if (lower != null && upper != null) {
+            double tmp;
+            if (lower.getX() > upper.getX()) {
+                System.out.println("Swapping x values " + lower.getX() + " and " + upper.getX());
+                tmp = lower.getX();
+                lower.setX(upper.getX());
+                upper.setX(tmp);
+            }
+            if (lower.getY() > upper.getY()) {
+                System.out.println("Swapping y values " + lower.getY() + " and " + upper.getY());
+                tmp = lower.getY();
+                lower.setY(upper.getY());
+                upper.setY(tmp);
+            }
+            if (lower.getZ() > upper.getZ()) {
+                System.out.println("Swapping z values " + lower.getZ() + " and " + upper.getZ());
+                tmp = lower.getZ();
+                lower.setZ(upper.getZ());
+                upper.setZ(tmp);
+            }
+        }
+        
+        // Set the coords and save
+        if (lower != null) coords.set(r1.name().toLowerCase(), lower);
+        if (upper != null) coords.set(r2.name().toLowerCase(), upper);
+        save();
+        
+        // Reload regions and verify data
+        reloadRegion();
+        verifyData();
+    }
+    
     public void set(String point, Location loc) {
         // Get the region point enum
         RegionPoint rp = Enums.getEnumFromString(RegionPoint.class, point);
@@ -379,84 +491,6 @@ public class ArenaRegion
         set(rp, loc);
     }
     
-    public void setP1(Location l) {
-        if (p2 != null) {
-            boolean modified = false;
-            
-            if (p2.getX() < l.getX()) {
-                double tmp = p2.getX();
-                p2.setX(l.getX());
-                l.setX(tmp);
-                modified = true;
-            }
-            if (p2.getZ() < l.getZ()) {
-                double tmp = p2.getZ();
-                p2.setZ(l.getZ());
-                l.setZ(tmp);
-                modified = true;
-            }
-            if (p2.getY() < l.getY()) {
-                double tmp = p2.getY();
-                p2.setY(l.getY());
-                l.setY(tmp);
-                modified = true;
-            }
-    
-            // If we made modifications, re-save P2
-            if (modified) {
-                coords.set("p2", p2);
-            }
-        }
-        
-        // Set P1 and save
-        coords.set("p1", l);
-        save();
-        
-        // Then reload the region
-        reloadRegion();
-        verifyData();
-    }
-
-    public void setP2(Location l) {
-        if (p1 != null) {
-            boolean modified = false;
-            
-            if (l.getX() < p1.getX()) {
-                double tmp = p1.getX();
-                p1.setX(l.getX());
-                l.setX(tmp);
-                modified = true;
-            }
-    
-            if (l.getZ() < p1.getZ()) {
-                double tmp = p1.getZ();
-                p1.setZ(l.getZ());
-                l.setZ(tmp);
-                modified = true;
-            }
-    
-            if (l.getY() < p1.getY()) {
-                double tmp = p1.getY();
-                p1.setY(l.getY());
-                l.setY(tmp);
-                modified = true;
-            }
-    
-            // If we made modifications, re-save P1
-            if (modified) {
-                coords.set("p1", p1);
-            }
-        }
-
-        // Set P2 and save
-        coords.set("p2", l);
-        save();
-
-        // Then reload the region
-        reloadRegion();
-        verifyData();
-    }
-    
     public void setWarp(RegionPoint point, Location l) {
         // Set the point and save
         coords.set(point.toString(), l);