Include suggestions from chitoyuu

Author: lilizoey

godot-core/src/builtin/dictionary.rs

@@ -51,8 +51,8 @@ impl Dictionary {
         self.as_inner().duplicate(true)
     }
 
-    /// Returns a shallow copy of the dictionary. All dictionary keys and values are copied, but 
-    /// any reference types (such as `Array`, `Dictionary` and `Object`) will still refer to the 
+    /// Returns a shallow copy of the dictionary. All dictionary keys and values are copied, but
+    /// any reference types (such as `Array`, `Dictionary` and `Object`) will still refer to the
     /// same value.
     ///
     /// To create a deep copy, use [`duplicate_deep()`] instead. To create a new reference to the
@@ -65,7 +65,7 @@ impl Dictionary {
 
     /// Removes a key from the map, and returns the value associated with
     /// the key if the key was in the dictionary.
-    pub fn remove<K: ToVariant>(&mut self, key: K) -> Option<Variant> {
+    pub fn remove(&mut self, key: impl ToVariant) -> Option<Variant> {
         let key = key.to_variant();
         let old_value = self.get(key.clone());
         self.as_inner().erase(key);
@@ -76,8 +76,8 @@ impl Dictionary {
     ///
     /// Unlike in godot, this will return `None` if the key does not exist
     /// and `Some(nil)` the key is `null`.
-    pub fn find_key_by_value(&self, value: Variant) -> Option<Variant> {
-        let key = self.as_inner().find_key(value);
+    pub fn find_key_by_value(&self, value: impl ToVariant) -> Option<Variant> {
+        let key = self.as_inner().find_key(value.to_variant());
 
         if !key.is_nil() || self.contains_key(key.clone()) {
             Some(key)
@@ -91,7 +91,7 @@ impl Dictionary {
     ///
     /// Unlike `get` in godot, this will return `None` if there is
     /// no value with the given key.
-    pub fn get<K: ToVariant>(&self, key: K) -> Option<Variant> {
+    pub fn get(&self, key: impl ToVariant) -> Option<Variant> {
         let key = key.to_variant();
         if !self.contains_key(key.clone()) {
             return None;
@@ -100,19 +100,19 @@ impl Dictionary {
         Some(self.get_or_nil(key))
     }
 
-    /// Returns the value at the key in the dictionary, or nil otherwise. This 
-    /// method does not let you differentiate `NIL` values stored as values from 
+    /// Returns the value at the key in the dictionary, or nil otherwise. This
+    /// method does not let you differentiate `NIL` values stored as values from
     /// absent keys. If you need that, use `get()`.
     ///
     /// This is equivalent to `get` in godot.
-    pub fn get_or_nil<K: ToVariant>(&self, key: K) -> Variant {
+    pub fn get_or_nil(&self, key: impl ToVariant) -> Variant {
         self.as_inner().get(key.to_variant(), Variant::nil())
     }
 
     /// Returns `true` if the dictionary contains the given key.
     ///
     /// This is equivalent to `has` in godot.
-    pub fn contains_key<K: ToVariant>(&self, key: K) -> bool {
+    pub fn contains_key(&self, key: impl ToVariant) -> bool {
         let key = key.to_variant();
         self.as_inner().has(key)
     }
@@ -164,7 +164,7 @@ impl Dictionary {
     /// Get the pointer corresponding to the given key in the dictionary,
     /// this pointer is null if there is no value at the given key.
     #[allow(dead_code)] // TODO: remove function if it turns out i'll never actually get any use out of it
-    fn get_ptr<K: ToVariant>(&self, key: K) -> *const Variant {
+    fn get_ptr(&self, key: impl ToVariant) -> *const Variant {
         let key = key.to_variant();
         unsafe {
             (interface_fn!(dictionary_operator_index_const))(self.sys_const(), key.var_sys_const())
@@ -175,7 +175,7 @@ impl Dictionary {
     /// Get the pointer corresponding to the given key in the dictionary,
     /// if there exists no value at the given key then a new one is created
     /// and initialized to a nil variant.
-    fn get_ptr_mut<K: ToVariant>(&mut self, key: K) -> *mut Variant {
+    fn get_ptr_mut(&mut self, key: impl ToVariant) -> *mut Variant {
         let key = key.to_variant();
         unsafe {
             let ptr =
@@ -190,41 +190,21 @@ impl Dictionary {
 
     /// Insert a value at the given key, returning the value
     /// that previously was at that key if there was one.
-    pub fn insert<K: ToVariant>(&mut self, key: K, value: Variant) -> Option<Variant> {
+    pub fn insert(&mut self, key: impl ToVariant, value: impl ToVariant) -> Option<Variant> {
         let key = key.to_variant();
         let old_value = self.get(key.clone());
         self.set(key, value);
         old_value
     }
 
     /// Set a key to a given value
-    pub fn set<K: ToVariant>(&mut self, key: K, value: Variant) {
+    pub fn set(&mut self, key: impl ToVariant, value: impl ToVariant) {
         let key = key.to_variant();
         unsafe {
-            *self.get_ptr_mut(key) = value;
+            *self.get_ptr_mut(key) = value.to_variant();
         }
     }
 
-    /// Convert this dictionary to a strongly typed rust `HashMap`. If the conversion
-    /// fails for any key or value, an error is returned.
-    pub fn try_to_hashmap<K: FromVariant + Eq + std::hash::Hash, V: FromVariant>(
-        &self,
-    ) -> Result<HashMap<K, V>, VariantConversionError> {
-        let mut map = HashMap::new();
-        for key in self.keys().into_iter() {
-            map.insert(key.try_to()?, self.get(key).unwrap().try_to()?);
-        }
-        Ok(map)
-    }
-
-    /// Convert the keys of this dictionary to a strongly typed rust `HashSet`. If the
-    /// conversion fails for any key, an error is returned.
-    pub fn try_to_hashset<K: FromVariant + Eq + std::hash::Hash>(
-        &self,
-    ) -> Result<HashSet<K>, VariantConversionError> {
-        Ok(self.keys().try_to_vec::<K>()?.into_iter().collect())
-    }
-
     #[doc(hidden)]
     pub fn as_inner(&self) -> inner::InnerDictionary {
         inner::InnerDictionary::from_outer(self)
@@ -236,7 +216,7 @@ impl Dictionary {
 impl<K: ToVariant, V: ToVariant> From<&HashMap<K, V>> for Dictionary {
     fn from(map: &HashMap<K, V>) -> Self {
         let mut dict = Dictionary::new();
-        for (k, v) in map.into_iter() {
+        for (k, v) in map.iter() {
             dict.insert(k.to_variant(), v.to_variant());
         }
         dict
@@ -245,16 +225,53 @@ impl<K: ToVariant, V: ToVariant> From<&HashMap<K, V>> for Dictionary {
 
 /// Creates a `Dictionary` from the given `HashSet`. Each key is converted
 /// to a `Variant`, and the values are all `true`.
+///
+///
 impl<K: ToVariant> From<&HashSet<K>> for Dictionary {
     fn from(set: &HashSet<K>) -> Self {
         let mut dict = Dictionary::new();
-        for k in set.into_iter() {
+        for k in set.iter() {
             dict.insert(k.to_variant(), true.to_variant());
         }
         dict
     }
 }
 
+/// Convert this dictionary to a strongly typed rust `HashMap`. If the conversion
+/// fails for any key or value, an error is returned.
+///
+/// Will be replaced by a proper iteration implementation.
+impl<K: FromVariant + Eq + std::hash::Hash, V: FromVariant> TryFrom<&Dictionary> for HashMap<K, V> {
+    type Error = VariantConversionError;
+
+    fn try_from(dictionary: &Dictionary) -> Result<Self, Self::Error> {
+        // TODO: try to panic or something if modified while iterating
+        // Though probably better to fix when implementing iteration proper
+        dictionary
+            .keys()
+            .iter()
+            .zip(dictionary.values().iter())
+            .map(|(key, value)| Ok((K::try_from_variant(&key)?, V::try_from_variant(&value)?)))
+            .collect()
+    }
+}
+
+/// Convert the keys of this dictionary to a strongly typed rust `HashSet`. If the
+/// conversion fails for any key, an error is returned.
+impl<K: FromVariant + Eq + std::hash::Hash> TryFrom<&Dictionary> for HashSet<K> {
+    type Error = VariantConversionError;
+
+    fn try_from(dictionary: &Dictionary) -> Result<Self, Self::Error> {
+        // TODO: try to panic or something if modified while iterating
+        // Though probably better to fix when implementing iteration proper
+        dictionary
+            .keys()
+            .iter()
+            .map(|key| K::try_from_variant(&key))
+            .collect()
+    }
+}
+
 /// Inserts all key-values from the iterator into the dictionary,
 /// replacing values with existing keys with new values returned
 /// from the iterator.
@@ -335,7 +352,7 @@ macro_rules! dict {
         {
             let mut d = ::godot::builtin::Dictionary::new();
             $(
-                d.set($key, ::godot::builtin::Variant::from($value));
+                d.set($key, $value);
             )*
             d
         }