Skip to content

Place blocks against actual hit face on non-full blocks - #3642

Merged
IntegratedQuantum merged 5 commits into
PixelGuys:masterfrom
yel0h:non-full-blocks
Oct 4, 2026
Merged

IntegratedQuantum merged 5 commits into
PixelGuys:masterfrom
yel0h:non-full-blocks

Conversation

@yel0h

@yel0h yel0h commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Added a placementMode block property (.boundingBox or .gridNeighbor, defaults to .boundingBox) so placement now uses the hit selection bounding box face for the neighbor cell by default. The carpet rotation blocks set this to .gridNeighbor since the old grid-crossing behavior is intentional for placing carpets next to each other.

Fixes #3520

@Wunka Wunka moved this to Easy to Review in PRs to review Sep 28, 2026
@IntegratedQuantum

Copy link
Copy Markdown
Member

I'll put this in waiting for artistic review to make a decision on whether this is good or not.

I personally don't think it's a good idea to tie it to a rotation mode, the interface is already complex and full of side effects, if someone e.g. wants to add a snow layer block with the same properties, then we have to introduce a new rotation mode just for that.
I think this would make more sense as a block property.

@careeoki careeoki left a comment

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.

Yes, feels so much better. Now you can bridge outwards with branches and such.
And yeah I think it as a block property makes sense

@careeoki careeoki moved this from Waiting for artistic review to In review in PRs to review Oct 2, 2026
@IntegratedQuantum

IntegratedQuantum commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Also I'm not really a fan of the naming here. Maybe it would be more helpful to use an enum here, e.g.

.placementMode = .faceNormal,
.placementMode = .gridNeighbor .gridFace .whatever?,

@yel0h

yel0h commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Alright, it's an enum now

@IntegratedQuantum IntegratedQuantum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also just a heads up, this has conflicts with #3648, and since #3648 is important for accessories, I would prefer to merge it first.

Comment thread src/renderer.zig Outdated
// TODO: Test entities
}

fn dominantAxisNeighbor(normal: Vec3f) Vec3i {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This has edge cases that are very unintuitive (e.g. grass vegetation and flowers have tilted faces which do not work well at all with this).

@yel0h

yel0h commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Sure, I'll wait for the merge
As for the edge cases, I made it only use the face normal when it is (at least close to) axis-aligned.

@IntegratedQuantum

Copy link
Copy Markdown
Member

In my opinion normal based placing feels rather unintuitive, I'd expect it to be placed based on the bounding box instead, otherwise it depends on surface details that may not even be highlighted.
https://github.com/user-attachments/assets/b3969c69-9a35-49c6-ae03-a04f697f9d09

Also I found even more edge cases (e.g. chains), where (because the textures have gaps) it doesn't even use the normal of the face you are seemingly pointing at.

@yel0h

yel0h commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Yep works way better using the bounding box

@IntegratedQuantum

Copy link
Copy Markdown
Member

#3648 is merged, please rebase

@yel0h

yel0h commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Rebased

@IntegratedQuantum
IntegratedQuantum merged commit 1adf75e into PixelGuys:master Oct 4, 2026
3 checks passed
@yel0h
yel0h deleted the non-full-blocks branch October 4, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Blocks are placed incorrectly next to a non-full block

4 participants