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

Create .uid files in EditorFileSystem #101650

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jan 16, 2025

Fixes #101591

Turns out the ensure_uid_file() method I once added was wrong, because EditorFileSystem has its own uid cache. The bug occured, because the new UID was not registered in the EditorFileSystem, which is used by ResourceSaver to determine UIDs when saving.
I fixed the bug for scripts and shaders, but I see more places where FileInfo is created. There might be some edge case where either UID is not registered or .uid file is not created, idk.

@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 20:51
Comment on lines +2372 to +2380
} else {
if (ResourceLoader::exists(file) && !ResourceLoader::has_custom_uid_support(file) && !FileAccess::exists(file + ".uid")) {
Ref<FileAccess> f = FileAccess::open(file + ".uid", FileAccess::WRITE);
if (f.is_valid()) {
const ResourceUID::ID id = ResourceUID::get_singleton()->create_id();
ResourceUID::get_singleton()->add_id(id, file);
f->store_line(ResourceUID::get_singleton()->id_to_text(id));
fi->uid = id;
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual fix is here. The changes around are because of the fi variable.

@Repiteo

This comment was marked as resolved.

@KoBeWi KoBeWi force-pushed the duality_of_uid_caches branch from 339ddb9 to 948bff0 Compare January 17, 2025 00:45
@geowarin
Copy link
Contributor

It seems to solve the original problem. 👍

However (and I don't know if it's in the scope of this PR), when a script is moved, its path in the scene does not change.

image

Which mean that if the original resource moved, the button "open script" will open a new file.

The path never changes, even after scene reload or editor restart.
I think it's ok if this is treated at a later date but I would have expected the path to change at some point.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jan 17, 2025

@geowarin I can't reproduce this problem, both with and without this PR, so it's likely unrelated. Open a new issue.

@Hilderin
Copy link
Contributor

Except for the little comment above, everything looks good.

@KoBeWi KoBeWi force-pushed the duality_of_uid_caches branch from 948bff0 to aa3048a Compare January 18, 2025 19:01
@KoBeWi KoBeWi force-pushed the duality_of_uid_caches branch from aa3048a to 8aa3ee6 Compare January 18, 2025 20:01
@Repiteo Repiteo merged commit e547ff0 into godotengine:master Jan 20, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 20, 2025

Thanks!

@KoBeWi KoBeWi deleted the duality_of_uid_caches branch January 20, 2025 16:20
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.

Attaching a script to a scene does not use its uid
5 participants