Skip to content

Commit b5b4df5

Browse files
committed
Base<T>: remove as_gd(); improve safety docs
1 parent 6c92a05 commit b5b4df5

File tree

1 file changed

+26
-14
lines changed

1 file changed

+26
-14
lines changed

godot-core/src/obj/base.rs

+26-14
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ use std::mem::ManuallyDrop;
1414
///
1515
/// Behaves similarly to [`Gd`][crate::obj::Gd], but is more constrained. Cannot be constructed by the user.
1616
pub struct Base<T: GodotClass> {
17+
// Like `Gd`, it's theoretically possible that Base is destroyed while there are still other Gd pointers to the underlying object. This is
18+
// safe, may however lead to unintended behavior. The base_test.rs file checks some of these scenarios.
19+
1720
// Internal smart pointer is never dropped. It thus acts like a weak pointer and is needed to break reference cycles between Gd<T>
1821
// and the user instance owned by InstanceStorage.
1922
//
@@ -34,19 +37,37 @@ pub struct Base<T: GodotClass> {
3437
}
3538

3639
impl<T: GodotClass> Base<T> {
40+
/// "Copy constructor": allows to share a `Base<T>` weak pointer.
41+
///
42+
/// The return value is a weak pointer, so it will not keep the instance alive.
43+
///
3744
/// # Safety
38-
/// The returned Base is a weak pointer, so holding it will not keep the object alive. It must not be accessed after the object is destroyed.
45+
/// `base` must be alive at the time of invocation, i.e. user `init()` (which could technically destroy it) must not have run yet.
46+
/// If `base` is destroyed while the returned `Base<T>` is in use, that constitutes a logic error, not a safety issue.
3947
pub(crate) unsafe fn from_base(base: &Base<T>) -> Base<T> {
40-
Base::from_obj(Gd::from_obj_sys_weak(base.as_gd().obj_sys()))
48+
debug_assert!(base.obj.is_instance_valid());
49+
Base::from_obj(Gd::from_obj_sys_weak(base.obj.obj_sys()))
4150
}
4251

52+
/// Create base from existing object (used in script instances).
53+
///
54+
/// The return value is a weak pointer, so it will not keep the instance alive.
55+
///
4356
/// # Safety
44-
/// The returned Base is a weak pointer, so holding it will not keep the object alive. It must not be accessed after the object is destroyed.
57+
/// `gd` must be alive at the time of invocation. If it is destroyed while the returned `Base<T>` is in use, that constitutes a logic
58+
/// error, not a safety issue.
4559
pub(crate) unsafe fn from_gd(gd: &Gd<T>) -> Self {
60+
debug_assert!(gd.is_instance_valid());
4661
Base::from_obj(Gd::from_obj_sys_weak(gd.obj_sys()))
4762
}
4863

49-
// Note: not &mut self, to only borrow one field and not the entire struct
64+
/// Create new base from raw Godot object.
65+
///
66+
/// The return value is a weak pointer, so it will not keep the instance alive.
67+
///
68+
/// # Safety
69+
/// `base_ptr` must point to a valid, live object at the time of invocation. If it is destroyed while the returned `Base<T>` is in use,
70+
/// that constitutes a logic error, not a safety issue.
5071
pub(crate) unsafe fn from_sys(base_ptr: sys::GDExtensionObjectPtr) -> Self {
5172
assert!(!base_ptr.is_null(), "instance base is null pointer");
5273

@@ -76,16 +97,7 @@ impl<T: GodotClass> Base<T> {
7697
(*self.obj).clone()
7798
}
7899

79-
/// Returns a [`Gd`] referencing the same object as this reference.
80-
///
81-
/// Using this method to call methods on the base field of a Rust object is discouraged, instead use the
82-
/// methods from [`WithBaseField`](super::WithBaseField) when possible.
83-
#[doc(hidden)]
84-
pub fn as_gd(&self) -> &Gd<T> {
85-
&self.obj
86-
}
87-
88-
// Currently only used in outbound virtual calls (for scripts).
100+
// Currently only used in outbound virtual calls (for scripts); search for: base_field(self).obj_sys().
89101
#[doc(hidden)]
90102
pub fn obj_sys(&self) -> sys::GDExtensionObjectPtr {
91103
self.obj.obj_sys()

0 commit comments

Comments
 (0)