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 logging when macOS export will fail due to disabled texture formats. #86769

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

StagnationPoint
Copy link
Contributor

Currently if a required texture format is disabled during macOS export, the export will fail without logging any information indicating the real source of the problem. This change adds error messaging that will direct the user toward the issue.

@StagnationPoint StagnationPoint requested a review from a team as a code owner January 4, 2024 05:07
Copy link
Member

@akien-mga akien-mga 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. Minor style nitpick.

platform/macos/export/export_plugin.cpp Outdated Show resolved Hide resolved
platform/macos/export/export_plugin.cpp Outdated Show resolved Hide resolved
…ts. Since ETC2 ASTC is required for universal builds, also ensure it is enabled for them.
@StagnationPoint
Copy link
Contributor Author

Thanks for the guidance, all! Much appreciated.

@akien-mga akien-mga merged commit 5b50df2 into godotengine:master Jan 5, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@@ -2051,13 +2051,17 @@ bool EditorExportPlatformMacOS::has_valid_export_configuration(const Ref<EditorE
String architecture = p_preset->get("binary_format/architecture");
if (architecture == "universal" || architecture == "x86_64") {
if (!ResourceImporterTextureSettings::should_import_s3tc_bptc()) {
err += TTR("Cannot export for universal or x86_64 if S3TC BPTC texture format is disabled. Enable it in the Project Settings (Rendering > Textures > VRAM Compression > Import S3TC BPTC).") + "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Enable it in the Project Settings [...]

Instead of requiring the user to dig into the settings, we should just provide a button to fix it. I implemented this in PR #78457, but the button seems to have disappeared somewhere in-between my PR and your PR? It's still present for Android, but missing for macOS for some weird reason.

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.

5 participants