Browse Source

Merge pull request #87712 from akien-mga/revert-gdscript-uid-annotations-for-now

Revert "Add UID support to GDScript files" (for now)
Rémi Verschelde 1 year ago
parent
commit
9ab5cedef6

+ 0 - 1
editor/plugins/script_text_editor.cpp

@@ -146,7 +146,6 @@ void ScriptTextEditor::set_edited_resource(const Ref<Resource> &p_res) {
 	ERR_FAIL_COND(p_res.is_null());
 
 	script = p_res;
-	script->connect_changed(callable_mp((ScriptEditorBase *)this, &ScriptEditorBase::reload_text));
 
 	code_editor->get_text_editor()->set_text(script->get_source_code());
 	code_editor->get_text_editor()->clear_undo_history();

+ 1 - 9
modules/gdscript/doc_classes/@GDScript.xml

@@ -627,7 +627,7 @@
 				[/codeblock]
 				[b]Note:[/b] Only the script can have a custom icon. Inner classes are not supported.
 				[b]Note:[/b] As annotations describe their subject, the [annotation @icon] annotation must be placed before the class definition and inheritance.
-				[b]Note:[/b] Unlike most other annotations, the argument of the [annotation @icon] annotation must be a string literal (constant expressions are not supported).
+				[b]Note:[/b] Unlike other annotations, the argument of the [annotation @icon] annotation must be a string literal (constant expressions are not supported).
 			</description>
 		</annotation>
 		<annotation name="@onready">
@@ -681,14 +681,6 @@
 				[b]Note:[/b] As annotations describe their subject, the [annotation @tool] annotation must be placed before the class definition and inheritance.
 			</description>
 		</annotation>
-		<annotation name="@uid">
-			<return type="void" />
-			<param index="0" name="uid" type="String" />
-			<description>
-				Stores information about UID of this script. This annotation is auto-generated when saving the script and must not be modified manually. Only applies to scripts saved as separate files (i.e. not built-in).
-				[b]Note:[/b] Unlike most other annotations, the argument of the [annotation @uid] annotation must be a string literal (constant expressions are not supported).
-			</description>
-		</annotation>
 		<annotation name="@warning_ignore" qualifiers="vararg">
 			<return type="void" />
 			<param index="0" name="warning" type="String" />

+ 11 - 111
modules/gdscript/gdscript.cpp

@@ -55,7 +55,6 @@
 
 #ifdef TOOLS_ENABLED
 #include "editor/editor_paths.h"
-#include "editor/editor_settings.h"
 #endif
 
 #include <stdint.h>
@@ -1077,36 +1076,6 @@ Ref<GDScript> GDScript::get_base() const {
 	return base;
 }
 
-String GDScript::get_raw_source_code(const String &p_path, bool *r_error) {
-	Ref<FileAccess> f = FileAccess::open(p_path, FileAccess::READ);
-	if (f.is_null()) {
-		if (r_error) {
-			*r_error = true;
-		}
-		return String();
-	}
-	return f->get_as_utf8_string();
-}
-
-Vector2i GDScript::get_uid_lines(const String &p_source) {
-	GDScriptParser parser;
-	parser.parse(p_source, "", false);
-	const GDScriptParser::ClassNode *c = parser.get_tree();
-	if (!c) {
-		return Vector2i(-1, -1);
-	}
-	return c->uid_lines;
-}
-
-String GDScript::create_uid_line(const String &p_uid_str) {
-#ifdef TOOLS_ENABLED
-	if (EDITOR_GET("text_editor/completion/use_single_quotes")) {
-		return vformat(R"(@uid('%s') # %s)", p_uid_str, RTR("Generated automatically, do not modify."));
-	}
-#endif
-	return vformat(R"(@uid("%s") # %s)", p_uid_str, RTR("Generated automatically, do not modify."));
-}
-
 bool GDScript::inherits_script(const Ref<Script> &p_script) const {
 	Ref<GDScript> gd = p_script;
 	if (gd.is_null()) {
@@ -2624,8 +2593,17 @@ bool GDScriptLanguage::handles_global_class_type(const String &p_type) const {
 }
 
 String GDScriptLanguage::get_global_class_name(const String &p_path, String *r_base_type, String *r_icon_path) const {
+	Error err;
+	Ref<FileAccess> f = FileAccess::open(p_path, FileAccess::READ, &err);
+	if (err) {
+		return String();
+	}
+
+	String source = f->get_as_utf8_string();
+
 	GDScriptParser parser;
-	parser.parse(GDScript::get_raw_source_code(p_path), p_path, false);
+	err = parser.parse(source, p_path, false);
+
 	const GDScriptParser::ClassNode *c = parser.get_tree();
 	if (!c) {
 		return String(); // No class parsed.
@@ -2839,22 +2817,6 @@ String ResourceFormatLoaderGDScript::get_resource_type(const String &p_path) con
 	return "";
 }
 
-ResourceUID::ID ResourceFormatLoaderGDScript::get_resource_uid(const String &p_path) const {
-	String ext = p_path.get_extension().to_lower();
-
-	if (ext != "gd") {
-		return ResourceUID::INVALID_ID;
-	}
-
-	GDScriptParser parser;
-	parser.parse(GDScript::get_raw_source_code(p_path), p_path, false);
-	const GDScriptParser::ClassNode *c = parser.get_tree();
-	if (!c) {
-		return ResourceUID::INVALID_ID;
-	}
-	return ResourceUID::get_singleton()->text_to_id(c->uid_string);
-}
-
 void ResourceFormatLoaderGDScript::get_dependencies(const String &p_path, List<String> *p_dependencies, bool p_add_types) {
 	Ref<FileAccess> file = FileAccess::open(p_path, FileAccess::READ);
 	ERR_FAIL_COND_MSG(file.is_null(), "Cannot open file '" + p_path + "'.");
@@ -2879,49 +2841,17 @@ Error ResourceFormatSaverGDScript::save(const Ref<Resource> &p_resource, const S
 	ERR_FAIL_COND_V(sqscr.is_null(), ERR_INVALID_PARAMETER);
 
 	String source = sqscr->get_source_code();
-	ResourceUID::ID uid = ResourceSaver::get_resource_id_for_path(p_path, !p_resource->is_built_in());
 
 	{
-		bool source_changed = false;
 		Error err;
 		Ref<FileAccess> file = FileAccess::open(p_path, FileAccess::WRITE, &err);
 
 		ERR_FAIL_COND_V_MSG(err, err, "Cannot save GDScript file '" + p_path + "'.");
 
-		if (uid != ResourceUID::INVALID_ID) {
-			GDScriptParser parser;
-			parser.parse(source, "", false);
-			const GDScriptParser::ClassNode *c = parser.get_tree();
-			if (c && ResourceUID::get_singleton()->text_to_id(c->uid_string) != uid) {
-				const Vector2i &uid_idx = c->uid_lines;
-				PackedStringArray lines = source.split("\n");
-
-				if (uid_idx.x > -1) {
-					for (int i = uid_idx.x + 1; i <= uid_idx.y; i++) {
-						// If UID is written across multiple lines, erase extra lines.
-						lines.remove_at(uid_idx.x + 1);
-					}
-					lines.write[uid_idx.x] = GDScript::create_uid_line(ResourceUID::get_singleton()->id_to_text(uid));
-				} else {
-					lines.insert(0, GDScript::create_uid_line(ResourceUID::get_singleton()->id_to_text(uid)));
-				}
-				source = String("\n").join(lines);
-				source_changed = true;
-				file->store_string(String("\n").join(lines));
-			} else {
-				file->store_string(source);
-			}
-		}
-
+		file->store_string(source);
 		if (file->get_error() != OK && file->get_error() != ERR_FILE_EOF) {
 			return ERR_CANT_CREATE;
 		}
-
-		if (source_changed) {
-			sqscr->set_source_code(source);
-			sqscr->reload();
-			sqscr->emit_changed();
-		}
 	}
 
 	if (ScriptServer::is_reload_scripts_on_save_enabled()) {
@@ -2940,33 +2870,3 @@ void ResourceFormatSaverGDScript::get_recognized_extensions(const Ref<Resource>
 bool ResourceFormatSaverGDScript::recognize(const Ref<Resource> &p_resource) const {
 	return Object::cast_to<GDScript>(*p_resource) != nullptr;
 }
-
-Error ResourceFormatSaverGDScript::set_uid(const String &p_path, ResourceUID::ID p_uid) {
-	ERR_FAIL_COND_V(p_path.get_extension() != "gd", ERR_INVALID_PARAMETER);
-	ERR_FAIL_COND_V(p_uid == ResourceUID::INVALID_ID, ERR_INVALID_PARAMETER);
-
-	bool error = false;
-	const String &source_code = GDScript::get_raw_source_code(p_path, &error);
-	if (error) {
-		return ERR_CANT_OPEN;
-	}
-
-	Ref<FileAccess> f = FileAccess::open(p_path, FileAccess::WRITE);
-	ERR_FAIL_COND_V(f.is_null(), ERR_CANT_OPEN);
-
-	const Vector2i &uid_idx = GDScript::get_uid_lines(source_code);
-	PackedStringArray lines = source_code.split("\n");
-
-	if (uid_idx.x > -1) {
-		for (int i = uid_idx.x + 1; i <= uid_idx.y; i++) {
-			// If UID is written across multiple lines, erase extra lines.
-			lines.remove_at(uid_idx.x + 1);
-		}
-		lines.write[uid_idx.x] = GDScript::create_uid_line(ResourceUID::get_singleton()->id_to_text(p_uid));
-	} else {
-		f->store_line(GDScript::create_uid_line(ResourceUID::get_singleton()->id_to_text(p_uid)));
-	}
-	f->store_string(String("\n").join(lines));
-
-	return OK;
-}

+ 0 - 6
modules/gdscript/gdscript.h

@@ -262,10 +262,6 @@ public:
 	bool is_tool() const override { return tool; }
 	Ref<GDScript> get_base() const;
 
-	static String get_raw_source_code(const String &p_path, bool *r_error = nullptr);
-	static Vector2i get_uid_lines(const String &p_source);
-	static String create_uid_line(const String &p_uid_str);
-
 	const HashMap<StringName, MemberInfo> &debug_get_member_indices() const { return member_indices; }
 	const HashMap<StringName, GDScriptFunction *> &debug_get_member_functions() const; //this is debug only
 	StringName debug_get_member_by_index(int p_idx) const;
@@ -620,7 +616,6 @@ public:
 	virtual void get_recognized_extensions(List<String> *p_extensions) const;
 	virtual bool handles_type(const String &p_type) const;
 	virtual String get_resource_type(const String &p_path) const;
-	virtual ResourceUID::ID get_resource_uid(const String &p_path) const;
 	virtual void get_dependencies(const String &p_path, List<String> *p_dependencies, bool p_add_types = false);
 };
 
@@ -629,7 +624,6 @@ public:
 	virtual Error save(const Ref<Resource> &p_resource, const String &p_path, uint32_t p_flags = 0);
 	virtual void get_recognized_extensions(const Ref<Resource> &p_resource, List<String> *p_extensions) const;
 	virtual bool recognize(const Ref<Resource> &p_resource) const;
-	virtual Error set_uid(const String &p_path, ResourceUID::ID p_uid);
 };
 
 #endif // GDSCRIPT_H

+ 3 - 35
modules/gdscript/gdscript_parser.cpp

@@ -93,7 +93,6 @@ bool GDScriptParser::annotation_exists(const String &p_annotation_name) const {
 GDScriptParser::GDScriptParser() {
 	// Register valid annotations.
 	if (unlikely(valid_annotations.is_empty())) {
-		register_annotation(MethodInfo("@uid", PropertyInfo(Variant::STRING, "uid")), AnnotationInfo::SCRIPT, &GDScriptParser::uid_annotation);
 		register_annotation(MethodInfo("@tool"), AnnotationInfo::SCRIPT, &GDScriptParser::tool_annotation);
 		register_annotation(MethodInfo("@icon", PropertyInfo(Variant::STRING, "icon_path")), AnnotationInfo::SCRIPT, &GDScriptParser::icon_annotation);
 		register_annotation(MethodInfo("@static_unload"), AnnotationInfo::SCRIPT, &GDScriptParser::static_unload_annotation);
@@ -521,8 +520,6 @@ void GDScriptParser::parse_program() {
 					// `@icon` needs to be applied in the parser. See GH-72444.
 					if (annotation->name == SNAME("@icon")) {
 						annotation->apply(this, head, nullptr);
-					} else if (annotation->name == SNAME("@uid")) {
-						annotation->apply(this, head, nullptr);
 					} else {
 						head->annotations.push_back(annotation);
 					}
@@ -3837,18 +3834,18 @@ bool GDScriptParser::validate_annotation_arguments(AnnotationNode *p_annotation)
 	}
 
 	// `@icon`'s argument needs to be resolved in the parser. See GH-72444.
-	if (p_annotation->name == SNAME("@icon") || p_annotation->name == SNAME("@uid")) {
+	if (p_annotation->name == SNAME("@icon")) {
 		ExpressionNode *argument = p_annotation->arguments[0];
 
 		if (argument->type != Node::LITERAL) {
-			push_error(vformat(R"(Argument 1 of annotation "%s" must be a string literal.)", p_annotation->name), argument);
+			push_error(R"(Argument 1 of annotation "@icon" must be a string literal.)", argument);
 			return false;
 		}
 
 		Variant value = static_cast<LiteralNode *>(argument)->value;
 
 		if (value.get_type() != Variant::STRING) {
-			push_error(vformat(R"(Argument 1 of annotation "%s" must be a string literal.)", p_annotation->name), argument);
+			push_error(R"(Argument 1 of annotation "@icon" must be a string literal.)", argument);
 			return false;
 		}
 
@@ -3860,35 +3857,6 @@ bool GDScriptParser::validate_annotation_arguments(AnnotationNode *p_annotation)
 	return true;
 }
 
-bool GDScriptParser::uid_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
-	ERR_FAIL_COND_V_MSG(p_target->type != Node::CLASS, false, R"("@uid" annotation can only be applied to classes.)");
-	ERR_FAIL_COND_V(p_annotation->resolved_arguments.is_empty(), false);
-
-#ifdef DEBUG_ENABLED
-	if (_has_uid) {
-		push_error(R"("@uid" annotation can only be used once.)", p_annotation);
-		return false;
-	}
-#endif // DEBUG_ENABLED
-
-	// Assign line range early to allow replacing invalid UIDs.
-	ClassNode *class_node = static_cast<ClassNode *>(p_target);
-	class_node->uid_lines = Vector2i(p_annotation->start_line - 1, p_annotation->end_line - 1); // Lines start from 1, so need to subtract.
-
-	const String &uid_string = p_annotation->resolved_arguments[0];
-#ifdef DEBUG_ENABLED
-	if (ResourceUID::get_singleton()->text_to_id(uid_string) == ResourceUID::INVALID_ID) {
-		push_error(R"(The annotated UID is invalid.)", p_annotation);
-		return false;
-	}
-#endif // DEBUG_ENABLED
-
-	class_node->uid_string = uid_string;
-
-	_has_uid = true;
-	return true;
-}
-
 bool GDScriptParser::tool_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class) {
 #ifdef DEBUG_ENABLED
 	if (_is_tool) {

+ 0 - 4
modules/gdscript/gdscript_parser.h

@@ -736,8 +736,6 @@ public:
 		IdentifierNode *identifier = nullptr;
 		String icon_path;
 		String simplified_icon_path;
-		String uid_string;
-		Vector2i uid_lines = Vector2i(-1, -1);
 		Vector<Member> members;
 		HashMap<StringName, int> members_indices;
 		ClassNode *outer = nullptr;
@@ -1320,7 +1318,6 @@ private:
 	friend class GDScriptAnalyzer;
 
 	bool _is_tool = false;
-	bool _has_uid = false;
 	String script_path;
 	bool for_completion = false;
 	bool panic_mode = false;
@@ -1476,7 +1473,6 @@ private:
 	static bool register_annotation(const MethodInfo &p_info, uint32_t p_target_kinds, AnnotationAction p_apply, const Vector<Variant> &p_default_arguments = Vector<Variant>(), bool p_is_vararg = false);
 	bool validate_annotation_arguments(AnnotationNode *p_annotation);
 	void clear_unused_annotations();
-	bool uid_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool tool_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool icon_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);
 	bool onready_annotation(const AnnotationNode *p_annotation, Node *p_target, ClassNode *p_class);

+ 0 - 5
modules/gdscript/tests/scripts/parser/errors/uid_duplicate.gd

@@ -1,5 +0,0 @@
-@uid("uid://c4ckv3ryprcn4")
-@uid("uid://c4ckv3ryprcn4")
-
-func test():
-    pass

+ 0 - 2
modules/gdscript/tests/scripts/parser/errors/uid_duplicate.out

@@ -1,2 +0,0 @@
-GDTEST_PARSER_ERROR
-"@uid" annotation can only be used once.

+ 0 - 4
modules/gdscript/tests/scripts/parser/errors/uid_invalid.gd

@@ -1,4 +0,0 @@
-@uid("not a valid uid")
-
-func test():
-    pass

+ 0 - 2
modules/gdscript/tests/scripts/parser/errors/uid_invalid.out

@@ -1,2 +0,0 @@
-GDTEST_PARSER_ERROR
-The annotated UID is invalid.

+ 0 - 5
modules/gdscript/tests/scripts/parser/errors/uid_too_late.gd

@@ -1,5 +0,0 @@
-extends Object
-@uid("uid://c4ckv3ryprcn4")
-
-func test():
-    pass

+ 0 - 2
modules/gdscript/tests/scripts/parser/errors/uid_too_late.out

@@ -1,2 +0,0 @@
-GDTEST_PARSER_ERROR
-Annotation "@uid" must be at the top of the script, before "extends" and "class_name".

+ 0 - 5
modules/gdscript/tests/scripts/parser/features/uid.gd

@@ -1,5 +0,0 @@
-@uid("uid://c4ckv3ryprcn4")
-extends Object
-
-func test():
-    pass

+ 0 - 1
modules/gdscript/tests/scripts/parser/features/uid.out

@@ -1 +0,0 @@
-GDTEST_OK

+ 0 - 115
modules/gdscript/tests/test_gdscript_uid.h

@@ -1,115 +0,0 @@
-/**************************************************************************/
-/*  test_gdscript_uid.h                                                   */
-/**************************************************************************/
-/*                         This file is part of:                          */
-/*                             GODOT ENGINE                               */
-/*                        https://godotengine.org                         */
-/**************************************************************************/
-/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
-/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur.                  */
-/*                                                                        */
-/* Permission is hereby granted, free of charge, to any person obtaining  */
-/* a copy of this software and associated documentation files (the        */
-/* "Software"), to deal in the Software without restriction, including    */
-/* without limitation the rights to use, copy, modify, merge, publish,    */
-/* distribute, sublicense, and/or sell copies of the Software, and to     */
-/* permit persons to whom the Software is furnished to do so, subject to  */
-/* the following conditions:                                              */
-/*                                                                        */
-/* The above copyright notice and this permission notice shall be         */
-/* included in all copies or substantial portions of the Software.        */
-/*                                                                        */
-/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,        */
-/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF     */
-/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
-/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY   */
-/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,   */
-/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE      */
-/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                 */
-/**************************************************************************/
-
-#ifndef TEST_GDSCRIPT_UID_H
-#define TEST_GDSCRIPT_UID_H
-
-#ifdef TOOLS_ENABLED
-
-#include "core/io/resource_saver.h"
-#include "core/os/os.h"
-#include "gdscript_test_runner.h"
-
-#include "../gdscript.h"
-#include "tests/test_macros.h"
-
-namespace GDScriptTests {
-
-static HashMap<String, ResourceUID::ID> id_cache;
-
-ResourceUID::ID _resource_saver_get_resource_id_for_path(const String &p_path, bool p_generate) {
-	return ResourceUID::get_singleton()->text_to_id("uid://baba");
-}
-
-static void test_script(const String &p_source, const String &p_target_source) {
-	const String script_path = OS::get_singleton()->get_cache_path().path_join("script.gd");
-
-	Ref<GDScript> script;
-	script.instantiate();
-	script->set_source_code(p_source);
-	ResourceSaver::save(script, script_path);
-
-	Ref<FileAccess> fa = FileAccess::open(script_path, FileAccess::READ);
-	CHECK_EQ(fa->get_as_text(), p_target_source);
-}
-
-TEST_SUITE("[Modules][GDScript][UID]") {
-	TEST_CASE("[ResourceSaver] Adding UID line to script") {
-		init_language("modules/gdscript/tests/scripts");
-		ResourceSaver::set_get_resource_id_for_path(_resource_saver_get_resource_id_for_path);
-
-		const String source = R"(extends Node
-class_name TestClass
-)";
-		const String final_source = R"(@uid("uid://baba") # Generated automatically, do not modify.
-extends Node
-class_name TestClass
-)";
-
-		// Script has no UID, add it.
-		test_script(source, final_source);
-	}
-
-	TEST_CASE("[ResourceSaver] Updating UID line in script") {
-		init_language("modules/gdscript/tests/scripts");
-		ResourceSaver::set_get_resource_id_for_path(_resource_saver_get_resource_id_for_path);
-
-		const String wrong_id_source = R"(
-
-@uid(
-	"uid://dead"
-	) # G
-extends Node
-class_name TestClass
-)";
-		const String corrected_id_source = R"(
-
-@uid("uid://baba") # Generated automatically, do not modify.
-extends Node
-class_name TestClass
-)";
-		const String correct_id_source = R"(@uid("uid://baba") # G
-extends Node
-class_name TestClass
-)";
-
-		// Script has wrong UID saved. Remove it and add a correct one.
-		// Inserts in the same line, but multiline annotations are flattened.
-		test_script(wrong_id_source, corrected_id_source);
-		// The stored UID is correct, so do not modify it.
-		test_script(correct_id_source, correct_id_source);
-	}
-}
-
-} // namespace GDScriptTests
-
-#endif
-
-#endif // TEST_GDSCRIPT_UID_H