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

Rework script external modification tracking #101616

Merged
merged 1 commit into from
Jan 26, 2025

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jan 16, 2025

Fixes #101579

I introduced a new struct EditedFileData in ScriptEditorBase, which holds basic data about the edited file - path and modified time. The path is used instead of the previous _edit_res_path meta, the modified time is used instead of Resource's "modified time", which is not file modified time.

Due to #101615 the changes can't be properly tested.

@KoBeWi KoBeWi added this to the 4.4 milestone Jan 16, 2025
@KoBeWi KoBeWi requested review from a team as code owners January 16, 2025 00:03
@Repiteo Repiteo requested a review from YeldhamDev January 22, 2025 17:55
@YeldhamDev
Copy link
Member

YeldhamDev commented Jan 26, 2025

I can still trigger the bug even with this PR (Fedora - GNU/Linux).

Nevermind, used the wrong executable. This fixes it on my end.

@Repiteo Repiteo merged commit 6fdf91c into godotengine:master Jan 26, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 26, 2025

Thanks!

@Jordyfel
Copy link
Contributor

Jordyfel commented Feb 6, 2025

@KoBeWi I am investigating uses of Resource.set_path_cache() in order to improve its documentation, since it got exposed recently.

What was the reason for this use in filesystem dock? The resource paths are updated right after this in _update_resource_paths_after_move, so this doesn't get in front of anything. In fact it would sabotage that logic, because the function would find the resource paths at the new path and decide that they don't need a set_path() call which would normally move them in the resource cache.

I tested and, on master, trying to move a script of an open scene generates an error on the second move, which is fixed after commenting out this call.

Worth noting that the recent revert of the gdscript set_path_cache() override would have changed this behavior

@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 6, 2025

tbh this looks like some previous solution I tried and forgot to remove. I can't remember why this change exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicating a script triggers modified outside Godot dialog
4 participants