From eba3e0a9fce1f10e8ad7311e84e9f3d38dae008e Mon Sep 17 00:00:00 2001 From: George Marques Date: Thu, 26 May 2022 12:56:39 -0300 Subject: [PATCH] GDScript: Support `%` in shorthand for `get_node` The `%` is used in scene unique nodes. Now `%` can also be used instead of `$` for the shorthand, besides being allowed generally anywhere in the path as the prefix for a node name. --- editor/plugins/script_text_editor.cpp | 15 +- .../gdscript/editor/gdscript_highlighter.cpp | 4 +- modules/gdscript/gdscript_compiler.cpp | 14 +- modules/gdscript/gdscript_parser.cpp | 147 +++++++++++------- modules/gdscript/gdscript_parser.h | 6 +- .../errors/dollar-assignment-bug-53696.out | 2 +- .../parser/errors/nothing_after_dollar.out | 2 +- .../errors/wrong_value_after_dollar.out | 2 +- .../errors/wrong_value_after_dollar_slash.out | 2 +- .../features/dollar_and_percent_get_node.gd | 49 ++++++ .../features/dollar_and_percent_get_node.out | 19 +++ 11 files changed, 182 insertions(+), 80 deletions(-) create mode 100644 modules/gdscript/tests/scripts/parser/features/dollar_and_percent_get_node.gd create mode 100644 modules/gdscript/tests/scripts/parser/features/dollar_and_percent_get_node.out diff --git a/editor/plugins/script_text_editor.cpp b/editor/plugins/script_text_editor.cpp index a4bccf30e31..f93ea651fd7 100644 --- a/editor/plugins/script_text_editor.cpp +++ b/editor/plugins/script_text_editor.cpp @@ -1568,9 +1568,11 @@ void ScriptTextEditor::drop_data_fw(const Point2 &p_point, const Variant &p_data continue; } + bool is_unique = false; String path; if (node->is_unique_name_in_owner()) { - path = "%" + node->get_name(); + path = node->get_name(); + is_unique = true; } else { path = sn->get_path_to(node); } @@ -1583,9 +1585,9 @@ void ScriptTextEditor::drop_data_fw(const Point2 &p_point, const Variant &p_data String variable_name = String(node->get_name()).camelcase_to_underscore(true).validate_identifier(); if (use_type) { - text_to_drop += vformat("@onready var %s: %s = $%s\n", variable_name, node->get_class_name(), path); + text_to_drop += vformat("@onready var %s: %s = %s%s\n", variable_name, node->get_class_name(), is_unique ? "%" : "$", path); } else { - text_to_drop += vformat("@onready var %s = $%s\n", variable_name, path); + text_to_drop += vformat("@onready var %s = %s%s\n", variable_name, is_unique ? "%" : "$", path); } } } else { @@ -1600,19 +1602,22 @@ void ScriptTextEditor::drop_data_fw(const Point2 &p_point, const Variant &p_data continue; } + bool is_unique = false; String path; if (node->is_unique_name_in_owner()) { - path = "%" + node->get_name(); + path = node->get_name(); + is_unique = true; } else { path = sn->get_path_to(node); } + for (const String &segment : path.split("/")) { if (!segment.is_valid_identifier()) { path = path.c_escape().quote(quote_style); break; } } - text_to_drop += "$" + path; + text_to_drop += (is_unique ? "%" : "$") + path; } } diff --git a/modules/gdscript/editor/gdscript_highlighter.cpp b/modules/gdscript/editor/gdscript_highlighter.cpp index 191568661d0..b86e9b386d4 100644 --- a/modules/gdscript/editor/gdscript_highlighter.cpp +++ b/modules/gdscript/editor/gdscript_highlighter.cpp @@ -387,9 +387,9 @@ Dictionary GDScriptSyntaxHighlighter::_get_line_syntax_highlighting_impl(int p_l in_member_variable = false; } - if (!in_node_path && in_region == -1 && str[j] == '$') { + if (!in_node_path && in_region == -1 && (str[j] == '$' || str[j] == '%')) { in_node_path = true; - } else if (in_region != -1 || (is_a_symbol && str[j] != '/')) { + } else if (in_region != -1 || (is_a_symbol && str[j] != '/' && str[j] != '%')) { in_node_path = false; } diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 478fafc9304..910f94a9366 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -667,20 +667,8 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code case GDScriptParser::Node::GET_NODE: { const GDScriptParser::GetNodeNode *get_node = static_cast(p_expression); - String node_name; - if (get_node->string != nullptr) { - node_name += String(get_node->string->value); - } else { - for (int i = 0; i < get_node->chain.size(); i++) { - if (i > 0) { - node_name += "/"; - } - node_name += get_node->chain[i]->name; - } - } - Vector args; - args.push_back(codegen.add_constant(NodePath(node_name))); + args.push_back(codegen.add_constant(NodePath(get_node->full_path))); GDScriptCodeGenerator::Address result = codegen.add_temporary(_gdtype_from_datatype(get_node->get_datatype())); diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index b93fff3914b..7367dfca7c1 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -2824,51 +2824,97 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_call(ExpressionNode *p_pre } GDScriptParser::ExpressionNode *GDScriptParser::parse_get_node(ExpressionNode *p_previous_operand, bool p_can_assign) { - if (match(GDScriptTokenizer::Token::LITERAL)) { - if (previous.literal.get_type() != Variant::STRING) { - push_error(R"(Expect node path as string or identifier after "$".)"); - return nullptr; - } - GetNodeNode *get_node = alloc_node(); - make_completion_context(COMPLETION_GET_NODE, get_node); - get_node->string = parse_literal(); - return get_node; - } else if (current.is_node_name()) { - GetNodeNode *get_node = alloc_node(); - int chain_position = 0; - do { - make_completion_context(COMPLETION_GET_NODE, get_node, chain_position++); - if (!current.is_node_name()) { - push_error(R"(Expect node path after "/".)"); - return nullptr; - } - advance(); - IdentifierNode *identifier = alloc_node(); - identifier->name = previous.get_identifier(); - get_node->chain.push_back(identifier); - } while (match(GDScriptTokenizer::Token::SLASH)); - return get_node; - } else if (match(GDScriptTokenizer::Token::SLASH)) { - GetNodeNode *get_node = alloc_node(); - IdentifierNode *identifier_root = alloc_node(); - get_node->chain.push_back(identifier_root); - int chain_position = 0; - do { - make_completion_context(COMPLETION_GET_NODE, get_node, chain_position++); - if (!current.is_node_name()) { - push_error(R"(Expect node path after "/".)"); - return nullptr; - } - advance(); - IdentifierNode *identifier = alloc_node(); - identifier->name = previous.get_identifier(); - get_node->chain.push_back(identifier); - } while (match(GDScriptTokenizer::Token::SLASH)); - return get_node; - } else { - push_error(R"(Expect node path as string or identifier after "$".)"); + if (!current.is_node_name() && !check(GDScriptTokenizer::Token::LITERAL) && !check(GDScriptTokenizer::Token::SLASH) && !check(GDScriptTokenizer::Token::PERCENT)) { + push_error(vformat(R"(Expected node path as string or identifier after "%s".)", previous.get_name())); return nullptr; } + + if (check(GDScriptTokenizer::Token::LITERAL)) { + if (current.literal.get_type() != Variant::STRING) { + push_error(vformat(R"(Expected node path as string or identifier after "%s".)", previous.get_name())); + return nullptr; + } + } + + GetNodeNode *get_node = alloc_node(); + + // Store the last item in the path so the parser knows what to expect. + // Allow allows more specific error messages. + enum PathState { + PATH_STATE_START, + PATH_STATE_SLASH, + PATH_STATE_PERCENT, + PATH_STATE_NODE_NAME, + } path_state = PATH_STATE_START; + + if (previous.type == GDScriptTokenizer::Token::DOLLAR) { + // Detect initial slash, which will be handled in the loop if it matches. + match(GDScriptTokenizer::Token::SLASH); +#ifdef DEBUG_ENABLED + } else { + get_node->use_dollar = false; +#endif + } + + int context_argument = 0; + + do { + if (previous.type == GDScriptTokenizer::Token::PERCENT) { + if (path_state != PATH_STATE_START && path_state != PATH_STATE_SLASH) { + push_error(R"("%" is only valid in the beginning of a node name (either after "$" or after "/"))"); + return nullptr; + } + get_node->full_path += "%"; + + path_state = PATH_STATE_PERCENT; + } else if (previous.type == GDScriptTokenizer::Token::SLASH) { + if (path_state != PATH_STATE_START && path_state != PATH_STATE_NODE_NAME) { + push_error(R"("/" is only valid at the beginning of the path or after a node name.)"); + return nullptr; + } + + get_node->full_path += "/"; + + path_state = PATH_STATE_SLASH; + } + + make_completion_context(COMPLETION_GET_NODE, get_node, context_argument++); + + if (match(GDScriptTokenizer::Token::LITERAL)) { + if (previous.literal.get_type() != Variant::STRING) { + String previous_token; + switch (path_state) { + case PATH_STATE_START: + previous_token = "$"; + break; + case PATH_STATE_PERCENT: + previous_token = "%"; + break; + case PATH_STATE_SLASH: + previous_token = "/"; + break; + default: + break; + } + push_error(vformat(R"(Expected node path as string or identifier after "%s".)", previous_token)); + return nullptr; + } + + get_node->full_path += previous.literal.operator String(); + + path_state = PATH_STATE_NODE_NAME; + } else if (current.is_node_name()) { + advance(); + get_node->full_path += previous.get_identifier(); + + path_state = PATH_STATE_NODE_NAME; + } else if (!check(GDScriptTokenizer::Token::SLASH) && !check(GDScriptTokenizer::Token::PERCENT)) { + push_error(vformat(R"(Unexpected "%s" in node path.)", current.get_name())); + return nullptr; + } + } while (match(GDScriptTokenizer::Token::SLASH) || match(GDScriptTokenizer::Token::PERCENT)); + + return get_node; } GDScriptParser::ExpressionNode *GDScriptParser::parse_preload(ExpressionNode *p_previous_operand, bool p_can_assign) { @@ -3273,7 +3319,7 @@ GDScriptParser::ParseRule *GDScriptParser::get_rule(GDScriptTokenizer::Token::Ty { nullptr, &GDScriptParser::parse_binary_operator, PREC_FACTOR }, // STAR, { nullptr, &GDScriptParser::parse_binary_operator, PREC_POWER }, // STAR_STAR, { nullptr, &GDScriptParser::parse_binary_operator, PREC_FACTOR }, // SLASH, - { nullptr, &GDScriptParser::parse_binary_operator, PREC_FACTOR }, // PERCENT, + { &GDScriptParser::parse_get_node, &GDScriptParser::parse_binary_operator, PREC_FACTOR }, // PERCENT, // Assignment { nullptr, &GDScriptParser::parse_assignment, PREC_ASSIGNMENT }, // EQUAL, { nullptr, &GDScriptParser::parse_assignment, PREC_ASSIGNMENT }, // PLUS_EQUAL, @@ -4253,17 +4299,10 @@ void GDScriptParser::TreePrinter::print_function(FunctionNode *p_function, const } void GDScriptParser::TreePrinter::print_get_node(GetNodeNode *p_get_node) { - push_text("$"); - if (p_get_node->string != nullptr) { - print_literal(p_get_node->string); - } else { - for (int i = 0; i < p_get_node->chain.size(); i++) { - if (i > 0) { - push_text("/"); - } - print_identifier(p_get_node->chain[i]); - } + if (p_get_node->use_dollar) { + push_text("$"); } + push_text(p_get_node->full_path); } void GDScriptParser::TreePrinter::print_identifier(IdentifierNode *p_identifier) { diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 96b9a10d3c1..e3f8d4b8baa 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -749,8 +749,10 @@ public: }; struct GetNodeNode : public ExpressionNode { - LiteralNode *string = nullptr; - Vector chain; + String full_path; +#ifdef DEBUG_ENABLED + bool use_dollar = true; +#endif GetNodeNode() { type = GET_NODE; diff --git a/modules/gdscript/tests/scripts/parser/errors/dollar-assignment-bug-53696.out b/modules/gdscript/tests/scripts/parser/errors/dollar-assignment-bug-53696.out index b3dc181a22b..9fafcb5a641 100644 --- a/modules/gdscript/tests/scripts/parser/errors/dollar-assignment-bug-53696.out +++ b/modules/gdscript/tests/scripts/parser/errors/dollar-assignment-bug-53696.out @@ -1,2 +1,2 @@ GDTEST_PARSER_ERROR -Expect node path as string or identifier after "$". +Expected node path as string or identifier after "$". diff --git a/modules/gdscript/tests/scripts/parser/errors/nothing_after_dollar.out b/modules/gdscript/tests/scripts/parser/errors/nothing_after_dollar.out index b3dc181a22b..9fafcb5a641 100644 --- a/modules/gdscript/tests/scripts/parser/errors/nothing_after_dollar.out +++ b/modules/gdscript/tests/scripts/parser/errors/nothing_after_dollar.out @@ -1,2 +1,2 @@ GDTEST_PARSER_ERROR -Expect node path as string or identifier after "$". +Expected node path as string or identifier after "$". diff --git a/modules/gdscript/tests/scripts/parser/errors/wrong_value_after_dollar.out b/modules/gdscript/tests/scripts/parser/errors/wrong_value_after_dollar.out index b3dc181a22b..9fafcb5a641 100644 --- a/modules/gdscript/tests/scripts/parser/errors/wrong_value_after_dollar.out +++ b/modules/gdscript/tests/scripts/parser/errors/wrong_value_after_dollar.out @@ -1,2 +1,2 @@ GDTEST_PARSER_ERROR -Expect node path as string or identifier after "$". +Expected node path as string or identifier after "$". diff --git a/modules/gdscript/tests/scripts/parser/errors/wrong_value_after_dollar_slash.out b/modules/gdscript/tests/scripts/parser/errors/wrong_value_after_dollar_slash.out index dcb4ccecb02..3062f0be70d 100644 --- a/modules/gdscript/tests/scripts/parser/errors/wrong_value_after_dollar_slash.out +++ b/modules/gdscript/tests/scripts/parser/errors/wrong_value_after_dollar_slash.out @@ -1,2 +1,2 @@ GDTEST_PARSER_ERROR -Expect node path after "/". +Expected node path as string or identifier after "/". diff --git a/modules/gdscript/tests/scripts/parser/features/dollar_and_percent_get_node.gd b/modules/gdscript/tests/scripts/parser/features/dollar_and_percent_get_node.gd new file mode 100644 index 00000000000..f04f4de08d8 --- /dev/null +++ b/modules/gdscript/tests/scripts/parser/features/dollar_and_percent_get_node.gd @@ -0,0 +1,49 @@ +extends Node + +func test(): + var child = Node.new() + child.name = "Child" + add_child(child) + child.owner = self + + var hey = Node.new() + hey.name = "Hey" + child.add_child(hey) + hey.owner = self + hey.unique_name_in_owner = true + + var fake_hey = Node.new() + fake_hey.name = "Hey" + add_child(fake_hey) + fake_hey.owner = self + + var sub_child = Node.new() + sub_child.name = "SubChild" + hey.add_child(sub_child) + sub_child.owner = self + + var howdy = Node.new() + howdy.name = "Howdy" + sub_child.add_child(howdy) + howdy.owner = self + howdy.unique_name_in_owner = true + + print(hey == $Child/Hey) + print(howdy == $Child/Hey/SubChild/Howdy) + + print(%Hey == hey) + print($%Hey == hey) + print(%"Hey" == hey) + print($"%Hey" == hey) + print($%"Hey" == hey) + print(%Hey/%Howdy == howdy) + print($%Hey/%Howdy == howdy) + print($"%Hey/%Howdy" == howdy) + print($"%Hey"/"%Howdy" == howdy) + print(%"Hey"/"%Howdy" == howdy) + print($%"Hey"/"%Howdy" == howdy) + print($"%Hey"/%"Howdy" == howdy) + print(%"Hey"/%"Howdy" == howdy) + print($%"Hey"/%"Howdy" == howdy) + print(%"Hey/%Howdy" == howdy) + print($%"Hey/%Howdy" == howdy) diff --git a/modules/gdscript/tests/scripts/parser/features/dollar_and_percent_get_node.out b/modules/gdscript/tests/scripts/parser/features/dollar_and_percent_get_node.out new file mode 100644 index 00000000000..041c4439b0d --- /dev/null +++ b/modules/gdscript/tests/scripts/parser/features/dollar_and_percent_get_node.out @@ -0,0 +1,19 @@ +GDTEST_OK +true +true +true +true +true +true +true +true +true +true +true +true +true +true +true +true +true +true