Skip to content

Refactor block material - #2997

Open
TomyLobo wants to merge 2 commits into
masterfrom
refactor-block-material
Open

Refactor block material#2997
TomyLobo wants to merge 2 commits into
masterfrom
refactor-block-material

Conversation

@TomyLobo

@TomyLobo TomyLobo commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator
  • Add BlockState.getMaterial()
  • Add ShapeType parameter to BlockMaterial.isFullCube

@TomyLobo
TomyLobo requested a review from a team as a code owner August 2, 2026 12:53
@TomyLobo
TomyLobo changed the base branch from version/7.3.x to master August 2, 2026 13:00
]
}
],
"Why on earth is this even an issue?": [

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.

If someone was implementing BlockMaterial, this can sometimes cause weird issues that I don't recall exactly. But we should have the actual reasoning here, e.g.

Suggested change
"Why on earth is this even an issue?": [
"No one should be implementing BlockMaterial": [


import java.util.EnumSet;

public abstract class AbstractBlockMaterial<VS> implements BlockMaterial {

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.

Instead of having a VS parameter, can we just push getShape into isShapeFullBlock? There doesn't really seem to be an advantage to having it here.

Also, we should probably just call isShapeFullBlock -> isFullCubeUncached?

Comment on lines +22 to +24
public enum ShapeType {
SHAPE,
VISUAL_SHAPE,

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 should explain what the difference is, and what a ShapeType is generally -- i.e., all members and the class should be documented.

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.

2 participants