diff --git a/gdnative-core/src/core_types/dictionary.rs b/gdnative-core/src/core_types/dictionary.rs index beb64dce2..036855d36 100644 --- a/gdnative-core/src/core_types/dictionary.rs +++ b/gdnative-core/src/core_types/dictionary.rs @@ -69,21 +69,49 @@ impl Dictionary { unsafe { (get_api().godot_dictionary_has_all)(self.sys(), keys.sys()) } } - /// Returns a copy of the value corresponding to the key. + /// Returns a copy of the value corresponding to the key if it exists. #[inline] - pub fn get(&self, key: K) -> Variant + pub fn get(&self, key: K) -> Option where K: ToVariant + ToVariantEq, + { + let key = key.to_variant(); + if self.contains(&key) { + // This should never return the default Nil, but there isn't a safe way to otherwise check + // if the entry exists in a single API call. + Some(self.get_or_nil(key)) + } else { + None + } + } + + /// Returns a copy of the value corresponding to the key, or `default` if it doesn't exist + #[inline] + pub fn get_or(&self, key: K, default: D) -> Variant + where + K: ToVariant + ToVariantEq, + D: ToVariant, { unsafe { - Variant((get_api().godot_dictionary_get)( + Variant((get_api().godot_dictionary_get_with_default)( self.sys(), key.to_variant().sys(), + default.to_variant().sys(), )) } } - /// Update an existing element corresponding ot the key. + /// Returns a copy of the element corresponding to the key, or `Nil` if it doesn't exist. + /// Shorthand for `self.get_or(key, Variant::new())`. + #[inline] + pub fn get_or_nil(&self, key: K) -> Variant + where + K: ToVariant + ToVariantEq, + { + self.get_or(key, Variant::new()) + } + + /// Update an existing element corresponding to the key. /// /// # Panics /// @@ -106,12 +134,14 @@ impl Dictionary { } } - /// Returns a reference to the value corresponding to the key. + /// Returns a reference to the value corresponding to the key, inserting a `Nil` value first if + /// it does not exist. /// /// # Safety /// /// The returned reference is invalidated if the same container is mutated through another - /// reference. + /// reference, and other references may be invalidated if the entry does not already exist + /// (which causes this function to insert `Nil` and thus possibly re-allocate). /// /// `Variant` is reference-counted and thus cheaply cloned. Consider using `get` instead. #[inline] @@ -125,13 +155,16 @@ impl Dictionary { )) } - /// Returns a mutable reference to the value corresponding to the key. + /// Returns a mutable reference to the value corresponding to the key, inserting a `Nil` value + /// first if it does not exist. /// /// # Safety /// /// The returned reference is invalidated if the same container is mutated through another - /// reference. It is possible to create two mutable references to the same memory location - /// if the same `key` is provided, causing undefined behavior. + /// reference, and other references may be invalidated if the `key` does not already exist + /// (which causes this function to insert `Nil` and thus possibly re-allocate). It is also + /// possible to create two mutable references to the same memory location if the same `key` + /// is provided, causing undefined behavior. #[inline] #[allow(clippy::mut_from_ref)] pub unsafe fn get_mut_ref(&self, key: K) -> &mut Variant @@ -425,7 +458,7 @@ unsafe fn iter_next( None } else { let key = Variant::cast_ref(next_ptr).clone(); - let value = dic.get(&key); + let value = dic.get_or_nil(&key); *last_key = Some(key.clone()); Some((key, value)) } @@ -591,7 +624,7 @@ godot_test!(test_dictionary { let mut iter_keys = HashSet::new(); let expected_keys = ["foo", "bar"].iter().map(|&s| s.to_string()).collect::>(); for (key, value) in &dict { - assert_eq!(value, dict.get(&key)); + assert_eq!(Some(value), dict.get(&key)); if !iter_keys.insert(key.to_string()) { panic!("key is already contained in set: {:?}", key); } diff --git a/gdnative-core/src/core_types/variant.rs b/gdnative-core/src/core_types/variant.rs index e6ef9aef0..97919faab 100644 --- a/gdnative-core/src/core_types/variant.rs +++ b/gdnative-core/src/core_types/variant.rs @@ -1753,7 +1753,7 @@ impl FromVariant for Result { match key.as_str() { "Ok" => { - let val = T::from_variant(&dict.get(key_variant)).map_err(|err| { + let val = T::from_variant(&dict.get_or_nil(key_variant)).map_err(|err| { FVE::InvalidEnumVariant { variant: "Ok", error: Box::new(err), @@ -1762,7 +1762,7 @@ impl FromVariant for Result { Ok(Ok(val)) } "Err" => { - let err = E::from_variant(&dict.get(key_variant)).map_err(|err| { + let err = E::from_variant(&dict.get_or_nil(key_variant)).map_err(|err| { FVE::InvalidEnumVariant { variant: "Err", error: Box::new(err), @@ -1912,11 +1912,11 @@ godot_test!( test_variant_result { let variant = Result::::Ok(42_i64).to_variant(); let dict = variant.try_to_dictionary().expect("should be dic"); - assert_eq!(Some(42), dict.get("Ok").try_to_i64()); + assert_eq!(Some(42), dict.get("Ok").and_then(|v| v.try_to_i64())); let variant = Result::<(), i64>::Err(54_i64).to_variant(); let dict = variant.try_to_dictionary().expect("should be dic"); - assert_eq!(Some(54), dict.get("Err").try_to_i64()); + assert_eq!(Some(54), dict.get("Err").and_then(|v| v.try_to_i64())); let variant = Variant::from_bool(true); assert_eq!( diff --git a/gdnative-derive/src/variant/from.rs b/gdnative-derive/src/variant/from.rs index d304556c9..5c8f2fc6a 100644 --- a/gdnative-derive/src/variant/from.rs +++ b/gdnative-derive/src/variant/from.rs @@ -87,7 +87,7 @@ pub(crate) fn expand_from_variant(derive_data: DeriveData) -> Result { - let #var_input_ident_iter = &__dict.get(&__keys.get(0)); + let #var_input_ident_iter = &__dict.get_or_nil(&__keys.get(0)); (#var_from_variants).map_err(|err| FVE::InvalidEnumVariant { variant: #ref_var_ident_string_literals, error: Box::new(err), diff --git a/gdnative-derive/src/variant/repr.rs b/gdnative-derive/src/variant/repr.rs index 4dab75670..f2c91175e 100644 --- a/gdnative-derive/src/variant/repr.rs +++ b/gdnative-derive/src/variant/repr.rs @@ -280,7 +280,7 @@ impl VariantRepr { let name_string_literals = name_strings.iter().map(|string| Literal::string(&string)); - let expr_variant = "e!(&__dict.get(&__key)); + let expr_variant = "e!(&__dict.get_or_nil(&__key)); let exprs = non_skipped_fields .iter() .map(|f| f.from_variant(expr_variant)); diff --git a/test/src/test_derive.rs b/test/src/test_derive.rs index 6ddeb8718..74c314a3b 100644 --- a/test/src/test_derive.rs +++ b/test/src/test_derive.rs @@ -83,19 +83,25 @@ fn test_derive_to_variant() -> bool { let variant = data.to_variant(); let dictionary = variant.try_to_dictionary().expect("should be dictionary"); - assert_eq!(Some(42), dictionary.get("foo").try_to_i64()); - assert_eq!(Some(54.0), dictionary.get("bar").try_to_f64()); + assert_eq!(Some(42), dictionary.get("foo").and_then(|v| v.try_to_i64())); + assert_eq!( + Some(54.0), + dictionary.get("bar").and_then(|v| v.try_to_f64()) + ); assert_eq!( Some("*mut ()".into()), - dictionary.get("ptr").try_to_string() + dictionary.get("ptr").and_then(|v| v.try_to_string()) ); assert!(!dictionary.contains("skipped")); let enum_dict = dictionary .get("baz") - .try_to_dictionary() + .and_then(|v| v.try_to_dictionary()) .expect("should be dictionary"); - assert_eq!(Some(true), enum_dict.get("Foo").try_to_bool()); + assert_eq!( + Some(true), + enum_dict.get("Foo").and_then(|v| v.try_to_bool()) + ); assert_eq!( Ok(ToVar:: { @@ -146,7 +152,7 @@ fn test_derive_owned_to_variant() -> bool { let dictionary = variant.try_to_dictionary().expect("should be dictionary"); let array = dictionary .get("arr") - .try_to_array() + .and_then(|v| v.try_to_array()) .expect("should be array"); assert_eq!(3, array.len()); assert_eq!(