Skip to content

Conversation

@Mickey42302
Copy link

I would like to suggest adding permissions for more of the gamemaster blocks that are included in Java Edition. The patch I created adds the following nodes:

• Structure Block ("minecraft.structureblock")

• Jigsaw Block ("minecraft.jigsawblock")

• Light Block ("minecraft.lightblock")
Note: This controls whether or not players can change the light level by interacting with a Light Block with one in their hand.

• Lectern Block ("minecraft.lecternblock")
Note: This permission node controls whether or not a lectern with NBT can be placed. Regular players can still place the block with no NBT.

• Test Block ("minecraft.testblock")

• Test Instance Block ("minecraft.testinstanceblock")

@Mickey42302 Mickey42302 requested a review from a team as a code owner November 11, 2025 02:44
@github-project-automation github-project-automation bot moved this to Awaiting review in Paper PR Queue Nov 11, 2025
@electronicboy
Copy link
Member

shouldn't be a feature patch, requireNonNull is dangerous, perm node names leave something to be desired

@Mickey42302
Copy link
Author

What do you think would be a better name for the permission nodes?

As for "requireNonNull", the IDE suggested that I add that to avoid a NullPointerException.

@Toffikk
Copy link
Contributor

Toffikk commented Nov 12, 2025

Its better to use Preconditions.checkState(x != null, “message”) than requireNonNull, its even outlined in the CONTRIBUTING.md file iirc

@electronicboy
Copy link
Member

requireNonNull, works by throwing an exception if the value is null, as does the preconditions, we shouldn't be introducing throws to such a trivial state, validate it

@Toffikk
Copy link
Contributor

Toffikk commented Nov 12, 2025

yea that might be smarter, also what is the reason for the unnecessary assert? it should be removed if im seeing correctly

@Mickey42302
Copy link
Author

What do you mean by assert?

BlockEntity blockEntity = level.getBlockEntity(pos);
- if (blockEntity instanceof JigsawBlockEntity && player.canUseGameMasterBlocks()) {
+ if (blockEntity instanceof JigsawBlockEntity && player.canUseGameMasterBlocks() || (player.isCreative() && player.getBukkitEntity().hasPermission("minecraft.jigsawblock"))) {
+ assert blockEntity instanceof net.minecraft.world.level.block.entity.JigsawBlockEntity;
Copy link
Contributor

Choose a reason for hiding this comment

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

i mean this

@lynxplay
Copy link
Contributor

#9339 if we want to implement this, something along the line of this PR might be better. For now, I don't see much benefit in spamming permission checks for every game master block like that.

@lynxplay lynxplay closed this Dec 19, 2025
@github-project-automation github-project-automation bot moved this from Awaiting review to Closed in Paper PR Queue Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

4 participants