Remove Export impls with surprising behavior

Also added documentation on the rationales behind leaving `Export`
unimplemented for standard Rust collections, pointing to alternatives.

Close #1009

Author: chitoyuu

examples/property-export/GDScriptPrinter.gd

@@ -4,17 +4,17 @@ func _ready():
 	var rust = get_node("../PropertyExport")
 
 	print("\n-----------------------------------------------------------------")
-	print("Print from GDScript (note the lexicographically ordered map/set):")
-	print("  Vec (name):");
+	print("Print from GDScript:")
+	print("  PoolArray<GodotString>:");
 	for name in rust.name_vec:
 		print("  * %s" % name)
 
-	print("\n  HashMap (string -> color):")
+	print("\n  Dictionary (string -> color):")
 	for string in rust.color_map:
 		var color = rust.color_map[string]
 		print("  * %s -> #%s" % [string, color.to_html(false)]);
 
-	print("\n  HashSet (ID):")
+	print("\n  PoolArray<i32>:")
 	for id in rust.id_set:
 		print("  * %s" % id)
 

examples/property-export/Main.tscn

@@ -12,13 +12,13 @@ library = ExtResource( 1 )
 
 [node name="PropertyExport" type="Node" parent="."]
 script = SubResource( 1 )
-name_vec = [ "Godot", "Godette", "Go ." ]
+name_vec = PoolStringArray("Godot", "Godette", "Go .")
 color_map = {
 "blue": Color( 0.184314, 0.160784, 0.8, 1 ),
 "green": Color( 0.0941176, 0.447059, 0.192157, 1 ),
 "teal": Color( 0.0941176, 0.423529, 0.564706, 1 )
 }
-id_set = [ 21, 77, 8, 90 ]
+id_set = PoolIntArray(21, 77, 8, 90)
 
 [node name="GDScriptPrinter" type="Node" parent="."]
 script = ExtResource( 2 )

examples/property-export/src/lib.rs

@@ -1,18 +1,16 @@
 use gdnative::prelude::*;
 
-use std::collections::{HashMap, HashSet};
-
 #[derive(NativeClass, Default)]
 #[inherit(Node)]
 pub struct PropertyExport {
     #[property]
-    name_vec: Vec<String>,
+    name_vec: PoolArray<GodotString>,
 
     #[property]
-    color_map: HashMap<GodotString, Color>,
+    color_map: Dictionary,
 
     #[property]
-    id_set: HashSet<i64>,
+    id_set: PoolArray<i32>,
 }
 
 #[methods]
@@ -24,19 +22,20 @@ impl PropertyExport {
     #[method]
     fn _ready(&self) {
         godot_print!("------------------------------------------------------------------");
-        godot_print!("Print from Rust (note the unordered map/set):");
-        godot_print!("  Vec (name):");
-        for name in &self.name_vec {
+        godot_print!("Print from Rust:");
+        godot_print!("  PoolArray<GodotString>:");
+        for name in &*self.name_vec.read() {
             godot_print!("  * {}", name);
         }
 
-        godot_print!("\n  HashMap (string -> color):");
+        godot_print!("\n  Dictionary (string -> color):");
         for (string, color) in &self.color_map {
+            let color = Color::from_variant(&color).unwrap();
             godot_print!("  * {} -> #{}", string, color.to_html(false));
         }
 
-        godot_print!("\n  HashSet (ID):");
-        for id in &self.id_set {
+        godot_print!("\n  PoolArray<i32>:");
+        for id in &*self.id_set.read() {
             godot_print!("  * {}", id);
         }
     }

gdnative-core/src/export/property.rs

@@ -23,6 +23,38 @@ mod invalid_accessor;
 pub mod hint;
 
 /// Trait for exportable types.
+///
+/// ## Rust collections
+///
+/// `Export` is intentionally unimplemented for standard Rust collections, such as [`Vec`] or
+/// [`HashMap`][std::collections::HashMap]. The reason is that such types exhibit surprising
+/// behavior when used from GDScript, due to how [`ToVariant`]/[`FromVariant`] conversions work
+/// for these types.
+///
+/// Godot has no concept of Rust collections, and cannot operate on them. Whenever a standard
+/// collection is converted to [`Variant`] via [`ToVariant`], what actually happens is that:
+///
+/// - First, a new Godot collection of the corresponding "kind" is allocated.
+/// - Then, the Rust collection is iterated over, and each element is converted and inserted into
+///   the new collection, possibly triggering many more allocations in the process.
+///
+/// With properties, this whole process happens anew *with each access to the property*, which
+/// means that:
+///
+/// - Modifying such properties from the remote debugger, or calling methods on the property
+///   directly from GDScript (e.g. `thing.exported_vec.append("foo")`) do not produce the desired
+///   behavior by the user.
+/// - Seemingly innocuous expressions such as
+///   `thing.exported_vec[0] + thing.exported_vec[1] + thing.exported_vec[2]` can be much more
+///   expensive computationally than what the user would expect.
+///
+/// As such, we do not allow these types to be exported as properties directly as a precaution.
+/// If you wish to export collections to GDScript, consider the following options:
+///
+/// - Exporting a [`Variant`] collection such as [`VariantArray`] or [`Dictionary`] explicitly,
+///   embracing their respective semantics.
+/// - Exporting not a property, but methods that have to be explicitly called, to set clear
+///   expectations that the return value might be expensive to produce.
 pub trait Export: crate::core_types::ToVariant {
     /// A type-specific hint type that is valid for the type being exported.
     ///
@@ -455,8 +487,6 @@ pub struct Property<T> {
 }
 
 mod impl_export {
-    use std::collections::{HashMap, HashSet};
-
     use super::*;
 
     /// Hint type indicating that there are no hints available for the time being.
@@ -615,41 +645,4 @@ mod impl_export {
             hint.unwrap_or_default().export_info()
         }
     }
-
-    impl<K, V> Export for HashMap<K, V>
-    where
-        K: std::hash::Hash + ToVariantEq + ToVariant,
-        V: ToVariant,
-    {
-        type Hint = NoHint;
-
-        #[inline]
-        fn export_info(_hint: Option<Self::Hint>) -> ExportInfo {
-            ExportInfo::new(VariantType::Dictionary)
-        }
-    }
-
-    impl<T> Export for HashSet<T>
-    where
-        T: ToVariant,
-    {
-        type Hint = NoHint;
-
-        #[inline]
-        fn export_info(_hint: Option<Self::Hint>) -> ExportInfo {
-            ExportInfo::new(VariantType::VariantArray)
-        }
-    }
-
-    impl<T> Export for Vec<T>
-    where
-        T: ToVariant,
-    {
-        type Hint = NoHint;
-
-        #[inline]
-        fn export_info(_hint: Option<Self::Hint>) -> ExportInfo {
-            ExportInfo::new(VariantType::VariantArray)
-        }
-    }
 }