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 browse_dialog in Blender scene importer to accept files #93411

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

raging-loon
Copy link
Contributor

@raging-loon raging-loon commented Jun 20, 2024

Changes EditorFileSystemImportFormatSupportQueryBlend::browse_dialog to accept file names. As it is now, it only accepts directory names. This will present an error as the callback "_select_install" and subsequent calls to "_validate_path" and "_get_blender_version" are all expecting filenames.

Closes #93235

@raging-loon raging-loon requested a review from a team as a code owner June 20, 2024 21:02
@raging-loon raging-loon marked this pull request as draft June 20, 2024 21:29
@raging-loon raging-loon marked this pull request as ready for review June 20, 2024 21:30
@raging-loon raging-loon changed the title changed browse_dialog in blender scene importer to accept files (issue #93235) Fixed browse_dialog in blender scene importer to accept files (issue #93235) Jun 20, 2024
@raging-loon raging-loon changed the title Fixed browse_dialog in blender scene importer to accept files (issue #93235) Fix browse_dialog in blender scene importer to accept files (issue #93235) Jun 20, 2024
@Calinou Calinou added this to the 4.3 milestone Jun 20, 2024
@AThousandShips AThousandShips changed the title Fix browse_dialog in blender scene importer to accept files (issue #93235) Fix browse_dialog in Blender scene importer to accept files Jun 21, 2024
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected.

However, the UI still refers to a Blender installation, which can be a bit ambiguous. I suggest renaming it to Blender executable instead.

image

On macOS, it might be worth hinting that you should point at Contents/MacOS/blender within Blender.app too (e.g. by appending to the label with #ifdef MACOS_ENABLED).

@raging-loon
Copy link
Contributor Author

I could change "installation" to "executable". However, for Mac support, it would be better for someone else handle that, as I have never touched a Mac let along program one.

Co-authored-by: Rémi Verschelde <rverschelde@gmail.com>
@akien-mga
Copy link
Member

I pushed an update myself that adjusts the label and error messages for clarity.

@akien-mga
Copy link
Member

For the record, I noticed #94914 while working on this, but it doesn't seem related to these changes, the bug also happens in RC 1.

@akien-mga akien-mga merged commit 107fed8 into godotengine:master Jul 29, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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.

Blend import dialog window ask for folder, not file.
3 participants