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

Classes without init break hot reloading #539

Closed
Bromeon opened this issue Dec 17, 2023 · 5 comments · Fixed by #593
Closed

Classes without init break hot reloading #539

Bromeon opened this issue Dec 17, 2023 · 5 comments · Fixed by #593
Labels
bug c: register Register classes, functions and other symbols to GDScript

Comments

@Bromeon
Copy link
Member

Bromeon commented Dec 17, 2023

It seems like a single class that is declared as something like:

#[derive(GodotClass)]
#[class(base=Node2D)]
struct MyClass {}

-- that is, no #[class(init)] and no manual override of fn init() -- is enough to break hot reloading.

The Godot error message is not helpful:

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.


There are several measures we can take:

  1. Prioritize the already planned init-refactoring (it's also easy to forget; maybe something like no_init could work?)
  2. Make sure that classes without init method are registered properly in GDExtension, meaning they are without a default constructor but otherwise functional.
    • Maybe the recreate_fn is needed.
  3. If having a constructor is a hard requirement for hot reloading in GDExtension, we can either:
    • Enforce that all classes have one, if hot reloading is enabled (if we can detect reloadable=true at startup)
    • Supply a "dummy" one just for this case, although that might come with follow-up issues

Long-term, we could probably expose something like a reinit() constructor that is invoked on hot reloads, and by default calls init. But before working on more features, I'd like to get the base robust.

@Bromeon Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Dec 17, 2023
@zaddok
Copy link

zaddok commented Dec 24, 2023

This explains half of the problems I was having.

@Bromeon
Copy link
Member Author

Bromeon commented Jan 11, 2024

I think absence of a constructor should be explicit. It's the exceptional case, and it's very easy to forget init. Apart from this problem, it can also cause other issues (e.g. in the editor or in scenes where classes are attempted to be default-constructed).

My current idea:

#[derive(GodotClass)]
struct MyClass { ... }

should still be allowed even with a missing #[class(init)], but it would require an accompanying

#[godot_api]
impl MyClass {
    fn init(base: Base<RefCounted>) -> Self { ... }
}

This can be statically enforced: #[derive(GodotClass)] generates a function that requires GodotInit as a bound. In other words, such code without the impl block would not compile anymore.

Classes that are deliberately not default-constructible could be annotated with #[class(no_init)], or this could become an alternative like #[class(init=disabled)] / #[class(init=default)].

@MarkWilds
Copy link

Same issue here. but hot reloading is not working all the time even with init for me...

@MartinVacheron
Copy link

Same here too.

@MartinVacheron
Copy link

Same here too.

Adding reloadable = true in the "{lib-name}.gdextension" file solved the problem. Saw that on Discord msg.

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants