Godot: Tool script doesn't call _ready or _enter_tree on reload

[Eymux]

Godot version:
3.0 stable
3.0.1 Build 689dfcd

OS/device including version:
Windows 7 Home

Issue description:
I have written a tool script, that draws connections between nodes. For that I have an onready variable, that loads the nodes. In the _draw function I get the position of the nodes and draw lines between them.
The lines are only drawn once after I reload the editor. When I change the script, nothing is drawn.
The problem is, that the onready variable is undefined after reloading the script. _ready and _enter_tree aren't called either. I have tried notifications, too. But no notifications for ready or enter_tree are received.

Steps to reproduce:

• create a new scene
• add two Node2D
• add a tool script to one node

extends Node2D

var node

func _ready():
    node = get_node("../Node2D2")

func _process(delta):
    update()

func _draw():
    draw_circle(node.position, 30, Color(1, 0, 0))

• when you reload the editor, the circle is drawn
• change the radius of the circle and save the script
• the circle is not redrawn

Minimal reproduction project:
Test2.zip

[RabbitB]

Tool scripts are _'isolated'_ so to speak. While running within the editor, they cannot reference or access any other nodes in the scene, as those nodes don't really exist yet. Same goes for singletons--they aren't yet instantiated inside the editor, so they don't exist until you run the game.

[Eymux]

The script is able to reference the the other nodes for the first time. And it can reference other nodes in the _process function.
So I don't think that's correct.

[Ranoller]

This is the behavior of an editor plugin. I don't like too. Every time a script is changed you have to reload entire editor to make plugin works. Make plugins is this way is a bit annoying, but there is for now other system. It will be fine to have option to reload editor without close and open (or automatic reload on editorplugin script change, but this is more difficult)

[Zylann]

I had this problem too with tool scripts. The problem is, script reloading RESETS variables. If it didnt do that, the script could be able to still function normally, provided that your changes are not too big. Recalling ready() would mean reloading the scene anyways (which you need to do manually at the moment, no need to restart the entire editor). So part of your game/tool state would still reset in some dirty way, which is certainly not intented.

So I believe the problem is not about those functions not being re-called, it's about member variables getting reset, while they should not. They should be remembered, and restored after the reload.

[Eymux]

How about simulating removing and readding the node?

[Zylann]

When I was doing my terrain plugin, that would mean freezing Godot for 3 seconds everytime I hit Ctrl+S xD

[Eymux]

3 seconds is faster, than restarting the whole editor and restoring everything I had opened. I could live with that.

[Zylann]

And 0 seconds are faster if my node didn't get readded^^ (i.e if variables didnt get reset). Note that this could also be non-viable for ingame reloading.

[Eymux]

But if I don't reload or readd, then my changes aren't applied. Which brings us to the start of the problem.
I'm writing code in iterations. I'll write something, then I test it immediately. When I write an editor tool, I want to see my changes, when I save the script.

[Zylann]

There are multiple solutions to reloading, some more suited than others, but there is no ultimate solution. There will always be a best case where only function reloading is needed, and there will always be a worst case where restarting the editor or the game is needed.

[Zylann]

I mean, I can't think of a way that contents everyone, unless having choice of how reloading is performed (which also means Ctrl+S can't really default to a single reloading method that works in all cases)

[Eymux]

Currently the only way is to restart the whole editor. And I don't think, that's necessary, when just reloading a script. Honestly I don't see where the problem is.

[Zylann]

The problem is how your script works. Calling ready and enter tree again supposes your scene is getting readded, which can have side effects such as connecting signals or creating nodes twice, because the node would get enter tree events while not really being added to the tree. Which means reloading the scene would work better in that case...

Really, I believe proper script reloading simply needs to restore ALL member variables as they were.

[Eymux]

Avoiding side effects in my own code is my business. I don't believe Godot needs to worry about that.
One way is to use Engine.editor_hint.

[Zylann]

That hint can't tell you _ready() was called because of instancing the node in editor, in game, or the script being reloaded.

[Eymux]

And? I still don't see our problem. I'm not trying to be obtuse. I just don't understand, why Godot has to bother with side effects in my code.
Do you have an example, where this would matter?

[volzhs]

probably #6005?

[Zylann]

The problem I see is that _ready and _enter_tree would loose their semantic. Who knows what people put in these functions, but it's to deal with when these events actually happen. Having them happen exceptionally on script reloading breaks that semantic, which might come unexpected, hence the possible side-effects, and why reloading the scene would not have these side-effects (for everyone, not just you). Also, what about the _init() constructor function? Same story here, would it be called again?

Another example of the problem I had with my plugin:
https://github.com/Zylann/godot_terrain_plugin/blob/master/addons/zylann.terrain/terrain.gd#L54
The sub-nodes and VisualScript RIDs created by the terrain would be leaked and created a second time, over the existing ones which my script forgets about because of getting reset by intrusive reloading. Having _ready() called again wouldn't solve anything, at least in that case. Hence again why, normally exiting the tree first and re-entering by reloading the scene works (or, in my case, not resetting data and only reloading functions). And reloading the scene is far from hot-reloading the script.

[Eymux]

It shouldn't be the job of Godot to worry about the users code. If a programmer is writing bad code, it's his/her own fault.
And in a tool script, I want the reload to reset everything to default and call _enter_tree and _ready. There is no second time here.

[Zylann]

@Eymux if you want this to happen, there is a need for the script to cleanup its state, or being able to reload by re-using its previous state. Calling _ready again because of reloading would become ambiguous, and here is yet another example:

You might think you can test if your objects exist before _ready to be called and clean them up if editor_hint. But that would make your node do that too when you reparent it, because that's also what the definition of _enter_tree and _ready are... and you don't necessarily want that to happen.

But tbh even that is impossible currently because as I said, the script gets reset quite blindly, leaking every object it might have created during its lifetime... and that's not bad code, that's just what happens if you work with non-resource object references (node refs, timers, RIDs...).

That's why I think there are in fact multiple hot-reload ways:

• Soft-reload, which reloads functions only and has zero cost as far as the script is concerned. It keeps the current state of the script which is good because we get minimal amount of state loss, getting closer to true hot-reloading ;
• The current reload, which also resets members for some reason, which defeats hot-reloading in several cases. Maybe it could do better by saving previous vars and re-assigning them beforehands unless their default value changed? (which is what Unity assembly-reload does AFAIK) ;
• Hard-reload, which reloads the scene (or the node where the script is), which users end up doing by restarting the editor (which is overkill because you could just reopen your scene) ;
• Your proposal for recalling life-cycle functions is yet another in-between

All of these have pros and cons. I can think of breaking cases for every one of them. The only one which doesn't is obviously reloading the scene (but is also the least effective one as far as hot-reload is concerned).

[Eymux]

I believe, we're getting into a different discussion here. My tool doesn't need to know, when it is reset. It doesn't instantiate anything. I just want to draw something to the screen.
Maybe Godot script needs a destructor.

I can't wait for full C# support.

[Zylann]

Why C# would have to do with this?

[Eymux]

Because it's a decent language with a garbage collector.

[volzhs]

I just want to draw something to the screen.

is that all you want?

tool
extends Node2D

export var radius = 30
var node

func _ready():
    node = get_node("../Node2D2")

func _process(delta):
    update()

func _draw():
    draw_circle(node.position, radius, Color(1, 0, 0))

this will solve your problem then.
you don't need to reload or something.
just change the radius value in the inpector panel.

[Eymux]

This is just the minimal example, I've created for the issue.
My script is more complicated than that.

[JordanMunroe]

@volzhs That example points out the exact issue here, if you wanted to change it to draw a polygon you would change the code, Ctrl+S and node would be lost causing errors until you reload the scene.

I see no issues with the proposal of calling _ready() on script reload, if it causes side-effects it should be my job to deal with them by not blindly adding new nodes without checking to make sure they don't already exist. If I save a script, it should behave exactly as it would if I just attached it to a node, not in a way that is unique to the action of saving.

[willnationsdev]

@Zylann

All of these have pros and cons. I can think of breaking cases for every one of them.

If that's the case, why not make them all available and make it configurable in the EditorSettings? That way users can change it whenever their use-case is appropriate?

[Zylann]

@willnationsdev well, you can make these hot-reload types available, but that doesn't change the fact they have pros and cons.

[willnationsdev]

What if we created a specific method for the use-case of "re-initializing a tool script after it has been saved"? You could define a _tool_reload() method and then just have that method and the _ready() method both execute a separate initialization method. That way the same logic gets executed in-game when the script is included, in-editor when you set it up as a dock/panel/etc. as the editor opens, and when you save the script within the editor. Could even implement custom logic for each specific case (since the _ready() could use the is_editor_hint() to differentiate between the first two cases).

Even if there are other potential solutions that are more severe, like reloading the scene, this particular issue directly addresses the problem of not having an initialization method in the script that triggers when you save a tool script. Rather than forcing the other methods to get triggered during that situation, we might as well come up with a separate method to handle that use case altogether, as it gives us the highest level of customization, as far as I tell.

[Zylann]

@willnationsdev as far as hot-reload of scripts is concerned, I don't see why such a callback would be tool only, though. Even if we had that, I can think of cases where scripts will break (not just the one being reloaded), I guess it's a matter of wether the system suits you or not (almost never if you have a complex plugin).

[neikeq]

Scripts should have two callbacks: about_to_reload and reloaded (Just an example, I don't like these names). In Unity you have OnEnable and OnDisable which are called on reload (they are also called in other situations that would not apply to Godot).

[Takanu]

I'm so happy I found the issue where people were talking about this, it has driven me _insane_ today figuring out why the state of my custom node suddenly drops on script reloads and it makes testing pretty tedious and unreliable.

I currently don't have a preference so long as I can deal with the dropped properties once the script has been reloaded, having separate functions be called when this occurs seems to make the most sense with the least potential collisions.

[Favorlock]

It would be great if they could fix this soon. As has been mentioned, resetting variables to default state on hot reload isn't very effective. I've been working on a plugin the last few days and this issue has been a plague. Often this will lead to orphaned nodes, which is bad.

[BenjaminNavarro]

I'm also quite bothered with this reloading issue. My solution for now is to not rely on onready or _ready to be called but to use getter functions to give me the references to the nodes I need.

[Shadowblitz16]

please fix this its a pain in the butt and I would consider it a bug.
literally saving/reloading the script in the editor is the same as closing and reopening it.
it should be treated as so.

[tliron]

This is a painful issue! I spent a day trying to understand what was going on. The problem is not only that _ready would not be called again (and this includes onready variable initializations), but also that setget setters for exported properties would be called again.

(Moreover, the editor will always call the setters before _ready if the property was given a non-default custom value. This, at least, makes sense, because you would likely want _ready to take the "real" values into account. But of course if _ready isn't called at all, you are stuck in an in-between state.)

Example:

extends Node

export(int) var selection = 0 setget set_selection
onready var children = get_children()

func set_selection(v):
  # "children" will sometimes be null!
  print(children)
  selection = v

There were a few suggestions here on how to solve this problem. I'll add my two cents: it would be nice if there was a _reload built-in callback function exactly for handling these situations. The logic for reloading is simply different from the logic for adding to the tree (readying). It wouldn't be trivial, but at least it would be clear where you should put code to handle this situation.

Furthermore, I don't think such a callback should be reserved just for tool scripts! After all, any runtime node in the game could be reloaded dynamically, too.