From e464c49592313be98907d1da38144431a85a122c Mon Sep 17 00:00:00 2001 From: DasLixou Date: Mon, 9 Sep 2024 12:28:23 +0200 Subject: [PATCH] Getting rid of most of unsafe stuff --- crates/blenvy/src/components/fake_entity.rs | 45 +++------- crates/blenvy/src/components/process_gltfs.rs | 89 +++++++++++++------ .../ronstring_to_reflect_component.rs | 4 +- 3 files changed, 77 insertions(+), 61 deletions(-) diff --git a/crates/blenvy/src/components/fake_entity.rs b/crates/blenvy/src/components/fake_entity.rs index 948de26..9cac12b 100644 --- a/crates/blenvy/src/components/fake_entity.rs +++ b/crates/blenvy/src/components/fake_entity.rs @@ -2,37 +2,24 @@ use std::{alloc::Layout, cell::Cell, num::NonZeroU32}; use bevy::{ core::Name, - ecs::world::DeferredWorld, + ecs::system::SystemParam, gltf::GltfExtras, log::{info, warn}, - prelude::{HierarchyQueryExt, Parent, QueryState, With, World}, + prelude::{HierarchyQueryExt, Parent, Query, With}, reflect::ReflectDeserialize, scene::{InstanceId, SceneInstance}, }; use serde::Deserialize; -pub(crate) struct BadWorldAccess { - world: *mut World, - names: QueryState<(bevy::ecs::entity::Entity, &'static Name), With>, - hierarchy: QueryState<&'static Parent, ()>, - scene_instances: QueryState<&'static SceneInstance, ()>, -} - -impl BadWorldAccess { - pub unsafe fn new(world: &mut World) -> Self { - BadWorldAccess { - world, - // We have to check that we focus on a node, not a mesh with the same name. - // Currently, the only possible way of checking this is with `GltfExtras`, so selected entities must at least have one component. - names: world.query_filtered::<(bevy::ecs::entity::Entity, &Name), With>(), - hierarchy: world.query::<&Parent>(), - scene_instances: world.query::<&SceneInstance>(), - } - } +#[derive(SystemParam)] +pub(crate) struct BadWorldAccess<'w, 's> { + pub(crate) names: Query<'w, 's, (bevy::ecs::entity::Entity, &'static Name), With>, + pub(crate) hierarchy: Query<'w, 's, &'static Parent, ()>, + pub(crate) scene_instances: Query<'w, 's, &'static SceneInstance, ()>, } thread_local! { - pub(crate) static BAD_WORLD_ACCESS: Cell> = Cell::new(None); + pub(crate) static BAD_WORLD_ACCESS: Cell>> = Cell::new(None); pub(crate) static INSTANCE_ID: Cell> = Cell::new(None); } @@ -70,25 +57,20 @@ impl<'de> Deserialize<'de> for Entity { let entity = if let Some(name) = entity_data.name { info!("Found name {name}"); let BadWorldAccess { - world, - mut names, - mut hierarchy, - mut scene_instances, + names, + hierarchy, + scene_instances, } = BAD_WORLD_ACCESS.take().expect("No bad world access :c"); let instance = INSTANCE_ID.get().expect("No instance id set :c"); let mut target = None; - let w = unsafe { &*world.cast_const() }; - 'search: for (e, n) in names.iter(w) { + 'search: for (e, n) in names.iter() { if !name.eq(n.as_str()) { continue; } - let mut dw = DeferredWorld::from(unsafe { &mut *world }); - let hierarchy = dw.query(&mut hierarchy); - for parent in hierarchy.iter_ancestors(e) { - let Ok(id) = scene_instances.get(w, parent) else { + let Ok(id) = scene_instances.get(parent) else { continue; }; if instance.eq(id) { @@ -99,7 +81,6 @@ impl<'de> Deserialize<'de> for Entity { } BAD_WORLD_ACCESS.set(Some(BadWorldAccess { - world, names, hierarchy, scene_instances, diff --git a/crates/blenvy/src/components/process_gltfs.rs b/crates/blenvy/src/components/process_gltfs.rs index 71b895f..a3a23b0 100644 --- a/crates/blenvy/src/components/process_gltfs.rs +++ b/crates/blenvy/src/components/process_gltfs.rs @@ -6,12 +6,13 @@ use bevy::{ entity::Entity, query::{Added, Without}, reflect::{AppTypeRegistry, ReflectComponent}, - world::{DeferredWorld, World}, + system::{SystemParam, SystemState}, + world::World, }, gltf::{GltfExtras, GltfMaterialExtras, GltfMeshExtras, GltfSceneExtras}, hierarchy::Parent, log::{debug, warn}, - prelude::HierarchyQueryExt, + prelude::{HierarchyQueryExt, Local, Query, Res}, reflect::{Reflect, TypeRegistration}, scene::SceneInstance, utils::HashMap, @@ -61,18 +62,56 @@ fn find_entity_components( (target_entity, reflect_components) } +#[derive(SystemParam)] +#[doc(hidden)] +pub struct ExtrasComponentQueries<'w, 's> { + extras: Query< + 'w, + 's, + (Entity, Option<&'static Name>, &'static GltfExtras), + (Added, Without), + >, + scene_extras: Query< + 'w, + 's, + (Entity, Option<&'static Name>, &'static GltfSceneExtras), + (Added, Without), + >, + mesh_extras: Query< + 'w, + 's, + (Entity, Option<&'static Name>, &'static GltfMeshExtras), + (Added, Without), + >, + material_extras: Query< + 'w, + 's, + (Entity, Option<&'static Name>, &'static GltfMaterialExtras), + (Added, Without), + >, + // Hierarchy and Scene instances are both here and in BadWorldAccess, but they don't clash because read-only. + bad_world_access: BadWorldAccess<'w, 's>, + hierarchy: Query<'w, 's, &'static Parent>, + scene_instances: Query<'w, 's, &'static SceneInstance>, + type_registry: Res<'w, AppTypeRegistry>, +} + /// main function: injects components into each entity in gltf files that have `gltf_extras`, using reflection -pub fn add_components_from_gltf_extras(world: &mut World) { - let mut extras = world.query_filtered::<(Entity, Option<&Name>, &GltfExtras), (Added, Without)>(); - let mut scene_extras = world.query_filtered::<(Entity, Option<&Name>, &GltfSceneExtras), (Added, Without)>(); - let mut mesh_extras = world.query_filtered::<(Entity, Option<&Name>, &GltfMeshExtras), (Added, Without)>(); - let mut material_extras = world.query_filtered::<(Entity, Option<&Name>, &GltfMaterialExtras), (Added, Without)>(); - - let mut scene_instances = world.query::<&SceneInstance>(); - - let mut hierarchy_state = world.query::<&Parent>(); - let mut __unsafe_dw = DeferredWorld::from(unsafe { &mut *(world as *mut _) }); - let hierarchy = __unsafe_dw.query(&mut hierarchy_state); +pub fn add_components_from_gltf_extras( + world: &mut World, + mut queries_state: Local>>>, +) { + let state = queries_state.get_or_insert_with(|| SystemState::new(world)); + let ExtrasComponentQueries { + extras, + scene_extras, + mesh_extras, + material_extras, + bad_world_access, + hierarchy, + scene_instances, + type_registry, + } = state.get_mut(world); let mut entity_components: HashMap, TypeRegistration)>> = HashMap::new(); @@ -80,11 +119,11 @@ pub fn add_components_from_gltf_extras(world: &mut World) { // let gltf_components_config = world.resource::(); unsafe { - // SAFETY: we don't do anything harmful until taking this, and have full world access - fake_entity::BAD_WORLD_ACCESS.set(Some(BadWorldAccess::new(world))); + // SAFETY: we set this to `None` again before using world again and fake_entity just uses it in that time. + fake_entity::BAD_WORLD_ACCESS.set(Some(core::mem::transmute(bad_world_access))); } - for (entity, name, extra) in extras.iter(world) { + for (entity, name, extra) in extras.iter() { let parent = hierarchy.get(entity).ok(); debug!( "Gltf Extra: Name: {:?}, entity {:?}, parent: {:?}, extras {:?}", @@ -93,7 +132,7 @@ pub fn add_components_from_gltf_extras(world: &mut World) { if let Some(instance) = hierarchy .iter_ancestors(entity) - .find_map(|p| scene_instances.get(world, p).ok()) + .find_map(|p| scene_instances.get(p).ok()) { fake_entity::INSTANCE_ID.set(Some(*instance.deref())); } else { @@ -101,7 +140,6 @@ pub fn add_components_from_gltf_extras(world: &mut World) { fake_entity::INSTANCE_ID.set(None); }; - let type_registry: &AppTypeRegistry = world.resource(); let mut type_registry = type_registry.write(); let reflect_components = ronstring_to_reflect_component(&extra.value, &mut type_registry); // let name = name.unwrap_or(&Name::new("")); @@ -112,7 +150,7 @@ pub fn add_components_from_gltf_extras(world: &mut World) { entity_components.insert(target_entity, updated_components); } - for (entity, name, extra) in scene_extras.iter(world) { + for (entity, name, extra) in scene_extras.iter() { let parent = hierarchy.get(entity).ok(); debug!( "Gltf Scene Extra: Name: {:?}, entity {:?}, parent: {:?}, scene_extras {:?}", @@ -121,7 +159,7 @@ pub fn add_components_from_gltf_extras(world: &mut World) { if let Some(instance) = hierarchy .iter_ancestors(entity) - .find_map(|p| scene_instances.get(world, p).ok()) + .find_map(|p| scene_instances.get(p).ok()) { fake_entity::INSTANCE_ID.set(Some(*instance.deref())); } else { @@ -129,7 +167,6 @@ pub fn add_components_from_gltf_extras(world: &mut World) { fake_entity::INSTANCE_ID.set(None); }; - let type_registry: &AppTypeRegistry = world.resource(); let mut type_registry = type_registry.write(); let reflect_components = ronstring_to_reflect_component(&extra.value, &mut type_registry); @@ -138,7 +175,7 @@ pub fn add_components_from_gltf_extras(world: &mut World) { entity_components.insert(target_entity, updated_components); } - for (entity, name, extra) in mesh_extras.iter(world) { + for (entity, name, extra) in mesh_extras.iter() { let parent = hierarchy.get(entity).ok(); debug!( "Gltf Mesh Extra: Name: {:?}, entity {:?}, parent: {:?}, mesh_extras {:?}", @@ -147,7 +184,7 @@ pub fn add_components_from_gltf_extras(world: &mut World) { if let Some(instance) = hierarchy .iter_ancestors(entity) - .find_map(|p| scene_instances.get(world, p).ok()) + .find_map(|p| scene_instances.get(p).ok()) { fake_entity::INSTANCE_ID.set(Some(*instance.deref())); } else { @@ -155,7 +192,6 @@ pub fn add_components_from_gltf_extras(world: &mut World) { fake_entity::INSTANCE_ID.set(None); }; - let type_registry: &AppTypeRegistry = world.resource(); let mut type_registry = type_registry.write(); let reflect_components = ronstring_to_reflect_component(&extra.value, &mut type_registry); @@ -164,7 +200,7 @@ pub fn add_components_from_gltf_extras(world: &mut World) { entity_components.insert(target_entity, updated_components); } - for (entity, name, extra) in material_extras.iter(world) { + for (entity, name, extra) in material_extras.iter() { let parent = hierarchy.get(entity).ok(); debug!( "Name: {:?}, entity {:?}, parent: {:?}, material_extras {:?}", @@ -173,7 +209,7 @@ pub fn add_components_from_gltf_extras(world: &mut World) { if let Some(instance) = hierarchy .iter_ancestors(entity) - .find_map(|p| scene_instances.get(world, p).ok()) + .find_map(|p| scene_instances.get(p).ok()) { fake_entity::INSTANCE_ID.set(Some(*instance.deref())); } else { @@ -181,7 +217,6 @@ pub fn add_components_from_gltf_extras(world: &mut World) { fake_entity::INSTANCE_ID.set(None); }; - let type_registry: &AppTypeRegistry = world.resource(); let mut type_registry = type_registry.write(); let reflect_components = ronstring_to_reflect_component(&extra.value, &mut type_registry); diff --git a/crates/blenvy/src/components/ronstring_to_reflect_component.rs b/crates/blenvy/src/components/ronstring_to_reflect_component.rs index f23c38a..ba1a473 100644 --- a/crates/blenvy/src/components/ronstring_to_reflect_component.rs +++ b/crates/blenvy/src/components/ronstring_to_reflect_component.rs @@ -1,7 +1,7 @@ use std::any::TypeId; -use bevy::log::{debug, info, warn}; -use bevy::reflect::serde::{ReflectDeserializer, ReflectSerializer}; +use bevy::log::{debug, warn}; +use bevy::reflect::serde::ReflectDeserializer; use bevy::reflect::{GetTypeRegistration, Reflect, TypeRegistration, TypeRegistry}; use bevy::utils::HashMap; use ron::Value;