Merge branch 'master' into qol/cached-function-pointers

# Conflicts:
#	godot-macros/src/method_registration/mod.rs
#	godot-macros/src/method_registration/register_method.rs
#	godot-macros/src/method_registration/virtual_method_callback.rs

Author: Bromeon

.github/workflows/full-ci.yml

@@ -79,9 +79,15 @@ jobs:
       # Note: could use `-- --no-deps` to not lint dependencies, however it doesn't really speed up and also skips deps in workspace.
       - name: "Check clippy"
         run: |
-          cargo clippy --all-targets $GDEXT_FEATURES ${{ matrix.rust-extra-args }} -- \
-          -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf \
-          -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings 
+          cargo clippy --all-targets $GDEXT_FEATURES -- \
+          -D clippy::suspicious \
+          -D clippy::style \
+          -D clippy::complexity \
+          -D clippy::perf \
+          -D clippy::dbg_macro \
+          -D clippy::todo \
+          -D clippy::unimplemented \
+          -D warnings
 
   unit-test:
     name: unit-test (${{ matrix.name }}${{ matrix.rust-special }})

.github/workflows/minimal-ci.yml

@@ -83,9 +83,15 @@ jobs:
 
       - name: "Check clippy"
         run: |
-          cargo clippy --all-targets $GDEXT_FEATURES ${{ matrix.rust-extra-args }} -- \
-          -D clippy::suspicious -D clippy::style -D clippy::complexity -D clippy::perf \
-          -D clippy::dbg_macro -D clippy::todo -D clippy::unimplemented -D warnings
+          cargo clippy --all-targets $GDEXT_FEATURES -- \
+          -D clippy::suspicious \
+          -D clippy::style \
+          -D clippy::complexity \
+          -D clippy::perf \
+          -D clippy::dbg_macro \
+          -D clippy::todo \
+          -D clippy::unimplemented \
+          -D warnings
           
 
   unit-test:

check.sh

@@ -158,8 +158,8 @@ function cmd_dok() {
 # Argument parsing
 ################################################################################
 
-# By default, disable `codegen-full` to reduce compile times and prevent flip-flopping
-# between `itest` compilations and `check.sh` runs.
+# By default, disable `codegen-full` to reduce compile times and prevent flip-flopping between
+# `itest` compilations and `check.sh` runs. Note that this means some runs are different from CI.
 extraCargoArgs=("--no-default-features")
 cmds=()
 nextArgIsFilter=false

godot-codegen/src/central_generator.rs

@@ -145,7 +145,7 @@ pub(crate) fn generate_sys_utilities_file(
         }
 
         let fn_name_str = &function.name;
-        let field = util::make_utility_function_ptr_name(&function);
+        let field = util::make_utility_function_ptr_name(function);
         let hash = function.hash;
 
         table.method_decls.push(quote! {
@@ -664,7 +664,7 @@ fn make_builtin_method_table(api: &ExtensionApi, builtin_types: &BuiltinTypeMap)
     // TODO reuse builtin_types without api
     for builtin in api.builtin_classes.iter() {
         let Some(builtin_type) = builtin_types.map.get(&builtin.name) else {
-            continue // for Nil
+            continue; // for Nil
         };
 
         populate_builtin_methods(&mut table, builtin, &builtin_type.type_names);

godot-codegen/src/class_generator.rs

@@ -523,6 +523,28 @@ fn make_class(class: &Class, class_name: &TyName, ctx: &mut Context) -> Generate
         (TokenStream::new(), TokenStream::new())
     };
 
+    // The base_ty of `Object` is `()`, and we dont want every engine class to deref to `()`.
+    let deref_impl = if class_name.rust_ty != "Object" {
+        quote! {
+            impl std::ops::Deref for #class_name {
+                type Target = #base_ty;
+
+                fn deref(&self) -> &Self::Target {
+                    // SAFETY: same assumptions as `impl Deref for Gd<T>`, see there for comments
+                    unsafe { std::mem::transmute::<&Self, &Self::Target>(self) }
+                }
+            }
+            impl std::ops::DerefMut for #class_name {
+                fn deref_mut(&mut self) -> &mut Self::Target {
+                    // SAFETY: see above
+                    unsafe { std::mem::transmute::<&mut Self, &mut Self::Target>(self) }
+                }
+            }
+        }
+    } else {
+        TokenStream::new()
+    };
+
     let all_bases = ctx.inheritance_tree().collect_all_bases(class_name);
     let (notification_enum, notification_enum_name) =
         make_notification_enum(class_name, &all_bases, ctx);
@@ -612,20 +634,7 @@ fn make_class(class: &Class, class_name: &TyName, ctx: &mut Context) -> Generate
 
             #exportable_impl
 
-            impl std::ops::Deref for #class_name {
-                type Target = #base_ty;
-
-                fn deref(&self) -> &Self::Target {
-                    // SAFETY: same assumptions as `impl Deref for Gd<T>`, see there for comments
-                    unsafe { std::mem::transmute::<&Self, &Self::Target>(self) }
-                }
-            }
-            impl std::ops::DerefMut for #class_name {
-                fn deref_mut(&mut self) -> &mut Self::Target {
-                    // SAFETY: see above
-                    unsafe { std::mem::transmute::<&mut Self, &mut Self::Target>(self) }
-                }
-            }
+            #deref_impl
 
             #[macro_export]
             #[allow(non_snake_case)]
@@ -707,7 +716,7 @@ fn make_notification_enum(
     all_bases: &Vec<TyName>,
     ctx: &mut Context,
 ) -> (Option<TokenStream>, Ident) {
-    let Some(all_constants) = ctx.notification_constants(class_name) else  {
+    let Some(all_constants) = ctx.notification_constants(class_name) else {
         // Class has no notification constants: reuse (direct/indirect) base enum
         return (None, ctx.notification_enum_name(class_name).name);
     };
@@ -1210,7 +1219,7 @@ pub(crate) fn make_utility_function_definition(
     }
 
     let function_name_str = &function.name;
-    let fn_ptr = util::make_utility_function_ptr_name(&function);
+    let fn_ptr = util::make_utility_function_ptr_name(function);
 
     let return_value = function
         .return_type

godot-codegen/src/codegen_special_cases.rs

@@ -28,22 +28,24 @@ pub(crate) fn is_class_excluded(_class: &str) -> bool {
 
 #[cfg(not(feature = "codegen-full"))]
 fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool {
+    use crate::{util, RustTy};
+
     fn is_rust_type_excluded(ty: &RustTy) -> bool {
         match ty {
             RustTy::BuiltinIdent(_) => false,
             RustTy::BuiltinArray(_) => false,
-            RustTy::RawPointer { inner, .. } => is_rust_type_excluded(&inner),
+            RustTy::RawPointer { inner, .. } => is_rust_type_excluded(inner),
             RustTy::EngineArray { elem_class, .. } => is_class_excluded(elem_class.as_str()),
             RustTy::EngineEnum {
                 surrounding_class, ..
             } => match surrounding_class.as_ref() {
                 None => false,
                 Some(class) => is_class_excluded(class.as_str()),
             },
-            RustTy::EngineClass { class, .. } => is_class_excluded(&class),
+            RustTy::EngineClass { class, .. } => is_class_excluded(class),
         }
     }
-    is_rust_type_excluded(&to_rust_type(ty, None, ctx))
+    is_rust_type_excluded(&util::to_rust_type(ty, None, ctx))
 }
 
 pub(crate) fn is_method_excluded(

godot-codegen/src/special_cases.rs

@@ -38,10 +38,12 @@ pub(crate) fn is_deleted(class_name: &TyName, godot_method_name: &str) -> bool {
 
 #[rustfmt::skip]
 pub(crate) fn is_class_deleted(class_name: &TyName) -> bool {
-    // Exclude experimental classes for now; possibly feature-gate them in the future.
-    if is_class_experimental(class_name) {
+    // TODO feature-gate experimental classes.
+    /*
+    if !cfg!(feature = "experimental-godot-api") && is_class_experimental(class_name) {
         return true;
     }
+    */
 
     match class_name.godot_ty.as_str() {
         // Hardcoded cases that are not accessible.
@@ -73,6 +75,7 @@ pub(crate) fn is_class_deleted(class_name: &TyName) -> bool {
 }
 
 #[rustfmt::skip]
+#[allow(dead_code)] // remove once used.
 fn is_class_experimental(class_name: &TyName) -> bool {
     // These classes are currently hardcoded, but the information is available in Godot's doc/classes directory.
     // The XML file contains a property <class name="NavigationMesh" ... is_experimental="true">.

godot-core/src/builtin/variant/variant_traits.rs

@@ -60,4 +60,28 @@ pub enum VariantConversionError {
 
     /// Variant value is missing a value for the target type
     MissingValue,
+
+    /// Variant value is null but expected to be non-null
+    VariantIsNil,
 }
+
+impl std::fmt::Display for VariantConversionError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            VariantConversionError::BadType => {
+                f.write_str("Variant type does not match expected type")
+            }
+            VariantConversionError::BadValue => {
+                f.write_str("Variant value cannot be represented in target type")
+            }
+            VariantConversionError::MissingValue => {
+                f.write_str("Variant value is missing a value for the target type")
+            }
+            VariantConversionError::VariantIsNil => {
+                f.write_str("Variant value is null but expected to be non-null")
+            }
+        }
+    }
+}
+
+impl std::error::Error for VariantConversionError {}

godot-core/src/obj/gd.rs

@@ -684,7 +684,8 @@ impl<T: GodotClass> Drop for Gd<T> {
         // No-op for manually managed objects
 
         out!("Gd::drop   <{}>", std::any::type_name::<T>());
-        let is_last = T::Mem::maybe_dec_ref(self); // may drop
+        // SAFETY: This `Gd` wont be dropped again after this.
+        let is_last = unsafe { T::Mem::maybe_dec_ref(self) }; // may drop
         if is_last {
             unsafe {
                 interface_fn!(object_destroy)(self.obj_sys());
@@ -788,7 +789,7 @@ impl<T: GodotClass> FromVariant for Gd<T> {
             // TODO(#234) remove this cast when Godot stops allowing illegal conversions
             // (See https://github.com/godot-rust/gdext/issues/158)
             .and_then(|obj| obj.owned_cast().ok())
-            .ok_or(VariantConversionError::BadType)
+            .ok_or(VariantConversionError::VariantIsNil)
     }
 }
 

godot-core/src/obj/traits.rs

@@ -320,9 +320,21 @@ pub mod mem {
         #[doc(hidden)]
         fn maybe_inc_ref<T: GodotClass>(obj: &Gd<T>);
 
-        /// If ref-counted, then decrement count
+        /// If ref-counted, then decrement count. Returns `true` if the count hit 0 and the object can be
+        /// safely freed.
+        ///
+        /// This behavior can be overriden by a script, making it possible for the function to return `false`
+        /// even when the reference count hits 0. This is meant to be used to have a separate reference count
+        /// from Godot's internal reference count, or otherwise stop the object from being freed when the
+        /// reference count hits 0.
+        ///
+        /// # Safety
+        ///
+        /// If this method is used on a [`Gd`] that inherits from [`RefCounted`](crate::engine::RefCounted)
+        /// then the reference count must either be incremented before it hits 0, or some [`Gd`] referencing
+        /// this object must be forgotten.
         #[doc(hidden)]
-        fn maybe_dec_ref<T: GodotClass>(obj: &Gd<T>) -> bool;
+        unsafe fn maybe_dec_ref<T: GodotClass>(obj: &Gd<T>) -> bool;
 
         /// Check if ref-counted, return `None` if information is not available (dynamic and obj dead)
         #[doc(hidden)]
@@ -362,7 +374,7 @@ pub mod mem {
             });
         }
 
-        fn maybe_dec_ref<T: GodotClass>(obj: &Gd<T>) -> bool {
+        unsafe fn maybe_dec_ref<T: GodotClass>(obj: &Gd<T>) -> bool {
             out!("  Stat::dec   <{}>", std::any::type_name::<T>());
             obj.as_ref_counted(|refc| {
                 let is_last = refc.unreference();
@@ -407,7 +419,7 @@ pub mod mem {
             }
         }
 
-        fn maybe_dec_ref<T: GodotClass>(obj: &Gd<T>) -> bool {
+        unsafe fn maybe_dec_ref<T: GodotClass>(obj: &Gd<T>) -> bool {
             out!("  Dyn::dec   <{}>", std::any::type_name::<T>());
             if obj
                 .instance_id_or_none()
@@ -435,7 +447,7 @@ pub mod mem {
     impl Memory for ManualMemory {
         fn maybe_init_ref<T: GodotClass>(_obj: &Gd<T>) {}
         fn maybe_inc_ref<T: GodotClass>(_obj: &Gd<T>) {}
-        fn maybe_dec_ref<T: GodotClass>(_obj: &Gd<T>) -> bool {
+        unsafe fn maybe_dec_ref<T: GodotClass>(_obj: &Gd<T>) -> bool {
             false
         }
         fn is_ref_counted<T: GodotClass>(_obj: &Gd<T>) -> Option<bool> {