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 an editor tool to automatically upgrade and re-save meshes #83613

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Oct 19, 2023

Fixes: #83287

This PR adds a popup the first time a surface that needs to be upgraded is encountered. The user can select "Upgrade" to have the engine automatically re-import/re-save all meshes so they are saved with the new format.

Screenshot from 2023-10-19 16-09-05

Screenshot from 2023-10-19 16-09-15

Screenshot from 2023-10-19 16-10-14

Also, changes the warning message (when p_path is available) to:

A surface of " + p_path + " uses an old surface format and needs to be upgraded. The upgrade happens automatically at load time every time until the mesh is saved again or re-imported. Once saved (or re-imported), this mesh will be incompatible with earlier versions of Godot.

@clayjohn clayjohn added this to the 4.2 milestone Oct 19, 2023
@clayjohn clayjohn force-pushed the surface_upgrade_tool branch from 3cd54ad to 92797cf Compare October 19, 2023 13:57
@clayjohn clayjohn marked this pull request as ready for review October 19, 2023 14:02
@clayjohn clayjohn requested a review from a team as a code owner October 19, 2023 14:02
@clayjohn clayjohn changed the title WIP implementation of tool to automatically upgrade a re-save meshes Editor Tool to automatically upgrade a re-save meshes Oct 19, 2023
@clayjohn clayjohn force-pushed the surface_upgrade_tool branch from 92797cf to 0cfc207 Compare October 19, 2023 14:42
@clayjohn clayjohn force-pushed the surface_upgrade_tool branch from 0cfc207 to 91be05a Compare October 19, 2023 16:57
@YuriSizov YuriSizov changed the title Editor Tool to automatically upgrade a re-save meshes Add an editor tool to automatically upgrade and re-save meshes Oct 19, 2023
scene/resources/mesh.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

I haven't tested it, but from the code point of few it looks good, aside from a couple nits and small suggestions.

@noidexe
Copy link
Contributor

noidexe commented Oct 19, 2023

I tested importing a project with 4.1.1 and then opening it with the windows-editor artifact generated by this PR and it seems to work well overall.

I only had an issue with a large gltf file, getting a "Safe save failed" error message. I could reproduce it on Windows 10 and Windows 11, even with Windows Defender disabled. Disabling safe save and reloading the project to re-trigger the conversion worked well. Even with safe save active, just clicking on reimport after the error also seemed to work well.

The rest of the gltf files converted flawlessly so it's probably an unrelated issue. In terms of UX I think everything's much clearer now.

@lostminds
Copy link

As reported here #83429 the importer on 4.2 will change the names of some meshes (due to previous bug with mesh name that is now fixed) breaking references. Could this tool be a good place to detect/warn or even deal with this as well when the reimport happens?

@YuriSizov
Copy link
Contributor

YuriSizov commented Oct 20, 2023

@lostminds This is tightly related to checks done in the rendering server, which has nothing to do with the importer. So without a completely different implementation this is not compatible with your suggestion.

For the linked issue we want to introduce a way to version importer behavior, so you can keep the old logic for existing scenes.

@akien-mga
Copy link
Member

Tested on https://github.com/team-godog/NGJ2023_Godog and another local prototype, both options work great!

@fire
Copy link
Member

fire commented Oct 23, 2023

I approve of the idea and it's necessary to avoid usability problems upgrading the mesh format.

@clayjohn clayjohn force-pushed the surface_upgrade_tool branch from 129aeac to 318ef84 Compare October 23, 2023 20:40
@akien-mga akien-mga merged commit 50d17f6 into godotengine:master Oct 23, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@TokisanGames
Copy link
Contributor

TokisanGames commented Oct 28, 2023

Is there now versioning inside of ArrayMesh?
What Godot versions are meshes upgraded from and to? Only from Godot 4.0 alpha 15 through 4.1.2 upgraded to 4.2?

@clayjohn clayjohn deleted the surface_upgrade_tool branch October 28, 2023 12:11
@clayjohn
Copy link
Member Author

Is there now versioning inside of ArrayMesh?

Yes.

What Godot versions are meshes upgraded from and to? Only from Godot 4.0 alpha 15 through 4.1.2 upgraded to 4.2?

That's right

@Lippanon
Copy link

Upgrading engine from v4.2.dev.custom_build [f7bc653] to v4.2.beta.custom_build [9144457]
It seems when upgrading a project that has an addon active (GodotJolt in my case), the first time opening the project there will be a prompt about restarting due to addon:
image
On selecting Restart, when the project reopens there will be no upgrade prompt, it automatically chooses to upgrade it seems (I can only tell from the console). That probably shouldn't happen.
On selecting Continue, it will eventually bring the mesh upgrade tool.

I'm upgrading a reasonable size project and it corrupts many scenes, removing everything from the tscn file except the first line. It is corrupting scenes that have no meshes in them but I'll try to gather more information before opening an issue.

@TokisanGames
Copy link
Contributor

FYI, only certain GDExtensions like Godot-Jolt and Terrain3D use this prompt.

What creates the restart prompt is set_minimum_library_initialization_level(MODULE_INITIALIZATION_LEVEL_SERVERS) instead of MODULE_INITIALIZATION_LEVEL_SCENE in register_types.cpp. Without this prompt, Godot crashes on the first startup, though that may be resolved in 4.2. See #80850 and #81478.

TokisanGames/Terrain3D@dc007d4#diff-c23d1074e35b344ac28577f61ecb59e92261b451b50fc76b18eec920384c91b0R40

@clayjohn
Copy link
Member Author

@TokisanGames this tool doesn't interact with add-ons at all. If you are having problems with the add-on prompt, I suggest you open an issue about it.

There are known problems with the surface upgrade tool (see #83991) and I am fixing them as quickly as I can.

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