make component retrieval work

Author: makspll

assets/tests/register_new_component/new_component_can_be_retrieved.lua

@@ -1,4 +1,4 @@
-local NewComponent = world.register_new_component("NewComponent")
+local NewComponent = world.register_new_component("ScriptComponentA")
 assert(NewComponent ~= nil, "Failed to register new component")
 assert(NewComponent:short_name() == "ScriptComponent", "Unexpected component type")
 

crates/bevy_mod_scripting_core/src/bindings/query.rs

@@ -1,18 +1,20 @@
 //! Utilities for querying the world.
 
-use super::{with_global_access, ReflectReference, WorldAccessGuard};
+use super::{with_global_access, ReflectReference, ScriptComponent, WorldAccessGuard, WorldGuard};
 use crate::error::InteropError;
 use bevy::{
     ecs::{
         component::ComponentId,
         entity::Entity,
         query::{QueryData, QueryState},
+        reflect::ReflectComponent,
         world::World,
     },
     prelude::{EntityRef, QueryBuilder},
+    ptr::OwningPtr,
     reflect::{ParsedPath, Reflect, TypeRegistration},
 };
-use std::{any::TypeId, collections::VecDeque, sync::Arc};
+use std::{any::TypeId, collections::VecDeque, ptr::NonNull, sync::Arc};
 
 /// A reference to a type which is not a `Resource` or `Component`.
 ///
@@ -27,9 +29,13 @@ pub struct ScriptTypeRegistration {
 /// A reference to a component type's reflection registration.
 ///
 /// In general think of this as a handle to a type.
+///
+/// Not to be confused with script registered dynamic components, although this can point to a script registered component.
 pub struct ScriptComponentRegistration {
     pub(crate) registration: ScriptTypeRegistration,
     pub(crate) component_id: ComponentId,
+    /// whether this is a component registered BY a script
+    pub(crate) is_dynamic_script_component: bool,
 }
 
 #[derive(Clone, Reflect, Debug)]
@@ -100,6 +106,8 @@ impl ScriptComponentRegistration {
     /// Creates a new [`ScriptComponentRegistration`] from a [`ScriptTypeRegistration`] and a [`ComponentId`].
     pub fn new(registration: ScriptTypeRegistration, component_id: ComponentId) -> Self {
         Self {
+            is_dynamic_script_component: registration.type_id()
+                == std::any::TypeId::of::<ScriptComponent>(),
             registration,
             component_id,
         }
@@ -120,6 +128,70 @@ impl ScriptComponentRegistration {
     pub fn into_type_registration(self) -> ScriptTypeRegistration {
         self.registration
     }
+
+    /// Inserts an instance of this component into the given entity
+    ///
+    /// Requires whole world access
+    pub fn insert_into_entity(
+        &self,
+        world: WorldGuard,
+        entity: Entity,
+        instance: Box<dyn Reflect>,
+    ) -> Result<(), InteropError> {
+        if self.is_dynamic_script_component {
+            // if dynamic we already know the type i.e. `ScriptComponent`
+            // so we can just insert it
+
+            world.with_global_access(|world| {
+                let mut entity = world
+                    .get_entity_mut(entity)
+                    .map_err(|_| InteropError::missing_entity(entity))?;
+                let mut cast = instance.downcast::<ScriptComponent>().map_err(|v| {
+                    InteropError::type_mismatch(TypeId::of::<ScriptComponent>(), Some(v.type_id()))
+                })?;
+                let ptr = (cast.as_mut() as *mut ScriptComponent).cast();
+                // Safety: cannot be null as we just created it from a valid reference
+                let non_null_ptr = unsafe { NonNull::new_unchecked(ptr) };
+                // Safety:
+                // - we know the type is ScriptComponent, as we just created the pointer
+                // - the box will stay valid for the life of this function, and we do not return the ptr
+                // - pointer is alligned correctly
+                let owning_ptr = unsafe { OwningPtr::new(non_null_ptr) };
+                // Safety:
+                // - Owning Ptr is valid as we just created it
+                // - TODO: do we need to check if ComponentId is from this world? How?
+                unsafe { entity.insert_by_id(self.component_id, owning_ptr) };
+                Ok(())
+            })?
+        } else {
+            let component_data = self
+                .type_registration()
+                .type_registration()
+                .data::<ReflectComponent>()
+                .ok_or_else(|| {
+                    InteropError::missing_type_data(
+                        self.registration.type_id(),
+                        "ReflectComponent".to_owned(),
+                    )
+                })?;
+
+            bevy::log::debug!("found component data");
+
+            //  TODO: this shouldn't need entire world access it feels
+            let type_registry = world.type_registry();
+            world.with_global_access(|world| {
+                let mut entity = world
+                    .get_entity_mut(entity)
+                    .map_err(|_| InteropError::missing_entity(entity))?;
+                {
+                    let registry = type_registry.read();
+                    bevy::log::debug!("inserting component instance using component data");
+                    component_data.insert(&mut entity, instance.as_partial_reflect(), &registry);
+                }
+                Ok(())
+            })?
+        }
+    }
 }
 
 impl std::fmt::Debug for ScriptTypeRegistration {

crates/bevy_mod_scripting_core/src/bindings/script_component.rs

@@ -6,7 +6,6 @@ use bevy::{
     app::{App, Plugin},
     ecs::{
         component::{Component, ComponentDescriptor, StorageType},
-        reflect::ReflectComponent,
         system::Resource,
     },
     reflect::{prelude::ReflectDefault, GetTypeRegistration, Reflect},
@@ -17,7 +16,7 @@ use std::{alloc::Layout, mem::needs_drop, sync::Arc};
 
 /// A dynamic script component, with script set
 #[derive(Reflect, Clone, Default)]
-#[reflect(Component, Default)]
+#[reflect(Default)]
 pub struct ScriptComponent {
     data: ScriptValue,
 }
@@ -74,6 +73,14 @@ impl WorldAccessGuard<'_> {
         &self,
         component_name: String,
     ) -> Result<ScriptComponentRegistration, InteropError> {
+        if !component_name.starts_with("Script") {
+            return Err(InteropError::unsupported_operation(
+                None,
+                None,
+                "script registered component name must start with 'Script'",
+            ));
+        }
+
         let component_id = self.with_global_access(|w| {
             let descriptor = unsafe {
                 // Safety: same safety guarantees as ComponentDescriptor::new
@@ -127,3 +134,52 @@ impl Plugin for DynamicScriptComponentPlugin {
             .register_type::<ScriptComponent>();
     }
 }
+
+// #[cfg(test)]
+// mod test {
+//     use std::ptr::NonNull;
+
+//     use super::*;
+//     use bevy::{ecs::world::World, ptr::OwningPtr};
+
+//     #[test]
+//     fn test_script_component() {
+//         let mut world = World::new();
+//         let component_name = "MyScriptComponent";
+
+//         #[derive(Reflect, Component)]
+//         struct UnderlyingComponent;
+
+//         // initialize component descriptor dynamically
+//         let descriptor = unsafe {
+//             // Safety: same safety guarantees as ComponentDescriptor::new
+//             // we know the type in advance
+//             // we only use this method to name the component
+//             ComponentDescriptor::new_with_layout(
+//                 component_name,
+//                 UnderlyingComponent::STORAGE_TYPE,
+//                 Layout::new::<UnderlyingComponent>(),
+//                 needs_drop::<UnderlyingComponent>()
+//                     .then_some(|x| x.drop_as::<UnderlyingComponent>()),
+//             )
+//         };
+
+//         // register with the world
+//         let component_id = world.register_component_with_descriptor(descriptor);
+
+//         // insert into the entity
+//         let entity = world.spawn_empty().id();
+//         let mut entity = world.entity_mut(entity);
+
+//         let value = Box::new(UnderlyingComponent);
+//         let value_ref = Box::into_raw(value).cast::<u8>();
+//         let ptr = unsafe { OwningPtr::new(NonNull::new(value_ref).unwrap()) };
+//         unsafe { entity.insert_by_id(component_id, ptr) };
+
+//         // check it gets inserted
+//         assert!(
+//             entity.contains_id(component_id),
+//             "entity does not contain freshly inserted component"
+//         )
+//     }
+// }

crates/bevy_mod_scripting_core/src/bindings/world.rs

@@ -24,7 +24,7 @@ use super::{
 use crate::{
     bindings::{
         function::{from::FromScript, from_ref::FromScriptRef},
-        with_access_read, with_access_write,
+        with_access_read, with_access_write, ScriptComponent,
     },
     error::InteropError,
     reflection_extensions::PartialReflectExt,
@@ -928,20 +928,6 @@ impl WorldAccessGuard<'_> {
         entity: Entity,
         registration: ScriptComponentRegistration,
     ) -> Result<(), InteropError> {
-        // let cell = self.as_unsafe_world_cell()?;
-        let component_data = registration
-            .type_registration()
-            .type_registration()
-            .data::<ReflectComponent>()
-            .ok_or_else(|| {
-                InteropError::missing_type_data(
-                    registration.registration.type_id(),
-                    "ReflectComponent".to_owned(),
-                )
-            })?;
-
-        bevy::log::debug!("found component data");
-
         // we look for ReflectDefault or ReflectFromWorld data then a ReflectComponent data
         let instance = if let Some(default_td) = registration
             .type_registration()
@@ -965,20 +951,7 @@ impl WorldAccessGuard<'_> {
             ));
         };
 
-        //  TODO: this shouldn't need entire world access it feels
-        self.with_global_access(|world| {
-            let type_registry = self.type_registry();
-
-            let mut entity = world
-                .get_entity_mut(entity)
-                .map_err(|_| InteropError::missing_entity(entity))?;
-            {
-                let registry = type_registry.read();
-                bevy::log::debug!("inserting component instance using component data");
-                component_data.insert(&mut entity, instance.as_partial_reflect(), &registry);
-            }
-            Ok(())
-        })?
+        registration.insert_into_entity(self.clone(), entity, instance)
     }
 
     /// insert the component into the entity
@@ -1038,14 +1011,17 @@ impl WorldAccessGuard<'_> {
             component_info
         );
 
-        if entity.contains_id(component_id)
-            || component_info
-                .type_id()
-                .is_some_and(|t| entity.contains_type_id(t))
-        {
+        if entity.contains_id(component_id) {
+            let type_id = component_info.type_id().or_else(|| {
+                // check its a script component
+                component_info
+                    .name()
+                    .starts_with("Script")
+                    .then_some(TypeId::of::<ScriptComponent>())
+            });
             Ok(Some(ReflectReference {
                 base: ReflectBaseType {
-                    type_id: component_info.type_id().ok_or_else(|| {
+                    type_id: type_id.ok_or_else(|| {
                         InteropError::unsupported_operation(
                             None,
                             None,