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

Mesh instance UV2 unwrapping improvements #83498

Merged
merged 1 commit into from
Oct 28, 2023

Conversation

SaracenOne
Copy link
Member

The new option to set the unwrap mesh UV2 option in PrimitiveMeshes from the MeshInstance3DEditorPlugin now respects the global rules which disallows modifications to foreign resources and pre-emptively shows more informative error messages for common ArrayMesh unwrapping fail cases rather than conveying a generic popup error message and providing the actual information as a notification.

@SaracenOne SaracenOne added this to the 4.2 milestone Oct 17, 2023
@SaracenOne SaracenOne requested a review from a team as a code owner October 17, 2023 13:01
@SaracenOne
Copy link
Member Author

Updated, removed redundant explicit casting.

Enforce foreign resource modification rules.
Add more helpful error handling for ArrayMesh unwrapping.
@SaracenOne
Copy link
Member Author

Fixed

@akien-mga akien-mga merged commit fe64786 into godotengine:master Oct 28, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

err_dialog->set_text(TTR("Mesh cannot unwrap UVs because it belongs to another resource which was imported from another file type. Make it unique first."));

uint64_t format = array_mesh->surface_get_format(i);
if (format & Mesh::ArrayFormat::ARRAY_FORMAT_NORMAL) {
Copy link
Member

@AThousandShips AThousandShips Nov 2, 2023

Choose a reason for hiding this comment

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

Is this not backwards? It checks that it has normals and throws an error if it does, it should be:

if (!(format & Mesh::ArrayFormat::ARRAY_FORMAT_NORMAL)) {

Right?

Copy link
Contributor

Choose a reason for hiding this comment

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

For future readers, continue to #84374.

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.

4 participants