From afc96be31955f2ca28ebe3d6cddc95525d7d4009 Mon Sep 17 00:00:00 2001 From: Hennadii Chernyshchyk Date: Thu, 4 Apr 2024 22:44:40 +0300 Subject: [PATCH] Split writing logic from `deserialize` Into separate `write` function to customize it independently. --- CHANGELOG.md | 5 ++- src/client.rs | 14 +++++-- src/core/replication_fns.rs | 73 +++++++++++++++++++++++++++-------- src/core/replication_rules.rs | 66 ++++++++++++++++++++----------- 4 files changed, 113 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bdc5e17..f1d1965b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,17 +11,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `AppReplicationExt::replicate_group` and `GroupRegistration` trait to register and customize component groups. - `ServerSet::StoreHierarchy` for systems that store hierarchy changes in `ParentSync`. -- `ReplicationRule` that describes how a component or a group of components will be serialized, deserialized and removed. +- `ReplicationRule` that describes how a component or a group of components will be serialized, deserialized, written and removed. ### Changed -- `AppReplicationExt::replicate_with` now accepts newly added `ReplicationFns` and the function is now `unsafe` (it was never "safe" before, caller must ensure that used `C` can be passed to the serialization function). +- `AppReplicationExt::replicate_with` now accepts newly added `ReplicationFns` and the function is now `unsafe` (it was never "safe" before, caller must ensure that functions operate on the same component). - Move `Replication` to `core` module. - Move all functions-related logic from `ReplicationRules` into a new `ReplicationFns`. - Rename `serialize_component` into `serialize` and move into `replication_fns` module. - Rename `deserialize_component` into `deserialize` and move into `replication_fns` module. - Rename `remove_component` into `remove` and move into `replication_fns` module. - Move `despawn_recursive` into `replication_fns` module. +- Split writing logic from `deserialize` into separate `write` function to customize it independently. ### Removed diff --git a/src/client.rs b/src/client.rs index 0ae0a08c..0c3720f1 100644 --- a/src/client.rs +++ b/src/client.rs @@ -350,9 +350,11 @@ fn apply_init_components( let fns_id = DefaultOptions::new().deserialize_from(&mut *cursor)?; let fns = replication_fns.component_fns(fns_id); match components_kind { - ComponentsKind::Insert => { - (fns.deserialize)(&mut entity, entity_map, cursor, replicon_tick)? - } + ComponentsKind::Insert => unsafe { + // SAFETY: User ensured that the registered deserialize function can + // safely call its writing function. + (fns.deserialize)(&mut entity, cursor, entity_map, replicon_tick, fns.write)? + }, ComponentsKind::Removal => (fns.remove)(&mut entity, replicon_tick), } components_len += 1; @@ -434,7 +436,11 @@ fn apply_update_components( while cursor.position() < end_pos { let fns_id = DefaultOptions::new().deserialize_from(&mut *cursor)?; let fns = replication_fns.component_fns(fns_id); - (fns.deserialize)(&mut entity, entity_map, cursor, message_tick)?; + // SAFETY: User ensured that the registered deserialize function can + // safely call its writing function. + unsafe { + (fns.deserialize)(&mut entity, cursor, entity_map, message_tick, fns.write)?; + } components_count += 1; } if let Some(stats) = &mut stats { diff --git a/src/core/replication_fns.rs b/src/core/replication_fns.rs index a62342a8..48072ac3 100644 --- a/src/core/replication_fns.rs +++ b/src/core/replication_fns.rs @@ -1,6 +1,10 @@ use std::io::Cursor; -use bevy::{ecs::entity::MapEntities, prelude::*, ptr::Ptr}; +use bevy::{ + ecs::entity::MapEntities, + prelude::*, + ptr::{OwningPtr, Ptr}, +}; use bincode::{DefaultOptions, Options}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; @@ -58,13 +62,17 @@ impl Default for ReplicationFns { pub type SerializeFn = unsafe fn(Ptr, &mut Cursor>) -> bincode::Result<()>; /// Signature of component deserialization functions. -pub type DeserializeFn = fn( +pub type DeserializeFn = unsafe fn( &mut EntityWorldMut, - &mut ServerEntityMap, &mut Cursor<&[u8]>, + &mut ServerEntityMap, RepliconTick, + WriteFn, ) -> bincode::Result<()>; +/// Signature of component writing functions. +pub type WriteFn = unsafe fn(&mut EntityWorldMut, OwningPtr, RepliconTick); + /// Signature of component removal functions. pub type RemoveFn = fn(&mut EntityWorldMut, RepliconTick); @@ -77,9 +85,12 @@ pub struct ComponentFns { /// Function that serializes a component into bytes. pub serialize: SerializeFn, - /// Function that deserializes a component from bytes and inserts it to [`EntityWorldMut`]. + /// Function that deserializes a component from bytes. pub deserialize: DeserializeFn, + /// Function that inserts a component to [`EntityWorldMut`]. + pub write: WriteFn, + /// Function that removes a component from [`EntityWorldMut`]. pub remove: RemoveFn, } @@ -95,6 +106,7 @@ impl ComponentFns { Self { serialize: serialize::, deserialize: deserialize::, + write: write::, remove: remove::, } } @@ -109,6 +121,7 @@ impl ComponentFns { Self { serialize: serialize::, deserialize: deserialize_mapped::, + write: write::, remove: remove::, } } @@ -120,38 +133,50 @@ impl ComponentFns { #[derive(Clone, Copy, Deserialize, Eq, Hash, PartialEq, Serialize)] pub struct ComponentFnsId(usize); -/// Default serialization function. +/// Default component serialization function. /// /// # Safety /// /// `C` must be the erased pointee type for this [`Ptr`]. pub unsafe fn serialize( - component: Ptr, + ptr: Ptr, cursor: &mut Cursor>, ) -> bincode::Result<()> { - let component: &C = component.deref(); + let component: &C = ptr.deref(); DefaultOptions::new().serialize_into(cursor, component) } -/// Default deserialization function. -pub fn deserialize( +/// Default component deserialization function. +/// +/// # Safety +/// +/// `write` must be safely callable with `C` as [`Ptr`]. +pub unsafe fn deserialize( entity: &mut EntityWorldMut, - _entity_map: &mut ServerEntityMap, cursor: &mut Cursor<&[u8]>, - _replicon_tick: RepliconTick, + _entity_map: &mut ServerEntityMap, + replicon_tick: RepliconTick, + write: WriteFn, ) -> bincode::Result<()> { let component: C = DefaultOptions::new().deserialize_from(cursor)?; - entity.insert(component); + OwningPtr::make(component, |ptr| { + (write)(entity, ptr, replicon_tick); + }); Ok(()) } /// Like [`deserialize`], but also maps entities before insertion. -pub fn deserialize_mapped( +/// +/// # Safety +/// +/// `write` must be safely callable with `C` as [`Ptr`]. +pub unsafe fn deserialize_mapped( entity: &mut EntityWorldMut, - entity_map: &mut ServerEntityMap, cursor: &mut Cursor<&[u8]>, - _replicon_tick: RepliconTick, + entity_map: &mut ServerEntityMap, + replicon_tick: RepliconTick, + write: WriteFn, ) -> bincode::Result<()> { let mut component: C = DefaultOptions::new().deserialize_from(cursor)?; @@ -159,11 +184,27 @@ pub fn deserialize_mapped( component.map_entities(&mut ClientMapper::new(world, entity_map)); }); - entity.insert(component); + OwningPtr::make(component, |ptr| { + (write)(entity, ptr, replicon_tick); + }); Ok(()) } +/// Default component writing function. +/// +/// # Safety +/// +/// `C` must be the erased pointee type for this [`Ptr`]. +pub unsafe fn write( + entity: &mut EntityWorldMut, + ptr: OwningPtr, + _replicon_tick: RepliconTick, +) { + let component: C = ptr.read(); + entity.insert(component); +} + /// Default component removal function. pub fn remove(entity: &mut EntityWorldMut, _replicon_tick: RepliconTick) { entity.remove::(); diff --git a/src/core/replication_rules.rs b/src/core/replication_rules.rs index 7b978504..0f6ade1d 100644 --- a/src/core/replication_rules.rs +++ b/src/core/replication_rules.rs @@ -17,7 +17,7 @@ pub trait AppReplicationExt { /// marker component. /// /// Component will be serialized and deserialized as-is using bincode. - /// To customize how the component will be serialized, use [`Self::replicate_group`]. + /// To customize it, use [`Self::replicate_group`]. /// /// If your component contains any [`Entity`] inside, use [`Self::replicate_mapped`]. /// @@ -26,7 +26,7 @@ pub trait AppReplicationExt { where C: Component + Serialize + DeserializeOwned, { - // SAFETY: Component is registered with the corresponding default serialization function. + // SAFETY: functions operate on the same component. unsafe { self.replicate_with::(ComponentFns::default_fns::()) }; self } @@ -40,7 +40,7 @@ pub trait AppReplicationExt { where C: Component + Serialize + DeserializeOwned + MapEntities, { - // SAFETY: Component is registered with the corresponding default serialization function. + // SAFETY: functions operate on the same component. unsafe { self.replicate_with::(ComponentFns::default_mapped_fns::()) }; self } @@ -53,27 +53,36 @@ pub trait AppReplicationExt { # Safety - Caller must ensure that component `C` can be safely passed to [`ComponentFns::serialize`]. + Caller must ensure that component `C` can be safely passed to [`ComponentFns::serialize`] and + [`ComponentFns::deserialize`] can be safely called with [`ComponentFns::write`]. + In other words, functions should operate on the same component. # Examples ``` use std::io::Cursor; - use bevy::{prelude::*, ptr::Ptr}; + use bevy::{ + prelude::*, + ptr::{OwningPtr, Ptr}, + }; use bevy_replicon::{ client::client_mapper::ServerEntityMap, - core::{replication_fns::{self, ComponentFns}, replicon_tick::RepliconTick}, + core::{ + replication_fns::{self, ComponentFns, WriteFn}, + replicon_tick::RepliconTick, + }, prelude::*, }; # let mut app = App::new(); # app.add_plugins(RepliconPlugins); - // SAFETY: `serialize_translation` expects `Transform`. + // SAFETY: functions operate on the same component. unsafe { app.replicate_with::(ComponentFns { serialize: serialize_translation, deserialize: deserialize_translation, + write: replication_fns::write::, remove: replication_fns::remove::, }); } @@ -83,27 +92,34 @@ pub trait AppReplicationExt { /// # Safety /// /// [`Transform`] must be the erased pointee type for this [`Ptr`]. - unsafe fn serialize_translation(component: Ptr, cursor: &mut Cursor>) -> bincode::Result<()> { - let transform: &Transform = component.deref(); + unsafe fn serialize_translation(ptr: Ptr, cursor: &mut Cursor>) -> bincode::Result<()> { + let transform: &Transform = ptr.deref(); bincode::serialize_into(cursor, &transform.translation) } /// Deserializes `translation` and creates [`Transform`] from it. - fn deserialize_translation( + /// # Safety + /// + /// `write` must be safely callable with [`Transform`] as [`Ptr`]. + unsafe fn deserialize_translation( entity: &mut EntityWorldMut, - _entity_map: &mut ServerEntityMap, cursor: &mut Cursor<&[u8]>, - _replicon_tick: RepliconTick, + _entity_map: &mut ServerEntityMap, + replicon_tick: RepliconTick, + write: WriteFn, ) -> bincode::Result<()> { let translation: Vec3 = bincode::deserialize_from(cursor)?; - entity.insert(Transform::from_translation(translation)); + OwningPtr::make(translation, |ptr| { + (write)(entity, ptr, replicon_tick); + }); Ok(()) } ``` - The [`remove`](super::replication_fns::remove) used in this example is the default component - removal function, but you can replace it with your own as well. + The [`write`](super::replication_fns::write) and [`remove`](super::replication_fns::remove) functions + used in this example are the default component writing and removal functions, + but you can replace them with your own as well. */ unsafe fn replicate_with(&mut self, component_fns: ComponentFns) -> &mut Self where @@ -125,7 +141,7 @@ pub trait AppReplicationExt { Replication for them will be stopped, unless they match other rules. We provide blanket impls for tuples to replicate them as-is, but a user could manually implement the trait - to customize how components will be serialized, deserialized and removed. For details see [`GroupReplication`]. + to customize how components will be serialized, deserialized, written and removed. For details see [`GroupReplication`]. # Panics @@ -210,7 +226,9 @@ impl ReplicationRule { /// # Safety /// /// Caller must ensure that in each pair the associated component can be safely - /// passed to the associated [`ComponentFns::serialize`]. + /// passed to [`ComponentFns::serialize`] and [`ComponentFns::deserialize`] can + /// be safely called with [`ComponentFns::write`]. + /// In other words, functions should operate on the same component. pub unsafe fn new(components: Vec<(ComponentId, ComponentFnsId)>) -> Self { Self { priority: components.len(), @@ -255,7 +273,7 @@ impl ReplicationRule { } /** -Describes how a component group should be serialized, deserialized, and removed. +Describes how a component group should be serialized, deserialized, written, and removed. Can be implemented on any struct to create a custom replication group. @@ -269,7 +287,7 @@ use bevy_replicon::{ client::client_mapper::ServerEntityMap, core::{ replication_rules::{self, GroupReplication, ReplicationRule}, - replication_fns::{self, ReplicationFns, ComponentFns}, + replication_fns::{self, ReplicationFns, ComponentFns, WriteFn}, replicon_tick::RepliconTick, }, prelude::*, @@ -298,7 +316,9 @@ impl GroupReplication for PlayerBundle { // For function definitions see the example from `AppReplicationExt::replicate_with`. serialize: serialize_translation, deserialize: deserialize_translation, - remove: replication_fns::remove::, // Use default removal function. + // Use default write and removal functions. + write: replication_fns::write::, + remove: replication_fns::remove::, }); // Serialize `player` as usual. @@ -315,13 +335,13 @@ impl GroupReplication for PlayerBundle { (visibility_id, visibility_fns_id), ]; - // SAFETY: all components can be safely passed to their serialization functions. + // SAFETY: in all pairs functions operate on the same component unsafe { ReplicationRule::new(components) } } } # fn serialize_translation(_: Ptr, _: &mut Cursor>) -> bincode::Result<()> { unimplemented!() } -# fn deserialize_translation(_: &mut EntityWorldMut, _: &mut ServerEntityMap, _: &mut Cursor<&[u8]>, _: RepliconTick) -> bincode::Result<()> { unimplemented!() } +# fn deserialize_translation(_: &mut EntityWorldMut, _: &mut Cursor<&[u8]>, _: &mut ServerEntityMap, _: RepliconTick, _: WriteFn) -> bincode::Result<()> { unimplemented!() } ``` **/ pub trait GroupReplication { @@ -341,7 +361,7 @@ macro_rules! impl_registrations { components.push((component_id, fns_id)); )* - // SAFETY: Components are registered with the appropriate default serialization functions. + // SAFETY: in all pairs functions operate on the same component unsafe { ReplicationRule::new(components) } } }