From 45c73775561808ccb2eb4a59927d9c59965e3d20 Mon Sep 17 00:00:00 2001 From: Fredia Huya-Kouadio Date: Sun, 17 Jul 2022 15:26:03 -0400 Subject: [PATCH] Refactor the export checking logic to improve separation of concerns --- editor/export/editor_export_platform.cpp | 18 +++++++++++++++ editor/export/editor_export_platform.h | 4 +++- editor/export/editor_export_platform_pc.cpp | 6 ++++- editor/export/editor_export_platform_pc.h | 3 ++- platform/android/export/export_plugin.cpp | 17 ++++++++++++-- platform/android/export/export_plugin.h | 3 ++- platform/ios/export/export_plugin.cpp | 15 ++++++++++-- platform/ios/export/export_plugin.h | 3 ++- platform/javascript/export/export_plugin.cpp | 24 ++++++++++++++++++-- platform/javascript/export/export_plugin.h | 3 ++- platform/macos/export/export_plugin.cpp | 13 ++++++++++- platform/macos/export/export_plugin.h | 3 ++- platform/uwp/export/export_plugin.cpp | 23 +++++++++++++++++-- platform/uwp/export/export_plugin.h | 3 ++- platform/windows/export/export_plugin.cpp | 15 ++++++++++-- platform/windows/export/export_plugin.h | 3 ++- 16 files changed, 136 insertions(+), 20 deletions(-) diff --git a/editor/export/editor_export_platform.cpp b/editor/export/editor_export_platform.cpp index b4d3973705c8..ab1586cb77a2 100644 --- a/editor/export/editor_export_platform.cpp +++ b/editor/export/editor_export_platform.cpp @@ -1176,5 +1176,23 @@ void EditorExportPlatform::gen_export_flags(Vector &r_flags, int p_flags } } +bool EditorExportPlatform::can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { + String templates_error; + bool valid_export_configuration = has_valid_export_configuration(p_preset, templates_error, r_missing_templates); + + String project_configuration_error; + bool valid_project_configuration = has_valid_project_configuration(p_preset, project_configuration_error); + + if (!templates_error.is_empty()) { + r_error += templates_error; + } + + if (!project_configuration_error.is_empty()) { + r_error += project_configuration_error; + } + + return valid_export_configuration && valid_project_configuration; +} + EditorExportPlatform::EditorExportPlatform() { } diff --git a/editor/export/editor_export_platform.h b/editor/export/editor_export_platform.h index ad1bcc1996e9..c870ee66aafa 100644 --- a/editor/export/editor_export_platform.h +++ b/editor/export/editor_export_platform.h @@ -200,7 +200,9 @@ class EditorExportPlatform : public RefCounted { virtual Ref get_run_icon() const { return get_logo(); } String test_etc2() const; - virtual bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const = 0; + bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const; + virtual bool has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const = 0; + virtual bool has_valid_project_configuration(const Ref &p_preset, String &r_error) const = 0; virtual List get_binary_extensions(const Ref &p_preset) const = 0; virtual Error export_project(const Ref &p_preset, bool p_debug, const String &p_path, int p_flags = 0) = 0; diff --git a/editor/export/editor_export_platform_pc.cpp b/editor/export/editor_export_platform_pc.cpp index 5e0044f2aebf..9fca4c908ad5 100644 --- a/editor/export/editor_export_platform_pc.cpp +++ b/editor/export/editor_export_platform_pc.cpp @@ -75,7 +75,7 @@ Ref EditorExportPlatformPC::get_logo() const { return logo; } -bool EditorExportPlatformPC::can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { +bool EditorExportPlatformPC::has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { String err; bool valid = false; @@ -106,6 +106,10 @@ bool EditorExportPlatformPC::can_export(const Ref &p_preset, return valid; } +bool EditorExportPlatformPC::has_valid_project_configuration(const Ref &p_preset, String &r_error) const { + return true; +} + Error EditorExportPlatformPC::export_project(const Ref &p_preset, bool p_debug, const String &p_path, int p_flags) { ExportNotifier notifier(*this, p_preset, p_debug, p_path, p_flags); diff --git a/editor/export/editor_export_platform_pc.h b/editor/export/editor_export_platform_pc.h index bdb86e924a47..cf96db6c2d76 100644 --- a/editor/export/editor_export_platform_pc.h +++ b/editor/export/editor_export_platform_pc.h @@ -52,7 +52,8 @@ class EditorExportPlatformPC : public EditorExportPlatform { virtual String get_os_name() const override; virtual Ref get_logo() const override; - virtual bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_project_configuration(const Ref &p_preset, String &r_error) const override; virtual Error export_project(const Ref &p_preset, bool p_debug, const String &p_path, int p_flags = 0) override; virtual Error sign_shared_object(const Ref &p_preset, bool p_debug, const String &p_path); virtual String get_template_file_name(const String &p_target, const String &p_arch) const = 0; diff --git a/platform/android/export/export_plugin.cpp b/platform/android/export/export_plugin.cpp index 6f1b4bde403e..34086add2ae1 100644 --- a/platform/android/export/export_plugin.cpp +++ b/platform/android/export/export_plugin.cpp @@ -2049,7 +2049,7 @@ String EditorExportPlatformAndroid::get_apksigner_path() { return apksigner_path; } -bool EditorExportPlatformAndroid::can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { +bool EditorExportPlatformAndroid::has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { String err; bool valid = false; const bool custom_build_enabled = p_preset->get("custom_build/use_custom_build"); @@ -2097,7 +2097,7 @@ bool EditorExportPlatformAndroid::can_export(const Ref &p_pr valid = installed_android_build_template && !r_missing_templates; } - // Validate the rest of the configuration. + // Validate the rest of the export configuration. String dk = p_preset->get("keystore/debug"); String dk_user = p_preset->get("keystore/debug_user"); @@ -2173,6 +2173,19 @@ bool EditorExportPlatformAndroid::can_export(const Ref &p_pr } } + if (!err.is_empty()) { + r_error = err; + } + + return valid; +} + +bool EditorExportPlatformAndroid::has_valid_project_configuration(const Ref &p_preset, String &r_error) const { + String err; + bool valid = true; + const bool custom_build_enabled = p_preset->get("custom_build/use_custom_build"); + + // Validate the project configuration. bool apk_expansion = p_preset->get("apk_expansion/enable"); if (apk_expansion) { diff --git a/platform/android/export/export_plugin.h b/platform/android/export/export_plugin.h index 1da3f68f9aad..94559670533e 100644 --- a/platform/android/export/export_plugin.h +++ b/platform/android/export/export_plugin.h @@ -186,7 +186,8 @@ class EditorExportPlatformAndroid : public EditorExportPlatform { static String get_apksigner_path(); - virtual bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_project_configuration(const Ref &p_preset, String &r_error) const override; virtual List get_binary_extensions(const Ref &p_preset) const override; diff --git a/platform/ios/export/export_plugin.cpp b/platform/ios/export/export_plugin.cpp index 001c9b803dfe..2771ab8ca251 100644 --- a/platform/ios/export/export_plugin.cpp +++ b/platform/ios/export/export_plugin.cpp @@ -1817,7 +1817,7 @@ Error EditorExportPlatformIOS::export_project(const Ref &p_p return OK; } -bool EditorExportPlatformIOS::can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { +bool EditorExportPlatformIOS::has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { String err; bool valid = false; @@ -1842,7 +1842,18 @@ bool EditorExportPlatformIOS::can_export(const Ref &p_preset valid = dvalid || rvalid; r_missing_templates = !valid; - // Validate the rest of the configuration. + if (!err.is_empty()) { + r_error = err; + } + + return valid; +} + +bool EditorExportPlatformIOS::has_valid_project_configuration(const Ref &p_preset, String &r_error) const { + String err; + bool valid = true; + + // Validate the project configuration. String team_id = p_preset->get("application/app_store_team_id"); if (team_id.length() == 0) { diff --git a/platform/ios/export/export_plugin.h b/platform/ios/export/export_plugin.h index e32aef82dd3c..8079eda8f68a 100644 --- a/platform/ios/export/export_plugin.h +++ b/platform/ios/export/export_plugin.h @@ -198,7 +198,8 @@ class EditorExportPlatformIOS : public EditorExportPlatform { } virtual Error export_project(const Ref &p_preset, bool p_debug, const String &p_path, int p_flags = 0) override; - virtual bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_project_configuration(const Ref &p_preset, String &r_error) const override; virtual void get_platform_features(List *r_features) const override { r_features->push_back("mobile"); diff --git a/platform/javascript/export/export_plugin.cpp b/platform/javascript/export/export_plugin.cpp index b99f88d067f2..0bdee110184b 100644 --- a/platform/javascript/export/export_plugin.cpp +++ b/platform/javascript/export/export_plugin.cpp @@ -362,7 +362,7 @@ Ref EditorExportPlatformJavaScript::get_logo() const { return logo; } -bool EditorExportPlatformJavaScript::can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { +bool EditorExportPlatformJavaScript::has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { #ifndef DEV_ENABLED // We don't provide export templates for the HTML5 platform currently as there // is no suitable renderer to use with them. So we forbid exporting and tell @@ -396,7 +396,27 @@ bool EditorExportPlatformJavaScript::can_export(const Ref &p valid = dvalid || rvalid; r_missing_templates = !valid; - // Validate the rest of the configuration. + if (!err.is_empty()) { + r_error = err; + } + + return valid; +} + +bool EditorExportPlatformJavaScript::has_valid_project_configuration(const Ref &p_preset, String &r_error) const { +#ifndef DEV_ENABLED + // We don't provide export templates for the HTML5 platform currently as there + // is no suitable renderer to use with them. So we forbid exporting and tell + // users why. This is skipped in DEV_ENABLED so that contributors can still test + // the pipeline once we start having WebGL or WebGPU support. + r_error = "The HTML5 platform is currently not supported in Godot 4.0, as there is no suitable renderer for it.\n"; + return false; +#endif + + String err; + bool valid = true; + + // Validate the project configuration. if (p_preset->get("vram_texture_compression/for_mobile")) { String etc_error = test_etc2(); diff --git a/platform/javascript/export/export_plugin.h b/platform/javascript/export/export_plugin.h index fbaa3615cb44..16bab02d540f 100644 --- a/platform/javascript/export/export_plugin.h +++ b/platform/javascript/export/export_plugin.h @@ -118,7 +118,8 @@ class EditorExportPlatformJavaScript : public EditorExportPlatform { virtual String get_os_name() const override; virtual Ref get_logo() const override; - virtual bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_project_configuration(const Ref &p_preset, String &r_error) const override; virtual List get_binary_extensions(const Ref &p_preset) const override; virtual Error export_project(const Ref &p_preset, bool p_debug, const String &p_path, int p_flags = 0) override; diff --git a/platform/macos/export/export_plugin.cpp b/platform/macos/export/export_plugin.cpp index bcc2636c076d..edce9c038009 100644 --- a/platform/macos/export/export_plugin.cpp +++ b/platform/macos/export/export_plugin.cpp @@ -1550,7 +1550,7 @@ void EditorExportPlatformMacOS::_zip_folder_recursive(zipFile &p_zip, const Stri da->list_dir_end(); } -bool EditorExportPlatformMacOS::can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { +bool EditorExportPlatformMacOS::has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { String err; bool valid = false; @@ -1580,6 +1580,17 @@ bool EditorExportPlatformMacOS::can_export(const Ref &p_pres valid = dvalid || rvalid; r_missing_templates = !valid; + if (!err.is_empty()) { + r_error = err; + } + + return valid; +} + +bool EditorExportPlatformMacOS::has_valid_project_configuration(const Ref &p_preset, String &r_error) const { + String err; + bool valid = true; + String identifier = p_preset->get("application/bundle_identifier"); String pn_err; if (!is_package_name_valid(identifier, &pn_err)) { diff --git a/platform/macos/export/export_plugin.h b/platform/macos/export/export_plugin.h index 21bc380d559a..4603c61a289e 100644 --- a/platform/macos/export/export_plugin.h +++ b/platform/macos/export/export_plugin.h @@ -119,7 +119,8 @@ class EditorExportPlatformMacOS : public EditorExportPlatform { } virtual Error export_project(const Ref &p_preset, bool p_debug, const String &p_path, int p_flags = 0) override; - virtual bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_project_configuration(const Ref &p_preset, String &r_error) const override; virtual void get_platform_features(List *r_features) const override { r_features->push_back("pc"); diff --git a/platform/uwp/export/export_plugin.cpp b/platform/uwp/export/export_plugin.cpp index 070c46242fc1..4e4afb970426 100644 --- a/platform/uwp/export/export_plugin.cpp +++ b/platform/uwp/export/export_plugin.cpp @@ -121,7 +121,7 @@ void EditorExportPlatformUWP::get_export_options(List *r_options) } } -bool EditorExportPlatformUWP::can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { +bool EditorExportPlatformUWP::has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { #ifndef DEV_ENABLED // We don't provide export templates for the UWP platform currently as it // has not been ported for Godot 4.0. This is skipped in DEV_ENABLED so that @@ -163,7 +163,26 @@ bool EditorExportPlatformUWP::can_export(const Ref &p_preset valid = dvalid || rvalid; r_missing_templates = !valid; - // Validate the rest of the configuration. + if (!err.is_empty()) { + r_error = err; + } + + return valid; +} + +bool EditorExportPlatformUWP::has_valid_project_configuration(const Ref &p_preset, String &r_error) const { +#ifndef DEV_ENABLED + // We don't provide export templates for the UWP platform currently as it + // has not been ported for Godot 4.0. This is skipped in DEV_ENABLED so that + // contributors can still test the pipeline if/when we can build it again. + r_error = "The UWP platform is currently not supported in Godot 4.0.\n"; + return false; +#endif + + String err; + bool valid = true; + + // Validate the project configuration. if (!_valid_resource_name(p_preset->get("package/short_name"))) { valid = false; diff --git a/platform/uwp/export/export_plugin.h b/platform/uwp/export/export_plugin.h index 4a3c5db377d8..71d047954311 100644 --- a/platform/uwp/export/export_plugin.h +++ b/platform/uwp/export/export_plugin.h @@ -429,7 +429,8 @@ class EditorExportPlatformUWP : public EditorExportPlatform { virtual void get_export_options(List *r_options) override; - virtual bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_project_configuration(const Ref &p_preset, String &r_error) const override; virtual Error export_project(const Ref &p_preset, bool p_debug, const String &p_path, int p_flags = 0) override; diff --git a/platform/windows/export/export_plugin.cpp b/platform/windows/export/export_plugin.cpp index febef5ad12fd..9d3ec31f7362 100644 --- a/platform/windows/export/export_plugin.cpp +++ b/platform/windows/export/export_plugin.cpp @@ -412,15 +412,26 @@ Error EditorExportPlatformWindows::_code_sign(const Ref &p_p return OK; } -bool EditorExportPlatformWindows::can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { +bool EditorExportPlatformWindows::has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const { String err = ""; - bool valid = EditorExportPlatformPC::can_export(p_preset, err, r_missing_templates); + bool valid = EditorExportPlatformPC::has_valid_export_configuration(p_preset, err, r_missing_templates); String rcedit_path = EditorSettings::get_singleton()->get("export/windows/rcedit"); if (p_preset->get("application/modify_resources") && rcedit_path.is_empty()) { err += TTR("The rcedit tool must be configured in the Editor Settings (Export > Windows > Rcedit) to change the icon or app information data.") + "\n"; } + if (!err.is_empty()) { + r_error = err; + } + + return valid; +} + +bool EditorExportPlatformWindows::has_valid_project_configuration(const Ref &p_preset, String &r_error) const { + String err = ""; + bool valid = true; + String icon_path = ProjectSettings::get_singleton()->globalize_path(p_preset->get("application/icon")); if (!icon_path.is_empty() && !FileAccess::exists(icon_path)) { err += TTR("Invalid icon path:") + " " + icon_path + "\n"; diff --git a/platform/windows/export/export_plugin.h b/platform/windows/export/export_plugin.h index b9e59829a01f..3ea8ff3dc996 100644 --- a/platform/windows/export/export_plugin.h +++ b/platform/windows/export/export_plugin.h @@ -49,7 +49,8 @@ class EditorExportPlatformWindows : public EditorExportPlatformPC { virtual List get_binary_extensions(const Ref &p_preset) const override; virtual void get_export_options(List *r_options) override; virtual bool get_export_option_visibility(const String &p_option, const HashMap &p_options) const override; - virtual bool can_export(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_export_configuration(const Ref &p_preset, String &r_error, bool &r_missing_templates) const override; + virtual bool has_valid_project_configuration(const Ref &p_preset, String &r_error) const override; virtual String get_template_file_name(const String &p_target, const String &p_arch) const override; virtual Error fixup_embedded_pck(const String &p_path, int64_t p_embedded_start, int64_t p_embedded_size) override; };