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

Remove unused EditorSceneFormatImporter::_get_import_flags #101531

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Jan 14, 2025

This has never been used since Godot was open sourced.

Import flags are used but directly through _import_scene.

We normally try not to break compatibility by removing APIs, but since this one really does nothing, it doesn't seem to serve any purpose to keep it until 5.0 (if we ever remember to remove it then).

@dsnopek
Copy link
Contributor

dsnopek commented Jan 14, 2025

Removing an unused virtual method should be fine.

When a virtual method is called, Godot asks the GDExtension if it has an implementation of that virtual method. Removing the virtual method basically means Godot will never ask, which won't lead to any crashes or anything.

The only compatibility break would be if existing GDExtensions had implementations of this virtual method, and their functionality depended on it being called. But since it's unused, there shouldn't be any such GDExtensions.

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.

Reasonable

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Jan 14, 2025
This has never been used since Godot was open sourced.

Import flags are used but directly through `_import_scene`.
@akien-mga akien-mga force-pushed the scene-import-remove-unused-_get_import_flags branch from 75895fd to 21fcb56 Compare January 14, 2025 15:57
@akien-mga akien-mga merged commit 67f54bd into godotengine:master Jan 14, 2025
20 checks passed
@akien-mga akien-mga deleted the scene-import-remove-unused-_get_import_flags branch January 14, 2025 17:34
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.

3 participants