Merge #234

234: Fix cast always succeeding due to Godot bug. r=Bromeon a=joshua-maros

See #158 . 

Currently, casting always succeeds due to a bug in Godot. This PR adds an explicit (slow) check to make sure that the cast is valid before performing it. This should mirror the behavior that Godot will have when the bug is fixed upstream. This is better than the fix mentioned in #158 as that fix only works when the object you are casting is *exactly* the class you are casting to, you could not cast a derived class to a parent class. This PR has no such limitation.

I tested it with the following code:

```rust
#[derive(GodotClass)]
#[class(init)]
pub struct TestClass {}

#[godot_api]
impl TestClass {}

#[derive(GodotClass)]
#[class(init)]
pub struct OtherRefCountedClass {}

#[godot_api]
impl OtherRefCountedClass {}

#[derive(GodotClass)]
#[class(init)]
pub struct TestClassRunner {
    #[export]
    #[init(default = Gd::new(TestClass {}))]
    pub tc: Gd<TestClass>,
}

#[godot_api]
impl TestClassRunner {
    #[func]
    pub fn tests(&self) {
        godot_print!("{:?}", self.tc.share().upcast::<Object>());
        godot_print!("{:?}", self.tc.share().upcast::<RefCounted>());
        godot_print!(
            "{:?}",
            self.tc.share().upcast::<Object>().cast::<TestClass>()
        );
        godot_print!(
            "{:?}",
            self.tc.share().upcast::<Object>().try_cast::<Node>()
        );
        godot_print!(
            "{:?}",
            self.tc
                .share()
                .upcast::<Object>()
                .try_cast::<OtherRefCountedClass>()
        );
    }
}
```

And constructing the runner and running the function in Godot produced the following output:
```
Gd { id: -9223372008501280223, class: TestClass }
Gd { id: -9223372008501280223, class: TestClass }
Gd { id: -9223372008501280223, class: TestClass }
None
None
```

Co-authored-by: joshua-maros <60271685+joshua-maros@users.noreply.github.com>

Author: bors[bot]

godot-core/src/obj/gd.rs

@@ -16,8 +16,9 @@ use sys::{ffi_methods, interface_fn, static_assert_eq_size, GodotFfi, PtrcallTyp
 
 use crate::builtin::meta::{ClassName, VariantMetadata};
 use crate::builtin::{
-    Callable, FromVariant, StringName, ToVariant, Variant, VariantConversionError,
+    Callable, FromVariant, GodotString, StringName, ToVariant, Variant, VariantConversionError,
 };
+use crate::engine::Object;
 use crate::obj::dom::Domain as _;
 use crate::obj::mem::Memory as _;
 use crate::obj::{cap, dom, mem, Export, GodotClass, Inherits, Share};
@@ -326,11 +327,31 @@ impl<T: GodotClass> Gd<T> {
         })
     }
 
+    // Temporary workaround for bug in Godot that makes casts always succeed.
+    // (See https://github.com/godot-rust/gdext/issues/158)
+    // TODO(#234) remove this code once the bug is fixed upstream.
+    fn is_cast_valid<U>(&self) -> bool
+    where
+        U: GodotClass,
+    {
+        let as_obj = unsafe { self.ffi_cast::<Object>() }.expect("Everything inherits object");
+        let cast_is_valid = as_obj.is_class(GodotString::from(U::CLASS_NAME));
+        std::mem::forget(as_obj);
+        cast_is_valid
+    }
+
     /// Returns `Ok(cast_obj)` on success, `Err(self)` on error
     fn owned_cast<U>(self) -> Result<Gd<U>, Self>
     where
         U: GodotClass,
     {
+        // Temporary workaround for bug in Godot that makes casts always
+        // succeed. (See https://github.com/godot-rust/gdext/issues/158)
+        // TODO(#234) remove this check once the bug is fixed upstream.
+        if !self.is_cast_valid::<U>() {
+            return Err(self);
+        }
+
         // The unsafe { std::mem::transmute<&T, &Base>(self.inner()) } relies on the C++ static_cast class casts
         // to return the same pointer, however in theory those may yield a different pointer (VTable offset,
         // virtual inheritance etc.). It *seems* to work so far, but this is no indication it's not UB.
@@ -643,7 +664,10 @@ impl<T: GodotClass> Export for Gd<T> {
 impl<T: GodotClass> FromVariant for Gd<T> {
     fn try_from_variant(variant: &Variant) -> Result<Self, VariantConversionError> {
         let result_or_none = unsafe {
-            Self::from_sys_init_opt(|self_ptr| {
+            // TODO(#234) replace Gd::<Object> with Self when Godot stops allowing
+            // illegal conversions (See
+            // https://github.com/godot-rust/gdext/issues/158)
+            Gd::<Object>::from_sys_init_opt(|self_ptr| {
                 let converter = sys::builtin_fn!(object_from_variant);
                 converter(self_ptr, variant.var_sys());
             })
@@ -653,6 +677,9 @@ impl<T: GodotClass> FromVariant for Gd<T> {
         // (This behaves differently in the opposite direction `object_to_variant`.)
         result_or_none
             .map(|obj| obj.with_inc_refcount())
+            // 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)
     }
 }

itest/rust/src/object_test.rs

@@ -426,6 +426,36 @@ fn object_engine_downcast() {
     node3d.free();
 }
 
+#[derive(GodotClass)]
+struct CustomClassA {}
+
+#[derive(GodotClass)]
+struct CustomClassB {}
+
+#[itest]
+fn object_reject_invalid_downcast() {
+    let a_instance = Gd::new(CustomClassA {});
+    let b_instance = Gd::new(CustomClassB {});
+
+    let a_obj = a_instance.upcast::<Object>();
+    let b_obj = b_instance.upcast::<Object>();
+
+    assert!(a_obj.try_cast::<CustomClassB>().is_none());
+    assert!(b_obj.try_cast::<CustomClassA>().is_none());
+}
+
+#[itest]
+fn variant_reject_invalid_downcast() {
+    let a_instance = Gd::new(CustomClassA {}).to_variant();
+    let b_instance = Gd::new(CustomClassB {}).to_variant();
+
+    assert!(a_instance.try_to::<Gd<CustomClassB>>().is_err());
+    assert!(b_instance.try_to::<Gd<CustomClassA>>().is_err());
+
+    assert!(a_instance.try_to::<Gd<CustomClassA>>().is_ok());
+    assert!(b_instance.try_to::<Gd<CustomClassB>>().is_ok());
+}
+
 #[itest]
 fn object_engine_downcast_reflexive() {
     let node3d: Gd<Node3D> = Node3D::new_alloc();