Merge pull request #87 from Bryntet/pumpkinerror-trait

Add a general error trait for simplified use of Pumpkin errors

pumpkin-inventory/src/error.rs

@@ -17,17 +17,3 @@ pub enum InventoryError {
     #[error("Player does not have enough permissions")]
     PermissionError,
 }
-
-impl InventoryError {
-    pub fn should_kick(&self) -> bool {
-        match self {
-            InventoryError::InvalidSlot
-            | InventoryError::ClosedContainerInteract(..)
-            | InventoryError::InvalidPacket
-            | InventoryError::PermissionError => true,
-            InventoryError::LockError
-            | InventoryError::OutOfOrderDragging
-            | InventoryError::MultiplePlayersDragging => false,
-        }
-    }
-}

pumpkin/src/entity/player.rs

@@ -15,7 +15,7 @@ use pumpkin_core::{
 use pumpkin_entity::{entity_type::EntityType, EntityId};
 use pumpkin_inventory::player::PlayerInventory;
 use pumpkin_protocol::{
-    bytebuf::{packet_id::Packet, DeserializerError},
+    bytebuf::packet_id::Packet,
     client::play::{
         CGameEvent, CPlayDisconnect, CPlayerAbilities, CPlayerInfoUpdate, CSetHealth,
         CSyncPlayerPosition, CSystemChatMessage, GameEvent, PlayerAction,
@@ -32,13 +32,15 @@ use pumpkin_protocol::{
 use pumpkin_protocol::server::play::{SCloseContainer, SKeepAlive};
 use pumpkin_world::item::ItemStack;
 
+use super::Entity;
+use crate::error::PumpkinError;
 use crate::{
     client::{authentication::GameProfile, Client, PlayerConfig},
     server::Server,
     world::World,
 };
 
-use super::{living::LivingEntity, Entity};
+use super::living::LivingEntity;
 
 /// Represents a Minecraft player entity.
 ///
@@ -278,9 +280,17 @@ impl Player {
             match self.handle_play_packet(server, &mut packet).await {
                 Ok(_) => {}
                 Err(e) => {
-                    let text = format!("Error while reading incoming packet {}", e);
-                    log::error!("{}", text);
-                    self.kick(TextComponent::text(&text))
+                    if e.is_kick() {
+                        if let Some(kick_reason) = e.client_kick_reason() {
+                            self.kick(TextComponent::text(&kick_reason))
+                        } else {
+                            self.kick(TextComponent::text(&format!(
+                                "Error while reading incoming packet {}",
+                                e
+                            )));
+                        }
+                    }
+                    e.log();
                 }
             };
         }
@@ -290,7 +300,7 @@ impl Player {
         &self,
         server: &Arc<Server>,
         packet: &mut RawPacket,
-    ) -> Result<(), DeserializerError> {
+    ) -> Result<(), Box<dyn PumpkinError>> {
         let bytebuf = &mut packet.bytebuf;
         match packet.id.0 {
             SConfirmTeleport::PACKET_ID => {
@@ -358,19 +368,16 @@ impl Player {
                 Ok(())
             }
             SSetCreativeSlot::PACKET_ID => {
-                self.handle_set_creative_slot(SSetCreativeSlot::read(bytebuf)?)
-                    .unwrap();
+                self.handle_set_creative_slot(SSetCreativeSlot::read(bytebuf)?)?;
                 Ok(())
             }
             SPlayPingRequest::PACKET_ID => {
                 self.handle_play_ping_request(SPlayPingRequest::read(bytebuf)?);
                 Ok(())
             }
             SClickContainer::PACKET_ID => {
-                // TODO
-                // self.handle_click_container(server, SClickContainer::read(bytebuf)?)
-                //     .await
-                //     .unwrap();
+                self.handle_click_container(server, SClickContainer::read(bytebuf)?)
+                    .await?;
                 Ok(())
             }
             SCloseContainer::PACKET_ID => {

pumpkin/src/error.rs

@@ -0,0 +1,61 @@
+use log::log;
+use pumpkin_inventory::InventoryError;
+use pumpkin_protocol::bytebuf::DeserializerError;
+use std::fmt::Display;
+
+pub trait PumpkinError: std::error::Error + Display {
+    fn is_kick(&self) -> bool;
+
+    fn log(&self) {
+        log!(self.severity(), "{}", self.to_string());
+    }
+
+    fn severity(&self) -> log::Level;
+
+    fn client_kick_reason(&self) -> Option<String>;
+}
+
+impl<ErrorType: PumpkinError + 'static> From<ErrorType> for Box<dyn PumpkinError> {
+    fn from(error: ErrorType) -> Self {
+        Box::new(error)
+    }
+}
+impl PumpkinError for InventoryError {
+    fn is_kick(&self) -> bool {
+        use InventoryError::*;
+        match self {
+            InvalidSlot | ClosedContainerInteract(..) | InvalidPacket | PermissionError => true,
+            LockError | OutOfOrderDragging | MultiplePlayersDragging => false,
+        }
+    }
+    fn severity(&self) -> log::Level {
+        use InventoryError::*;
+        match self {
+            LockError
+            | InvalidSlot
+            | ClosedContainerInteract(..)
+            | InvalidPacket
+            | PermissionError => log::Level::Error,
+            OutOfOrderDragging => log::Level::Info,
+            MultiplePlayersDragging => log::Level::Warn,
+        }
+    }
+
+    fn client_kick_reason(&self) -> Option<String> {
+        None
+    }
+}
+
+impl PumpkinError for DeserializerError {
+    fn is_kick(&self) -> bool {
+        true
+    }
+
+    fn severity(&self) -> log::Level {
+        log::Level::Error
+    }
+
+    fn client_kick_reason(&self) -> Option<String> {
+        None
+    }
+}

pumpkin/src/main.rs

@@ -33,6 +33,7 @@ use std::time::Duration;
 pub mod client;
 pub mod commands;
 pub mod entity;
+pub mod error;
 pub mod proxy;
 pub mod rcon;
 pub mod server;