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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What about isFaceFull though?

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.

It can have the same thing done to it as well? You literally just do isFaceFull(getShape(shapeType), face) and do not store the shape in any way.

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.

@me4502

me4502 commented Aug 4, 2026

Copy link
Copy Markdown
Member

Also alongside octy's review, this currently does not compile, if you'd be able to please fix that too

@TomyLobo
TomyLobo force-pushed the refactor-block-material branch from 1cc4e70 to 2dbb87d Compare August 16, 2026 11:21
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.

3 participants