Skip to content

ItemList::add_item errors with somewhat unexpected "unimplemented" reference #391

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
bluenote10 opened this issue Aug 27, 2023 · 4 comments
Labels
status: duplicate This issue or pull request already exists

Comments

@bluenote10
Copy link
Contributor

bluenote10 commented Aug 27, 2023

Calling ItemList::add_item currently panics with not implemented: see #156 referencing issue #156. However, at least on first glance, there is no obvious relation to that issue, and neither has the particular code that causes the panic:

"null" => {
return match ty {
RustTy::BuiltinIdent(ident) if ident == "Variant" => quote! { Variant::nil() },
RustTy::EngineClass { .. } => quote! { unimplemented!("see #156") },
_ => panic!("null not representable in target type {ty:?}"),
}
}

Is it possible that calling that function ends up in that particular panic just by accident, and the issue is actually something else?

A minimal reproduction project would be:

use godot::engine::{ItemList, NodeVirtual};
use godot::prelude::*;

struct ExtensionImpl;

#[gdextension]
unsafe impl ExtensionLibrary for ExtensionImpl {}

#[derive(GodotClass)]
#[class(base=Node)]
pub struct Ui {}

#[godot_api]
impl NodeVirtual for Ui {
    fn init(mut base: Base<Self::Base>) -> Self {
        let mut item_list = ItemList::new_alloc();
        item_list.add_item("foobar".into()); // panic here
        base.add_child(item_list.upcast());
        Self {}
    }
}

Or here as a small project: ItemListPanic.zip

The full panic is:

Rust traceback
thread '<unnamed>' panicked at 'not implemented: see #156', /home/fabian/.cargo/git/checkouts/gdext-76630c89719e160c/bb2be22/godot-core/src/gen/classes/item_list.rs:1470:42
stack backtrace:
   0: rust_begin_unwind
             at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/core/src/panicking.rs:67:14
   2: godot_core::gen::classes::item_list::ExAddItem::new
             at /home/fabian/.cargo/git/checkouts/gdext-76630c89719e160c/bb2be22/godot-core/src/gen/classes/item_list.rs:1470:42
   3: godot_core::gen::classes::item_list::re_export::ItemList::add_item_ex
             at /home/fabian/.cargo/git/checkouts/gdext-76630c89719e160c/bb2be22/godot-core/src/gen/classes/item_list.rs:151:13
   4: godot_core::gen::classes::item_list::re_export::ItemList::add_item
             at /home/fabian/.cargo/git/checkouts/gdext-76630c89719e160c/bb2be22/godot-core/src/gen/classes/item_list.rs:147:13
   5: <hello_world::Ui as godot_core::gen::classes::node::re_export::NodeVirtual>::init
             at ./rust/src/lib.rs:18:9
   6: <hello_world::Ui as godot_core::obj::traits::cap::GodotInit>::__godot_init
             at ./rust/src/lib.rs:13:1
   7: core::ops::function::FnOnce::call_once
             at /rustc/5680fa18feaa87f3ff04063800aec256c3d4b4be/library/core/src/ops/function.rs:250:5
   8: godot_core::registry::callbacks::create_custom
             at /home/fabian/.cargo/git/checkouts/gdext-76630c89719e160c/bb2be22/godot-core/src/registry.rs:316:29
   9: godot_core::registry::callbacks::create
             at /home/fabian/.cargo/git/checkouts/gdext-76630c89719e160c/bb2be22/godot-core/src/registry.rs:299:9
  10: <unknown>
  11: <unknown>
  12: <unknown>
  13: <unknown>
  14: __libc_start_call_main
             at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  15: __libc_start_main_impl
             at ./csu/../csu/libc-start.c:392:3
  16: <unknown>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal runtime error: failed to initiate panic, error 5

Observed with the latest commit bb2be22 but also an earlier one.

@bluenote10
Copy link
Contributor Author

bluenote10 commented Aug 27, 2023

After having a closer look, I think I see the connection to #156:

Godot's add_item function has the signature:

int add_item ( String text, Texture2D icon=null, bool selectable=true )

And it is probably the nullability of icon that leads to the same issue as in #156.

I figured I may be able to work-around the issue by actually setting the icon to something null-ish explicitly with the following work-around, but this also panics:

item_list
    .add_item_ex("foobar".into())
    .icon(Gd::from_variant(&Variant::nil()))
    .done();

@Bromeon
Copy link
Member

Bromeon commented Aug 27, 2023

Since it looks like the same issue, I'd probably close this one? Maybe the title of the other one can be improved.

Btw, this just came up yesterday in Discord 🙂

@perry-blueberry
Copy link

I ran into the same issue recently with the same confusion about the message. One current workaround is to first set the icon an empty texture and then set the text (if you need the text):

let idx = self.base.add_icon_item(Texture2D::new());
self.base.set_item_text(idx, "text".into());

@Bromeon Bromeon closed this as not planned Won't fix, can't repro, duplicate, stale Aug 27, 2023
@Bromeon Bromeon added the status: duplicate This issue or pull request already exists label Aug 27, 2023
@Bromeon
Copy link
Member

Bromeon commented Aug 27, 2023

Object::call() with Variant::nil() might also be a possible workaround.

I updated the title in the other issue and added some landing-page description 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants