Skip to content

Commit

Permalink
prevent strong handle crash
Browse files Browse the repository at this point in the history
  • Loading branch information
raffaeleragni committed Dec 17, 2023
1 parent 847cdd9 commit 7edc3ae
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 75 deletions.
18 changes: 12 additions & 6 deletions src/client/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,15 @@ pub(crate) fn react_on_changed_components(
) {
let registry = registry.read();
while let Some(change) = track.changed_components_to_send.pop_front() {
client.send_message(
let Ok(bin) = compo_to_bin(change.data.as_reflect(), &registry) else {
break;
};
client.send_message(
DefaultChannel::ReliableOrdered,
bincode::serialize(&Message::ComponentUpdated {
id: change.change_id.id,
name: change.change_id.name,
data: compo_to_bin(change.data.as_reflect(), &registry),
data: bin,
})
.unwrap(),
);
Expand All @@ -104,19 +107,22 @@ pub(crate) fn react_on_changed_materials(
match event {
AssetEvent::Added { id } | AssetEvent::Modified { id } => {
let Some(material) = materials.get(*id) else {
return;
break;
};
let AssetId::Uuid { uuid: id } = id else {
return;
break;
};
if track.skip_network_handle_change(*id) {
return;
break;
}
let Ok(bin) = compo_to_bin(material.as_reflect(), &registry) else {
break;
};
client.send_message(
DefaultChannel::ReliableOrdered,
bincode::serialize(&Message::StandardMaterialUpdated {
id: *id,
material: compo_to_bin(material.as_reflect(), &registry),
material: bin,
})
.unwrap(),
);
Expand Down
11 changes: 5 additions & 6 deletions src/proto_serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@ use bevy::reflect::{
DynamicStruct, Reflect, ReflectFromReflect, TypeRegistry,
};

use bincode::{DefaultOptions, Options};
use bincode::{DefaultOptions, Options, ErrorKind};
use serde::de::DeserializeSeed;

pub(crate) fn compo_to_bin(compo: &dyn Reflect, registry: &TypeRegistry) -> Vec<u8> {
pub(crate) fn compo_to_bin(compo: &dyn Reflect, registry: &TypeRegistry) -> Result<Vec<u8>, Box<ErrorKind>> {
let serializer = ReflectSerializer::new(compo, registry);
DefaultOptions::new()
.with_fixint_encoding()
.allow_trailing_bytes()
.serialize(&serializer)
.unwrap()
}

pub(crate) fn bin_to_compo(data: &[u8], registry: &TypeRegistry) -> Box<dyn Reflect> {
Expand Down Expand Up @@ -59,7 +58,7 @@ mod test {
let mut registry = TypeRegistry::default();
registry.register::<T>();

let data = compo_to_bin(compo_orig.as_reflect(), &registry);
let data = compo_to_bin(compo_orig.as_reflect(), &registry).unwrap();

let compo_result = bin_to_compo(&data, &registry);
let compo_result = compo_result.downcast::<T>().unwrap();
Expand Down Expand Up @@ -87,7 +86,7 @@ mod test {
registry.register_type_data::<Vec3, ReflectFromReflect>();
registry.register_type_data::<Quat, ReflectFromReflect>();

let data = compo_to_bin(compo_orig.as_reflect(), &registry);
let data = compo_to_bin(compo_orig.as_reflect(), &registry).unwrap();

let compo_result = bin_to_compo(&data, &registry);
let compo_result = compo_result.downcast::<Transform>().unwrap();
Expand All @@ -113,7 +112,7 @@ mod test {
registry.register::<OpaqueRendererMethod>();
registry.register_type_data::<StandardMaterial, ReflectFromReflect>();

let data = compo_to_bin(material_orig.as_reflect(), &registry);
let data = compo_to_bin(material_orig.as_reflect(), &registry).unwrap();

let result = bin_to_compo(&data, &registry);
let result = result.downcast::<StandardMaterial>().unwrap();
Expand Down
10 changes: 5 additions & 5 deletions src/server/initial_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ fn check_entity_components(world: &World, result: &mut Vec<Message>) -> Result<(
let entity = world.entity(arch_entity.entity());
let e_id = entity.id();
let component = reflect_component.reflect(entity).ok_or("not registered")?;
let compo_bin = compo_to_bin(component.as_reflect(), &registry);
let Ok(compo_bin) = compo_to_bin(component.as_reflect(), &registry) else {break};
result.push(Message::ComponentUpdated {
id: e_id,
name: type_name.into(),
Expand Down Expand Up @@ -144,10 +144,10 @@ fn check_materials(world: &World, result: &mut Vec<Message>) -> Result<(), Box<d
let AssetId::Uuid { uuid: id } = id else {
continue;
};
result.push(Message::StandardMaterialUpdated {
id,
material: compo_to_bin(material.as_reflect(), &registry),
});
let Ok(bin) = compo_to_bin(material.as_reflect(), &registry) else {
break;
};
result.push(Message::StandardMaterialUpdated { id, material: bin });
}
}
Ok(())
Expand Down
32 changes: 19 additions & 13 deletions src/server/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub(crate) fn entity_created_on_server(
mut query: Query<Entity, Added<SyncMark>>,
) {
for id in query.iter_mut() {
for client_id in server.clients_id().into_iter() {
for client_id in server.clients_id().into_iter() {
server.send_message(
client_id,
DefaultChannel::ReliableOrdered,
Expand Down Expand Up @@ -61,7 +61,7 @@ pub(crate) fn reply_back_to_client_generated_entity(
mut query: Query<(Entity, &SyncClientGeneratedEntity), Added<SyncClientGeneratedEntity>>,
) {
for (entity_id, marker_component) in query.iter_mut() {
server.send_message(
server.send_message(
marker_component.client_id,
DefaultChannel::ReliableOrdered,
bincode::serialize(&Message::EntitySpawnBack {
Expand Down Expand Up @@ -118,16 +118,19 @@ pub(crate) fn react_on_changed_components(
) {
let registry = registry.read();
while let Some(change) = track.changed_components_to_send.pop_front() {
let Ok(bin) = compo_to_bin(change.data.as_reflect(), &registry) else {
break;
};
let msg = &Message::ComponentUpdated {
id: change.change_id.id,
name: change.change_id.name.clone(),
data: bin,
};
for cid in server.clients_id().into_iter() {
server.send_message(
cid,
DefaultChannel::ReliableOrdered,
bincode::serialize(&Message::ComponentUpdated {
id: change.change_id.id,
name: change.change_id.name.clone(),
data: compo_to_bin(change.data.as_reflect(), &registry),
})
.unwrap(),
bincode::serialize(msg).unwrap(),
);
}
}
Expand All @@ -153,15 +156,18 @@ pub(crate) fn react_on_changed_materials(
if track.skip_network_handle_change(*id) {
return;
}
let Ok(bin) = compo_to_bin(material.as_reflect(), &registry) else {
break;
};
let msg = &Message::StandardMaterialUpdated {
id: *id,
material: bin,
};
for cid in server.clients_id().into_iter() {
server.send_message(
cid,
DefaultChannel::ReliableOrdered,
bincode::serialize(&Message::StandardMaterialUpdated {
id: *id,
material: compo_to_bin(material.as_reflect(), &registry),
})
.unwrap(),
bincode::serialize(msg).unwrap(),
);
}
}
Expand Down
27 changes: 1 addition & 26 deletions tests/asset_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use assert::{assets_has_sample_mesh, material_has_color};
use bevy::prelude::*;
use bevy_sync::SyncComponent;
use serial_test::serial;
use setup::{spawn_new_material, spawn_new_mesh, TestRun, load_cube};
use setup::{spawn_new_material, spawn_new_mesh, TestRun};

#[test]
#[serial]
Expand Down Expand Up @@ -106,28 +106,3 @@ fn test_mesh_transferred_from_client() {
);
}

#[test]
#[serial]
fn test_with_asset_loader() {
TestRun::default().run(
1,
|env| {
env.setup_registration::<Handle<Mesh>>();
env.setup_registration::<Handle<StandardMaterial>>();
for app in [&mut env.server, &mut env.clients[0]] {
app.sync_meshes(true);
app.sync_materials(true);
}
},
|env| {
let app = &mut env.server;
load_cube(app)
},
|env, _, (mesh_id, material_id)| {
println!("{:?} {:?}", mesh_id, material_id);
assets_has_sample_mesh(&mut env.clients[0], mesh_id);
material_has_color(&mut env.clients[0], material_id, Color::RED);
},
);
}

36 changes: 35 additions & 1 deletion tests/initial_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use assert::{assets_has_sample_mesh, material_has_color};
use bevy::prelude::*;
use bevy_sync::{SyncComponent, SyncExclude, SyncMark};
use serial_test::serial;
use setup::{spawn_new_material, spawn_new_mesh, MySynched, TestEnv, TestRun};
use setup::{spawn_new_material, spawn_new_mesh, MySynched, TestEnv, TestRun, spawn_new_material_nouuid, spawn_new_mesh_nouuid};

use crate::{assert::count_entities_with_component, setup::MySynched2};

Expand Down Expand Up @@ -150,3 +150,37 @@ fn test_initial_with_parenting() {
},
);
}

#[test]
#[serial]
fn test_initial_world_sync_without_uuid() {
TestRun::default().run(
1,
|env| {
env.setup_registration::<MySynched>();
env.setup_registration::<Handle<StandardMaterial>>();
env.setup_registration::<Handle<Mesh>>();
env.server.sync_materials(true);
env.server.sync_meshes(true);
let e_id = env.server.world.spawn(SyncMark {}).id();

let material_id = spawn_new_material_nouuid(&mut env.server);
let mesh_id = spawn_new_mesh_nouuid(&mut env.server);

let mut e = env.server.world.entity_mut(e_id);
e.insert((MySynched { value: 7 }, material_id, mesh_id));

1
},
TestRun::no_setup,
|env, entity_count: u32, _| {
assert::initial_sync_for_client_happened(
&mut env.server,
&mut env.clients[0],
entity_count,
);
assert::no_messages_left_for_server(&mut env.server);
assert::no_messages_left_for_client(&mut env.clients[0]);
},
);
}
34 changes: 16 additions & 18 deletions tests/setup/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{
error::Error,
fmt::Display,
net::{IpAddr, Ipv4Addr}, os::unix::thread, time::Duration,
net::{IpAddr, Ipv4Addr},
};

use bevy::{
Expand Down Expand Up @@ -228,6 +228,21 @@ pub(crate) fn spawn_new_mesh(app: &mut App) -> AssetId<Mesh> {
id.into()
}

#[allow(dead_code)]
pub(crate) fn spawn_new_material_nouuid(app: &mut App) -> Handle<StandardMaterial> {
let mut materials = app.world.resource_mut::<Assets<StandardMaterial>>();
materials.add(StandardMaterial {
base_color: Color::RED,
..Default::default()
})
}

#[allow(dead_code)]
pub(crate) fn spawn_new_mesh_nouuid(app: &mut App) -> Handle<Mesh> {
let mut meshes = app.world.resource_mut::<Assets<Mesh>>();
meshes.add(sample_mesh())
}

#[allow(dead_code)]
pub(crate) fn sample_mesh() -> Mesh {
let mut mesh = Mesh::new(PrimitiveTopology::TriangleList);
Expand All @@ -243,20 +258,3 @@ pub(crate) fn sample_mesh() -> Mesh {
mesh
}

#[allow(dead_code)]
pub(crate) fn load_cube(app: &mut App) -> (AssetId<Mesh>, AssetId<StandardMaterial>) {
app.init_asset::<Scene>();
app.init_asset::<Mesh>();
app.init_asset::<StandardMaterial>();
let assets = app.world.resource_mut::<AssetServer>();
let scene = assets.load("cube.glb#Scene0");
app.world.spawn(SceneBundle {
scene,
..Default::default()
});
let meshes = app.world.resource_mut::<Assets<Mesh>>();
let mesh_id = meshes.iter().next().unwrap().0;
let materials = app.world.resource_mut::<Assets<StandardMaterial>>();
let material_id = materials.iter().next().unwrap().0;
(mesh_id, material_id)
}

0 comments on commit 7edc3ae

Please sign in to comment.