Skip to content
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

#[class(no_init)] makes extension non-reloadable #874

Open
snatvb opened this issue Aug 26, 2024 · 4 comments
Open

#[class(no_init)] makes extension non-reloadable #874

snatvb opened this issue Aug 26, 2024 · 4 comments
Labels
bug c: register Register classes, functions and other symbols to GDScript status: upstream Depending on upstream fix (typically Godot)

Comments

@snatvb
Copy link

snatvb commented Aug 26, 2024

Note

Edit bromeon: updated problem description.

When disabling the default constructor via #[class(no_init)], the class can no longer be reloaded. Godot uses this as a reason to disable reloading for the entire extension, with the following error:

Extension marked as reloadable, but attempted to register class 'MyClass' which doesn't support reloading. Perhaps your language binding don't support it? Reloading disabled for this extension.

It looks like the problem arises in the following part:

let mut create_fn = quote! { None };
let mut recreate_fn = quote! { None };
let mut is_instantiable = true;
match struct_cfg.init_strategy {
InitStrategy::Generated => {
godot_init_impl = make_godot_init_impl(class_name, &fields);
create_fn = quote! { Some(#prv::callbacks::create::<#class_name>) };
if cfg!(since_api = "4.2") {
recreate_fn = quote! { Some(#prv::callbacks::recreate::<#class_name>) };
}
}
InitStrategy::UserDefined => {
let fn_name = format_ident!("class_{}_must_have_an_init_method", class_name);
init_expecter = quote! {
#[allow(non_snake_case)]
fn #fn_name() {
fn __type_check<T: ::godot::obj::cap::GodotDefault>() {}
__type_check::<#class_name>();
}
}
}
InitStrategy::Absent => {
is_instantiable = false;
}
};

If no_init is specified, the init strategy will be Absent, and neither create nor recreate functions are set. I think particularly the latter causes problems, although this needs to be confirmed. The class is also marked abstract, but I'm not sure if that interferes with it.

We would need to see how we can enable reloading in the absence of a default constructor. If there are no instances of a class to be reloaded, there shouldn't be a problem -- so maybe Godot is overzealous at forbidding reloading if there is no constructor?

Update: there's now an upstream issue to enable this workflow in Godot:


Original message:

Hello,

I've noticed that when I've updated gdext and godot on 4.3 I got broken reload. I used gdext_coroutines. After disable this part on another project that I've created for testing - reloadable starts work fine. After I read the library and didn't notice any specific cases and tried to add imports coroutines into my extenstion directrly and got same issue.

Reproduce:

  1. Use nightly rust (coroutines is unstable feature)
  2. Add #![feature(coroutines, coroutine_trait)]
  3. Add use std::ops::{Coroutine, CoroutineState};

Read more: Houtamelo/gdext_coroutines#2
showcase.zip

@Bromeon
Copy link
Member

Bromeon commented Aug 31, 2024

Your project has a ton of dependencies, that's not really helpful. Please really come up with a minimal complete example, a single Rust file + if relevant, Cargo.toml and .gdextension file. Avoid any dependencies that are not necessary. In particular, see if you can somehow trigger the problem without gdext_coroutines (but still using Rust coroutines).

Also, ZIPs are nice -- but additionally, please add the above-mentioned files as inline snippets here (make sure they're short and do not contain irrelevant info). This is more accessible for anyone joining the discussion, and lets people try out things without first downloading and unpacking an archive.

@snatvb
Copy link
Author

snatvb commented Sep 6, 2024

iteresting, I got error when I started it from scratch:

core/extension/gdextension.cpp:508 - Extension marked as reloadable, but attempted to register class 'SpireCoroutine' which doesn't support reloading. Perhaps your language binding don't support it? Reloading disabled for this extension.

I think that it's issue on the lib side, if it so, please, close this issue(just I am not sure):
Houtamelo/gdext_coroutines#2 (comment)

@Bromeon
Copy link
Member

Bromeon commented Sep 10, 2024

So it looks like this has nothing to do with coroutines, but rather #[class(no_init)]. I'll edit the issue description.

@Bromeon Bromeon changed the title Coroutines breaks reloadable feature #[class(no_init)] makes extension non-reloadable Sep 10, 2024
@Bromeon Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Sep 10, 2024
@Bromeon
Copy link
Member

Bromeon commented Sep 10, 2024

Confirmed to be a limitation in Godot: godotengine/godot#96823

@Bromeon Bromeon added the status: upstream Depending on upstream fix (typically Godot) label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: register Register classes, functions and other symbols to GDScript status: upstream Depending on upstream fix (typically Godot)
Projects
None yet
Development

No branches or pull requests

4 participants
@Bromeon @snatvb and others