Hot-reloading of scripts ("Synchronize Script Changes") doesn't work when using external editor (VSCode)

[retro-git]

Godot version

4.0.beta17

System information

MacOS

Issue description

When the "Synchronize Script Changes" option is selected, scripts should hot-reload when changes are saved. However, this does not work when using Godot 4 with an external editor (only tested on VSCode). It does work when using the in-built text editor, or when using Godot 3.

Steps to reproduce

1. Configure Godot to use VSCode as an external editor.
2. Enable Synchronize Script Changes.
3. Edit a script file within VSCode while the game is running.

Minimal reproduction project

N/A

[rosshadden]

I don't want to turn this into a "me too!" comment stream, but I will say it's not just VS Code. I have this problem with Neovim as well, so it is very likely simply Godot isn't monitoring for inotify events or whatever equivalents on non-Unix systems.

I'm guessing the integrated editor runs some code on save that initiates the reloading, which would certainly be more efficient than monitoring the filesystem when applicable so it is very understandable. But it would be great if it did monitor the filesystem when users work with external editors.

[PathogenDavid]

I did some digging into this and I believe I've found the underlying cause as well as a potential fix.

This code in ScriptEditor::_update_modified_scripts_for_external_editor is what's typically responsible for telling the running game that it needs to reload scripts via _trigger_live_script_reload:

godot/editor/plugins/script_text_editor.cpp

Lines 728 to 739 in 013a457

	uint64_t last_date = scr->get_last_modified_time();
	uint64_t date = FileAccess::get_modified_time(scr->get_path());
	if (last_date != date) {
	Ref<Script> rel_scr = ResourceLoader::load(scr->get_path(), scr->get_class(), ResourceFormatLoader::CACHE_MODE_IGNORE);
	ERR_CONTINUE(!rel_scr.is_valid());
	scr->set_source_code(rel_scr->get_source_code());
	scr->set_last_modified_time(rel_scr->get_last_modified_time());
	scr->update_exports();
	_trigger_live_script_reload();
	}

Separately: When an external editor is connected to the language server, it sends a didSave event when you save a file, which in turn calls GDScriptTextDocument::didSave, which reloads the script using GDScript::load_source_code:

godot/modules/gdscript/language_server/gdscript_text_document.cpp

Line 104 in 550a779

if (scr.is_valid() && (scr->load_source_code(path) == OK)) {

This in turn causes the script's last_modified_time to be updated.

Since the didSave callback happens well before _update_modified_scripts_for_external_editor, the date check in ScriptEditor::_update_modified_scripts_for_external_editor will never succeed as the timestamps will always be the same.

---

I did not have time to investigate fully, but I suspect this issue was introduced by #66867 as it introduced reloading the script within didSave.

---

I found that I can fix the issue by calling ScriptEditor::_trigger_live_script_reload from GDScriptTextDocument::didSave. See here: PathogenDavid@210edab

(As an added bonus this also fixes #73808, but only if you're connected to the language server)

However today is the first time I've ever really used Godot, so it's not immediately clear to me whether this is the most appropriate fix. (If it is, would be more than happy to clean things up and make a PR!)

[OmarShehata]

Thank you so much for the code snippet here @PathogenDavid . I've been using a custom build of Godot 4 with your patch and it has significantly improved my workflow. It would be great to have a PR for this. Maybe this could even only run if Use an external editor is on? If there's any concern on this triggering more than is needed for users who aren't using an external editor?

EDIT: Actually there is one potential issue: if you are typing in VSCode, and save incorrect syntax, the editor will come into focus. You then need to click on the editor, then click back on VSCode.

[clemens-tolboom]

<C++ Source> modules/gdscript/gdscript_vm.cpp:697 @ call()

In #75550 we use the build-in GDScript editor which @dakennedyd thinks relates to this issue. Does it?

[Dexterminator]

Is there any reason for not making the fix @PathogenDavid made into a PR? PathogenDavid@210edab

[PathogenDavid]

@Dexterminator I mainly left my comment to share my findings for someone who wishes to investigate this further and to share my quick and dirty fix (and perhaps learn if someone more involved with Godot's development thought it was good enough as-is.)

Based on the quirk @OmarShehata ran into, I suspect the real fix will need to be more complicated.

A lot has changed for me in the past 6 months and I'm no longer using Godot, so I most likely won't be the one to investigate further.

[geowarin]

@PathogenDavid I think it's a good change.
From what I understand, the LanguageServerProtocol will trigger a didSave request, which will be forwarded to the gdscript_text_document, which will trigger a reload.

You should open a PR so that people may test the change easily and maintainers can get a chance to review it.