[4.3] Hot reload of @tool scripts stops working after first reload

[dmlary]

Tested versions

• Reproduces in v4.3.stable.official [77dcf97]
• Fixed in v4.4.dev.custom_build [621cadc] (confirmed fixed in this commit; prior commit still has issue)

System information

Godot v4.3.stable - macOS 14.6.1 - Vulkan (Forward+) - integrated Apple M3 Max - Apple M3 Max (16 Threads)

Issue description

@tool scripts do not hot reload after the first hot reload when using an external editor. During the first hot reload the following error is generated:

modules/gdscript/gdscript.cpp:745 - Condition "!p_keep_state && has_instances" is true. Returning: ERR_ALREADY_IN_USE

Note this is not the same issue as #97238 which is 4.4 specific.

Steps to reproduce

• Create a new project

• Add a Node3D

• Attach a script to that Node3D

• Make script a @tool script, and add a print to the _ready() function; save it

• Switch back to godot editor

• Save & reopen scene in editor (seems to be necessary on macos right now)

• Confirm print from _ready()

• Edit script to change print message; save script

• Switch back to godot editor

• Godot will display the ERR_ALREADY_IN_USE error

• Close and reopen scene and updated _ready() message will be printed

• Edit script, changing the print message again: save script

• Switch back to godot editor

• Godot will not print any new message

• close & reopens scene and the message from the previous edit will be shown

Minimal reproduction project (MRP)

hot-reload-issue.zip

[dmlary]

@AThousandShips This bug is present in 4.3. I put a comment in #97168 asking for a cherry-pick to 4.3.1, and they asked for a separate 4.3 issue.

[AThousandShips]

My bad, reopening, didn't notice

[dmlary]

Also confirmed that commit from #97168 (621cadc) fixes the issue. Prior to that commit, the problem still reproduces.

[AThousandShips]

Prior to that commit, the problem still reproduces.

Did you confirm it was actually occurring in between? Or did you cherry-pick the commit to 4.3?

[dmlary]

@AThousandShips bah, you're right. I only confirmed it was fixed on 4.4 branch. I'll see if I can cherry pick it.

[AThousandShips]

Since this was bisected to [9fd431f] it's unlikely to be exactly the same, as the bug didn't occur prior to that commit, but a cherry pick will confirm if it fixes it

I'd say the bug was instead fixed at some point between 4.3 and that commit, to check you should also test in the latest 4.3.x development branch in case it might have been cherry picked then

[dmlary]

I cherry-picked 9638220 onto 4.3-stable and the fix worked.

I'll test 4.3.x branch clean, and then cherry-picking it onto there.

[dmlary]

One important node about #97168 is that they went through looking for other places reload(false) and changed them to reload(true). I think that's why I'm seeing the fix on 4.3.

#97168 (comment)

[AThousandShips]

That implies a 4.3 specific fix is better than a cherry pick as we want to avoid any unforeseen issues from that

[dmlary]

confirmed broken in 4.3 branch as of v4.3.1.rc.custom_build [6699ae7]

cherry-pick 9638220 onto 4.3 does have merge conflicts. Working through them.

That implies a 4.3 specific fix is better than a cherry pick as we want to avoid any unforeseen issues from that

Agreed. I'll see if I can narrow down 9638220 to the specific changes needed for 4.3.

[dmlary]

Worked through the merge conflicts and cherry-picking 9648220 onto the 4.3 branch works. Patch cherry-pick-9638220-onto-4.3.x.txt

At first glance, there's not much to trim down here.

[dmlary]

@AThousandShips I went through 9638220 and tried to cut out anything that was specific to 4.4, but couldn't pinpoint anything that didn't also apply to 4.3.

@Hilderin as the author of #97168 can you think of anything in the fix that is 4.4 specific, and should be dropped from a backport to 4.3?

[Hilderin]

After some digging, I found the source of the issue for tool script reload outside the editor. This is a regression from #89261 which was merged for 4.3. This PR reintroduce the reload call without the true parameter. I was able to reproduce the issue on 4.3 stable with a small tool script.

We have 2 ways to fix the issue for 4.3.1:

First option

• Chery pick Fix script properties reload from external editor #96002
• Then cherry pick Fix reloading scripts already in use #97168 since it's basically a fix for a regression introduce in Fix script properties reload from external editor #96002
  If you take all the modifications from Fix reloading scripts already in use #97168 but not the modifications from Fix script properties reload from external editor #96002, the external modified scripts that are not already in memory will not be reloaded because of the modifications in EditorFileSystem::_update_scan_actions in the first PR (Fix script properties reload from external editor #96002).

Second option

• Only take the modifications in Script::reload_from_file from Fix reloading scripts already in use #97168

Personally, I think we need a bit more testing to cherry pick both PRs into 4.3.1 right now, It already created some problematic regressions. It really depends on when 4.3.1 is coming out and how much it was tested.

[dmlary]

@Hilderin thanks for taking a look.

Tomorrow I’ll cherry pick both of those commits on top of the 4.3 branch and use it as my primary godot version while I work on tool scripts. If there’s anything explicit you’d like me to test let me know.

[dmlary]

Cherry-picked 46edd6d & 9638220 on to the 4.3 branch (EDIT: fix commits to match edit from next comment). There's a merge conflict cherry-picking the second one, but it's a minor change.

They resolve my issue, but I'll continue using this build to see if anything else breaks.

[dmlary]

Ok, y'all are gonna think I'm nuts, but this is a cursed build when used in combination with godot-cpp (4.3-stable).

I cherry-picked the following commits onto 6699ae7:

• 46edd6d
• 9638220
  (EDIT: copied the wrong numbers initially)

I'm working on a gdextension providing hexagon gridmaps. I've spent probably the last 8 hours trying to figure out why cpu cycles are seemingly lost returning from random functions in the extension. For example the function says it took 150us, the calling function says the called function took 3ms. The place it went really sideways is that if I renamed the function, the issue went away. This happened with multiple functions -__-. This even happened with empty functions, so not a destructor issue.

This is related to the build because swapping back to 4.3-stable everything resolved itself. I need to figure out if the issue is that my godot-cpp is technically out of sync with the 4.3.1 godot branch.

I'll try that tomorrow.

(EDIT: tried godot-cpp/4.3 branch and issue is still there. Something is definitely broken)

[Hilderin]

I'm really not an expert in godot-cpp, but did you build the engine build with production these scons parameters: production=yes optimize=speed lto=full. I suggest you create a new issue or ask in contributor chat about that if you still have issue.

[dmlary]

@Hilderin I had doubts, but yea, you were right. Switching to the production build did cause all of those inexplicable hangs to disappear. I am baffled by what could have been causing the repeatable delay that was resolved by renaming my function.

I've been running the production release with the hotfixes for the past few days, and haven't seen any issues. I only use an external editor (nvim), so I cannot say what other behavior may be impacted by the changes.

[dmlary]

Just an update; I've spent the past few days actively developing scripts, and everything is still working great.

Is there anything else y'all would like tested to ensure this gets into the 4.3.1 release?

[AstroStucky]

Cherry-picked 46edd6d & 46edd6d on to the 4.3 branch. There's a merge conflict cherry-picking the second one, but it's a minor change.

You named the same commit twice; I assume by mistake. Could you please tell us what the second commit was?

[dmlary]

Cherry-picked 46edd6d & 46edd6d on to the 4.3 branch. There's a merge conflict cherry-picking the second one, but it's a minor change.

You named the same commit twice; I assume by mistake. Could you please tell us what the second commit was?

I edited it in the original comment when I first posted, but here again: Nope, it was the next comment.

I cherry-picked the following commits onto 6699ae7:

46edd6d
9638220

[dmlary]

And I've been running that build for the past 3 weeks or so with no issues.