Merge pull request #82 from caspark/no-bytemuck-bounds-on-input

RFC: feat: Input type must be Default+Serde but not POD

Cargo.toml

@@ -21,7 +21,6 @@ rand = "0.8"
 bitfield-rle = "0.2.1"
 parking_lot = "0.12"
 instant = "0.1"
-bytemuck = {version = "1.9", features = ["derive"]}
 getrandom = {version = "0.2", optional = true}
 
 [target.'cfg(target_arch = "wasm32")'.dependencies]

examples/ex_game/ex_game.rs

@@ -1,6 +1,5 @@
 use std::net::SocketAddr;
 
-use bytemuck::{Pod, Zeroable};
 use ggrs::{Config, Frame, GameStateCell, GgrsRequest, InputStatus, PlayerHandle, NULL_FRAME};
 use macroquad::prelude::*;
 use serde::{Deserialize, Serialize};
@@ -24,7 +23,7 @@ const MAX_SPEED: f32 = 7.0;
 const FRICTION: f32 = 0.98;
 
 #[repr(C)]
-#[derive(Copy, Clone, PartialEq, Pod, Zeroable)]
+#[derive(Copy, Clone, PartialEq, Default, Serialize, Deserialize)]
 pub struct Input {
     pub inp: u8,
 }

src/frame_info.rs

@@ -27,36 +27,23 @@ impl<S> Default for GameState<S> {
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub(crate) struct PlayerInput<I>
 where
-    I: Copy
-        + Clone
-        + PartialEq
-        + bytemuck::NoUninit
-        + bytemuck::CheckedBitPattern
-        + bytemuck::Zeroable,
+    I: Copy + Clone + PartialEq,
 {
     /// The frame to which this info belongs to. -1/[`NULL_FRAME`] represents an invalid frame
     pub frame: Frame,
     /// The input struct given by the user
     pub input: I,
 }
 
-impl<
-        I: Copy
-            + Clone
-            + PartialEq
-            + bytemuck::NoUninit
-            + bytemuck::Zeroable
-            + bytemuck::CheckedBitPattern,
-    > PlayerInput<I>
-{
+impl<I: Copy + Clone + PartialEq + Default> PlayerInput<I> {
     pub(crate) fn new(frame: Frame, input: I) -> Self {
         Self { frame, input }
     }
 
     pub(crate) fn blank_input(frame: Frame) -> Self {
         Self {
             frame,
-            input: I::zeroed(),
+            input: I::default(),
         }
     }
 
@@ -72,10 +59,9 @@ impl<
 #[cfg(test)]
 mod game_input_tests {
     use super::*;
-    use bytemuck::{Pod, Zeroable};
 
     #[repr(C)]
-    #[derive(Copy, Clone, PartialEq, Pod, Zeroable)]
+    #[derive(Copy, Clone, PartialEq, Default)]
     struct TestInput {
         inp: u8,
     }

src/input_queue.rs

@@ -11,7 +11,7 @@ pub(crate) struct InputQueue<T>
 where
     T: Config,
 {
-    /// The head of the queue. The newest `PlayerInput` is saved here      
+    /// The head of the queue. The newest `PlayerInput` is saved here
     head: usize,
     /// The tail of the queue. The oldest `PlayerInput` still valid is saved here.
     tail: usize,
@@ -250,12 +250,12 @@ mod input_queue_tests {
 
     use std::net::SocketAddr;
 
-    use bytemuck::{Pod, Zeroable};
+    use serde::{Deserialize, Serialize};
 
     use super::*;
 
     #[repr(C)]
-    #[derive(Copy, Clone, PartialEq, Pod, Zeroable)]
+    #[derive(Copy, Clone, PartialEq, Default, Serialize, Deserialize)]
     struct TestInput {
         inp: u8,
     }

src/lib.rs

@@ -13,6 +13,7 @@ pub use error::GgrsError;
 pub use network::messages::Message;
 pub use network::network_stats::NetworkStats;
 pub use network::udp_socket::UdpNonBlockingSocket;
+use serde::{de::DeserializeOwned, Serialize};
 pub use sessions::builder::SessionBuilder;
 pub use sessions::p2p_session::P2PSession;
 pub use sessions::p2p_spectator_session::SpectatorSession;
@@ -205,12 +206,9 @@ pub trait Config: 'static + Send + Sync {
     /// The input type for a session. This is the only game-related data
     /// transmitted over the network.
     ///
-    /// Reminder: Types implementing [Pod] may not have the same byte representation
-    /// on platforms with different endianness. GGRS assumes that all players are
-    /// running with the same endianness when encoding and decoding inputs.
-    ///
-    /// [Pod]: bytemuck::Pod
-    type Input: Copy + Clone + PartialEq + bytemuck::Pod + bytemuck::Zeroable + Send + Sync;
+    /// The implementation of [Default] is used for representing "no input" for
+    /// a player, including when a player is disconnected.
+    type Input: Copy + Clone + PartialEq + Default + Serialize + DeserializeOwned + Send + Sync;
 
     /// The save state type for the session.
     type State: Clone + Send + Sync;
@@ -242,17 +240,9 @@ pub trait Config: 'static {
     /// The input type for a session. This is the only game-related data
     /// transmitted over the network.
     ///
-    /// Reminder: Types implementing [Pod] may not have the same byte representation
-    /// on platforms with different endianness. GGRS assumes that all players are
-    /// running with the same endianness when encoding and decoding inputs.
-    ///
-    /// [Pod]: bytemuck::Pod
-    type Input: Copy
-        + Clone
-        + PartialEq
-        + bytemuck::NoUninit
-        + bytemuck::CheckedBitPattern
-        + bytemuck::Zeroable;
+    /// The implementation of [Default] is used for representing "no input" for
+    /// a player, including when a player is disconnected.
+    type Input: Copy + Clone + PartialEq + Default + Serialize + DeserializeOwned;
 
     /// The save state type for the session.
     type State;

src/network/protocol.rs

@@ -73,8 +73,9 @@ impl InputBytes {
                 if input.frame != NULL_FRAME {
                     frame = input.frame;
                 }
-                let byte_vec = bytemuck::bytes_of(&input.input);
-                bytes.extend_from_slice(byte_vec);
+
+                bincode::serialize_into(&mut bytes, &input.input)
+                    .expect("input serialization failed");
             }
         }
         Self { frame, bytes }
@@ -87,8 +88,9 @@ impl InputBytes {
         for p in 0..num_players {
             let start = p * size;
             let end = start + size;
-            let input = *bytemuck::checked::try_from_bytes::<T::Input>(&self.bytes[start..end])
-                .expect("Expected received data to be valid.");
+            let player_byte_slice = &self.bytes[start..end];
+            let input: T::Input =
+                bincode::deserialize(player_byte_slice).expect("input deserialization failed");
             player_inputs.push(PlayerInput::new(self.frame, input));
         }
         player_inputs

src/sync_layer.rs

@@ -1,4 +1,3 @@
-use bytemuck::Zeroable;
 use parking_lot::{MappedMutexGuard, Mutex};
 use std::ops::Deref;
 use std::sync::Arc;
@@ -282,7 +281,7 @@ impl<T: Config> SyncLayer<T> {
         let mut inputs = Vec::new();
         for (i, con_stat) in connect_status.iter().enumerate() {
             if con_stat.disconnected && con_stat.last_frame < self.current_frame {
-                inputs.push((T::Input::zeroed(), InputStatus::Disconnected));
+                inputs.push((T::Input::default(), InputStatus::Disconnected));
             } else {
                 inputs.push(self.input_queues[i].input(self.current_frame));
             }
@@ -380,11 +379,11 @@ impl<T: Config> SyncLayer<T> {
 mod sync_layer_tests {
 
     use super::*;
-    use bytemuck::{Pod, Zeroable};
+    use serde::{Deserialize, Serialize};
     use std::net::SocketAddr;
 
     #[repr(C)]
-    #[derive(Copy, Clone, PartialEq, Pod, Zeroable)]
+    #[derive(Copy, Clone, PartialEq, Default, Serialize, Deserialize)]
     struct TestInput {
         inp: u8,
     }

tests/stubs.rs

@@ -1,4 +1,5 @@
 use rand::{prelude::ThreadRng, thread_rng, Rng};
+use serde::{Deserialize, Serialize};
 use std::collections::hash_map::DefaultHasher;
 use std::hash::{Hash, Hasher};
 use std::net::SocketAddr;
@@ -14,10 +15,9 @@ fn calculate_hash<T: Hash>(t: &T) -> u64 {
 pub struct GameStub {
     pub gs: StateStub,
 }
-use bytemuck::{Pod, Zeroable};
 
 #[repr(C)]
-#[derive(Copy, Clone, PartialEq, Pod, Zeroable)]
+#[derive(Copy, Clone, PartialEq, Default, Serialize, Deserialize)]
 pub struct StubInput {
     pub inp: u32,
 }

tests/stubs_enum.rs

@@ -3,6 +3,7 @@ use std::hash::{Hash, Hasher};
 use std::net::SocketAddr;
 
 use ggrs::{Config, Frame, GameStateCell, GgrsRequest, InputStatus};
+use serde::{Deserialize, Serialize};
 
 fn calculate_hash<T: Hash>(t: &T) -> u64 {
     let mut s = DefaultHasher::new();
@@ -13,22 +14,16 @@ fn calculate_hash<T: Hash>(t: &T) -> u64 {
 pub struct GameStubEnum {
     pub gs: StateStubEnum,
 }
-use bytemuck::{CheckedBitPattern, NoUninit, Zeroable};
 
 #[allow(dead_code)]
 #[repr(u8)]
-#[derive(Copy, Clone, PartialEq, CheckedBitPattern, NoUninit)]
+#[derive(Copy, Clone, PartialEq, Default, Serialize, Deserialize)]
 pub enum EnumInput {
+    #[default]
     Val1,
     Val2,
 }
 
-unsafe impl Zeroable for EnumInput {
-    fn zeroed() -> Self {
-        unsafe { core::mem::zeroed() }
-    }
-}
-
 pub struct StubEnumConfig;
 
 impl Config for StubEnumConfig {