Added native operator permission management #348
Conversation
This commit adds the ops.json configuration found in vanilla servers, and updates the basic configuration to handle the default permission level. Server now uses ops file and defaults players to op level 0 The /op command has been added but needs players to rejoin for now.
I found the source of the DeadLock. Updated set permission function. Fix cargo formatting issues and clippy issues.
|
You added a bunch of deps and put OP into pumpkin-config. This is not a Config. It is used as Runtime data which can be changed. Im not sure if we should handle OPs per Server or per World base. |
Per server sounds more accurate but I'm not sure what you're getting at. I initially added it to config because, from my perspective, it is a config. But I can see what you're saying. But I'm not seeing the immediate solution. What am I changing? Is there anything specific I should change? |
As Snowiii pointed out, OperatorConfig is runtime data. Revert most changes in pumpkin-config. op_permission_level must say in the basic configuration for parity with Minecraft.
KairuDeibisu
changed the title
- Move PermissionLvl to core and removed OpLevel - Move ops.json to /data/ops.json - Fix permissions issue with /op command - Shorten /op command description
KairuDeibisu
force-pushed
the
feature-permissions
branch
from
fd194e3 to
f49fe5a
Compare
|
Is there a way to change default op? for instance, on the test server, if we want to go to creative mode it would no longer be possible without an admin to give op permission. Maybe the setting could configure the default permission to be op level |
| uuid: Uuid, | ||
| name: String, | ||
| level: PermissionLvl, | ||
| bypasses_player_limit: bool, |
There was a problem hiding this comment.
prefer to use enums instead of bools, the function call is hard to read:
Op::new(..., false)is hard to read
| Path::new("data/ops.json") | ||
| } | ||
| fn validate(&self) { | ||
| // TODO: Validate the operator configuration |
| 2 => Ok(PermissionLvl::Two), | ||
| 3 => Ok(PermissionLvl::Three), | ||
| 4 => Ok(PermissionLvl::Four), | ||
| _ => Err(serde::de::Error::custom(format!( |
There was a problem hiding this comment.
you can use serde::de::Error::invalid_value instead here
This commit adds the ops.json configuration found in vanilla servers, and updates the basic configuration to handle the default permission level.
Server now uses ops file and defaults players to op level 0
The /op command has been added.
Testing
Before starting the server:
Start server:
/op JustATempest; this should fail./op JustATempestin the console; this should work.Testing Results
Design challenges
Operator Level Management
To maintain parity with Minecraft, the operator level must be configurable within the basic configuration. Initially, the idea of introducing a new operator level type and conversions was explored. However, this approach led to duplicated code and complexity. Instead, it was suggested to centralize permission levels within the core, as the configuration system already depends on the core module.
Operator List (ops.json)
For parity with Minecraft, an ops.json file is required to store the list of operators on the server. This file is loaded at startup, and its data is referenced whenever a player is constructed. If a player’s UUID is not found in the list, they are assigned the default permission level of 0, which matches Minecraft’s behavior and cannot be changed.
/op Command
The /op command is also necessary for parity. This command accepts a single argument: the target player's name. A player must have a permission level of 3 or higher to use this command. Executing /op adds the target player to the operator list with the default permission level specified in the basic configuration.
Edge Case: Overlapping Permission Levels
An edge case exists where a player with permission level 3 could use /op to grant another player a higher permission level (e.g., 4). To resolve this, the command checks the sender’s permission level against the default operator level in the basic configuration and applies the lower value.
Session Update
After a player is added to the operator list, their current session must reflect the updated permission level. This required adding a mutex to synchronize access to the permission level within the player structure.
Command Builder Limitation
This change introduced a side effect: the require function in the command builder does not support async functions. To address this, we opted to use the parking_lot crate instead of tokio, avoiding the need for async and await in the builder.
Fixes
Checklist
Things need to be done before this Pull Request can be merged.
cargo fmtcargo clippycargo test