Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDScript: Allow quoteless strings as arguments in some annotations #76512

Closed
wants to merge 1 commit into from
Closed

GDScript: Allow quoteless strings as arguments in some annotations #76512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doc/classes/MultiplayerAPI.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@
Used with [method Node.rpc_config] to disable a method or property for all RPC calls, making it unavailable. Default for all methods.
</constant>
<constant name="RPC_MODE_ANY_PEER" value="1" enum="RPCMode">
Used with [method Node.rpc_config] to set a method to be callable remotely by any peer. Analogous to the [code]@rpc("any_peer")[/code] annotation. Calls are accepted from all remote peers, no matter if they are node's authority or not.
Used with [method Node.rpc_config] to set a method to be callable remotely by any peer. Analogous to the [code]@rpc(any_peer)[/code] annotation. Calls are accepted from all remote peers, no matter if they are node's authority or not.
</constant>
<constant name="RPC_MODE_AUTHORITY" value="2" enum="RPCMode">
Used with [method Node.rpc_config] to set a method to be callable remotely only by the current multiplayer authority (which is the server by default). Analogous to the [code]@rpc("authority")[/code] annotation. See [method Node.set_multiplayer_authority].
Used with [method Node.rpc_config] to set a method to be callable remotely only by the current multiplayer authority (which is the server by default). Analogous to the [code]@rpc(authority)[/code] annotation. See [method Node.set_multiplayer_authority].
</constant>
</constants>
</class>
2 changes: 1 addition & 1 deletion doc/classes/Node.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@
channel = 0,
}
[/codeblock]
See [enum MultiplayerAPI.RPCMode] and [enum MultiplayerPeer.TransferMode]. An alternative is annotating methods and properties with the corresponding [annotation @GDScript.@rpc] annotation ([code]@rpc("any_peer")[/code], [code]@rpc("authority")[/code]). By default, methods are not exposed to networking (and RPCs).
See [enum MultiplayerAPI.RPCMode] and [enum MultiplayerPeer.TransferMode]. An alternative is annotating methods and properties with the corresponding [annotation @GDScript.@rpc] annotation ([code]@rpc(any_peer)[/code], [code]@rpc(authority)[/code]). By default, methods are not exposed to networking (and RPCs).
</description>
</method>
<method name="rpc_id" qualifiers="vararg">
Expand Down
2 changes: 1 addition & 1 deletion editor/plugins/script_text_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ void ScriptTextEditor::_warning_clicked(Variant p_line) {
} else {
annotation_indent = String(" ").repeat(text_editor->get_indent_size() * indent);
}
text_editor->insert_line_at(line, annotation_indent + "@warning_ignore(" + code.quote(quote_style) + ")");
text_editor->insert_line_at(line, annotation_indent + "@warning_ignore(" + code + ")");
}

_validate_script();
Expand Down
40 changes: 20 additions & 20 deletions editor/project_converter_3_to_4.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,14 +870,14 @@ bool ProjectConverter3To4::test_conversion(RegExContainer &reg_container) {
valid = valid && test_conversion_with_regex("\texport_dialog", "\texport_dialog", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("export", "@export", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex(" export", " export", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nremote func", "\n\n@rpc(\"any_peer\") func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nremotesync func", "\n\n@rpc(\"any_peer\", \"call_local\") func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nsync func", "\n\n@rpc(\"any_peer\", \"call_local\") func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nremote func", "\n\n@rpc(any_peer) func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nremotesync func", "\n\n@rpc(any_peer, call_local) func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nsync func", "\n\n@rpc(any_peer, call_local) func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nslave func", "\n\n@rpc func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\npuppet func", "\n\n@rpc func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\npuppetsync func", "\n\n@rpc(\"call_local\") func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\npuppetsync func", "\n\n@rpc(call_local) func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nmaster func", "\n\nThe master and mastersync rpc behavior is not officially supported anymore. Try using another keyword or making custom logic using get_multiplayer().get_remote_sender_id()\n@rpc func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nmastersync func", "\n\nThe master and mastersync rpc behavior is not officially supported anymore. Try using another keyword or making custom logic using get_multiplayer().get_remote_sender_id()\n@rpc(\"call_local\") func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);
valid = valid && test_conversion_with_regex("\n\nmastersync func", "\n\nThe master and mastersync rpc behavior is not officially supported anymore. Try using another keyword or making custom logic using get_multiplayer().get_remote_sender_id()\n@rpc(call_local) func", &ProjectConverter3To4::rename_gdscript_keywords, "gdscript keyword", reg_container);

valid = valid && test_conversion_gdscript_builtin("var size: Vector2 = Vector2() setget set_function, get_function", "var size: Vector2 = Vector2(): get = get_function, set = set_function", &ProjectConverter3To4::rename_gdscript_functions, "custom rename", reg_container, false);
valid = valid && test_conversion_gdscript_builtin("var size: Vector2 = Vector2() setget set_function, ", "var size: Vector2 = Vector2(): set = set_function", &ProjectConverter3To4::rename_gdscript_functions, "custom rename", reg_container, false);
Expand Down Expand Up @@ -2461,13 +2461,13 @@ void ProjectConverter3To4::rename_gdscript_keywords(Vector<SourceLine> &source_l
line = reg_container.keyword_gdscript_onready.sub(line, "@onready", true);
}
if (line.contains("remote")) {
line = reg_container.keyword_gdscript_remote.sub(line, "@rpc(\"any_peer\") func", true);
line = reg_container.keyword_gdscript_remote.sub(line, "@rpc(any_peer) func", true);
}
if (line.contains("remote")) {
line = reg_container.keyword_gdscript_remotesync.sub(line, "@rpc(\"any_peer\", \"call_local\") func", true);
line = reg_container.keyword_gdscript_remotesync.sub(line, "@rpc(any_peer, call_local) func", true);
}
if (line.contains("sync")) {
line = reg_container.keyword_gdscript_sync.sub(line, "@rpc(\"any_peer\", \"call_local\") func", true);
line = reg_container.keyword_gdscript_sync.sub(line, "@rpc(any_peer, call_local) func", true);
}
if (line.contains("slave")) {
line = reg_container.keyword_gdscript_slave.sub(line, "@rpc func", true);
Expand All @@ -2476,13 +2476,13 @@ void ProjectConverter3To4::rename_gdscript_keywords(Vector<SourceLine> &source_l
line = reg_container.keyword_gdscript_puppet.sub(line, "@rpc func", true);
}
if (line.contains("puppet")) {
line = reg_container.keyword_gdscript_puppetsync.sub(line, "@rpc(\"call_local\") func", true);
line = reg_container.keyword_gdscript_puppetsync.sub(line, "@rpc(call_local) func", true);
}
if (line.contains("master")) {
line = reg_container.keyword_gdscript_master.sub(line, error_message + "@rpc func", true);
}
if (line.contains("master")) {
line = reg_container.keyword_gdscript_mastersync.sub(line, error_message + "@rpc(\"call_local\") func", true);
line = reg_container.keyword_gdscript_mastersync.sub(line, error_message + "@rpc(call_local) func", true);
}
}
}
Expand Down Expand Up @@ -2530,25 +2530,25 @@ Vector<String> ProjectConverter3To4::check_for_rename_gdscript_keywords(Vector<S

if (line.contains("remote")) {
old = line;
line = reg_container.keyword_gdscript_remote.sub(line, "@rpc(\"any_peer\") func", true);
line = reg_container.keyword_gdscript_remote.sub(line, "@rpc(any_peer) func", true);
if (old != line) {
found_renames.append(line_formatter(current_line, "remote func", "@rpc(\"any_peer\") func", line));
found_renames.append(line_formatter(current_line, "remote func", "@rpc(any_peer) func", line));
}
}

if (line.contains("remote")) {
old = line;
line = reg_container.keyword_gdscript_remotesync.sub(line, "@rpc(\"any_peer\", \"call_local\")) func", true);
line = reg_container.keyword_gdscript_remotesync.sub(line, "@rpc(any_peer, call_local)) func", true);
if (old != line) {
found_renames.append(line_formatter(current_line, "remotesync func", "@rpc(\"any_peer\", \"call_local\")) func", line));
found_renames.append(line_formatter(current_line, "remotesync func", "@rpc(any_peer, call_local)) func", line));
}
}

if (line.contains("sync")) {
old = line;
line = reg_container.keyword_gdscript_sync.sub(line, "@rpc(\"any_peer\", \"call_local\")) func", true);
line = reg_container.keyword_gdscript_sync.sub(line, "@rpc(any_peer, call_local)) func", true);
if (old != line) {
found_renames.append(line_formatter(current_line, "sync func", "@rpc(\"any_peer\", \"call_local\")) func", line));
found_renames.append(line_formatter(current_line, "sync func", "@rpc(any_peer, call_local)) func", line));
}
}

Expand All @@ -2570,9 +2570,9 @@ Vector<String> ProjectConverter3To4::check_for_rename_gdscript_keywords(Vector<S

if (line.contains("puppet")) {
old = line;
line = reg_container.keyword_gdscript_puppetsync.sub(line, "@rpc(\"call_local\") func", true);
line = reg_container.keyword_gdscript_puppetsync.sub(line, "@rpc(call_local) func", true);
if (old != line) {
found_renames.append(line_formatter(current_line, "puppetsync func", "@rpc(\"call_local\") func", line));
found_renames.append(line_formatter(current_line, "puppetsync func", "@rpc(call_local) func", line));
}
}

Expand All @@ -2586,9 +2586,9 @@ Vector<String> ProjectConverter3To4::check_for_rename_gdscript_keywords(Vector<S

if (line.contains("master")) {
old = line;
line = reg_container.keyword_gdscript_master.sub(line, "@rpc(\"call_local\") func", true);
line = reg_container.keyword_gdscript_master.sub(line, "@rpc(call_local) func", true);
if (old != line) {
found_renames.append(line_formatter(current_line, "mastersync func", "@rpc(\"call_local\") func", line));
found_renames.append(line_formatter(current_line, "mastersync func", "@rpc(call_local) func", line));
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions modules/gdscript/doc_classes/@GDScript.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@
Export a [NodePath] property with a filter for allowed node types.
See also [constant PROPERTY_HINT_NODE_PATH_VALID_TYPES].
[codeblock]
@export_node_path("Button", "TouchScreenButton") var some_button
@export_node_path(Button, TouchScreenButton) var some_button
[/codeblock]
</description>
</annotation>
Expand Down Expand Up @@ -614,10 +614,10 @@
@rpc
func fn(): pass

@rpc("any_peer", "unreliable_ordered")
@rpc(any_peer, unreliable_ordered)
func fn_update_pos(): pass

@rpc("authority", "call_remote", "unreliable", 0) # Equivalent to @rpc
@rpc(authority, call_remote, unreliable, 0) # Equivalent to @rpc
func fn_default(): pass
[/codeblock]
</description>
Expand All @@ -642,7 +642,7 @@
func test():
print("hello")
return
@warning_ignore("unreachable_code")
@warning_ignore(unreachable_code)
print("unreachable")
[/codeblock]
</description>
Expand Down
48 changes: 36 additions & 12 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1435,22 +1435,41 @@ void GDScriptAnalyzer::resolve_annotation(GDScriptParser::AnnotationNode *p_anno
E = E->next();
}

reduce_expression(argument);
bool is_identifier = argument->type == GDScriptParser::Node::IDENTIFIER;
bool accept_id_as_string = argument_info.hint == PROPERTY_HINT_ENUM || argument_info.hint == PROPERTY_HINT_NODE_TYPE;

if (!argument->is_constant) {
push_error(vformat(R"(Argument %d of annotation "%s" isn't a constant expression.)", i + 1, p_annotation->name), argument);
return;
if (is_identifier) {
reduce_identifier(static_cast<GDScriptParser::IdentifierNode *>(argument), true, true);
} else {
reduce_expression(argument);
}

Variant value;

if (argument->is_constant) {
value = argument->reduced_value;
} else {
if (!accept_id_as_string || !is_identifier) {
push_error(vformat(R"(Argument %d of annotation "%s" isn't a constant expression.)", i + 1, p_annotation->name), argument);
return;
} else if (is_identifier) {
value = static_cast<GDScriptParser::IdentifierNode *>(argument)->name.operator String();
}
}

Variant value = argument->reduced_value;
GDScriptParser::DataType arg_type = argument->get_datatype();
if (argument_info.hint == PROPERTY_HINT_NODE_TYPE && arg_type.is_meta_type) {
// Make a constant with a type become the name of type, since the argument accepts a type.
arg_type.is_meta_type = false;
value = arg_type.to_string();
}

if (value.get_type() != argument_info.type) {
#ifdef DEBUG_ENABLED
if (argument_info.type == Variant::INT && value.get_type() == Variant::FLOAT) {
parser->push_warning(argument, GDScriptWarning::NARROWING_CONVERSION);
}
#endif

if (!Variant::can_convert_strict(value.get_type(), argument_info.type)) {
push_error(vformat(R"(Invalid argument for annotation "%s": argument %d should be "%s" but is "%s".)", p_annotation->name, i + 1, Variant::get_type_name(argument_info.type), argument->get_datatype().to_string()), argument);
return;
Expand Down Expand Up @@ -3522,7 +3541,7 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
}
}

void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_identifier, bool can_be_builtin) {
void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_identifier, bool p_can_be_builtin, bool p_can_be_not_found) {
// TODO: This is an opportunity to further infer types.

// Check if we are inside an enum. This allows enum values to access other elements of the same enum.
Expand Down Expand Up @@ -3643,7 +3662,7 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
// non-builtin types so we allow doing e.g. Object.new()
Variant::Type builtin_type = GDScriptParser::get_builtin_type(name);
if (builtin_type != Variant::OBJECT && builtin_type < Variant::VARIANT_MAX) {
if (can_be_builtin) {
if (p_can_be_builtin) {
p_identifier->set_datatype(make_builtin_meta_type(builtin_type));
return;
} else {
Expand Down Expand Up @@ -3726,14 +3745,14 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident

if (CoreConstants::is_global_enum(name)) {
p_identifier->set_datatype(make_global_enum_type(name, StringName(), true));
if (!can_be_builtin) {
if (!p_can_be_builtin) {
push_error(vformat(R"(Global enum "%s" cannot be used on its own.)", name), p_identifier);
}
return;
}

// Allow "Variant" here since it might be used for nested enums.
if (can_be_builtin && name == SNAME("Variant")) {
if (p_can_be_builtin && name == SNAME("Variant")) {
GDScriptParser::DataType variant;
variant.kind = GDScriptParser::DataType::VARIANT;
variant.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
Expand All @@ -3756,14 +3775,19 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
rename_hint = " " + vformat(R"(Did you mean to use "%s"?)", renamed_identifier_name);
}
}
push_error(vformat(R"(Identifier "%s" not declared in the current scope.%s)", name, rename_hint), p_identifier);
if (!p_can_be_not_found) {
push_error(vformat(R"(Identifier "%s" not declared in the current scope.%s)", name, rename_hint), p_identifier);
}
#else
push_error(vformat(R"(Identifier "%s" not declared in the current scope.)", name), p_identifier);
if (!p_can_be_not_found) {
push_error(vformat(R"(Identifier "%s" not declared in the current scope.)", name), p_identifier);
}
#endif // SUGGEST_GODOT4_RENAMES
}
GDScriptParser::DataType dummy;
dummy.kind = GDScriptParser::DataType::VARIANT;
p_identifier->set_datatype(dummy); // Just so type is set to something.
return;
}

void GDScriptAnalyzer::reduce_lambda(GDScriptParser::LambdaNode *p_lambda) {
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/gdscript_analyzer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class GDScriptAnalyzer {
void reduce_cast(GDScriptParser::CastNode *p_cast);
void reduce_dictionary(GDScriptParser::DictionaryNode *p_dictionary);
void reduce_get_node(GDScriptParser::GetNodeNode *p_get_node);
void reduce_identifier(GDScriptParser::IdentifierNode *p_identifier, bool can_be_builtin = false);
void reduce_identifier(GDScriptParser::IdentifierNode *p_identifier, bool p_can_be_builtin = false, bool p_can_be_not_found = false);
void reduce_identifier_from_base(GDScriptParser::IdentifierNode *p_identifier, GDScriptParser::DataType *p_base = nullptr);
void reduce_lambda(GDScriptParser::LambdaNode *p_lambda);
void reduce_literal(GDScriptParser::LiteralNode *p_literal);
Expand Down
17 changes: 12 additions & 5 deletions modules/gdscript/gdscript_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,30 +790,37 @@ static void _find_annotation_arguments(const GDScriptParser::AnnotationNode *p_a
}
} else if (p_annotation->name == SNAME("@export_node_path")) {
ScriptLanguage::CodeCompletionOption node("Node", ScriptLanguage::CODE_COMPLETION_KIND_CLASS);
node.insert_text = node.display.quote(p_quote_style);
r_result.insert(node.display, node);
List<StringName> node_types;
ClassDB::get_inheriters_from_class("Node", &node_types);
StringName node_class = SNAME("Node");
ClassDB::get_inheriters_from_class(node_class, &node_types);
for (const StringName &E : node_types) {
if (!ClassDB::is_class_exposed(E)) {
continue;
}
ScriptLanguage::CodeCompletionOption option(E, ScriptLanguage::CODE_COMPLETION_KIND_CLASS);
option.insert_text = option.display.quote(p_quote_style);
r_result.insert(option.display, option);
}

List<StringName> global_script_classes;
ScriptServer::get_global_class_list(&global_script_classes);
for (const StringName &E : global_script_classes) {
if (!ClassDB::is_parent_class(ScriptServer::get_global_class_native_base(E), node_class)) {
continue;
}
ScriptLanguage::CodeCompletionOption option(E, ScriptLanguage::CODE_COMPLETION_KIND_CLASS);
r_result.insert(option.display, option);
}
} else if (p_annotation->name == SNAME("@warning_ignore")) {
for (int warning_code = 0; warning_code < GDScriptWarning::WARNING_MAX; warning_code++) {
ScriptLanguage::CodeCompletionOption warning(GDScriptWarning::get_name_from_code((GDScriptWarning::Code)warning_code).to_lower(), ScriptLanguage::CODE_COMPLETION_KIND_PLAIN_TEXT);
warning.insert_text = warning.display.quote(p_quote_style);
r_result.insert(warning.display, warning);
}
} else if (p_annotation->name == SNAME("@rpc")) {
if (p_argument == 0 || p_argument == 1 || p_argument == 2) {
static const char *options[7] = { "call_local", "call_remote", "any_peer", "authority", "reliable", "unreliable", "unreliable_ordered" };
for (int i = 0; i < 7; i++) {
ScriptLanguage::CodeCompletionOption option(options[i], ScriptLanguage::CODE_COMPLETION_KIND_PLAIN_TEXT);
option.insert_text = option.display.quote(p_quote_style);
r_result.insert(option.display, option);
}
}
Expand Down
Loading