Skip to content

Commit

Permalink
Converted API over to use new World.
Browse files Browse the repository at this point in the history 
This breaks backwards compatibility for all getWorld() methods, but
shim methods were added for binary compatibility with method calls that
use LocalWorld.
  • Loading branch information
@sk89q
sk89q committed Apr 5, 2014
1 parent 63a2ca8 commit 24f8fbc
Show file tree
Hide file tree
Showing 49 changed files with 822 additions and 227 deletions.
19 changes: 9 additions & 10 deletions src/bukkit/java/com/sk89q/worldedit/bukkit/WorldEditListener.java
View file
Expand Up @@ -21,6 +21,12 @@

package com.sk89q.worldedit.bukkit;

import com.sk89q.util.StringUtil;
import com.sk89q.worldedit.LocalPlayer;
import com.sk89q.worldedit.WorldEdit;
import com.sk89q.worldedit.WorldVector;
import com.sk89q.worldedit.internal.LocalWorldAdapter;
import com.sk89q.worldedit.world.World;
import org.bukkit.Bukkit;
import org.bukkit.block.Block;
import org.bukkit.event.Event.Result;
Expand All @@ -33,12 +39,6 @@
import org.bukkit.event.player.PlayerInteractEvent;
import org.bukkit.event.player.PlayerQuitEvent;

import com.sk89q.util.StringUtil;
import com.sk89q.worldedit.LocalPlayer;
import com.sk89q.worldedit.LocalWorld;
import com.sk89q.worldedit.WorldEdit;
import com.sk89q.worldedit.WorldVector;

/**
* Handles all events thrown in relation to a Player
*/
Expand Down Expand Up @@ -119,14 +119,13 @@ public void onPlayerInteract(PlayerInteractEvent event) {
}

final LocalPlayer player = plugin.wrapPlayer(event.getPlayer());
final LocalWorld world = player.getWorld();
final World world = player.getWorld();
final WorldEdit we = plugin.getWorldEdit();

Action action = event.getAction();
if (action == Action.LEFT_CLICK_BLOCK) {
final Block clickedBlock = event.getClickedBlock();
final WorldVector pos = new WorldVector(world, clickedBlock.getX(),
clickedBlock.getY(), clickedBlock.getZ());
final WorldVector pos = new WorldVector(LocalWorldAdapter.wrap(world), clickedBlock.getX(), clickedBlock.getY(), clickedBlock.getZ());

if (we.handleBlockLeftClick(player, pos)) {
event.setCancelled(true);
Expand Down Expand Up @@ -160,7 +159,7 @@ public void run() {

} else if (action == Action.RIGHT_CLICK_BLOCK) {
final Block clickedBlock = event.getClickedBlock();
final WorldVector pos = new WorldVector(world, clickedBlock.getX(),
final WorldVector pos = new WorldVector(LocalWorldAdapter.wrap(world), clickedBlock.getX(),
clickedBlock.getY(), clickedBlock.getZ());

if (we.handleBlockRightClick(player, pos)) {
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/com/sk89q/worldedit/EditSession.java
View file
Expand Up @@ -70,6 +70,7 @@
import com.sk89q.worldedit.util.TreeGenerator;
import com.sk89q.worldedit.util.collection.DoubleArrayList;
import com.sk89q.worldedit.util.eventbus.EventBus;
import com.sk89q.worldedit.world.World;

import javax.annotation.Nullable;
import java.util.*;
Expand Down Expand Up @@ -100,7 +101,7 @@ public enum Stage {
}

@SuppressWarnings("ProtectedField")
protected final LocalWorld world;
protected final World world;
private final ChangeSet changeSet = new BlockOptimizedHistory();

private final FastModeExtent fastModeExtent;
Expand Down Expand Up @@ -147,7 +148,7 @@ public EditSession(LocalWorld world, int maxBlocks, @Nullable BlockBag blockBag)
* @param blockBag an optional {@link BlockBag} to use, otherwise null
* @param event the event to call with the extent
*/
EditSession(EventBus eventBus, LocalWorld world, int maxBlocks, @Nullable BlockBag blockBag, EditSessionEvent event) {
EditSession(EventBus eventBus, World world, int maxBlocks, @Nullable BlockBag blockBag, EditSessionEvent event) {
checkNotNull(eventBus);
checkNotNull(world);
checkArgument(maxBlocks >= -1, "maxBlocks >= -1 required");
Expand All @@ -164,7 +165,7 @@ public EditSession(LocalWorld world, int maxBlocks, @Nullable BlockBag blockBag)
extent = wrapExtent(extent, eventBus, event, Stage.BEFORE_CHANGE);
extent = quirkExtent = new BlockQuirkExtent(extent, world);
extent = validator = new DataValidatorExtent(extent, world);
extent = blockBagExtent = new BlockBagExtent(extent, world, blockBag);
extent = blockBagExtent = new BlockBagExtent(extent, blockBag);

// This extent can be skipped by calling rawSetBlock()
extent = reorderExtent = new MultiStageReorder(extent, false);
Expand Down Expand Up @@ -193,7 +194,7 @@ private Extent wrapExtent(Extent extent, EventBus eventBus, EditSessionEvent eve
*
* @return the world
*/
public LocalWorld getWorld() {
public World getWorld() {
return world;
}

Expand Down
37 changes: 29 additions & 8 deletions src/main/java/com/sk89q/worldedit/EditSessionFactory.java
View file
Expand Up @@ -22,6 +22,7 @@
import com.sk89q.worldedit.event.extent.EditSessionEvent;
import com.sk89q.worldedit.extent.inventory.BlockBag;
import com.sk89q.worldedit.util.eventbus.EventBus;
import com.sk89q.worldedit.world.World;

import static com.google.common.base.Preconditions.checkNotNull;

Expand All @@ -35,38 +36,58 @@
*/
public class EditSessionFactory {

@Deprecated
public EditSession getEditSession(LocalWorld world, int maxBlocks) {
return getEditSession((World) world, maxBlocks);
}

/**
* Construct an edit session with a maximum number of blocks.
*
* @param world the world
* @param maxBlocks the maximum number of blocks that can be changed, or -1 to use no limit
*/
public EditSession getEditSession(LocalWorld world, int maxBlocks) {
public EditSession getEditSession(World world, int maxBlocks) {
throw new IllegalArgumentException("This class is being removed");
}

@Deprecated
public EditSession getEditSession(LocalWorld world, int maxBlocks, LocalPlayer player) {
return getEditSession((World) world, maxBlocks, player);
}

/**
* Construct an edit session with a maximum number of blocks.
*
* @param world the world
* @param maxBlocks the maximum number of blocks that can be changed, or -1 to use no limit
* @param player the player that the {@link EditSession} is for
*/
public EditSession getEditSession(LocalWorld world, int maxBlocks, LocalPlayer player) {
public EditSession getEditSession(World world, int maxBlocks, LocalPlayer player) {
throw new IllegalArgumentException("This class is being removed");
}

@Deprecated
public EditSession getEditSession(LocalWorld world, int maxBlocks, BlockBag blockBag) {
return getEditSession((World) world, maxBlocks, blockBag);
}

/**
* Construct an edit session with a maximum number of blocks and a block bag.
*
* @param world the world
* @param maxBlocks the maximum number of blocks that can be changed, or -1 to use no limit
* @param blockBag an optional {@link BlockBag} to use, otherwise null
*/
public EditSession getEditSession(LocalWorld world, int maxBlocks, BlockBag blockBag) {
public EditSession getEditSession(World world, int maxBlocks, BlockBag blockBag) {
throw new IllegalArgumentException("This class is being removed");
}

@Deprecated
public EditSession getEditSession(LocalWorld world, int maxBlocks, BlockBag blockBag, LocalPlayer player) {
return getEditSession((World) world, maxBlocks, blockBag, player);
}

/**
* Construct an edit session with a maximum number of blocks and a block bag.
*
Expand All @@ -75,7 +96,7 @@ public EditSession getEditSession(LocalWorld world, int maxBlocks, BlockBag bloc
* @param blockBag an optional {@link BlockBag} to use, otherwise null
* @param player the player that the {@link EditSession} is for
*/
public EditSession getEditSession(LocalWorld world, int maxBlocks, BlockBag blockBag, LocalPlayer player) {
public EditSession getEditSession(World world, int maxBlocks, BlockBag blockBag, LocalPlayer player) {
throw new IllegalArgumentException("This class is being removed");
}

Expand All @@ -97,22 +118,22 @@ public EditSessionFactoryImpl(EventBus eventBus) {
}

@Override
public EditSession getEditSession(LocalWorld world, int maxBlocks) {
public EditSession getEditSession(World world, int maxBlocks) {
return new EditSession(eventBus, world, maxBlocks, null, new EditSessionEvent(world, null, maxBlocks, null));
}

@Override
public EditSession getEditSession(LocalWorld world, int maxBlocks, LocalPlayer player) {
public EditSession getEditSession(World world, int maxBlocks, LocalPlayer player) {
return new EditSession(eventBus, world, maxBlocks, null, new EditSessionEvent(world, player, maxBlocks, null));
}

@Override
public EditSession getEditSession(LocalWorld world, int maxBlocks, BlockBag blockBag) {
public EditSession getEditSession(World world, int maxBlocks, BlockBag blockBag) {
return new EditSession(eventBus, world, maxBlocks, blockBag, new EditSessionEvent(world, null, maxBlocks, null));
}

@Override
public EditSession getEditSession(LocalWorld world, int maxBlocks, BlockBag blockBag, LocalPlayer player) {
public EditSession getEditSession(World world, int maxBlocks, BlockBag blockBag, LocalPlayer player) {
return new EditSession(eventBus, world, maxBlocks, blockBag, new EditSessionEvent(world, player, maxBlocks, null));
}

Expand Down
36 changes: 29 additions & 7 deletions src/main/java/com/sk89q/worldedit/LocalSession.java
View file
Expand Up @@ -27,6 +27,7 @@
import com.sk89q.worldedit.command.tool.BrushTool;
import com.sk89q.worldedit.command.tool.SinglePickaxe;
import com.sk89q.worldedit.command.tool.Tool;
import com.sk89q.worldedit.extension.platform.Actor;
import com.sk89q.worldedit.extent.inventory.BlockBag;
import com.sk89q.worldedit.internal.cui.CUIEvent;
import com.sk89q.worldedit.internal.cui.CUIRegion;
Expand All @@ -36,6 +37,7 @@
import com.sk89q.worldedit.regions.Region;
import com.sk89q.worldedit.regions.RegionSelector;
import com.sk89q.worldedit.session.request.Request;
import com.sk89q.worldedit.world.World;
import com.sk89q.worldedit.world.snapshot.Snapshot;

import java.util.*;
Expand Down Expand Up @@ -175,14 +177,19 @@ public EditSession redo(BlockBag newBlockBag, LocalPlayer player) {
return null;
}

@Deprecated
public RegionSelector getRegionSelector(LocalWorld world) {
return getRegionSelector((World) world);
}

/**
* Get the region selector for defining the selection. If the selection
* was defined for a different world, the old selection will be discarded.
*
* @param world
* @return position
*/
public RegionSelector getRegionSelector(LocalWorld world) {
public RegionSelector getRegionSelector(World world) {
if (selector.getIncompleteRegion().getWorld() == null) {
selector = new CuboidRegionSelector(world);
} else if (!selector.getIncompleteRegion().getWorld().equals(world)) {
Expand All @@ -203,13 +210,18 @@ public RegionSelector getRegionSelector() {
return selector;
}

@Deprecated
public void setRegionSelector(LocalWorld world, RegionSelector selector) {
setRegionSelector((World) world, selector);
}

/**
* Set the region selector.
*
* @param world
* @param selector
*/
public void setRegionSelector(LocalWorld world, RegionSelector selector) {
public void setRegionSelector(World world, RegionSelector selector) {
selector.getIncompleteRegion().setWorld(world);
this.selector = selector;
}
Expand All @@ -224,13 +236,18 @@ public boolean isRegionDefined() {
return selector.isDefined();
}

@Deprecated
public boolean isSelectionDefined(LocalWorld world) {
return isSelectionDefined((World) world);
}

/**
* Returns true if the region is fully defined for the specified world.
*
* @param world
* @return
*/
public boolean isSelectionDefined(LocalWorld world) {
public boolean isSelectionDefined(World world) {
if (selector.getIncompleteRegion().getWorld() == null || !selector.getIncompleteRegion().getWorld().equals(world)) {
return false;
}
Expand All @@ -248,6 +265,11 @@ public Region getRegion() throws IncompleteRegionException {
return selector.getRegion();
}

@Deprecated
public Region getSelection(LocalWorld world) throws IncompleteRegionException {
return getSelection((World) world);
}

/**
* Get the selection region. If you change the region, you should
* call learnRegionChanges(). If the selection is defined in
Expand All @@ -258,7 +280,7 @@ public Region getRegion() throws IncompleteRegionException {
* @return region
* @throws IncompleteRegionException
*/
public Region getSelection(LocalWorld world) throws IncompleteRegionException {
public Region getSelection(World world) throws IncompleteRegionException {
if (selector.getIncompleteRegion().getWorld() == null || !selector.getIncompleteRegion().getWorld().equals(world)) {
throw new IncompleteRegionException();
}
Expand All @@ -270,7 +292,7 @@ public Region getSelection(LocalWorld world) throws IncompleteRegionException {
*
* @return
*/
public LocalWorld getSelectionWorld() {
public World getSelectionWorld() {
return selector.getIncompleteRegion().getWorld();
}

Expand Down Expand Up @@ -544,7 +566,7 @@ public void tellVersion(LocalPlayer player) {
* @param player
* @param event
*/
public void dispatchCUIEvent(LocalPlayer player, CUIEvent event) {
public void dispatchCUIEvent(Actor player, CUIEvent event) {
if (hasCUISupport) {
player.dispatchCUIEvent(event);
}
Expand Down Expand Up @@ -585,7 +607,7 @@ public void dispatchCUISelection(LocalPlayer player) {
}
}

public void describeCUI(LocalPlayer player) {
public void describeCUI(Actor player) {
if (!hasCUISupport) {
return;
}
Expand Down
27 changes: 10 additions & 17 deletions src/main/java/com/sk89q/worldedit/command/BiomeCommands.java
View file
Expand Up @@ -19,30 +19,23 @@

package com.sk89q.worldedit.command;

import static com.sk89q.minecraft.util.commands.Logging.LogMode.REGION;

import java.util.HashSet;
import java.util.List;
import java.util.Set;

import com.sk89q.minecraft.util.commands.Command;
import com.sk89q.minecraft.util.commands.CommandContext;
import com.sk89q.minecraft.util.commands.CommandPermissions;
import com.sk89q.minecraft.util.commands.Logging;
import com.sk89q.worldedit.BiomeType;
import com.sk89q.worldedit.EditSession;
import com.sk89q.worldedit.LocalPlayer;
import com.sk89q.worldedit.LocalSession;
import com.sk89q.worldedit.LocalWorld;
import com.sk89q.worldedit.Vector;
import com.sk89q.worldedit.Vector2D;
import com.sk89q.worldedit.WorldEdit;
import com.sk89q.worldedit.WorldEditException;
import com.sk89q.worldedit.*;
import com.sk89q.worldedit.masks.BiomeTypeMask;
import com.sk89q.worldedit.masks.InvertedMask;
import com.sk89q.worldedit.masks.Mask;
import com.sk89q.worldedit.regions.FlatRegion;
import com.sk89q.worldedit.regions.Region;
import com.sk89q.worldedit.world.World;

import java.util.HashSet;
import java.util.List;
import java.util.Set;

import static com.sk89q.minecraft.util.commands.Logging.LogMode.REGION;

public class BiomeCommands {

Expand Down Expand Up @@ -116,7 +109,7 @@ public void biomeInfo(CommandContext args, LocalSession session, LocalPlayer pla
BiomeType biome = player.getWorld().getBiome(player.getPosition().toVector2D());
player.print("Biome: " + biome.getName());
} else {
LocalWorld world = player.getWorld();
World world = player.getWorld();
Region region = session.getSelection(world);
Set<BiomeType> biomes = new HashSet<BiomeType>();

Expand Down Expand Up @@ -180,7 +173,7 @@ public void setBiome(CommandContext args, LocalSession session, LocalPlayer play
}
} else {
int affected = 0;
LocalWorld world = player.getWorld();
World world = player.getWorld();
Region region = session.getSelection(world);

if (region instanceof FlatRegion) {
Expand Down

12 comments on commit 24f8fbc

@kangarko
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,
apologises for posting it here, but I am unofficially updating plugin called CreativeControl and the worldedit integration broken in the very recent snapshot. Can you please help me with updating these two classes?
https://github.com/kangarko/CreativeControl/tree/master/src/main/java/me/FurH/CreativeControl/integration/worldedit
Thanks very much!

Regards,
kangarko

@sk89q
Copy link
Member Author

@sk89q sk89q commented on 24f8fbc Apr 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to get back to you later. Note that, if possible, I would prefer if I could make a change in WorldEdit so that existing calls don't break (by adding shim classes/deprecated methods/etc.), so I will see what can be done there.

@kangarko
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I can wait, I have made some non breaking changes (https://github.com/kangarko/CreativeControl/commit/67256af7ef718e755afa329e2c86ec7773d80c06).

@sk89q
Copy link
Member Author

@sk89q sk89q commented on 24f8fbc Apr 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could, replace EditSession with DelegateExtent and override setBlock(). To execute a block set, use super.setBlock()

And then register your new delegate extent with something like this: https://github.com/LogBlock/LogBlock/pull/542/files#diff-7aa28538d928ec2f63148c2ac7c5e031R31

You don't need EditSessionFactory anymore.

Please note that the old code assumes that the world is a BukkitWorld. It may or may not be the case, even if you run on Bukkit. This is because, for example, if you install Bukkit and Forge versions of WE in the future on the same server, it may use Bukkit for perms but actually use ForgeWorld to make the changes.

@kangarko
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,
thank you very much for help. I tried editing it (https://github.com/kangarko/CreativeControl/commit/421be31d9d741b18a1a07cb58432a581e02a1038) but it gives me StackOverflowError. I am not really experienced with creativecontrol, plugin isn´t mine, I am only updating it since official version is broken with 3 plugins already.

@wizjany
Copy link
Collaborator

@wizjany wizjany commented on 24f8fbc Apr 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, you should no longer be calling block logging plugins. Since the change to extents, each plugin that registers with WE will independently get the changes.
As for the SOE, it would help if you could paste the error.

Edit: Also just saw that you just copied the logblock code. Since you're trying to prevent changes, rather than just log them, you don't need an AbstractLoggingExtent. Check this comment: #293 (comment)

@sk89q
Copy link
Member Author

@sk89q sk89q commented on 24f8fbc Apr 10, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kangarko As wiz said, you should not be extending AbstractLoggingExtent. AbstractLoggingExtent is made for block loggers, and the code that it has for setBlock is below:

boolean setBlock(...) {
    onBlockChange(...);
    return super.setBlock(...);
}

As you see, onBlockChange() gets called but the block gets set anyway. If you call super.setBlock(...), you merely run the code above again, which calls your method again, which results in an infinite loop (and then a stack overflow).

The Extent Interface

What you really want to do is implement the Extent interface. First, understand that extents are "stacked":

HistoryExtent -> BlockReorderExtent -> BukkitWorld

Each extent handles the block placement attempt, and if it wants it to continue, it passes it onto the next Extent. The idea is that you inject your own extent:

HistoryExtent -> BlockReorderExtent -> YourExtent -> BukkitWorld

Then when you want to prevent a block place, you don't pass it onto BukkitWorld:

HistoryExtent -> BlockReorderExtent -> YourExtent

If BukkitWorld.setBlock() never gets called, the block is not set.

You Actually Want AbstractDelegateExtent

However, you should not directly implement Extent. Use AbstractDelegateExtent, which, by default, implements all methods as merely a proxy to the next extent. Because it does that, you don't need to actually implement any methods to make your class compile:

class MyExtent extends AbstractDelegateExtent {
    public MyExtent(Extent extent) { super(extent); } // Required constructor
}

But since you want to explicitly override how blocks are placed, you override the setBlock method. You can call super.setBlock(...), which refers to AbstractDelegateExtent.setBlock(...), which merely passes the call onto the next extent (and thus setting the block). You can see that in the example below.

class MyExtent extends AbstractDelegateExtent {
    public MyExtent(Extent extent) { super(extent); } // Required constructor

    @Override
    public boolean setBlock(...) {
        return super.setBlock(...);
    }
}

Of course, if that was your Extent's code, then it would do nothing useful. Because it just passes the call onto the next extent, blocks get placed like normal.

You would implement some condition:

class MyExtent extends AbstractDelegateExtent {
    public MyExtent(Extent extent) { super(extent); } // Required constructor

    @Override
    public boolean setBlock(...) {
        if (shouldPlaceBlock(...)) {
            return super.setBlock(...); // Set the block like normal using the 'next' extent
        } else {
            return false; // False tells calling code that the block set failed
        }
    }
}

By the way, the whole WorldEdit.getInstance().getEventBus().register(new Object() { ... basically makes a new anonymous class and registers it in one go. For code organization, you may want to make it a separate class (in a separate file), and then register an instance of that class:

WorldEdit.getInstance().getEventBus().register(new CreativeControlExtent(this));

@kangarko
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,
thank you so much for help. I have finally recoded entirely, just it by myself as I found myself being unable to fix it.
I found working code in section Sample Code at #293 (comment)
Thanks!

The changed file is here: https://github.com/kangarko/CreativeControl/blob/28837ef33fc7cb112e165b737f79ccf5ff0cd6e1/src/main/java/me/FurH/CreativeControl/integration/worldedit/CreativeEditSession.java
I would really appreciate if you can test it, I did and it worked but maybe I missed something.

@sk89q
Copy link
Member Author

@sk89q sk89q commented on 24f8fbc Apr 10, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kangarko Use BEFORE_CHANGE rather than BEFORE_HISTORY. Some things skip history and therefore would skip your hook.

Also I don't think VERY_LATE is necessary. Keep it NORMAL.

VERY_LATE is better for purely-logging purposes.

@DarkArc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you suggest LogBlock's subscribe priority be changed to VERY_LATE?

@kangarko
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I´ll fix it.
Edit: Changes commited ;)

@sk89q
Copy link
Member Author

@sk89q sk89q commented on 24f8fbc Apr 11, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DarkArc Possibly, though we might want have something like MONITOR.

Please sign in to comment.