From 9bb45a6eaa1f23c09c64a2221b555a9eb5969bfd Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 20 Sep 2025 21:12:03 +0200 Subject: [PATCH 1/9] Remove unsound conversion from `VariantArray` to `PackedArray` --- godot-core/src/builtin/collections/packed_array.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/godot-core/src/builtin/collections/packed_array.rs b/godot-core/src/builtin/collections/packed_array.rs index f39549dce..4e0a215f4 100644 --- a/godot-core/src/builtin/collections/packed_array.rs +++ b/godot-core/src/builtin/collections/packed_array.rs @@ -771,8 +771,6 @@ impl fmt::Display for PackedArray { // ---------------------------------------------------------------------------------------------------------------------------------------------- // Specific API for PackedByteArray -impl_builtin_froms!(PackedByteArray; VariantArray => packed_byte_array_from_array); - macro_rules! declare_encode_decode { // $Via could be inferred, but ensures we have the correct type expectations. ($Ty:ty, $bytes:literal, $encode_fn:ident, $decode_fn:ident, $Via:ty) => { From 41c7fa405da81e749587e23c7e98cb3ad4341097 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 20 Sep 2025 21:37:00 +0200 Subject: [PATCH 2/9] Document soft invariants in builtins --- godot-core/src/builtin/aabb.rs | 8 +++++--- godot-core/src/builtin/plane.rs | 5 ++++- godot-core/src/builtin/rect2.rs | 5 ++++- godot-core/src/builtin/rect2i.rs | 5 ++++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/godot-core/src/builtin/aabb.rs b/godot-core/src/builtin/aabb.rs index 52a997e20..b0a66e640 100644 --- a/godot-core/src/builtin/aabb.rs +++ b/godot-core/src/builtin/aabb.rs @@ -28,8 +28,11 @@ use crate::builtin::{real, Plane, Vector3, Vector3Axis}; /// [`Rect2`]: crate::builtin::Rect2 /// [`Rect2i`]: crate::builtin::Rect2i /// -/// # Godot docs +/// # Soft invariants +/// `Aabb` requires non-negative size for certain operations, which is validated only on a best-effort basis. Violations may +/// cause panics in Debug mode. See also [_Builtin API design_](../__docs/index.html#6-public-fields-and-soft-invariants). /// +/// # Godot docs /// [`AABB`](https://docs.godotengine.org/en/stable/classes/class_aabb.html) #[derive(Default, Copy, Clone, PartialEq, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -421,8 +424,7 @@ impl Aabb { /// /// Most functions will fail to give a correct result if the size is negative. #[inline] - /// TODO(v0.3): make private, change to debug_assert(). - pub fn assert_nonnegative(self) { + fn assert_nonnegative(self) { assert!( self.size.x >= 0.0 && self.size.y >= 0.0 && self.size.z >= 0.0, "size {:?} is negative", diff --git a/godot-core/src/builtin/plane.rs b/godot-core/src/builtin/plane.rs index cd71d6b61..6091a23ea 100644 --- a/godot-core/src/builtin/plane.rs +++ b/godot-core/src/builtin/plane.rs @@ -23,8 +23,11 @@ use crate::builtin::{real, Vector3}; /// unit length and will panic if this invariant is violated. This is not separately /// annotated for each method. /// -/// # Godot docs +/// # Soft invariants +/// `Plane` requires that the normal vector has unit length for most operations, which is validated only on a best-effort basis. Violations may +/// cause panics in Debug mode. See also [_Builtin API design_](../__docs/index.html#6-public-fields-and-soft-invariants). /// +/// # Godot docs /// [Plane (stable)](https://docs.godotengine.org/en/stable/classes/class_plane.html) #[derive(Copy, Clone, PartialEq, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] diff --git a/godot-core/src/builtin/rect2.rs b/godot-core/src/builtin/rect2.rs index c2734c932..b817646f1 100644 --- a/godot-core/src/builtin/rect2.rs +++ b/godot-core/src/builtin/rect2.rs @@ -27,8 +27,11 @@ use crate::builtin::{real, Rect2i, Side, Vector2}; /// /// [`Aabb`]: crate::builtin::Aabb /// -/// # Godot docs +/// # Soft invariants +/// `Rect2` requires non-negative size for certain operations, which is validated only on a best-effort basis. Violations may +/// cause panics in Debug mode. See also [_Builtin API design_](../__docs/index.html#6-public-fields-and-soft-invariants). /// +/// # Godot docs /// [`Rect2` (stable)](https://docs.godotengine.org/en/stable/classes/class_rect2.html) #[derive(Default, Copy, Clone, PartialEq, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] diff --git a/godot-core/src/builtin/rect2i.rs b/godot-core/src/builtin/rect2i.rs index e212c1cdf..476ff6963 100644 --- a/godot-core/src/builtin/rect2i.rs +++ b/godot-core/src/builtin/rect2i.rs @@ -28,8 +28,11 @@ use crate::builtin::{Rect2, Side, Vector2i}; /// /// [`Aabb`]: crate::builtin::Aabb /// -/// # Godot docs +/// # Soft invariants +/// `Rect2i` requires non-negative size for certain operations, which is validated only on a best-effort basis. Violations may +/// cause panics in Debug mode. See also [_Builtin API design_](../__docs/index.html#6-public-fields-and-soft-invariants). /// +/// # Godot docs /// [`Rect2i` (stable)](https://docs.godotengine.org/en/stable/classes/class_rect2i.html) #[derive(Default, Copy, Clone, Eq, PartialEq, Hash, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] From 2a8a56221e08e3336476cd23164359f21eb79981 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 20 Sep 2025 21:40:19 +0200 Subject: [PATCH 3/9] Smaller TODO updates, historic pointer-init comment --- godot-codegen/src/conv/type_conversions.rs | 4 +--- godot-core/src/builtin/callable.rs | 2 +- godot-core/src/builtin/variant/impls.rs | 10 ++++------ godot-core/src/meta/args/as_arg.rs | 2 -- godot-core/src/meta/mod.rs | 7 ++++--- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/godot-codegen/src/conv/type_conversions.rs b/godot-codegen/src/conv/type_conversions.rs index 992a8982b..5e55d922b 100644 --- a/godot-codegen/src/conv/type_conversions.rs +++ b/godot-codegen/src/conv/type_conversions.rs @@ -308,9 +308,7 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream { _ => panic!("null not representable in target type {ty:?}"), } } - // empty string appears only for Callable/Rid in 4.0; default ctor syntax in 4.1+ - // TODO(v0.4): check if we can remove "" - "" | "RID()" | "Callable()" if !is_inner => { + "RID()" | "Callable()" if !is_inner => { return match ty { RustTy::BuiltinIdent { ty: ident, .. } if ident == "Rid" => quote! { Rid::Invalid }, RustTy::BuiltinIdent { ty: ident, .. } if ident == "Callable" => { diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index ad0d25ca1..558bc3d9f 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -615,7 +615,7 @@ mod custom_callable { #[allow(clippy::result_unit_err)] // TODO remove once there's a clear error type here. fn invoke(&mut self, args: &[&Variant]) -> Variant; - // TODO(v0.3): add object_id(). + // TODO(v0.5): add object_id(). /// Returns whether the callable is considered valid. /// diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index 816b9499d..c6be07f2b 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -22,12 +22,10 @@ use crate::task::{impl_dynamic_send, DynamicSend, IntoDynamicSend, ThreadConfine // ---------------------------------------------------------------------------------------------------------------------------------------------- // Macro definitions -// Certain types need to be passed as initialized pointers in their from_variant implementations in 4.0. Because -// 4.0 uses `*ptr = value` to return the type, and some types in C++ override `operator=` in C++ in a way -// that requires the pointer to be initialized. But some other types will cause a memory leak in 4.1 if initialized. -// -// Therefore, we can use `init` to indicate when it must be initialized in 4.0. -// TODO(v0.4): see if above comment is still relevant for 4.2+. +// Historical note: In Godot 4.0, certain types needed to be passed as initialized pointers in their from_variant implementations, because +// 4.0 used `*ptr = value` to return the type, and some types in C++ override `operator=` in a way that requires the pointer to be initialized. +// However, those same types would cause memory leaks in Godot 4.1 if pre-initialized. A compat layer `new_with_uninit_or_init()` addressed this. +// As these Godot versions are no longer supported, the current implementation uses `new_with_uninit()` uniformly for all versions. macro_rules! impl_ffi_variant { (ref $T:ty, $from_fn:ident, $to_fn:ident $(; $GodotTy:ident)?) => { impl_ffi_variant!(@impls by_ref; $T, $from_fn, $to_fn $(; $GodotTy)?); diff --git a/godot-core/src/meta/args/as_arg.rs b/godot-core/src/meta/args/as_arg.rs index 12249b71a..707982663 100644 --- a/godot-core/src/meta/args/as_arg.rs +++ b/godot-core/src/meta/args/as_arg.rs @@ -133,8 +133,6 @@ where // ---------------------------------------------------------------------------------------------------------------------------------------------- // Object (Gd + DynGd) impls -// TODO(v0.4): all objects + optional objects should be pass-by-ref. - // Convert `Gd` -> `Gd` (with upcast). impl AsArg> for &Gd where diff --git a/godot-core/src/meta/mod.rs b/godot-core/src/meta/mod.rs index a3471919a..90cdfdc8c 100644 --- a/godot-core/src/meta/mod.rs +++ b/godot-core/src/meta/mod.rs @@ -37,10 +37,11 @@ //! ## Argument conversions //! //! Rust does not support implicit conversions, however it has something very close: the `impl Into` idiom, which can be used to convert -//! "T-compatible" arguments into `T`. This library specializes this idea with two traits: +//! "T-compatible" arguments into `T`. //! -//! - [`AsArg`] allows argument conversions from arguments into `T`. This is most interesting in the context of strings (so you can pass -//! `&str` to a function expecting `GString`), but is generic to support object arguments like `Gd` and array insertion. +//! This library specializes this idea with the trait [`AsArg`]. `AsArg` allows argument conversions from arguments into `T`. +//! This is most interesting in the context of strings (so you can pass `&str` to a function expecting `GString`) and objects (pass +//! `&Gd` to a function expecting `Node2D` objects). mod args; mod class_id; From f8a6529015a5a78806df48b1b109adbac8a73d54 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 21 Sep 2025 09:21:04 +0200 Subject: [PATCH 4/9] Remove EngineEnum::godot_name() --- godot-codegen/src/generator/enums.rs | 51 ++----------------- godot-core/src/builtin/vectors/vector_axis.rs | 8 --- godot-core/src/obj/traits.rs | 24 --------- itest/rust/src/object_tests/enum_test.rs | 23 --------- 4 files changed, 4 insertions(+), 102 deletions(-) diff --git a/godot-codegen/src/generator/enums.rs b/godot-codegen/src/generator/enums.rs index 71c184aa6..1599e48d7 100644 --- a/godot-codegen/src/generator/enums.rs +++ b/godot-codegen/src/generator/enums.rs @@ -268,7 +268,7 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T } }); - let str_functions = make_enum_str_functions(enum_); + let str_functions = make_enum_as_str(enum_); let values_and_constants_functions = make_enum_values_and_constants_functions(enum_); quote! { @@ -296,7 +296,7 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T // However, those with masks don't have strict validation when marshalling from integers, and a Debug repr which includes the mask. let unique_ords = enum_.unique_ords().expect("self is an enum"); - let str_functions = make_enum_str_functions(enum_); + let str_functions = make_enum_as_str(enum_); let values_and_constants_functions = make_enum_values_and_constants_functions(enum_); // We can technically check against all possible mask values, remove each mask, and then verify it's a valid base-enum value. @@ -391,49 +391,10 @@ fn make_all_constants_function(enum_: &Enum) -> TokenStream { } } -/// Creates the `as_str` and `godot_name` implementations for the enum. -fn make_enum_str_functions(enum_: &Enum) -> TokenStream { +/// Creates the `as_str()` implementation for the enum. +fn make_enum_as_str(enum_: &Enum) -> TokenStream { let as_str_enumerators = make_enum_to_str_cases(enum_); - // Only enumerations with different godot names are specified. - // `as_str` is called for the rest of them. - let godot_different_cases = { - let enumerators = enum_ - .enumerators - .iter() - .filter(|enumerator| enumerator.name != enumerator.godot_name) - .map(|enumerator| { - let Enumerator { - name, godot_name, .. - } = enumerator; - let godot_name_str = godot_name.to_string(); - quote! { - Self::#name => #godot_name_str, - } - }); - - quote! { - #( #enumerators )* - } - }; - - let godot_name_match = if godot_different_cases.is_empty() { - // If empty, all the Rust names match the Godot ones. - // Remove match statement to avoid `clippy::match_single_binding`. - quote! { - self.as_str() - } - } else { - quote! { - // Many enums have duplicates, thus allow unreachable. - #[allow(unreachable_patterns)] - match *self { - #godot_different_cases - _ => self.as_str(), - } - } - }; - quote! { #[inline] fn as_str(&self) -> &'static str { @@ -444,10 +405,6 @@ fn make_enum_str_functions(enum_: &Enum) -> TokenStream { _ => "", } } - - fn godot_name(&self) -> &'static str { - #godot_name_match - } } } diff --git a/godot-core/src/builtin/vectors/vector_axis.rs b/godot-core/src/builtin/vectors/vector_axis.rs index 63ce498cf..2c27c6782 100644 --- a/godot-core/src/builtin/vectors/vector_axis.rs +++ b/godot-core/src/builtin/vectors/vector_axis.rs @@ -47,14 +47,6 @@ macro_rules! impl_vector_axis_enum { } } - fn godot_name(&self) -> &'static str { - match *self { - $( - Self::$axis => concat!("AXIS_", stringify!($axis)), - )+ - } - } - fn values() -> &'static [Self] { // For vector axis enums, all values are distinct, so both are the same &[ diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index 692f7b38d..b622f46b9 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -211,30 +211,6 @@ pub trait EngineEnum: Copy { /// If the value does not match one of the known enumerators, the empty string is returned. fn as_str(&self) -> &'static str; - /// The equivalent name of the enumerator, as specified in Godot. - /// - /// If the value does not match one of the known enumerators, the empty string is returned. - /// - /// # Deprecation - /// Design change is due to the fact that Godot enums may have multiple constants with the same ordinal value, and `godot_name()` cannot - /// always return a unique name for it. So there are cases where this method returns unexpected results. - /// - /// To keep the old -- possibly incorrect -- behavior, you can write the following function. However, it runs in linear rather than constant - /// time (which is often OK, given that there are very few constants per enum). - /// ``` - /// use godot::obj::EngineEnum; - /// - /// fn godot_name(value: T) -> &'static str { - /// T::all_constants() - /// .iter() - /// .find(|c| c.value() == value) - /// .map(|c| c.godot_name()) - /// .unwrap_or("") // Previous behavior. - /// } - /// ``` - #[deprecated = "Moved to introspection API, see `EngineEnum::all_constants()` and `EnumConstant::godot_name()`"] - fn godot_name(&self) -> &'static str; - /// Returns a slice of distinct enum values. /// /// This excludes `MAX` constants at the end (existing only to express the number of enumerators) and deduplicates aliases, diff --git a/itest/rust/src/object_tests/enum_test.rs b/itest/rust/src/object_tests/enum_test.rs index 52034d130..5f93e61d0 100644 --- a/itest/rust/src/object_tests/enum_test.rs +++ b/itest/rust/src/object_tests/enum_test.rs @@ -114,29 +114,6 @@ fn enum_godot_name() { assert_eq!(godot_name(Key::from_ord(1234)), ""); } -#[itest] -#[expect(deprecated)] -fn enum_godot_name_deprecated() { - use godot::obj::EngineEnum; - assert_eq!( - Orientation::VERTICAL.godot_name(), - Orientation::VERTICAL.as_str() - ); - assert_eq!( - Orientation::HORIZONTAL.godot_name(), - Orientation::HORIZONTAL.as_str() - ); - - assert_eq!(Key::NONE.godot_name(), "KEY_NONE"); - assert_eq!(Key::SPECIAL.godot_name(), "KEY_SPECIAL"); - assert_eq!(Key::ESCAPE.godot_name(), "KEY_ESCAPE"); - assert_eq!(Key::TAB.godot_name(), "KEY_TAB"); - assert_eq!(Key::A.godot_name(), "KEY_A"); - - // Unknown enumerators (might come from the future). - assert_eq!(Key::from_ord(1234).godot_name(), ""); -} - fn godot_name(value: T) -> &'static str { T::all_constants() .iter() From d53c0b133de000343e0e86002dfa8f87af1a5143 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 21 Sep 2025 09:21:21 +0200 Subject: [PATCH 5/9] Remove Base::to_gd() --- godot-core/src/obj/base.rs | 10 ---------- itest/rust/src/object_tests/base_test.rs | 17 ----------------- 2 files changed, 27 deletions(-) diff --git a/godot-core/src/obj/base.rs b/godot-core/src/obj/base.rs index d13f7969b..1e02e038c 100644 --- a/godot-core/src/obj/base.rs +++ b/godot-core/src/obj/base.rs @@ -156,16 +156,6 @@ impl Base { } } - /// Returns a [`Gd`] referencing the same object as this reference. - /// - /// Using this method to call methods on the base field of a Rust object is discouraged, instead use the - /// methods from [`WithBaseField`](super::WithBaseField) when possible. - #[doc(hidden)] - #[deprecated = "Private API. Use `Base::to_init_gd()` or `WithBaseField::to_gd()` instead."] // TODO(v0.4): remove. - pub fn to_gd(&self) -> Gd { - (*self.obj).clone() - } - /// Returns a [`Gd`] referencing the base object, for exclusive use during object initialization and `NOTIFICATION_POSTINITIALIZE`. /// /// Can be used during an initialization function [`I*::init()`][crate::classes::IObject::init] or [`Gd::from_init_fn()`], or [`POSTINITIALIZE`][crate::classes::notify::ObjectNotification::POSTINITIALIZE]. diff --git a/itest/rust/src/object_tests/base_test.rs b/itest/rust/src/object_tests/base_test.rs index 57ebf74f0..7278cc32e 100644 --- a/itest/rust/src/object_tests/base_test.rs +++ b/itest/rust/src/object_tests/base_test.rs @@ -70,23 +70,6 @@ fn base_debug() { obj.free(); } -// Compatibility check until v0.4 Base::to_gd() is removed. -#[itest] -fn base_with_init() { - let obj = Gd::::from_init_fn(|base| { - #[allow(deprecated)] - base.to_gd().set_rotation(11.0); - Based { base, i: 732 } - }); - - { - let guard = obj.bind(); - assert_eq!(guard.i, 732); - assert_eq!(guard.base().get_rotation(), 11.0); - } - obj.free(); -} - #[itest] fn base_gd_self() { let obj = Based::new_alloc(); From f5eee5ce434cbb3cb4a75a38c18c1951a19d304c Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sun, 21 Sep 2025 09:29:15 +0200 Subject: [PATCH 6/9] Remove support for export-range `radians` --- godot-core/src/deprecated.rs | 8 +--- .../src/class/data_models/field_export.rs | 38 ++++++++----------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/godot-core/src/deprecated.rs b/godot-core/src/deprecated.rs index 313813306..bde072132 100644 --- a/godot-core/src/deprecated.rs +++ b/godot-core/src/deprecated.rs @@ -38,10 +38,6 @@ pub use crate::emit_deprecated_warning; // Library-side deprecations -- see usage description above. // ---------------------------------------------------------------------------------------------------------------------------------------------- -// Godot-side deprecations +// Godot-side deprecations (we may mark them deprecated but keep support). -// This is a Godot-side deprecation. Since it's the only way in Godot 4.1, we keep compatibility for now. -// TODO(v0.4): remove with error. -#[deprecated = "\nUse #[export(range = (radians_as_degrees))] and not #[export(range = (radians))].\n\ - More information on https://github.com/godotengine/godot/pull/82195."] -pub const fn export_range_radians() {} +// Past removals: `radians` in #[export(range)]. diff --git a/godot-macros/src/class/data_models/field_export.rs b/godot-macros/src/class/data_models/field_export.rs index 300fcb6a5..4f4489aa3 100644 --- a/godot-macros/src/class/data_models/field_export.rs +++ b/godot-macros/src/class/data_models/field_export.rs @@ -10,7 +10,7 @@ use std::collections::{HashMap, HashSet}; use proc_macro2::{Ident, Span, TokenStream}; use quote::quote; -use crate::util::{ident, KvParser, ListParser}; +use crate::util::{bail, ident, KvParser, ListParser}; use crate::ParseResult; pub struct FieldExport { @@ -72,7 +72,6 @@ pub enum ExportType { or_less: bool, exp: bool, radians_as_degrees: bool, - radians: bool, degrees: bool, hide_slider: bool, suffix: Option, @@ -331,11 +330,19 @@ impl ExportType { let key_maybe_value = parser.next_allowed_key_optional_value(&FLAG_OPTIONS, &KV_OPTIONS)?; match key_maybe_value { - Some((option, None)) => { - flags.insert(option.to_string()); + Some((ident, None)) => { + if ident == "radians" { + return bail!( + &ident, + "#[export(range = (...))]: `radians` is broken in Godot and superseded by `radians_as_degrees`.\n\ + See https://github.com/godotengine/godot/pull/82195 for details." + ); + } + + flags.insert(ident.to_string()); } - Some((option, Some(value))) => { - kvs.insert(option.to_string(), value.expr()?); + Some((ident, Some(value))) => { + kvs.insert(ident.to_string(), value.expr()?); } None => break, } @@ -351,7 +358,6 @@ impl ExportType { or_less: flags.contains("or_less"), exp: flags.contains("exp"), radians_as_degrees: flags.contains("radians_as_degrees"), - radians: flags.contains("radians"), degrees: flags.contains("degrees"), hide_slider: flags.contains("hide_slider"), suffix: kvs.get("suffix").cloned(), @@ -441,7 +447,6 @@ impl ExportType { or_less, exp, radians_as_degrees, - radians, degrees, hide_slider, suffix, @@ -452,22 +457,9 @@ impl ExportType { quote! { None } }; let export_func = quote_export_func! { - export_range(#min, #max, #step, #or_greater, #or_less, #exp, #radians_as_degrees || #radians, #degrees, #hide_slider, #suffix) + export_range(#min, #max, #step, #or_greater, #or_less, #exp, #radians_as_degrees, #degrees, #hide_slider, #suffix) }?; - let deprecation_warning = if *radians { - // For some reason, rustfmt formatting like this. Probably a bug. - // See https://github.com/godot-rust/gdext/pull/783#discussion_r1669105958 and - // https://github.com/rust-lang/rustfmt/issues/6233 - quote! { - #export_func; - ::godot::__deprecated::emit_deprecated_warning!(export_range_radians); - } - } else { - quote! { #export_func } - }; - Some(quote! { - #deprecation_warning - }) + Some(export_func) } Self::Enum { variants } => { From 978867c3e0cfc80352cecf33fe3bbc5443ed4860 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 27 Sep 2025 13:41:18 +0200 Subject: [PATCH 7/9] Move ToSignalObject -> meta::ObjectToOwned --- godot-core/src/meta/mod.rs | 2 ++ godot-core/src/meta/object_to_owned.rs | 36 +++++++++++++++++++ godot-core/src/meta/uniform_object_deref.rs | 3 ++ .../src/registry/signal/connect_builder.rs | 16 ++++----- .../src/registry/signal/typed_signal.rs | 33 ++++------------- 5 files changed, 55 insertions(+), 35 deletions(-) create mode 100644 godot-core/src/meta/object_to_owned.rs diff --git a/godot-core/src/meta/mod.rs b/godot-core/src/meta/mod.rs index 90cdfdc8c..73aeb751e 100644 --- a/godot-core/src/meta/mod.rs +++ b/godot-core/src/meta/mod.rs @@ -48,6 +48,7 @@ mod class_id; mod element_type; mod godot_convert; mod method_info; +mod object_to_owned; mod param_tuple; mod property_info; mod signature; @@ -67,6 +68,7 @@ pub use class_id::{ClassId, ClassName}; pub use element_type::{ElementScript, ElementType}; pub use godot_convert::{FromGodot, GodotConvert, ToGodot}; pub use method_info::MethodInfo; +pub use object_to_owned::ObjectToOwned; pub use param_tuple::{InParamTuple, OutParamTuple, ParamTuple}; pub use property_info::{PropertyHintInfo, PropertyInfo}; #[cfg(feature = "trace")] diff --git a/godot-core/src/meta/object_to_owned.rs b/godot-core/src/meta/object_to_owned.rs new file mode 100644 index 000000000..24a38af1a --- /dev/null +++ b/godot-core/src/meta/object_to_owned.rs @@ -0,0 +1,36 @@ +/* + * 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 crate::obj::{Gd, GodotClass, WithBaseField}; + +/// Obtain owned `Gd` from either `&self` or `&Gd`. +/// +/// This trait allows passing either `Gd` or `C` (where `C: WithBaseField`) to functions that need an owned `Gd`. +/// +/// This is primarily used for signal connection methods in [`TypedSignal`][crate::registry::signal::TypedSignal] and +/// [`ConnectBuilder`][crate::registry::signal::ConnectBuilder], where you can pass either a `&Gd` (outside) or `&SomeClass` +/// (from within `impl` block) as the receiver object. +/// +/// # Similar traits +/// - [`UniformObjectDeref`][crate::meta::UniformObjectDeref] provides unified dereferencing of user and engine classes. +/// - [`AsArg`][crate::meta::AsArg] enables general argument conversions for Godot APIs. +pub trait ObjectToOwned { + /// Converts the object reference to an owned `Gd`. + fn object_to_owned(&self) -> Gd; +} + +impl ObjectToOwned for Gd { + fn object_to_owned(&self) -> Gd { + self.clone() + } +} + +impl ObjectToOwned for C { + fn object_to_owned(&self) -> Gd { + WithBaseField::to_gd(self) + } +} diff --git a/godot-core/src/meta/uniform_object_deref.rs b/godot-core/src/meta/uniform_object_deref.rs index ca9e77e9d..b6b4a0885 100644 --- a/godot-core/src/meta/uniform_object_deref.rs +++ b/godot-core/src/meta/uniform_object_deref.rs @@ -60,6 +60,9 @@ use crate::obj::{Gd, GdMut, GdRef, GodotClass, WithBaseField}; /// abstract_over_objects(&user_obj); /// } /// ``` +/// +/// # Similar traits +/// - [`ObjectToOwned`][crate::meta::ObjectToOwned] provides conversion from `&self` or `&Gd` to owned `Gd`. // // The crate `https://crates.io/crates/disjoint_impls` handles this in a more user-friendly way, we should // consider using it if disjoint impls are going to be frequently used. diff --git a/godot-core/src/registry/signal/connect_builder.rs b/godot-core/src/registry/signal/connect_builder.rs index 495785b95..52175b1d0 100644 --- a/godot-core/src/registry/signal/connect_builder.rs +++ b/godot-core/src/registry/signal/connect_builder.rs @@ -11,10 +11,10 @@ use crate::builtin::Callable; use crate::builtin::{GString, Variant}; use crate::classes::object::ConnectFlags; use crate::meta; -use crate::meta::InParamTuple; +use crate::meta::{InParamTuple, ObjectToOwned}; use crate::obj::{bounds, Bounds, Gd, GodotClass, WithSignals}; use crate::registry::signal::signal_receiver::{IndirectSignalReceiver, SignalReceiver}; -use crate::registry::signal::{ConnectHandle, ToSignalObj, TypedSignal}; +use crate::registry::signal::{ConnectHandle, TypedSignal}; /// Builder for customizing signal connections. /// @@ -239,7 +239,7 @@ impl ConnectBuilder<'_, '_, C, Ps> { /// - If you need cross-thread signals, use [`connect_sync()`](#method.connect_sync) instead (requires feature "experimental-threads"). pub fn connect_other_mut( self, - object: &impl ToSignalObj, + object: &impl ObjectToOwned, mut method: F, ) -> ConnectHandle where @@ -247,7 +247,7 @@ impl ConnectBuilder<'_, '_, C, Ps> { for<'c_rcv> F: SignalReceiver<&'c_rcv mut OtherC, Ps>, for<'c_rcv> IndirectSignalReceiver<'c_rcv, &'c_rcv mut OtherC, Ps, F>: From<&'c_rcv mut F>, { - let mut gd = object.to_signal_obj(); + let mut gd = object.object_to_owned(); let godot_fn = make_godot_fn(move |args| { let mut guard = Gd::bind_mut(&mut gd); @@ -256,7 +256,7 @@ impl ConnectBuilder<'_, '_, C, Ps> { .call(&mut *guard, args); }); - self.inner_connect_godot_fn::(godot_fn, &object.to_signal_obj()) + self.inner_connect_godot_fn::(godot_fn, &object.object_to_owned()) } /// Connect a method with any `&mut Gd` as the first parameter (user + engine classes). @@ -272,7 +272,7 @@ impl ConnectBuilder<'_, '_, C, Ps> { /// - If you need cross-thread signals, use [`connect_sync()`](#method.connect_sync) instead (requires feature "experimental-threads"). pub fn connect_other_gd( self, - object: &impl ToSignalObj, + object: &impl ObjectToOwned, mut method: F, ) -> ConnectHandle where @@ -280,7 +280,7 @@ impl ConnectBuilder<'_, '_, C, Ps> { F: SignalReceiver, Ps>, for<'c_rcv> IndirectSignalReceiver<'c_rcv, Gd, Ps, F>: From<&'c_rcv mut F>, { - let gd = object.to_signal_obj(); + let gd = object.object_to_owned(); let godot_fn = make_godot_fn(move |args| { IndirectSignalReceiver::from(&mut method) @@ -288,7 +288,7 @@ impl ConnectBuilder<'_, '_, C, Ps> { .call(gd.clone(), args); }); - self.inner_connect_godot_fn::(godot_fn, &object.to_signal_obj()) + self.inner_connect_godot_fn::(godot_fn, &object.object_to_owned()) } /// Connect to this signal using a thread-safe function, allows the signal to be called across threads. diff --git a/godot-core/src/registry/signal/typed_signal.rs b/godot-core/src/registry/signal/typed_signal.rs index c764f6eee..7e2f3a78a 100644 --- a/godot-core/src/registry/signal/typed_signal.rs +++ b/godot-core/src/registry/signal/typed_signal.rs @@ -13,31 +13,10 @@ use super::{make_callable_name, make_godot_fn, ConnectBuilder, ConnectHandle, Si use crate::builtin::{Callable, Variant}; use crate::classes::object::ConnectFlags; use crate::meta; -use crate::meta::{InParamTuple, UniformObjectDeref}; -use crate::obj::{Gd, GodotClass, WithBaseField, WithSignals}; +use crate::meta::{InParamTuple, ObjectToOwned, UniformObjectDeref}; +use crate::obj::{Gd, GodotClass, WithSignals}; use crate::registry::signal::signal_receiver::{IndirectSignalReceiver, SignalReceiver}; -// TODO(v0.4): find more general name for trait. -/// Object part of the signal receiver (handler). -/// -/// Functionality overlaps partly with [`meta::AsObjectArg`] and [`meta::AsArg`]. Can however not directly be replaced -/// with `AsObjectArg`, since that allows nullability and doesn't require `&mut T`. Maybe there's a way to reuse them though. -pub trait ToSignalObj { - fn to_signal_obj(&self) -> Gd; -} - -impl ToSignalObj for Gd { - fn to_signal_obj(&self) -> Gd { - self.clone() - } -} - -impl ToSignalObj for C { - fn to_signal_obj(&self) -> Gd { - WithBaseField::to_gd(self) - } -} - // ---------------------------------------------------------------------------------------------------------------------------------------------- /// Type-safe version of a Godot signal. @@ -234,7 +213,7 @@ impl TypedSignal<'_, C, Ps> { /// The parameter `object` can be of 2 different "categories": /// - Any `&Gd` (e.g.: `&Gd`, `&Gd`). /// - `&OtherC`, as long as `OtherC` is a user class that contains a `base` field (it implements the - /// [`WithBaseField`](WithBaseField) trait). + /// [`WithBaseField`][crate::obj::WithBaseField] trait). /// /// --- /// @@ -242,7 +221,7 @@ impl TypedSignal<'_, C, Ps> { /// - If you need [`connect flags`](ConnectFlags) or cross-thread signals, use [`builder()`][Self::builder]. pub fn connect_other( &self, - object: &impl ToSignalObj, + object: &impl ObjectToOwned, mut method: F, ) -> ConnectHandle where @@ -250,7 +229,7 @@ impl TypedSignal<'_, C, Ps> { for<'c_rcv> F: SignalReceiver<&'c_rcv mut OtherC, Ps> + 'static, for<'c_rcv> IndirectSignalReceiver<'c_rcv, &'c_rcv mut OtherC, Ps, F>: From<&'c_rcv mut F>, { - let mut gd = object.to_signal_obj(); + let mut gd = object.object_to_owned(); let godot_fn = make_godot_fn(move |args| { let mut target = OtherC::object_as_mut(&mut gd); @@ -260,6 +239,6 @@ impl TypedSignal<'_, C, Ps> { .call(target_mut, args); }); - self.inner_connect_godot_fn::(godot_fn, &object.to_signal_obj()) + self.inner_connect_godot_fn::(godot_fn, &object.object_to_owned()) } } From e1fb507069ab249cb41abfbcfb3dfba9d94b5130 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 27 Sep 2025 23:10:00 +0200 Subject: [PATCH 8/9] Restore deprecated `apply_deferred()` for longer migration period --- godot-core/src/obj/gd.rs | 9 +++++++++ godot-core/src/obj/traits.rs | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 282839f86..cd986f460 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -714,6 +714,15 @@ impl Gd { }); callable.call_deferred(&[]); } + + #[deprecated = "Split into `run_deferred()` + `run_deferred_gd()`."] + pub fn apply_deferred(&mut self, rust_function: F) + where + T: WithBaseField, + F: FnOnce(&mut T) + 'static, + { + self.run_deferred(rust_function) + } } /// _The methods in this impl block are only available for objects `T` that are manually managed, diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index b622f46b9..2a69012c9 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -550,6 +550,14 @@ pub trait WithBaseField: GodotClass + Bounds { { self.to_gd().run_deferred_gd(gd_function) } + + #[deprecated = "Split into `run_deferred()` + `run_deferred_gd()`."] + fn apply_deferred(&mut self, rust_function: F) + where + F: FnOnce(&mut Self) + 'static, + { + self.run_deferred(rust_function) + } } /// Implemented for all classes with registered signals, both engine- and user-declared. From 099e87322ec76f1fea08c9aa869dc000314f27e9 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 27 Sep 2025 23:33:41 +0200 Subject: [PATCH 9/9] Safety doc for FFI tables `load` constructor --- .../src/generator/extension_interface.rs | 7 +++---- godot-codegen/src/generator/method_tables.rs | 17 +++++++---------- godot-codegen/src/util.rs | 8 ++++++++ 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/godot-codegen/src/generator/extension_interface.rs b/godot-codegen/src/generator/extension_interface.rs index fefe90a26..937dac437 100644 --- a/godot-codegen/src/generator/extension_interface.rs +++ b/godot-codegen/src/generator/extension_interface.rs @@ -12,7 +12,7 @@ use proc_macro2::{Ident, Literal, TokenStream}; use quote::quote; use regex::Regex; -use crate::util::ident; +use crate::util::{ident, make_load_safety_doc}; use crate::SubmitFn; pub fn generate_sys_interface_file( @@ -77,15 +77,14 @@ fn generate_proc_address_funcs(h_path: &Path) -> TokenStream { } // Do not derive Copy -- even though the struct is bitwise-copyable, this is rarely needed and may point to an error. + let safety_doc = make_load_safety_doc(); let code = quote! { pub struct GDExtensionInterface { #( #fptr_decls )* } impl GDExtensionInterface { - // TODO: Figure out the right safety preconditions. This currently does not have any because incomplete safety docs - // can cause issues with people assuming they are sufficient. - #[allow(clippy::missing_safety_doc)] + #safety_doc pub(crate) unsafe fn load( get_proc_address: crate::GDExtensionInterfaceGetProcAddress, ) -> Self { diff --git a/godot-codegen/src/generator/method_tables.rs b/godot-codegen/src/generator/method_tables.rs index 602d67d2b..31eb0a937 100644 --- a/godot-codegen/src/generator/method_tables.rs +++ b/godot-codegen/src/generator/method_tables.rs @@ -16,7 +16,7 @@ use crate::models::domain::{ BuiltinClass, BuiltinMethod, BuiltinVariant, Class, ClassCodegenLevel, ClassLike, ClassMethod, ExtensionApi, FnDirection, Function, TyName, }; -use crate::util::ident; +use crate::util::{ident, make_load_safety_doc}; use crate::{conv, generator, special_cases, util}; pub fn make_builtin_lifecycle_table(api: &ExtensionApi) -> TokenStream { @@ -276,6 +276,7 @@ fn make_named_method_table(info: NamedMethodTable) -> TokenStream { // Assumes that both decls and inits already have a trailing comma. // This is necessary because some generators emit multiple lines (statements) per element. + let safety_doc = make_load_safety_doc(); quote! { #imports @@ -288,9 +289,7 @@ fn make_named_method_table(info: NamedMethodTable) -> TokenStream { pub const CLASS_COUNT: usize = #class_count; pub const METHOD_COUNT: usize = #method_count; - // TODO: Figure out the right safety preconditions. This currently does not have any because incomplete safety docs - // can cause issues with people assuming they are sufficient. - #[allow(clippy::missing_safety_doc)] + #safety_doc pub unsafe fn load( #ctor_parameters ) -> Self { @@ -374,6 +373,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream { // Assumes that inits already have a trailing comma. // This is necessary because some generators emit multiple lines (statements) per element. + let safety_doc = make_load_safety_doc(); quote! { #imports @@ -387,9 +387,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream { pub const CLASS_COUNT: usize = #class_count; pub const METHOD_COUNT: usize = #method_count; - // TODO: Figure out the right safety preconditions. This currently does not have any because incomplete safety docs - // can cause issues with people assuming they are sufficient. - #[allow(clippy::missing_safety_doc)] + #safety_doc #unused_attr pub unsafe fn load( #ctor_parameters @@ -440,6 +438,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream { // Assumes that inits already have a trailing comma. // This is necessary because some generators emit multiple lines (statements) per element. + let safety_doc = make_load_safety_doc(); quote! { #imports use crate::StringCache; @@ -462,9 +461,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream { pub const CLASS_COUNT: usize = #class_count; pub const METHOD_COUNT: usize = #method_count; - // TODO: Figure out the right safety preconditions. This currently does not have any because incomplete safety docs - // can cause issues with people assuming they are sufficient. - #[allow(clippy::missing_safety_doc)] + #safety_doc #unused_attr pub unsafe fn load() -> Self { // SAFETY: interface and lifecycle tables are initialized at this point, so we can get 'static references to them. diff --git a/godot-codegen/src/util.rs b/godot-codegen/src/util.rs index e249085f5..20f7a92b0 100644 --- a/godot-codegen/src/util.rs +++ b/godot-codegen/src/util.rs @@ -85,6 +85,14 @@ pub fn lifetime(s: &str) -> TokenStream { TokenStream::from_iter([tk_apostrophe, tk_lifetime]) } +pub fn make_load_safety_doc() -> TokenStream { + quote! { + /// # Safety + /// - Must be called exactly once during library initialization. + /// - All parameters (dependencies) must have been initialized and valid. + } +} + // This function is duplicated in godot-macros\src\util\mod.rs #[rustfmt::skip] pub fn safe_ident(s: &str) -> Ident {