Fixs issues @Commandcracker found in review

- Move PermissionLvl to core and removed OpLevel
- Move ops.json to /data/ops.json
- Fix permissions issue with /op command
- Shorten /op command description

pumpkin-config/src/lib.rs

@@ -1,7 +1,6 @@
 use log::warn;
 use logging::LoggingConfig;
-use op::OpLevel;
-use pumpkin_core::{Difficulty, GameMode};
+use pumpkin_core::{permission::PermissionLvl, Difficulty, GameMode};
 use query::QueryConfig;
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 
@@ -79,7 +78,7 @@ pub struct BasicConfiguration {
     /// The default game difficulty.
     pub default_difficulty: Difficulty,
     /// The op level assign by the /op command
-    pub op_permission_level: OpLevel,
+    pub op_permission_level: PermissionLvl,
     /// Whether the Nether dimension is enabled.
     pub allow_nether: bool,
     /// Whether the server is in hardcore mode.
@@ -110,7 +109,7 @@ impl Default for BasicConfiguration {
             view_distance: 10,
             simulation_distance: 10,
             default_difficulty: Difficulty::Normal,
-            op_permission_level: OpLevel::Owner,
+            op_permission_level: PermissionLvl::Four,
             allow_nether: true,
             hardcore: false,
             online_mode: true,

pumpkin-config/src/op.rs

@@ -1,49 +1,27 @@
-use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use pumpkin_core::permission::PermissionLvl;
+use serde::{Deserialize, Serialize};
 use uuid::Uuid;
 
-#[derive(Clone, Copy)]
-#[repr(u8)]
-pub enum OpLevel {
-    None = 0,
-    Basic = 1,
-    Moderator = 2,
-    Admin = 3,
-    Owner = 4,
-}
-
-impl Serialize for OpLevel {
-    fn serialize<S>(&self, serializer: S) -> Result<<S as Serializer>::Ok, <S as Serializer>::Error>
-    where
-        S: Serializer,
-    {
-        serializer.serialize_u8(*self as u8)
-    }
-}
-
-impl<'de> Deserialize<'de> for OpLevel {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        let value = u8::deserialize(deserializer)?;
-        match value {
-            0 => Ok(OpLevel::None),
-            1 => Ok(OpLevel::Basic),
-            2 => Ok(OpLevel::Moderator),
-            3 => Ok(OpLevel::Admin),
-            4 => Ok(OpLevel::Owner),
-            _ => Err(serde::de::Error::custom(format!(
-                "Invalid value for OpLevel: {}",
-                value
-            ))),
-        }
-    }
-}
-
-#[derive(Serialize, Deserialize, Clone)]
+#[derive(Serialize, Deserialize, Clone, Default)]
 pub struct Op {
     pub uuid: Uuid,
     pub name: String,
-    pub level: OpLevel,
+    pub level: PermissionLvl,
     pub bypasses_player_limit: bool,
 }
+
+impl Op {
+    pub fn new(
+        uuid: Uuid,
+        name: String,
+        level: PermissionLvl,
+        bypasses_player_limit: bool,
+    ) -> Self {
+        Self {
+            uuid,
+            name,
+            level,
+            bypasses_player_limit,
+        }
+    }
+}

pumpkin-core/src/lib.rs

@@ -1,5 +1,6 @@
 pub mod gamemode;
 pub mod math;
+pub mod permission;
 pub mod random;
 pub mod text;
 

pumpkin-core/src/permission.rs

@@ -0,0 +1,55 @@
+use num_derive::{FromPrimitive, ToPrimitive};
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
+
+/// Represents the player's permission level
+///
+/// Permission levels determine the player's access to commands and server operations.
+/// Each numeric level corresponds to a specific role:
+/// - `Zero`: None
+/// - `One`: Basic
+/// - `Two`: Moderator
+/// - `Three`: Admin
+/// - `Four`: Owner
+#[derive(FromPrimitive, ToPrimitive, Clone, Copy, Default, PartialEq, Eq)]
+#[repr(i8)]
+pub enum PermissionLvl {
+    #[default]
+    Zero = 0,
+    Two = 2,
+    Three = 3,
+    Four = 4,
+}
+
+impl PartialOrd for PermissionLvl {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        (*self as u8).partial_cmp(&(*other as u8))
+    }
+}
+
+impl Serialize for PermissionLvl {
+    fn serialize<S>(&self, serializer: S) -> Result<<S as Serializer>::Ok, <S as Serializer>::Error>
+    where
+        S: Serializer,
+    {
+        serializer.serialize_u8(*self as u8)
+    }
+}
+
+impl<'de> Deserialize<'de> for PermissionLvl {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let value = u8::deserialize(deserializer)?;
+        match value {
+            0 => Ok(PermissionLvl::Zero),
+            2 => Ok(PermissionLvl::Two),
+            3 => Ok(PermissionLvl::Three),
+            4 => Ok(PermissionLvl::Four),
+            _ => Err(serde::de::Error::custom(format!(
+                "Invalid value for OpLevel: {}",
+                value
+            ))),
+        }
+    }
+}

pumpkin/src/command/commands/cmd_fill.rs

@@ -4,10 +4,11 @@ use crate::command::args::{ConsumedArgs, FindArg};
 use crate::command::tree::CommandTree;
 use crate::command::tree_builder::{argument, literal, require};
 use crate::command::{CommandError, CommandExecutor, CommandSender};
-use crate::entity::player::PermissionLvl;
+
 use async_trait::async_trait;
 use pumpkin_core::math::position::WorldPosition;
 use pumpkin_core::math::vector3::Vector3;
+use pumpkin_core::permission::PermissionLvl;
 use pumpkin_core::text::TextComponent;
 
 const NAMES: [&str; 1] = ["fill"];

pumpkin/src/command/commands/cmd_gamemode.rs

@@ -3,8 +3,8 @@ use async_trait::async_trait;
 use crate::command::args::arg_gamemode::GamemodeArgumentConsumer;
 use crate::command::args::GetCloned;
 
-use crate::entity::player::PermissionLvl;
 use crate::TextComponent;
+use pumpkin_core::permission::PermissionLvl;
 
 use crate::command::args::arg_players::PlayersArgumentConsumer;
 

pumpkin/src/command/commands/cmd_give.rs

@@ -9,7 +9,7 @@ use crate::command::args::{ConsumedArgs, FindArg, FindArgDefaultName};
 use crate::command::tree::CommandTree;
 use crate::command::tree_builder::{argument, argument_default_name, require};
 use crate::command::{CommandError, CommandExecutor, CommandSender};
-use crate::entity::player::PermissionLvl;
+use pumpkin_core::permission::PermissionLvl;
 
 const NAMES: [&str; 1] = ["give"];
 

pumpkin/src/command/commands/cmd_op.rs

@@ -5,16 +5,16 @@ use crate::{
         tree_builder::{argument, require},
         CommandError, CommandExecutor, CommandSender,
     },
-    entity::player::PermissionLvl,
     server::json_config::{SaveJSONConfiguration, OPERATOR_CONFIG},
 };
 use async_trait::async_trait;
 use pumpkin_config::{op::Op, BASIC_CONFIG};
+use pumpkin_core::permission::PermissionLvl;
 use pumpkin_core::text::TextComponent;
 use CommandError::InvalidConsumption;
 
 const NAMES: [&str; 1] = ["op"];
-const DESCRIPTION: &str = "Specifies one or more game profiles (player profiles). Must be a player name (should be a real one if the server is in online mode) or a player-type target selector";
+const DESCRIPTION: &str = "Grants operator status to a player.";
 const ARG_TARGET: &str = "player";
 
 struct OpExecutor;
@@ -35,28 +35,31 @@ impl CommandExecutor for OpExecutor {
 
         // log each player to the console.
         for player in targets {
-            let op_entry = Op {
-                uuid: player.gameprofile.id,
-                name: player.gameprofile.name.clone(),
-                level: BASIC_CONFIG.op_permission_level,
-                bypasses_player_limit: false,
+            let new_level = if BASIC_CONFIG.op_permission_level > sender.permission_lvl() {
+                sender.permission_lvl()
+            } else {
+                BASIC_CONFIG.op_permission_level
             };
+
+            let op_entry = Op::new(
+                player.gameprofile.id,
+                player.gameprofile.name.clone(),
+                new_level,
+                false,
+            );
             if let Some(op) = config
                 .ops
                 .iter_mut()
                 .find(|o| o.uuid == player.gameprofile.id)
             {
-                op.level = BASIC_CONFIG.op_permission_level;
+                op.level = new_level;
             } else {
                 config.ops.push(op_entry);
             }
             config.save();
 
             player
-                .set_permission_lvl(
-                    BASIC_CONFIG.op_permission_level.into(),
-                    &server.command_dispatcher,
-                )
+                .set_permission_lvl(new_level, &server.command_dispatcher)
                 .await;
 
             let player_name = player.gameprofile.name.clone();
@@ -71,7 +74,7 @@ impl CommandExecutor for OpExecutor {
 
 pub fn init_command_tree<'a>() -> CommandTree<'a> {
     CommandTree::new(NAMES, DESCRIPTION).with_child(
-        require(&|sender| sender.has_permission_lvl(PermissionLvl::Four))
+        require(&|sender| sender.has_permission_lvl(PermissionLvl::Three))
             .with_child(argument(ARG_TARGET, &PlayersArgumentConsumer).execute(&OpExecutor)),
     )
 }

pumpkin/src/command/commands/cmd_say.rs

@@ -2,15 +2,13 @@ use async_trait::async_trait;
 use pumpkin_core::text::TextComponent;
 use pumpkin_protocol::client::play::CSystemChatMessage;
 
-use crate::{
-    command::{
-        args::{arg_message::MsgArgConsumer, Arg, ConsumedArgs},
-        tree::CommandTree,
-        tree_builder::{argument, require},
-        CommandError, CommandExecutor, CommandSender,
-    },
-    entity::player::PermissionLvl,
+use crate::command::{
+    args::{arg_message::MsgArgConsumer, Arg, ConsumedArgs},
+    tree::CommandTree,
+    tree_builder::{argument, require},
+    CommandError, CommandExecutor, CommandSender,
 };
+use pumpkin_core::permission::PermissionLvl;
 use CommandError::InvalidConsumption;
 
 const NAMES: [&str; 1] = ["say"];

pumpkin/src/command/commands/cmd_seed.rs

@@ -2,8 +2,8 @@ use crate::command::tree_builder::require;
 use crate::command::{
     args::ConsumedArgs, tree::CommandTree, CommandError, CommandExecutor, CommandSender,
 };
-use crate::entity::player::PermissionLvl;
 use async_trait::async_trait;
+use pumpkin_core::permission::PermissionLvl;
 use pumpkin_core::text::click::ClickEvent;
 use pumpkin_core::text::hover::HoverEvent;
 use pumpkin_core::text::{color::NamedColor, TextComponent};

pumpkin/src/command/commands/cmd_setblock.rs

@@ -8,7 +8,7 @@ use crate::command::args::{ConsumedArgs, FindArg};
 use crate::command::tree::CommandTree;
 use crate::command::tree_builder::{argument, literal, require};
 use crate::command::{CommandError, CommandExecutor, CommandSender};
-use crate::entity::player::PermissionLvl;
+use pumpkin_core::permission::PermissionLvl;
 
 const NAMES: [&str; 1] = ["setblock"];
 

pumpkin/src/command/commands/cmd_stop.rs

@@ -6,7 +6,7 @@ use crate::command::args::ConsumedArgs;
 use crate::command::tree::CommandTree;
 use crate::command::tree_builder::require;
 use crate::command::{CommandError, CommandExecutor, CommandSender};
-use crate::entity::player::PermissionLvl;
+use pumpkin_core::permission::PermissionLvl;
 
 const NAMES: [&str; 1] = ["stop"];
 

pumpkin/src/command/commands/cmd_teleport.rs

@@ -12,7 +12,7 @@ use crate::command::tree::CommandTree;
 use crate::command::tree_builder::{argument, literal, require};
 use crate::command::CommandError;
 use crate::command::{CommandExecutor, CommandSender};
-use crate::entity::player::PermissionLvl;
+use pumpkin_core::permission::PermissionLvl;
 
 const NAMES: [&str; 2] = ["teleport", "tp"];
 const DESCRIPTION: &str = "Teleports entities, including players."; // todo

pumpkin/src/command/commands/cmd_time.rs

@@ -5,13 +5,11 @@ use pumpkin_core::text::TextComponent;
 use crate::command::args::arg_bounded_num::BoundedNumArgumentConsumer;
 use crate::command::args::FindArgDefaultName;
 use crate::command::tree_builder::{argument_default_name, literal};
-use crate::{
-    command::{
-        tree::CommandTree, tree_builder::require, CommandError, CommandExecutor, CommandSender,
-        ConsumedArgs,
-    },
-    entity::player::PermissionLvl,
+use crate::command::{
+    tree::CommandTree, tree_builder::require, CommandError, CommandExecutor, CommandSender,
+    ConsumedArgs,
 };
+use pumpkin_core::permission::PermissionLvl;
 
 const NAMES: [&str; 1] = ["time"];
 

pumpkin/src/command/commands/cmd_transfer.rs

@@ -13,7 +13,7 @@ use crate::command::tree_builder::{argument, argument_default_name, require};
 use crate::command::{
     args::ConsumedArgs, tree::CommandTree, CommandError, CommandExecutor, CommandSender,
 };
-use crate::entity::player::PermissionLvl;
+use pumpkin_core::permission::PermissionLvl;
 
 const NAMES: [&str; 1] = ["transfer"];
 

pumpkin/src/command/mod.rs

@@ -4,7 +4,7 @@ use std::sync::Arc;
 use crate::command::commands::cmd_seed;
 use crate::command::commands::{cmd_bossbar, cmd_transfer};
 use crate::command::dispatcher::CommandDispatcher;
-use crate::entity::player::{PermissionLvl, Player};
+use crate::entity::player::Player;
 use crate::server::Server;
 use crate::world::World;
 use args::ConsumedArgs;
@@ -17,6 +17,7 @@ use commands::{
 };
 use dispatcher::CommandError;
 use pumpkin_core::math::vector3::Vector3;
+use pumpkin_core::permission::PermissionLvl;
 use pumpkin_core::text::TextComponent;
 
 pub mod args;