From 5535ff352b7f6db711317d2928b070c6443b3a12 Mon Sep 17 00:00:00 2001
From: Jeff Ward <jeff@fuzzybinary.com>
Date: Tue, 29 Oct 2024 22:38:56 -0400
Subject: [PATCH] feat: Add correct default hint for Nodes and Resource
 properties.

The generator now checks for Node and Resource subclasses and adds the correct property hint and hintString.

This also fixes an issue where property hints weren't correctly translated from Dart to C.
---
 src/cpp/gde_dart_converters.cpp               |  10 +-
 src/cpp/script/dart_script.cpp                | 108 +++++++++++-------
 src/cpp/script/dart_script.h                  |  10 +-
 src/cpp/script/dart_script_instance.cpp       |  73 +++---------
 src/dart/godot_dart_build/CHANGELOG.md        |   4 +
 .../lib/src/godot_script_generator.dart       |  22 ++++
 src/dart/godot_dart_build/pubspec.yaml        |   6 +-
 7 files changed, 126 insertions(+), 107 deletions(-)

diff --git a/src/cpp/gde_dart_converters.cpp b/src/cpp/gde_dart_converters.cpp
index 3cd3af5..86f2eae 100644
--- a/src/cpp/gde_dart_converters.cpp
+++ b/src/cpp/gde_dart_converters.cpp
@@ -2,8 +2,8 @@
 
 #include <dart_api.h>
 
-#include "dart_helpers.h"
 #include "dart_bindings.h"
+#include "dart_helpers.h"
 #include "gde_wrapper.h"
 #include "godot_string_wrappers.h"
 
@@ -25,7 +25,7 @@ void *get_object_address(Dart_Handle engine_handle) {
 }
 
 void gde_method_info_from_dart(Dart_Handle dart_method_info, GDExtensionMethodInfo *method_info) {
-  DART_CHECK(dart_name, Dart_GetField(dart_method_info, Dart_NewStringFromCString("name")), "Failed to get name");  
+  DART_CHECK(dart_name, Dart_GetField(dart_method_info, Dart_NewStringFromCString("name")), "Failed to get name");
   method_info->name = create_godot_string_name_ptr(dart_name);
   // TODO: id?
   method_info->id = 0;
@@ -61,7 +61,7 @@ void gde_method_info_from_dart(Dart_Handle dart_method_info, GDExtensionMethodIn
 
 void gde_free_method_info_fields(GDExtensionMethodInfo *method_info) {
   if (method_info->name != nullptr) {
-    delete reinterpret_cast<godot::StringName*>(method_info->name);
+    delete reinterpret_cast<godot::StringName *>(method_info->name);
   }
 
   gde_free_property_info_fields(&method_info->return_value);
@@ -89,8 +89,10 @@ void gde_property_info_from_dart(Dart_Handle dart_property_info, GDExtensionProp
 
   DART_CHECK(dart_property_hint, Dart_GetField(dart_property_info, Dart_NewStringFromCString("hint")),
              "Failed to get hint");
+  DART_CHECK(dart_hint_value, Dart_GetField(dart_property_hint, Dart_NewStringFromCString("value")),
+             "Failed to get PropertyHint.value");
   uint64_t hint = 0;
-  Dart_IntegerToUint64(dart_property_hint, &hint);
+  Dart_IntegerToUint64(dart_hint_value, &hint);
   prop_info->hint = uint32_t(hint);
 
   DART_CHECK(dart_hint_string, Dart_GetField(dart_property_info, Dart_NewStringFromCString("hintString")),
diff --git a/src/cpp/script/dart_script.cpp b/src/cpp/script/dart_script.cpp
index 8f4d77f..5a6400b 100644
--- a/src/cpp/script/dart_script.cpp
+++ b/src/cpp/script/dart_script.cpp
@@ -1,16 +1,16 @@
 #include "dart_script.h"
 
-#include <godot_cpp/classes/engine.hpp>
-#include <godot_cpp/classes/file_access.hpp>
 #include <godot_cpp/classes/editor_file_system.hpp>
 #include <godot_cpp/classes/editor_interface.hpp>
+#include <godot_cpp/classes/engine.hpp>
+#include <godot_cpp/classes/file_access.hpp>
 
 #include "../dart_bindings.h"
 
 #include "../dart_helpers.h"
 #include "../godot_string_wrappers.h"
-#include "script/dart_script_language.h"
 #include "script/dart_script_instance.h"
+#include "script/dart_script_language.h"
 
 using namespace godot;
 
@@ -18,6 +18,8 @@ DartScript::DartScript() : _source_code(), _needs_refresh(false), _dart_type(nul
 }
 
 DartScript::~DartScript() {
+  clear_property_cache();
+
   GodotDartBindings *bindings = GodotDartBindings::instance();
   if (bindings == nullptr) {
     return;
@@ -39,7 +41,7 @@ void DartScript::_bind_methods() {
 }
 
 godot::Ref<Script> DartScript::_get_base_script() const {
-  refresh_type();
+  const_cast<DartScript *>(this)->refresh_type();
 
   return _base_script;
 }
@@ -71,7 +73,7 @@ bool DartScript::_can_instantiate() const {
     return default_return;                                                                                             \
   }                                                                                                                    \
                                                                                                                        \
-  refresh_type();                                                                                                      \
+  const_cast<DartScript *>(this)->refresh_type();                                                                      \
   if (_script_info == nullptr) {                                                                                       \
     return default_return;                                                                                             \
   }
@@ -213,27 +215,20 @@ godot::TypedArray<godot::Dictionary> DartScript::_get_script_property_list() con
 
   godot::TypedArray<godot::Dictionary> ret_val;
 
-  bindings->execute_on_dart_thread([&] {
-    DartBlockScope scope;
-
-    Dart_Handle script_info = Dart_HandleFromPersistent(_script_info);
-
-    Dart_Handle dart_prop_name = Dart_NewStringFromCString("properties");
-    DART_CHECK(dart_property_list, Dart_GetField(script_info, dart_prop_name), "Error getting field properties");
-
-    intptr_t property_count = 0;
-    Dart_ListLength(dart_property_list, &property_count);
-    for (intptr_t i = 0; i < property_count; ++i) {
-      Dart_Handle property_info = Dart_ListGetAt(dart_property_list, i);
-
-      Dart_Handle d_as_dict = Dart_NewStringFromCString("asDict");
-      DART_CHECK(dart_godot_dict, Dart_Invoke(property_info, d_as_dict, 0, nullptr), "Error calling asDict");
-
-      void *dict_pointer = get_object_address(dart_godot_dict);
-      godot::Dictionary propety_dict(*((godot::Dictionary *)dict_pointer));
-      ret_val.append(propety_dict);
-    }
-  });
+  // TODO: Look if there's a better way to store these strings as constants
+  for (const auto &prop : _properties_cache) {
+    const auto &prop_info = prop.second;
+    godot::Dictionary info_dict;
+    info_dict[godot::Variant(godot::String("type"))] = Variant(prop_info.type);
+    info_dict[godot::Variant(godot::String("name"))] = Variant(*reinterpret_cast<godot::String *>(prop_info.name));
+    info_dict[godot::Variant(godot::String("class_name"))] =
+        Variant(*reinterpret_cast<godot::StringName *>(prop_info.name));
+    info_dict[godot::Variant(godot::String("hint"))] = Variant(prop_info.hint);
+    info_dict[godot::Variant(godot::String("hint_string"))] =
+        Variant(*reinterpret_cast<godot::String *>(prop_info.hint_string));
+    info_dict[godot::Variant(godot::String("usage"))] = Variant(prop_info.usage);
+    ret_val.push_back(info_dict);
+  }
 
   return ret_val;
 }
@@ -290,6 +285,8 @@ godot::Variant DartScript::_get_property_default_value(const godot::StringName &
 }
 
 void DartScript::_update_exports() {
+  refresh_type();
+  
   for (const auto &script_instance : _placeholders) {
     script_instance->notify_property_list_changed();
   }
@@ -319,12 +316,12 @@ godot::StringName DartScript::_get_global_name() const {
   return ret;
 }
 
-void DartScript::load_from_disk(const godot::String &path) {  
+void DartScript::load_from_disk(const godot::String &path) {
   Ref<FileAccess> file = FileAccess::open(path, FileAccess::READ);
   if (!file.is_null()) {
     String text = file->get_as_text();
     set_source_code(text);
-    file->close();   
+    file->close();
     _path = path;
   }
 }
@@ -332,7 +329,7 @@ void DartScript::load_from_disk(const godot::String &path) {
 void DartScript::did_hot_reload() {
   _update_exports();
   auto editor_interface = godot::EditorInterface::get_singleton();
-  if (editor_interface) {    
+  if (editor_interface) {
     editor_interface->get_resource_filesystem()->update_file(_path);
   }
 }
@@ -347,7 +344,7 @@ void *DartScript::_instance_create(Object *for_object) const {
     return nullptr;
   }
 
-  refresh_type();
+  const_cast<DartScript *>(this)->refresh_type();
   if (_dart_type == nullptr) {
     return nullptr;
   }
@@ -365,7 +362,7 @@ void *DartScript::_placeholder_instance_create(Object *for_object) const {
     return nullptr;
   }
 
-  refresh_type();
+  const_cast<DartScript *>(this)->refresh_type();
   if (_dart_type == nullptr) {
     return nullptr;
   }
@@ -373,32 +370,32 @@ void *DartScript::_placeholder_instance_create(Object *for_object) const {
   return create_script_instance_internal(for_object, true);
 }
 
-void *DartScript::create_script_instance_internal(Object* for_object, bool is_placeholder) const {
+void *DartScript::create_script_instance_internal(Object *for_object, bool is_placeholder) const {
   GodotDartBindings *bindings = GodotDartBindings::instance();
   GDExtensionScriptInstancePtr godot_script_instance = nullptr;
 
   RefCounted *rc = Object::cast_to<RefCounted>(for_object);
-  
+
   bindings->execute_on_dart_thread([&] {
     DartBlockScope scope;
-  
+
     Dart_Handle dart_pointer = bindings->new_dart_void_pointer(for_object->_owner);
     Dart_Handle args[1] = {dart_pointer};
     DART_CHECK(dart_object, Dart_New(_dart_type, Dart_NewStringFromCString("withNonNullOwner"), 1, args),
-                "Error creating bindings");
-  
-    DartScriptInstance *script_instance = new DartScriptInstance(dart_object, const_cast<DartScript *>(this),
-                                                                  for_object, is_placeholder, rc != nullptr);
-  
+               "Error creating bindings");
+
+    DartScriptInstance *script_instance =
+        new DartScriptInstance(dart_object, const_cast<DartScript *>(this), for_object, is_placeholder, rc != nullptr);
+
     godot_script_instance =
         gde_script_instance_create2(DartScriptInstance::get_script_instance_info(),
                                     reinterpret_cast<GDExtensionScriptInstanceDataPtr>(script_instance));
-    
+
     if (is_placeholder) {
       _placeholders.insert(script_instance);
     }
   });
-  
+
   return godot_script_instance;
 }
 
@@ -406,7 +403,14 @@ void DartScript::dart_placeholder_erased(DartScriptInstance *p_placeholder) {
   _placeholders.erase(p_placeholder);
 }
 
-void DartScript::refresh_type() const {
+void DartScript::clear_property_cache() {
+  for (auto &prop : _properties_cache) {
+    gde_free_property_info_fields(&prop.second);
+  }
+  _properties_cache.clear();
+}
+
+void DartScript::refresh_type() {
   GodotDartBindings *bindings = GodotDartBindings::instance();
   if (bindings == nullptr) {
     return;
@@ -444,12 +448,30 @@ void DartScript::refresh_type() const {
                  "Failed to get scriptInfo");
       if (script_info != nullptr) {
         _script_info = Dart_NewPersistentHandle(script_info);
-
+        
         // Find the base type
         Dart_Handle base_type = Dart_GetField(type_info, Dart_NewStringFromCString("parentType"));
         if (Dart_IsNull(base_type)) {
           _base_script = language->find_script_for_type(script_info);
         }
+
+        clear_property_cache();
+
+        // TODO: Get properties from our base class?
+        DART_CHECK(properties_list, Dart_GetField(script_info, Dart_NewStringFromCString("properties")),
+                   "Failed to get properties info");
+        intptr_t prop_count = 0;
+        Dart_ListLength(properties_list, &prop_count);
+
+        if (prop_count > 0) {
+          for (auto i = 0; i < prop_count; ++i) {
+            DART_CHECK(dart_property, Dart_ListGetAt(properties_list, i), "Failed to get property at index");
+            GDExtensionPropertyInfo property_info;
+            gde_property_info_from_dart(dart_property, &property_info);
+            godot::StringName *prop_name = reinterpret_cast<godot::StringName *>(property_info.name);
+            _properties_cache[*prop_name] = property_info;
+          }
+        }
       }
     }
   });
diff --git a/src/cpp/script/dart_script.h b/src/cpp/script/dart_script.h
index 56dca37..23b7fb3 100644
--- a/src/cpp/script/dart_script.h
+++ b/src/cpp/script/dart_script.h
@@ -47,22 +47,26 @@ class DartScript : public godot::ScriptExtension {
 
   void *_instance_create(Object *for_object) const override;
   void *_placeholder_instance_create(Object *for_object) const override;
+
+  const std::unordered_map<godot::StringName, GDExtensionPropertyInfo>& get_properties() const {
+    return _properties_cache;
+  }
   
   // ScriptInstances in extensions are never of the type that calls
   // _placeholder_erased, so we handle this manually on instance free
   void dart_placeholder_erased(DartScriptInstance *p_placeholder);
 	
-
 protected:
   static void _bind_methods();
 
 private:
-  // Not actually const
-  void refresh_type() const;
+  void refresh_type();
+  void clear_property_cache();
   void *create_script_instance_internal(Object *for_object, bool is_placeholder) const;
 
   godot::String _source_code;
   godot::String _path;
+  std::unordered_map<godot::StringName, GDExtensionPropertyInfo> _properties_cache;
   mutable std::unordered_set<DartScriptInstance *> _placeholders;
   mutable bool _needs_refresh;
   mutable godot::Ref<DartScript> _base_script;
diff --git a/src/cpp/script/dart_script_instance.cpp b/src/cpp/script/dart_script_instance.cpp
index 8fd404f..df5c09c 100644
--- a/src/cpp/script/dart_script_instance.cpp
+++ b/src/cpp/script/dart_script_instance.cpp
@@ -117,70 +117,35 @@ bool DartScriptInstance::get_class_category(GDExtensionPropertyInfo *p_class_cat
 }
 
 const GDExtensionPropertyInfo *DartScriptInstance::get_property_list(uint32_t *r_count) {
-  GodotDartBindings *gde = GodotDartBindings::instance();
-  if (gde == nullptr) {
-    return nullptr;
-  }
-
-  GDExtensionPropertyInfo *prop_list = nullptr;
-  gde->execute_on_dart_thread([&] {
-    DartBlockScope scope;
-
-    // This is a lot of work just to get the size of the list
-    DART_CHECK(object, _binding.get_dart_object(), "Failed to get instance from persistent handle");
-    DART_CHECK(obj_type_info, Dart_GetField(object, Dart_NewStringFromCString("typeInfo")), "Failed to find typeInfo");
-    DART_CHECK(dart_script_info, Dart_GetField(obj_type_info, Dart_NewStringFromCString("scriptInfo")),
-               "Failed to get scirpt info");
-    DART_CHECK(properties_list, Dart_GetField(dart_script_info, Dart_NewStringFromCString("properties")),
-               "Failed to get properties info");
-    intptr_t prop_count = 0;
-    Dart_ListLength(properties_list, &prop_count);
-    *r_count = prop_count;
-
-    if (prop_count > 0) {
-      prop_list = new GDExtensionPropertyInfo[prop_count];
-      for (auto i = 0; i < prop_count; ++i) {
-        DART_CHECK(dart_property, Dart_ListGetAt(properties_list, i), "Failed to get property at index");
-        gde_property_info_from_dart(dart_property, &prop_list[i]);
-      }
+  GDExtensionPropertyInfo *prop_list{nullptr};
+  const auto &prop_map = _dart_script->get_properties();
+  size_t prop_count = prop_map.size();
+  *r_count = prop_count;
+  if (prop_count > 0) {
+    prop_list = new GDExtensionPropertyInfo[prop_count];
+    size_t index = 0;
+    for (const auto &property : prop_map) {
+      prop_list[index] = property.second;
+      ++index;
     }
-  });
+  }
 
   return prop_list;
 }
 
 void DartScriptInstance::free_property_list(const GDExtensionPropertyInfo *p_list) {
-  GodotDartBindings *gde = GodotDartBindings::instance();
-  if (gde == nullptr) {
-    return;
-  }
-
-  gde->execute_on_dart_thread([&] {
-    DartBlockScope scope;
-
-    // This is a lot of work just to get the size of the list
-    DART_CHECK(object, _binding.get_dart_object(), "Failed to get instance from persistent handle");
-    DART_CHECK(obj_type_info, Dart_GetField(object, Dart_NewStringFromCString("typeInfo")), "Failed to find typeInfo");
-    DART_CHECK(dart_script_info, Dart_GetField(obj_type_info, Dart_NewStringFromCString("scriptInfo")),
-               "Failed to get scirpt info");
-    DART_CHECK(properties_list, Dart_GetField(dart_script_info, Dart_NewStringFromCString("properties")),
-               "Failed to get properties info");
-    intptr_t prop_count = 0;
-    Dart_ListLength(properties_list, &prop_count);
-
-    if (p_list != nullptr) {
-      for (intptr_t i = 0; i < prop_count; ++i) {
-        gde_free_property_info_fields(const_cast<GDExtensionPropertyInfo *>(&p_list[i]));
-      }
-
-      delete[] p_list;
-    }
-  });
+  delete[] p_list;
 }
 
 GDExtensionVariantType DartScriptInstance::get_property_type(const godot::StringName &p_name,
                                                              GDExtensionBool *r_is_valid) {
-  return GDExtensionVariantType();
+  const auto &properties = _dart_script->get_properties();
+  const auto &prop_itr = properties.find(p_name);
+  *r_is_valid = prop_itr != properties.end();
+  if (r_is_valid) {
+    return prop_itr->second.type;
+  }
+  return GDExtensionVariantType{};
 }
 
 bool DartScriptInstance::validate_property(GDExtensionPropertyInfo *p_property) {
diff --git a/src/dart/godot_dart_build/CHANGELOG.md b/src/dart/godot_dart_build/CHANGELOG.md
index 1a0e45a..edb8b25 100644
--- a/src/dart/godot_dart_build/CHANGELOG.md
+++ b/src/dart/godot_dart_build/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.4.0
+
+- Add correct default property hints to Node and Resource types.
+
 ## 0.3.2
 
 - Fix a different version constraint issue.
diff --git a/src/dart/godot_dart_build/lib/src/godot_script_generator.dart b/src/dart/godot_dart_build/lib/src/godot_script_generator.dart
index 8ca0516..7178e6b 100644
--- a/src/dart/godot_dart_build/lib/src/godot_script_generator.dart
+++ b/src/dart/godot_dart_build/lib/src/godot_script_generator.dart
@@ -180,11 +180,33 @@ class GodotScriptAnnotationGenerator
     buffer.writeln('PropertyInfo(');
     buffer.writeln('  name: \'$exportName\',');
     buffer.writeln('  typeInfo: ${_typeInfoForType(field.type)},');
+
+    final propertyHint = _getPropertyHint(field.type);
+    if (propertyHint != null) {
+      buffer.writeln('  hint: ${propertyHint.toString()},');
+      buffer.writeln(
+          '  hintString: \'${field.type.getDisplayString(withNullability: false)}\',');
+    }
+
     buffer.write(')');
 
     return buffer.toString();
   }
 
+  PropertyHint? _getPropertyHint(DartType type) {
+    final element = type.element;
+    if (element is ClassElement) {
+      for (final supertype in element.allSupertypes) {
+        if (supertype.element.name == 'Node') {
+          return PropertyHint.nodeType;
+        } else if (supertype.element.name == 'Resource') {
+          return PropertyHint.resourceType;
+        }
+      }
+    }
+    return null;
+  }
+
   String _typeInfoForType(DartType type) {
     bool isPrimitive(DartType type) {
       return type.isDartCoreBool ||
diff --git a/src/dart/godot_dart_build/pubspec.yaml b/src/dart/godot_dart_build/pubspec.yaml
index 4fb4fdf..85bc0a7 100644
--- a/src/dart/godot_dart_build/pubspec.yaml
+++ b/src/dart/godot_dart_build/pubspec.yaml
@@ -1,10 +1,10 @@
 name: godot_dart_build
 description: build_runner interface for simplifying binding to Godot
 repository: https://github.com/fuzzybinary/godot_dart
-version: 0.3.2
+version: 0.4.0
 
 environment:
-  sdk: '>=3.2.0 <4.0.0'
+  sdk: '>=3.5.0 <4.0.0'
 
 dependencies:
   analyzer: ^5.13.0
@@ -14,7 +14,7 @@ dependencies:
   build: ^2.0.0
   build_config: ^1.1.1
   source_gen: ^1.3.2
-  godot_dart: '>=0.4.0<1.0.0'
+  godot_dart: '>=0.6.0<1.0.0'
   code_builder: ^4.5.0
   dart_style: ^2.3.1
   glob: ^2.1.2