Skip to content

Commit

Permalink
Merge pull request #901 from godot-rust/qol/deprecate-instance-utilities
Browse files Browse the repository at this point in the history
Deprecate instance utilities in `godot::global`
  • Loading branch information
Bromeon committed Sep 17, 2024
2 parents e47936e + ece7360 commit 671fcb5
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 2 deletions.
25 changes: 25 additions & 0 deletions godot-core/src/global/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,30 @@ 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/892."]
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/892."]
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/892."]
pub fn is_instance_id_valid(instance_id: i64) -> bool {
crate::gen::utilities::is_instance_id_valid(instance_id)
}
3 changes: 3 additions & 0 deletions godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,12 @@ impl<T: GodotClass> Gd<T> {

/// ⚠️ Looks up the given instance ID and returns the associated object.
///
/// Corresponds to Godot's global function `instance_from_id()`.
///
/// # Panics
/// If no such instance ID is registered, or if the dynamic type of the object behind that instance ID
/// is not compatible with `T`.
#[doc(alias = "instance_from_id")]
pub fn from_instance_id(instance_id: InstanceId) -> Self {
Self::try_from_instance_id(instance_id).unwrap_or_else(|err| {
panic!(
Expand Down
11 changes: 11 additions & 0 deletions godot-core/src/obj/instance_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ impl InstanceId {
self.to_u64() & (1u64 << 63) != 0
}

/// Dynamically checks if the instance behind the ID exists.
///
/// Rather slow, involves engine round-trip plus object DB lookup. If you need the object, use
/// [`Gd::from_instance_id()`][crate::obj::Gd::from_instance_id] instead.
///
/// 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())
}

// Private: see rationale above
pub(crate) fn to_u64(self) -> u64 {
self.value.get()
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/obj/raw_gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::obj::bounds::{Declarer, DynMemory as _};
use crate::obj::rtti::ObjectRtti;
use crate::obj::{bounds, Bounds, GdDerefTarget, GdMut, GdRef, GodotClass, InstanceId};
use crate::storage::{InstanceCache, InstanceStorage, Storage};
use crate::{classes, global, out};
use crate::{classes, out};

/// Low-level bindings for object pointers in Godot.
///
Expand Down Expand Up @@ -112,7 +112,7 @@ impl<T: GodotClass> RawGd<T> {
pub(crate) fn is_instance_valid(&self) -> bool {
self.cached_rtti
.as_ref()
.map(|rtti| global::is_instance_id_valid(rtti.instance_id().to_i64()))
.map(|rtti| rtti.instance_id().lookup_validity())
.unwrap_or(false)
}

Expand Down
3 changes: 3 additions & 0 deletions itest/rust/src/engine_tests/utilities_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

// TODO remove once instance_from_id() etc are removed.
#![allow(deprecated)]

use crate::framework::itest;

use godot::builtin::{GString, Variant};
Expand Down
3 changes: 3 additions & 0 deletions itest/rust/src/object_tests/object_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use godot::classes::{
file_access, Area2D, Camera3D, Engine, FileAccess, IRefCounted, Node, 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};
Expand Down Expand Up @@ -171,6 +172,7 @@ fn object_instance_from_id() {

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>();
Expand All @@ -182,6 +184,7 @@ fn object_instance_from_id() {

#[itest]
fn object_instance_from_invalid_id() {
#[allow(deprecated)]
let gd_from_instance_id = instance_from_id(0);

assert!(
Expand Down

0 comments on commit 671fcb5

Please sign in to comment.