More clippy lints

Also made some vars NonZero

.github/workflows/rust.yml

@@ -32,7 +32,7 @@ jobs:
       - uses: actions/checkout@v4
       - run: rustup update ${{ matrix.toolchain }} && rustup default ${{ matrix.toolchain }}
       - uses: Swatinem/rust-cache@v2
-      - run: cargo clippy --all-targets --all-features --no-default-features
+      - run: cargo clippy --all-targets --all-features
   build_and_test:
     name: Build project and test
     runs-on: ${{ matrix.os }}
@@ -77,4 +77,4 @@ jobs:
       - uses: actions/checkout@v4
       - run: rustup update ${{ matrix.toolchain }} && rustup default ${{ matrix.toolchain }}
       - uses: Swatinem/rust-cache@v2
-      - run: cargo clippy --release --all-targets --all-features --no-default-features
+      - run: cargo clippy --release --all-targets --all-features

pumpkin-config/src/lib.rs

@@ -7,6 +7,7 @@ use serde::{de::DeserializeOwned, Deserialize, Serialize};
 use std::{
     fs,
     net::{Ipv4Addr, SocketAddr},
+    num::NonZeroU8,
     path::Path,
     sync::LazyLock,
 };
@@ -71,9 +72,9 @@ pub struct BasicConfiguration {
     /// The maximum number of players allowed on the server. Specifying `0` disables the limit.
     pub max_players: u32,
     /// The maximum view distance for players.
-    pub view_distance: u8,
+    pub view_distance: NonZeroU8,
     /// The maximum simulated view distance.
-    pub simulation_distance: u8,
+    pub simulation_distance: NonZeroU8,
     /// The default game difficulty.
     pub default_difficulty: Difficulty,
     /// Whether the Nether dimension is enabled.
@@ -103,8 +104,8 @@ impl Default for BasicConfiguration {
             server_address: SocketAddr::new(Ipv4Addr::new(0, 0, 0, 0).into(), 25565),
             seed: "".to_string(),
             max_players: 100000,
-            view_distance: 10,
-            simulation_distance: 10,
+            view_distance: NonZeroU8::new(10).unwrap(),
+            simulation_distance: NonZeroU8::new(10).unwrap(),
             default_difficulty: Difficulty::Normal,
             allow_nether: true,
             hardcore: false,
@@ -176,9 +177,14 @@ impl LoadConfiguration for BasicConfiguration {
     }
 
     fn validate(&self) {
-        assert!(self.view_distance >= 2, "View distance must be at least 2");
         assert!(
-            self.view_distance <= 32,
+            self.view_distance
+                .ge(unsafe { &NonZeroU8::new_unchecked(2) }),
+            "View distance must be at least 2"
+        );
+        assert!(
+            self.view_distance
+                .le(unsafe { &NonZeroU8::new_unchecked(32) }),
             "View distance must be less than 32"
         );
         if self.online_mode {

pumpkin-protocol/src/lib.rs

@@ -1,3 +1,5 @@
+use std::num::NonZeroU16;
+
 use bytebuf::{packet_id::Packet, ReadingError};
 use bytes::{Bytes, BytesMut};
 use pumpkin_core::text::{style::Style, TextComponent};
@@ -19,7 +21,7 @@ pub use var_long::*;
 
 /// To current Minecraft protocol
 /// Don't forget to change this when porting
-pub const CURRENT_MC_PROTOCOL: u32 = 769;
+pub const CURRENT_MC_PROTOCOL: NonZeroU16 = unsafe { NonZeroU16::new_unchecked(769) };
 
 pub const MAX_PACKET_SIZE: i32 = 2097152;
 

pumpkin-protocol/src/var_int.rs

@@ -1,3 +1,5 @@
+use std::num::NonZeroUsize;
+
 use bytes::{Buf, BufMut};
 use serde::{
     de::{SeqAccess, Visitor},
@@ -15,7 +17,7 @@ pub struct VarInt(pub VarIntType);
 
 impl VarInt {
     /// The maximum number of bytes a `VarInt` can occupy.
-    pub const MAX_SIZE: usize = 5;
+    pub const MAX_SIZE: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(5) };
 
     /// Returns the exact number of bytes this varint will write when
     /// [`Encode::encode`] is called, assuming no error occurs.
@@ -28,7 +30,7 @@ impl VarInt {
 
     pub fn encode(&self, w: &mut impl BufMut) {
         let mut val = self.0;
-        for _ in 0..Self::MAX_SIZE {
+        for _ in 0..Self::MAX_SIZE.get() {
             let b: u8 = val as u8 & 0b01111111;
             val >>= 7;
             w.put_u8(if val == 0 { b } else { b | 0b10000000 });
@@ -40,7 +42,7 @@ impl VarInt {
 
     pub fn decode(r: &mut impl Buf) -> Result<Self, VarIntDecodeError> {
         let mut val = 0;
-        for i in 0..Self::MAX_SIZE {
+        for i in 0..Self::MAX_SIZE.get() {
             if !r.has_remaining() {
                 return Err(VarIntDecodeError::Incomplete);
             }
@@ -130,7 +132,7 @@ impl<'de> Deserialize<'de> for VarInt {
                 A: SeqAccess<'de>,
             {
                 let mut val = 0;
-                for i in 0..VarInt::MAX_SIZE {
+                for i in 0..VarInt::MAX_SIZE.get() {
                     if let Some(byte) = seq.next_element::<u8>()? {
                         val |= (i32::from(byte) & 0b01111111) << (i * 7);
                         if byte & 0b10000000 == 0 {

pumpkin-protocol/src/var_long.rs

@@ -1,3 +1,5 @@
+use std::num::NonZeroUsize;
+
 use bytes::{Buf, BufMut};
 use serde::{
     de::{self, SeqAccess, Visitor},
@@ -15,7 +17,7 @@ pub struct VarLong(pub VarLongType);
 
 impl VarLong {
     /// The maximum number of bytes a `VarLong`
-    pub const MAX_SIZE: usize = 10;
+    pub const MAX_SIZE: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(10) };
 
     /// Returns the exact number of bytes this varlong will write when
     /// [`Encode::encode`] is called, assuming no error occurs.
@@ -28,7 +30,7 @@ impl VarLong {
 
     pub fn encode(&self, w: &mut impl BufMut) {
         let mut x = self.0;
-        for _ in 0..Self::MAX_SIZE {
+        for _ in 0..Self::MAX_SIZE.get() {
             let byte = (x & 0x7F) as u8;
             x >>= 7;
             if x == 0 {
@@ -41,7 +43,7 @@ impl VarLong {
 
     pub fn decode(r: &mut impl Buf) -> Result<Self, VarLongDecodeError> {
         let mut val = 0;
-        for i in 0..Self::MAX_SIZE {
+        for i in 0..Self::MAX_SIZE.get() {
             if !r.has_remaining() {
                 return Err(VarLongDecodeError::Incomplete);
             }
@@ -131,7 +133,7 @@ impl<'de> Deserialize<'de> for VarLong {
                 A: SeqAccess<'de>,
             {
                 let mut val = 0;
-                for i in 0..VarLong::MAX_SIZE {
+                for i in 0..VarLong::MAX_SIZE.get() {
                     if let Some(byte) = seq.next_element::<u8>()? {
                         val |= (i64::from(byte) & 0b01111111) << (i * 7);
                         if byte & 0b10000000 == 0 {

pumpkin-world/src/cylindrical_chunk_iterator.rs

@@ -1,13 +1,15 @@
+use std::num::NonZeroU8;
+
 use pumpkin_core::math::vector2::Vector2;
 
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub struct Cylindrical {
     pub center: Vector2<i32>,
-    pub view_distance: u8,
+    pub view_distance: NonZeroU8,
 }
 
 impl Cylindrical {
-    pub fn new(center: Vector2<i32>, view_distance: u8) -> Self {
+    pub fn new(center: Vector2<i32>, view_distance: NonZeroU8) -> Self {
         Self {
             center,
             view_distance,
@@ -36,19 +38,19 @@ impl Cylindrical {
     }
 
     fn left(&self) -> i32 {
-        self.center.x - self.view_distance as i32 - 1
+        self.center.x - self.view_distance.get() as i32 - 1
     }
 
     fn bottom(&self) -> i32 {
-        self.center.z - self.view_distance as i32 - 1
+        self.center.z - self.view_distance.get() as i32 - 1
     }
 
     fn right(&self) -> i32 {
-        self.center.x + self.view_distance as i32 + 1
+        self.center.x + self.view_distance.get() as i32 + 1
     }
 
     fn top(&self) -> i32 {
-        self.center.z + self.view_distance as i32 + 1
+        self.center.z + self.view_distance.get() as i32 + 1
     }
 
     fn is_within_distance(&self, x: i32, z: i32) -> bool {
@@ -59,7 +61,7 @@ impl Cylindrical {
         let min_leg = rel_x.min(rel_z) as i64;
 
         let hyp_sqr = max_leg * max_leg + min_leg * min_leg;
-        hyp_sqr < (self.view_distance as i64 * self.view_distance as i64)
+        hyp_sqr < (self.view_distance.get() as i64 * self.view_distance.get() as i64)
     }
 
     /// Returns an iterator of all chunks within this cylinder
@@ -82,12 +84,14 @@ impl Cylindrical {
 #[cfg(test)]
 mod test {
 
+    use std::num::NonZeroU8;
+
     use super::Cylindrical;
     use pumpkin_core::math::vector2::Vector2;
 
     #[test]
     fn test_bounds() {
-        let cylinder = Cylindrical::new(Vector2::new(0, 0), 10);
+        let cylinder = Cylindrical::new(Vector2::new(0, 0), unsafe { NonZeroU8::new_unchecked(1) });
         for chunk in cylinder.all_chunks_within() {
             assert!(chunk.x >= cylinder.left() && chunk.x <= cylinder.right());
             assert!(chunk.z >= cylinder.bottom() && chunk.z <= cylinder.top());

pumpkin/src/block/block_manager.rs

@@ -21,8 +21,7 @@ pub struct BlockManager {
 
 impl BlockManager {
     pub fn register<T: PumpkinBlock + BlockMetadata + 'static>(&mut self, block: T) {
-        self.blocks
-            .insert(block.name().to_string(), Arc::new(block));
+        self.blocks.insert(block.name(), Arc::new(block));
     }
 
     pub async fn on_use(

pumpkin/src/client/client_packet.rs

@@ -29,7 +29,10 @@ use pumpkin_protocol::{
     },
     ConnectionState, KnownPack, VarInt, CURRENT_MC_PROTOCOL,
 };
-use std::sync::LazyLock;
+use std::{
+    num::{NonZeroI32, NonZeroU8},
+    sync::LazyLock,
+};
 use uuid::Uuid;
 
 static LINKS: LazyLock<Vec<Link>> = LazyLock::new(|| {
@@ -106,7 +109,7 @@ impl Client {
         self.connection_state.store(handshake.next_state);
         if self.connection_state.load() != ConnectionState::Status {
             let protocol = version;
-            match protocol.cmp(&(CURRENT_MC_PROTOCOL as i32)) {
+            match protocol.cmp(&NonZeroI32::from(CURRENT_MC_PROTOCOL).get()) {
                 std::cmp::Ordering::Less => {
                     self.kick(&format!("Client outdated ({protocol}), Server uses Minecraft {CURRENT_MC_VERSION}, Protocol {CURRENT_MC_PROTOCOL}")).await;
                 }
@@ -132,10 +135,7 @@ impl Client {
     }
 
     fn is_valid_player_name(name: &str) -> bool {
-        name.len() <= 16
-            && name
-                .chars()
-                .all(|c| c > 32_u8 as char && c < 127_u8 as char)
+        name.len() <= 16 && name.chars().all(|c| c > 32u8 as char && c < 127u8 as char)
     }
 
     pub async fn handle_login_start(&self, server: &Server, login_start: SLoginStart) {
@@ -400,7 +400,9 @@ impl Client {
         ) {
             *self.config.lock().await = Some(PlayerConfig {
                 locale: client_information.locale,
-                view_distance: client_information.view_distance as u8,
+                view_distance: unsafe {
+                    NonZeroU8::new_unchecked(client_information.view_distance as u8)
+                },
                 chat_mode,
                 chat_colors: client_information.chat_colors,
                 skin_parts: client_information.skin_parts,

pumpkin/src/client/mod.rs

@@ -1,6 +1,7 @@
 use std::{
     collections::VecDeque,
     net::SocketAddr,
+    num::NonZeroU8,
     sync::{
         atomic::{AtomicBool, AtomicI32},
         Arc,
@@ -53,7 +54,7 @@ pub struct PlayerConfig {
     /// The player's preferred language.
     pub locale: String, // 16
     /// The maximum distance at which chunks are rendered.
-    pub view_distance: u8,
+    pub view_distance: NonZeroU8,
     /// The player's chat mode settings
     pub chat_mode: ChatMode,
     /// Whether chat colors are enabled.
@@ -72,7 +73,7 @@ impl Default for PlayerConfig {
     fn default() -> Self {
         Self {
             locale: "en_us".to_string(),
-            view_distance: 2,
+            view_distance: unsafe { NonZeroU8::new_unchecked(10) },
             chat_mode: ChatMode::Enabled,
             chat_colors: true,
             skin_parts: 0,

pumpkin/src/client/player_packet.rs

@@ -1,3 +1,4 @@
+use std::num::NonZeroU8;
 use std::sync::Arc;
 
 use super::PlayerConfig;
@@ -436,7 +437,7 @@ impl Player {
     }
 
     pub async fn handle_client_information(
-        self: &Arc<Player>,
+        self: &Arc<Self>,
         client_information: SClientInformationPlay,
     ) {
         if let (Some(main_hand), Some(chat_mode)) = (
@@ -458,25 +459,27 @@ impl Player {
 
                 let old_view_distance = config.view_distance;
 
-                let update_watched = if old_view_distance == client_information.view_distance as u8
-                {
-                    false
-                } else {
-                    log::debug!(
-                        "Player {} ({}) updated render distance: {} -> {}.",
-                        self.gameprofile.name,
-                        self.client.id,
-                        old_view_distance,
-                        client_information.view_distance
-                    );
-
-                    true
-                };
+                let update_watched =
+                    if old_view_distance.get() == client_information.view_distance as u8 {
+                        false
+                    } else {
+                        log::debug!(
+                            "Player {} ({}) updated render distance: {} -> {}.",
+                            self.gameprofile.name,
+                            self.client.id,
+                            old_view_distance,
+                            client_information.view_distance
+                        );
+
+                        true
+                    };
 
                 *config = PlayerConfig {
                     locale: client_information.locale,
                     // A Negative view distance would be impossible and make no sense right ?, Mojang: Lets make is signed :D
-                    view_distance: client_information.view_distance as u8,
+                    view_distance: unsafe {
+                        NonZeroU8::new_unchecked(client_information.view_distance as u8)
+                    },
                     chat_mode,
                     chat_colors: client_information.chat_colors,
                     skin_parts: client_information.skin_parts,

pumpkin/src/command/args/arg_block.rs

@@ -65,12 +65,14 @@ impl<'a> FindArg<'a> for BlockArgumentConsumer {
 
     fn find_arg(args: &'a super::ConsumedArgs, name: &'a str) -> Result<Self::Data, CommandError> {
         match args.get(name) {
-            Some(Arg::Block(name)) => match block_registry::get_block(name) {
-                Some(block) => Ok(block),
-                None => Err(CommandError::GeneralCommandIssue(format!(
-                    "Block {name} does not exist."
-                ))),
-            },
+            Some(Arg::Block(name)) => block_registry::get_block(name).map_or_else(
+                || {
+                    Err(CommandError::GeneralCommandIssue(format!(
+                        "Block {name} does not exist."
+                    )))
+                },
+                Result::Ok,
+            ),
             _ => Err(CommandError::InvalidConsumption(Some(name.to_string()))),
         }
     }