Skip to content

Fix block placement handler#839

Open
Thodor12 wants to merge 1 commit into
version/mainfrom
feature/fix_block_placement
Open

Fix block placement handler#839
Thodor12 wants to merge 1 commit into
version/mainfrom
feature/fix_block_placement

Conversation

@Thodor12
Copy link
Copy Markdown
Contributor

Closes #812

Changes proposed in this pull request

  • Use the BlockItem#place method similar logic instead of custom placement logic

Testing

  • Yes I tested this before submitting it.
  • I also did a multiplayer test.

Review please

@MotionlessTrain
Copy link
Copy Markdown
Contributor

Could you double check whether anything changed regarding torches which were standing on the block?
Just in case they first managed to stand, but now pop off

@Thodor12
Copy link
Copy Markdown
Contributor Author

This is only used during block replacement

@MotionlessTrain
Copy link
Copy Markdown
Contributor

Yeah, that is what I am talking about
When using this to replace a wall of your building with a lower tier material (when designing a lower level building), it would be annoying to pick up all torches that fell inside, and put them back at the right place
Iirc Structurize has support for that (no pun intended), and I was wondering whether that was this code

@Thodor12
Copy link
Copy Markdown
Contributor Author

Oh yeah that'll still be broken but that's impossible to fix because we have to give a block update unfortunately

@MotionlessTrain
Copy link
Copy Markdown
Contributor

As long as that doesn’t get worse than it is now

{
final Block targetBlock = blockItem.getBlock();
fakePlayer.setItemInHand(InteractionHand.MAIN_HAND, stackToPlace);
final BlockPlaceContext ctx = new BlockPlaceContext(new UseOnContext(fakePlayer, InteractionHand.MAIN_HAND,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We shouldn't go over this placement context because this will mess with blocks like stairs, etc as they use hit directions etc which will result in strange things.
Which of these things here specifically do the necessary things for the panels to work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The problem is I need getStateForPlacement, which is what requires the placement context

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can do a test with stairs but important is that this is only used by scan tool replaces it looks like

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

And the copy properties method that goes after the placement state should copy the direction of a stair over

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

getStateForPlacement probably translates some meta data into the blockstate property we need. We should be able to get this from the internal call that method uses

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just in case, according to documentation (that is, the chat message in game), if you replace a block with another block with similar block states, it should inherit the block states
If the new block has block states that are not present on the old block, the default block state property is used (and the chat message is printed)
Which means, in particular for stairs, if you are replacing one stair with another, it should have the same shape in the same direction, and if you are replacing a non-stair non-shingle with a stair, it should always face in the default direction (north?), independent from how you are standing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

getStateForPlacement probably translates some meta data into the blockstate property we need. We should be able to get this from the internal call that method uses

There's nothing else for this, check PanelBlock in DO

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I verified and stairs continue to work normal. Because of:

BlockState newState = copyFirstCommonBlockStateProperties(placementState != null ? placementState : targetBlock.defaultBlockState(), blockState);

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.

Scan tool replaced the "base" panel variant, rather than the variant selected when replacing trap doors

3 participants