From 20639301d85f74633006a2d78b81e75e15b349f8 Mon Sep 17 00:00:00 2001 From: Jovan Gerodetti Date: Fri, 29 Mar 2024 18:00:01 +0800 Subject: [PATCH] Store Property / Method list in ScriptInstanceData #647 Co-authored-by: Lili Zoey --- godot-core/src/builtin/meta/mod.rs | 4 +- godot-core/src/engine/script_instance.rs | 69 ++++++++++++++----- .../script/script_instance_tests.rs | 8 +-- 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/godot-core/src/builtin/meta/mod.rs b/godot-core/src/builtin/meta/mod.rs index 185989817..05636f718 100644 --- a/godot-core/src/builtin/meta/mod.rs +++ b/godot-core/src/builtin/meta/mod.rs @@ -246,7 +246,7 @@ where /// Rusty abstraction of `sys::GDExtensionPropertyInfo`. /// /// Keeps the actual allocated values (the `sys` equivalent only keeps pointers, which fall out of scope). -#[derive(Debug)] +#[derive(Debug, Clone)] // Note: is not #[non_exhaustive], so adding fields is a breaking change. Mostly used internally at the moment though. pub struct PropertyInfo { pub variant_type: VariantType, @@ -288,7 +288,7 @@ impl PropertyInfo { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct MethodInfo { pub id: i32, pub method_name: StringName, diff --git a/godot-core/src/engine/script_instance.rs b/godot-core/src/engine/script_instance.rs index 6af78402c..51c09d824 100644 --- a/godot-core/src/engine/script_instance.rs +++ b/godot-core/src/engine/script_instance.rs @@ -6,7 +6,9 @@ */ use std::cell::RefCell; +use std::collections::HashMap; use std::ffi::c_void; +use std::sync::Mutex; use crate::builtin::meta::{MethodInfo, PropertyInfo}; use crate::builtin::{GString, StringName, Variant, VariantType}; @@ -78,10 +80,10 @@ pub trait ScriptInstance { fn get_property(&self, name: StringName) -> Option; /// A list of all the properties a script exposes to the engine. - fn get_property_list(&self) -> &[PropertyInfo]; + fn get_property_list(&self) -> Vec; /// A list of all the methods a script exposes to the engine. - fn get_method_list(&self) -> &[MethodInfo]; + fn get_method_list(&self) -> Vec; /// Method invoker for Godot's virtual dispatch system. The engine will call this function when it wants to call a method on the script. /// @@ -150,11 +152,11 @@ impl ScriptInstance for Box { self.as_ref().get_property(name) } - fn get_property_list(&self) -> &[PropertyInfo] { + fn get_property_list(&self) -> Vec { self.as_ref().get_property_list() } - fn get_method_list(&self) -> &[MethodInfo] { + fn get_method_list(&self) -> Vec { self.as_ref().get_method_list() } @@ -219,6 +221,8 @@ type ScriptInstanceInfo = sys::GDExtensionScriptInstanceInfo2; struct ScriptInstanceData { inner: RefCell, script_instance_ptr: *mut ScriptInstanceInfo, + property_list: Mutex>>, + method_list: Mutex>>, } impl Drop for ScriptInstanceData { @@ -290,6 +294,8 @@ pub fn create_script_instance(rs_instance: T) -> *mut c_void let data = ScriptInstanceData { inner: RefCell::new(rs_instance), script_instance_ptr: instance_ptr, + property_list: Default::default(), + method_list: Default::default(), }; let data_ptr = Box::into_raw(Box::new(data)); @@ -364,13 +370,13 @@ mod script_instance_info { /// # Safety /// /// - The returned `*const T` is guaranteed to point to a list that has an equal length and capacity. - fn transfer_ptr_list_to_godot(ptr_list: Vec, list_length: &mut u32) -> *const T { + fn transfer_ptr_list_to_godot(ptr_list: Box<[T]>, list_length: &mut u32) -> *mut T { *list_length = ptr_list.len() as u32; - let ptr = Box::into_raw(ptr_list.into_boxed_slice()); + let ptr = Box::into_raw(ptr_list); // SAFETY: `ptr` was just created in the line above and should be safe to dereference. - unsafe { (*ptr).as_ptr() } + unsafe { (*ptr).as_mut_ptr() } } /// The returned pointer's lifetime is equal to the lifetime of `script` @@ -487,23 +493,32 @@ mod script_instance_info { ) -> *const sys::GDExtensionPropertyInfo { let ctx = || format!("error when calling {}::get_property_list", type_name::()); - let property_list = handle_panic(ctx, || { + let (property_list, property_sys_list) = handle_panic(ctx, || { let instance = instance_data_as_script_instance::(p_instance); - let property_list: Vec<_> = borrow_instance(instance) - .get_property_list() + let property_list = borrow_instance(instance).get_property_list(); + + let property_sys_list: Box<[_]> = property_list .iter() .map(|prop| prop.property_sys()) .collect(); - property_list + (property_list, property_sys_list) }) .unwrap_or_default(); // SAFETY: list_length has to be a valid pointer to a u32. let list_length = unsafe { &mut *r_count }; + let return_pointer = transfer_ptr_list_to_godot(property_sys_list, list_length); + + let instance = instance_data_as_script_instance::(p_instance); + instance + .property_list + .lock() + .expect("Mutex should not be poisoned") + .insert(return_pointer, property_list); - transfer_ptr_list_to_godot(property_list, list_length) + return_pointer } /// # Safety @@ -516,23 +531,33 @@ mod script_instance_info { ) -> *const sys::GDExtensionMethodInfo { let ctx = || format!("error when calling {}::get_method_list", type_name::()); - let method_list = handle_panic(ctx, || { + let (method_list, method_sys_list) = handle_panic(ctx, || { let instance = instance_data_as_script_instance::(p_instance); - let method_list: Vec<_> = borrow_instance(instance) - .get_method_list() + let method_list = borrow_instance(instance).get_method_list(); + + let method_sys_list = method_list .iter() .map(|method| method.method_sys()) .collect(); - method_list + (method_list, method_sys_list) }) .unwrap_or_default(); // SAFETY: list_length has to be a valid pointer to a u32. let list_length = unsafe { &mut *r_count }; + let return_pointer = transfer_ptr_list_to_godot(method_sys_list, list_length); + + let instance = instance_data_as_script_instance::(p_instance); - transfer_ptr_list_to_godot(method_list, list_length) + instance + .method_list + .lock() + .expect("mutex should not be poisoned") + .insert(return_pointer, method_list); + + return_pointer } /// # Safety @@ -561,6 +586,16 @@ mod script_instance_info { // and therefore should have the same length as before. get_propery_list_func // also guarantees that both vector length and capacity are equal. let _drop = transfer_ptr_list_from_godot(p_prop_info, length); + + // now drop the backing data + let instance = instance_data_as_script_instance::(p_instance); + + let _drop = instance + .property_list + .lock() + .expect("mutex should not be poisoned") + .remove(&p_prop_info) + .expect("we can not free a list that has not been allocated"); } /// # Safety diff --git a/itest/rust/src/builtin_tests/script/script_instance_tests.rs b/itest/rust/src/builtin_tests/script/script_instance_tests.rs index 29205b18f..90d4308b4 100644 --- a/itest/rust/src/builtin_tests/script/script_instance_tests.rs +++ b/itest/rust/src/builtin_tests/script/script_instance_tests.rs @@ -116,12 +116,12 @@ impl ScriptInstance for TestScriptInstance { } } - fn get_property_list(&self) -> &[PropertyInfo] { - &self.prop_list + fn get_property_list(&self) -> Vec { + self.prop_list.clone() } - fn get_method_list(&self) -> &[MethodInfo] { - &self.method_list + fn get_method_list(&self) -> Vec { + self.method_list.clone() } fn call(