New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Item Placement with Signs #902

Merged
merged 3 commits into from Apr 25, 2018

Conversation

Projects
None yet
3 participants
@smcconke
Contributor

smcconke commented Apr 22, 2018

This PR aims to fix the block placement interaction involving both sign materials, SIGN_POST and WALL_SIGN.

Observed Behavior: While holding items only pertaining to BlockType, the player can place these materials around and on top of the sign. When placing the blocks, the player's target block will be a variation of the sign and the face will be either North, East, South, West, or Up. The same output occurs when attempting to place a sign onto another variation of the sign.

Expected Behavior: With Vanilla Minecraft, a player cannot place a block or sign onto a placed SIGN_POST or WALL_SIGN unless the player is sneaking while right clicking on the target.

The implementation uses a private variable, pertaining to all BlockTypes, that will be set once a player attempts to right click and place a block. This approach allows for all BlockTypes to check this condition without passing the instance of the player as another argument in canPlaceAt. While compiling BlockNeedsAttached.java, checkstyle produced an error pertaining to javadocs, therefore documentation was given for canAttachTo. In BlockSign.java, canPlaceAt is overrided to provide a different implementation from that of BlockNeedsAttached.

*/
@Getter
@Setter
private boolean shiftClickPlace = false;

This comment has been minimized.

@Pr0methean

Pr0methean Apr 22, 2018

Contributor

This won't work properly in the case where multiple players try to interact with the same block in a short timespan. Instead, whatever method calls canPlaceAt or canAttachTo should check whether the relevant player isSneaking().

@smcconke

This comment has been minimized.

Contributor

smcconke commented Apr 22, 2018

The following changes take into consideration of the relevant player by considering if the instance of the GlowPlayer player passed in rightClickBlock is sneaking. The switch statement will determine when canPlaceAt is called for the specified targetMat. BlockSign now returns True if it can attach to the block, or if the target material is that of SIGN_POST or WALL_SIGN.

Also, when determining the target material, would it be best to place this in a method within BlockType since we are determining target material in BlockSign as well in canPlaceAt?

Thanks!

@Pr0methean Pr0methean merged commit 0786606 into GlowstoneMC:dev Apr 25, 2018

2 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details
@momothereal

This comment has been minimized.

Member

momothereal commented Apr 26, 2018

Thank you for your valuable contributions to the project so far 😃. We have invited you as a member of the Glowstone Project organization; you should receive an email from Github shortly.

In addition, we'd like to invite you to join our Discord server, where you'll be granted the Developer role. Thanks!

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