From fb2c6c8595ab38bf0334195e1f16923b87e19cb4 Mon Sep 17 00:00:00 2001 From: cBournhonesque Date: Wed, 5 Jun 2024 12:51:03 -0400 Subject: [PATCH 1/2] a --- lightyear/src/packet/packet.rs | 8 ++--- lightyear/src/packet/packet_builder.rs | 47 ++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/lightyear/src/packet/packet.rs b/lightyear/src/packet/packet.rs index aee749a67..5f4a0c231 100644 --- a/lightyear/src/packet/packet.rs +++ b/lightyear/src/packet/packet.rs @@ -21,14 +21,10 @@ cfg_if::cfg_if!( // Internal id that we assign to each packet sent over the network wrapping_id!(PacketId); -/// Maximum number of bytes to write the header -/// PacketType: 2 bits -/// Rest: 10 bytes +/// Number of btyes to write the header const HEADER_BYTES: usize = 11; /// The maximum of bytes that the payload of the packet can contain (excluding the header) -/// remove 1 byte for byte alignment at the end -// TODO: we removed 10 bytes at the end to take some margin, but we should understand why 1 does not work! -pub(crate) const MTU_PAYLOAD_BYTES: usize = MAX_PACKET_SIZE - HEADER_BYTES - 10; +pub(crate) const MTU_PAYLOAD_BYTES: usize = MAX_PACKET_SIZE - HEADER_BYTES; /// The maximum number of bytes for a message before it is fragmented /// The final size of the fragmented packet (channel_net_id: 2, fragment_id: 1, tick: 2, message_id: 2, num_fragments: 1, number of bytes in fragment: 4) diff --git a/lightyear/src/packet/packet_builder.rs b/lightyear/src/packet/packet_builder.rs index 768cec27a..40e52fffe 100644 --- a/lightyear/src/packet/packet_builder.rs +++ b/lightyear/src/packet/packet_builder.rs @@ -1,4 +1,5 @@ //! Module to take a buffer of messages to send and build packets +use crate::connection::netcode::MAX_PACKET_SIZE; use byteorder::WriteBytesExt; use bytes::Bytes; use std::collections::VecDeque; @@ -59,7 +60,7 @@ impl PacketBuilder { // TODO: get the vec from a pool of preallocated buffers fn get_new_buffer(&self) -> Payload { - Vec::with_capacity(MTU_PAYLOAD_BYTES) + Vec::with_capacity(MAX_PACKET_SIZE) } /// Start building new packet, we start with an empty packet @@ -242,7 +243,11 @@ impl PacketBuilder { let mut packet = self.current_packet.take().unwrap(); // we need to call this to preassign the channel_id if !packet.can_fit_channel(*channel_id) { - unreachable!(); + // can't add any more messages (since we sorted messages from smallest to largest) + // finish packet and go back to trying to write fragment messages + self.current_packet = Some(packet); + packets.push(self.finish_packet()); + continue 'out; } // number of messages for this channel that we will write // (we wait until we know the full number, because we want to write that) @@ -493,6 +498,44 @@ mod tests { Ok(()) } + /// We cannot write the channel id of the next channel in the packet, so we need to finish the current + /// packet and start a new one. + /// We have 1200 -11 (header) -1 (channel_id) - 1(num_message) = 1184 bytes per message + /// + /// Test both with different channels and same channels + #[test] + fn test_pack_cannot_write_channel_id() -> Result<(), PacketError> { + let channel_registry = get_channel_registry(); + let mut manager = PacketBuilder::new(1.5); + let channel_kind1 = ChannelKind::of::(); + let channel_id1 = channel_registry.get_net_from_kind(&channel_kind1).unwrap(); + let channel_kind2 = ChannelKind::of::(); + let channel_id2 = channel_registry.get_net_from_kind(&channel_kind2).unwrap(); + + let small_bytes = Bytes::from(vec![7u8; 1184]); + let small_message = SingleData::new(None, small_bytes.clone()); + + { + let single_data = vec![( + *channel_id1, + VecDeque::from(vec![small_message.clone(), small_message.clone()]), + )]; + let fragment_data = vec![]; + let packets = manager.build_packets(Tick(0), single_data, fragment_data)?; + assert_eq!(packets.len(), 2); + } + { + let single_data = vec![ + (*channel_id1, VecDeque::from(vec![small_message.clone()])), + (*channel_id2, VecDeque::from(vec![small_message.clone()])), + ]; + let fragment_data = vec![]; + let packets = manager.build_packets(Tick(0), single_data, fragment_data)?; + assert_eq!(packets.len(), 2); + } + Ok(()) + } + /// A bunch of small messages that all fit in the same packet #[test] fn test_pack_many_small_messages() -> Result<(), PacketError> { From 212b8d64b3ac4736ae0b9b888112b86c1788f4ed Mon Sep 17 00:00:00 2001 From: cBournhonesque Date: Wed, 5 Jun 2024 12:57:31 -0400 Subject: [PATCH 2/2] remove unused variables --- lightyear/src/client/connection.rs | 5 +++-- lightyear/src/packet/packet.rs | 10 +++------- lightyear/src/packet/packet_builder.rs | 8 ++------ lightyear/src/server/connection.rs | 7 ++++--- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/lightyear/src/client/connection.rs b/lightyear/src/client/connection.rs index 3f138cc60..07da3169c 100644 --- a/lightyear/src/client/connection.rs +++ b/lightyear/src/client/connection.rs @@ -17,8 +17,9 @@ use crate::client::error::ClientError; use crate::client::message::ClientMessage; use crate::client::replication::send::ReplicateCache; use crate::client::sync::SyncConfig; +use crate::connection::netcode::MAX_PACKET_SIZE; use crate::packet::message_manager::MessageManager; -use crate::packet::packet_builder::{Payload, RecvPayload, PACKET_BUFFER_CAPACITY}; +use crate::packet::packet_builder::{Payload, RecvPayload}; use crate::prelude::{Channel, ChannelKind, ClientId, Message}; use crate::protocol::channel::ChannelRegistry; use crate::protocol::component::ComponentRegistry; @@ -139,7 +140,7 @@ impl ConnectionManager { #[cfg(feature = "leafwing")] received_leafwing_input_messages: HashMap::default(), received_messages: HashMap::default(), - writer: Writer::with_capacity(PACKET_BUFFER_CAPACITY), + writer: Writer::with_capacity(MAX_PACKET_SIZE), } } diff --git a/lightyear/src/packet/packet.rs b/lightyear/src/packet/packet.rs index 5f4a0c231..0c1fbd824 100644 --- a/lightyear/src/packet/packet.rs +++ b/lightyear/src/packet/packet.rs @@ -21,16 +21,12 @@ cfg_if::cfg_if!( // Internal id that we assign to each packet sent over the network wrapping_id!(PacketId); -/// Number of btyes to write the header +/// Number of bytes to write the header const HEADER_BYTES: usize = 11; -/// The maximum of bytes that the payload of the packet can contain (excluding the header) -pub(crate) const MTU_PAYLOAD_BYTES: usize = MAX_PACKET_SIZE - HEADER_BYTES; /// The maximum number of bytes for a message before it is fragmented -/// The final size of the fragmented packet (channel_net_id: 2, fragment_id: 1, tick: 2, message_id: 2, num_fragments: 1, number of bytes in fragment: 4) -/// must be lower than MTU_PAYLOAD_BYTES -/// (might even be 13 in some situations?) -pub(crate) const FRAGMENT_SIZE: usize = MTU_PAYLOAD_BYTES - 12; +/// MAX_PACKET_SIZE - HEADER_BYTES - 1 (channel_net_id) - 4 (message_id/fragment_id/num_fragments) - 2 (num bytes in fragment) +pub(crate) const FRAGMENT_SIZE: usize = MAX_PACKET_SIZE - HEADER_BYTES - 7; /// Data structure that will help us write the packet #[derive(Debug)] diff --git a/lightyear/src/packet/packet_builder.rs b/lightyear/src/packet/packet_builder.rs index 40e52fffe..b5a20a392 100644 --- a/lightyear/src/packet/packet_builder.rs +++ b/lightyear/src/packet/packet_builder.rs @@ -8,7 +8,7 @@ use tracing::{instrument, Level}; use crate::packet::header::PacketHeaderManager; use crate::packet::message::{FragmentData, MessageAck, SingleData}; -use crate::packet::packet::{Packet, FRAGMENT_SIZE, MTU_PAYLOAD_BYTES}; +use crate::packet::packet::{Packet, FRAGMENT_SIZE}; use crate::packet::packet_type::PacketType; use crate::prelude::Tick; use crate::protocol::channel::ChannelId; @@ -16,10 +16,6 @@ use crate::protocol::registry::NetId; use crate::serialize::varint::varint_len; use crate::serialize::{SerializationError, ToBytes}; -// enough to hold a biggest fragment + writing channel/message_id/etc. -// pub(crate) const PACKET_BUFFER_CAPACITY: usize = MTU_PAYLOAD_BYTES * (u8::BITS as usize) + 50; -pub(crate) const PACKET_BUFFER_CAPACITY: usize = MTU_PAYLOAD_BYTES * (u8::BITS as usize); - pub type Payload = Vec; /// We use `Bytes` on the receive side because we want to be able to refer to sub-slices of the original @@ -616,7 +612,7 @@ mod tests { let channel_kind3 = ChannelKind::of::(); let channel_id3 = channel_registry.get_net_from_kind(&channel_kind3).unwrap(); - let num_big_bytes = (1.5 * MTU_PAYLOAD_BYTES as f32) as usize; + let num_big_bytes = (1.5 * FRAGMENT_SIZE as f32) as usize; let big_bytes = Bytes::from(vec![1u8; num_big_bytes]); let fragmenter = FragmentSender::new(); let fragments = fragmenter diff --git a/lightyear/src/server/connection.rs b/lightyear/src/server/connection.rs index 1ac107e72..30bb43f27 100644 --- a/lightyear/src/server/connection.rs +++ b/lightyear/src/server/connection.rs @@ -18,8 +18,9 @@ use crate::channel::receivers::ChannelReceive; use crate::channel::senders::ChannelSend; use crate::client::message::ClientMessage; use crate::connection::id::ClientId; +use crate::connection::netcode::MAX_PACKET_SIZE; use crate::packet::message_manager::MessageManager; -use crate::packet::packet_builder::{Payload, RecvPayload, PACKET_BUFFER_CAPACITY}; +use crate::packet::packet_builder::{Payload, RecvPayload}; use crate::prelude::server::{DisconnectEvent, RoomId, RoomManager}; use crate::prelude::{ Channel, ChannelKind, Message, PreSpawnedPlayerObject, ReplicationGroup, ShouldBePredicted, @@ -96,7 +97,7 @@ impl ConnectionManager { delta_manager: DeltaManager::default(), replicate_component_cache: EntityHashMap::default(), new_clients: vec![], - writer: Writer::with_capacity(PACKET_BUFFER_CAPACITY), + writer: Writer::with_capacity(MAX_PACKET_SIZE), replication_config, packet_config, ping_config, @@ -441,7 +442,7 @@ impl Connection { received_input_messages: HashMap::default(), #[cfg(feature = "leafwing")] received_leafwing_input_messages: HashMap::default(), - writer: Writer::with_capacity(PACKET_BUFFER_CAPACITY), + writer: Writer::with_capacity(MAX_PACKET_SIZE), messages_to_rebroadcast: vec![], } }