-
-
Notifications
You must be signed in to change notification settings - Fork 145
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
PlaceBlock #511
base: main
Are you sure you want to change the base?
PlaceBlock #511
Conversation
impl PlacementRule { | ||
//TODO: WIP | ||
pub const fn from_kind(kind: BlockKind) -> Self { | ||
match kind { |
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.
Unfortunately this approach isn't really maintainable. I don't want to manually update this list every time a new Minecraft version is released.
I think we should investigate how Vanilla is mapping the blocks to their placement rules and extract that information if possible. If not, then perhaps we could use some heuristic based on properties, but this is not my desired solution.
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.
I don't think there's much room for alternatives, if we want to mimic the client behaviour. Minecraft seems to hardcode these rules for these sets of blocks as well. As you said these rules are very complicated and I don't see a good way around this. Having a good default, recognizing similarities and thus being able to deduplicate code are things I try to do and hopefully can limit the maintanence cost. Also adding new blocks to these rules shouldn't be too much work.
Only alternative I can think of is dropping this in the scope of Valence entirely and make a crate for vanilla behaviour, which is allowed to be behind Valence and the newest Minecraft update a bit.
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.
Yeah minecraft implements placement behavior for each block by implementing this function:
public BlockState getPlacementState(ItemPlacementContext ctx)
For example, this is the impl in EnderChestBlock
:
public BlockState getPlacementState(ItemPlacementContext ctx) {
FluidState fluidState = ctx.getWorld().getFluidState(ctx.getBlockPos());
return (BlockState)((BlockState)this.getDefaultState().with(FACING, ctx.getHorizontalPlayerFacing().getOpposite())).with(WATERLOGGED, fluidState.getFluid() == Fluids.WATER);
}
It might be feasible to have some codegen that reads the with()
calls and derives the correct placement rule for the block. eg, if with(FACING, ...)
is present, the block probably should face the player.
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.
so extracting properties ? cause we already extract them, but yea I don't see placement rules that couldn't be derive from some properties.
I don't know if it would be a good idea, but we could bruteforce blockstate for every context them infer the rules from that
Objective
Fixes #503
Solution
Adds
PlaceBlockCommand
(used withCommand::add
), which places a block according to infos about itself, the entity placing the block and its environment. Tries to mimic client behaviour as close as possible.This is the foundation and definitely hasn't all rules implemented. It's a proposal for the form and helper structs/functions to make implementing the rest of the rules simpler and more elegant. I tried to document my sources, open problems and other thoughts in the comments.
Maybe it makes sense to merge this is in a unfinished state, when most common interactions are handled and it generelly is preferable over
set_block
. At the moment it uses a lot ofunimplemented!
to catch missing features, those could be converted into silent ones.