PneumaticDoor assorted issues
LemADEC opened this issue ยท 7 comments
Minecraft Version
1.12.2
Forge Version
14.23.5.2770
Mod Version
pneumaticcraft-repressurized-1.12.2-0.8.4-303
Describe your problem, including steps to reproduce it
While in creative mode, breaking the top part of a Pneumatic Door will drop an item. There should be no drop.
While in survival mode, breaking the top part of a Pneumatic Door will show the breaking overlay shifted up by 1 block (i.e. the block above the door appears to be breaking, while the bottom part of the door isn't).
When top part of the door is missing or invalid, a crash will happen here:
The code assume the top is always there which can't be certain. I suggest to add a check so it doesn't later try to rotate an invalid orientation.
Any other comments?
I suspect an issue in the way the door is saved in the world as the metadata seems to differs between saved on disk and in-game.
How did you get to a point where the top half of the door was missing / invalid, and can you post a full stracktrace of the crash please? The code doesn't assume the top half always exists:
private TileEntityPneumaticDoorBase getDoorBase(World world, BlockPos pos) {
if (world.getBlockState(pos).getBlock() != this) return null;
if (!isTopDoor(world.getBlockState(pos))) {
return getDoorBase(world, pos.offset(EnumFacing.UP));
} else {
...Note the first line of the method verifying that the block in question is actually a BlockPneumaticDoor.
Creative mode dropping and bad block break overlay problems are fixed in dev now.
Regarding the metadata saving issue, can you be more specific please? How did you determine that there's an inconsistency, and what is the effect of that?
Yes, the code does check the block is present, but it fails to check the metadata is valid.
The callstack looks like that:
java.lang.IllegalStateException: Unable to get Y-rotated facing of up
at net.minecraft.util.EnumFacing.rotateY(EnumFacing.java:138)
at me.desht.pneumaticcraft.common.block.BlockPneumaticDoor.getDoorBase(BlockPneumaticDoor.java:239)
at me.desht.pneumaticcraft.common.block.BlockPneumaticDoor.getDoorBase(BlockPneumaticDoor.java:236)
at me.desht.pneumaticcraft.common.block.BlockPneumaticDoor.neighborChanged(BlockPneumaticDoor.java:224)
at net.minecraft.block.state.BlockStateContainer$StateImplementation.neighborChanged(BlockStateContainer.java:511)
at net.minecraft.world.World.neighborChanged(World.java:638)
at net.minecraft.world.World.notifyNeighborsOfStateExcept(World.java:601)
at dan200.computercraft.shared.util.RedstoneUtil.propagateRedstoneOutput(RedstoneUtil.java:82)
at dan200.computercraft.shared.computer.blocks.TileComputerBase.updateOutput(TileComputerBase.java:383)
at dan200.computercraft.shared.computer.blocks.TileComputerBase.update(TileComputerBase.java:224)
at net.minecraft.world.World.updateEntities(World.java:2019)
at net.minecraft.world.WorldServer.updateEntities(WorldServer.java:643)
at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:842)
at net.minecraft.server.MinecraftServer.tick(MinecraftServer.java:743)
at net.minecraft.server.integrated.IntegratedServer.tick(IntegratedServer.java:192)
at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:592)
at java.lang.Thread.run(Thread.java:748)
There's an Advanced computer from ComputerCraft right next to the pneumatic door, which, for unknown reasons, try to propagate redstone while door has an invalid metadata.
I'm in process of updating WarpDrive compatibility with PneumaticCraft. This involve rotating blocks by updating their metadata and NBT values. While doing so, I've seen metadata values for the top block that change beyond my understanding, so there's probably something I'm missing in the way those values are defined.
Yeah, the door should never have a facing of UP.
Setting aside the overall inadvisability of rotating blocks by directly tweaking block meta values, or undocumented tile entity fields, the block meta behaviour for the Pneumatic Door is defined here: https://github.com/TeamPneumatic/pnc-repressurized/blob/master/src/main/java/me/desht/pneumaticcraft/common/block/BlockPneumaticDoor.java#L47-L55
So bits 0-3 encode the rotation (DUNSWE, same as EnumFacing order), and bit 4 is 0 for a bottom-half door, and 1 for a top-half door. The block doesn't explicitly forbid setting a rotation of up or down, but it should never normally happen. The array definition at https://github.com/LemADEC/WarpDrive/blob/MC1.7/src/main/java/cr0s/warpdrive/compat/CompatPneumaticCraft.java#L52 looks wrong for the second 8 values; it should be the same as the first 8 values, but with 8 added.
A safer approach here would be to use the ROTATION block property rather than tweaking the block meta directly, but if you fix the mrotDoor[] array, it should work for now. Of course, that code will have to be completely rewritten for 1.13 anyway...
I'll also add an additional check for an invalid rotation in BlockPneumaticDoor#getDoorBase() (will return a null TE if the rotation is wrong).
That code is for 1.7.10 branch.
The 1.12.2 is only my local and looks like this:
private static final byte[] mrotPneumaticDoor = { 0, 1, 5, 4, 2, 3, 6, 7, 11, 10, 8, 9, 12, 13, 14, 15 };
...
if (classBlockPneumaticDoor.isInstance(block)) {
switch (rotationSteps) {
case 1:
return mrotPneumaticDoor[metadata];
case 2:
return mrotPneumaticDoor[mrotPneumaticDoor[metadata]];
case 3:
return mrotPneumaticDoor[mrotPneumaticDoor[mrotPneumaticDoor[metadata]]];
default:
return metadata;
}
}
...
The code for PneumaticDoor says it's offset by 8, but that cause crashes.
The map save says it's offset by 6 and that's what is coded currently.
I think the reason is here:
https://github.com/TeamPneumatic/pnc-repressurized/blob/master/src/main/java/me/desht/pneumaticcraft/common/block/BlockPneumaticCraft.java#L225
it does a modulo 6 due to this:
public static EnumFacing byIndex(int index)
{
return VALUES[MathHelper.abs(index % VALUES.length)];
}
That probably explain why I see different meta values in-game from the map save.
I wonder if other blocks would be impacted by this.