Skip to content

Commit

Permalink
Docs + cleanup
Browse files Browse the repository at this point in the history
Rename dbind() -> dyn_bind().
DynGd::from_gd() should now be used as gd.into_dyn().
  • Loading branch information
Bromeon committed Nov 24, 2024
1 parent 8176c01 commit d3516b8
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 50 deletions.
22 changes: 21 additions & 1 deletion godot-core/src/meta/args/object_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use crate::builtin::Variant;
use crate::meta::error::ConvertError;
use crate::meta::{ClassName, FromGodot, GodotConvert, GodotFfiVariant, GodotType, ToGodot};
use crate::obj::{bounds, Bounds, Gd, GodotClass, Inherits, RawGd};
use crate::obj::{bounds, Bounds, DynGd, Gd, GodotClass, Inherits, RawGd};
use crate::{obj, sys};
use godot_ffi::{GodotFfi, GodotNullableFfi, PtrcallType};
use std::ptr;
Expand Down Expand Up @@ -81,6 +81,7 @@ where
}
}
*/

impl<T, U> AsObjectArg<T> for &Gd<U>
where
T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
Expand All @@ -97,6 +98,25 @@ where
}
}

impl<T, U, D> AsObjectArg<T> for &DynGd<U, D>
where
T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
U: Inherits<T>,
D: ?Sized,
{
fn as_object_arg(&self) -> ObjectArg<T> {
// Reuse Deref.
let gd: &Gd<U> = self;
<&Gd<U>>::as_object_arg(&gd)
}

fn consume_arg(self) -> ObjectCow<T> {
// Reuse Deref.
let gd: &Gd<U> = self;
<&Gd<U>>::consume_arg(gd)
}
}

impl<T, U> AsObjectArg<T> for Option<U>
where
T: GodotClass + Bounds<Declarer = bounds::DeclEngine>,
Expand Down
38 changes: 22 additions & 16 deletions godot-core/src/obj/dyn_gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ use std::ops;
/// To register the `T` -> `D` relation with godot-rust, `T` must implement [`AsDyn<D>`]. This can be automated with the
/// [`#[godot_dyn]`](../register/attr.godot_dyn.html) attribute macro.
///
/// # Public API
/// The API is very close to `Gd`. In fact, both `Deref` and `DerefMut` are implemented for `DynGd` -> `Gd`, so you can access all the
/// # Construction and API
/// You can convert between `Gd` and `DynGd` using [`Gd::into_dyn()`] and [`DynGd::into_gd()`]. The former sometimes needs an explicit
/// `::<dyn Trait>` type argument, but can often be inferred.
///
/// The `DynGd` API is very close to `Gd`. In fact, both `Deref` and `DerefMut` are implemented for `DynGd` -> `Gd`, so you can access all the
/// underlying `Gd` methods as well as Godot class APIs directly.
///
/// The main new parts are two methods [`dbind()`][Self::dbind] and [`dbind_mut()`][Self::dbind_mut]. These are very similar to `Gd`'s
/// The main new parts are two methods [`dyn_bind()`][Self::dyn_bind] and [`dyn_bind_mut()`][Self::dyn_bind_mut]. These are very similar to `Gd`'s
/// [`bind()`][Gd::bind] and [`bind_mut()`][Gd::bind_mut], but return a reference guard to the trait object `D` instead of the Godot class `T`.
///
/// # Example
Expand Down Expand Up @@ -66,11 +69,11 @@ use std::ops;
/// let mut dyn_monster = dyn_monster.upcast::<RefCounted>();
///
/// // Due to RefCounted abstraction, you can no longer access concrete Monster properties.
/// // However, the trait Health is still accessible through dbind().
/// assert!(dyn_monster.dbind().is_alive());
/// // However, the trait Health is still accessible through dyn_bind().
/// assert!(dyn_monster.dyn_bind().is_alive());
///
/// // To mutate the object, call dbind_mut(). Rust borrow rules apply.
/// let mut guard = dyn_monster.dbind_mut();
/// // To mutate the object, call dyn_bind_mut(). Rust borrow rules apply.
/// let mut guard = dyn_monster.dyn_bind_mut();
/// guard.deal_damage(120);
/// assert!(!guard.is_alive());
/// ```
Expand All @@ -90,7 +93,7 @@ where
T: AsDyn<D> + Bounds<Declarer = bounds::DeclUser>,
D: ?Sized,
{
pub fn from_gd(gd_instance: Gd<T>) -> Self {
pub(crate) fn from_gd(gd_instance: Gd<T>) -> Self {
let erased_obj = Box::new(gd_instance.clone());

Self {
Expand All @@ -111,17 +114,17 @@ where
/// The resulting guard implements `Deref<Target = D>`, allowing shared access to the trait's methods.
///
/// See [`Gd::bind()`][Gd::bind] for borrow checking semantics and panics.
pub fn dbind(&self) -> DynGdRef<D> {
self.erased_obj.dbind()
pub fn dyn_bind(&self) -> DynGdRef<D> {
self.erased_obj.dyn_bind()
}

/// Acquires an exclusive reference guard to the trait object `D`.
///
/// The resulting guard implements `DerefMut<Target = D>`, allowing exclusive mutable access to the trait's methods.
///
/// See [`Gd::bind_mut()`][Gd::bind_mut] for borrow checking semantics and panics.
pub fn dbind_mut(&mut self) -> DynGdMut<D> {
self.erased_obj.dbind_mut()
pub fn dyn_bind_mut(&mut self) -> DynGdMut<D> {
self.erased_obj.dyn_bind_mut()
}

// Certain methods "overridden" from deref'ed Gd here, so they're more idiomatic to use.
Expand Down Expand Up @@ -201,8 +204,8 @@ where
// Type erasure

trait ErasedGd<D: ?Sized> {
fn dbind(&self) -> DynGdRef<D>;
fn dbind_mut(&mut self) -> DynGdMut<D>;
fn dyn_bind(&self) -> DynGdRef<D>;
fn dyn_bind_mut(&mut self) -> DynGdMut<D>;

fn clone_box(&self) -> Box<dyn ErasedGd<D>>;
}
Expand All @@ -212,15 +215,18 @@ where
T: AsDyn<D> + Bounds<Declarer = bounds::DeclUser>,
D: ?Sized,
{
fn dbind(&self) -> DynGdRef<D> {
fn dyn_bind(&self) -> DynGdRef<D> {
DynGdRef::from_guard::<T>(Gd::bind(self))
}

fn dbind_mut(&mut self) -> DynGdMut<D> {
fn dyn_bind_mut(&mut self) -> DynGdMut<D> {
DynGdMut::from_guard::<T>(Gd::bind_mut(self))
}

fn clone_box(&self) -> Box<dyn ErasedGd<D>> {
Box::new(Gd::clone(self))
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
// Integration with Godot traits
11 changes: 10 additions & 1 deletion godot-core/src/obj/gd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,11 @@ impl<T: GodotClass> Gd<T> {
}
}

/// Upgrades to a `DynGd<T, D>` pointer, enabling the `D` abstraction.
///
/// The `D` parameter can typically be inferred when there is a single `AsDyn<...>` implementation for `T`. \
/// Otherwise, use it as `gd.into_dyn::<dyn MyTrait>()`.
#[must_use]
pub fn into_dyn<D>(self) -> DynGd<T, D>
where
T: crate::obj::AsDyn<D> + Bounds<Declarer = bounds::DeclUser>,
Expand Down Expand Up @@ -702,15 +707,18 @@ impl<T: GodotClass> FromGodot for Gd<T> {
}

impl<T: GodotClass> GodotType for Gd<T> {
// Some #[doc(hidden)] are repeated despite already declared in trait; some IDEs suggest in auto-complete otherwise.
type Ffi = RawGd<T>;

type ToFfi<'f> = RefArg<'f, RawGd<T>>
where Self: 'f;

#[doc(hidden)]
fn to_ffi(&self) -> Self::ToFfi<'_> {
RefArg::new(&self.raw)
}

#[doc(hidden)]
fn into_ffi(self) -> Self::Ffi {
self.raw
}
Expand All @@ -723,7 +731,7 @@ impl<T: GodotClass> GodotType for Gd<T> {
}
}

fn class_name() -> crate::meta::ClassName {
fn class_name() -> ClassName {
T::class_name()
}

Expand Down Expand Up @@ -781,6 +789,7 @@ impl<T: GodotClass> ArrayElement for Option<Gd<T>> {
}

impl<'r, T: GodotClass> AsArg<Gd<T>> for &'r Gd<T> {
#[doc(hidden)] // Repeated despite already hidden in trait; some IDEs suggest this otherwise.
fn into_arg<'cow>(self) -> CowArg<'cow, Gd<T>>
where
'r: 'cow, // Original reference must be valid for at least as long as the returned cow.
Expand Down
4 changes: 2 additions & 2 deletions godot-core/src/obj/guards.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl<'a, T: GodotClass> ErasedGuard<'a> for GdMut<'a, T> {}

/// Shared reference guard for a [`DynGd`][crate::obj::DynGd] smart pointer.
///
/// Returned by [`DynGd::dbind()`][crate::obj::DynGd::dbind].
/// Returned by [`DynGd::dyn_bind()`][crate::obj::DynGd::dyn_bind].
pub struct DynGdRef<'a, D: ?Sized> {
/// Never accessed, but is kept alive to ensure dynamic borrow checks are upheld and the object isn't freed.
_guard: Box<dyn ErasedGuard<'a>>,
Expand Down Expand Up @@ -139,7 +139,7 @@ impl<D: ?Sized> Drop for DynGdRef<'_, D> {

/// Mutably/exclusively bound reference guard for a [`DynGd`][crate::obj::DynGd] smart pointer.
///
/// Returned by [`DynGd::dbind_mut()`][crate::obj::DynGd::dbind_mut].
/// Returned by [`DynGd::dyn_bind_mut()`][crate::obj::DynGd::dyn_bind_mut].
pub struct DynGdMut<'a, D: ?Sized> {
/// Never accessed, but is kept alive to ensure dynamic borrow checks are upheld and the object isn't freed.
_guard: Box<dyn ErasedGuard<'a>>,
Expand Down
57 changes: 27 additions & 30 deletions itest/rust/src/object_tests/dyn_gd_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,35 @@
* 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::framework::{expect_panic, itest};
// Test that all important dyn-related symbols are in the prelude.
use godot::prelude::*;

#[itest]
fn dyn_gd_creation_bind() {
// Type inference on this works because only 1 AsDyn<...> trait is implemented for RefcHealth. It would fail with another.
let _unused = DynGd::from_gd(Gd::from_object(RefcHealth { hp: 1 }));
let _unused = Gd::from_object(RefcHealth { hp: 1 }).into_dyn();

let user_obj = RefcHealth { hp: 34 };
let mut dyn_gd = DynGd::from_gd(Gd::from_object(user_obj));
let mut dyn_gd = Gd::from_object(user_obj).into_dyn();

{
// Exclusive bind.
// Interesting: this can be type inferred because RefcHealth implements only 1 AsDyn<...> trait.
// If there were another, type inference would fail.
let mut health = dyn_gd.dbind_mut();
let mut health = dyn_gd.dyn_bind_mut();
health.deal_damage(4);
}
{
// Test multiple shared binds.
let health_a = dyn_gd.dbind();
let health_b = dyn_gd.dbind();
let health_a = dyn_gd.dyn_bind();
let health_b = dyn_gd.dyn_bind();

assert_eq!(health_b.get_hitpoints(), 30);
assert_eq!(health_a.get_hitpoints(), 30);
}
{
let mut health = dyn_gd.dbind_mut();
let mut health = dyn_gd.dyn_bind_mut();
health.kill();

assert_eq!(health.get_hitpoints(), 0);
Expand All @@ -45,15 +44,13 @@ fn dyn_gd_creation_deref() {
let node = foreign::NodeHealth::new_alloc();
let original_id = node.instance_id();

// let mut node = node.into_dyn::<dyn Health>();
// The first line only works because both type parameters are inferred as RefcHealth, and there's no `dyn Health`.
let mut node = DynGd::from_gd(node);
let mut node = node.into_dyn::<dyn Health>();

let dyn_id = node.instance_id();
assert_eq!(dyn_id, original_id);

deal_20_damage(&mut *node.dbind_mut());
assert_eq!(node.dbind().get_hitpoints(), 80);
deal_20_damage(&mut *node.dyn_bind_mut());
assert_eq!(node.dyn_bind().get_hitpoints(), 80);

node.free();
}
Expand All @@ -72,40 +69,40 @@ fn dyn_gd_upcast() {
let mut node = concrete.clone().upcast::<Node>();
let object = concrete.upcast::<Object>();

node.dbind_mut().deal_damage(25);
node.dyn_bind_mut().deal_damage(25);

// Make sure identity is intact.
assert_eq!(node.instance_id(), original_copy.instance_id());

// Ensure applied to the object polymorphically. Concrete object can access bind(), no dbind().
// Ensure applied to the object polymorphically. Concrete object can access bind(), no dyn_bind().
assert_eq!(original_copy.bind().get_hitpoints(), 75);

// Check also another angle (via Object). Here dbind().
assert_eq!(object.dbind().get_hitpoints(), 75);
// Check also another angle (via Object). Here dyn_bind().
assert_eq!(object.dyn_bind().get_hitpoints(), 75);

node.free();
}

#[itest]
fn dyn_gd_exclusive_guard() {
let mut a = DynGd::from_gd(foreign::NodeHealth::new_alloc());
let mut a = foreign::NodeHealth::new_alloc().into_dyn();
let mut b = a.clone();

let guard = a.dbind_mut();
let guard = a.dyn_bind_mut();

expect_panic(
"Cannot acquire dbind() guard while dbind_mut() is held",
"Cannot acquire dyn_bind() guard while dyn_bind_mut() is held",
|| {
let _ = b.dbind();
let _ = b.dyn_bind();
},
);
expect_panic(
"Cannot acquire 2nd dbind_mut() guard while dbind_mut() is held",
"Cannot acquire 2nd dyn_bind_mut() guard while dyn_bind_mut() is held",
|| {
let _ = b.dbind_mut();
let _ = b.dyn_bind_mut();
},
);
expect_panic("Cannot free object while dbind_mut() is held", || {
expect_panic("Cannot free object while dyn_bind_mut() is held", || {
b.free();
});

Expand All @@ -115,26 +112,26 @@ fn dyn_gd_exclusive_guard() {

#[itest]
fn dyn_gd_shared_guard() {
let a = DynGd::from_gd(foreign::NodeHealth::new_alloc());
let a = foreign::NodeHealth::new_alloc().into_dyn();
let b = a.clone();
let mut c = a.clone();

let guard_a = a.dbind();
let guard_a = a.dyn_bind();

// CAN acquire another dbind() while an existing one exists.
let guard_b = b.dbind();
// CAN acquire another dyn_bind() while an existing one exists.
let guard_b = b.dyn_bind();
drop(guard_a);

// guard_b still alive here.
expect_panic(
"Cannot acquire dbind_mut() guard while dbind() is held",
"Cannot acquire dyn_bind_mut() guard while dyn_bind() is held",
|| {
let _ = c.dbind_mut();
let _ = c.dyn_bind_mut();
},
);

// guard_b still alive here.
expect_panic("Cannot free object while dbind() is held", || {
expect_panic("Cannot free object while dyn_bind() is held", || {
c.free();
});

Expand Down

0 comments on commit d3516b8

Please sign in to comment.