Carry On

Carry On

112M Downloads

[1.12.2] Metal Chest support?

TheSnowyChickens opened this issue ยท 21 comments

commented

Expected Behavior

Correct side from the lock, after end of carrying.

Actual Behavior

It noted the lock side and dont rotate to the new placed side.

Steps to Reproduce

Place the chest -> use carry on -> place the chest on a other block.

Version of Minecraft, Carry On, Forge

1.12.2
Forge 2747
C.O.: v1.9.3
Metal Chest: v1.2.2

Screenshots encouraged

2018-08-05_20 14 30

2018-08-05_20 14 36

2018-08-05_20 14 42

commented

CarryOn can only do rotation if mods implement one of the vanilla ways how rotation is handled. I'm gonna take a look at it but I'm pretty sure there's nothing I can do about it.

commented

@Tschipp Do you utilize rotateBlock anywhere? All I see is reference to setting PropertyDirection (https://github.com/Tschipp/CarryOn/blob/master/src/main/java/tschipp/carryon/common/item/ItemTile.java#L118).

commented

Oh okay, then I report this problem to the Metal Chest developer.

Thx for this info. :)

commented

Wow, no I was unaware of the method. I will implement it in the next release. Thanks for the info.

commented

So after taking a closer look at rotateBlock, I sadly must come to the conclusion that I cannot use it for my purposes, as its implementation depends entirely on the modder that implements it. See this comment of the method:

Rotate the block. For vanilla blocks this rotates around the axis passed in (generally, it should be the "face" that was hit).
Note: for mod blocks, this is up to the block and modder to decide. It is not mandated that it be a 
rotation around theface, but could be a rotation to orient *to* that face, 
or a visiting of possible rotations.
The method should return true if the rotation was successful though.

If we imagine that every modder implements it like vanilla does, to rotate the block correctly I would need to do this: (keep in mind this would also only be for y-axis rotation)

  1. Check the rotation of the block when it was picked up
  2. call rotateBlock until the block faces the player.

However, to be able to know at what rotation the block is currently on when being picked up (step 1), I need to get its PropertyDirection. Now, if the block handles rotation in NBT, this won't be possible and it will be impossible for me to know, how many times I need to call rotateBlock so that it faces the player.

I also thought about resetting the rotation to a "default rotation", so that I always know how many times I need to rotate, just based on the player. But that method also requires knowledge about the current rotationstate of the block.

Realistically, most modders won't all write rotateBlock in the vanilla way either way, so that ruins the method I described above.

So sadly, I think I'm gonna have to stick with my ugly method, because it works. Unless I've missed something major, I don't think it's possible with rotateBlock. If you have any suggestions of how I might do it, I'm all ears! @T145

commented

Do something like ChestTransporter, where there's a developer API that handles how the block is rotated. This is basically like that mod, w/ the exception of being able to use your arms.

As for rotateBlock, if you inspect the actual method, you'll realize it's extremely similar to what you already do when placing the chest. You're overthinking this way too much. The vanilla way uses PropertyDirection, which is what you're doing right now. If someone implements the method themselves, then it just does something else. You don't need to worry about the NBT at all. It's on the other developer to implement the method in their block to adjust their tile's NBT. Here's what you do:

  1. Remove the while-loop here: ItemTile.java#L110
  2. Replace it w/ setting the set boolean equal to rotateBlock's result
  3. You've got it from here
    /**
     * Rotate the block. For vanilla blocks this rotates around the axis passed in (generally, it should be the "face" that was hit).
     * Note: for mod blocks, this is up to the block and modder to decide. It is not mandated that it be a rotation around the
     * face, but could be a rotation to orient *to* that face, or a visiting of possible rotations.
     * The method should return true if the rotation was successful though.
     *
     * @param world The world
     * @param pos Block position in world
     * @param axis The axis to rotate around
     * @return True if the rotation was successful, False if the rotation failed, or is not possible
     */
    public boolean rotateBlock(World world, BlockPos pos, EnumFacing axis)
    {
        IBlockState state = world.getBlockState(pos);
        for (IProperty<?> prop : state.getProperties().keySet())
        {
            if ((prop.getName().equals("facing") || prop.getName().equals("rotation")) && prop.getValueClass() == EnumFacing.class)
            {
                Block block = state.getBlock();
                if (!(block instanceof BlockBed) && !(block instanceof BlockPistonExtension))
                {
                    IBlockState newState;
                    //noinspection unchecked
                    IProperty<EnumFacing> facingProperty = (IProperty<EnumFacing>) prop;
                    EnumFacing facing = state.getValue(facingProperty);
                    java.util.Collection<EnumFacing> validFacings = facingProperty.getAllowedValues();

                    // rotate horizontal facings clockwise
                    if (validFacings.size() == 4 && !validFacings.contains(EnumFacing.UP) && !validFacings.contains(EnumFacing.DOWN))
                    {
                        newState = state.withProperty(facingProperty, facing.rotateY());
                    }
                    else
                    {
                        // rotate other facings about the axis
                        EnumFacing rotatedFacing = facing.rotateAround(axis.getAxis());
                        if (validFacings.contains(rotatedFacing))
                        {
                            newState = state.withProperty(facingProperty, rotatedFacing);
                        }
                        else // abnormal facing property, just cycle it
                        {
                            newState = state.cycleProperty(facingProperty);
                        }
                    }

                    world.setBlockState(pos, newState);
                    return true;
                }
            }
        }
        return false;
    }
commented

I see, but rotating the block is not the problem. The problem is, how many times we need to call rotateBlock. Let me make an example:
If we pick up a chest that is facing south (the player is facing north), and the player places it back down while facing south, the chest needs to be rotated to face north. That means, we need to call rotateBlock twice, and rotate it around the y-axis.
To be able to calculate how many times we need to rotate the block so that it faces the player correctly, we need to somehow figure out what the rotation of the block was, before it was picked up. If blocks handle rotation in the vanilla way, this can be done easily by just getting the rotation property. But in case of modded blocks that don't handle their rotation with states but with nbt, there is no way to know in what direction the block was facing before the pickup, because there sadly is no getRotation method (as least as far as I know. If you know a way to get the rotation of any block, please let me know!).
So if we cannot determine at what rotation the block was at before the pickup, there is no way to know how many times we need to call rotateBlock in order to make the block face the player properly.

Edit: on the topic of API, yes I have thought about making one, but I don't want CarryOn's compatibility to depend on if other modders use my API, I'd like to support as much as possible out of the box, to enhance the player experience and to not have to make huge changes to the config.

commented

Is this maybe 1 problem? Metal Chests dont have this 1 tag in F3.
facing

commented

No. The problem is that there is no way for me to retrieve the rotation of a block, that stores its rotation in NBT.

commented

@T145 before I open a new issue at your Github (because the topic is suitable here), I can see, that the Thermal Foundation crescent hammer can't rotate your chests either. (but vanilla chests)

So as an idea for CarryOn, is maybe behind the Thermal F. crescent hammer some usefull code to solve the rotation problem?

commented

I will take a look at their code

commented

To be able to calculate how many times we need to rotate the block so that it faces the player correctly, we need to somehow figure out what the rotation of the block was, before it was picked up.

Just use player.getHorizontalFacing().getOpposite().

@TheSnowyChickens The difference between my chests and most others is that I use the block metadata to store the block variants rather than which direction it's facing. Thaumcraft's Hungry Chest will work w/ this mod for example b/c it extends BlockChest.

commented

EnderStorage does something similar to me, so adding support for rotateBlock would add support for that mod, and I'm assuming quite a few more: https://github.com/TheCBProject/EnderStorage/blob/master/src/main/java/codechicken/enderstorage/block/BlockEnderStorage.java#L262

commented

Just use player.getHorizontalFacing().getOpposite()

Well yes, that is the rotation that the block should have after it has been placed down. But that still doesn't tell me anything about the rotation of the block before I picked it up.

commented

My point is that you don't need that information. Just the direction facing the player.

commented

My point is that you don't need that information. Just the direction facing the player.

How so? That just tells me what direction the block should be facing after I've placed it down. It tells me nothing about how many times I need to call rotateBlock

commented

Ok I see your issue. You want to get the initial facing value so you can determine if it's an opposite side, and therefore needs multiple rotations. This information cannot be easily retrieved from a tile entity (unless you set up a simple IFacing interface or something cough). If you need to get a value from NBT, set up a HashSet of all the likely values:

HashSet<String> facingValues = new HashSet<>();
facingValues.add("facing");
facingValues.add("Facing");
facingValues.add("front");
facingValues.add("Front");
facingValues.add("orientation");
facingValues.add("Orientation");

Check to see if there's any match. The HashSet will allow for O(1) checks. If we get a match, then we need to determine what data type it is. It can either be a String, Integer or Byte. Of course that would just be something like:

if (facingTag instanceof String) {
    return EnumFacing.valueOf((String) facingTag);
} else if (facingTag instanceof Integer) {
    return EnumFacing.getIndex((int) facingTag);
} else if (facingTag instanceof Byte) {
    return EnumFacing.getIndex((byte) facingTag);
}

Now you have the EnumFacing value; check if it's opposite, and rotate twice if so, else determine the number of clockwise rotations needed (max is 3; i.e. North > West).

commented

You're welcome :P

commented

Hmm... yeah that seems doable. Ok, I will include a check for common NBT values for rotation!

commented

Thank you guys! ๐Ÿ‘

For me, adding this feature has sounded so easy, but now I saw that it didn't seem that way.
(I had basic Java in school^^)

commented

Heey, it works! (now with both metal chest and iron chest)
gif
@T145 Thank you for your support!