- Add VariantConversionError::VariantIsNil

- Implement `Display` and `Error` for `VariantConversionError` - Remove `Deref` impl for `Object` with target `()` - Add an `#[allow]` for a clippy lint that was added in 1.72
godot-rust · Aug 26, 2023 · 24db64d · 24db64d
1 parent 88d22f8
commit 24db64d
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 17 deletions.
diff --git a/godot-codegen/src/class_generator.rs b/godot-codegen/src/class_generator.rs
@@ -524,6 +524,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);
@@ -613,20 +635,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)]
@@ -708,7 +717,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);
     };

diff --git a/godot-core/src/builtin/variant/variant_traits.rs b/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 {}
diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs
@@ -788,7 +788,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)
     }
 }
 

diff --git a/itest/rust/src/object_test.rs b/itest/rust/src/object_test.rs
@@ -336,7 +336,7 @@ fn object_engine_convert_variant_nil() {
 
     assert_eq!(
         Gd::<Area2D>::try_from_variant(&nil),
-        Err(VariantConversionError::BadType),
+        Err(VariantConversionError::VariantIsNil),
         "try_from_variant(&nil)"
     );
 

diff --git a/itest/rust/src/option_ffi_test.rs b/itest/rust/src/option_ffi_test.rs
@@ -17,6 +17,8 @@ fn option_some_sys_conversion() {
     let v2 = unsafe { Option::<Gd<Object>>::from_sys(ptr) };
     assert_eq!(v2, v);
 
+    // We're testing this behavior.
+    #[allow(clippy::unnecessary_literal_unwrap)]
     v.unwrap().free();
 }