From f47b8a1ed146a983259dcfe23ae6f8b87854e57f Mon Sep 17 00:00:00 2001 From: StripedMonkey Date: Mon, 2 Sep 2024 16:03:36 -0400 Subject: [PATCH 1/6] Eliminate Unnecessary Arc Tokens already implement copy and clone, keeping a shared reference to the token doesn't actually do anything, and is larger than a token is to begin with. clone copies --- pumpkin/src/client/mod.rs | 5 ++--- pumpkin/src/client/player_packet.rs | 2 +- pumpkin/src/main.rs | 10 ++++------ pumpkin/src/server.rs | 4 ++-- pumpkin/src/world/mod.rs | 10 +++++----- 5 files changed, 14 insertions(+), 17 deletions(-) diff --git a/pumpkin/src/client/mod.rs b/pumpkin/src/client/mod.rs index 131e12df9..c100dae3f 100644 --- a/pumpkin/src/client/mod.rs +++ b/pumpkin/src/client/mod.rs @@ -1,7 +1,6 @@ use std::{ io::{self, Write}, net::SocketAddr, - sync::Arc, }; use crate::{ @@ -71,7 +70,7 @@ pub struct Client { pub connection_state: ConnectionState, pub encryption: bool, pub closed: bool, - pub token: Arc, + pub token: Token, pub connection: TcpStream, pub address: SocketAddr, enc: PacketEncoder, @@ -82,7 +81,7 @@ pub struct Client { } impl Client { - pub fn new(token: Arc, connection: TcpStream, address: SocketAddr) -> Self { + pub fn new(token: Token, connection: TcpStream, address: SocketAddr) -> Self { Self { protocol_version: 0, gameprofile: None, diff --git a/pumpkin/src/client/player_packet.rs b/pumpkin/src/client/player_packet.rs index 45fb667a5..32f328e79 100644 --- a/pumpkin/src/client/player_packet.rs +++ b/pumpkin/src/client/player_packet.rs @@ -340,7 +340,7 @@ impl Player { let world = world.lock().await; let attacked_player = world.get_by_entityid(self, entity_id.0 as EntityId); if let Some(mut player) = attacked_player { - let token = player.client.token.clone(); + let token = player.client.token; let velo = player.velocity; if config.protect_creative && player.gamemode == GameMode::Creative { return; diff --git a/pumpkin/src/main.rs b/pumpkin/src/main.rs index 8b9d8ef82..a0e76eeab 100644 --- a/pumpkin/src/main.rs +++ b/pumpkin/src/main.rs @@ -81,7 +81,7 @@ fn main() -> io::Result<()> { let rcon = ADVANCED_CONFIG.rcon.clone(); let mut clients: HashMap = HashMap::new(); - let mut players: HashMap, Arc>> = HashMap::new(); + let mut players: HashMap>> = HashMap::new(); let server = Arc::new(tokio::sync::Mutex::new(Server::new())); log::info!("Started Server took {}ms", time.elapsed().as_millis()); @@ -150,8 +150,7 @@ fn main() -> io::Result<()> { token, Interest::READABLE.add(Interest::WRITABLE), )?; - let rc_token = Arc::new(token); - let client = Client::new(Arc::clone(&rc_token), connection, addr); + let client = Client::new(token, connection, addr); clients.insert(token, client); }, @@ -195,10 +194,9 @@ fn main() -> io::Result<()> { if done { poll.registry().deregister(&mut client.connection)?; } else if make_player { - let token = client.token.clone(); + let token = client.token; let mut server = server.lock().await; - let (player, world) = - server.add_player(token.clone(), client).await; + let (player, world) = server.add_player(token, client).await; players.insert(token, player.clone()); let mut world = world.lock().await; world.spawn_player(&BASIC_CONFIG, player).await; diff --git a/pumpkin/src/server.rs b/pumpkin/src/server.rs index 8082b1daf..d765f7cf9 100644 --- a/pumpkin/src/server.rs +++ b/pumpkin/src/server.rs @@ -107,7 +107,7 @@ impl Server { pub async fn add_player( &mut self, - token: Arc, + token: Token, client: Client, ) -> (Arc>, Arc>) { let entity_id = self.new_entity_id(); @@ -129,7 +129,7 @@ impl Server { } /// Sends a Packet to all Players in all worlds - pub fn broadcast_packet_all

(&self, expect: &[&Arc], packet: &P) + pub fn broadcast_packet_all

(&self, expect: &[&Token], packet: &P) where P: ClientPacket, { diff --git a/pumpkin/src/world/mod.rs b/pumpkin/src/world/mod.rs index 0761acf0d..967368d01 100644 --- a/pumpkin/src/world/mod.rs +++ b/pumpkin/src/world/mod.rs @@ -26,7 +26,7 @@ use crate::{client::Client, entity::player::Player}; pub struct World { pub level: Arc>, - pub current_players: HashMap, Arc>>, + pub current_players: HashMap>>, // entities, players... } @@ -39,7 +39,7 @@ impl World { } /// Sends a Packet to all Players, Expect some players. Because we can't lock them twice - pub fn broadcast_packet

(&self, expect: &[&Arc], packet: &P) + pub fn broadcast_packet

(&self, expect: &[&Token], packet: &P) where P: ClientPacket, { @@ -139,7 +139,7 @@ impl World { for (_, playerr) in self .current_players .iter() - .filter(|c| c.0 != &player.client.token) + .filter(|(c, _)| **c != player.client.token) { let playerr = playerr.as_ref().lock().unwrap(); let gameprofile = &playerr.gameprofile; @@ -184,7 +184,7 @@ impl World { ), ); // spawn players for our client - let token = player.client.token.clone(); + let token = player.client.token; for (_, existing_player) in self.current_players.iter().filter(|c| c.0 != &token) { let existing_player = existing_player.as_ref().lock().unwrap(); let entity = &existing_player.entity; @@ -277,7 +277,7 @@ impl World { None } - pub fn add_player(&mut self, token: Arc, player: Arc>) { + pub fn add_player(&mut self, token: Token, player: Arc>) { self.current_players.insert(token, player); } From b55991d8de69492f87a375486648f131af20f113 Mon Sep 17 00:00:00 2001 From: StripedMonkey Date: Mon, 2 Sep 2024 16:54:29 -0400 Subject: [PATCH 2/6] shift `server` into its own module --- pumpkin/src/{server.rs => server/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename pumpkin/src/{server.rs => server/mod.rs} (100%) diff --git a/pumpkin/src/server.rs b/pumpkin/src/server/mod.rs similarity index 100% rename from pumpkin/src/server.rs rename to pumpkin/src/server/mod.rs From dc093f4a21a8c48e5d94f27b2edb730fc329669a Mon Sep 17 00:00:00 2001 From: StripedMonkey Date: Tue, 3 Sep 2024 19:28:56 -0400 Subject: [PATCH 3/6] remove unnecessary references It's already a usize, so the references are meaningless when the pointer is the same size as the type. --- pumpkin/src/client/player_packet.rs | 28 ++++++++++++++-------------- pumpkin/src/entity/player.rs | 6 +++--- pumpkin/src/server/mod.rs | 2 +- pumpkin/src/world/mod.rs | 14 +++++++------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/pumpkin/src/client/player_packet.rs b/pumpkin/src/client/player_packet.rs index 32f328e79..8fa1852c9 100644 --- a/pumpkin/src/client/player_packet.rs +++ b/pumpkin/src/client/player_packet.rs @@ -95,7 +95,7 @@ impl Player { let world = self.world.clone(); let world = world.lock().await; world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CUpdateEntityPos::new( entity_id.into(), (x * 4096.0 - lastx * 4096.0) as i16, @@ -149,7 +149,7 @@ impl Player { let world = world.lock().await; world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CUpdateEntityPosRot::new( entity_id.into(), (x * 4096.0 - lastx * 4096.0) as i16, @@ -161,7 +161,7 @@ impl Player { ), ); world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CHeadRot::new(entity_id.into(), yaw as u8), ); @@ -186,10 +186,10 @@ impl Player { let world = self.world.lock().await; let packet = CUpdateEntityRot::new(entity_id.into(), yaw as u8, pitch as u8, on_ground); // self.client.send_packet(&packet); - world.broadcast_packet(&[&self.client.token], &packet); + world.broadcast_packet(&[self.client.token], &packet); let packet = CHeadRot::new(entity_id.into(), yaw as u8); // self.client.send_packet(&packet); - world.broadcast_packet(&[&self.client.token], &packet); + world.broadcast_packet(&[self.client.token], &packet); } pub fn handle_chat_command(&mut self, server: &mut Server, command: SChatCommand) { @@ -248,7 +248,7 @@ impl Player { let id = self.entity_id(); let world = self.world.lock().await; world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CEntityAnimation::new(id.into(), animation as u8), ) } @@ -272,7 +272,7 @@ impl Player { let world = self.world.lock().await; world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CPlayerChatMessage::new( pumpkin_protocol::uuid::UUID(gameprofile.id), 1.into(), @@ -371,7 +371,7 @@ impl Player { self.client.send_packet(packet); player.client.send_packet(packet); world.broadcast_packet( - &[&self.client.token, &token], + &[self.client.token, token], &CHurtAnimation::new(&entity_id, 10.0), ) } @@ -411,12 +411,12 @@ impl Player { // TODO: currently this is always dirt replace it let world = self.world.lock().await; world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CWorldEvent::new(2001, &location, 11, false), ); // AIR world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CBlockUpdate::new(&location, 0.into()), ); } @@ -439,12 +439,12 @@ impl Player { // TODO: currently this is always dirt replace it let world = self.world.lock().await; world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CWorldEvent::new(2001, &location, 11, false), ); // AIR world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CBlockUpdate::new(&location, 0.into()), ); // TODO: Send this every tick @@ -491,11 +491,11 @@ impl Player { if let Ok(block_state_id) = BlockId::new(minecraft_id, None) { let world = self.world.lock().await; world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CBlockUpdate::new(&location, block_state_id.get_id_mojang_repr().into()), ); world.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CBlockUpdate::new( &WorldPosition(location.0 + face.to_offset()), block_state_id.get_id_mojang_repr().into(), diff --git a/pumpkin/src/entity/player.rs b/pumpkin/src/entity/player.rs index d5688a5d6..1cb2f3b47 100644 --- a/pumpkin/src/entity/player.rs +++ b/pumpkin/src/entity/player.rs @@ -230,7 +230,7 @@ impl Player { self.world .lock() .await - .broadcast_packet(&[&self.client.token], &packet); + .broadcast_packet(&[self.client.token], &packet); } pub async fn set_pose(&mut self, pose: EntityPose) { @@ -244,7 +244,7 @@ impl Player { self.world .lock() .await - .broadcast_packet(&[&self.client.token], &packet) + .broadcast_packet(&[self.client.token], &packet) } pub fn teleport(&mut self, x: f64, y: f64, z: f64, yaw: f32, pitch: f32) { @@ -326,7 +326,7 @@ impl Player { }], )); self.world.lock().await.broadcast_packet( - &[&self.client.token], + &[self.client.token], &CPlayerInfoUpdate::new( 0x04, &[pumpkin_protocol::client::play::Player { diff --git a/pumpkin/src/server/mod.rs b/pumpkin/src/server/mod.rs index d765f7cf9..8da9e3531 100644 --- a/pumpkin/src/server/mod.rs +++ b/pumpkin/src/server/mod.rs @@ -129,7 +129,7 @@ impl Server { } /// Sends a Packet to all Players in all worlds - pub fn broadcast_packet_all

(&self, expect: &[&Token], packet: &P) + pub fn broadcast_packet_all

(&self, expect: &[Token], packet: &P) where P: ClientPacket, { diff --git a/pumpkin/src/world/mod.rs b/pumpkin/src/world/mod.rs index 967368d01..525a58efb 100644 --- a/pumpkin/src/world/mod.rs +++ b/pumpkin/src/world/mod.rs @@ -39,14 +39,14 @@ impl World { } /// Sends a Packet to all Players, Expect some players. Because we can't lock them twice - pub fn broadcast_packet

(&self, expect: &[&Token], packet: &P) + pub fn broadcast_packet

(&self, expect: &[Token], packet: &P) where P: ClientPacket, { for (_, player) in self .current_players .iter() - .filter(|c| !expect.contains(&c.0)) + .filter(|c| !expect.contains(c.0)) { let mut player = player.lock().unwrap(); player.client.send_packet(packet); @@ -118,7 +118,7 @@ impl World { }], )); self.broadcast_packet( - &[&player.client.token], + &[player.client.token], &CPlayerInfoUpdate::new( 0x01 | 0x08, &[pumpkin_protocol::client::play::Player { @@ -165,7 +165,7 @@ impl World { // spawn player for every client self.broadcast_packet( - &[&player.client.token], + &[player.client.token], // TODO: add velo &CSpawnEntity::new( entity_id.into(), @@ -213,7 +213,7 @@ impl World { Metadata::new(17, VarInt(0), config.skin_parts), ); player.client.send_packet(&packet); - self.broadcast_packet(&[&player.client.token], &packet) + self.broadcast_packet(&[player.client.token], &packet) } // Spawn in inital chunks @@ -288,9 +288,9 @@ impl World { let id = player.entity_id(); let uuid = player.gameprofile.id; self.broadcast_packet( - &[&player.client.token], + &[player.client.token], &CRemovePlayerInfo::new(1.into(), &[UUID(uuid)]), ); - self.broadcast_packet(&[&player.client.token], &CRemoveEntities::new(&[id.into()])) + self.broadcast_packet(&[player.client.token], &CRemoveEntities::new(&[id.into()])) } } From 8b73cafaf04d4b9cf1a6e415f4cf42dd01bf9b8a Mon Sep 17 00:00:00 2001 From: StripedMonkey Date: Tue, 3 Sep 2024 19:29:16 -0400 Subject: [PATCH 4/6] expect -> except --- pumpkin/src/server/mod.rs | 4 ++-- pumpkin/src/world/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pumpkin/src/server/mod.rs b/pumpkin/src/server/mod.rs index 8da9e3531..8791677d8 100644 --- a/pumpkin/src/server/mod.rs +++ b/pumpkin/src/server/mod.rs @@ -129,12 +129,12 @@ impl Server { } /// Sends a Packet to all Players in all worlds - pub fn broadcast_packet_all

(&self, expect: &[Token], packet: &P) + pub fn broadcast_packet_all

(&self, except: &[Token], packet: &P) where P: ClientPacket, { for world in &self.worlds { - world.blocking_lock().broadcast_packet(expect, packet) + world.blocking_lock().broadcast_packet(except, packet) } } diff --git a/pumpkin/src/world/mod.rs b/pumpkin/src/world/mod.rs index 525a58efb..fbca46437 100644 --- a/pumpkin/src/world/mod.rs +++ b/pumpkin/src/world/mod.rs @@ -39,14 +39,14 @@ impl World { } /// Sends a Packet to all Players, Expect some players. Because we can't lock them twice - pub fn broadcast_packet

(&self, expect: &[Token], packet: &P) + pub fn broadcast_packet

(&self, except: &[Token], packet: &P) where P: ClientPacket, { for (_, player) in self .current_players .iter() - .filter(|c| !expect.contains(c.0)) + .filter(|c| !except.contains(c.0)) { let mut player = player.lock().unwrap(); player.client.send_packet(packet); From 286253cdc688429d68549bc6cf6c9246d9cb6840 Mon Sep 17 00:00:00 2001 From: StripedMonkey Date: Tue, 3 Sep 2024 19:30:10 -0400 Subject: [PATCH 5/6] eliminate unnecessary (and overloaded) variable --- pumpkin/src/main.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/pumpkin/src/main.rs b/pumpkin/src/main.rs index a0e76eeab..18bc812b0 100644 --- a/pumpkin/src/main.rs +++ b/pumpkin/src/main.rs @@ -156,25 +156,22 @@ fn main() -> io::Result<()> { token => { // Poll Players - let done = if let Some(player) = players.get_mut(&token) { + if let Some(player) = players.get_mut(&token) { let mut player = player.lock().unwrap(); player.client.poll(event).await; if !player.client.closed { let mut server = server.lock().await; player.process_packets(&mut server).await; } - player.client.closed - } else { - false - }; - - if done { - if let Some(player) = players.remove(&token) { - let mut player = player.lock().unwrap(); - player.remove().await; - poll.registry().deregister(&mut player.client.connection)?; + if player.client.closed { + drop(player); + if let Some(player) = players.remove(&token) { + let mut player = player.lock().unwrap(); + player.remove().await; + poll.registry().deregister(&mut player.client.connection)?; + } } - } + }; // Poll current Clients (non players) // Maybe received an event for a TCP connection. From 4dce8aa90da11973058def75e82de3a0748937d5 Mon Sep 17 00:00:00 2001 From: StripedMonkey Date: Tue, 3 Sep 2024 20:47:30 -0400 Subject: [PATCH 6/6] Allow From> for (_,_,_) for easy destructuring. --- pumpkin-core/src/math/vector3.rs | 6 ++++++ pumpkin/src/client/player_packet.rs | 18 ++++++------------ pumpkin/src/entity/player.rs | 12 +++--------- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/pumpkin-core/src/math/vector3.rs b/pumpkin-core/src/math/vector3.rs index 67bc038bc..65da454e6 100644 --- a/pumpkin-core/src/math/vector3.rs +++ b/pumpkin-core/src/math/vector3.rs @@ -98,6 +98,12 @@ impl From<(T, T, T)> for Vector3 { } } +impl From> for (T, T, T) { + fn from(vector: Vector3) -> Self { + (vector.x, vector.y, vector.z) + } +} + pub trait Math: Mul + Neg diff --git a/pumpkin/src/client/player_packet.rs b/pumpkin/src/client/player_packet.rs index 8fa1852c9..3d38f7b71 100644 --- a/pumpkin/src/client/player_packet.rs +++ b/pumpkin/src/client/player_packet.rs @@ -76,9 +76,7 @@ impl Player { return; } let entity = &mut self.entity; - self.lastx = entity.pos.x; - self.lasty = entity.pos.y; - self.lastz = entity.pos.z; + self.last_position = entity.pos; entity.set_pos( Self::clamp_horizontal(position.x), Self::clamp_vertical(position.feet_y), @@ -89,9 +87,8 @@ impl Player { // send new position to all other players let on_ground = self.on_ground; let entity_id = entity.entity_id; - let (x, lastx) = (entity.pos.x, self.lastx); - let (y, lasty) = (entity.pos.y, self.lasty); - let (z, lastz) = (entity.pos.z, self.lastz); + let (x, y, z) = entity.pos.into(); + let (lastx, lasty, lastz) = self.last_position.into(); let world = self.world.clone(); let world = world.lock().await; world.broadcast_packet( @@ -125,9 +122,7 @@ impl Player { } let entity = &mut self.entity; - self.lastx = entity.pos.x; - self.lasty = entity.pos.y; - self.lastz = entity.pos.z; + self.last_position = entity.pos; entity.set_pos( Self::clamp_horizontal(position_rotation.x), Self::clamp_vertical(position_rotation.feet_y), @@ -139,9 +134,8 @@ impl Player { // send new position to all other players let on_ground = self.on_ground; let entity_id = entity.entity_id; - let (x, lastx) = (entity.pos.x, self.lastx); - let (y, lasty) = (entity.pos.y, self.lasty); - let (z, lastz) = (entity.pos.z, self.lastz); + let (x, y, z) = entity.pos.into(); + let (lastx, lasty, lastz) = self.last_position.into(); let yaw = modulus(entity.yaw * 256.0 / 360.0, 256.0); let pitch = modulus(entity.pitch * 256.0 / 360.0, 256.0); // let head_yaw = (entity.head_yaw * 256.0 / 360.0).floor(); diff --git a/pumpkin/src/entity/player.rs b/pumpkin/src/entity/player.rs index 1cb2f3b47..35b9b46b8 100644 --- a/pumpkin/src/entity/player.rs +++ b/pumpkin/src/entity/player.rs @@ -69,9 +69,7 @@ pub struct Player { /// send `send_abilties_update` when changed pub abilities: PlayerAbilities, - pub lastx: f64, - pub lasty: f64, - pub lastz: f64, + pub last_position: Vector3, // Client side value, Should be not trusted pub on_ground: bool, @@ -132,9 +130,7 @@ impl Player { abilities: PlayerAbilities::default(), gamemode, watched_section: Vector3::new(0, 0, 0), - lastx: 0.0, - lasty: 0.0, - lastz: 0.0, + last_position: Vector3::new(0.0, 0.0, 0.0), } } @@ -255,9 +251,7 @@ impl Player { } let entity = &mut self.entity; entity.set_pos(x, y, z); - self.lastx = x; - self.lasty = y; - self.lastz = z; + self.last_position = entity.pos; entity.yaw = yaw; entity.pitch = pitch; self.awaiting_teleport = Some((self.teleport_id_count.into(), Vector3::new(x, y, z)));