From 6e6b455d1fe791894e1a6bd66e1f5f8471c02abc Mon Sep 17 00:00:00 2001 From: Ignacio Etcheverry Date: Tue, 17 Oct 2017 14:02:19 +0200 Subject: [PATCH] Export attribute fixes and improvements - Allow non-public fields to be exported as well (to avoid confusion). - Set PROPERTY_HINT_RESOURCE_TYPE for resource derived fields. - Support enums and automatically fill PROPERTY_HINT_ENUM's hint_string for enum fields. --- modules/mono/csharp_script.cpp | 48 ++++++++++++------- modules/mono/glue/cs_files/ExportAttribute.cs | 4 +- modules/mono/mono_gd/gd_mono_class.cpp | 32 +++++++++++++ modules/mono/mono_gd/gd_mono_class.h | 6 +++ modules/mono/mono_gd/gd_mono_field.cpp | 4 ++ modules/mono/mono_gd/gd_mono_marshal.cpp | 16 ++++++- modules/mono/mono_gd/gd_mono_marshal.h | 1 + modules/mono/mono_gd/gd_mono_utils.cpp | 4 +- modules/mono/mono_gd/gd_mono_utils.h | 4 +- 9 files changed, 94 insertions(+), 25 deletions(-) diff --git a/modules/mono/csharp_script.cpp b/modules/mono/csharp_script.cpp index 69895e58e2..01ac4d83bb 100644 --- a/modules/mono/csharp_script.cpp +++ b/modules/mono/csharp_script.cpp @@ -1268,8 +1268,10 @@ bool CSharpScript::_update_exports() { for (int i = 0; i < fields.size(); i++) { GDMonoField *field = fields[i]; - if (field->is_static() || field->get_visibility() != GDMono::PUBLIC) + if (field->is_static()) { + ERR_PRINTS("Cannot export field because it is static: " + top->get_full_name() + "." + field->get_name()); continue; + } String name = field->get_name(); StringName cname = name; @@ -1277,17 +1279,39 @@ bool CSharpScript::_update_exports() { if (member_info.has(cname)) continue; - Variant::Type type = GDMonoMarshal::managed_to_variant_type(field->get_type()); + ManagedType field_type = field->get_type(); + Variant::Type type = GDMonoMarshal::managed_to_variant_type(field_type); if (field->has_attribute(CACHED_CLASS(ExportAttribute))) { + // Field has Export attribute MonoObject *attr = field->get_attribute(CACHED_CLASS(ExportAttribute)); - // Field has Export attribute - int hint = CACHED_FIELD(ExportAttribute, hint)->get_int_value(attr); - String hint_string = CACHED_FIELD(ExportAttribute, hint_string)->get_string_value(attr); - int usage = CACHED_FIELD(ExportAttribute, usage)->get_int_value(attr); + PropertyHint hint; + String hint_string; - PropertyInfo prop_info = PropertyInfo(type, name, PropertyHint(hint), hint_string, PropertyUsageFlags(usage)); + if (type == Variant::NIL) { + ERR_PRINTS("Unknown type of exported field: " + top->get_full_name() + "." + field->get_name()); + continue; + } else if (type == Variant::INT && field_type.type_encoding == MONO_TYPE_VALUETYPE && mono_class_is_enum(field_type.type_class->get_raw())) { + type = Variant::INT; + hint = PROPERTY_HINT_ENUM; + + Vector fields = field_type.type_class->get_enum_fields(); + + for (int i = 0; i < fields.size(); i++) { + if (i > 0) + hint_string += ","; + hint_string += mono_field_get_name(fields[i]); + } + } else if (type == Variant::OBJECT && CACHED_CLASS(GodotReference)->is_assignable_from(field_type.type_class)) { + hint = PROPERTY_HINT_RESOURCE_TYPE; + hint_string = NATIVE_GDMONOCLASS_NAME(field_type.type_class); + } else { + hint = PropertyHint(CACHED_FIELD(ExportAttribute, hint)->get_int_value(attr)); + hint_string = CACHED_FIELD(ExportAttribute, hint_string)->get_string_value(attr); + } + + PropertyInfo prop_info = PropertyInfo(type, name, hint, hint_string, PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_SCRIPT_VARIABLE); member_info[cname] = prop_info; exported_members_cache.push_back(prop_info); @@ -1711,16 +1735,6 @@ void CSharpScript::update_exports() { #ifdef TOOLS_ENABLED _update_exports(); - - if (placeholders.size()) { - Map values; - List propnames; - _update_exports_values(values, propnames); - - for (Set::Element *E = placeholders.front(); E; E = E->next()) { - E->get()->update(propnames, values); - } - } #endif } diff --git a/modules/mono/glue/cs_files/ExportAttribute.cs b/modules/mono/glue/cs_files/ExportAttribute.cs index af3f603d6d..a4e7d447dd 100644 --- a/modules/mono/glue/cs_files/ExportAttribute.cs +++ b/modules/mono/glue/cs_files/ExportAttribute.cs @@ -7,13 +7,11 @@ namespace Godot { private int hint; private string hint_string; - private int usage; - public ExportAttribute(int hint = GD.PROPERTY_HINT_NONE, string hint_string = "", int usage = GD.PROPERTY_USAGE_DEFAULT) + public ExportAttribute(int hint = GD.PROPERTY_HINT_NONE, string hint_string = "") { this.hint = hint; this.hint_string = hint_string; - this.usage = usage; } } } diff --git a/modules/mono/mono_gd/gd_mono_class.cpp b/modules/mono/mono_gd/gd_mono_class.cpp index 0134ace5d7..77ba0ee90e 100644 --- a/modules/mono/mono_gd/gd_mono_class.cpp +++ b/modules/mono/mono_gd/gd_mono_class.cpp @@ -43,6 +43,14 @@ bool GDMonoClass::is_assignable_from(GDMonoClass *p_from) const { return mono_class_is_assignable_from(mono_class, p_from->mono_class); } +String GDMonoClass::get_full_name() const { + + String res = namespace_name; + if (res.length()) + res += "."; + return res + class_name; +} + GDMonoClass *GDMonoClass::get_parent_class() { if (assembly) { @@ -56,6 +64,30 @@ GDMonoClass *GDMonoClass::get_parent_class() { return NULL; } +#ifdef TOOLS_ENABLED +Vector GDMonoClass::get_enum_fields() { + + bool class_is_enum = mono_class_is_enum(mono_class); + ERR_FAIL_COND_V(!class_is_enum, Vector()); + + Vector enum_fields; + + void *iter = NULL; + MonoClassField *raw_field = NULL; + while ((raw_field = mono_class_get_fields(get_raw(), &iter)) != NULL) { + uint32_t field_flags = mono_field_get_flags(raw_field); + + // Enums have an instance field named value__ which holds the value of the enum. + // Enum constants are static, so we will use this to ignore the value__ field. + if (field_flags & MONO_FIELD_ATTR_PUBLIC && field_flags & MONO_FIELD_ATTR_STATIC) { + enum_fields.push_back(raw_field); + } + } + + return enum_fields; +} +#endif + bool GDMonoClass::has_method(const StringName &p_name) { return get_method(p_name) != NULL; diff --git a/modules/mono/mono_gd/gd_mono_class.h b/modules/mono/mono_gd/gd_mono_class.h index 1e72553879..ef1ca425a7 100644 --- a/modules/mono/mono_gd/gd_mono_class.h +++ b/modules/mono/mono_gd/gd_mono_class.h @@ -98,8 +98,14 @@ public: _FORCE_INLINE_ MonoClass *get_raw() const { return mono_class; } _FORCE_INLINE_ const GDMonoAssembly *get_assembly() const { return assembly; } + String get_full_name() const; + GDMonoClass *get_parent_class(); +#ifdef TOOLS_ENABLED + Vector get_enum_fields(); +#endif + bool has_method(const StringName &p_name); bool has_attribute(GDMonoClass *p_attr_class); diff --git a/modules/mono/mono_gd/gd_mono_field.cpp b/modules/mono/mono_gd/gd_mono_field.cpp index c2d8eeaa32..81315ee87a 100644 --- a/modules/mono/mono_gd/gd_mono_field.cpp +++ b/modules/mono/mono_gd/gd_mono_field.cpp @@ -51,6 +51,7 @@ void GDMonoField::set_value(MonoObject *p_object, const Variant &p_value) { { \ m_type val = p_value.operator m_type(); \ mono_field_set_value(p_object, mono_field, &val); \ + break; \ } #define SET_FROM_ARRAY_AND_BREAK(m_type) \ @@ -137,6 +138,9 @@ void GDMonoField::set_value(MonoObject *p_object, const Variant &p_value) { if (tclass == CACHED_CLASS(Plane)) SET_FROM_STRUCT_AND_BREAK(Plane); + if (mono_class_is_enum(tclass->get_raw())) + SET_FROM_PRIMITIVE(signed int); + ERR_EXPLAIN(String() + "Attempted to set the value of a field of unmarshallable type: " + tclass->get_name()); ERR_FAIL(); } break; diff --git a/modules/mono/mono_gd/gd_mono_marshal.cpp b/modules/mono/mono_gd/gd_mono_marshal.cpp index 9a6c8f0cd6..77a1ef3cb0 100644 --- a/modules/mono/mono_gd/gd_mono_marshal.cpp +++ b/modules/mono/mono_gd/gd_mono_marshal.cpp @@ -112,6 +112,9 @@ Variant::Type managed_to_variant_type(const ManagedType &p_type) { if (tclass == CACHED_CLASS(Plane)) return Variant::PLANE; + + if (mono_class_is_enum(tclass->get_raw())) + return Variant::INT; } break; case MONO_TYPE_ARRAY: @@ -165,9 +168,12 @@ Variant::Type managed_to_variant_type(const ManagedType &p_type) { return Variant::DICTIONARY; } } break; + + default: { + } break; } - // No error, the caller will decide what to do in this case + // Unknown return Variant::NIL; } @@ -299,6 +305,11 @@ MonoObject *variant_to_mono_object(const Variant *p_var, const ManagedType &p_ty if (tclass == CACHED_CLASS(Plane)) RETURN_BOXED_STRUCT(Plane, p_var); + + if (mono_class_is_enum(tclass->get_raw())) { + int val = p_var->operator signed int(); + return BOX_ENUM(tclass->get_raw(), val); + } } break; case MONO_TYPE_ARRAY: @@ -515,6 +526,9 @@ Variant mono_object_to_variant(MonoObject *p_obj, const ManagedType &p_type) { if (tclass == CACHED_CLASS(Plane)) RETURN_UNBOXED_STRUCT(Plane, p_obj); + + if (mono_class_is_enum(tclass->get_raw())) + return unbox(p_obj); } break; case MONO_TYPE_ARRAY: diff --git a/modules/mono/mono_gd/gd_mono_marshal.h b/modules/mono/mono_gd/gd_mono_marshal.h index 38dd22357d..9f403b787f 100644 --- a/modules/mono/mono_gd/gd_mono_marshal.h +++ b/modules/mono/mono_gd/gd_mono_marshal.h @@ -53,6 +53,7 @@ T unbox(MonoObject *p_obj) { #define BOX_UINT8(x) mono_value_box(mono_domain_get(), CACHED_CLASS_RAW(uint8_t), &x) #define BOX_BOOLEAN(x) mono_value_box(mono_domain_get(), CACHED_CLASS_RAW(bool), &x) #define BOX_PTR(x) mono_value_box(mono_domain_get(), CACHED_CLASS_RAW(IntPtr), x) +#define BOX_ENUM(m_enum_class, x) mono_value_box(mono_domain_get(), m_enum_class, &x) Variant::Type managed_to_variant_type(const ManagedType &p_type); diff --git a/modules/mono/mono_gd/gd_mono_utils.cpp b/modules/mono/mono_gd/gd_mono_utils.cpp index d23fcf8a67..53e45002c4 100644 --- a/modules/mono/mono_gd/gd_mono_utils.cpp +++ b/modules/mono/mono_gd/gd_mono_utils.cpp @@ -86,6 +86,7 @@ void MonoCache::clear_members() { class_NodePath = NULL; class_RID = NULL; class_GodotObject = NULL; + class_GodotReference = NULL; class_Node = NULL; class_Control = NULL; class_Spatial = NULL; @@ -95,7 +96,6 @@ void MonoCache::clear_members() { class_ExportAttribute = NULL; field_ExportAttribute_hint = NULL; field_ExportAttribute_hint_string = NULL; - field_ExportAttribute_usage = NULL; class_ToolAttribute = NULL; class_RemoteAttribute = NULL; class_SyncAttribute = NULL; @@ -153,6 +153,7 @@ void update_godot_api_cache() { CACHE_CLASS_AND_CHECK(NodePath, GODOT_API_CLASS(NodePath)); CACHE_CLASS_AND_CHECK(RID, GODOT_API_CLASS(NodePath)); CACHE_CLASS_AND_CHECK(GodotObject, GODOT_API_CLASS(Object)); + CACHE_CLASS_AND_CHECK(GodotReference, GODOT_API_CLASS(Reference)); CACHE_CLASS_AND_CHECK(Node, GODOT_API_CLASS(Node)); CACHE_CLASS_AND_CHECK(Control, GODOT_API_CLASS(Control)); CACHE_CLASS_AND_CHECK(Spatial, GODOT_API_CLASS(Spatial)); @@ -163,7 +164,6 @@ void update_godot_api_cache() { CACHE_CLASS_AND_CHECK(ExportAttribute, GODOT_API_CLASS(ExportAttribute)); CACHE_FIELD_AND_CHECK(ExportAttribute, hint, CACHED_CLASS(ExportAttribute)->get_field("hint")); CACHE_FIELD_AND_CHECK(ExportAttribute, hint_string, CACHED_CLASS(ExportAttribute)->get_field("hint_string")); - CACHE_FIELD_AND_CHECK(ExportAttribute, usage, CACHED_CLASS(ExportAttribute)->get_field("usage")); CACHE_CLASS_AND_CHECK(ToolAttribute, GODOT_API_CLASS(ToolAttribute)); CACHE_CLASS_AND_CHECK(RemoteAttribute, GODOT_API_CLASS(RemoteAttribute)); CACHE_CLASS_AND_CHECK(SyncAttribute, GODOT_API_CLASS(SyncAttribute)); diff --git a/modules/mono/mono_gd/gd_mono_utils.h b/modules/mono/mono_gd/gd_mono_utils.h index bbda3a01d4..e3af57e78a 100644 --- a/modules/mono/mono_gd/gd_mono_utils.h +++ b/modules/mono/mono_gd/gd_mono_utils.h @@ -88,6 +88,7 @@ struct MonoCache { GDMonoClass *class_NodePath; GDMonoClass *class_RID; GDMonoClass *class_GodotObject; + GDMonoClass *class_GodotReference; GDMonoClass *class_Node; GDMonoClass *class_Control; GDMonoClass *class_Spatial; @@ -97,7 +98,6 @@ struct MonoCache { GDMonoClass *class_ExportAttribute; GDMonoField *field_ExportAttribute_hint; GDMonoField *field_ExportAttribute_hint_string; - GDMonoField *field_ExportAttribute_usage; GDMonoClass *class_ToolAttribute; GDMonoClass *class_RemoteAttribute; GDMonoClass *class_SyncAttribute; @@ -164,7 +164,7 @@ String get_exception_name_and_message(MonoObject *p_ex); } // GDMonoUtils -#define NATIVE_GDMONOCLASS_NAME(m_class) (GDMonoMarshal::mono_string_to_godot((MonoString *)m_class->get_field("nativeName")->get_value(NULL))) +#define NATIVE_GDMONOCLASS_NAME(m_class) (GDMonoMarshal::mono_string_to_godot((MonoString *)m_class->get_field(BINDINGS_NATIVE_NAME_FIELD)->get_value(NULL))) #define CACHED_CLASS(m_class) (GDMonoUtils::mono_cache.class_##m_class) #define CACHED_CLASS_RAW(m_class) (GDMonoUtils::mono_cache.class_##m_class->get_raw())