Skip to content

Conversation

@MartijnMuijsers
Copy link
Contributor

@MartijnMuijsers MartijnMuijsers commented Jan 8, 2026

This removes hard-coded max stack sizes from Material, and defers to ItemType instead.

I've kept the old behavior of returning 64 for all blocks (since maxStack was initialized to that value for all blocks) and 0 for LEGACY_AIR, to prevent old plugins that expect those values from breaking. This could be replaced by

Preconditions.checkArgument(type != null, "The Material is not an item!");

but that could break legacy plugins, which is the whole reason for maintaining Material I suppose.

EDIT: This would break max stack sizes for legacy materials because legacy conversion is currently broken, but that is fixed by #13529

This removes hard-coded max stack sizes from Material, and defers to ItemType instead.

I've kept the old behavior of returning 64 for all blocks (since maxStack was initialized to that value for all blocks) and 0 for LEGACY_AIR, to prevent old plugins that expect those values from breaking.
This could be replaced by
```
Preconditions.checkArgument(type != null, "The Material is not an item!");
```
but that could break legacy plugins, which is the whole reason for maintaining `Material` I suppose.
@MartijnMuijsers MartijnMuijsers requested a review from a team as a code owner January 8, 2026 18:20
@github-project-automation github-project-automation bot moved this to Awaiting review in Paper PR Queue Jan 8, 2026
@Lulu13022002
Copy link
Contributor

Might as well fix the legacy material conversion (didn't really cared about it at the time) for example take the following in the TestPlugin (onEnable):

Material.LEGACY_RECORD_3.getMaxStackSize() <-- return 1 OK
Material.LEGACY_RECORD_4.getMaxStackSize() <-- return 64 WRONG

In all case you would need to update the generator for the non legacy material.

@MartijnMuijsers
Copy link
Contributor Author

MartijnMuijsers commented Jan 9, 2026

@Lulu13022002 Done! #13529

@Lulu13022002
Copy link
Contributor

Thanks, can you update the generator too for this PR?

@MartijnMuijsers
Copy link
Contributor Author

MartijnMuijsers commented Jan 9, 2026

Of course! I've updated the generator, but I'm unfamiliar with if there is currently any test to verify it~ (apart from a review)

@Lulu13022002
Copy link
Contributor

The way to test it is generally to run it, normally you would have used the generator in the first place ^^

@MartijnMuijsers
Copy link
Contributor Author

I don't know how to run it haha. I've tried running all the tasks under generation but they have no effect, I'm not sure where the output ends up. I can't find any documentation about it.

@Lulu13022002
Copy link
Contributor

Ah sorry the only documentation for this is on my original commit (which has probably been squashed during the update). Make sure to uncomment the line in paper-generator.settings.gradle.kts to include the project then you want the rewriteApi task (because Material is part of the api and you want to generate content between the comment markers).
The changes are applied directly so if you see no difference then it probably works but you can just delete all the constants to make sure it works (not the legacy tho). From my test it works fine!

@MartijnMuijsers
Copy link
Contributor Author

I did get :paper-generator:rewriteApi running successfully by the way, but it didn't add or modify any files for me. Is it supposed to overwrite Material.java?

@Lulu13022002
Copy link
Contributor

The changes are applied directly so if you see no difference then it probably works but you can just delete all the constants to make sure it works (not the legacy tho). From my test it works fine!

Yes it's fine you already did what the generator was already supposed to generate but manually.

@Lulu13022002 Lulu13022002 merged commit b5e7257 into PaperMC:main Jan 17, 2026
4 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting review to Merged in Paper PR Queue Jan 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

2 participants