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

Add UID upgrade tool #102071

Merged
merged 1 commit into from
Jan 30, 2025
Merged

Add UID upgrade tool #102071

merged 1 commit into from
Jan 30, 2025

Conversation

Meorge
Copy link
Contributor

@Meorge Meorge commented Jan 27, 2025

Closes godotengine/godot-proposals#11639 .

This PR adds a new tool to the Project > Tools menu, currently titled "Update UIDs...". When run, it finds all of the Resource and PackedScene files in the project and re-saves them, causing the UID references to scripts and shaders to be added to them.

CleanShot 2025-01-26 at 19 40 24@2x

Here is a small project I put together with a few PackedScenes and a custom Resource subclass:
uid-upgrader-test_2025-01-26_19-36-02.zip

After running the tool, you can see that the UIDs have been added to all of the scene and resource files:
CleanShot 2025-01-26 at 19 39 21@2x

The code in this PR is adapted from the "Upgrade Mesh Surfaces..." tool (SurfaceUpgradeTool and SurfaceUpgradeDialog in the source code).

Note

I think it would be nice to have this pop-up display automatically when a project is first opened in 4.4 from a previous version, although I'm not sure yet how easy that would be to do. I hope to look more into that in the coming days, but in the meantime the tool is accessible through the menu bar as described previously.

To Do

  • Add "Learn More" button to window - should open blog post page
  • Look into _open_selected_projects_check_warnings for possibility of auto-opening tool when upgrading to 4.4

@arkology
Copy link
Contributor

I think it would be nice to have this pop-up display automatically when a project is first opened in 4.4 from a previous version, although I'm not sure yet how easy that would be to do. I hope to look more into that in the coming days, but in the meantime the tool is accessible through the menu bar as described previously.

Try to check ProjectManager::_open_selected_projects_check_warnings().

@KoBeWi
Copy link
Member

KoBeWi commented Jan 27, 2025

I wonder... We have Surface Upgrade Tool and now the UID Upgrade, but they both serve more or less the same task - open and re-save files. The problem is that they don't handle all files, so if we want to upgrade something else in the future, we will need another upgrade tool.

The script I posted in godotengine/godot-proposals#11639 (comment) handles pretty much all relevant files in a future-proof way, and it's only 27 lines. Maybe we should just have a single universal "upgrade tool" that user is supposed to run manually when opening in a new version? If it handles every resource there won't be any need for new tools, unless for something very specific that involves more than load and save.

@Calinou
Copy link
Member

Calinou commented Jan 27, 2025

I wonder if we can add a third button in the dialog that opens https://godotengine.org/article/uid-changes-coming-to-godot-4-4/ in a web browser. It would be helpful to give more context to users as to why the change was done, especially if they don't follow changelogs.

At the very least, the dialog's text should state that the new .uid files should be added to version control.

@Meorge Meorge force-pushed the feat/uid-upgrader branch from 9777af8 to 8a93cea Compare January 27, 2025 17:24
@Meorge
Copy link
Contributor Author

Meorge commented Jan 27, 2025

Try to check ProjectManager::_open_selected_projects_check_warnings().

Thanks, I'll give that a look soon!

The script I posted in godotengine/godot-proposals#11639 (comment) handles pretty much all relevant files in a future-proof way, and it's only 27 lines. Maybe we should just have a single universal "upgrade tool" that user is supposed to run manually when opening in a new version? If it handles every resource there won't be any need for new tools, unless for something very specific that involves more than load and save.

I like this idea! As stated previously, this PR is basically just an adaptation of the existing surface tool code anyways, so there's a lot of code there that's at best duplicate-adjacent.

I'm not sure how "hard" the 4.4 feature freeze is; my hope was that, because the UID change introduces such a big friction point for updating and keeping clean version control, this tool could be added to 4.4 so that people could use it to upgrade their projects as soon as 4.4 becomes available. In that case, I'd be a bit nervous about expanding the scope of this PR to act as a more general project updater, since there'd be more possibility of bugs cropping up.

However, if the feature freeze wouldn't allow this to be included until 4.4.1 (or 4.5) then we'd have a lot more time to get it polished and ready for its release. In the meantime, your script and instructions on how to run it could be included in the blog posts about 4.4's release.

I wonder if we can add a third button in the dialog that opens https://godotengine.org/article/uid-changes-coming-to-godot-4-4/ in a web browser. It would be helpful to give more context to users as to why the change was done, especially if they don't follow changelogs.

At the very least, the dialog's text should state that the new .uid files should be added to version control.

I've added a bit of text stating the user should add .uid files. As for the third button, I see that AcceptDialog does have a method to add custom buttons to it, so I can look into adding a "learn more" button to the dialog.

@KoBeWi
Copy link
Member

KoBeWi commented Jan 27, 2025

Well for 4.4 the current solution makes sense. We could refactor it in the future. The universal tool would replace existing ones.

@Meorge
Copy link
Contributor Author

Meorge commented Jan 28, 2025

I've added a "Learn more" button to the dialog box that opens the blog post about UIDs. It appears the current API for AcceptDialog only allows for it to be placed either to the left or to the right of all the buttons, so for now I have it on the right.

CleanShot 2025-01-27 at 17 55 59@2x


I looked at _open_selected_projects_check_warnings and it doesn't sound to me like it would quite fit what we need, since it runs in the project manager before the main editor is open. I also gave a shot at reading through the core/editor code to find where we could detect a 4.3 (or lower) project had been opened in 4.4, but nothing stuck out to me. I may give it another shot tomorrow, but if it looks like it's going to get too complex adding this check in, we may just want to tell people to find the option in the Tools menu.

I looked into it a bit further and found a spot that looks promising! Working on it now.

@Meorge Meorge force-pushed the feat/uid-upgrader branch from 8a93cea to 0e13dbc Compare January 28, 2025 04:49
@Meorge
Copy link
Contributor Author

Meorge commented Jan 28, 2025

The tool window is now displayed automatically when the editor is started with a project on 4.3 or less!

Godot.4.4.Display.UID.Upgrade.on.Project.Open.mp4

@Meorge Meorge requested a review from KoBeWi January 28, 2025 05:12
@KoBeWi
Copy link
Member

KoBeWi commented Jan 28, 2025

The dialog appears too early and progress dialog will overlay it:
image

@Repiteo Repiteo modified the milestones: 4.x, 4.4 Jan 28, 2025
@Meorge
Copy link
Contributor Author

Meorge commented Jan 28, 2025

Thanks for pointing this out! I was feeling like popping it up at the end of the EditorNode constructor wasn't quite right, but I wasn't sure where else to put it...

With your screenshot, it occurred to me that the check could be run immediately after _load_editor_layout when doing the first scan. I'll give that a try soon!

@Meorge
Copy link
Contributor Author

Meorge commented Jan 29, 2025

I've changed the order of detecting a 4.3 project and displaying the popup now so that it appears after the progress dialog:

Godot.4.4.Display.UID.Upgrade.v2.mp4
  • When the EditorNode receives the READY notification, it checks the project's version number before performing an initial save of the project settings. If it detects the project is from 4.3 or before, it activates a flag variable.
  • Later, when the _sources_changed method is evaluated, it checks that flag variable. If the flag is true, it sets it back to false (to avoid re-triggering the popup in the future) and then displays the popup.

For right now, I'm avoiding squashing commits so that I can refer back to code from previous commits if need be. When it looks good, I can squash them so the resulting single commit is ready to merge 😄

@Meorge Meorge requested a review from KoBeWi January 29, 2025 05:47
}

void UIDUpgradeTool::prepare_upgrade() {
EditorSettings::get_singleton()->set_project_metadata("uid_upgrade_tool", "run_on_restart", true);
Copy link
Member

@Calinou Calinou Jan 29, 2025

Choose a reason for hiding this comment

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

"uid_upgrade_tool" could be moved to a static class string constant to avoid typos and benefit from autocompletion, e.g:

static constexpr const char *META_UID_UPGRADE_TOOL = "uid_upgrade_tool";

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally on various demo projects (with GDScripts and shaders), it works as expected. Code looks good to me.

I've also tested --import, --headless --import, --export and --headless --export (each time in a clean 4.3 repository) and they all work as expected.

Apply suggestions from code review

Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>

memdelete the UID upgrade tool

Remove redeclaration of singleton

Add note about committing .uid files to version control

Add "Learn more" button that links to Godot blog post about UIDs

Detect project from 4.3 or less and automatically display UID upgrade window

Display popup after first run of `_sources_changed`

Apply suggestions from code review

Co-authored-by: Hugo Locurcio <hugo.locurcio@hugo.pro>
Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>

Replace magic strings with constants

Update editor/editor_node.cpp

Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>
@Meorge Meorge force-pushed the feat/uid-upgrader branch from 453da08 to d034d12 Compare January 30, 2025 16:42
@Repiteo Repiteo merged commit 5f4a0be into godotengine:master Jan 30, 2025
19 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 30, 2025

Thanks!

@QwertTheWert
Copy link

Is there a way to manually enable this tool? Since i have updated my project to 4.4 beta-1 before and so the dialogue doesn't show up.

@AThousandShips
Copy link
Member

Does it not work when using the tool menu button "Upgrade UIDs..."?

@QwertTheWert
Copy link

I... missed that. whoops

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.

Add tool to update all scenes and resources to use UIDs for 4.4
7 participants