-
-
Notifications
You must be signed in to change notification settings - Fork 200
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
Fix issue #574 "All torches place in a wall state" #618
base: master
Are you sure you want to change the base?
Conversation
# Conflicts: # pumpkin/src/block/mod.rs # pumpkin/src/block/properties/mod.rs
marked as draft as the new block property stuff has partially fixed this |
Maybe it can be reworked using the new block property code? |
@lwalton101 The properties recently got reworked by @4lve. Can you please use the new system |
pumpkin/src/block/blocks/torch.rs
Outdated
} | ||
} | ||
|
||
return if possible_directions.contains(_player_direction) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_player_direction should be player_direction
pumpkin/src/block/blocks/torch.rs
Outdated
} | ||
} | ||
|
||
return if possible_directions.contains(_player_direction) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_player_direction should be player_direction
pumpkin/src/block/blocks/torch.rs
Outdated
} | ||
} | ||
|
||
return if possible_directions.contains(_player_direction) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_player_direction should be player_direction
Also why you copied the same thing 3 times ?. You could just call one function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the ...LikeProperties
should be type aliased into a definitive type, because although the changes are relatively low, a minecraft update can change those names. Type aliasing will make this easier to fix when breaking updates happen.
block_pos: &BlockPos, | ||
face: &BlockDirection, | ||
player_direction: &HorizontalFacing, | ||
mut block_properties: Vec<(String, String)>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of serializing the block properties, you should mutate the LikeProperties
struct itself, which contains enums of valid states.
I have created a trait for blocks which either attach vertically to a wall or the ground. For example, a torch block can either be wall_torch or just torch.
The trait will place the standing torch if it is placed on the right face and if it places a wall attached torch it inverts the face so that the torch faces the correct way.
This issue was reported in Issue #574
Testing
All 3 torch types are working
Please follow our Coding Guidelines