Skip to content

Commit 4ffff0f

Browse files
Fix cast always succeeding due to Godot bug.
1 parent bebbda8 commit 4ffff0f

File tree

2 files changed

+59
-2
lines changed

2 files changed

+59
-2
lines changed

godot-core/src/obj/gd.rs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ use sys::{ffi_methods, interface_fn, static_assert_eq_size, GodotFfi, PtrcallTyp
1616

1717
use crate::builtin::meta::{ClassName, VariantMetadata};
1818
use crate::builtin::{
19-
Callable, FromVariant, StringName, ToVariant, Variant, VariantConversionError,
19+
Callable, FromVariant, GodotString, StringName, ToVariant, Variant, VariantConversionError,
2020
};
21+
use crate::engine::Object;
2122
use crate::obj::dom::Domain as _;
2223
use crate::obj::mem::Memory as _;
2324
use crate::obj::{cap, dom, mem, Export, GodotClass, Inherits, Share};
@@ -326,11 +327,31 @@ impl<T: GodotClass> Gd<T> {
326327
})
327328
}
328329

330+
// Temporary workaround for bug in Godot that makes casts always succeed.
331+
// (See https://github.com/godot-rust/gdext/issues/158)
332+
// TODO(#234) remove this code once the bug is fixed upstream.
333+
fn is_cast_valid<U>(&self) -> bool
334+
where
335+
U: GodotClass,
336+
{
337+
let as_obj = unsafe { self.ffi_cast::<Object>() }.expect("Everything inherits object");
338+
let cast_is_valid = as_obj.is_class(GodotString::from(U::CLASS_NAME));
339+
std::mem::forget(as_obj);
340+
cast_is_valid
341+
}
342+
329343
/// Returns `Ok(cast_obj)` on success, `Err(self)` on error
330344
fn owned_cast<U>(self) -> Result<Gd<U>, Self>
331345
where
332346
U: GodotClass,
333347
{
348+
// Temporary workaround for bug in Godot that makes casts always
349+
// succeed. (See https://github.com/godot-rust/gdext/issues/158)
350+
// TODO(#234) remove this check once the bug is fixed upstream.
351+
if !self.is_cast_valid::<U>() {
352+
return Err(self);
353+
}
354+
334355
// The unsafe { std::mem::transmute<&T, &Base>(self.inner()) } relies on the C++ static_cast class casts
335356
// to return the same pointer, however in theory those may yield a different pointer (VTable offset,
336357
// 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> {
643664
impl<T: GodotClass> FromVariant for Gd<T> {
644665
fn try_from_variant(variant: &Variant) -> Result<Self, VariantConversionError> {
645666
let result_or_none = unsafe {
646-
Self::from_sys_init_opt(|self_ptr| {
667+
// TODO(#234) replace Gd::<Object> with Self when Godot stops allowing
668+
// illegal conversions (See
669+
// https://github.com/godot-rust/gdext/issues/158)
670+
Gd::<Object>::from_sys_init_opt(|self_ptr| {
647671
let converter = sys::builtin_fn!(object_from_variant);
648672
converter(self_ptr, variant.var_sys());
649673
})
@@ -653,6 +677,9 @@ impl<T: GodotClass> FromVariant for Gd<T> {
653677
// (This behaves differently in the opposite direction `object_to_variant`.)
654678
result_or_none
655679
.map(|obj| obj.with_inc_refcount())
680+
// TODO(#234) remove this cast when Godot stops allowing illegal conversions
681+
// (See https://github.com/godot-rust/gdext/issues/158)
682+
.and_then(|obj| obj.owned_cast().ok())
656683
.ok_or(VariantConversionError)
657684
}
658685
}

itest/rust/src/object_test.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,36 @@ fn object_engine_downcast() {
426426
node3d.free();
427427
}
428428

429+
#[derive(GodotClass)]
430+
struct CustomClassA {}
431+
432+
#[derive(GodotClass)]
433+
struct CustomClassB {}
434+
435+
#[itest]
436+
fn object_reject_invalid_downcast() {
437+
let a_instance = Gd::new(CustomClassA {});
438+
let b_instance = Gd::new(CustomClassB {});
439+
440+
let a_obj = a_instance.upcast::<Object>();
441+
let b_obj = b_instance.upcast::<Object>();
442+
443+
assert!(a_obj.try_cast::<CustomClassB>().is_none());
444+
assert!(b_obj.try_cast::<CustomClassA>().is_none());
445+
}
446+
447+
#[itest]
448+
fn variant_reject_invalid_downcast() {
449+
let a_instance = Gd::new(CustomClassA {}).to_variant();
450+
let b_instance = Gd::new(CustomClassB {}).to_variant();
451+
452+
assert!(a_instance.try_to::<Gd<CustomClassB>>().is_err());
453+
assert!(b_instance.try_to::<Gd<CustomClassA>>().is_err());
454+
455+
assert!(a_instance.try_to::<Gd<CustomClassA>>().is_ok());
456+
assert!(b_instance.try_to::<Gd<CustomClassB>>().is_ok());
457+
}
458+
429459
#[itest]
430460
fn object_engine_downcast_reflexive() {
431461
let node3d: Gd<Node3D> = Node3D::new_alloc();

0 commit comments

Comments
 (0)