satoshinm / WebSandboxMC

Bukkit plugin providing a web-based interface with an interactive WebGL 3D preview or glimpse of your server 🕷⏳📦 ⛺
https://www.spigotmc.org/resources/websandboxmc.39415/
MIT License
19 stars 5 forks source link

Reduce usage of deprecated Bukkit data methods (for GH-66) #68

Closed satoshinm closed 7 years ago

satoshinm commented 7 years ago

https://github.com/satoshinm/WebSandboxMC/issues/66

satoshinm commented 7 years ago

Making progress but running into problems porting the sign. First of all note there is org.bukkit.material.Sign a MaterialData for setFacingDirection()/getFacing() BlockState, and org.bukkit.block.Sign a BlockState for getLines()/setLines(), the sign text. This is how signs were created using the deprecated methods (simplified slightly):

    public void clientNewSign(ChannelHandlerContext ctx, int x, int y, int z, int face, String text) {
        byte data = 0;
        switch (face) {
            case 0: // west
                data = 4; // west
                x -= 1;
                break;
            case 1: // east
                data = 5; // east
                x += 1;
                break;
            case 2: // north
                data = 2; // north
                z -= 1;
                break;
            case 3: // south
                data = 3; // south
                z += 1;
                break;
        }

        Location location = toBukkitLocation(x, y, z);

        Block block = location.getWorld().getBlockAt(location);
        block.setType(Material.WALL_SIGN);
        block.setData(data); // deprecated!
        webSocketServerThread.log(Level.FINEST, "setting sign at "+location+" data="+data);
        BlockState blockState = block.getState();
        if (!(blockState instanceof Sign)) {
            webSocketServerThread.log(Level.WARNING, "failed to place sign at "+location);
            return;
        }
        Sign sign = (Sign) blockState;

        sign.setLine(0, text);
        sign.update();
    }

To remove block.setData(data), the plan is to use BlockState to set the direction with org.bukkit.material.Sign, trying something like this:

        BlockFace blockFace;
        switch (face) {
            case 0: // west
                blockFace = BlockFace.WEST;
                x -= 1;
                break;
            case 1: // east
                blockFace = BlockFace.EAST;
                x += 1;
                break;
            default:
            case 2: // north
                blockFace = BlockFace.NORTH;
                z -= 1;
                break;
            case 3: // south
                blockFace = BlockFace.SOUTH;
                z += 1;
                break;
        }
        org.bukkit.material.Sign signDirection = new org.bukkit.material.Sign();
        signDirection.setFacingDirection(blockFace);

North and West are correct but East and South are off by a block. And Glowstone chokes in notifySignChange:

    public void notifySignChange(Location location, Material material, BlockState blockState, String[] lines) {
        int x = toWebLocationBlockX(location);
        int y = toWebLocationBlockY(location);
        int z = toWebLocationBlockZ(location);
        BlockFace blockFace = BlockFace.NORTH;
        if (blockState.getData() instanceof org.bukkit.material.Sign) {
            org.bukkit.material.Sign sign = (org.bukkit.material.Sign) blockState.getData();
            if (sign != null) {
                blockFace = sign.getFacing(); // HERE
            }
        }

sign is non-null, but it crashes in Sign.java:

21:03:28 [SEVERE] [WebSandboxMC] Error while executing GlowTask{id=49, plugin=WebSandboxMC v1.8.0, sync=true: io.github.satoshinm.WebSandboxMC.ws.WebSocketFrameHandler$1@266f960e}
java.lang.NullPointerException
    at org.bukkit.material.Sign.getFacing(Sign.java:147)
    at io.github.satoshinm.WebSandboxMC.bridge.BlockBridge.notifySignChange(BlockBridge.java:596)
    at io.github.satoshinm.WebSandboxMC.bridge.BlockBridge.setBlockUpdate(BlockBridge.java:299)
    at io.github.satoshinm.WebSandboxMC.bridge.BlockBridge.sendWorld(BlockBridge.java:128)
    at io.github.satoshinm.WebSandboxMC.ws.WebSocketServerThread.handleNewClient(WebSocketServerThread.java:175)
    at io.github.satoshinm.WebSandboxMC.ws.WebSocketFrameHandler$1.run(WebSocketFrameHandler.java:47)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at net.glowstone.scheduler.GlowTask.run(GlowTask.java:167)
    at net.glowstone.scheduler.GlowScheduler.pulse(GlowScheduler.java:152)
    at net.glowstone.scheduler.GlowScheduler.lambda$start$0(GlowScheduler.java:83)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:748)

in Sign.java in Bukkit:

    public BlockFace getFacing() {

        } else {
            return getAttachedFace().getOppositeFace();
        }

so getAttachedFace() is returning null, wrong data?

        if (isWallSign()) {
            byte data = getData();

            switch (data) {
            case 0x2:
                return BlockFace.SOUTH;

            case 0x3:
                return BlockFace.NORTH;

            case 0x4:
                return BlockFace.EAST;

            case 0x5:
                return BlockFace.WEST;
            }

            return null;
        } else {
            return BlockFace.DOWN;
        }

wall signs should only be 2, 3, 4, or 5.

satoshinm commented 7 years ago

Reverted the sign code to a known-working state, back to the deprecated methods (getData/setData magic values). Sign API may be triggering bugs in Glowstone, possibly chunk corruption, would need to investigate further!

satoshinm commented 7 years ago

If I revert https://github.com/satoshinm/WebSandboxMC/pull/68/commits/b674201cc41919a5ae15a61cec884ba7ff5717f4, and also try/catch wrap sign.getFacing(), then hits another internal NPE in Glowstone:

22:03:39 [SEVERE] [WebSandboxMC] Error while executing GlowTask{id=49, plugin=WebSandboxMC v1.8.0, sync=true: io.github.satoshinm.WebSandboxMC.ws.WebSocketFrameHandler$2@71e61728}
java.lang.NullPointerException
    at net.glowstone.block.GlowBlock.getRelative(GlowBlock.java:166)
    at net.glowstone.block.blocktype.BlockNeedsAttached.updatePhysics(BlockNeedsAttached.java:23)
    at net.glowstone.block.blocktype.BlockNeedsAttached.onNearBlockChanged(BlockNeedsAttached.java:16)
    at net.glowstone.block.GlowBlock.applyPhysics(GlowBlock.java:555)
    at net.glowstone.block.GlowBlock.setTypeIdAndData(GlowBlock.java:243)
    at net.glowstone.block.GlowBlock.setTypeId(GlowBlock.java:222)
    at net.glowstone.block.GlowBlock.setTypeId(GlowBlock.java:217)
    at net.glowstone.block.GlowBlock.setType(GlowBlock.java:183)
    at io.github.satoshinm.WebSandboxMC.bridge.BlockBridge.clientNewSign(BlockBridge.java:706)
    at io.github.satoshinm.WebSandboxMC.ws.WebSocketServerThread.handle(WebSocketServerThread.java:221)
    at io.github.satoshinm.WebSandboxMC.ws.WebSocketFrameHandler$2.run(WebSocketFrameHandler.java:69)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at net.glowstone.scheduler.GlowTask.run(GlowTask.java:167)
    at net.glowstone.scheduler.GlowScheduler.pulse(GlowScheduler.java:152)
    at net.glowstone.scheduler.GlowScheduler.lambda$start$0(GlowScheduler.java:83)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:748)
>

code:

// GlowBlock.java
    @Override
    public GlowBlock getRelative(BlockFace face) {
        return getRelative(face.getModX(), face.getModY(), face.getModZ());
    }

// called from here, BlockNeedsAttached.java:

    @Override
    public void updatePhysics(GlowBlock me) {
        BlockFace attachedTo = getAttachedFace(me);
        if (me.getRelative(attachedTo).getType() == Material.AIR || !canPlaceAt(me, attachedTo.getOppositeFace())) {
            dropMe(me);
        }
    }

Appears related to free-floating signs, but I am (trying) to attach a sign to a block. After reverting, still hit java.lang.NullPointerException at net.glowstone.block.GlowBlock.getRelative(GlowBlock.java:166) now on this line:

        Block block = location.getWorld().getBlockAt(location);
        block.setType(Material.WALL_SIGN);
satoshinm commented 7 years ago

Disabling physics, it crashes deeper:

22:10:27 [SEVERE] [WebSandboxMC] Error while executing GlowTask{id=48, plugin=WebSandboxMC v1.8.0, sync=true: io.github.satoshinm.WebSandboxMC.ws.WebSocketFrameHandler$2@140ef2e5}
java.lang.NullPointerException
        at net.glowstone.block.GlowBlock.getRelative(GlowBlock.java:166)
        at net.glowstone.block.blocktype.BlockNeedsAttached.updatePhysics(BlockNeedsAttached.java:23)
        at net.glowstone.block.blocktype.BlockNeedsAttached.onNearBlockChanged(BlockNeedsAttached.java:16)
        at net.glowstone.block.GlowBlock.applyPhysics(GlowBlock.java:555)
        at net.glowstone.block.GlowBlock.setTypeIdAndData(GlowBlock.java:243)
        at net.glowstone.block.GlowBlockState.update(GlowBlockState.java:154)
        at net.glowstone.block.state.GlowSign.update(GlowSign.java:42)
        at net.glowstone.block.GlowBlockState.update(GlowBlockState.java:142)
        at io.github.satoshinm.WebSandboxMC.bridge.BlockBridge.clientNewSign(BlockBridge.java:717)
        at io.github.satoshinm.WebSandboxMC.ws.WebSocketServerThread.handle(WebSocketServerThread.java:221)
        at io.github.satoshinm.WebSandboxMC.ws.WebSocketFrameHandler$2.run(WebSocketFrameHandler.java:69)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at net.glowstone.scheduler.GlowTask.run(GlowTask.java:167)
        at net.glowstone.scheduler.GlowScheduler.pulse(GlowScheduler.java:152)
        at net.glowstone.scheduler.GlowScheduler.lambda$start$0(GlowScheduler.java:83)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:748)
satoshinm commented 7 years ago

This isn't complete, remaining deprecated usages:

but it is an improvement, leaving https://github.com/satoshinm/WebSandboxMC/issues/66 open but merging what this is so far