Paper icon indicating copy to clipboard operation
Paper copied to clipboard

Check minecraft.commandblock permission in canUseGameMasterBlocks (Improve commandblock permission)

Open Remynfv opened this issue 3 years ago • 6 comments

The Problem

The existing minecraft.commandblock permission only allows you to edit command blocks. It looks like it was meant to also allow for breaking/placing command blocks, but in it's current state it does not.

There are also other blocks that require op level 2 or higher to break, place, and use. Currently however, there is no permission to allow this.

The (Proposed) Solution

Upgrade the existing minecraft.commandblock permission to allow all of these things. If a player can edit command blocks, they can (presumably) create/break/modify other command blocks via commands like /setblock.

This PR changes the Player.canUseGameMasterBlocks function to return true when the player has the minecraft.commandblock permission. It also removes all the old code (save the permission registration.) Most of the command-block-specific code seems to be broken, as you can't currently place/break command blocks with the permission.

Things to Consider

One small side effect of this specific implementation of the change, is that minecraft.commandblock will also allow use of debug sticks while in creative mode, because this also checks canUseGameMasterBlocks.

This would also cause it to override the minecraft.nbt.place permission. Whether this should be considered a significant drawback is up for discussion. My thought is that command blocks can be every bit as destructive as NBT, but maybe server owners attempting to sandbox command blocks could have issues with this?

Potentially these could all be split into different permissions, but I don't see much of a point when command blocks can already do all of these things on their own. If someone needs that particular behavior, it wouldn't be hard to implement as a plugin.

Only other downside I can see is that minecraft.commandblock is a bit misleading, since it would now include two other blocks, as well as the debug stick.

Remynfv avatar Oct 10 '22 00:10 Remynfv

The existing minecraft.commandblock permission only allows you to edit command blocks. It looks like it was meant to also allow for breaking/placing command blocks, but in it's current state it does not.

I thought the permission was supposed to restrict people who could already modify command blocks from being able to modify them. Like you would remove the permission from players you didn't want to be able to edit command blocks, but still met all the other requirements, like instabuild (creative) and perm level 2.

EDIT: After examining the conditions required for place/break/edit of command blocks, it looks like in all 5 uses of that permission, if Player#canUseGameMasterBlocks returns false, it then checks if the player is in creative and has the permission. So in effect it is either player.canUseGameMasterBlocks() || (player.isCreative() && player.hasPermission(cmdBlockPerm)) or as its shown in the code !player.canUseGameMasterBlocks() && (!player.isCreative() || !player.hasPermission(cmdBlockPerm)) which just the inverse

EDIT #2: Ok, there is at least one issue I DO see. GameMasterBlockItem needs to include the permission check if its a command block. Cause right now you can't place it. You can destroy it however if you are a non-opped player in creative with the permission (which is I think how its supposed to be used)

Machine-Maker avatar Oct 10 '22 18:10 Machine-Maker

Hmm, it makes sense that you'd want to prevent people from modifying command blocks. There should definitely be a way to allow non-opped players to use all the GameMasterBlocks though.

I thought the permission was supposed to restrict people who could already modify command blocks from being able to modify them. Like you would remove the permission from players you didn't want to be able to edit command blocks, but still met all the other requirements, like instabuild (creative) and perm level 2.

I'm not super familiar with how op permission levels are handled in regards to custom permissions. I thought if you were opped at all you would always return true for permission checks.

This solution seemed intuitive to me, but maybe there is a different way to give the ability to use GameMasterBlocks?

Remynfv avatar Oct 10 '22 23:10 Remynfv

Hmm, it makes sense that you'd want to prevent people from modifying command blocks. There should definitely be a way to allow non-opped players to use all the GameMasterBlocks though.

The tricky thing is we would need to send to the client to update their op status as they aren't even able to place these down unless they have the proper operator status.

Owen1212055 avatar Nov 26 '22 01:11 Owen1212055

The tricky thing is we would need to send to the client to update their op status as they aren't even able to place these down unless they have the proper operator status.

Maybe a function on Player that just sends the packets? There's plenty of other API features that make client-side changes. Something like Player::sendOperatorStatus? Not sure what the best name would be though.

Remynfv avatar Jan 13 '23 23:01 Remynfv

Maybe a function on Player that just sends the packets? There's plenty of other API features that make client-side changes. Something like Player::sendOperatorStatus? Not sure what the best name would be though.

That already exists. Player#sendOpLevel

Machine-Maker avatar Jan 15 '23 21:01 Machine-Maker

That already exists. Player#sendOpLevel

Oh, haha. I had no idea.

So, what needs to be figured out before this PR can be merged?

Server owners can send players their OP level, and grant them the permission. I think that works just fine. However, I can see how the "two steps" part might be confusing for users.

One alternative, to prevent dealing with OP levels and permissions not matching, would be to just have a line in the server config to allow non-opped command block use. It would be less flexible than a permission, but would probably do the job in most situations, and could make it easier to understand for server owners.

Remynfv avatar Jan 16 '23 21:01 Remynfv

The key problem here is that there's no way to place command blocks and other gamemaster blocks without being fully opped. Right now my jank workaround is to listen for interact events, get all the block state data from the held item, and place a command block approximately where you clicked. This could all be saved if there was a way to place command blocks and gamemaster blocks without OP!

Remynfv avatar Nov 07 '23 07:11 Remynfv