Skip to content

Gd::from_variant(&Variant::nil()) causes crash #155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
atrefonas opened this issue Mar 7, 2023 · 1 comment · Fixed by #157
Closed

Gd::from_variant(&Variant::nil()) causes crash #155

atrefonas opened this issue Mar 7, 2023 · 1 comment · Fixed by #157
Labels
bug c: core Core components

Comments

@atrefonas
Copy link

atrefonas commented Mar 7, 2023

This follows a discussion we were having in the Discord.

Gd::from_variant(&Variant::nil()) causes a hard crash.

My backtrace from the crash is:

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x68)
  * frame #0: 0x000000010405a2a0 godot.macos.editor.arm64`___lldb_unnamed_symbol144153
    frame #1: 0x0000000113a22670 libeco_game.dylib`godot_core::obj::gd::Gd$LT$T$GT$::from_opaque::h855c84a7a86296cb(opaque=(storage = "", marker = core::marker::PhantomData<unsigned char *> @ 0x000000016fdfc410)) at gd.rs:219:27
    frame #2: 0x00000001139bdf60 libeco_game.dylib`_$LT$godot_core..obj..gd..Gd$LT$T$GT$$u20$as$u20$godot_ffi..godot_ffi..GodotFfi$GT$::from_sys_init::hbe5869979e1f6c59(init={closure_env#0}<godot_core::gen::classes::kinematic_collision_3d::re_export::KinematicCollision3D> @ 0x000000016fdfc4b0) at godot_ffi.rs:149:13
    frame #3: 0x00000001139bbe88 libeco_game.dylib`_$LT$godot_core..obj..gd..Gd$LT$T$GT$$u20$as$u20$godot_core..builtin..variant..variant_traits..FromVariant$GT$::try_from_variant::ha7b25a7a4b6e343e(variant=0x000000016fdfcb48) at gd.rs:595:13
    frame #4: 0x00000001139bbe3c libeco_game.dylib`godot_core::builtin::variant::variant_traits::FromVariant::from_variant::h47e14a615036a1f6(variant=0x000000016fdfcb48) at variant_traits.rs:19:9
    frame #5: 0x00000001139ddc74 libeco_game.dylib`eco_game::main_scene_3d::Main3D::on_ray_process::h1ef5fa7f2599b46d(self=0x000000011108f708) at main_scene_3d.rs:112:11

The reason this is important is so that null default arguments can be passed into functions in GDExtension. For example in the original GDScript API, PhysicsBody3D.test_move() has a null default argument for the 3rd argument.

An ugly workaround that reportedly works is unsafe { mem::transmute((ptr::null::<Node>(), None::<InstanceId>)) }, although I have not tried it yet with the PhysicsBody3D api.

Also, ideally it seems the 3rd argument of PhysicsBody3D.test_move() should be an Option<Gd> not just a Gd, so there is also a codegen issue here, which maybe is the bigger issue actually.

@Bromeon
Copy link
Member

Bromeon commented Mar 7, 2023

Thanks! Ideally, Gd::try_from_variant(&Variant::nil()) should return Err(VariantConversionError), and from_variant() thus panic.

This goes a bit in a similar direction as #77, although the two are not directly related.

bors bot added a commit that referenced this issue Mar 7, 2023
157: `Gd::eq()` + fix `Gd::from_variant(nil)` r=Bromeon a=Bromeon

* Fix crash when converting `Variant::nil()` to `Gd<T>`
* Implement `Eq` for `Gd<T>`

Fixes #155.
bors r+

Co-authored-by: Jan Haller <[email protected]>
@bors bors bot closed this as completed in c169522 Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: core Core components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants