Skip to content

Add WeakGd #1280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
9 changes: 9 additions & 0 deletions godot-core/src/obj/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use std::mem::ManuallyDrop;
use std::rc::Rc;

use crate::builtin::{Callable, Variant};
use crate::obj::weak_gd::WeakGd;
use crate::obj::{bounds, Gd, GodotClass, InstanceId};
use crate::{classes, sys};

Expand Down Expand Up @@ -166,6 +167,10 @@ impl<T: GodotClass> Base<T> {
(*self.obj).clone()
}

pub fn to_weak_gd(&self) -> WeakGd<T> {
WeakGd::from_base(self)
}

/// Returns a [`Gd`] referencing the base object, for exclusive use during object initialization.
///
/// Can be used during an initialization function [`I*::init()`][crate::classes::IObject::init] or [`Gd::from_init_fn()`].
Expand Down Expand Up @@ -328,6 +333,10 @@ impl<T: GodotClass> Base<T> {

(*self.obj).clone()
}

pub(crate) fn is_instance_valid(&self) -> bool {
self.obj.is_instance_valid()
}
}

impl<T: GodotClass> Debug for Base<T> {
Expand Down
2 changes: 2 additions & 0 deletions godot-core/src/obj/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ mod on_editor;
mod on_ready;
mod raw_gd;
mod traits;
mod weak_gd;

pub(crate) mod rtti;

Expand All @@ -35,6 +36,7 @@ pub use on_editor::*;
pub use on_ready::*;
pub use raw_gd::*;
pub use traits::*;
pub use weak_gd::*;

pub mod bounds;
pub mod script;
Expand Down
21 changes: 0 additions & 21 deletions godot-core/src/obj/raw_gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,27 +358,6 @@ impl<T: GodotClass> RawGd<T> {
// Validation object identity.
self.check_rtti("upcast_ref");
debug_assert!(!self.is_null(), "cannot upcast null object refs");

// In Debug builds, go the long path via Godot FFI to verify the results are the same.
#[cfg(debug_assertions)]
{
// SAFETY: we forget the object below and do not leave the function before.
let ffi_dest = self.ffi_cast::<Base>().expect("failed FFI upcast");

// The ID check is not that expressive; we should do a complete comparison of the ObjectRtti, but currently the dynamic types can
// be different (see comment in ObjectRtti struct). This at least checks that the transmuted object is not complete garbage.
// We get direct_id from Self and not Base because the latter has no API with current bounds; but this equivalence is tested in Deref.
let direct_id = self.instance_id_unchecked().expect("direct_id null");
let ffi_id = ffi_dest
.as_dest_ref()
.instance_id_unchecked()
.expect("ffi_id null");

assert_eq!(
direct_id, ffi_id,
"upcast_ref: direct and FFI IDs differ. This is a bug, please report to godot-rust maintainers."
);
}
}

/// Verify that the object is non-null and alive. In Debug mode, additionally verify that it is of type `T` or derived.
Expand Down
4 changes: 3 additions & 1 deletion godot-core/src/obj/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::builtin::GString;
use crate::init::InitLevel;
use crate::meta::inspect::EnumConstant;
use crate::meta::ClassName;
use crate::obj::{bounds, Base, BaseMut, BaseRef, Bounds, Gd};
use crate::obj::{bounds, Base, BaseMut, BaseRef, Bounds, Gd, WeakGd};
#[cfg(since_api = "4.2")]
use crate::registry::signal::SignalObject;
use crate::storage::Storage;
Expand Down Expand Up @@ -364,6 +364,8 @@ pub trait WithBaseField: GodotClass + Bounds<Declarer = bounds::DeclUser> {
#[doc(hidden)]
fn base_field(&self) -> &Base<Self::Base>;

fn to_weak_gd(&self) -> WeakGd<Self>;

/// Returns a shared reference suitable for calling engine methods on this object.
///
/// Holding a shared guard prevents other code paths from obtaining a _mutable_ reference to `self`, as such it is recommended to drop the
Expand Down
119 changes: 119 additions & 0 deletions godot-core/src/obj/weak_gd.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* 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 std::fmt::{Debug, Formatter, Result as FmtResult};
use std::mem::ManuallyDrop;
use std::ops::{Deref, DerefMut};

use godot_ffi as sys;

use crate::classes;
use crate::obj::{bounds, Base, Bounds, Gd, GdDerefTarget, GodotClass, RawGd};

/// Weak pointer to objects owned by the Godot engine.
/// `WeakGd<T>` doesn't guarantee the validation of the object and can hold a dead object.
/// For `RefCounted`, it doesn't affect the reference count, which means it doesn't decrease reference count when dropped and doesn't prevent the `RefCounted` from being released.
/// Can be used during initialization [`I*::init()`][crate::classes::IObject::init] or [`Gd::from_init_fn()`], and deconstruction [Drop].
///
/// # Panics
/// If the weak pointer is invalid when dereferencing to call a Godot method.
pub struct WeakGd<T: GodotClass> {
raw: Option<ManuallyDrop<RawGd<T>>>,
}

impl<T: GodotClass> WeakGd<T> {
#[doc(hidden)]
pub unsafe fn from_obj_sys_weak(val: sys::GDExtensionObjectPtr) -> Self {
Self {
raw: if !val.is_null() {
Some(ManuallyDrop::new(RawGd::from_obj_sys_weak(val)))
} else {
None
},
}
}

pub(crate) fn from_raw_gd(val: &RawGd<T>) -> Self {
unsafe {
Self::from_obj_sys_weak(if val.is_instance_valid() {
val.obj_sys()
} else {
std::ptr::null_mut()
})
}
}

/// Create a weak pointer from a [Gd].
pub fn from_gd(val: &Gd<T>) -> Self {
Self::from_raw_gd(&val.raw)
}

/// Create a weak pointer from a [Base].
pub fn from_base(val: &Base<T>) -> Self {
Self {
raw: if val.is_instance_valid() {
unsafe { Some(ManuallyDrop::new(RawGd::from_obj_sys_weak(val.obj_sys()))) }
} else {
None
},
}
}

/// Checks if this weak pointer points to a live object.
pub fn is_instance_valid(&self) -> bool {
self.raw
.as_ref()
.map(|v| v.is_instance_valid())
.unwrap_or(false)
}
}

impl<T: GodotClass> Deref for WeakGd<T>
where
GdDerefTarget<T>: Bounds<Declarer = bounds::DeclEngine>,
{
type Target = GdDerefTarget<T>;

fn deref(&self) -> &Self::Target {
self.raw
.as_ref()
.expect("WeakGd points to an invalid instance")
.as_target()
}
}

impl<T: GodotClass> DerefMut for WeakGd<T>
where
GdDerefTarget<T>: Bounds<Declarer = bounds::DeclEngine>,
{
fn deref_mut(&mut self) -> &mut Self::Target {
self.raw
.as_mut()
.expect("WeakGd points to an invalid instance")
.as_target_mut()
}
}

impl<T: GodotClass> Clone for WeakGd<T> {
fn clone(&self) -> Self {
if let Some(raw) = self.raw.as_ref() {
Self::from_raw_gd(raw)
} else {
Self { raw: None }
}
}
}

impl<T: GodotClass> Debug for WeakGd<T> {
fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
if self.is_instance_valid() {
classes::debug_string_nullable(self.raw.as_ref().unwrap(), f, "WeakGd")
} else {
write!(f, "WeakGd {{ null }}")
}
}
}
8 changes: 8 additions & 0 deletions godot-macros/src/class/derive_godot_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
fn base_field(&self) -> &::godot::obj::Base<<#class_name as ::godot::obj::GodotClass>::Base> {
&self.#name
}

fn to_weak_gd(&self) -> ::godot::obj::WeakGd<#class_name> {
let base = <#class_name as ::godot::obj::WithBaseField>::base_field(self);
unsafe {
// Force casting `Base` to `T`.
::godot::obj::WeakGd::<#class_name>::from_obj_sys_weak(base.obj_sys())
}
}
}
}
} else {
Expand Down
1 change: 1 addition & 0 deletions itest/rust/src/object_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ mod base_init_test;
mod validate_property_test;
mod virtual_methods_niche_test;
mod virtual_methods_test;
mod weak_gd_test;

// Need to test this in the init level method.
pub use init_level_test::initialize_init_level_test;
50 changes: 50 additions & 0 deletions itest/rust/src/object_tests/weak_gd_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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 godot::obj::WeakGd;
use godot::prelude::*;

use crate::framework::{expect_panic, itest};

#[derive(GodotClass, Debug)]
#[class(base=RefCounted)]
struct RefcBasedDrop {
pub base: Base<RefCounted>,
}

#[godot_api]
impl IRefCounted for RefcBasedDrop {
fn init(base: Base<RefCounted>) -> Self {
let mut obj = base.to_weak_gd();
obj.set_meta("meta", &"inited".to_variant());
assert_eq!(obj.get_reference_count(), 1);
Self { base }
}
}

impl Drop for RefcBasedDrop {
fn drop(&mut self) {
let obj = self.to_weak_gd();
assert_eq!(obj.get_meta("meta"), "inited".to_variant());

// FIXME: Accessing godot methods except Object is UB.
// assert_eq!(obj.get_reference_count(), 0);
}
}

#[itest]
fn weak_gd_init_drop_refcounted() {
let obj = RefcBasedDrop::new_gd();
let weak = WeakGd::from_gd(&obj);
drop(obj);
expect_panic(
"WeakGd calling Godot method with dead object should panic",
|| {
weak.get_reference_count();
},
);
}
Loading