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 referring to ResourceImporterScene static importers via instances #82988

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Oct 8, 2023

(Note: This will conflict with #82899. We can merge in either order, I will rebase the other PR).

In a small number of places (once in the glTF module and multiple times in the filesystem dock), static data in the ResourceImporterScene class was being referred to by the singleton instance when it did not need to be. This PR fixes those places.

Part of the problem is the confusion between the parent class ResourceImporter working with "importer"s and this class ResourceImporterScene working with "importer"s. This PR includes a rename of the ResourceImporterScene importer methods to "scene_importer" to avoid confusion with ResourceImporter's "importer".

This PR also reduces code duplication in the filesystem dock, previously it was choosing between the different singletons but now that is no longer necessary. ResourceImporterScene::get_scene_importer_extensions is accessing static data so we don't need to use the singletons. The show_advanced_options method is just a shortcut for accessing the scene import dialog and passing along with a bool for if this is the animation dialog or not, so instead of using the singletons of the importer we should just have this part of the editor UI talk to the other part of the editor UI.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Dec 12, 2023
@akien-mga akien-mga merged commit d7d53cf into godotengine:master Dec 12, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@aaronfranke aaronfranke deleted the res-imp-scene-static branch December 12, 2023 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants