From 7a3ff279414be18881b0e04479e133da34be6ed5 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Wed, 9 Oct 2024 13:28:43 +0200 Subject: [PATCH 1/2] Add Variant::object_id() --- godot-core/src/builtin/variant/mod.rs | 26 ++++++++++++++----- .../builtin_tests/containers/variant_test.rs | 21 +++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index b3936ca7a..08dd53779 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -46,14 +46,14 @@ impl Variant { /// Create a variant holding a non-nil value. /// - /// Equivalent to `value.to_variant()`. + /// Equivalent to [`value.to_variant()`][ToGodot::to_variant], but consumes the argument. pub fn from(value: T) -> Self { value.to_variant() } /// ⚠️ Convert to type `T`, panicking on failure. /// - /// Equivalent to `T::from_variant(&self)`. + /// Equivalent to [`T::from_variant(&self)`][FromGodot::from_variant]. /// /// # Panics /// When this variant holds a different type. @@ -63,14 +63,14 @@ impl Variant { /// Convert to type `T`, returning `Err` on failure. /// - /// Equivalent to `T::try_from_variant(&self)`. + /// Equivalent to [`T::try_from_variant(&self)`][FromGodot::try_from_variant]. pub fn try_to(&self) -> Result { T::try_from_variant(self) } /// Checks whether the variant is empty (`null` value in GDScript). /// - /// See also [`Self::get_type`]. + /// See also [`get_type()`][Self::get_type]. pub fn is_nil(&self) -> bool { // Use get_type() rather than sys_type(), to also cover nullptr OBJECT as NIL self.get_type() == VariantType::NIL @@ -79,8 +79,8 @@ impl Variant { /// Returns the type that is currently held by this variant. /// /// If this variant holds a type `Object` but no instance (represented as a null object pointer), then `Nil` will be returned for - /// consistency. This may deviate from Godot behavior -- for example, calling `Node::get_node_or_null()` with an invalid - /// path returns a variant that has type `Object` but acts like `Nil` for all practical purposes. + /// consistency. This may deviate from Godot behavior -- for example, calling [`Node::get_node_or_null()`][crate::classes::Node::get_node_or_null] + /// with an invalid path returns a variant that has type `Object` but acts like `Nil` for all practical purposes. pub fn get_type(&self) -> VariantType { let sys_type = self.sys_type(); @@ -106,6 +106,20 @@ impl Variant { } } + /// For variants holding an object, returns the object's instance ID. + /// + /// If the variant is not an object, returns `None`. + /// + /// If the object is dead, the instance ID is still returned. Use [`Variant::try_to::>()`][Self::try_to] + /// to retrieve only live objects. + #[cfg(since_api = "4.4")] + pub fn object_id(&self) -> Option { + // SAFETY: safe to call for non-object variants (returns 0). + let raw_id: u64 = unsafe { interface_fn!(variant_get_object_instance_id)(self.var_sys()) }; + + crate::obj::InstanceId::try_from_u64(raw_id) + } + /// ⚠️ Calls the specified `method` with the given `args`. /// /// Supports `Object` as well as built-ins with methods (e.g. `Array`, `Vector3`, `GString`, etc.). diff --git a/itest/rust/src/builtin_tests/containers/variant_test.rs b/itest/rust/src/builtin_tests/containers/variant_test.rs index 04f0575d2..d3b88e90a 100644 --- a/itest/rust/src/builtin_tests/containers/variant_test.rs +++ b/itest/rust/src/builtin_tests/containers/variant_test.rs @@ -186,6 +186,27 @@ fn variant_get_type() { assert_eq!(variant.get_type(), VariantType::BASIS) } +#[cfg(since_api = "4.4")] +#[itest] +fn variant_object_id() { + let variant = Variant::nil(); + assert_eq!(variant.object_id(), None); + + let variant = Variant::from(77); + assert_eq!(variant.object_id(), None); + + let node = Node::new_alloc(); + let id = node.instance_id(); + + let variant = node.to_variant(); + assert_eq!(variant.object_id(), Some(id)); + + node.free(); + + // When freed, variant still returns the object ID. + assert_eq!(variant.object_id(), Some(id)); +} + #[itest] fn variant_equal() { assert_eq!(Variant::nil(), ().to_variant()); From 9a0d8ba4be03087aa6af8dc4fb48f1a6503cc7d2 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Wed, 9 Oct 2024 13:33:32 +0200 Subject: [PATCH 2/2] Add test + doc for Signal/Callable::object_id() on dead objects --- godot-core/src/builtin/callable.rs | 6 ++++-- godot-core/src/builtin/signal.rs | 5 ++++- .../src/builtin_tests/containers/callable_test.rs | 14 ++++++++++---- .../src/builtin_tests/containers/signal_test.rs | 10 ++++++++-- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index a88ff657d..af482163b 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -205,8 +205,8 @@ impl Callable { /// Returns the object on which this callable is called. /// - /// Returns `None` when this callable doesn't have any target object to call a method on, regardless of - /// if the method exists for that target or not. + /// Returns `None` when this callable doesn't have any target object to call a method on (regardless of whether the method exists for that + /// target or not). Also returns `None` if the object is dead. You can differentiate these two cases using [`object_id()`][Self::object_id]. /// /// _Godot equivalent: `get_object`_ pub fn object(&self) -> Option> { @@ -222,6 +222,8 @@ impl Callable { /// /// Returns `None` when this callable doesn't have any target to call a method on. /// + /// If the pointed-to object is dead, the ID will still be returned. Use [`object()`][Self::object] to check for liveness. + /// /// _Godot equivalent: `get_object_id`_ pub fn object_id(&self) -> Option { let id = self.as_inner().get_object_id(); diff --git a/godot-core/src/builtin/signal.rs b/godot-core/src/builtin/signal.rs index 1e6393a11..4184a214a 100644 --- a/godot-core/src/builtin/signal.rs +++ b/godot-core/src/builtin/signal.rs @@ -121,7 +121,8 @@ impl Signal { /// Returns the object to which this signal belongs. /// - /// Returns [`None`] when this signal doesn't have any object. + /// Returns [`None`] when this signal doesn't have any object, or the object is dead. You can differentiate these two situations using + /// [`object_id()`][Self::object_id]. /// /// _Godot equivalent: `get_object`_ pub fn object(&self) -> Option> { @@ -135,6 +136,8 @@ impl Signal { /// /// Returns [`None`] when this signal doesn't have any object. /// + /// If the pointed-to object is dead, the ID will still be returned. Use [`object()`][Self::object] to check for liveness. + /// /// _Godot equivalent: `get_object_id`_ pub fn object_id(&self) -> Option { let id = self.as_inner().get_object_id(); diff --git a/itest/rust/src/builtin_tests/containers/callable_test.rs b/itest/rust/src/builtin_tests/containers/callable_test.rs index c5e6bded9..504391d91 100644 --- a/itest/rust/src/builtin_tests/containers/callable_test.rs +++ b/itest/rust/src/builtin_tests/containers/callable_test.rs @@ -67,13 +67,19 @@ fn callable_hash() { #[itest] fn callable_object_method() { - let obj = CallableTestObj::new_gd(); - let callable = obj.callable("foo"); + let object = CallableTestObj::new_gd(); + let object_id = object.instance_id(); + let callable = object.callable("foo"); - assert_eq!(callable.object(), Some(obj.clone().upcast::())); - assert_eq!(callable.object_id(), Some(obj.instance_id())); + assert_eq!(callable.object(), Some(object.clone().upcast::())); + assert_eq!(callable.object_id(), Some(object_id)); assert_eq!(callable.method_name(), Some("foo".into())); + // Invalidating the object still returns the old ID, however not the object. + drop(object); + assert_eq!(callable.object_id(), Some(object_id)); + assert_eq!(callable.object(), None); + assert_eq!(Callable::invalid().object(), None); assert_eq!(Callable::invalid().object_id(), None); assert_eq!(Callable::invalid().method_name(), None); diff --git a/itest/rust/src/builtin_tests/containers/signal_test.rs b/itest/rust/src/builtin_tests/containers/signal_test.rs index 19dcb8440..d7c860747 100644 --- a/itest/rust/src/builtin_tests/containers/signal_test.rs +++ b/itest/rust/src/builtin_tests/containers/signal_test.rs @@ -95,6 +95,7 @@ fn signals() { #[itest] fn instantiate_signal() { let mut object = RefCounted::new_gd(); + let object_id = object.instance_id(); object.add_user_signal("test_signal".into()); @@ -102,8 +103,13 @@ fn instantiate_signal() { assert!(!signal.is_null()); assert_eq!(signal.name(), StringName::from("test_signal")); - assert_eq!(signal.object().unwrap(), object.clone().upcast()); - assert_eq!(signal.object_id().unwrap(), object.instance_id()); + assert_eq!(signal.object(), Some(object.clone().upcast())); + assert_eq!(signal.object_id(), Some(object_id)); + + // Invalidating the object still returns the old ID, however not the object. + drop(object); + assert_eq!(signal.object_id(), Some(object_id)); + assert_eq!(signal.object(), None); } #[itest]