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

Script reference is broken when script is moved in FileSystem dock #101615

Closed
KoBeWi opened this issue Jan 16, 2025 · 3 comments · Fixed by #102429
Closed

Script reference is broken when script is moved in FileSystem dock #101615

KoBeWi opened this issue Jan 16, 2025 · 3 comments · Fixed by #102429

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Jan 16, 2025

Tested versions

4.4 beta1
Didn't test other versions, but probably a regression.

System information

W10

Issue description

When you have a script opened and then you move it to another directory, the script reference in the editor is broken. The internal path does not update, so re-focusing editor causes errors about missing file and trying to save the file will have no effect.

Steps to reproduce

  1. Open a script file in script editor
  2. In FileSystem dock, move the file to another directory
  3. Open the moved file in external editor
  4. In Godot, modify and save the still opened script
  5. See in external editor that changes are not saved

Alternatively:
3. Unfocus and focus the editor
4. Error in output

Minimal reproduction project (MRP)

N/A

@Jordyfel
Copy link
Contributor

Jordyfel commented Jan 16, 2025

Investigated a bit and something weird is going on.

The bug only happens if both the scene and its script are open. During the filesystem dock move, a scene reload is requested, which ends up asking the script editor to edit the script. When iterating over the already opened scripts in the script editor, the following check for whether the script is already open weirdly fails:

if ((scr.is_valid() && se->get_edited_resource() == p_resource) || se->get_edited_resource()->get_path() == p_resource->get_path()) {

Try setting a breakpoint here and then moving the file, the debugger shows 2 different objects with different pointer addresses and refcounts. The path_cache value also differs. The object with the correct path is the one passed to the function, and the wrong path belongs to the object returned by se->get_edited_resource().

@KoBeWi
Copy link
Member Author

KoBeWi commented Jan 16, 2025

It's another regression from #96499
CC @beev1s

@Jordyfel
Copy link
Contributor

Jordyfel commented Feb 1, 2025

Investigated some more and I think that PR should be reverted.

It is difficult to pin down, but it seems that, by sometimes avoiding registration in ResourceCache, the scripts end up getting duplicated. Some codepaths assume that scripts will be in the ResourceCache, so it doesn't seem like a good idea to occasionally avoid it.

One of the reasons for duplication seems to be this getting skipped when moving (because the script is not in the ResourceCache)

// Rename all resources loaded, be it subresources or actual resources.
List<Ref<Resource>> cached;
ResourceCache::get_cached_resources(&cached);
for (Ref<Resource> &r : cached) {
String base_path = r->get_path();
String extra_path;
int sep_pos = r->get_path().find("::");
if (sep_pos >= 0) {
extra_path = base_path.substr(sep_pos, base_path.length());
base_path = base_path.substr(0, sep_pos);
}
if (p_renames.has(base_path)) {
base_path = p_renames[base_path];
r->set_path(base_path + extra_path);
}
}

The set_path() call would normally update the path in the GDScriptCache, but it gets skipped, then if the scene the script is attached to is open, the scene reloads the script, which is not found in the GDScriptCache at the new path, then the script gets recompiled and duplicated.

sondredl pushed a commit to sondredl/godot that referenced this issue Feb 6, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
SirQuartz pushed a commit to SirQuartz/godot that referenced this issue Feb 7, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
dugramen pushed a commit to dugramen/godot that referenced this issue Feb 8, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
WhalesState pushed a commit to WhalesState/blazium that referenced this issue Feb 17, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
WhalesState pushed a commit to WhalesState/blazium that referenced this issue Feb 17, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
WhalesState pushed a commit to WhalesState/blazium that referenced this issue Feb 20, 2025
This reverts commit fd5fc9f.

This caused significant regressions which are worse than the bug that godotengine#96499
aimed to address.

- Reverts godotengine#96499.
- Reopens godotengine#95909.
- Supersedes godotengine#102063.
- Fixes godotengine#99006.
- Fixes godotengine#101615.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Release Blocker
Development

Successfully merging a pull request may close this issue.

3 participants