From f5eff0d634e975a7475c147101eb426ba47267d4 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 9 Nov 2025 23:23:02 +0100 Subject: [PATCH] Add `GodotImmutable` to constrain `#[opt(default)]` types --- godot-core/src/meta/mod.rs | 2 +- godot-core/src/meta/traits.rs | 85 ++++++++++++++++++++++ godot-core/src/private.rs | 19 +++++ godot-macros/src/class/data_models/func.rs | 4 +- itest/rust/src/register_tests/func_test.rs | 32 +++++++- 5 files changed, 137 insertions(+), 5 deletions(-) diff --git a/godot-core/src/meta/mod.rs b/godot-core/src/meta/mod.rs index 73aeb751e..f2178308a 100644 --- a/godot-core/src/meta/mod.rs +++ b/godot-core/src/meta/mod.rs @@ -76,7 +76,7 @@ pub use signature::trace; #[doc(hidden)] pub use signature::*; pub use signed_range::{wrapped, SignedRange}; -pub use traits::{ArrayElement, GodotType, PackedArrayElement}; +pub use traits::{ArrayElement, GodotImmutable, GodotType, PackedArrayElement}; pub use uniform_object_deref::UniformObjectDeref; // Public due to signals emit() needing it. Should be made pub(crate) again if that changes. diff --git a/godot-core/src/meta/traits.rs b/godot-core/src/meta/traits.rs index bc11cadd1..a810f3397 100644 --- a/godot-core/src/meta/traits.rs +++ b/godot-core/src/meta/traits.rs @@ -236,3 +236,88 @@ pub(crate) fn element_godot_type_name() -> String { // pub(crate) fn element_godot_type_name() -> String { // ::godot_type_name() // } + +// ---------------------------------------------------------------------------------------------------------------------------------------------- + +/// Implemented for types that can be used as immutable default parameters in `#[func]` methods. +/// +/// This trait ensures that default parameter values cannot be mutated by callers, preventing the Python "mutable default argument" problem +/// where a single default value is shared across multiple calls. +/// +/// Post-processes the default value in some cases, e.g. makes `Array` read-only via `into_read_only()`. +/// +/// At the moment, this trait is conservatively implemented for types where immutability can be statically guaranteed. +/// Depending on usage, the API might be expanded in the future to allow defaults whose immutability is only determined +/// at runtime (e.g. untyped arrays/dictionaries where all element types are immutable). +/// +/// # Safety +/// Allows to use the implementors in a limited `Sync` context. Implementing this trait asserts that `Self` is either: +/// - `Copy`, i.e. each instance is truly independent. +/// - Thread-safe in the sense that `clone()` is thread-safe. Individual clones must not offer a way to mutate the value or cause race conditions. +#[diagnostic::on_unimplemented( + message = "#[opt(default = ...)] only supports a set of truly immutable types", + label = "this type is not immutable and thus not eligible for a default value" +)] +pub unsafe trait GodotImmutable: GodotConvert + Sized { + fn into_runtime_immutable(self) -> Self { + self + } +} + +mod godot_immutable_impls { + use super::GodotImmutable; + use crate::builtin::*; + use crate::meta::ArrayElement; + + unsafe impl GodotImmutable for bool {} + unsafe impl GodotImmutable for i8 {} + unsafe impl GodotImmutable for u8 {} + unsafe impl GodotImmutable for i16 {} + unsafe impl GodotImmutable for u16 {} + unsafe impl GodotImmutable for i32 {} + unsafe impl GodotImmutable for u32 {} + unsafe impl GodotImmutable for i64 {} + unsafe impl GodotImmutable for f32 {} + unsafe impl GodotImmutable for f64 {} + + // No NodePath, Callable, Signal, Rid, Variant. + unsafe impl GodotImmutable for Aabb {} + unsafe impl GodotImmutable for Basis {} + unsafe impl GodotImmutable for Color {} + unsafe impl GodotImmutable for GString {} + unsafe impl GodotImmutable for Plane {} + unsafe impl GodotImmutable for Projection {} + unsafe impl GodotImmutable for Quaternion {} + unsafe impl GodotImmutable for Rect2 {} + unsafe impl GodotImmutable for Rect2i {} + unsafe impl GodotImmutable for StringName {} + unsafe impl GodotImmutable for Transform2D {} + unsafe impl GodotImmutable for Transform3D {} + unsafe impl GodotImmutable for Vector2 {} + unsafe impl GodotImmutable for Vector2i {} + unsafe impl GodotImmutable for Vector3 {} + unsafe impl GodotImmutable for Vector3i {} + unsafe impl GodotImmutable for Vector4 {} + unsafe impl GodotImmutable for Vector4i {} + + unsafe impl GodotImmutable for PackedByteArray {} + unsafe impl GodotImmutable for PackedColorArray {} + unsafe impl GodotImmutable for PackedFloat32Array {} + unsafe impl GodotImmutable for PackedFloat64Array {} + unsafe impl GodotImmutable for PackedInt32Array {} + unsafe impl GodotImmutable for PackedInt64Array {} + unsafe impl GodotImmutable for PackedStringArray {} + unsafe impl GodotImmutable for PackedVector2Array {} + unsafe impl GodotImmutable for PackedVector3Array {} + #[cfg(since_api = "4.3")] + unsafe impl GodotImmutable for PackedVector4Array {} + + unsafe impl GodotImmutable for Array + where + T: GodotImmutable + ArrayElement, + { + fn into_runtime_immutable(self) -> Self { + self.into_read_only() + } + } +} diff --git a/godot-core/src/private.rs b/godot-core/src/private.rs index 4cc225061..9c4c94f03 100644 --- a/godot-core/src/private.rs +++ b/godot-core/src/private.rs @@ -238,6 +238,25 @@ pub fn is_class_runtime(is_tool: bool) -> bool { global_config.tool_only_in_editor } +/// Converts a default parameter value to a runtime-immutable `Variant`. +/// +/// This function is used internally by the `#[opt(default)]` attribute to: +/// 1. Convert the value using `AsArg` trait for argument conversions (e.g. `"str"` for `AsArg`). +/// 2. Apply immutability transformation. +/// 3. Convert to `Variant` for Godot's storage. +pub fn opt_default_value(value: impl crate::meta::AsArg) -> crate::builtin::Variant +where + T: crate::meta::GodotImmutable + crate::meta::ToGodot + Clone, +{ + // We currently need cow_into_owned() to create an owned value for the immutability transform. This may be revisited once `#[opt]` + // supports more types (e.g. `Gd`, where `cow_into_owned()` would increment ref-counts). + + let value = crate::meta::AsArg::::into_arg(value); + let value = value.cow_into_owned(); + let value = ::into_runtime_immutable(value); + crate::builtin::Variant::from(value) +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Panic *hook* management diff --git a/godot-macros/src/class/data_models/func.rs b/godot-macros/src/class/data_models/func.rs index 60e0d38ff..c744045ba 100644 --- a/godot-macros/src/class/data_models/func.rs +++ b/godot-macros/src/class/data_models/func.rs @@ -217,9 +217,7 @@ fn make_default_argument_vec( .zip(optional_param_types) .map(|(value, param_type)| { quote! { - ::godot::builtin::Variant::from( - ::godot::meta::AsArg::<#param_type>::into_arg(#value) - ) + ::godot::private::opt_default_value::<#param_type>(#value) } }); diff --git a/itest/rust/src/register_tests/func_test.rs b/itest/rust/src/register_tests/func_test.rs index eed5592f0..9cf7bc910 100644 --- a/itest/rust/src/register_tests/func_test.rs +++ b/itest/rust/src/register_tests/func_test.rs @@ -66,6 +66,15 @@ impl FuncObj { varray![required, string, integer] } + #[func] + fn method_with_immutable_array_default( + &self, + #[opt(default = &array![1, 2, 3])] arr: Array, + ) -> Array { + arr + } + + /* For now, Gd types cannot be used as default parameters due to immutability requirement. #[func] fn static_with_defaults( #[opt(default = &RefCounted::new_gd())] mut required: Gd, @@ -79,6 +88,7 @@ impl FuncObj { required.set_meta("nullable_id", &id.to_variant()); required } + */ } impl FuncObj { @@ -345,6 +355,7 @@ fn func_default_parameters() { let c = obj.call("method_with_defaults", vslice![2, "Another string", 456]); assert_eq!(c.to::(), varray![2, "Another string", 456]); + /* For now, Gd defaults are disabled due to immutability. // Test that object is passed through, and that Option with default Gd::null_arg() works. let first = RefCounted::new_gd(); let d = obj @@ -360,8 +371,10 @@ fn func_default_parameters() { .to::>(); assert_eq!(e.instance_id(), first.instance_id()); assert_eq!(e.get_meta("nullable_id"), second.instance_id().to_variant()); + */ } +/* For now, Gd defaults are disabled due to immutability. #[itest] fn func_defaults_re_evaluate_expr() { // ClassDb::class_call_static() added in Godot 4.4, but non-static dispatch works even before. @@ -386,8 +399,23 @@ fn func_defaults_re_evaluate_expr() { "#[opt = EXPR] should create evaluate EXPR on each call" ); } +*/ -// No test for Gd::from_object(), as that simply moves the existing object without running user code. +#[itest] +fn func_immutable_defaults() { + let mut obj = FuncObj::new_gd(); + + // Test Array default parameter. + let arr = obj + .call("method_with_immutable_array_default", &[]) + .to::>(); + assert_eq!(arr, array![1, 2, 3]); + + assert!( + arr.is_read_only(), + "GodotImmutable trait did its job to make array read-only" + ); +} #[itest] fn cfg_doesnt_interfere_with_valid_method_impls() { @@ -431,6 +459,8 @@ fn cfg_removes_or_keeps_signals() { assert!(!class_has_signal::("cfg_removes_signal")); } +// No test for Gd::from_object(), as that simply moves the existing object without running user code. + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Helpers