- Cleanup docs in rawgd/gd

- Other cleanup

Author: lilizoey

godot-core/src/builtin/callable.rs

@@ -46,7 +46,8 @@ impl Callable {
         unsafe {
             sys::from_sys_init_or_init_default::<Self>(|self_ptr| {
                 let ctor = sys::builtin_fn!(callable_from_object_method);
-                let args = [object.to_ffi().as_arg_ptr(), method.sys_const()];
+                let raw = object.to_ffi();
+                let args = [raw.as_arg_ptr(), method.sys_const()];
                 ctor(self_ptr, args.as_ptr());
             })
         }

godot-core/src/builtin/meta/godot_compat/impls.rs

@@ -45,16 +45,16 @@ where
 }
 
 impl GodotCompatible for sys::VariantType {
-    type Via = i64;
+    type Via = i32;
 }
 
 impl ToGodot for sys::VariantType {
     fn to_godot(&self) -> Self::Via {
-        *self as i64
+        *self as i32
     }
 
     fn into_godot(self) -> Self::Via {
-        self as i64
+        self as i32
     }
 }
 
@@ -65,16 +65,16 @@ impl FromGodot for sys::VariantType {
 }
 
 impl GodotCompatible for sys::VariantOperator {
-    type Via = i64;
+    type Via = i32;
 }
 
 impl ToGodot for sys::VariantOperator {
     fn to_godot(&self) -> Self::Via {
-        *self as i64
+        *self as i32
     }
 
     fn into_godot(self) -> Self::Via {
-        self as i64
+        self as i32
     }
 }
 

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

@@ -183,11 +183,6 @@ where
     }
 }
 
-/// Stores meta-information about registered types or properties.
-///
-/// Filling this information properly is important so that Godot can use ptrcalls instead of varcalls
-/// (requires typed GDScript + sufficient information from the extension side)
-
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 
 /// Rusty abstraction of `sys::GDExtensionPropertyInfo`.

godot-core/src/builtin/meta/signature.rs

@@ -112,8 +112,6 @@ macro_rules! impl_varcall_signature_for_tuple {
         $R:ident
         $(, ($pn:ident, $n:tt) : $Pn:ident)* // $n cannot be literal if substituted as tuple index .0
     ) => {
-        // R: FromVariantIndirect, Pn: ToVariant -> when calling engine APIs
-        // R: ToVariant, Pn:
         #[allow(unused_variables)]
         impl<$R, $($Pn,)*> VarcallSignatureTuple for ($R, $($Pn,)*)
             where
@@ -400,7 +398,7 @@ unsafe fn varcall_return<R: ToGodot>(
 /// # Safety
 /// See [`varcall_return`].
 #[cfg(since_api = "4.2")] // unused before
-pub(crate) unsafe fn varcall_return_checked<R: ToVariant>(
+pub(crate) unsafe fn varcall_return_checked<R: ToGodot>(
     ret_val: Result<R, ()>, // TODO Err should be custom CallError enum
     ret: sys::GDExtensionVariantPtr,
     err: *mut sys::GDExtensionCallError,

godot-core/src/builtin/string/string_name.rs

@@ -4,15 +4,15 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+use std::fmt;
+
 use godot_ffi as sys;
 use sys::{ffi_methods, GodotFfi};
 
 use crate::builtin::inner;
 use crate::builtin::meta::impl_godot_as_self;
 use crate::builtin::{GodotString, NodePath};
 
-use std::fmt;
-
 /// A string optimized for unique names.
 ///
 /// StringNames are immutable strings designed for representing unique names. StringName ensures that only

godot-core/src/builtin/vectors/vector_axis.rs

@@ -98,18 +98,18 @@ impl EngineEnum for Vector2Axis {
 }
 
 impl GodotCompatible for Vector2Axis {
-    type Via = i64;
+    type Via = i32;
 }
 
 impl ToGodot for Vector2Axis {
     fn to_godot(&self) -> Self::Via {
-        self.ord() as i64
+        self.ord()
     }
 }
 
 impl FromGodot for Vector2Axis {
     fn try_from_godot(via: Self::Via) -> Option<Self> {
-        Self::try_from_ord(via as i32)
+        Self::try_from_ord(via)
     }
 }
 
@@ -146,18 +146,18 @@ impl EngineEnum for Vector3Axis {
 }
 
 impl GodotCompatible for Vector3Axis {
-    type Via = i64;
+    type Via = i32;
 }
 
 impl ToGodot for Vector3Axis {
     fn to_godot(&self) -> Self::Via {
-        self.ord() as i64
+        self.ord()
     }
 }
 
 impl FromGodot for Vector3Axis {
     fn try_from_godot(via: Self::Via) -> Option<Self> {
-        Self::try_from_ord(via as i32)
+        Self::try_from_ord(via)
     }
 }
 
@@ -197,18 +197,18 @@ impl EngineEnum for Vector4Axis {
 }
 
 impl GodotCompatible for Vector4Axis {
-    type Via = i64;
+    type Via = i32;
 }
 
 impl ToGodot for Vector4Axis {
     fn to_godot(&self) -> Self::Via {
-        self.ord() as i64
+        self.ord()
     }
 }
 
 impl FromGodot for Vector4Axis {
     fn try_from_godot(via: Self::Via) -> Option<Self> {
-        Self::try_from_ord(via as i32)
+        Self::try_from_ord(via)
     }
 }
 

godot-core/src/engine.rs

@@ -6,8 +6,6 @@
 
 //! Godot engine classes and methods.
 
-use godot_ffi::GodotNullableFfi;
-
 // Re-exports of generated symbols
 use crate::builtin::{GodotString, NodePath};
 use crate::obj::dom::EngineDomain;
@@ -223,7 +221,7 @@ pub(crate) fn ensure_object_alive(
     method_name: &'static str,
 ) {
     let Some(instance_id) = instance_id else {
-        panic!("{method_name}: instance id is null")
+        panic!("{method_name}: cannot call method on null object")
     };
 
     let new_object_ptr = object_ptr_from_id(instance_id);

godot-core/src/obj/gd.rs

@@ -10,7 +10,7 @@ use std::ptr;
 
 use godot_ffi as sys;
 use godot_ffi::VariantType;
-use sys::{interface_fn, static_assert_eq_size, GodotNullableFfi};
+use sys::static_assert_eq_size;
 
 use crate::builtin::meta::{FromGodot, GodotCompatible, GodotType, ToGodot};
 use crate::builtin::{Callable, StringName};
@@ -149,28 +149,6 @@ where
     pub fn bind_mut(&mut self) -> GdMut<T> {
         self.raw.bind_mut()
     }
-
-    /// Storage object associated with the extension instance.
-    // pub(crate) fn storage_mut(&mut self) -> &mut InstanceStorage<T> {
-    //     // SAFETY:
-    //     unsafe {
-    //         let binding = self.resolve_instance_ptr();
-    //         crate::private::as_storage_mut::<T>(binding)
-    //     }
-    // }
-
-    unsafe fn resolve_instance_ptr(&self) -> sys::GDExtensionClassInstancePtr {
-        let callbacks = crate::storage::nop_instance_callbacks();
-        let token = sys::get_library() as *mut std::ffi::c_void;
-        let binding = interface_fn!(object_get_instance_binding)(self.obj_sys(), token, &callbacks);
-
-        debug_assert!(
-            !binding.is_null(),
-            "Class {} -- null instance; does the class have a Godot creator function?",
-            std::any::type_name::<T>()
-        );
-        binding as sys::GDExtensionClassInstancePtr
-    }
 }
 
 /// _The methods in this impl block are available for any `T`._ <br><br>
@@ -182,13 +160,9 @@ impl<T: GodotClass> Gd<T> {
     pub fn try_from_instance_id(instance_id: InstanceId) -> Option<Self> {
         let ptr = engine::object_ptr_from_id(instance_id);
 
-        if ptr.is_null() {
-            None
-        } else {
-            // SAFETY: assumes that the returned GDExtensionObjectPtr is convertible to Object* (i.e. C++ upcast doesn't modify the pointer)
-            let untyped = unsafe { Gd::<engine::Object>::from_obj_sys(ptr) };
-            untyped.owned_cast::<T>().ok()
-        }
+        // SAFETY: assumes that the returned GDExtensionObjectPtr is convertible to Object* (i.e. C++ upcast doesn't modify the pointer)
+        let untyped = unsafe { Gd::<engine::Object>::from_obj_sys_or_none(ptr)? };
+        untyped.owned_cast::<T>().ok()
     }
 
     /// ⚠️ Looks up the given instance ID and returns the associated object.
@@ -227,9 +201,13 @@ impl<T: GodotClass> Gd<T> {
     /// ⚠️ Returns the last known, possibly invalid instance ID of this object.
     ///
     /// This function does not check that the returned instance ID points to a valid instance!
-    /// Unless performance is a problem, use [`instance_id()`][Self::instance_id] or [`instance_id_or_none()`][Self::instance_id_or_none] instead.
+    /// Unless performance is a problem, use [`instance_id()`][Self::instance_id] or
+    /// [`instance_id_or_none()`][Self::instance_id_or_none] instead.
     pub fn instance_id_unchecked(&self) -> InstanceId {
-        self.raw.cached_instance_id.get().unwrap()
+        // SAFETY:
+        // A `Gd` can only be created from a non-null `RawGd`. Meaning `raw.instance_id_unchecked()` will
+        // always return `Some`.
+        unsafe { self.raw.instance_id_unchecked().unwrap_unchecked() }
     }
 
     /// Checks if this smart pointer points to a live object (read description!).
@@ -308,23 +286,33 @@ impl<T: GodotClass> Gd<T> {
             .map_err(Self::from_ffi)
     }
 
+    #[doc(hidden)]
+    pub(crate) unsafe fn from_obj_sys_or_none(ptr: sys::GDExtensionObjectPtr) -> Option<Self> {
+        Self::try_from_ffi(RawGd::from_obj_sys(ptr))
+    }
+
     /// Initializes this `Gd<T>` from the object pointer as a **strong ref**, meaning
     /// it initializes/increments the reference counter and keeps the object alive.
     ///
     /// This is the default for most initializations from FFI. In cases where reference counter
     /// should explicitly **not** be updated, [`Self::from_obj_sys_weak`] is available.
     #[doc(hidden)]
-    pub unsafe fn from_obj_sys(ptr: sys::GDExtensionObjectPtr) -> Self {
-        Self::from_ffi(RawGd::from_obj_sys(ptr))
+    pub(crate) unsafe fn from_obj_sys(ptr: sys::GDExtensionObjectPtr) -> Self {
+        Self::from_obj_sys_or_none(ptr).unwrap()
     }
 
     #[doc(hidden)]
-    pub unsafe fn from_obj_sys_weak(ptr: sys::GDExtensionObjectPtr) -> Self {
-        Self::from_ffi(RawGd::from_obj_sys_weak(ptr))
+    pub(crate) unsafe fn from_obj_sys_weak_or_none(ptr: sys::GDExtensionObjectPtr) -> Option<Self> {
+        Self::try_from_ffi(RawGd::from_obj_sys_weak(ptr))
     }
 
     #[doc(hidden)]
-    pub fn obj_sys(&self) -> sys::GDExtensionObjectPtr {
+    pub(crate) unsafe fn from_obj_sys_weak(ptr: sys::GDExtensionObjectPtr) -> Self {
+        Self::from_obj_sys_weak_or_none(ptr).unwrap()
+    }
+
+    #[doc(hidden)]
+    pub(crate) fn obj_sys(&self) -> sys::GDExtensionObjectPtr {
         self.raw.obj_sys()
     }
 
@@ -341,34 +329,13 @@ impl<T: GodotClass> Deref for Gd<T> {
     type Target = <<T as GodotClass>::Declarer as dom::Domain>::DerefTarget<T>;
 
     fn deref(&self) -> &Self::Target {
-        // SAFETY:
-        //
-        // This relies on `Gd<Node3D>` having the layout as `Node3D` (as an example),
-        // which also needs #[repr(transparent)]:
-        //
-        // struct Gd<T: GodotClass> {
-        //     opaque: OpaqueObject,        // <- size of GDExtensionObjectPtr
-        //     cached_instance_id,          // <- Cell is #[repr(transparent)] to its inner T
-        //     _marker: PhantomData,        // <- ZST
-        // }
-        // struct Node3D {
-        //     object_ptr: sys::GDExtensionObjectPtr,
-        // }
-        self.raw.as_target()
+        self.raw.as_target().expect("`Gd` is never null")
     }
 }
 
 impl<T: GodotClass> DerefMut for Gd<T> {
     fn deref_mut(&mut self) -> &mut Self::Target {
-        // SAFETY: see also Deref
-        //
-        // The resulting `&mut T` is transmuted from `&mut OpaqueObject`, i.e. a *pointer* to the `opaque` field.
-        // `opaque` itself has a different *address* for each Gd instance, meaning that two simultaneous
-        // DerefMut borrows on two Gd instances will not alias, *even if* the underlying Godot object is the
-        // same (i.e. `opaque` has the same value, but not address).
-        //
-        // The `&mut self` guarantees that no other base access can take place for *the same Gd instance* (access to other Gds is OK).
-        self.raw.as_target_mut()
+        self.raw.as_target_mut().expect("`Gd` is never null")
     }
 }
 

godot-core/src/obj/instance_id.rs

@@ -89,28 +89,3 @@ impl FromGodot for InstanceId {
         Self::try_from_u64(via)
     }
 }
-
-/*
-// Note: Option impl is only possible as long as `FromVariant` and `InstanceId` are in same crate.
-// (Rust rationale: if upstream crate later adds blanket `impl FromVariant for Option<T>`, this would collide)
-impl FromVariant for Option<InstanceId> {
-    fn try_from_variant(variant: &Variant) -> Result<Self, VariantConversionError> {
-        if variant.is_nil() {
-            Ok(None)
-        } else {
-            // Should 0 in variant be mapped to None, or cause an error like now?
-            i64::try_from_variant(variant).map(|i| InstanceId::try_from_i64(i))
-        }
-    }
-}
-
-impl ToVariant for Option<InstanceId> {
-    fn to_variant(&self) -> Variant {
-        if let Some(id) = self {
-            id.to_variant()
-        } else {
-            0i64.to_variant()
-        }
-    }
-}
-*/