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 validation to glTF importer for Blendshape and Animation #94783

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

TokageItLab
Copy link
Member

I don't know if there are other places where this is needed, but these related to animation need to be validated as they can break the Track path.

I am not sure if this needs to be punt to 4.4, as it breaks compatibility but is an invalid property name to begin with; : is used by type annotation and subpath.

@TokageItLab TokageItLab force-pushed the validate-gltf-anim-name branch from 5c0393a to b9845a8 Compare July 26, 2024 08:59
@TokageItLab TokageItLab force-pushed the validate-gltf-anim-name branch from b9845a8 to 0235086 Compare July 26, 2024 09:05
@akien-mga akien-mga requested a review from lyuma August 8, 2024 08:35
@TokageItLab TokageItLab requested a review from fire August 28, 2024 01:36
@akien-mga akien-mga requested review from adamscott and aaronfranke and removed request for adamscott September 4, 2024 16:42
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Looks good to me. I did not test it but the code makes sense and is quite straightforward. The new validate_blend_shape_name is nearly identical to AnimationLibrary::validate_library_name.

@fire
Copy link
Member

fire commented Sep 7, 2024

May have to do to the fbx importer too

@akien-mga
Copy link
Member

May have to do to the fbx importer too

@TokageItLab Could you assess this?
Happy to merge this now or wait for an update for fbx if it's needed, let me know.

@TokageItLab
Copy link
Member Author

Confirmed that fix is valid with fbx as well.

image
image

Suzanne_shape.zip

@akien-mga akien-mga merged commit b9b07d6 into godotengine:master Sep 12, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

glTF importer does not check for invalid property name on blendshapes
5 participants