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

Fix same importer will be added multiple times in get_importers_for_extension #92718

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

jsjtxietian
Copy link
Contributor

Fixes #92710

The problem lies in the fact that both EditorSceneFormatImporterUFBX and EditorSceneFormatImporterFBX2GLTF can import .fbx, so local_exts for scene importer will contain duplicate fbx entry.

I suppose we can also make local_exts a unique set instead of list? But put a break here seems reasonable.

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

LGTM

@AThousandShips AThousandShips added cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jun 3, 2024
@AThousandShips AThousandShips requested a review from a team June 3, 2024 10:57
@akien-mga
Copy link
Member

What does this imply for actually being able to choose which importer to use? Like choosing between ufbx and fbx2gltf in the import settings.

@AThousandShips
Copy link
Member

AThousandShips commented Jun 3, 2024

This makes it so that each importer is added just once, it doesn't stop multiple importers from being selected, it just doesn't add the same importer for each matched extension

Though it might be better to identify why it has multiple different identical extensions

@jsjtxietian
Copy link
Contributor Author

What does this imply for actually being able to choose which importer to use?

IMHO what the import dock (or shall we say the current importer abstraction ) supports now is to choose a importer, it's up to the importer itself (like in this case, a scene importer) to decide how to import this file.

@akien-mga akien-mga merged commit 4629845 into godotengine:master Jun 11, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@jsjtxietian jsjtxietian deleted the importer-duplicate branch June 11, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:editor topic:import
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicating menu items in the FBX import menu
3 participants