FabricMC / yarn

Libre Minecraft mappings, free to use for everyone. No exceptions.
Creative Commons Zero v1.0 Universal
908 stars 376 forks source link

Block.skipRenderingSide -> isSideInvisible #654

Closed Runemoro closed 5 years ago

Runemoro commented 5 years ago

The current name makes it seem like the method should return true if the side is being culled, but in fact, the method only returns true for transparent blocks that merge when placed together, such as ice and glass.

ChloeDawn commented 5 years ago

I think something like shouldCullFace/shouldCullSide would be more suitable. This check is used for manually stepping over the smart occlusion run within Block.shouldDrawSide.

Fwiw I personally prefer referring to "sides" as "faces" in this context, but that can go up for debate another time.

Runemoro commented 5 years ago

I think something like shouldCullFace

No, the problem is that the method is not doing culling. It's checking if the side is invisible because of the glass blocks merging together. Slime blocks are also transparent, but return false, so we can see all their sides when placed side by side.

This is a method used to change the appearance of the block, not culling logic.

asiekierka commented 5 years ago

It's checking if the side is invisible

Isn't that the definition of culling logic? A block explicitly not using culling logic does not mean that's not culling logic.

Runemoro commented 5 years ago

Culling means hiding faces that can't be seen. In the case of glass, the face should be seen, but it actually doesn't exist because of the way glass works in Minecraft.

asiekierka commented 5 years ago

Ah, I see.

sfPlayer1 commented 5 years ago

If this is what I think it is, it causes the adjacent (neighbor) block model's "cullface" to be skipped/removed. Considering the name in block model jsons is cullface and the operation is usually for regular culling, with some abuse for glass "merging", something like shouldCullNeighborFace, cullsNeighborFace or similar sounds ideal to me.

Runemoro commented 5 years ago

the operation is usually for regular culling, with some abuse for glass "merging"

It is ONLY used for glass, ice and liquid merging.