From 81af945fb5c436c5a5af5ba9ab544db5f3e823c2 Mon Sep 17 00:00:00 2001 From: rune-scape Date: Tue, 15 Oct 2024 21:53:24 -0700 Subject: [PATCH] Improved lambda hotswapping when uniquely named --- modules/gdscript/gdscript.cpp | 1 - modules/gdscript/gdscript_compiler.cpp | 182 +++++++++--------- modules/gdscript/gdscript_compiler.h | 50 +++-- modules/gdscript/gdscript_function.h | 1 + modules/gdscript/gdscript_parser.cpp | 19 +- modules/gdscript/gdscript_parser.h | 18 ++ .../gdscript/tests/gdscript_test_runner.cpp | 142 +++++++++----- modules/gdscript/tests/gdscript_test_runner.h | 8 +- .../tests/scripts/hotswap/hotswap_lambda.gd | 21 ++ .../tests/scripts/hotswap/hotswap_lambda.out | 4 + .../scripts/hotswap/hotswap_lambda.swap1 | 36 ++++ .../scripts/hotswap/hotswap_lambda.swap1.out | 7 + .../scripts/hotswap/hotswap_lambda.swap2 | 34 ++++ .../scripts/hotswap/hotswap_lambda.swap2.out | 7 + .../runtime/features/lambda_get_method.gd | 4 +- .../runtime/features/lambda_get_method.out | 4 +- 16 files changed, 371 insertions(+), 167 deletions(-) create mode 100644 modules/gdscript/tests/scripts/hotswap/hotswap_lambda.gd create mode 100644 modules/gdscript/tests/scripts/hotswap/hotswap_lambda.out create mode 100644 modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap1 create mode 100644 modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap1.out create mode 100644 modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap2 create mode 100644 modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap2.out diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 7b9aa70686a7..cd17f602ce7c 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -1511,7 +1511,6 @@ void GDScript::_recurse_replace_function_ptrs(const HashMapptr = replacement->value; } else { - // Probably a lambda from another reload, ignore. updatable->ptr = nullptr; } } diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index f4f445e09667..8669cadb0931 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -30,6 +30,7 @@ #include "gdscript_compiler.h" +#include "core/error/error_macros.h" #include "gdscript.h" #include "gdscript_byte_codegen.h" #include "gdscript_cache.h" @@ -2258,6 +2259,9 @@ GDScriptFunction *GDScriptCompiler::_parse_function(Error &r_error, GDScript *p_ if (p_func) { if (p_func->identifier) { func_name = p_func->identifier->name; + } else if (p_func->source_lambda && p_func->source_lambda->parent_variable) { + GDScriptParser::AssignableNode *parent_variable = p_func->source_lambda->parent_variable; + func_name = vformat("", parent_variable->identifier->name); } else { func_name = ""; } @@ -3107,129 +3111,133 @@ void GDScriptCompiler::make_scripts(GDScript *p_script, const GDScriptParser::Cl } } -GDScriptCompiler::FunctionLambdaInfo GDScriptCompiler::_get_function_replacement_info(GDScriptFunction *p_func, int p_index, int p_depth, GDScriptFunction *p_parent_func) { - FunctionLambdaInfo info; - info.function = p_func; - info.parent = p_parent_func; - info.script = p_func->get_script(); - info.name = p_func->get_name(); - info.line = p_func->_initial_line; - info.index = p_index; - info.depth = p_depth; - info.capture_count = 0; - info.use_self = false; - info.arg_count = p_func->_argument_count; - info.default_arg_count = p_func->_default_arg_count; - info.sublambdas = _get_function_lambda_replacement_info(p_func, p_depth, p_parent_func); - - ERR_FAIL_NULL_V(info.script, info); - GDScript::LambdaInfo *extra_info = info.script->lambda_info.getptr(p_func); - if (extra_info != nullptr) { - info.capture_count = extra_info->capture_count; - info.use_self = extra_info->use_self; - } else { - info.capture_count = 0; +void GDScriptCompiler::LambdaSourceInfoList::collect_function_lambda_replacement_info(GDScriptFunction *p_func) { + // Only collect the lambdas inside. + for (int i = 0; i < p_func->lambdas.size(); ++i) { + LambdaSourceInfoList::Element *E = infos.push_back({}); + LambdaSourceInfo &info = E->get(); + GDScriptFunction *lambda = p_func->lambdas[i]; + + info.function = lambda; + info.parent = p_func; + info.script = lambda->get_script(); + info.name = lambda->get_name(); + info.is_static = lambda->is_static(); info.use_self = false; - } - - return info; -} + info.capture_count = 0; + info.arg_count = lambda->get_argument_count(); + info.default_arg_count = lambda->get_default_argument_count(); + info.sublambdas.collect_function_lambda_replacement_info(lambda); + + if (info.script) { + GDScript::LambdaInfo *extra_info = info.script->lambda_info.getptr(lambda); + if (extra_info != nullptr) { + info.capture_count = extra_info->capture_count; + info.use_self = extra_info->use_self; + } else { + info.capture_count = 0; + info.use_self = false; + } + } -Vector GDScriptCompiler::_get_function_lambda_replacement_info(GDScriptFunction *p_func, int p_depth, GDScriptFunction *p_parent_func) { - Vector result; - // Only scrape the lambdas inside p_func. - for (int i = 0; i < p_func->lambdas.size(); ++i) { - result.push_back(_get_function_replacement_info(p_func->lambdas[i], i, p_depth + 1, p_func)); + info.self_element = E; + infos_by_name[info.name].push_back(&info); } - return result; } -GDScriptCompiler::ScriptLambdaInfo GDScriptCompiler::_get_script_lambda_replacement_info(GDScript *p_script) { - ScriptLambdaInfo info; - +void GDScriptCompiler::ScriptLambdaInfo::collect_script_lambda_replacement_info(GDScript *p_script) { if (p_script->implicit_initializer) { - info.implicit_initializer_info = _get_function_lambda_replacement_info(p_script->implicit_initializer); + initializers.collect_function_lambda_replacement_info(p_script->implicit_initializer); } if (p_script->implicit_ready) { - info.implicit_ready_info = _get_function_lambda_replacement_info(p_script->implicit_ready); + initializers.collect_function_lambda_replacement_info(p_script->implicit_ready); } if (p_script->static_initializer) { - info.static_initializer_info = _get_function_lambda_replacement_info(p_script->static_initializer); + initializers.collect_function_lambda_replacement_info(p_script->static_initializer); } for (const KeyValue &E : p_script->member_functions) { - info.member_function_infos.insert(E.key, _get_function_lambda_replacement_info(E.value)); + member_functions[E.key].collect_function_lambda_replacement_info(E.value); } for (const KeyValue> &KV : p_script->get_subclasses()) { - info.subclass_info.insert(KV.key, _get_script_lambda_replacement_info(KV.value.ptr())); + subclasses[KV.key].collect_script_lambda_replacement_info(KV.value.ptr()); } - - return info; } -bool GDScriptCompiler::_do_function_infos_match(const FunctionLambdaInfo &p_old_info, const FunctionLambdaInfo *p_new_info) { - if (p_new_info == nullptr) { +bool GDScriptCompiler::LambdaSourceInfo::can_be_replaced_by(const LambdaSourceInfo &p_new_info) const { + if (p_new_info.capture_count != capture_count || p_new_info.use_self != use_self) { return false; } - if (p_new_info->capture_count != p_old_info.capture_count || p_new_info->use_self != p_old_info.use_self) { + if (p_new_info.script != script) { return false; } - int old_required_arg_count = p_old_info.arg_count - p_old_info.default_arg_count; - int new_required_arg_count = p_new_info->arg_count - p_new_info->default_arg_count; - if (new_required_arg_count > old_required_arg_count || p_new_info->arg_count < old_required_arg_count) { + int old_required_arg_count = arg_count - default_arg_count; + int new_required_arg_count = p_new_info.arg_count - p_new_info.default_arg_count; + if (new_required_arg_count > old_required_arg_count || p_new_info.arg_count < old_required_arg_count) { return false; } return true; } -void GDScriptCompiler::_get_function_ptr_replacements(HashMap &r_replacements, const FunctionLambdaInfo &p_old_info, const FunctionLambdaInfo *p_new_info) { - ERR_FAIL_COND(r_replacements.has(p_old_info.function)); - if (!_do_function_infos_match(p_old_info, p_new_info)) { - p_new_info = nullptr; +void GDScriptCompiler::_collect_function_ptr_replacements(HashMap &r_replacements, LambdaSourceInfoList &p_old, LambdaSourceInfoList *p_new) { + if (p_new) { + for (KeyValue> &old_named_infos : p_old.infos_by_name) { + if (old_named_infos.value.size() == 1) { + // Uniquely named lambda. + if (HashMap>::Iterator new_named_infos_it = p_new->infos_by_name.find(old_named_infos.key)) { + if (new_named_infos_it->value.size() == 1) { + // Also uniquely named. + LambdaSourceInfo *&old_info = old_named_infos.value.write[0]; + LambdaSourceInfo *&new_info = new_named_infos_it->value.write[0]; + ERR_CONTINUE(old_info == nullptr); + ERR_CONTINUE(new_info == nullptr); + r_replacements[old_info->function] = new_info->function; + old_info->self_element->erase(); + new_info->self_element->erase(); + old_info = nullptr; + new_info = nullptr; + } + } + } + } } - r_replacements.insert(p_old_info.function, p_new_info != nullptr ? p_new_info->function : nullptr); - _get_function_ptr_replacements(r_replacements, p_old_info.sublambdas, p_new_info != nullptr ? &p_new_info->sublambdas : nullptr); -} - -void GDScriptCompiler::_get_function_ptr_replacements(HashMap &r_replacements, const Vector &p_old_infos, const Vector *p_new_infos) { - for (int i = 0; i < p_old_infos.size(); ++i) { - const FunctionLambdaInfo &old_info = p_old_infos[i]; - const FunctionLambdaInfo *new_info = nullptr; - if (p_new_infos != nullptr && p_new_infos->size() == p_old_infos.size()) { + LambdaSourceInfoList::Element *old_info_E = p_old.infos.front(); + LambdaSourceInfoList::Element *new_info_E = p_new ? p_new->infos.front() : nullptr; + while (old_info_E) { + LambdaSourceInfo &old_info = old_info_E->get(); + LambdaSourceInfo *new_info = nullptr; + if (p_new && p_new->infos.size() == p_old.infos.size()) { // For now only attempt if the size is the same. - new_info = &p_new_infos->get(i); + new_info = &new_info_E->get(); + } + + if (new_info && old_info.can_be_replaced_by(*new_info)) { + r_replacements[old_info.function] = new_info->function; + } + + _collect_function_ptr_replacements(r_replacements, old_info.sublambdas, new_info ? &new_info->sublambdas : nullptr); + old_info_E = old_info_E->next(); + if (new_info_E) { + new_info_E = new_info_E->next(); } - _get_function_ptr_replacements(r_replacements, old_info, new_info); } } -void GDScriptCompiler::_get_function_ptr_replacements(HashMap &r_replacements, const ScriptLambdaInfo &p_old_info, const ScriptLambdaInfo *p_new_info) { - _get_function_ptr_replacements(r_replacements, p_old_info.implicit_initializer_info, p_new_info != nullptr ? &p_new_info->implicit_initializer_info : nullptr); - _get_function_ptr_replacements(r_replacements, p_old_info.implicit_ready_info, p_new_info != nullptr ? &p_new_info->implicit_ready_info : nullptr); - _get_function_ptr_replacements(r_replacements, p_old_info.static_initializer_info, p_new_info != nullptr ? &p_new_info->static_initializer_info : nullptr); +void GDScriptCompiler::_collect_function_ptr_replacements(HashMap &r_replacements, ScriptLambdaInfo &p_old_info, ScriptLambdaInfo *p_new_info) { + _collect_function_ptr_replacements(r_replacements, p_old_info.initializers, p_new_info ? &p_new_info->initializers : nullptr); - for (const KeyValue> &old_kv : p_old_info.member_function_infos) { - _get_function_ptr_replacements(r_replacements, old_kv.value, p_new_info != nullptr ? p_new_info->member_function_infos.getptr(old_kv.key) : nullptr); - } - for (int i = 0; i < p_old_info.other_function_infos.size(); ++i) { - const FunctionLambdaInfo &old_other_info = p_old_info.other_function_infos[i]; - const FunctionLambdaInfo *new_other_info = nullptr; - if (p_new_info != nullptr && p_new_info->other_function_infos.size() == p_old_info.other_function_infos.size()) { - // For now only attempt if the size is the same. - new_other_info = &p_new_info->other_function_infos[i]; - } - // Needs to be called on all old lambdas, even if there's no replacement. - _get_function_ptr_replacements(r_replacements, old_other_info, new_other_info); + for (KeyValue &old_kv : p_old_info.member_functions) { + _collect_function_ptr_replacements(r_replacements, old_kv.value, p_new_info ? p_new_info->member_functions.getptr(old_kv.key) : nullptr); } - for (const KeyValue &old_kv : p_old_info.subclass_info) { - const ScriptLambdaInfo &old_subinfo = old_kv.value; - const ScriptLambdaInfo *new_subinfo = p_new_info != nullptr ? p_new_info->subclass_info.getptr(old_kv.key) : nullptr; - _get_function_ptr_replacements(r_replacements, old_subinfo, new_subinfo); + for (KeyValue &old_kv : p_old_info.subclasses) { + ScriptLambdaInfo &old_subinfo = old_kv.value; + ScriptLambdaInfo *new_subinfo = p_new_info ? p_new_info->subclasses.getptr(old_kv.key) : nullptr; + _collect_function_ptr_replacements(r_replacements, old_subinfo, new_subinfo); } } @@ -3243,7 +3251,8 @@ Error GDScriptCompiler::compile(const GDScriptParser *p_parser, GDScript *p_scri source = p_script->get_path(); - ScriptLambdaInfo old_lambda_info = _get_script_lambda_replacement_info(p_script); + ScriptLambdaInfo old_lambda_info; + old_lambda_info.collect_script_lambda_replacement_info(p_script); // Create scripts for subclasses beforehand so they can be referenced make_scripts(p_script, root, p_keep_state); @@ -3260,10 +3269,11 @@ Error GDScriptCompiler::compile(const GDScriptParser *p_parser, GDScript *p_scri return err; } - ScriptLambdaInfo new_lambda_info = _get_script_lambda_replacement_info(p_script); + ScriptLambdaInfo new_lambda_info; + new_lambda_info.collect_script_lambda_replacement_info(p_script); HashMap func_ptr_replacements; - _get_function_ptr_replacements(func_ptr_replacements, old_lambda_info, &new_lambda_info); + _collect_function_ptr_replacements(func_ptr_replacements, old_lambda_info, &new_lambda_info); main_script->_recurse_replace_function_ptrs(func_ptr_replacements); if (has_static_data && !root->annotated_static_unload) { diff --git a/modules/gdscript/gdscript_compiler.h b/modules/gdscript/gdscript_compiler.h index 45f0f9e19b97..1752033353a4 100644 --- a/modules/gdscript/gdscript_compiler.h +++ b/modules/gdscript/gdscript_compiler.h @@ -36,6 +36,7 @@ #include "gdscript_function.h" #include "gdscript_parser.h" +#include "core/templates/hash_map.h" #include "core/templates/hash_set.h" class GDScriptCompiler { @@ -44,34 +45,44 @@ class GDScriptCompiler { HashSet parsing_classes; GDScript *main_script = nullptr; - struct FunctionLambdaInfo { +public: + struct LambdaSourceInfo; + + struct LambdaSourceInfoList { + using Element = List::Element; + List infos; + HashMap> infos_by_name; + + void collect_function_lambda_replacement_info(GDScriptFunction *p_func); + }; + + struct LambdaSourceInfo { GDScriptFunction *function = nullptr; GDScriptFunction *parent = nullptr; GDScript *script = nullptr; StringName name; - int line = 0; - int index = 0; - int depth = 0; //uint64_t code_hash; //int code_size; - int capture_count = 0; + bool is_static = false; bool use_self = false; + int capture_count = 0; int arg_count = 0; int default_arg_count = 0; - //Vector argument_types; - //GDScriptDataType return_type; - Vector sublambdas; + LambdaSourceInfoList sublambdas; + LambdaSourceInfoList::Element *self_element; + + bool can_be_replaced_by(const LambdaSourceInfo &p_new_info) const; }; struct ScriptLambdaInfo { - Vector implicit_initializer_info; - Vector implicit_ready_info; - Vector static_initializer_info; - HashMap> member_function_infos; - Vector other_function_infos; - HashMap subclass_info; + LambdaSourceInfoList initializers; + HashMap member_functions; + HashMap subclasses; + + void collect_script_lambda_replacement_info(GDScript *p_script); }; +private: struct CodeGen { GDScript *script = nullptr; const GDScriptParser::ClassNode *class_node = nullptr; @@ -161,13 +172,10 @@ class GDScriptCompiler { Error _parse_setter_getter(GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::VariableNode *p_variable, bool p_is_setter); Error _prepare_compilation(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state); Error _compile_class(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state); - FunctionLambdaInfo _get_function_replacement_info(GDScriptFunction *p_func, int p_index = -1, int p_depth = 0, GDScriptFunction *p_parent_func = nullptr); - Vector _get_function_lambda_replacement_info(GDScriptFunction *p_func, int p_depth = 0, GDScriptFunction *p_parent_func = nullptr); - ScriptLambdaInfo _get_script_lambda_replacement_info(GDScript *p_script); - bool _do_function_infos_match(const FunctionLambdaInfo &p_old_info, const FunctionLambdaInfo *p_new_info); - void _get_function_ptr_replacements(HashMap &r_replacements, const FunctionLambdaInfo &p_old_info, const FunctionLambdaInfo *p_new_info); - void _get_function_ptr_replacements(HashMap &r_replacements, const Vector &p_old_infos, const Vector *p_new_infos); - void _get_function_ptr_replacements(HashMap &r_replacements, const ScriptLambdaInfo &p_old_info, const ScriptLambdaInfo *p_new_info); + + static void _collect_function_ptr_replacements(HashMap &r_replacements, LambdaSourceInfoList &p_old, LambdaSourceInfoList *p_new); + static void _collect_function_ptr_replacements(HashMap &r_replacements, ScriptLambdaInfo &p_old_info, ScriptLambdaInfo *p_new_info); + int err_line = 0; int err_column = 0; StringName source; diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h index 6433072b5560..f40effe789e8 100644 --- a/modules/gdscript/gdscript_function.h +++ b/modules/gdscript/gdscript_function.h @@ -580,6 +580,7 @@ class GDScriptFunction { _FORCE_INLINE_ bool is_static() const { return _static; } _FORCE_INLINE_ MethodInfo get_method_info() const { return method_info; } _FORCE_INLINE_ int get_argument_count() const { return _argument_count; } + _FORCE_INLINE_ int get_default_argument_count() const { return _default_arg_count; } _FORCE_INLINE_ Variant get_rpc_config() const { return rpc_config; } _FORCE_INLINE_ int get_max_stack_size() const { return _stack_size; } diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 111a39d73029..c0ffb5a51ecb 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -812,9 +812,8 @@ bool GDScriptParser::has_class(const GDScriptParser::ClassNode *p_class) const { GDScriptParser::ClassNode *GDScriptParser::parse_class(bool p_is_static) { ClassNode *n_class = alloc_node(); - ClassNode *previous_class = current_class; - current_class = n_class; - n_class->outer = previous_class; + n_class->outer = current_class; + ScopedSet scoped_set_current_class(current_class, n_class); if (consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected identifier for the class name after "class".)")) { n_class->identifier = parse_identifier(); @@ -838,7 +837,6 @@ GDScriptParser::ClassNode *GDScriptParser::parse_class(bool p_is_static) { bool multiline = match(GDScriptTokenizer::Token::NEWLINE); if (multiline && !consume(GDScriptTokenizer::Token::INDENT, R"(Expected indented block after class declaration.)")) { - current_class = previous_class; complete_extents(n_class); return n_class; } @@ -858,7 +856,6 @@ GDScriptParser::ClassNode *GDScriptParser::parse_class(bool p_is_static) { consume(GDScriptTokenizer::Token::DEDENT, R"(Missing unindent at the end of the class body.)"); } - current_class = previous_class; return n_class; } @@ -981,6 +978,8 @@ void GDScriptParser::parse_class_member(T *(GDScriptParser::*p_parse_function)(b } void GDScriptParser::parse_class_body(bool p_is_multiline) { + ScopedSet scoped_set_current_variable(current_variable, nullptr); + bool class_end = false; bool next_is_static = false; while (!class_end && !is_at_end()) { @@ -1079,6 +1078,8 @@ GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static) { GDScriptParser::VariableNode *GDScriptParser::parse_variable(bool p_is_static, bool p_allow_property) { VariableNode *variable = alloc_node(); + ScopedSet scoped_set_current_variable(current_variable, variable); + if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected variable name after "var".)")) { complete_extents(variable); return nullptr; @@ -1315,6 +1316,8 @@ void GDScriptParser::parse_property_getter(VariableNode *p_variable) { GDScriptParser::ConstantNode *GDScriptParser::parse_constant(bool p_is_static) { ConstantNode *constant = alloc_node(); + ScopedSet scoped_set_current_variable(current_variable, constant); + if (!consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected constant name after "const".)")) { complete_extents(constant); return nullptr; @@ -1358,6 +1361,9 @@ GDScriptParser::ParameterNode *GDScriptParser::parse_parameter() { } ParameterNode *parameter = alloc_node(); + + ScopedSet scoped_set_current_variable(current_variable, parameter); + parameter->identifier = parse_identifier(); if (match(GDScriptTokenizer::Token::COLON)) { @@ -1705,6 +1711,8 @@ bool GDScriptParser::register_annotation(const MethodInfo &p_info, uint32_t p_ta } GDScriptParser::SuiteNode *GDScriptParser::parse_suite(const String &p_context, SuiteNode *p_suite, bool p_for_lambda) { + ScopedSet scoped_set_current_variable(current_variable, nullptr); + SuiteNode *suite = p_suite != nullptr ? p_suite : alloc_node(); suite->parent_block = current_suite; suite->parent_function = current_function; @@ -3410,6 +3418,7 @@ GDScriptParser::ExpressionNode *GDScriptParser::parse_lambda(ExpressionNode *p_p LambdaNode *lambda = alloc_node(); lambda->parent_function = current_function; lambda->parent_lambda = current_lambda; + lambda->parent_variable = current_variable; FunctionNode *function = alloc_node(); function->source_lambda = lambda; diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 7f64ae902b03..591835fb541b 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -931,6 +931,7 @@ class GDScriptParser { FunctionNode *function = nullptr; FunctionNode *parent_function = nullptr; LambdaNode *parent_lambda = nullptr; + AssignableNode *parent_variable = nullptr; Vector captures; HashMap captures_indices; bool use_self = false; @@ -1325,6 +1326,22 @@ class GDScriptParser { }; private: + template + class ScopedSet { + T *var; + T old_value; + + public: + template + ScopedSet(T &p_var, const TNew &p_new_value) : + var(&p_var), old_value(p_var) { + p_var = p_new_value; + } + ~ScopedSet() { + *var = old_value; + } + }; + friend class GDScriptAnalyzer; friend class GDScriptParserRef; @@ -1365,6 +1382,7 @@ class GDScriptParser { FunctionNode *current_function = nullptr; LambdaNode *current_lambda = nullptr; SuiteNode *current_suite = nullptr; + AssignableNode *current_variable = nullptr; CompletionContext completion_context; CompletionCall completion_call; diff --git a/modules/gdscript/tests/gdscript_test_runner.cpp b/modules/gdscript/tests/gdscript_test_runner.cpp index 025fcbd32a35..b345d839891f 100644 --- a/modules/gdscript/tests/gdscript_test_runner.cpp +++ b/modules/gdscript/tests/gdscript_test_runner.cpp @@ -38,8 +38,11 @@ #include "core/config/project_settings.h" #include "core/core_globals.h" +#include "core/error/error_macros.h" #include "core/io/dir_access.h" #include "core/io/file_access_pack.h" +#include "core/object/object.h" +#include "core/object/ref_counted.h" #include "core/os/os.h" #include "core/string/string_builder.h" #include "scene/resources/packed_scene.h" @@ -202,19 +205,21 @@ int GDScriptTestRunner::run_tests() { if (print_filenames) { print_line(test.get_source_relative_filepath()); } - GDScriptTest::TestResult result = test.run_test(); - - String expected = FileAccess::get_file_as_string(test.get_output_file()); + for (GDScriptTest::TestResult &result : test.run_test()) { + String source_file = result.source_test->get_source_file(); + String output_file = result.source_test->get_output_file(); + String expected = FileAccess::get_file_as_string(output_file); #ifndef DEBUG_ENABLED - expected = strip_warnings(expected); + expected = strip_warnings(expected); #endif - INFO(test.get_source_file()); - if (!result.passed) { - INFO(expected); - failed++; - } + INFO(source_file); + if (!result.passed) { + INFO(expected); + failed++; + } - CHECK_MESSAGE(result.passed, (result.passed ? String() : result.output)); + CHECK_MESSAGE(result.passed, (result.passed ? String() : result.output)); + } } return failed; @@ -298,7 +303,8 @@ bool GDScriptTestRunner::make_tests_for_dir(const String &p_dir) { } #endif - String out_file = next.get_basename() + ".out"; + String script_file_basename = next.get_basename(); + String out_file = script_file_basename + ".out"; ERR_FAIL_COND_V_MSG(!is_generating && !dir->file_exists(out_file), false, "Could not find output file for " + next); if (next.ends_with(".bin.gd")) { @@ -314,6 +320,22 @@ bool GDScriptTestRunner::make_tests_for_dir(const String &p_dir) { if (binary_tokens) { test.set_tokenizer_mode(GDScriptTest::TOKENIZER_BUFFER); } + for (int i = 1; true; ++i) { + String hotswap_file = script_file_basename + ".swap" + String::num_int64(i); + if (!dir->file_exists(hotswap_file)) { + break; + } + + // Arbitrary safety limit. + ERR_FAIL_COND_V_MSG(i >= 1000, false, "Too many hotswap files!"); + String hotswap_out_file = hotswap_file + ".out"; + ERR_FAIL_COND_V_MSG(!is_generating && !dir->file_exists(hotswap_out_file), false, "Could not find output file for " + hotswap_file); + GDScriptTest hotswap_test(current_dir.path_join(hotswap_file), current_dir.path_join(hotswap_out_file), source_dir); + if (binary_tokens) { + test.set_tokenizer_mode(GDScriptTest::TOKENIZER_BUFFER); + } + test.add_hotswap_test(hotswap_test); + } tests.push_back(test); } } @@ -527,27 +549,28 @@ String GDScriptTest::get_text_for_status(GDScriptTest::TestStatus p_status) cons return ""; } -GDScriptTest::TestResult GDScriptTest::execute_test_code(bool p_is_generating) { - disable_stdout(); - +GDScriptTest::TestResult GDScriptTest::execute_test_code(Ref &p_script, Object *&p_obj, Ref &p_obj_ref, GDScriptInstance *&p_instance, bool p_is_generating) { TestResult result; + result.source_test = this; result.status = GDTEST_OK; result.output = String(); result.passed = false; Error err = OK; - // Create script. - Ref script; - script.instantiate(); - script->set_path(source_file); + bool is_hotreload = true; + if (p_script.is_null()) { + p_script.instantiate(); + is_hotreload = false; + } + p_script->set_path(source_file); if (tokenizer_mode == TOKENIZER_TEXT) { - err = script->load_source_code(source_file); + err = p_script->load_source_code(source_file); } else { String code = FileAccess::get_file_as_string(source_file, &err); if (!err) { Vector buffer = GDScriptTokenizerBuffer::parse_code_string(code, GDScriptTokenizerBuffer::COMPRESS_ZSTD); - script->set_binary_tokens_source(buffer); + p_script->set_binary_tokens_source(buffer); } } if (err != OK) { @@ -560,9 +583,9 @@ GDScriptTest::TestResult GDScriptTest::execute_test_code(bool p_is_generating) { // Test parsing. GDScriptParser parser; if (tokenizer_mode == TOKENIZER_TEXT) { - err = parser.parse(script->get_source_code(), source_file, false); + err = parser.parse(p_script->get_source_code(), source_file, false); } else { - err = parser.parse_binary(script->get_binary_tokens_source(), source_file); + err = parser.parse_binary(p_script->get_binary_tokens_source(), source_file); } if (err != OK) { enable_stdout(); @@ -617,7 +640,7 @@ GDScriptTest::TestResult GDScriptTest::execute_test_code(bool p_is_generating) { // Test compiling. GDScriptCompiler compiler; - err = compiler.compile(&parser, script.ptr(), false); + err = compiler.compile(&parser, p_script.ptr(), is_hotreload); if (err != OK) { enable_stdout(); result.status = GDTEST_COMPILER_ERROR; @@ -635,7 +658,7 @@ GDScriptTest::TestResult GDScriptTest::execute_test_code(bool p_is_generating) { return result; } // Test running. - const HashMap::ConstIterator test_function_element = script->get_member_functions().find(GDScriptTestRunner::test_function_name); + const HashMap::ConstIterator test_function_element = p_script->get_member_functions().find(GDScriptTestRunner::test_function_name); if (!test_function_element) { enable_stdout(); result.status = GDTEST_LOAD_ERROR; @@ -652,7 +675,7 @@ GDScriptTest::TestResult GDScriptTest::execute_test_code(bool p_is_generating) { add_print_handler(&_print_handler); add_error_handler(&_error_handler); - err = script->reload(); + err = p_script->reload(is_hotreload); if (err) { enable_stdout(); result.status = GDTEST_LOAD_ERROR; @@ -661,18 +684,19 @@ GDScriptTest::TestResult GDScriptTest::execute_test_code(bool p_is_generating) { ERR_FAIL_V_MSG(result, "\nCould not reload script: '" + source_file + "'"); } - // Create object instance for test. - Object *obj = ClassDB::instantiate(script->get_native()->get_name()); - Ref obj_ref; - if (obj->is_ref_counted()) { - obj_ref = Ref(Object::cast_to(obj)); + if (!p_instance) { + // Create object instance for test. + p_obj = ClassDB::instantiate(p_script->get_native()->get_name()); + if (p_obj->is_ref_counted()) { + p_obj_ref = Ref(Object::cast_to(p_obj)); + } + p_obj->set_script(p_script); + p_instance = static_cast(p_obj->get_script_instance()); } - obj->set_script(script); - GDScriptInstance *instance = static_cast(obj->get_script_instance()); // Call test function. Callable::CallError call_err; - instance->callp(GDScriptTestRunner::test_function_name, nullptr, 0, call_err); + p_instance->callp(GDScriptTestRunner::test_function_name, nullptr, 0, call_err); // Tear down output handlers. remove_print_handler(&_print_handler); @@ -691,37 +715,59 @@ GDScriptTest::TestResult GDScriptTest::execute_test_code(bool p_is_generating) { result.passed = check_output(result.output); } - if (obj_ref.is_null()) { + return result; +} + +Vector GDScriptTest::execute_test_code(bool p_is_generating) { + disable_stdout(); + + Ref script; + Object *obj = nullptr; + Ref obj_ref; + GDScriptInstance *instance = nullptr; + + Vector test_results; + test_results.push_back(execute_test_code(script, obj, obj_ref, instance, p_is_generating)); + + for (GDScriptTest &hotswap_test : hotswap_tests) { + test_results.push_back(hotswap_test.execute_test_code(script, obj, obj_ref, instance, p_is_generating)); + } + + if (obj && obj_ref.is_null()) { memdelete(obj); } + obj = nullptr; + obj_ref = nullptr; + instance = nullptr; enable_stdout(); GDScriptCache::remove_script(script->get_path()); - return result; + return test_results; } -GDScriptTest::TestResult GDScriptTest::run_test() { +Vector GDScriptTest::run_test() { return execute_test_code(false); } bool GDScriptTest::generate_output() { - TestResult result = execute_test_code(true); - if (result.status == GDTEST_LOAD_ERROR) { - return false; - } + for (TestResult &result : execute_test_code(true)) { + if (result.status == GDTEST_LOAD_ERROR) { + return false; + } - Error err = OK; - Ref out_file = FileAccess::open(output_file, FileAccess::WRITE, &err); - if (err != OK) { - return false; - } + Error err = OK; + Ref out_file = FileAccess::open(result.source_test->output_file, FileAccess::WRITE, &err); + if (err != OK) { + return false; + } - String output = result.output.strip_edges(); // TODO: may be hacky. - output += "\n"; // Make sure to insert newline for CI static checks. + String output = result.output.strip_edges(); // TODO: may be hacky. + output += "\n"; // Make sure to insert newline for CI static checks. - out_file->store_string(output); + out_file->store_string(output); + } return true; } diff --git a/modules/gdscript/tests/gdscript_test_runner.h b/modules/gdscript/tests/gdscript_test_runner.h index 57e3ac86f9d1..0c73b1ef1149 100644 --- a/modules/gdscript/tests/gdscript_test_runner.h +++ b/modules/gdscript/tests/gdscript_test_runner.h @@ -57,6 +57,7 @@ class GDScriptTest { }; struct TestResult { + GDScriptTest *source_test; TestStatus status; String output; bool passed; @@ -80,6 +81,7 @@ class GDScriptTest { String source_file; String output_file; String base_dir; + Vector hotswap_tests; PrintHandlerList _print_handler; ErrorHandlerList _error_handler; @@ -91,18 +93,20 @@ class GDScriptTest { bool check_output(const String &p_output) const; String get_text_for_status(TestStatus p_status) const; - TestResult execute_test_code(bool p_is_generating); + TestResult execute_test_code(Ref &p_script, Object *&p_obj, Ref &p_obj_ref, GDScriptInstance *&p_instance, bool p_is_generating); + Vector execute_test_code(bool p_is_generating); public: static void print_handler(void *p_this, const String &p_message, bool p_error, bool p_rich); static void error_handler(void *p_this, const char *p_function, const char *p_file, int p_line, const char *p_error, const char *p_explanation, bool p_editor_notify, ErrorHandlerType p_type); - TestResult run_test(); + Vector run_test(); bool generate_output(); const String &get_source_file() const { return source_file; } const String get_source_relative_filepath() const { return source_file.trim_prefix(base_dir); } const String &get_output_file() const { return output_file; } + void add_hotswap_test(const GDScriptTest &hotswap_test) { hotswap_tests.push_back(hotswap_test); } void set_tokenizer_mode(TokenizerMode p_tokenizer_mode) { tokenizer_mode = p_tokenizer_mode; } TokenizerMode get_tokenizer_mode() const { return tokenizer_mode; } diff --git a/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.gd b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.gd new file mode 100644 index 000000000000..52ed3b022419 --- /dev/null +++ b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.gd @@ -0,0 +1,21 @@ +var member1 := func(): + return "Member 1" + +var local1: Callable + +var param1: Callable + +func test(): + var _v1 = func(): return "Local 1" + local1 = _v1 + + print(local1.call()) + + print(member1.call()) + + test_parameters() + + print(param1.call()) + +func test_parameters(_v1 = func(): return "Param 1"): + param1 = _v1 diff --git a/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.out b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.out new file mode 100644 index 000000000000..12348e1d1767 --- /dev/null +++ b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.out @@ -0,0 +1,4 @@ +GDTEST_OK +Local 1 +Member 1 +Param 1 diff --git a/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap1 b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap1 new file mode 100644 index 000000000000..2a0334342281 --- /dev/null +++ b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap1 @@ -0,0 +1,36 @@ +var member1 := func(): + return "Member 1 Hotswap 1" + +var member2 := func(): + return "Member 2 Hotswap 1" + +var local1: Callable +var local2: Callable + +var param1: Callable +var param2: Callable + +func test(): + var _v1 = func(): return "Local 1 Hotswap 1" + var _v2 = func(): return "Local 2 Hotswap 1" + # Only set local2 here, local1 has already been set. + local2 = _v2 + + print(local1.call()) + print(local2.call()) + + + print(member1.call()) + if member2: + print(member2.call()) + else: + print(member2) + + test_parameters() + + print(param1.call()) + print(param2.call()) + +func test_parameters(_v1 = func(): return "Param 1 Hotswap 1", _v2 = func(): return "Param 2 Hotswap 1"): + # Only set param2 here, param1 has already been set. + param2 = _v2 diff --git a/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap1.out b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap1.out new file mode 100644 index 000000000000..c8a3951f3524 --- /dev/null +++ b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap1.out @@ -0,0 +1,7 @@ +GDTEST_OK +Local 1 Hotswap 1 +Local 2 Hotswap 1 +Member 1 Hotswap 1 + +Param 1 Hotswap 1 +Param 2 Hotswap 1 diff --git a/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap2 b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap2 new file mode 100644 index 000000000000..78aa0adf4e9d --- /dev/null +++ b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap2 @@ -0,0 +1,34 @@ +var member2 := func(): + return "Member 2 Hotswap 2" + +var member1 := func(): + return "Member 1 Hotswap 2" + +var local1: Callable +var local2: Callable + +var param1: Callable +var param2: Callable + +func test(): + var _v2 = func(): return "Local 2 Hotswap 2" + var _v1 = func(): return "Local 1 Hotswap 2" + # local1 and local2 have already been set. + + print(local1.call()) + print(local2.call()) + + print(member1.call()) + if member2: + print(member2.call()) + else: + print(member2) + + test_parameters() + + print(param1.call()) + print(param2.call()) + +func test_parameters(_v2 = func(): return "Param 2 Hotswap 2", _v1 = func(): return "Param 1 Hotswap 2"): + # param1 and param2 have already been set. + pass diff --git a/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap2.out b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap2.out new file mode 100644 index 000000000000..855a721b2933 --- /dev/null +++ b/modules/gdscript/tests/scripts/hotswap/hotswap_lambda.swap2.out @@ -0,0 +1,7 @@ +GDTEST_OK +Local 1 Hotswap 2 +Local 2 Hotswap 2 +Member 1 Hotswap 2 + +Param 1 Hotswap 2 +Param 2 Hotswap 2 diff --git a/modules/gdscript/tests/scripts/runtime/features/lambda_get_method.gd b/modules/gdscript/tests/scripts/runtime/features/lambda_get_method.gd index 160e43a797eb..3efc948e65e9 100644 --- a/modules/gdscript/tests/scripts/runtime/features/lambda_get_method.gd +++ b/modules/gdscript/tests/scripts/runtime/features/lambda_get_method.gd @@ -10,7 +10,7 @@ func test(): foo() print(lambda_self.get_method()) # Should print "test". - print(anon_lambda_self.get_method()) # Should print "". + print(anon_lambda_self.get_method()) # Should print "". var lambda_non_self := func test() -> void: pass @@ -18,4 +18,4 @@ func test(): pass print(lambda_non_self.get_method()) # Should print "test". - print(anon_lambda_non_self.get_method()) # Should print "". + print(anon_lambda_non_self.get_method()) # Should print "". diff --git a/modules/gdscript/tests/scripts/runtime/features/lambda_get_method.out b/modules/gdscript/tests/scripts/runtime/features/lambda_get_method.out index 17ee47fca284..30b558c08c30 100644 --- a/modules/gdscript/tests/scripts/runtime/features/lambda_get_method.out +++ b/modules/gdscript/tests/scripts/runtime/features/lambda_get_method.out @@ -1,5 +1,5 @@ GDTEST_OK test - + test - +