From ddced1ae62496b02dfba83378620dec87b3b10e8 Mon Sep 17 00:00:00 2001
From: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Date: Mon, 20 May 2024 15:21:07 +0200
Subject: [PATCH] Fix some arguments being passed as references in virtual
functions
These can't be used safely in extensions, replaced with `Dictionary`
returns
---
doc/classes/EditorImportPlugin.xml | 6 +--
.../EditorResourcePreviewGenerator.xml | 8 +---
doc/classes/EditorTranslationParserPlugin.xml | 4 +-
editor/editor_resource_preview.cpp | 28 ++++++++++----
editor/editor_resource_preview.h | 4 +-
editor/editor_translation_parser.cpp | 37 ++++++++++---------
editor/editor_translation_parser.h | 2 +-
editor/import/editor_import_plugin.cpp | 35 +++++++++++++-----
editor/import/editor_import_plugin.h | 2 +-
.../4.2-stable.expected | 14 +++++++
10 files changed, 89 insertions(+), 51 deletions(-)
diff --git a/doc/classes/EditorImportPlugin.xml b/doc/classes/EditorImportPlugin.xml
index 88a6eeec268b0..1a732340ee25f 100644
--- a/doc/classes/EditorImportPlugin.xml
+++ b/doc/classes/EditorImportPlugin.xml
@@ -223,14 +223,12 @@
-
+
-
-
- Imports [param source_file] into [param save_path] with the import [param options] specified. The [param platform_variants] and [param gen_files] arrays will be modified by this function.
+ Imports [param source_file] into [param save_path] with the import [param options] specified.
This method must be overridden to do the actual importing work. See this class' description for an example of overriding this method.
diff --git a/doc/classes/EditorResourcePreviewGenerator.xml b/doc/classes/EditorResourcePreviewGenerator.xml
index fcfdbb5c44be8..3d63ed102241c 100644
--- a/doc/classes/EditorResourcePreviewGenerator.xml
+++ b/doc/classes/EditorResourcePreviewGenerator.xml
@@ -17,27 +17,23 @@
-
+
-
Generate a preview from a given resource with the specified size. This must always be implemented.
Returning an empty texture is an OK way to fail and let another generator take care.
Care must be taken because this function is always called from a thread (not the main thread).
- [param metadata] dictionary can be modified to store file-specific metadata that can be used in [method EditorResourceTooltipPlugin._make_tooltip_for_path] (like image size, sample length etc.).
-
+
-
Generate a preview directly from a path with the specified size. Implementing this is optional, as default code will load and call [method _generate].
Returning an empty texture is an OK way to fail and let another generator take care.
Care must be taken because this function is always called from a thread (not the main thread).
- [param metadata] dictionary can be modified to store file-specific metadata that can be used in [method EditorResourceTooltipPlugin._make_tooltip_for_path] (like image size, sample length etc.).
diff --git a/doc/classes/EditorTranslationParserPlugin.xml b/doc/classes/EditorTranslationParserPlugin.xml
index a47a41594ef73..766700e6c4dfb 100644
--- a/doc/classes/EditorTranslationParserPlugin.xml
+++ b/doc/classes/EditorTranslationParserPlugin.xml
@@ -107,10 +107,8 @@
-
+
-
-
Override this method to define a custom parsing logic to extract the translatable strings.
diff --git a/editor/editor_resource_preview.cpp b/editor/editor_resource_preview.cpp
index 742d29fef030f..16b64bba89a6f 100644
--- a/editor/editor_resource_preview.cpp
+++ b/editor/editor_resource_preview.cpp
@@ -52,17 +52,29 @@ bool EditorResourcePreviewGenerator::handles(const String &p_type) const {
}
Ref EditorResourcePreviewGenerator::generate(const Ref &p_from, const Size2 &p_size, Dictionary &p_metadata) const {
- Ref preview;
- if (GDVIRTUAL_CALL(_generate, p_from, p_size, p_metadata, preview)) {
- return preview;
+ Dictionary ret;
+ if (GDVIRTUAL_CALL(_generate, p_from, p_size, ret)) {
+ if (!ret.has("result")) {
+ return Ref();
+ }
+ if (ret.has("metadata")) {
+ p_metadata = ret["metadata"];
+ }
+ return ret["result"];
}
ERR_FAIL_V_MSG(Ref(), "EditorResourcePreviewGenerator::_generate needs to be overridden.");
}
Ref EditorResourcePreviewGenerator::generate_from_path(const String &p_path, const Size2 &p_size, Dictionary &p_metadata) const {
- Ref preview;
- if (GDVIRTUAL_CALL(_generate_from_path, p_path, p_size, p_metadata, preview)) {
- return preview;
+ Dictionary ret;
+ if (GDVIRTUAL_CALL(_generate_from_path, p_path, p_size, ret)) {
+ if (!ret.has("result")) {
+ return Ref();
+ }
+ if (ret.has("metadata")) {
+ p_metadata = ret["metadata"];
+ }
+ return ret["result"];
}
Ref res = ResourceLoader::load(p_path);
@@ -86,8 +98,8 @@ bool EditorResourcePreviewGenerator::can_generate_small_preview() const {
void EditorResourcePreviewGenerator::_bind_methods() {
GDVIRTUAL_BIND(_handles, "type");
- GDVIRTUAL_BIND(_generate, "resource", "size", "metadata");
- GDVIRTUAL_BIND(_generate_from_path, "path", "size", "metadata");
+ GDVIRTUAL_BIND(_generate, "resource", "size");
+ GDVIRTUAL_BIND(_generate_from_path, "path", "size");
GDVIRTUAL_BIND(_generate_small_preview_automatically);
GDVIRTUAL_BIND(_can_generate_small_preview);
}
diff --git a/editor/editor_resource_preview.h b/editor/editor_resource_preview.h
index 2870f9a2019b9..0880c09bc1c41 100644
--- a/editor/editor_resource_preview.h
+++ b/editor/editor_resource_preview.h
@@ -46,8 +46,8 @@ class EditorResourcePreviewGenerator : public RefCounted {
static void _bind_methods();
GDVIRTUAL1RC(bool, _handles, String)
- GDVIRTUAL3RC(Ref, _generate, Ref, Vector2i, Dictionary)
- GDVIRTUAL3RC(Ref, _generate_from_path, String, Vector2i, Dictionary)
+ GDVIRTUAL2RC(Dictionary, _generate, Ref, Vector2i)
+ GDVIRTUAL2RC(Dictionary, _generate_from_path, String, Vector2i)
GDVIRTUAL0RC(bool, _generate_small_preview_automatically)
GDVIRTUAL0RC(bool, _can_generate_small_preview)
diff --git a/editor/editor_translation_parser.cpp b/editor/editor_translation_parser.cpp
index 8a77ce4a82ffa..a50e5e1cd1c97 100644
--- a/editor/editor_translation_parser.cpp
+++ b/editor/editor_translation_parser.cpp
@@ -38,25 +38,28 @@
EditorTranslationParser *EditorTranslationParser::singleton = nullptr;
Error EditorTranslationParserPlugin::parse_file(const String &p_path, Vector *r_ids, Vector> *r_ids_ctx_plural) {
- TypedArray ids;
- TypedArray ids_ctx_plural;
-
- if (GDVIRTUAL_CALL(_parse_file, p_path, ids, ids_ctx_plural)) {
- // Add user's extracted translatable messages.
- for (int i = 0; i < ids.size(); i++) {
- r_ids->append(ids[i]);
+ Dictionary ret;
+
+ if (GDVIRTUAL_CALL(_parse_file, p_path, ret)) {
+ // TODO: Errors for invalid return?
+ if (ret.has("ids")) {
+ TypedArray ids = ret["ids"];
+ for (const String id : ids) {
+ r_ids->push_back(id);
+ }
}
- // Add user's collected translatable messages with context or plurals.
- for (int i = 0; i < ids_ctx_plural.size(); i++) {
- Array arr = ids_ctx_plural[i];
- ERR_FAIL_COND_V_MSG(arr.size() != 3, ERR_INVALID_DATA, "Array entries written into `msgids_context_plural` in `parse_file()` method should have the form [\"message\", \"context\", \"plural message\"]");
+ if (ret.has("ids_ctx_plural")) {
+ TypedArray ids_ctx_plural = ret["ids_ctx_plural"];
+ for (const Array arr : ids_ctx_plural) {
+ ERR_FAIL_COND_V_MSG(arr.size() != 3, ERR_INVALID_DATA, "Array entries written into `msgids_context_plural` in `parse_file()` method should have the form [\"message\", \"context\", \"plural message\"]");
- Vector id_ctx_plural;
- id_ctx_plural.push_back(arr[0]);
- id_ctx_plural.push_back(arr[1]);
- id_ctx_plural.push_back(arr[2]);
- r_ids_ctx_plural->append(id_ctx_plural);
+ Vector id_ctx_plural;
+ id_ctx_plural.push_back(arr[0]);
+ id_ctx_plural.push_back(arr[1]);
+ id_ctx_plural.push_back(arr[2]);
+ r_ids_ctx_plural->push_back(id_ctx_plural);
+ }
}
return OK;
} else {
@@ -77,7 +80,7 @@ void EditorTranslationParserPlugin::get_recognized_extensions(List *r_ex
}
void EditorTranslationParserPlugin::_bind_methods() {
- GDVIRTUAL_BIND(_parse_file, "path", "msgids", "msgids_context_plural");
+ GDVIRTUAL_BIND(_parse_file, "path");
GDVIRTUAL_BIND(_get_recognized_extensions);
}
diff --git a/editor/editor_translation_parser.h b/editor/editor_translation_parser.h
index 78dc726c52e0e..beecd172bc7f3 100644
--- a/editor/editor_translation_parser.h
+++ b/editor/editor_translation_parser.h
@@ -42,7 +42,7 @@ class EditorTranslationParserPlugin : public RefCounted {
protected:
static void _bind_methods();
- GDVIRTUAL3(_parse_file, String, TypedArray, TypedArray)
+ GDVIRTUAL1R(Dictionary, _parse_file, String)
GDVIRTUAL0RC(Vector, _get_recognized_extensions)
public:
diff --git a/editor/import/editor_import_plugin.cpp b/editor/import/editor_import_plugin.cpp
index 3243dcf256958..0f32fa4da8c4f 100644
--- a/editor/import/editor_import_plugin.cpp
+++ b/editor/import/editor_import_plugin.cpp
@@ -165,7 +165,6 @@ bool EditorImportPlugin::get_option_visibility(const String &p_path, const Strin
Error EditorImportPlugin::import(const String &p_source_file, const String &p_save_path, const HashMap &p_options, List *r_platform_variants, List *r_gen_files, Variant *r_metadata) {
Dictionary options;
- TypedArray platform_variants, gen_files;
HashMap::ConstIterator E = p_options.begin();
while (E) {
@@ -173,15 +172,33 @@ Error EditorImportPlugin::import(const String &p_source_file, const String &p_sa
++E;
}
- Error err = OK;
- if (GDVIRTUAL_CALL(_import, p_source_file, p_save_path, options, platform_variants, gen_files, err)) {
- for (int i = 0; i < platform_variants.size(); i++) {
- r_platform_variants->push_back(platform_variants[i]);
+ Dictionary ret;
+ if (GDVIRTUAL_CALL(_import, p_source_file, p_save_path, options, ret)) {
+ if (!ret.has("result")) {
+ return ERR_UNAVAILABLE;
}
- for (int i = 0; i < gen_files.size(); i++) {
- r_gen_files->push_back(gen_files[i]);
+
+ if (r_platform_variants != nullptr && ret.has("platform_variants")) {
+ TypedArray platform_variants = ret["platform_variants"];
+ for (const String platform_variant : platform_variants) {
+ r_platform_variants->push_back(platform_variant);
+ }
+ }
+
+ if (r_gen_files != nullptr && ret.has("gen_files")) {
+ TypedArray gen_files = ret["gen_files"];
+ for (const String gen_file : gen_files) {
+ r_gen_files->push_back(gen_file);
+ }
}
- return err;
+
+ if (r_metadata != nullptr && ret.has("metadata")) {
+ *r_metadata = ret["metadata"];
+ }
+
+ Error result = Error(int(ret["result"]));
+
+ return result;
}
ERR_FAIL_V_MSG(ERR_METHOD_NOT_FOUND, "Unimplemented _import in add-on.");
}
@@ -221,7 +238,7 @@ void EditorImportPlugin::_bind_methods() {
GDVIRTUAL_BIND(_get_priority)
GDVIRTUAL_BIND(_get_import_order)
GDVIRTUAL_BIND(_get_option_visibility, "path", "option_name", "options")
- GDVIRTUAL_BIND(_import, "source_file", "save_path", "options", "platform_variants", "gen_files");
+ GDVIRTUAL_BIND(_import, "source_file", "save_path", "options");
GDVIRTUAL_BIND(_can_import_threaded);
ClassDB::bind_method(D_METHOD("append_import_external_resource", "path", "custom_options", "custom_importer", "generator_parameters"), &EditorImportPlugin::_append_import_external_resource, DEFVAL(Dictionary()), DEFVAL(String()), DEFVAL(Variant()));
}
diff --git a/editor/import/editor_import_plugin.h b/editor/import/editor_import_plugin.h
index ea5cfc2682dc0..3934b8948c84f 100644
--- a/editor/import/editor_import_plugin.h
+++ b/editor/import/editor_import_plugin.h
@@ -51,7 +51,7 @@ class EditorImportPlugin : public ResourceImporter {
GDVIRTUAL0RC(float, _get_priority)
GDVIRTUAL0RC(int, _get_import_order)
GDVIRTUAL3RC(bool, _get_option_visibility, String, StringName, Dictionary)
- GDVIRTUAL5RC(Error, _import, String, String, Dictionary, TypedArray, TypedArray)
+ GDVIRTUAL3RC(Dictionary, _import, String, String, Dictionary)
GDVIRTUAL0RC(bool, _can_import_threaded)
Error _append_import_external_resource(const String &p_file, const Dictionary &p_custom_options = Dictionary(), const String &p_custom_importer = String(), Variant p_generator_parameters = Variant());
diff --git a/misc/extension_api_validation/4.2-stable.expected b/misc/extension_api_validation/4.2-stable.expected
index b8f244ca0237b..3b6d44c460c18 100644
--- a/misc/extension_api_validation/4.2-stable.expected
+++ b/misc/extension_api_validation/4.2-stable.expected
@@ -365,3 +365,17 @@ Validate extension JSON: Error: Field 'classes/Animation/methods/track_find_key/
Added optional arguments to avoid finding keys out of the animation range (GH-86661), and to handle backward seeking.
Compatibility method registered.
+
+
+GH-92175
+--------
+Validate extension JSON: Error: Field 'classes/EditorImportPlugin/methods/_import/arguments': size changed value in new API, from 5 to 3.
+Validate extension JSON: Error: Field 'classes/EditorImportPlugin/methods/_import/return_value': type changed value in new API, from "enum::Error" to "Dictionary".
+Validate extension JSON: Error: Field 'classes/EditorResourcePreviewGenerator/methods/_generate/arguments': size changed value in new API, from 3 to 2.
+Validate extension JSON: Error: Field 'classes/EditorResourcePreviewGenerator/methods/_generate/return_value': type changed value in new API, from "Texture2D" to "Dictionary".
+Validate extension JSON: Error: Field 'classes/EditorResourcePreviewGenerator/methods/_generate_from_path/arguments': size changed value in new API, from 3 to 2.
+Validate extension JSON: Error: Field 'classes/EditorResourcePreviewGenerator/methods/_generate_from_path/return_value': type changed value in new API, from "Texture2D" to "Dictionary".
+Validate extension JSON: Error: Field 'classes/EditorTranslationParserPlugin/methods/_parse_file/arguments': size changed value in new API, from 3 to 1.
+Validate extension JSON: JSON file: Field was added in a way that breaks compatibility 'classes/EditorTranslationParserPlugin/methods/_parse_file': return_value
+
+Return values were passed in constant reference arguments. Changed to using a return Dictionary.