Fix build packet when we cannot write the channel_id

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),
         }
     }
 

lightyear/src/packet/packet.rs

@@ -21,20 +21,12 @@ 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 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)
-/// 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;
 
 /// 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)]

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;
@@ -7,18 +8,14 @@ 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;
 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<u8>;
 
 /// We use `Bytes` on the receive side because we want to be able to refer to sub-slices of the original
@@ -59,7 +56,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 +239,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 +494,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::<Channel1>();
+        let channel_id1 = channel_registry.get_net_from_kind(&channel_kind1).unwrap();
+        let channel_kind2 = ChannelKind::of::<Channel2>();
+        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> {
@@ -573,7 +612,7 @@ mod tests {
         let channel_kind3 = ChannelKind::of::<Channel3>();
         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

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![],
         }
     }