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: Fix get_*_list() methods return incorrect info #81079

Merged
merged 1 commit into from
Sep 11, 2023
Merged

GDScript: Fix get_*_list() methods return incorrect info #81079

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
9 changes: 6 additions & 3 deletions modules/gdscript/editor/gdscript_docgen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ static void _doctype_from_gdtype(const GDType &p_gdtype, String &r_type, String
case GDType::SCRIPT:
if (p_gdtype.script_type.is_valid()) {
if (p_gdtype.script_type->get_global_name() != StringName()) {
r_type = _get_script_path(p_gdtype.script_type->get_global_name());
r_type = p_gdtype.script_type->get_global_name();
return;
}
if (!p_gdtype.script_type->get_path().is_empty()) {
Expand Down Expand Up @@ -129,10 +129,10 @@ void GDScriptDocGen::generate_docs(GDScript *p_script, const GDP::ClassNode *p_c
DocData::ClassDoc &doc = p_script->doc;

doc.script_path = _get_script_path(p_script->get_script_path());
if (p_script->name.is_empty()) {
if (p_script->local_name == StringName()) {
doc.name = doc.script_path;
} else {
doc.name = p_script->name;
doc.name = p_script->local_name;
}

if (p_script->_owner) {
Expand Down Expand Up @@ -204,6 +204,9 @@ void GDScriptDocGen::generate_docs(GDScript *p_script, const GDP::ClassNode *p_c

if (m_func->return_type) {
_doctype_from_gdtype(m_func->return_type->get_datatype(), method_doc.return_type, method_doc.return_enum, true);
} else if (!m_func->body->has_return) {
// If no `return` statement, then return type is `void`, not `Variant`.
method_doc.return_type = "void";
} else {
method_doc.return_type = "Variant";
}
Expand Down
127 changes: 35 additions & 92 deletions modules/gdscript/gdscript.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ Ref<Script> GDScript::get_base_script() const {
}

StringName GDScript::get_global_name() const {
return name;
return global_name;
}

StringName GDScript::get_instance_base_type() const {
Expand Down Expand Up @@ -284,27 +284,9 @@ void GDScript::_get_script_method_list(List<MethodInfo> *r_list, bool p_include_
const GDScript *current = this;
while (current) {
for (const KeyValue<StringName, GDScriptFunction *> &E : current->member_functions) {
GDScriptFunction *func = E.value;
MethodInfo mi;
mi.name = E.key;

if (func->is_static()) {
mi.flags |= METHOD_FLAG_STATIC;
}

for (int i = 0; i < func->get_argument_count(); i++) {
PropertyInfo arginfo = func->get_argument_type(i);
#ifdef TOOLS_ENABLED
arginfo.name = func->get_argument_name(i);
#endif
mi.arguments.push_back(arginfo);
}
#ifdef TOOLS_ENABLED
mi.default_arguments.append_array(func->get_default_arg_values());
#endif
mi.return_val = func->get_return_type();
r_list->push_back(mi);
r_list->push_back(E.value->get_method_info());
}

if (!p_include_base) {
return;
}
Expand All @@ -323,18 +305,17 @@ void GDScript::_get_script_property_list(List<PropertyInfo> *r_list, bool p_incl

while (sptr) {
Vector<_GDScriptMemberSort> msort;
for (const KeyValue<StringName, PropertyInfo> &E : sptr->member_info) {
for (const KeyValue<StringName, MemberInfo> &E : sptr->member_indices) {
_GDScriptMemberSort ms;
ERR_CONTINUE(!sptr->member_indices.has(E.key));
ms.index = sptr->member_indices[E.key].index;
ms.index = E.value.index;
ms.name = E.key;
msort.push_back(ms);
}

msort.sort();
msort.reverse();
for (int i = 0; i < msort.size(); i++) {
props.push_front(sptr->member_info[msort[i].name]);
props.push_front(sptr->member_indices[msort[i].name].property_info);
}

#ifdef TOOLS_ENABLED
Expand Down Expand Up @@ -368,15 +349,7 @@ MethodInfo GDScript::get_method_info(const StringName &p_method) const {
return MethodInfo();
}

GDScriptFunction *func = E->value;
MethodInfo mi;
mi.name = E->key;
for (int i = 0; i < func->get_argument_count(); i++) {
mi.arguments.push_back(func->get_argument_type(i));
}

mi.return_val = func->get_return_type();
return mi;
return E->value->get_method_info();
}

bool GDScript::get_property_default_value(const StringName &p_property, Variant &r_value) const {
Expand Down Expand Up @@ -557,13 +530,7 @@ bool GDScript::_update_exports(bool *r_err, bool p_recursive_call, PlaceHolderSc
member_default_values_cache[member.variable->identifier->name] = default_value;
} break;
case GDScriptParser::ClassNode::Member::SIGNAL: {
// TODO: Cache this in parser to avoid loops like this.
Vector<StringName> parameters_names;
parameters_names.resize(member.signal->parameters.size());
for (int j = 0; j < member.signal->parameters.size(); j++) {
parameters_names.write[j] = member.signal->parameters[j]->identifier->name;
}
_signals[member.signal->identifier->name] = parameters_names;
_signals[member.signal->identifier->name] = member.signal->method_info;
} break;
case GDScriptParser::ClassNode::Member::GROUP: {
members_cache.push_back(member.annotation->export_info);
Expand Down Expand Up @@ -977,22 +944,26 @@ bool GDScript::_set(const StringName &p_name, const Variant &p_value) {
void GDScript::_get_property_list(List<PropertyInfo> *p_properties) const {
p_properties->push_back(PropertyInfo(Variant::STRING, "script/source", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NO_EDITOR | PROPERTY_USAGE_INTERNAL));

List<PropertyInfo> property_list;

List<const GDScript *> classes;
const GDScript *top = this;
while (top) {
for (const KeyValue<StringName, MemberInfo> &E : top->static_variables_indices) {
PropertyInfo pi = PropertyInfo(E.value.data_type);
pi.name = E.key;
pi.usage |= PROPERTY_USAGE_SCRIPT_VARIABLE; // For the script (as a class) it is a non-static property.
property_list.push_back(pi);
}

classes.push_back(top);
top = top->_base;
}

for (const List<PropertyInfo>::Element *E = property_list.back(); E; E = E->prev()) {
p_properties->push_back(E->get());
for (const List<const GDScript *>::Element *E = classes.back(); E; E = E->prev()) {
Vector<_GDScriptMemberSort> msort;
for (const KeyValue<StringName, MemberInfo> &F : E->get()->static_variables_indices) {
_GDScriptMemberSort ms;
ms.index = F.value.index;
ms.name = F.key;
msort.push_back(ms);
}
msort.sort();

for (int i = 0; i < msort.size(); i++) {
p_properties->push_back(E->get()->static_variables_indices[msort[i].name].property_info);
}
}
}

Expand Down Expand Up @@ -1110,7 +1081,7 @@ GDScript *GDScript::find_class(const String &p_qualified_name) {
Vector<String> class_names;
GDScript *result = nullptr;
// Empty initial name means start here.
if (first.is_empty() || first == name) {
if (first.is_empty() || first == global_name) {
class_names = p_qualified_name.split("::");
result = this;
} else if (p_qualified_name.begins_with(get_root_script()->path)) {
Expand Down Expand Up @@ -1245,15 +1216,8 @@ bool GDScript::has_script_signal(const StringName &p_signal) const {
}

void GDScript::_get_script_signal_list(List<MethodInfo> *r_list, bool p_include_base) const {
for (const KeyValue<StringName, Vector<StringName>> &E : _signals) {
MethodInfo mi;
mi.name = E.key;
for (int i = 0; i < E.value.size(); i++) {
PropertyInfo arg;
arg.name = E.value[i];
mi.arguments.push_back(arg);
}
r_list->push_back(mi);
for (const KeyValue<StringName, MethodInfo> &E : _signals) {
r_list->push_back(E.value);
}

if (!p_include_base) {
Expand All @@ -1274,21 +1238,6 @@ void GDScript::get_script_signal_list(List<MethodInfo> *r_signals) const {
_get_script_signal_list(r_signals, true);
}

String GDScript::_get_gdscript_reference_class_name(const GDScript *p_gdscript) {
ERR_FAIL_NULL_V(p_gdscript, String());

String class_name;
while (p_gdscript) {
if (class_name.is_empty()) {
class_name = p_gdscript->get_script_class_name();
} else {
class_name = p_gdscript->get_script_class_name() + "." + class_name;
}
p_gdscript = p_gdscript->_owner;
}
return class_name;
}

GDScript *GDScript::_get_gdscript_from_variant(const Variant &p_variant) {
Object *obj = p_variant;
if (obj == nullptr || obj->get_instance_id().is_null()) {
Expand Down Expand Up @@ -1420,8 +1369,8 @@ String GDScript::debug_get_script_name(const Ref<Script> &p_script) {
if (p_script.is_valid()) {
Ref<GDScript> gdscript = p_script;
if (gdscript.is_valid()) {
if (!gdscript->get_script_class_name().is_empty()) {
return gdscript->get_script_class_name();
if (gdscript->get_local_name() != StringName()) {
return gdscript->get_local_name();
}
return gdscript->get_fully_qualified_name().get_file();
}
Expand Down Expand Up @@ -1667,7 +1616,7 @@ bool GDScriptInstance::get(const StringName &p_name, Variant &r_ret) const {
}

{
HashMap<StringName, Vector<StringName>>::ConstIterator E = sptr->_signals.find(p_name);
HashMap<StringName, MethodInfo>::ConstIterator E = sptr->_signals.find(p_name);
if (E) {
r_ret = Signal(this->owner, E->key);
return true;
Expand Down Expand Up @@ -1717,11 +1666,11 @@ bool GDScriptInstance::get(const StringName &p_name, Variant &r_ret) const {
Variant::Type GDScriptInstance::get_property_type(const StringName &p_name, bool *r_is_valid) const {
const GDScript *sptr = script.ptr();
while (sptr) {
if (sptr->member_info.has(p_name)) {
if (sptr->member_indices.has(p_name)) {
if (r_is_valid) {
*r_is_valid = true;
}
return sptr->member_info[p_name].type;
return sptr->member_indices[p_name].property_info.type;
}
sptr = sptr->_base;
}
Expand Down Expand Up @@ -1798,18 +1747,17 @@ void GDScriptInstance::get_property_list(List<PropertyInfo> *p_properties) const
//instance a fake script for editing the values

Vector<_GDScriptMemberSort> msort;
for (const KeyValue<StringName, PropertyInfo> &F : sptr->member_info) {
for (const KeyValue<StringName, GDScript::MemberInfo> &F : sptr->member_indices) {
_GDScriptMemberSort ms;
ERR_CONTINUE(!sptr->member_indices.has(F.key));
ms.index = sptr->member_indices[F.key].index;
ms.index = F.value.index;
ms.name = F.key;
msort.push_back(ms);
}

msort.sort();
msort.reverse();
for (int i = 0; i < msort.size(); i++) {
props.push_front(sptr->member_info[msort[i].name]);
props.push_front(sptr->member_indices[msort[i].name].property_info);
}

#ifdef TOOLS_ENABLED
Expand Down Expand Up @@ -1872,12 +1820,7 @@ void GDScriptInstance::get_method_list(List<MethodInfo> *p_list) const {
const GDScript *sptr = script.ptr();
while (sptr) {
for (const KeyValue<StringName, GDScriptFunction *> &E : sptr->member_functions) {
MethodInfo mi;
mi.name = E.key;
for (int i = 0; i < E.value->get_argument_count(); i++) {
mi.arguments.push_back(PropertyInfo(Variant::NIL, "arg" + itos(i)));
}
p_list->push_back(mi);
p_list->push_back(E.value->get_method_info());
}
sptr = sptr->_base;
}
Expand Down
14 changes: 6 additions & 8 deletions modules/gdscript/gdscript.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class GDScript : public Script {
StringName setter;
StringName getter;
GDScriptDataType data_type;
PropertyInfo property_info;
};

struct ClearData {
Expand Down Expand Up @@ -100,7 +101,7 @@ class GDScript : public Script {
HashMap<StringName, GDScriptFunction *> member_functions;
HashMap<StringName, MemberInfo> member_indices; //members are just indices to the instantiated script.
HashMap<StringName, Ref<GDScript>> subclasses;
HashMap<StringName, Vector<StringName>> _signals;
HashMap<StringName, MethodInfo> _signals;
Dictionary rpc_config;

#ifdef TOOLS_ENABLED
Expand All @@ -126,8 +127,6 @@ class GDScript : public Script {
void _add_doc(const DocData::ClassDoc &p_inner_class);
#endif

HashMap<StringName, PropertyInfo> member_info;

GDScriptFunction *implicit_initializer = nullptr;
GDScriptFunction *initializer = nullptr; //direct pointer to new , faster to locate
GDScriptFunction *implicit_ready = nullptr;
Expand All @@ -142,7 +141,8 @@ class GDScript : public Script {
//exported members
String source;
String path;
String name;
StringName local_name; // Inner class identifier or `class_name`.
StringName global_name; // `class_name`.
String fully_qualified_name;
String simplified_icon_path;
SelfList<GDScript> script_list;
Expand Down Expand Up @@ -174,9 +174,6 @@ class GDScript : public Script {
void _get_script_method_list(List<MethodInfo> *r_list, bool p_include_base) const;
void _get_script_signal_list(List<MethodInfo> *r_list, bool p_include_base) const;

// This method will map the class name from "RefCounted" to "MyClass.InnerClass".
static String _get_gdscript_reference_class_name(const GDScript *p_gdscript);

GDScript *_get_gdscript_from_variant(const Variant &p_variant);
void _get_dependencies(RBSet<GDScript *> &p_dependencies, const GDScript *p_except);

Expand All @@ -194,6 +191,8 @@ class GDScript : public Script {
static String debug_get_script_name(const Ref<Script> &p_script);
#endif

_FORCE_INLINE_ StringName get_local_name() const { return local_name; }

void clear(GDScript::ClearData *p_clear_data = nullptr);

virtual bool is_valid() const override { return valid; }
Expand All @@ -214,7 +213,6 @@ class GDScript : public Script {
}
const HashMap<StringName, GDScriptFunction *> &get_member_functions() const { return member_functions; }
const Ref<GDScriptNativeClass> &get_native() const { return native; }
const String &get_script_class_name() const { return name; }

RBSet<GDScript *> get_dependencies();
RBSet<GDScript *> get_inverted_dependencies();
Expand Down
10 changes: 6 additions & 4 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1000,10 +1000,11 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
GDScriptParser::ParameterNode *param = member.signal->parameters[j];
GDScriptParser::DataType param_type = type_from_metatype(resolve_datatype(param->datatype_specifier));
param->set_datatype(param_type);
mi.arguments.push_back(PropertyInfo(param_type.builtin_type, param->identifier->name));
// TODO: add signal parameter default values
mi.arguments.push_back(param_type.to_property_info(param->identifier->name));
// Signals do not support parameter default values.
}
member.signal->set_datatype(make_signal_type(mi));
member.signal->method_info = mi;

// Apply annotations.
for (GDScriptParser::AnnotationNode *&E : member.signal->annotations) {
Expand Down Expand Up @@ -1604,17 +1605,18 @@ void GDScriptAnalyzer::resolve_function_signature(GDScriptParser::FunctionNode *
}
is_shadowing(p_function->parameters[i]->identifier, "function parameter", true);
#endif // DEBUG_ENABLED
#ifdef TOOLS_ENABLED

if (p_function->parameters[i]->initializer) {
#ifdef TOOLS_ENABLED
default_value_count++;
#endif // TOOLS_ENABLED

if (p_function->parameters[i]->initializer->is_constant) {
p_function->default_arg_values.push_back(p_function->parameters[i]->initializer->reduced_value);
} else {
p_function->default_arg_values.push_back(Variant()); // Prevent shift.
}
}
#endif // TOOLS_ENABLED
}

if (!p_is_lambda && function_name == GDScriptLanguage::get_singleton()->strings._init) {
Expand Down
3 changes: 0 additions & 3 deletions modules/gdscript/gdscript_byte_codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
#include "core/debugger/engine_debugger.h"

uint32_t GDScriptByteCodeGenerator::add_parameter(const StringName &p_name, bool p_is_optional, const GDScriptDataType &p_type) {
#ifdef TOOLS_ENABLED
function->arg_names.push_back(p_name);
#endif
function->_argument_count++;
function->argument_types.push_back(p_type);
if (p_is_optional) {
Expand Down
Loading