Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "validate_property" virtual func #1030

Merged
merged 1 commit into from
Feb 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions godot-codegen/src/generator/virtual_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,18 @@ fn make_special_virtual_methods(notification_enum_name: &Ident) -> TokenStream {
unimplemented!()
}

/// Called whenever Godot retrieves value of property. Allows to customize existing properties.
/// Every property info goes through this method, except properties **added** with `get_property_list()`.
///
/// Exposed `property` here is a shared mutable reference obtained (and returned to) from Godot.
///
/// See also in the Godot docs:
/// * [`Object::_validate_property`](https://docs.godotengine.org/en/stable/classes/class_object.html#class-object-private-method-validate-property)
#[cfg(since_api = "4.2")]
fn validate_property(&self, property: &mut crate::meta::PropertyInfo) {
unimplemented!()
}
Comment on lines +129 to +132
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to add docs, see other methods 🙂


/// Called by Godot to tell if a property has a custom revert or not.
///
/// Return `None` for no custom revert, and return `Some(value)` to specify the custom revert.
Expand Down
11 changes: 11 additions & 0 deletions godot-core/src/builtin/string/gstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,17 @@ impl GString {
*boxed
}

/// Convert a `GString` sys pointer to a mutable reference with unbounded lifetime.
///
/// # Safety
///
/// - `ptr` must point to a live `GString` for the duration of `'a`.
/// - Must be exclusive - no other reference to given `GString` instance can exist for the duration of `'a`.
pub(crate) unsafe fn borrow_string_sys_mut<'a>(ptr: sys::GDExtensionStringPtr) -> &'a mut Self {
sys::static_assert_eq_size_align!(StringName, sys::types::OpaqueString);
&mut *(ptr.cast::<GString>())
}

/// Moves this string into a string sys pointer. This is the same as using [`GodotFfi::move_return_ptr`].
///
/// # Safety
Expand Down
13 changes: 13 additions & 0 deletions godot-core/src/builtin/string/string_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,19 @@ impl StringName {
&*(ptr.cast::<StringName>())
}

/// Convert a `StringName` sys pointer to a mutable reference with unbounded lifetime.
///
/// # Safety
///
/// - `ptr` must point to a live `StringName` for the duration of `'a`.
/// - Must be exclusive - no other reference to given `StringName` instance can exist for the duration of `'a`.
pub(crate) unsafe fn borrow_string_sys_mut<'a>(
ptr: sys::GDExtensionStringNamePtr,
) -> &'a mut StringName {
sys::static_assert_eq_size_align!(StringName, sys::types::OpaqueStringName);
&mut *(ptr.cast::<StringName>())
}

#[doc(hidden)]
pub fn as_inner(&self) -> inner::InnerStringName {
inner::InnerStringName::from_outer(self)
Expand Down
47 changes: 47 additions & 0 deletions godot-core/src/meta/property_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::global::{PropertyHint, PropertyUsageFlags};
use crate::meta::{
element_godot_type_name, ArrayElement, ClassName, GodotType, PackedArrayElement,
};
use crate::obj::{EngineBitfield, EngineEnum};
use crate::registry::property::{Export, Var};
use crate::sys;
use godot_ffi::VariantType;
Expand Down Expand Up @@ -194,6 +195,52 @@ impl PropertyInfo {
let _hint_string = GString::from_owned_string_sys(info.hint_string);
}
}

/// Moves its values into given `GDExtensionPropertyInfo`, dropping previous values if necessary.
///
/// # Safety
///
/// * `property_info_ptr` must be valid.
///
pub(crate) unsafe fn move_into_property_info_ptr(
self,
property_info_ptr: *mut sys::GDExtensionPropertyInfo,
) {
let ptr = &mut *property_info_ptr;

ptr.usage = u32::try_from(self.usage.ord()).expect("usage.ord()");
ptr.hint = u32::try_from(self.hint_info.hint.ord()).expect("hint.ord()");
ptr.type_ = self.variant_type.sys();

*StringName::borrow_string_sys_mut(ptr.name) = self.property_name;
*GString::borrow_string_sys_mut(ptr.hint_string) = self.hint_info.hint_string;

if self.class_name != ClassName::none() {
*StringName::borrow_string_sys_mut(ptr.class_name) = self.class_name.to_string_name();
}
}

/// Creates copy of given `sys::GDExtensionPropertyInfo`.
///
/// # Safety
///
/// * `property_info_ptr` must be valid.
pub(crate) unsafe fn new_from_sys(
property_info_ptr: *mut sys::GDExtensionPropertyInfo,
) -> Self {
let ptr = *property_info_ptr;

Self {
variant_type: VariantType::from_sys(ptr.type_),
class_name: ClassName::none(),
property_name: StringName::new_from_string_sys(ptr.name),
hint_info: PropertyHintInfo {
hint: PropertyHint::from_ord(ptr.hint.to_owned() as i32),
hint_string: GString::new_from_string_sys(ptr.hint_string),
},
usage: PropertyUsageFlags::from_ord(ptr.usage as u64),
}
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
Expand Down
8 changes: 8 additions & 0 deletions godot-core/src/obj/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ where
pub mod cap {
use super::*;
use crate::builtin::{StringName, Variant};
use crate::meta::PropertyInfo;
use crate::obj::{Base, Bounds, Gd};
use std::any::Any;

Expand Down Expand Up @@ -571,6 +572,13 @@ pub mod cap {
fn __godot_property_get_revert(&self, property: StringName) -> Option<Variant>;
}

#[doc(hidden)]
#[cfg(since_api = "4.2")]
pub trait GodotValidateProperty: GodotClass {
#[doc(hidden)]
fn __godot_validate_property(&self, property: &mut PropertyInfo);
}

/// Auto-implemented for `#[godot_api] impl MyClass` blocks
pub trait ImplementsGodotApi: GodotClass {
#[doc(hidden)]
Expand Down
30 changes: 30 additions & 0 deletions godot-core/src/registry/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use crate::builder::ClassBuilder;
use crate::builtin::{StringName, Variant};
use crate::classes::Object;
use crate::meta::PropertyInfo;
use crate::obj::{bounds, cap, AsDyn, Base, Bounds, Gd, GodotClass, Inherits, UserClass};
use crate::registry::plugin::ErasedDynGd;
use crate::storage::{as_storage, InstanceStorage, Storage, StorageRefCounted};
Expand Down Expand Up @@ -357,6 +358,35 @@ pub unsafe extern "C" fn property_get_revert<T: cap::GodotPropertyGetRevert>(

sys::conv::SYS_TRUE
}

/// Callback for `validate_property`.
///
/// Exposes `PropertyInfo` created out of `*mut GDExtensionPropertyInfo` ptr to user and moves edited values back to the pointer.
///
/// # Safety
///
/// - Must only be called by Godot as a callback for `validate_property` for a rust-defined class of type `T`.
/// - `property_info_ptr` must be valid for the whole duration of this function call (i.e. - can't be freed nor consumed).
///
#[deny(unsafe_op_in_unsafe_fn)]
#[cfg(since_api = "4.2")]
pub unsafe extern "C" fn validate_property<T: cap::GodotValidateProperty>(
instance: sys::GDExtensionClassInstancePtr,
property_info_ptr: *mut sys::GDExtensionPropertyInfo,
) -> sys::GDExtensionBool {
// SAFETY: `instance` is a valid `T` instance pointer for the duration of this function call.
let storage = unsafe { as_storage::<T>(instance) };
let instance = storage.get();

// SAFETY: property_info_ptr must be valid.
let mut property_info = unsafe { PropertyInfo::new_from_sys(property_info_ptr) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safety statement? 🙂

T::__godot_validate_property(&*instance, &mut property_info);

// SAFETY: property_info_ptr remains valid & unchanged.
unsafe { property_info.move_into_property_info_ptr(property_info_ptr) };

sys::conv::SYS_TRUE
}
// ----------------------------------------------------------------------------------------------------------------------------------------------
// Safe, higher-level methods

Expand Down
6 changes: 6 additions & 0 deletions godot-core/src/registry/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,8 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
user_property_get_revert_fn,
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
virtual_method_docs: _,
#[cfg(since_api = "4.2")]
validate_property_fn,
}) => {
c.user_register_fn = user_register_fn;

Expand All @@ -477,6 +479,10 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
c.godot_params.property_can_revert_func = user_property_can_revert_fn;
c.godot_params.property_get_revert_func = user_property_get_revert_fn;
c.user_virtual_fn = get_virtual_fn;
#[cfg(since_api = "4.2")]
{
c.godot_params.validate_property_func = validate_property_fn;
}
}
PluginItem::DynTraitImpl(dyn_trait_impl) => {
let type_id = dyn_trait_impl.dyn_trait_typeid();
Expand Down
16 changes: 16 additions & 0 deletions godot-core/src/registry/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,13 @@ pub struct ITraitImpl {
r_ret: sys::GDExtensionVariantPtr,
) -> sys::GDExtensionBool,
>,
#[cfg(since_api = "4.2")]
pub(crate) validate_property_fn: Option<
unsafe extern "C" fn(
p_instance: sys::GDExtensionClassInstancePtr,
p_property: *mut sys::GDExtensionPropertyInfo,
) -> sys::GDExtensionBool,
>,
}

impl ITraitImpl {
Expand Down Expand Up @@ -485,6 +492,15 @@ impl ITraitImpl {
);
self
}

#[cfg(since_api = "4.2")]
pub fn with_validate_property<T: GodotClass + cap::GodotValidateProperty>(mut self) -> Self {
set(
&mut self.validate_property_fn,
callbacks::validate_property::<T>,
);
self
}
}

/// Representation of a `#[godot_dyn]` invocation.
Expand Down
20 changes: 20 additions & 0 deletions godot-macros/src/class/data_models/interface_trait_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub fn transform_trait_impl(original_impl: venial::Impl) -> ParseResult<TokenStr
let mut set_property_impl = TokenStream::new();
let mut get_property_list_impl = TokenStream::new();
let mut property_get_revert_impl = TokenStream::new();
let mut validate_property_impl = TokenStream::new();

let mut modifiers = Vec::new();

Expand Down Expand Up @@ -207,6 +208,24 @@ pub fn transform_trait_impl(original_impl: venial::Impl) -> ParseResult<TokenStr
modifiers.push((cfg_attrs, ident("with_property_get_revert")));
}

#[cfg(since_api = "4.2")]
"validate_property" => {
let inactive_class_early_return = make_inactive_class_check(TokenStream::new());
validate_property_impl = quote! {
#(#cfg_attrs)*
impl ::godot::obj::cap::GodotValidateProperty for #class_name {
fn __godot_validate_property(&self, property: &mut ::godot::meta::PropertyInfo) {
use ::godot::obj::UserClass as _;

#inactive_class_early_return

<Self as #trait_path>::validate_property(self, property);
}
}
};
modifiers.push((cfg_attrs, ident("with_validate_property")));
}

// Other virtual methods, like ready, process etc.
method_name_str => {
#[cfg(since_api = "4.4")]
Expand Down Expand Up @@ -317,6 +336,7 @@ pub fn transform_trait_impl(original_impl: venial::Impl) -> ParseResult<TokenStr
#set_property_impl
#get_property_list_impl
#property_get_revert_impl
#validate_property_impl

impl ::godot::private::You_forgot_the_attribute__godot_api for #class_name {}

Expand Down
3 changes: 3 additions & 0 deletions itest/rust/src/object_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ mod property_template_test;
mod property_test;
mod reentrant_test;
mod singleton_test;
// `validate_property` is only supported in Godot 4.2+.
#[cfg(since_api = "4.2")]
mod validate_property_test;
mod virtual_methods_test;

// Need to test this in the init level method.
Expand Down
2 changes: 1 addition & 1 deletion itest/rust/src/object_tests/object_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use godot::sys::{self, interface_fn, GodotFfi};
use crate::framework::{expect_panic, itest, TestContext};

// TODO:
// * make sure that ptrcalls are used when possible (ie. when type info available; maybe GDScript integration test)
// * make sure that ptrcalls are used when possible (i.e. when type info available; maybe GDScript integration test)
// * Deref impl for user-defined types

#[itest]
Expand Down
77 changes: 77 additions & 0 deletions itest/rust/src/object_tests/validate_property_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright (c) godot-rust; Bromeon and contributors.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

use godot::builtin::{Array, Dictionary, GString, StringName};
use godot::classes::IObject;
use godot::global::{PropertyHint, PropertyUsageFlags};
use godot::meta::PropertyInfo;
use godot::obj::NewAlloc;
use godot::register::{godot_api, GodotClass};
use godot::test::itest;

#[derive(GodotClass)]
#[class(base = Object, init)]
pub struct ValidatePropertyTest {
#[var(hint = NONE, hint_string = "initial")]
#[export]
my_var: i64,
}

#[godot_api]
impl IObject for ValidatePropertyTest {
fn validate_property(&self, property: &mut PropertyInfo) {
if property.property_name.to_string() == "my_var" {
property.usage = PropertyUsageFlags::NO_EDITOR;
property.property_name = StringName::from("SuperNewTestPropertyName");
property.hint_info.hint_string = GString::from("SomePropertyHint");
property.hint_info.hint = PropertyHint::TYPE_STRING;

// Makes no sense, but allows to check if given ClassName can be properly moved to GDExtensionPropertyInfo.
property.class_name = <ValidatePropertyTest as godot::obj::GodotClass>::class_name();
}
}
}

#[itest]
fn validate_property_test() {
let obj = ValidatePropertyTest::new_alloc();
let properties: Array<Dictionary> = obj.get_property_list();

let property = properties
.iter_shared()
.find(|dict| {
dict.get("name")
.is_some_and(|v| v.to_string() == "SuperNewTestPropertyName")
})
.expect("Test failed – unable to find validated property.");

let hint_string = property
.get("hint_string")
.expect("validated property dict should contain a `hint_string` entry.")
.to::<GString>();
assert_eq!(hint_string, GString::from("SomePropertyHint"));

let class = property
.get("class_name")
.expect("Validated property dict should contain a class_name entry.")
.to::<StringName>();
assert_eq!(class, StringName::from("ValidatePropertyTest"));

let usage = property
.get("usage")
.expect("Validated property dict should contain an usage entry.")
.to::<PropertyUsageFlags>();
assert_eq!(usage, PropertyUsageFlags::NO_EDITOR);

let hint = property
.get("hint")
.expect("Validated property dict should contain a hint entry.")
.to::<PropertyHint>();
assert_eq!(hint, PropertyHint::TYPE_STRING);

obj.free();
}
Loading