Added permission system #106
No reviewers
Labels
No labels
complexity
high
complexity
low
complexity
medium
importance
high
importance
low
importance
medium
type
bug
type
chore
type
feature suggestion
type
missing feature
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
thetxt/oxide!106
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "Fix448/oxide:permission"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Added permission system with /op and /deop command
@ -138,6 +139,7 @@ impl Clone for Player {dimension: self.dimension.clone(),loaded_chunks: self.loaded_chunks.clone(),portal_cooldown: self.portal_cooldown,permission: Permission::Everyone,this should be self.permission
thanks for contributing! Please talk to me first in the future, to ensure that I can merge your changes.
From my quick look I only found one minor thing that should be fixed (attached a comment in the review).
Other than that I think this is missing saving/loading permissions to/from disk. Ideally using the vanilla ops file format
Added permission systemto WIP: Added permission systemWIP: Added permission systemto Added permission systemWould it be OK to use serde and serde_json for the ops.json file?
@Fix448 wrote in #106 (comment):
I would prefer to not add additional dependencies if we can avoid it
I don't think we can avoid it in the long run.
okay, fine
sorry for the delay... I think it makes more sense to load the permissions of a player from disk when they join as a part of lib::types::player::Player::new
And then everything that is now in lib/src/config/ops.rs should probably just be part of a new lib/src/types/permissions.rs file with the enum definition from lib/src/types/command.rs also merged in there
Then the player should probably have set_permission method that also calls the appropriate function to save to disk. This way there is only one function call to set a permission and save it, instead of having to think of both.
Please let me know if overlooked something or if anything I wrote didnt make sense
I just wanted to ask why you have so many diffrent permissions because in minecraft there are only Player Operator and Console. An if oxide in the future maybe gets a plugins system or like that a person can make a luckperms port or smth like that.
Minecraft actually has all of these permissions internally, as described on the corresponding Minecraft Wiki page, but you can only change them by editing the file.
yeah thats correct, but it reminds that I do manually send the EntityEvent packet to set the permission level to 4. This should also be updated to send the correct level down to the client
I found those in the Player::change_dimsion Player::respawn and new_player.rs files (or grep for //set op permission level 4)
While developing, I ran into two issues:
@thetxt wrote in #106 (comment):
In my tests, this new method was only called if the player had not already joined (I don't know if this is intended).
This method isn't complete, because I cannot send the
lib::packets::clientbound::play::Commandspacket from the lib crate, as theget_command_packet_datais in the server cratelooks good so far, will do some testing in the following days and then merge. Thanks :)
just did some testing and everything worked fine, just that there is no autocomplete for the /op and /deop command players names and that the search is also case sensitive (although I think thats also the case in vanilla, based on short testing).
According to the generated files from the official server they use the game profile parser, although there seems to be some special sauce to autocomplete existing ops in the /deop command. But I think that isn't very important right now