Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Region Claim #114

Closed
wants to merge 4 commits into from

3 participants

@nicklozon

Don't hurt me, my first pull request ever.

You may hit me over the nose with a rolled up news paper if need be.

@wizjany
Collaborator

Right, except that this isn't the point of the region claim command, hence it not doing that...

@nicklozon

Better retract your comment on the issue tracker and update the wiki...and delete the entire claim command because it does the same thing as /region define.

@wizjany
Collaborator

You should probably drop the attitude and double check your sources.

@nicklozon

No attitude here janie. I'm just saying that there is comments on the issue tracker (down btw for over 24 hrs) and a wiki page (looks slightly updated to make more sense) that depicts what the functionality should be.

Are you saying the claim command is to be what the define command is BUT not the ability to define an owner; only the player is the owner? This makes sense, but I think the code is still lacking owner reset and possibly redefining the selection.

General consensus from talking to others' is that this command would allow players to claim predefined regions.

Maybe the following should be updated to say that a selection is required rather than a "plot" of land:

http://wiki.sk89q.com/wiki/WorldGuard/Regions

Allows a regular user to claim a plot of land and be set as its areas owner. Users can only claim areas that don't overlap other people's areas and they cannot replace another existing area that they don't own. They redefine areas they own with this command. Re-claiming an existing owned area will clear its owner list.

@nicklozon nicklozon closed this
@wizjany
Collaborator

The wiki is severely out of date btw. And no where there does it say the claim command is to claim existing regions. I still stand by what I said on redmine (yea we know it's down) which was that the claim command is the limited, user-oriented version of the define command, which is unrestricted.

@nicklozon

Ya, makes sense now...after I made my changes.

I was thinking of a Towny-like capability. Maybe I will make a front-end that uses WorldGuard now that we know the functionality is to be separate.

@wizjany
Collaborator
@mrapple

/me gets the newspaper ready

@nicklozon

You never once explained the functionality, nor did you say directly "it is working as expected."

Anyhow, I've approached both you and sk89q and explained that I am an experienced professional developer, but I am just learning jUnit and this is my first experience with contributing to a GitHub project. I asked for your patience and professional feedback as I would be a great asset to your projects, that I admire.

You have been very unprofessional and rude. If you want professional developers to contribute to your projects, I advise you conduct yourself as such (professionally).

This will be my first and last push request for any of your projects; I will find somewhere else to donate my free time.

@mrapple

wizjany said " the claim command is the limited, user-oriented version of the define command, which is unrestricted."

Also, just for the record, I was joking :P

If you're looking for true professional conduct around bukkit, I doubt you'll find a ton, as most of us are in the 13-18 range. (and generally this age group doesn't have business level experience.) So if you do want to help out, well, good luck :P

Even the kid who runs bukkit.org is 18 iirc. (lukegb) not sure about evilseph.

One option would be to join the bukkit team and help out there, msg me on the forum if that sounds interesting. Username is MonsieurApple.

@nicklozon

I know you were kidding ;)

Wizjany's last comment seemed as if I were still confused on the subject, when I am very clear on the functionality...still rude nonetheless.

Thanks for the info, you may hear from me. I'm usually in IRC under the name Lozon or Lulzon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 20, 2011
  1. @nicklozon

    Updated to WorldEdit 4.6

    nicklozon authored
Commits on Jun 22, 2011
  1. @nicklozon

    Region Claim actually claims predefined regions.

    nicklozon authored
    If region already exists, it will wipe the owners' list except for the
    claimer.
  2. @nicklozon

    Updated pom.xml to WorldEdit 4.6 - was having some dependency issues

    nicklozon authored
    with 4.4.2.
    Added Mockito dependency for my unit testing.
  3. @nicklozon
This page is out of date. Refresh to see the latest.
View
9 pom.xml
@@ -35,7 +35,7 @@
<dependency>
<groupId>com.sk89q</groupId>
<artifactId>worldedit</artifactId>
- <version>4.4.2</version>
+ <version>4.6</version>
</dependency>
<!-- Bukkit -->
@@ -61,6 +61,13 @@
<artifactId>opencsv</artifactId>
<version>2.0</version>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-all</artifactId>
+ <version>1.8.5</version>
+ <type>jar</type>
+ <scope>compile</scope>
+ </dependency>
</dependencies>
<build>
View
116 src/main/java/com/sk89q/worldguard/bukkit/commands/RegionCommands.java 100755 → 100644
@@ -21,7 +21,10 @@
import java.io.IOException;
import java.util.Arrays;
+import java.util.Iterator;
import java.util.Map;
+import java.util.Set;
+
import org.bukkit.ChatColor;
import org.bukkit.World;
import org.bukkit.command.CommandSender;
@@ -185,84 +188,100 @@ public static void redefine(CommandContext args, WorldGuardPlugin plugin,
}
@Command(aliases = {"claim"},
- usage = "<id> [<owner1> [<owner2> [<owners...>]]]",
- desc = "Claim a region",
+ usage = "<id>",
+ desc = "Claim an existing region",
flags = "", min = 1, max = -1)
@CommandPermissions({"worldguard.region.claim"})
public static void claim(CommandContext args, WorldGuardPlugin plugin,
CommandSender sender) throws CommandException {
+ // Get player information
Player player = plugin.checkPlayer(sender);
LocalPlayer localPlayer = plugin.wrapPlayer(player);
- WorldEditPlugin worldEdit = plugin.getWorldEdit();
- String id = args.getString(0);
+ World world = player.getWorld();
- if (!ProtectedRegion.isValidId(id)) {
- throw new CommandException("Invalid region ID specified!");
- }
+ // Check the number of arguments
+ if(args.argsLength() != 1) {
+ throw new CommandException("Incorrect parameter syntax!");
+ }
+ String id = args.getString(0);
+ // Check that the region id is not __global__
if (id.equalsIgnoreCase("__global__")) {
throw new CommandException("A region cannot be named __global__");
}
- // Attempt to get the player's selection from WorldEdit
- Selection sel = worldEdit.getSelection(player);
+ // Get the region and region manager
+ RegionManager mgr = plugin.getGlobalRegionManager().get(world);
+ ProtectedRegion region = mgr.getRegion(id);
- if (sel == null) {
- throw new CommandException("Select a region with WorldEdit first.");
+ // Check if the region id supplied is a valid one
+ if(region == null) {
+ throw new CommandException("Could not find a region by that ID.");
}
- ProtectedRegion region;
-
- // Detect the type of region from WorldEdit
- if (sel instanceof Polygonal2DSelection) {
- Polygonal2DSelection polySel = (Polygonal2DSelection) sel;
- int minY = polySel.getNativeMinimumPoint().getBlockY();
- int maxY = polySel.getNativeMaximumPoint().getBlockY();
- region = new ProtectedPolygonalRegion(id, polySel.getNativePoints(), minY, maxY);
- } else if (sel instanceof CuboidSelection) {
- BlockVector min = sel.getNativeMinimumPoint().toBlockVector();
- BlockVector max = sel.getNativeMaximumPoint().toBlockVector();
- region = new ProtectedCuboidRegion(id, min, max);
- } else {
- throw new CommandException(
- "The type of region selected in WorldEdit is unsupported in WorldGuard!");
- }
+ // Get the world configuration
+ WorldConfiguration wcfg = plugin.getGlobalStateManager().get(player.getWorld());
- // Get the list of region owners
- if (args.argsLength() > 1) {
- region.setOwners(RegionUtil.parseDomainString(args.getSlice(1), 1));
+ // Check whether the region has an owner
+ DefaultDomain owners = region.getOwners();
+ if (owners.getPlayers().size() > 0) {
+ // Is the player an owner?
+ if(!owners.getPlayers().contains(player.getName())) {
+ throw new CommandException("This region already has an owner - you can't claim or redefine it.");
+ }
+
+ // Do they have a WorldEdit selection?
+ WorldEditPlugin worldEdit = plugin.getWorldEdit();
+ Selection sel = worldEdit.getSelection(player);
+ if(sel != null) {
+ // Redefine region
+ // TODO Should we just copy the code rather than calling the method?
+ redefine(args, plugin, sender);
+ }
+
+ // Reset owners list
+ synchronized(owners.getPlayers()) {
+ for(Iterator<String> i = owners.getPlayers().iterator(); i.hasNext();) {
+ String s = i.next();
+ if(!s.equals(player.getName())) {
+ i.remove();
+ }
+ }
+ }
+
+ // Attempt to save
+ try {
+ mgr.save();
+ sender.sendMessage(ChatColor.YELLOW + "Region " + id + " has been redefined.");
+ return;
+ } catch (IOException e) {
+ throw new CommandException("Failed to write regions file: "
+ + e.getMessage());
+ }
}
-
- WorldConfiguration wcfg = plugin.getGlobalStateManager().get(player.getWorld());
- RegionManager mgr = plugin.getGlobalRegionManager().get(sel.getWorld());
// Check whether the player has created too many regions
if (wcfg.maxRegionCountPerPlayer >= 0
&& mgr.getRegionCountOfPlayer(localPlayer) >= wcfg.maxRegionCountPerPlayer) {
throw new CommandException("You own too many regions, delete one first to claim a new one.");
}
-
- ProtectedRegion existing = mgr.getRegion(id);
-
- // Check for an existing region
- if (existing != null) {
- if (!existing.getOwners().contains(localPlayer)) {
- throw new CommandException("This region already exists and you don't own it.");
- }
- }
+ // Get the applicable regions for the region selection
ApplicableRegionSet regions = mgr.getApplicableRegions(region);
// Check if this region overlaps any other region
if (regions.size() > 0) {
- if (!regions.isOwnerOfAll(localPlayer)) {
+ // This is for if a player is defining regions based on their own selection
+ /*if (!regions.isOwnerOfAll(localPlayer)) {
throw new CommandException("This region overlaps with someone else's region.");
- }
+ }*/
} else {
+ /* Good for "town" setups; players can't claim other regions unless it is inside
+ * of a parent. */
if (wcfg.claimOnlyInsideExistingRegions) {
throw new CommandException("You may only claim regions inside " +
- "existing regions that you or your group own.");
+ "existing regions that you or your group own.");
}
}
@@ -289,16 +308,19 @@ public static void claim(CommandContext args, WorldGuardPlugin plugin,
}
}*/
+ // Check if the region size is too large to claim
if (region.volume() > wcfg.maxClaimVolume) {
- player.sendMessage(ChatColor.RED + "This region is to large to claim.");
+ player.sendMessage(ChatColor.RED + "This region is too large to claim.");
player.sendMessage(ChatColor.RED +
"Max. volume: " + wcfg.maxClaimVolume + ", your volume: " + region.volume());
return;
}
- region.getOwners().addPlayer(player.getName());
+ // Add the player as the owner
+ owners.addPlayer(player.getName());
mgr.addRegion(region);
+ // Attempt to save
try {
mgr.save();
sender.sendMessage(ChatColor.YELLOW + "Region saved as " + id + ".");
View
242 src/test/java/com/sk89q/worldguard/bukkit/commands/TestRegionClaim.java
@@ -0,0 +1,242 @@
+package com.sk89q.worldguard.bukkit.commands;
+
+// JUnit and Mockito
+import java.util.HashSet;
+import java.util.Set;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+// Imports for mocking
+import org.bukkit.ChatColor;
+import org.bukkit.World;
+import org.bukkit.entity.Player;
+import com.sk89q.minecraft.util.commands.CommandContext;
+import com.sk89q.worldedit.bukkit.WorldEditPlugin;
+import com.sk89q.worldedit.bukkit.selections.Selection;
+import com.sk89q.worldguard.LocalPlayer;
+import com.sk89q.worldguard.bukkit.ConfigurationManager;
+import com.sk89q.worldguard.bukkit.WorldConfiguration;
+import com.sk89q.worldguard.bukkit.WorldGuardPlugin;
+import com.sk89q.worldguard.domains.DefaultDomain;
+import com.sk89q.worldguard.protection.ApplicableRegionSet;
+import com.sk89q.worldguard.protection.GlobalRegionManager;
+import com.sk89q.worldguard.protection.managers.RegionManager;
+import com.sk89q.worldguard.protection.regions.ProtectedRegion;
+
+// Exceptions
+import com.sk89q.minecraft.util.commands.CommandException;
+import java.io.IOException;
+
+public class TestRegionClaim {
+ Player sender;
+ LocalPlayer localPlayer;
+ WorldGuardPlugin plugin;
+ CommandContext args;
+ World world;
+ RegionManager mgr;
+ ProtectedRegion region;
+ WorldConfiguration wcfg;
+ GlobalRegionManager grm;
+ ConfigurationManager cm;
+ Player player;
+ DefaultDomain owners;
+ Set<String> setOwners;
+ WorldEditPlugin worldEdit;
+ Selection sel;
+ ApplicableRegionSet regions;
+
+ @Before
+ public void setUp() throws Exception {
+ // Mock objects
+ sender = mock(Player.class);
+ localPlayer = mock(LocalPlayer.class);
+ plugin = mock(WorldGuardPlugin.class);
+ args = mock(CommandContext.class);
+ world = mock(World.class);
+ mgr = mock(RegionManager.class);
+ region = mock(ProtectedRegion.class);
+ wcfg = mock(WorldConfiguration.class);
+ grm = mock(GlobalRegionManager.class);
+ cm = mock(ConfigurationManager.class);
+ player = mock(Player.class);
+ owners = mock(DefaultDomain.class);
+ worldEdit = mock(WorldEditPlugin.class);
+ sel = mock(Selection.class);
+ regions = mock(ApplicableRegionSet.class);
+
+ setOwners = new HashSet<String>();
+ setOwners.add("Shaniqua");
+ setOwners.add("Taniqua");
+ setOwners.add("Shanaynay");
+
+ // Set up function stubbing
+ when(plugin.checkPlayer(sender)).thenReturn(player);
+ when(plugin.wrapPlayer(player)).thenReturn(localPlayer);
+ when(sender.getWorld()).thenReturn(world);
+ when(args.argsLength()).thenReturn(1);
+ when(args.getString(0)).thenReturn("test_region");
+ when(plugin.getGlobalRegionManager()).thenReturn(grm);
+ when(grm.get(world)).thenReturn(mgr);
+ when(mgr.getRegion("test_region")).thenReturn(region);
+ when(plugin.getGlobalStateManager()).thenReturn(cm);
+ when(player.getWorld()).thenReturn(world);
+ when(cm.get(world)).thenReturn(wcfg);
+ wcfg.maxRegionCountPerPlayer = 1;
+ when(mgr.getRegionCountOfPlayer(localPlayer)).thenReturn(0);
+ when(region.getOwners()).thenReturn(owners);
+ when(owners.getPlayers()).thenReturn(setOwners);
+ when(player.getName()).thenReturn("Shanaynay");
+ when(plugin.getWorldEdit()).thenReturn(worldEdit);
+ when(worldEdit.getSelection(player)).thenReturn(sel);
+ when(worldEdit.getSelection(player)).thenReturn(null);
+ when(mgr.getApplicableRegions(region)).thenReturn(regions);
+ when(regions.size()).thenReturn(5000);// Inside a town
+ wcfg.claimOnlyInsideExistingRegions = true;
+ when(region.volume()).thenReturn(5000);
+ wcfg.maxClaimVolume = 10000;
+ }
+
+ // Incorrect parameter syntax w/ 0 parameters
+ @Test
+ public void testNoParameters() {
+ try {
+ when(args.argsLength()).thenReturn(0);
+ RegionCommands.claim(args, plugin, sender);
+ fail();
+ } catch(CommandException ex) {
+ assertEquals(ex.getMessage(), "Incorrect parameter syntax!");
+ }
+ }
+
+ // Incorrect parameter syntax w/ 2 parameters
+ @Test
+ public void testTooManyParameters() {
+ try {
+ when(args.argsLength()).thenReturn(2);
+ RegionCommands.claim(args, plugin, sender);
+ fail();
+ } catch(CommandException ex) {
+ assertEquals(ex.getMessage(), "Incorrect parameter syntax!");
+ }
+ }
+
+ // Use __global__ as a region
+ @Test
+ public void testGlobalRegionId() {
+ try {
+ when(args.getString(0)).thenReturn("__global__");
+ RegionCommands.claim(args, plugin, sender);
+ fail();
+ } catch(CommandException ex) {
+ assertEquals(ex.getMessage(), "A region cannot be named __global__");
+ }
+ }
+
+ // Could not find region by that id
+ @Test
+ public void testRegionNotFound() {
+ try {
+ when(mgr.getRegion("test_region")).thenReturn(null);
+ RegionCommands.claim(args, plugin, sender);
+ fail();
+ } catch(CommandException ex) {
+ assertEquals(ex.getMessage(), "Could not find a region by that ID.");
+ }
+ }
+
+ // You own too many regions
+ @Test
+ public void testTooManyRegionsOwned() {
+ try {
+ Set<String> emptyOwners = new HashSet<String>();
+ when(owners.getPlayers()).thenReturn(emptyOwners);
+
+ when(mgr.getRegionCountOfPlayer(localPlayer)).thenReturn(1);
+ RegionCommands.claim(args, plugin, sender);
+ fail();
+ } catch(CommandException ex) {
+ assertEquals(ex.getMessage(), "You own too many regions, delete one first to claim a new one.");
+ }
+ }
+
+ // Region already has an owner
+ @Test
+ public void testRegionAlreadyHasOwner() {
+ try {
+ when(player.getName()).thenReturn("Chris");
+ RegionCommands.claim(args, plugin, sender);
+ fail();
+ } catch(CommandException ex) {
+ assertEquals(ex.getMessage(), "This region already has an owner - you can't claim or redefine it.");
+ }
+ }
+
+ // Owners reset
+ @Test
+ public void testResetOwners() throws CommandException, IOException {
+ sel = null;
+
+ RegionCommands.claim(args, plugin, sender);
+
+ assertEquals(owners.getPlayers().size(), 1);
+ assertTrue(owners.getPlayers().contains("Shanaynay"));
+ verify(mgr).save();
+ verify(sender).sendMessage(anyString());
+ }
+
+ // Owners and Selection reset
+ @Test
+ @Ignore("No way to mock static methods - need to implement PowerMock")
+ public void testResetOwnersAndSelection() throws CommandException, IOException {
+ RegionCommands.claim(args, plugin, sender);
+
+ assertEquals(owners.getPlayers().size(), 1);
+ assertTrue(owners.getPlayers().contains("Shanaynay"));
+ verify(mgr).save();
+ verify(sender).sendMessage(anyString());
+ }
+
+ // Region must be within another region; claimOnlyInsideExistingRegions
+ @Test
+ public void testClaimOnlyInsideExistingRegions() {
+ Set<String> emptyOwners = new HashSet<String>();
+ when(owners.getPlayers()).thenReturn(emptyOwners);
+
+ when(regions.size()).thenReturn(0);
+
+ try {
+ RegionCommands.claim(args, plugin, sender);
+ fail();
+ } catch(CommandException ex) {
+ assertEquals(ex.getMessage(), "You may only claim regions inside existing regions that you or your group own.");
+ }
+ }
+
+ // Region is too large
+ @Test
+ public void testRegionIsTooLarge() throws CommandException {
+ Set<String> emptyOwners = new HashSet<String>();
+ when(owners.getPlayers()).thenReturn(emptyOwners);
+
+ wcfg.maxClaimVolume = 4999;
+ RegionCommands.claim(args, plugin, sender);
+
+ verify(player).sendMessage(ChatColor.RED + "This region is too large to claim.");
+ verify(player).sendMessage(ChatColor.RED + "Max. volume: " + wcfg.maxClaimVolume + ", your volume: " + region.volume());
+ }
+
+ // Finally, claim a region
+ @Test
+ public void testClaimRegion() throws CommandException, IOException {
+ Set<String> emptyOwners = new HashSet<String>();
+ when(owners.getPlayers()).thenReturn(emptyOwners);
+
+ RegionCommands.claim(args, plugin, sender);
+
+ verify(mgr).save();
+ verify(sender).sendMessage(ChatColor.YELLOW + "Region saved as test_region.");
+ }
+}
Something went wrong with that request. Please try again.