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

Hot reloaded nodes with "tool" enabled lose state and don't run initialization functions #908

Open
Soremwar opened this issue Sep 28, 2024 · 9 comments
Labels
bug c: register Register classes, functions and other symbols to GDScript

Comments

@Soremwar
Copy link

Soremwar commented Sep 28, 2024

Given the class

#[derive(GodotClass)]
#[class(base=CharacterBody2D, init, tool)]
pub struct Player {
	base: Base<CharacterBody2D>,
	#[init(val = 1)]
	x: i8,
}
#[godot_api]
impl ICharacterBody2D for Player {
	// Or enter_tree, it don't make a different
	fn ready(&mut self) {
		self.x = 0;
	}

	fn process(&mut self, _: f64) {
		godot_print!("{}", self.x);
	}
}

One would expect the console to spit out 0 all the time right? Well that would only be true when the scene is first opened, because if any change triggers hot reload x will be reset to 1 as if ready was never run.

This might not seem like a big deal, until you test this with something like

#[derive(GodotClass)]
#[class(base=CharacterBody2D, init, tool)]
pub struct Player {
	base: Base<CharacterBody2D>,
	#[init(val = None)]
	camera: Option<Gd<Camera2D>>,
}

This also applies for properties initialized with OnReady, so hot reload + tool is pretty much a no go

@Bromeon Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Sep 28, 2024
@TitanNano
Copy link
Contributor

On hot reload, the engine does:

  1. create a backup of all class properties (you have to add #[var])
  2. destroys the old instance
  3. creates a new instance and runs fn init() on it
  4. restores the property backup by calling the property setters with the stored values

@Bromeon
Copy link
Member

Bromeon commented Oct 1, 2024

@TitanNano what's the takeaway? You don't mention #[class(tool)] (opposite of "runtime classes"), so are any differences expected in that regard?

@TitanNano
Copy link
Contributor

@Bromeon looking at the implementation, @Soremwar is currently not defining his Godot class in a way that can be hot reloaded the way he expects. Data that should be persisted between reloads has to be exposed to the engine. But from user's perspective, I get that this is not obvious. Maybe that is just a documentation deficiency?

Tool classes are the only ones affected by this, as runtime classes do not have instances which need reloading. Hot reloading is also only supported in the editor, as far as I'm aware.

@Soremwar
Copy link
Author

Soremwar commented Oct 1, 2024

@TitanNano So in this example exposing my variables with #[var] should work as expected?

@TitanNano
Copy link
Contributor

@Soremwar I just checked again, and you have to add #[var(usage_flags = [STORAGE])] to every field that you want to get restored.

@Soremwar
Copy link
Author

Soremwar commented Oct 1, 2024

If I'm reading the docs right, this would modify the scene file wouldn't it?

@TitanNano
Copy link
Contributor

@Soremwar yes I think so too. It feels like a bug in the engine. It is unclear to me why this was chosen, but properties must have the storage usage flag so they get backed-up before hot reloading.

@Soremwar
Copy link
Author

Soremwar commented Oct 1, 2024

TBH it makes complete sense for a regular use case, since no Node would be expected to change their initial state after they have been instantiated. This is only true for @tool nodes, which makes me think that a way around this problem should be implemented on the side of the library.

I'm thinking of a hook (similar to _ready) that runs when the node is hot-reloaded, where it's left up to the user how to do state management between reloads. This would aleviate the problem of OnReady node references and similar approaches while preventing side effects on regular nodes

I'm up to implement this if you guys think it's a good idea

@Yarwin
Copy link
Contributor

Yarwin commented Oct 3, 2024

I'm thinking of a hook (similar to _ready) that runs when the node is hot-reloaded, where it's left up to the user how to do state management between reloads.

IMO this kind of hook that doesn't provide some kind of automatic re-evaulation of initial/onready values (a bit similar to init) would be pointless, since it already exists – Godot sends ObjectNotification::EXTENSION_RELOADED after reload which can be easily accessed on the user side.
This way has been suggested in PR.

Albeit providing some kind of "on_recreate" function that behaves similar to "init" (read - automagically recreates all the values) could be very cool

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

No branches or pull requests

4 participants