Utilities around instance ID: remove from public API + document

Author: Bromeon

godot-codegen/src/models/domain_mapping.rs

@@ -546,6 +546,7 @@ impl UtilityFunction {
         if special_cases::is_utility_function_deleted(function, ctx) {
             return None;
         }
+        let is_private = special_cases::is_utility_function_private(function);
 
         // Some vararg functions like print() or str() are declared with a single argument "arg1: Variant", but that seems
         // to be a mistake. We change their parameter list by removing that.
@@ -571,7 +572,7 @@ impl UtilityFunction {
                 parameters,
                 return_value: FnReturn::new(&return_value, ctx),
                 is_vararg: function.is_vararg,
-                is_private: false,
+                is_private,
                 is_virtual_required: false,
                 direction: FnDirection::Outbound {
                     hash: function.hash,

godot-codegen/src/special_cases/special_cases.rs

@@ -559,13 +559,25 @@ pub fn is_builtin_type_scalar(name: &str) -> bool {
 
 #[rustfmt::skip]
 pub fn is_utility_function_deleted(function: &JsonUtilityFunction, ctx: &mut Context) -> bool {
-    /*let hardcoded = match function.name.as_str() {
-        | "..."
+    let hardcoded = match function.name.as_str() {
+        // Removed in v0.3, but available as dedicated APIs.
+        | "instance_from_id"
 
         => true, _ => false
     };
 
-    hardcoded ||*/ codegen_special_cases::is_utility_function_excluded(function, ctx)
+    hardcoded || codegen_special_cases::is_utility_function_excluded(function, ctx)
+}
+
+#[rustfmt::skip]
+pub fn is_utility_function_private(function: &JsonUtilityFunction) -> bool {
+    match function.name.as_str() {
+        // Removed from public interface in v0.3, but available as dedicated APIs.
+        | "is_instance_valid"    // used in Variant::is_object_alive().
+        | "is_instance_id_valid" // used in InstanceId::lookup_validity().
+
+        => true, _ => false
+    }
 }
 
 pub fn maybe_rename_class_method<'m>(class_name: &TyName, godot_method_name: &'m str) -> &'m str {

godot-core/src/builtin/variant/mod.rs

@@ -274,7 +274,7 @@ impl Variant {
     pub(crate) fn is_object_alive(&self) -> bool {
         debug_assert_eq!(self.get_type(), VariantType::OBJECT);
 
-        crate::gen::utilities::is_instance_valid(self)
+        crate::global::is_instance_valid(self)
 
         // In case there are ever problems with this approach, alternative implementation:
         // self.stringify() != "<Freed Object>".into()

godot-core/src/global/mod.rs

@@ -21,6 +21,24 @@
 //! - Variant: [`VariantType`][crate::builtin::VariantType], [`VariantOperator`][crate::builtin::VariantOperator]
 //! - Vector: [`Vector2Axis`][crate::builtin::Vector2Axis], [`Vector3Axis`][crate::builtin::Vector3Axis], [`Vector4Axis`][crate::builtin::Vector4Axis]
 //!
+//! # Functions moved to dedicated APIs
+//!
+//! Some methods in `@GlobalScope` are not directly available in `godot::global` module, but rather in their related types.  \
+//! You can find them as follows:
+//!
+//! | Godot utility function | godot-rust APIs                                                                                                                      |
+//! |------------------------|--------------------------------------------------------------------------------------------------------------------------------------|
+//! | `instance_from_id`     | [`Gd::from_instance_id()`][crate::obj::Gd::from_instance_id]<br>[`Gd::try_from_instance_id()`][crate::obj::Gd::try_from_instance_id()] |
+//! | `is_instance_valid`    | [`Gd::is_instance_valid()`][crate::obj::Gd::is_instance_valid()]                                                                     |
+//! | `is_instance_id_valid` | [`InstanceId::lookup_validity()`][crate::obj::InstanceId::lookup_validity()]                                                         |
+//!
+
+// Doc aliases are also available in dedicated APIs, but directing people here may give them a bit more context.
+#![doc(
+    alias = "instance_from_id",
+    alias = "is_instance_valid",
+    alias = "is_instance_id_valid"
+)]
 
 mod print;
 
@@ -31,30 +49,8 @@ pub use crate::gen::central::global_enums::*;
 pub use crate::gen::utilities::*;
 
 // This is needed for generated classes to find symbols, even those that have been moved to crate::builtin.
-use crate::builtin::Variant;
 #[allow(unused_imports)] // micromanaging imports for generated code is not fun
 pub(crate) use crate::builtin::{Corner, EulerOrder, Side};
-use crate::obj::Gd;
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Deprecations
-
-// Reminder: remove #![allow(deprecated)] in utilities.test along with the below functions.
-
-#[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
-    For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
-pub fn instance_from_id(instance_id: i64) -> Option<Gd<crate::classes::Object>> {
-    crate::gen::utilities::instance_from_id(instance_id)
-}
-
-#[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
-    For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
-pub fn is_instance_valid(instance: Variant) -> bool {
-    crate::gen::utilities::is_instance_valid(&instance)
-}
-
-#[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
-    For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
-pub fn is_instance_id_valid(instance_id: i64) -> bool {
-    crate::gen::utilities::is_instance_id_valid(instance_id)
-}

godot-core/src/obj/instance_id.rs

@@ -66,7 +66,7 @@ impl InstanceId {
     /// This corresponds to Godot's global function `is_instance_id_valid()`.
     #[doc(alias = "is_instance_id_valid")]
     pub fn lookup_validity(self) -> bool {
-        crate::gen::utilities::is_instance_id_valid(self.to_i64())
+        crate::global::is_instance_id_valid(self.to_i64())
     }
 
     // Private: see rationale above

itest/rust/src/engine_tests/utilities_test.rs

@@ -5,15 +5,13 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-// TODO remove once instance_from_id() etc are removed.
-#![allow(deprecated)]
+// Note: in the past, we had is_instance_valid() -- we also tested that godot-rust is not susceptible to the godot-cpp issue
+// https://github.com/godotengine/godot-cpp/issues/1390.
 
 use crate::framework::itest;
 
 use godot::builtin::{GString, Variant};
-use godot::classes::Node3D;
 use godot::global::*;
-use godot::obj::NewAlloc;
 
 #[itest]
 fn utilities_abs() {
@@ -80,14 +78,3 @@ fn utilities_max() {
     );
     assert_eq!(output, Variant::from(-1.0));
 }
-
-// Checks that godot-rust is not susceptible to the godot-cpp issue https://github.com/godotengine/godot-cpp/issues/1390.
-#[itest]
-fn utilities_is_instance_valid() {
-    let node = Node3D::new_alloc();
-    let variant = Variant::from(node.clone());
-    assert!(is_instance_valid(variant.clone()));
-
-    node.free();
-    assert!(!is_instance_valid(variant));
-}

itest/rust/src/object_tests/object_test.rs

@@ -16,7 +16,6 @@ use godot::classes::{
     file_access, Engine, FileAccess, IRefCounted, Node, Node2D, Node3D, Object, RefCounted,
 };
 #[allow(deprecated)]
-use godot::global::instance_from_id;
 use godot::meta::{FromGodot, GodotType, ToGodot};
 use godot::obj::{Base, Gd, Inherits, InstanceId, NewAlloc, NewGd, RawGd};
 use godot::register::{godot_api, GodotClass};
@@ -181,36 +180,6 @@ fn object_from_invalid_instance_id() {
         .expect_err("invalid instance id should not return a valid object");
 }
 
-// `instance_from_id` is a normal FFI call, so works slightly differently from `Gd::try_from_instance_id`.
-#[itest]
-fn object_instance_from_id() {
-    let node = Node::new_alloc();
-
-    assert!(node.is_instance_valid());
-
-    let instance_id = node.instance_id();
-
-    #[allow(deprecated)]
-    let gd_from_instance_id = instance_from_id(instance_id.to_i64())
-        .expect("instance should be valid")
-        .cast::<Node>();
-
-    assert_eq!(gd_from_instance_id, node);
-
-    node.free();
-}
-
-#[itest]
-fn object_instance_from_invalid_id() {
-    #[allow(deprecated)]
-    let gd_from_instance_id = instance_from_id(0);
-
-    assert!(
-        gd_from_instance_id.is_none(),
-        "instance id 0 should never be valid"
-    );
-}
-
 #[itest]
 fn object_from_instance_id_inherits_type() {
     let descr = GString::from("some very long description");
@@ -283,7 +252,14 @@ fn object_user_free_during_bind() {
         obj.is_instance_valid(),
         "object lives on after failed free()"
     );
+
+    let copy = obj.clone();
     obj.free(); // now succeeds
+
+    assert!(
+        !copy.is_instance_valid(),
+        "object is finally destroyed after successful free()"
+    );
 }
 
 #[itest]