Skip to content

Commit ed7f30d

Browse files
committed
Fix crash when converting Variant::nil() to Gd<T>
1 parent 38c67e9 commit ed7f30d

File tree

2 files changed

+26
-5
lines changed

2 files changed

+26
-5
lines changed

godot-core/src/obj/gd.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ use crate::{callbacks, engine, out};
2828
/// This smart pointer can only hold _objects_ in the Godot sense: instances of Godot classes (`Node`, `RefCounted`, etc.)
2929
/// or user-declared structs (`#[derive(GodotClass)]`). It does **not** hold built-in types (`Vector3`, `Color`, `i32`).
3030
///
31+
/// `Gd<T>` never holds null objects. If you need nullability, use `Option<Gd<T>>`.
32+
///
3133
/// This smart pointer behaves differently depending on `T`'s associated types, see [`GodotClass`] for their documentation.
3234
/// In particular, the memory management strategy is fully dependent on `T`:
3335
///
@@ -591,16 +593,18 @@ impl<T: GodotClass> Share for Gd<T> {
591593

592594
impl<T: GodotClass> FromVariant for Gd<T> {
593595
fn try_from_variant(variant: &Variant) -> Result<Self, VariantConversionError> {
594-
let result = unsafe {
595-
Self::from_sys_init(|self_ptr| {
596+
let result_or_none = unsafe {
597+
Self::from_sys_init_opt(|self_ptr| {
596598
let converter = sys::builtin_fn!(object_from_variant);
597599
converter(self_ptr, variant.var_sys());
598600
})
599601
};
600602

601603
// The conversion method `variant_to_object` does NOT increment the reference-count of the object; we need to do that manually.
602604
// (This behaves differently in the opposite direction `object_to_variant`.)
603-
Ok(result.with_inc_refcount())
605+
result_or_none
606+
.map(|obj| obj.with_inc_refcount())
607+
.ok_or(VariantConversionError)
604608
}
605609
}
606610

itest/rust/src/object_test.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66

77
use crate::{expect_panic, itest};
88
use godot::bind::{godot_api, GodotClass, GodotExt};
9-
use godot::builtin::{FromVariant, GodotString, StringName, ToVariant, Variant, Vector3};
10-
use godot::engine::{file_access, Camera3D, FileAccess, Node, Node3D, Object, RefCounted};
9+
use godot::builtin::{
10+
FromVariant, GodotString, StringName, ToVariant, Variant, VariantConversionError, Vector3,
11+
};
12+
use godot::engine::{file_access, Area2D, Camera3D, FileAccess, Node, Node3D, Object, RefCounted};
1113
use godot::obj::{Base, Gd, InstanceId};
1214
use godot::obj::{Inherits, Share};
1315
use godot::sys::GodotFfi;
@@ -254,6 +256,21 @@ fn check_convert_variant_refcount(obj: Gd<RefCounted>) {
254256
assert_eq!(obj.get_reference_count(), 1);
255257
}
256258

259+
#[itest]
260+
fn object_engine_convert_variant_nil() {
261+
let nil = Variant::nil();
262+
263+
assert_eq!(
264+
Gd::<Area2D>::try_from_variant(&nil),
265+
Err(VariantConversionError),
266+
"try_from_variant(&nil)"
267+
);
268+
269+
expect_panic("from_variant(&nil)", || {
270+
Gd::<Area2D>::from_variant(&nil);
271+
});
272+
}
273+
257274
#[itest]
258275
fn object_engine_returned_refcount() {
259276
let Some(file) = FileAccess::open("res://itest.gdextension".into(), file_access::ModeFlags::READ) else {

0 commit comments

Comments
 (0)