Skip to content

Conversation

@Owen1212055
Copy link
Contributor

Minecraft diff

Would love feedback on the method pruning, not sure if there is a prefered way I should be doing this.
Additionally, I have to store a flag that a method wrapper is a compact constructor as I prune the fields making the check no longer valid later in the code.

@Owen1212055 Owen1212055 changed the base branch from master to develop/1.12.0 July 2, 2025 23:51
Copy link
Member

@jaskarth jaskarth left a comment

Choose a reason for hiding this comment

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

Generally this looks good! I just have one small comment here. We could also hide canonical constructors that have no code other than the field assignments, but we can do that later.

return !hideMethod;
}

private static int getParamCount(StructMethod mt, TextBuffer buffer, int indent, MethodWrapper methodWrapper, MethodDescriptor md, boolean isEnum, boolean init, boolean thisVar, GenericMethodDescriptor descriptor, int paramCount, boolean isInterface, int flags, StructClass cl) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this could use a slightly more descriptive name, since it has side effects as well as returning the param count.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Yeah, this was certainly not the correct name.

@jaskarth jaskarth added Type: Enhancement New feature or request Subsystem: Writing Anything concerning how expressions are written Priority: Medium Medium priority labels Aug 14, 2025
@Owen1212055
Copy link
Contributor Author

Generally this looks good! I just have one small comment here. We could also hide canonical constructors that have no code other than the field assignments, but we can do that later.

We do that already, there is one place that doesn't occur and that's because there is an annotation on the constructor.

Copy link
Member

@jaskarth jaskarth 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, thanks a lot!

@jaskarth jaskarth merged commit 37737b0 into Vineflower:develop/1.12.0 Aug 21, 2025
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: Medium Medium priority Subsystem: Writing Anything concerning how expressions are written Type: Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants