Skip to content
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
2 changes: 1 addition & 1 deletion godot-core/src/meta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
85 changes: 85 additions & 0 deletions godot-core/src/meta/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,88 @@ pub(crate) fn element_godot_type_name<T: ArrayElement>() -> String {
// pub(crate) fn element_godot_type_name<T: ArrayElement>() -> String {
// <T::Via as GodotType>::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<T>` 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 {}
Copy link
Contributor

Choose a reason for hiding this comment

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

bool?

Copy link
Member Author

Choose a reason for hiding this comment

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

Also forgot f32/f64. Thanks!

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<T> GodotImmutable for Array<T>
where
T: GodotImmutable + ArrayElement,
{
fn into_runtime_immutable(self) -> Self {
self.into_read_only()
}
}
}
19 changes: 19 additions & 0 deletions godot-core/src/private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<GString>`).
/// 2. Apply immutability transformation.
/// 3. Convert to `Variant` for Godot's storage.
pub fn opt_default_value<T>(value: impl crate::meta::AsArg<T>) -> 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<RefCounted>`, where `cow_into_owned()` would increment ref-counts).

let value = crate::meta::AsArg::<T>::into_arg(value);
let value = value.cow_into_owned();
let value = <T as crate::meta::GodotImmutable>::into_runtime_immutable(value);
crate::builtin::Variant::from(value)
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Panic *hook* management

Expand Down
4 changes: 1 addition & 3 deletions godot-macros/src/class/data_models/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
});

Expand Down
32 changes: 31 additions & 1 deletion itest/rust/src/register_tests/func_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<i64>,
) -> Array<i64> {
arr
}

/* For now, Gd<T> types cannot be used as default parameters due to immutability requirement.
#[func]
fn static_with_defaults(
#[opt(default = &RefCounted::new_gd())] mut required: Gd<RefCounted>,
Expand All @@ -79,6 +88,7 @@ impl FuncObj {
required.set_meta("nullable_id", &id.to_variant());
required
}
*/
}

impl FuncObj {
Expand Down Expand Up @@ -345,6 +355,7 @@ fn func_default_parameters() {
let c = obj.call("method_with_defaults", vslice![2, "Another string", 456]);
assert_eq!(c.to::<VariantArray>(), varray![2, "Another string", 456]);

/* For now, Gd<T> defaults are disabled due to immutability.
// Test that object is passed through, and that Option<Gd> with default Gd::null_arg() works.
let first = RefCounted::new_gd();
let d = obj
Expand All @@ -360,8 +371,10 @@ fn func_default_parameters() {
.to::<Gd<RefCounted>>();
assert_eq!(e.instance_id(), first.instance_id());
assert_eq!(e.get_meta("nullable_id"), second.instance_id().to_variant());
*/
}

/* For now, Gd<T> 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.
Expand All @@ -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<T> default parameter.
let arr = obj
.call("method_with_immutable_array_default", &[])
.to::<Array<i64>>();
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() {
Expand Down Expand Up @@ -431,6 +459,8 @@ fn cfg_removes_or_keeps_signals() {
assert!(!class_has_signal::<GdSelfObj>("cfg_removes_signal"));
}

// No test for Gd::from_object(), as that simply moves the existing object without running user code.

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Helpers

Expand Down
Loading