Skip to content

Commit

Permalink
GDScript: Fix get_*_list() methods return incorrect info
Browse files Browse the repository at this point in the history
  • Loading branch information
dalexeev committed Sep 3, 2023
1 parent fa3428f commit 462d8f4
Show file tree
Hide file tree
Showing 15 changed files with 429 additions and 305 deletions.
9 changes: 6 additions & 3 deletions modules/gdscript/editor/gdscript_docgen.cpp
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
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
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
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
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

0 comments on commit 462d8f4

Please sign in to comment.