Skip to content
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

Add branch block type #1118

Merged
merged 25 commits into from
Mar 3, 2025
Merged

Conversation

Argmaster
Copy link
Contributor

@Argmaster Argmaster commented Feb 26, 2025

This pull request adds a branch block. Branch blocks work similarly to fences but connect in all 3 directions:

image

Image show a 3D cross made out of automatically connecting branch blocks.

image

Image shows a base form of branch block when not connected to any block, compared to solid oak log block.

image

Tree with detailed branches made out of oak branch block.

@ikabod-kee
Copy link
Collaborator

@careeoki opinion on this?

@careeoki
Copy link
Contributor

I would like this block implemented with a new rotation mode that connects to itself as proposed in #194. That would make it more versatile and unique.
Also, it should be called "oak branch"

@Argmaster
Copy link
Contributor Author

Do anyone know what creates those bush-like trees with one block trunks? I can't really trace that down 😄

image

@ikabod-kee
Copy link
Collaborator

Should just be trees that have a height of 1

@Argmaster Argmaster changed the title Add oak pillar Add branch block type Feb 28, 2025
@Argmaster
Copy link
Contributor Author

Let me ping you guys 😄
@ikabod-kee @careeoki @IntegratedQuantum

I consider the texture temporary, It definitely needs love I can't give it no matter how hard I try.

@ikabod-kee
Copy link
Collaborator

Looks good. I agree that Carrie should probably do a PR for its textures, or it could just use the existing wood textures.

@Argmaster Argmaster mentioned this pull request Feb 28, 2025
@Argmaster
Copy link
Contributor Author

It can't use current wood textures as they are one directional (top-down) and this is two directionaly (top-down AND left-right), It looks awkward, I have checked and thats why I committed that atrocity of a texture 😅

@Argmaster
Copy link
Contributor Author

It could probably also use a item texture, because right now this is what you see:

image

@careeoki
Copy link
Contributor

This is great! I can start working on the textures for all the branch types, as well as their item textures. I would add those in a followup pull request so you don't have to wait on me.

One thought though:
Would it be possible for branches to not always connect in all directions, to allow for some more detailed shapes (without it turning into a blob of branch)
current:
image
ideally:
image

I'm imagining a mix between the log rotation and the branch rotation. I don't know how doable this is but I think it would make branches a lot more useful
image

Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

It would also be great if you could figure out Carrie's suggestion. That would probably require working with the generateData function as well.

@Argmaster
Copy link
Contributor Author

Argmaster commented Feb 28, 2025

The directional thing is tricky, because you would have to stop a block from being updated in a situation like this:

image

This is currently not possible because you don't know the source of block update. If I understand correctly updateData applies to all blocks surrounding the block placed, hence if we create u and place a block on top of it we will cause unconditional block update to on existing blocks converting u to b shape, even though we set the block data correctly previously. To avoid that we would have to know where this block update comes from, so to limit what updates are allowed, I don't think it is possible right now.

Logs work because existing logs don't have to be updated to connect to new logs, since they are full blocks and they are automatically connected to all neighbor blocks.

We would need something like directional, that do full block update only a block that is behind placing direction, while other blocks have partial updates if necessary (lighting and stuff? idk, didn't read all the code yet :P )

@IntegratedQuantum
Copy link
Member

The directional thing is tricky, because you would have to stop a block from being updated in a situation like this:

Consider the following approach:
Untitled

@Argmaster
Copy link
Contributor Author

Yee this is why you are the maintainer and I am the contributor 😄

@Argmaster Argmaster closed this Feb 28, 2025
@Argmaster Argmaster reopened this Feb 28, 2025
@Argmaster
Copy link
Contributor Author

For documentation purposes here is the result:

image

@careeoki
Copy link
Contributor

Branches are connecting to non-full blocks:
image

@Argmaster
Copy link
Contributor Author

Fixed:

image

Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

Some proposals to simplify the code:
Instead of defining your own packed struct, you could use just a u6 and Neighbor.bitMask to avoid those big, space-consuming switches. And it would also avoid some of the bitcasting.

To change a direction value:

currentData &= ~bitMask;
if(condition) currentData |= bitMask;

To check if a direction is set:

if(data & Neighbor.dirNegX.bitMask() != 0)

@IntegratedQuantum
Copy link
Member

I don't think the logs should connect to any viewThrough blocks like glasss or leaves:
Screenshot at 2025-03-01 10-16-18

Also I think it might be better to not automatically connect them to all neighboring solid blocks.
Instead I would propose to add a way to manually add more connections. e.g. in the following image, right clicking should place a connection between the two branches:
Screenshot at 2025-03-01 10-17-08

@Argmaster
Copy link
Contributor Author

Okay, so here are the changes:

  • branches now connect to the block they are placed on if it is sold or also branch
  • branches do not automatically connect to other blocks around
  • you can connect branch to another branch or solid block by right clicking it with a branch block
  • you can disconnect branch block from whatever it is connected to by hitting the connection
  • you can destroy branch by destroying its middle part

When destroying branch from below the hitbox is funky, but I can't figure out why, Im talking about situation like this:
image

When (1) I break the connection on top of bottom block, for some reason whole branch gets destroyed as if i were aiming at the center of it. When (2) I destroy connection from the center block, everything works fine.

1:

image

2:

image

Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

I think we shouldn't have the extra face on the outside, if this is used everywhere in the forest, we need to save every triangle we can. Sure it disables some potential future terrain options, but if we ever do want them I'd prefer adding them with an extra bit for it in the rotation mode, instead of always dragging around these extra faces.

Furthermore I still think that branches should not connect to viewThrough blocks, this is even more true with the extra face due to z-fighting:

Screenshot at 2025-03-01 21-44-28

@Argmaster
Copy link
Contributor Author

Extra faces removed, code cleaned.

If we don't connect to viewThrough blocks player can never connect a branch to leaves. You can generate them in worldgen since you don't cause block updates and you can insert any data you want, but it is kind of inconvenient. Maybe we could add exception for them at some point?

@careeoki
Copy link
Contributor

careeoki commented Mar 1, 2025

Yeah, I think branches connecting to leaves is important. I didn't find it too noticeable that there was open faces when connecting to leaves, since the leaf texture is in the way.

@IntegratedQuantum
Copy link
Member

IntegratedQuantum commented Mar 2, 2025

Yeah, I think branches connecting to leaves is important.

I would prefer branches having like one leaf block around it if they are inside the tree. This would also allow for branches without making the leaf culling any less efficient. (but that shouldn't be done in this PR, since it requires some other stuff)

@Argmaster
Copy link
Contributor Author

Ok, I think we should keep it as is for now (ie. no player created leaves connections) and possibly tweak it further down the road, so this PR can be finally merged. We will see how important that actually is after biomes and structures are adapted to presence of new block. If players even notice this we will surely get an issue for that anyway.

Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

some more small things

@IntegratedQuantum
Copy link
Member

Please rebase the PR to fix merge conflicts, and also fix all the formatting issues that will likely follow.

@Argmaster
Copy link
Contributor Author

Argmaster commented Mar 2, 2025

That:

 		fn branchTransform(quad: *main.models.QuadInfo, data: BranchData) void {
 			for(&quad.corners) |*corner| {
-				if(
-					(!data.isConnected(Neighbor.dirNegX) and corner[0] == 0)
-					or (!data.isConnected(Neighbor.dirPosX) and corner[0] == 1)
-					or (!data.isConnected(Neighbor.dirNegY) and corner[1] == 0)
-					or (!data.isConnected(Neighbor.dirPosY) and corner[1] == 1)
-					or (!data.isConnected(Neighbor.dirDown) and corner[2] == 0)
-					or (!data.isConnected(Neighbor.dirUp) and corner[2] == 1)
-				) return degenerateQuad(quad);
+				if((!data.isConnected(Neighbor.dirNegX) and corner[0] == 0) or (!data.isConnected(Neighbor.dirPosX) and corner[0] == 1) or (!data.isConnected(Neighbor.dirNegY) and corner[1] == 0) or (!data.isConnected(Neighbor.dirPosY) and corner[1] == 1) or (!data.isConnected(Neighbor.dirDown) and corner[2] == 0) or (!data.isConnected(Neighbor.dirUp) and corner[2] == 1)) return degenerateQuad(quad);
 			}
 		}

ain't happening.

Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

Functionally it seems to work great now, just a few more small fixes and I am ready to merge this.

Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

Looks good now, thank you for working on this, and thank you for bearing through the review process.

@IntegratedQuantum IntegratedQuantum merged commit c4a4d29 into PixelGuys:master Mar 3, 2025
1 check passed
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.

4 participants