Skip to content

Commit 2d043d1

Browse files
committed
Add "validate_property" virtual func
- Create internal API for easier handling of ``*mut GDExtensionPropertyInfo` - functions that allow to easily handle conversions from/to PropertyInfo and `borrow_string_sys_mut` for GString and StringName. - Properly drop refcounted values in exposed Godot's PropertyInfo while assigning new values to given memory locations.
1 parent 15eabea commit 2d043d1

File tree

7 files changed

+87
-65
lines changed

7 files changed

+87
-65
lines changed

godot-core/src/builtin/string/gstring.rs

+10
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,16 @@ impl GString {
148148
*boxed
149149
}
150150

151+
/// Convert a `GString` sys pointer to a mutable reference with unbounded lifetime.
152+
///
153+
/// # Safety
154+
///
155+
/// `ptr` must point to a live `GString` for the duration of `'a`.
156+
pub(crate) unsafe fn borrow_string_sys_mut<'a>(ptr: sys::GDExtensionStringPtr) -> &'a mut Self {
157+
sys::static_assert_eq_size_align!(StringName, sys::types::OpaqueString);
158+
&mut *(ptr.cast::<GString>())
159+
}
160+
151161
/// Moves this string into a string sys pointer. This is the same as using [`GodotFfi::move_return_ptr`].
152162
///
153163
/// # Safety

godot-core/src/builtin/string/string_name.rs

+13-12
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ pub struct StringName {
5656
}
5757

5858
impl StringName {
59-
fn from_opaque(opaque: sys::types::OpaqueStringName) -> Self {
59+
pub(crate) fn from_opaque(opaque: sys::types::OpaqueStringName) -> Self {
6060
Self { opaque }
6161
}
6262

@@ -143,17 +143,6 @@ impl StringName {
143143
*boxed
144144
}
145145

146-
/// Moves this string into a string name sys pointer. This is the same as using [`GodotFfi::move_return_ptr`].
147-
///
148-
/// # Safety
149-
///
150-
/// `dst` must be a valid string name pointer.
151-
pub(crate) unsafe fn move_into_string_ptr(self, dst: sys::GDExtensionStringNamePtr) {
152-
let dst: sys::GDExtensionTypePtr = dst.cast();
153-
154-
self.move_return_ptr(dst, sys::PtrcallType::Standard);
155-
}
156-
157146
/// Convert a `StringName` sys pointer to a reference with unbounded lifetime.
158147
///
159148
/// # Safety
@@ -166,6 +155,18 @@ impl StringName {
166155
&*(ptr.cast::<StringName>())
167156
}
168157

158+
/// Convert a `StringName` sys pointer to a mutable reference with unbounded lifetime.
159+
///
160+
/// # Safety
161+
///
162+
/// * `ptr` must point to a live `StringName` for the duration of `'a`.
163+
pub(crate) unsafe fn borrow_string_sys_mut<'a>(
164+
ptr: sys::GDExtensionStringNamePtr,
165+
) -> &'a mut StringName {
166+
sys::static_assert_eq_size_align!(StringName, sys::types::OpaqueStringName);
167+
&mut *(ptr.cast::<StringName>())
168+
}
169+
169170
#[doc(hidden)]
170171
pub fn as_inner(&self) -> inner::InnerStringName {
171172
inner::InnerStringName::from_outer(self)

godot-core/src/meta/property_info.rs

+30-35
Original file line numberDiff line numberDiff line change
@@ -180,28 +180,42 @@ impl PropertyInfo {
180180
}
181181
}
182182

183-
/// Consumes self, moving the values into `sys::GDExtensionPropertyInfo` pointer.
183+
/// Properly frees a `sys::GDExtensionPropertyInfo` created by [`into_owned_property_sys`](Self::into_owned_property_sys).
184+
///
185+
/// # Safety
186+
///
187+
/// * Must only be used on a struct returned from a call to `into_owned_property_sys`, without modification.
188+
/// * Must not be called more than once on a `sys::GDExtensionPropertyInfo` struct.
189+
pub(crate) unsafe fn free_owned_property_sys(info: sys::GDExtensionPropertyInfo) {
190+
// SAFETY: This function was called on a pointer returned from `into_owned_property_sys`, thus both `info.name` and
191+
// `info.hint_string` were created from calls to `into_owned_string_sys` on their respective types.
192+
// Additionally, this function isn't called more than once on a struct containing the same `name` or `hint_string` pointers.
193+
unsafe {
194+
let _name = StringName::from_owned_string_sys(info.name);
195+
let _hint_string = GString::from_owned_string_sys(info.hint_string);
196+
}
197+
}
198+
/// Moves its values into given `GDExtensionPropertyInfo`, dropping previous values if necessary.
184199
///
185200
/// # Safety
186201
///
187202
/// * `property_info_ptr` must be valid.
203+
///
188204
pub(crate) unsafe fn move_into_property_info_ptr(
189205
self,
190206
property_info_ptr: *mut sys::GDExtensionPropertyInfo,
191207
) {
192-
(*property_info_ptr).type_ = self.variant_type.sys();
208+
(*property_info_ptr).usage = u32::try_from(self.usage.ord()).expect("usage.ord()");
193209
(*property_info_ptr).hint = u32::try_from(self.hint_info.hint.ord()).expect("hint.ord()");
210+
(*property_info_ptr).type_ = self.variant_type.sys();
194211

195-
self.hint_info
196-
.hint_string
197-
.move_into_string_ptr((*property_info_ptr).hint_string);
198-
self.property_name
199-
.move_into_string_ptr((*property_info_ptr).name);
212+
let ptr = *property_info_ptr;
200213

201-
(*property_info_ptr).usage = u32::try_from(self.usage.ord()).expect("usage.ord()");
214+
*StringName::borrow_string_sys_mut(ptr.name) = self.property_name;
215+
*GString::borrow_string_sys_mut(ptr.hint_string) = self.hint_info.hint_string;
202216

203217
if self.class_name != ClassName::none() {
204-
(*property_info_ptr).class_name = sys::SysPtr::force_mut(self.class_name.string_sys());
218+
*StringName::borrow_string_sys_mut(ptr.class_name) = self.class_name.to_string_name();
205219
}
206220
}
207221

@@ -213,34 +227,15 @@ impl PropertyInfo {
213227
pub(crate) unsafe fn new_from_sys(
214228
property_info_ptr: *mut sys::GDExtensionPropertyInfo,
215229
) -> Self {
216-
let variant_type = VariantType::from_sys((*property_info_ptr).type_);
217-
let property_name = StringName::new_from_string_sys((*property_info_ptr).name);
218-
let hint_string = GString::new_from_string_sys((*property_info_ptr).hint_string);
219-
let hint = PropertyHint::from_ord((*property_info_ptr).hint.to_owned() as i32);
220-
let usage = PropertyUsageFlags::from_ord((*property_info_ptr).usage as u64);
221-
222230
Self {
223-
variant_type,
231+
variant_type: VariantType::from_sys((*property_info_ptr).type_),
224232
class_name: ClassName::none(),
225-
property_name,
226-
hint_info: PropertyHintInfo { hint, hint_string },
227-
usage,
228-
}
229-
}
230-
231-
/// Properly frees a `sys::GDExtensionPropertyInfo` created by [`into_owned_property_sys`](Self::into_owned_property_sys).
232-
///
233-
/// # Safety
234-
///
235-
/// * Must only be used on a struct returned from a call to `into_owned_property_sys`, without modification.
236-
/// * Must not be called more than once on a `sys::GDExtensionPropertyInfo` struct.
237-
pub(crate) unsafe fn free_owned_property_sys(info: sys::GDExtensionPropertyInfo) {
238-
// SAFETY: This function was called on a pointer returned from `into_owned_property_sys`, thus both `info.name` and
239-
// `info.hint_string` were created from calls to `into_owned_string_sys` on their respective types.
240-
// Additionally, this function isn't called more than once on a struct containing the same `name` or `hint_string` pointers.
241-
unsafe {
242-
let _name = StringName::from_owned_string_sys(info.name);
243-
let _hint_string = GString::from_owned_string_sys(info.hint_string);
233+
property_name: StringName::new_from_string_sys((*property_info_ptr).name),
234+
hint_info: PropertyHintInfo {
235+
hint: PropertyHint::from_ord((*property_info_ptr).hint.to_owned() as i32),
236+
hint_string: GString::new_from_string_sys((*property_info_ptr).hint_string),
237+
},
238+
usage: PropertyUsageFlags::from_ord((*property_info_ptr).usage as u64),
244239
}
245240
}
246241
}

godot-core/src/registry/callbacks.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -359,10 +359,15 @@ pub unsafe extern "C" fn property_get_revert<T: cap::GodotPropertyGetRevert>(
359359
sys::conv::SYS_TRUE
360360
}
361361

362+
/// Callback for `validate_property`.
363+
///
364+
/// Exposes `PropertyInfo` created out of `*mut GDExtensionPropertyInfo` ptr to user and moves edited values back to the pointer.
365+
///
362366
/// # Safety
363367
///
364368
/// - Must only be called by Godot as a callback for `validate_property` for a rust-defined class of type `T`.
365369
/// - `sys_property_info` must be valid for the whole duration of this function call (ie - can't be freed nor consumed).
370+
///
366371
#[deny(unsafe_op_in_unsafe_fn)]
367372
#[cfg(since_api = "4.2")]
368373
pub unsafe extern "C" fn validate_property<T: cap::GodotValidateProperty>(
@@ -375,7 +380,8 @@ pub unsafe extern "C" fn validate_property<T: cap::GodotValidateProperty>(
375380

376381
let mut property_info = unsafe { PropertyInfo::new_from_sys(property_info_ptr) };
377382
T::__godot_validate_property(&*instance, &mut property_info);
378-
// SAFETY: property_info_ptr is still valid
383+
384+
// SAFETY: property_info_ptr remains valid & unchanged.
379385
unsafe { property_info.move_into_property_info_ptr(property_info_ptr) };
380386

381387
sys::conv::SYS_TRUE

godot-core/src/registry/class.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
479479
c.godot_params.property_can_revert_func = user_property_can_revert_fn;
480480
c.godot_params.property_get_revert_func = user_property_get_revert_fn;
481481
c.user_virtual_fn = get_virtual_fn;
482-
// Not present in GdExtension
482+
// Not present in GdExtension.
483483
#[cfg(since_api = "4.2")]
484484
{
485485
c.godot_params.validate_property_func = validate_property_fn;

itest/rust/src/object_tests/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ mod property_template_test;
2323
mod property_test;
2424
mod reentrant_test;
2525
mod singleton_test;
26-
// `validate_property` is only supported in Godot 4.2+
26+
// `validate_property` is only supported in Godot 4.2+.
2727
#[cfg(since_api = "4.2")]
2828
mod validate_property_test;
2929
mod virtual_methods_test;

itest/rust/src/object_tests/validate_property_test.rs

+25-15
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ use godot::test::itest;
1616
#[derive(GodotClass)]
1717
#[class(base = Object, init)]
1818
pub struct ValidatePropertyTest {
19+
#[var(
20+
hint = NONE,
21+
hint_string = "initial"
22+
)]
1923
#[export]
2024
my_var: i64,
2125
}
@@ -28,7 +32,7 @@ impl IObject for ValidatePropertyTest {
2832
property.property_name = StringName::from("SuperNewTestPropertyName");
2933
property.hint_info.hint_string = GString::from("SomePropertyHint");
3034

31-
// Makes no sense but allows to check if given ClassName can be properly moved to GDExtensionPropertyInfo
35+
// Makes no sense but allows to check if given ClassName can be properly moved to GDExtensionPropertyInfo.
3236
property.class_name = <ValidatePropertyTest as godot::obj::GodotClass>::class_name();
3337
}
3438
}
@@ -40,24 +44,30 @@ fn validate_property_test() {
4044
let properties: Array<Dictionary> = obj.get_property_list();
4145

4246
for property in properties.iter_shared() {
43-
if property
47+
if !property
4448
.get("name")
45-
.map(|v| v.to_string() == "SuperNewTestPropertyName")
46-
.unwrap_or(false)
49+
.is_some_and(|v| v.to_string() == "SuperNewTestPropertyName")
4750
{
48-
let Some(usage) = property.get("usage").map(|v| v.to::<PropertyUsageFlags>()) else {
49-
continue;
50-
};
51-
assert_eq!(usage, PropertyUsageFlags::NO_EDITOR);
51+
continue;
52+
}
5253

53-
let Some(usage) = property.get("hint_string").map(|v| v.to::<GString>()) else {
54-
continue;
55-
};
56-
assert_eq!(usage, GString::from("SomePropertyHint"));
54+
let Some(hint_string) = property.get("hint_string").map(|v| v.to::<GString>()) else {
55+
continue;
56+
};
57+
assert_eq!(hint_string, GString::from("SomePropertyHint"));
5758

58-
obj.free();
59-
return;
60-
}
59+
let Some(class) = property.get("class_name").map(|v| v.to::<StringName>()) else {
60+
continue;
61+
};
62+
assert_eq!(class, StringName::from("ValidatePropertyTest"));
63+
64+
let Some(usage) = property.get("usage").map(|v| v.to::<PropertyUsageFlags>()) else {
65+
continue;
66+
};
67+
assert_eq!(usage, PropertyUsageFlags::NO_EDITOR);
68+
69+
obj.free();
70+
return;
6171
}
6272

6373
obj.free();

0 commit comments

Comments
 (0)