From 431558d419dc7263dfebe6045a8457c52fc0e453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Fri, 1 Jul 2022 21:19:32 +0200 Subject: [PATCH] Android: Only warn when Target SDK is non default The validation logic for Min Sdk and Target Sdk was flawed as it triggers errors for users not using Custom Build even if they never modified the values themselves. The default values for those settings get saved in `export_presets.cfg` and thus the error gets triggered when moving from 3.4.4 or earlier to 3.4.5, as the target SDK changed from 30 to 31. So instead we just show a warning to make users aware of this non-default Min Sdk or mostly Target Sdk that might be in their preset. We also warn when they do use Custom Build as the target SDK 30 would likely still be an upgrade issue and not an intentional choice, especially given that Google Play will now require SDK 31. The export info dialog is now exclusive so that when it doesn't auto-close, i.e. when it errors, you don't close it by mistake by clicking outside. The valid range for both options is no longer limited to Godot's own default target SDK, but can accept higher values (they are not guaranteed to work, but they might). To be able to display warning, the relevant code is backported from 30ee208bd. Fixes #62465 without breaking compatibility for 3.4.5. --- editor/editor_node.cpp | 2 ++ editor/project_export.cpp | 21 ++++++++++++++- editor/project_export.h | 1 + platform/android/export/export_plugin.cpp | 32 ++++++++++++++++------- 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index 4a52ee065efc..234fef462c7f 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -6995,6 +6995,8 @@ EditorNode::EditorNode() { execute_output_dialog = memnew(AcceptDialog); execute_output_dialog->add_child(execute_outputs); execute_output_dialog->set_title(""); + // Prevent closing too fast and missing important information. + execute_output_dialog->set_exclusive(true); gui_base->add_child(execute_output_dialog); EditorFileSystem::get_singleton()->connect("sources_changed", this, "_sources_changed"); diff --git a/editor/project_export.cpp b/editor/project_export.cpp index 9c28613bf129..53c5aaf04126 100644 --- a/editor/project_export.cpp +++ b/editor/project_export.cpp @@ -246,12 +246,12 @@ void ProjectExportDialog::_edit_preset(int p_index) { } error += " - " + items[i]; } - export_error->set_text(error); export_error->show(); } else { export_error->hide(); } + export_warning->hide(); if (needs_templates) { export_templates_error->show(); } else { @@ -262,6 +262,20 @@ void ProjectExportDialog::_edit_preset(int p_index) { get_ok()->set_disabled(true); } else { + if (error != String()) { + Vector items = error.split("\n", false); + error = ""; + for (int i = 0; i < items.size(); i++) { + if (i > 0) { + error += "\n"; + } + error += " - " + items[i]; + } + export_warning->set_text(error); + export_warning->show(); + } else { + export_warning->hide(); + } export_error->hide(); export_templates_error->hide(); export_button->set_disabled(false); @@ -1144,6 +1158,11 @@ ProjectExportDialog::ProjectExportDialog() { export_error->hide(); export_error->add_color_override("font_color", EditorNode::get_singleton()->get_gui_base()->get_color("error_color", "Editor")); + export_warning = memnew(Label); + main_vb->add_child(export_warning); + export_warning->hide(); + export_warning->add_color_override("font_color", EditorNode::get_singleton()->get_gui_base()->get_color("warning_color", "Editor")); + export_templates_error = memnew(HBoxContainer); main_vb->add_child(export_templates_error); export_templates_error->hide(); diff --git a/editor/project_export.h b/editor/project_export.h index 1fada22975f5..7201255a702e 100644 --- a/editor/project_export.h +++ b/editor/project_export.h @@ -99,6 +99,7 @@ class ProjectExportDialog : public ConfirmationDialog { Label *script_key_error; Label *export_error; + Label *export_warning; HBoxContainer *export_templates_error; String default_filename; diff --git a/platform/android/export/export_plugin.cpp b/platform/android/export/export_plugin.cpp index cea215671a95..8b95368d885f 100644 --- a/platform/android/export/export_plugin.cpp +++ b/platform/android/export/export_plugin.cpp @@ -226,7 +226,7 @@ static const char *AAB_ASSETS_DIRECTORY = "res://android/build/assetPacks/instal static const int DEFAULT_MIN_SDK_VERSION = 19; // Should match the value in 'platform/android/java/app/config.gradle#minSdk' static const int DEFAULT_TARGET_SDK_VERSION = 31; // Should match the value in 'platform/android/java/app/config.gradle#targetSdk' -const String SDK_VERSION_RANGE = vformat("%s,%s,1", DEFAULT_MIN_SDK_VERSION, DEFAULT_TARGET_SDK_VERSION); +const String SDK_VERSION_RANGE = vformat("%s,%s,1,or_greater", DEFAULT_MIN_SDK_VERSION, DEFAULT_TARGET_SDK_VERSION + 1); void EditorExportPlatformAndroid::_check_for_changes_poll_thread(void *ud) { EditorExportPlatformAndroid *ea = (EditorExportPlatformAndroid *)ud; @@ -2249,20 +2249,34 @@ bool EditorExportPlatformAndroid::can_export(const Ref &p_pr err += "\n"; } - // Check the min sdk version + // Check the min and target sdk version. + + // They're only used for custom_build_enabled, but since we save their default values + // in the export preset, users would get an unexpected error when updating to a Godot + // version that has different values (GH-62465). + // So we don't make a blocking error, instead we just show a warning. + int min_sdk_version = p_preset->get("version/min_sdk"); if (min_sdk_version != DEFAULT_MIN_SDK_VERSION && !custom_build_enabled) { - valid = false; - err += TTR("Changing the \"Min Sdk\" is only valid when \"Use Custom Build\" is enabled."); + err += vformat(TTR("\"Min Sdk\" was changed from the default \"%d\" to \"%d\". This option requires \"Use Custom Build\" to be enabled.\n>> Change it to \"%d\" to silence this warning, or enable \"Use Custom Build\" to use this min SDK."), DEFAULT_MIN_SDK_VERSION, min_sdk_version, DEFAULT_MIN_SDK_VERSION); err += "\n"; } - // Check the target sdk version + // Here we also handle compatibility with Godot 3.4 to 3.4.4 where target SDK was 30. + // Version 3.4.5 updated it to 31 to match platform requirements, so make sure that + // users notice it. int target_sdk_version = p_preset->get("version/target_sdk"); - if (target_sdk_version != DEFAULT_TARGET_SDK_VERSION && !custom_build_enabled) { - valid = false; - err += TTR("Changing the \"Target Sdk\" is only valid when \"Use Custom Build\" is enabled."); - err += "\n"; + if (target_sdk_version != DEFAULT_TARGET_SDK_VERSION) { + if (!custom_build_enabled) { + err += vformat(TTR("\"Target Sdk\" was changed from the default \"%d\" to \"%d\". This option requires \"Use Custom Build\" to be enabled.\n>> Change it to \"%d\" to silence this warning, or enable \"Use Custom Build\" to use this target SDK."), DEFAULT_TARGET_SDK_VERSION, target_sdk_version, DEFAULT_TARGET_SDK_VERSION); + err += "\n"; + } else if (target_sdk_version == 30) { // Compatibility with < 3.4.5. + err += vformat(TTR("\"Target Sdk\" is set to 30, while the current default is \"%d\". This might be due to upgrading from a previous Godot release.\n>> Consider changing it to \"%d\" to stay up-to-date with platform requirements."), DEFAULT_TARGET_SDK_VERSION, DEFAULT_TARGET_SDK_VERSION); + err += "\n"; + } else if (target_sdk_version > DEFAULT_TARGET_SDK_VERSION) { + err += vformat(TTR("\"Target Sdk\" %d is higher than the default version %d. This may work, but wasn't tested and may be unstable."), target_sdk_version, DEFAULT_TARGET_SDK_VERSION); + err += "\n"; + } } if (target_sdk_version < min_sdk_version) {