Skip to content

Conversation

@BernardoCBranco
Copy link
Contributor

This is a collection of small fixes/improvements. Maybe I should have created multiple PRs for this stuff but I thought it wasn't really worth it(I will if you insist though).

1. Added .gitignore

Maybe I'm an idiot and there's a good reason for there not being one but anyway I went and created one.

2. Made cables return BlockFaceShape.UNDEFINED

This is so there's no issues with water textures when cables are underwater.

Before:
2018-04-25_10 39 56

After:
2018-04-25_10 55 14

3. Added a missing @SideOnly

Added missing @Sidonly annotation to getSelectedBoundingBox.

4. Added proper collision boxes to cables and facades

Closes #23.

Also changed facades so they return the selectedBoundingBox from mimicked blocks.

2018-04-25_10 49 55
2018-04-25_10 53 52

5. Changed facade raytrace so it uses the mimicked block method

Before:
2018-04-25_11 11 59

After:
2018-04-25_11 12 41

6. Added a check to see if the mimicked block fits the cable

I'm not sure about this one. This more of an idea. I changed this so things like pressure plates and torches can't be mimicked.

Before:
2018-04-25_17 39 20

After:
2018-04-25_17 42 57

@McJty
Copy link
Collaborator

McJty commented Apr 25, 2018

That's an impressive set of changes. I'll check it out soon

public void addCollisionBoxToList(IBlockState state, World worldIn, BlockPos pos, AxisAlignedBB entityBox, List<AxisAlignedBB> collidingBoxes, @Nullable Entity entityIn, boolean isActualState) {
IBlockState mimicBlock = getMimicBlock(worldIn, pos);
if (mimicBlock != null) {
mimicBlock.getBlock().addCollisionBoxToList(mimicBlock, worldIn, pos, entityBox, collidingBoxes, entityIn, isActualState);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm worried about isActualState here. The mimic block's actual-state-ness isn't necessarily correlated to the real state's actual-state-ness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is something like this better?

IBlockState mimicBlock = getMimicBlock(worldIn, pos);
if (mimicBlock != null) {
    mimicBlock = mimicBlock.getActualState(worldIn, pos);
    mimicBlock.getBlock().addCollisionBoxToList(mimicBlock, worldIn, pos, entityBox, collidingBoxes, entityIn, true);
    return;
}

public void addCollisionBoxToList(IBlockState state, World worldIn, BlockPos pos, AxisAlignedBB entityBox, List<AxisAlignedBB> collidingBoxes, @Nullable Entity entityIn, boolean isActualState) {
IBlockState mimicBlock = getMimicBlock(worldIn, pos);
if (mimicBlock != null) {
mimicBlock.getBlock().addCollisionBoxToList(mimicBlock, worldIn, pos, entityBox, collidingBoxes, entityIn, isActualState);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

@SideOnly(Side.CLIENT)
public AxisAlignedBB getSelectedBoundingBox(IBlockState state, World worldIn, BlockPos pos) {
IBlockState mimicBlock = getMimicBlock(worldIn, pos);
return mimicBlock != null ? mimicBlock.getSelectedBoundingBox(worldIn, pos) : FULL_BLOCK_AABB;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is FULL_BLOCK_AABB right here?

Copy link
Contributor Author

@BernardoCBranco BernardoCBranco Apr 26, 2018

Choose a reason for hiding this comment

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

It shouldn't really matter in this case, if it was on the connector it would be an issue but here I don't think so. mimicBlock would normally never be null, so if it was it would be because something was wrong but I'll make it call the super class method instead anyway.

IBlockState mimicBlock = getMimicBlock(itemstack);
if (!canMimicFit(state.getBoundingBox(world, pos), mimicBlock.getBoundingBox(world, pos))) {
if (world.isRemote) {
player.sendStatusMessage(new TextComponentString(mimicBlock.getBlock().getLocalizedName() + " is too small to fit"), false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this all duplicated from above?

Copy link
Contributor Author

@BernardoCBranco BernardoCBranco Apr 26, 2018

Choose a reason for hiding this comment

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

It's duplicated because is common to both conditions, I can remove the duplicated code but it'll have to check the conditions twice instead.

} else {
if (!canMimicFit(GenericCableBlock.AABB_CENTER, state.getBoundingBox(world, pos))) {
if (world.isRemote) {
player.sendStatusMessage(new TextComponentString(block.getLocalizedName() + " is too small to mimic"), false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Duplicated again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's slightly different from the above, here we don't know the size of the bounding box of the cable that the mimic block will be applied to so we have to check the "minimum" size to see if it will fit.

@josephcsible
Copy link
Collaborator

Splitting these into different PRs will likely get most of them merged more quickly.

@BernardoCBranco
Copy link
Contributor Author

@josephcsible Is this better? I also created new PRs for stuff that I think should not have problems being merged.

@josephcsible
Copy link
Collaborator

This PR appears to still contain all of the changes that you split out into the others, so they're now causing conflicts. Can you remove them from here?

@josephcsible
Copy link
Collaborator

Also, can you split the "fits" check out? It could potentially break people's builds, so I'm not sure if we want to merge it right now.

@BernardoCBranco
Copy link
Contributor Author

It's now split in a new PR. I unintentionally deleted one of the commits, but the changes are still there, just not in the same one. Still learning how this works. >.<

@McJty
Copy link
Collaborator

McJty commented May 3, 2018

Note: we're not forgetting your PRs. Just very busy with other things right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Network cable is not a full block

3 participants