Merge #819

819: Re-organize Variant conversion methods r=Bromeon a=chitoyuu

Inherent conversion methods are now genericized into `new`, `to`, `try_to`, and `coerce_to`, to reduce the clutter and make other `Variant` methods more discoverable in docs.

- `new` replaces the `from_*` methods. The original version that returns nil variants is renamed to `Variant::nil`, to better reflect its behavior.
- `to` and `try_to` replaces the `try_to_*` methods, with naming more consistent with the rest of the Rust ecosystem.
- `to_object` and `try_to_object` are kept for convenience around `Ref`.
- `coerce_to` replaces the `to_*` methods. A new trait `CoerceFromVariant` is introduced and implemented for all GDScript built-in types.
- Documentation is updated around convertion methods/traits to highlight what is used for exported methods.

Close #774

Rare PR with negative LoC


Co-authored-by: Chitose Yuuzaki <[email protected]>

Author: bors[bot]

bindings_generator/src/methods.rs

@@ -628,11 +628,6 @@ mod varargs_call {
                             let #name: Variant = Variant::from_object_ptr(#name);
                         }
                     }
-                    Ty::String => {
-                        quote! {
-                            let #name: Variant = Variant::from_godot_string(&#name);
-                        }
-                    }
                     _ => {
                         quote! {
                            let #name: Variant = (&#name).to_variant();
@@ -706,11 +701,6 @@ mod varcall {
                         let #name: Variant = Variant::from_object_ptr(#name);
                     }
                 }
-                Ty::String => {
-                    quote! {
-                        let #name: Variant = Variant::from_godot_string(&#name);
-                    }
-                }
                 _ => {
                     quote! {
                        let #name: Variant = (&#name).to_variant();

examples/rpc/src/client.rs

@@ -46,7 +46,7 @@ impl ServerPuppet {
 
     #[export]
     fn on_connected_to_server(&mut self, owner: TRef<Node>) {
-        owner.rpc("greet_server", &[Variant::from_str("hello")]);
+        owner.rpc("greet_server", &[Variant::new("hello")]);
     }
 
     #[export(rpc = "puppet")]

examples/rpc/src/server.rs

@@ -38,7 +38,7 @@ impl Server {
         owner.rpc_id(
             tree.get_rpc_sender_id(),
             "return_greeting",
-            &[Variant::from_str("hello")],
+            &[Variant::new("hello")],
         );
     }
 }

examples/scene_create/src/lib.rs

@@ -140,13 +140,9 @@ fn update_panel(owner: &Spatial, num_children: i64) {
         let panel_node = unsafe { panel_node.assume_safe() };
 
         // Put the Node
-        let mut as_variant = Variant::from_object(panel_node);
-        let result = unsafe {
-            as_variant.call(
-                "set_num_children",
-                &[Variant::from_u64(num_children as u64)],
-            )
-        };
+        let mut as_variant = Variant::new(panel_node);
+        let result =
+            unsafe { as_variant.call("set_num_children", &[Variant::new(num_children as u64)]) };
 
         match result {
             Ok(_) => godot_print!("Called Panel OK."),

examples/signals/src/lib.rs

@@ -22,7 +22,7 @@ impl SignalEmitter {
             // Argument list used by the editor for GUI and generation of GDScript handlers. It can be omitted if the signal is only used from code.
             args: &[SignalArgument {
                 name: "data",
-                default: Variant::from_i64(100),
+                default: Variant::new(100),
                 export_info: ExportInfo::new(VariantType::I64),
                 usage: PropertyUsage::DEFAULT,
             }],
@@ -48,7 +48,7 @@ impl SignalEmitter {
         if self.data % 2 == 0 {
             owner.emit_signal("tick", &[]);
         } else {
-            owner.emit_signal("tick_with_data", &[Variant::from_i64(self.data)]);
+            owner.emit_signal("tick_with_data", &[Variant::new(self.data)]);
         }
     }
 }
@@ -96,7 +96,7 @@ impl SignalSubscriber {
     fn notify_with_data(&mut self, owner: &Label, data: Variant) {
         let msg = format!(
             "Received signal \"tick_with_data\" with data {}",
-            data.try_to_u64().unwrap()
+            data.try_to::<u64>().unwrap()
         );
 
         owner.set_text(msg);

gdnative-async/src/method.rs

@@ -106,22 +106,22 @@ impl<C: NativeClass, F: AsyncMethod<C>> Method<C> for Async<F> {
                         Self::site().unwrap_or_default(),
                         format_args!("unable to spawn future: {}", err),
                     );
-                    Variant::new()
+                    Variant::nil()
                 }
                 None => {
                     log::error(
                         Self::site().unwrap_or_default(),
                         format_args!("implementation did not spawn a future"),
                     );
-                    Variant::new()
+                    Variant::nil()
                 }
             }
         } else {
             log::error(
                 Self::site().unwrap_or_default(),
                 "a global executor must be set before any async methods can be called on this thread",
             );
-            Variant::new()
+            Variant::nil()
         }
     }
 }

gdnative-async/src/rt/bridge.rs

@@ -125,7 +125,7 @@ impl Method<SignalBridge> for OnSignalFn {
             })
             .unwrap();
 
-        Variant::new()
+        Variant::nil()
     }
 
     fn site() -> Option<gdnative_core::log::Site<'static>> {

gdnative-async/src/rt/func_state.rs

@@ -36,7 +36,7 @@ impl NativeClass for FuncState {
             name: "completed",
             args: &[SignalArgument {
                 name: "value",
-                default: Variant::new(),
+                default: Variant::nil(),
                 export_info: ExportInfo::new(VariantType::Nil),
                 usage: PropertyUsage::DEFAULT,
             }],

gdnative-core/src/core_types/dictionary.rs

@@ -95,13 +95,13 @@ impl<Own: Ownership> Dictionary<Own> {
     }
 
     /// 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())`.
+    /// Shorthand for `self.get_or(key, Variant::nil())`.
     #[inline]
     pub fn get_or_nil<K>(&self, key: K) -> Variant
     where
         K: OwnedToVariant + ToVariantEq,
     {
-        self.get_or(key, Variant::new())
+        self.get_or(key, Variant::nil())
     }
 
     /// Update an existing element corresponding to the key.
@@ -534,12 +534,12 @@ godot_test!(test_dictionary {
     use std::collections::HashSet;
 
     use crate::core_types::VariantType;
-    let foo = Variant::from_str("foo");
-    let bar = Variant::from_str("bar");
-    let nope = Variant::from_str("nope");
+    let foo = Variant::new("foo");
+    let bar = Variant::new("bar");
+    let nope = Variant::new("nope");
 
-    let x = Variant::from_i64(42);
-    let y = Variant::from_i64(1337);
+    let x = Variant::new(42);
+    let y = Variant::new(1337);
 
     let dict = Dictionary::new();
 
@@ -551,7 +551,7 @@ godot_test!(test_dictionary {
     assert!(!dict.contains(&nope));
 
     let keys_array = dict.keys();
-    let baz = Variant::from_str("baz");
+    let baz = Variant::new("baz");
     keys_array.push(&baz);
     dict.insert(&baz, &x);
 
@@ -561,14 +561,14 @@ godot_test!(test_dictionary {
 
     assert!(!dict.contains_all(&keys_array));
 
-    let variant = Variant::from_dictionary(&dict.duplicate().into_shared());
+    let variant = Variant::new(&dict.duplicate().into_shared());
     assert!(variant.get_type() == VariantType::Dictionary);
 
     let dict2 = dict.duplicate();
     assert!(dict2.contains(&foo));
     assert!(dict2.contains(&bar));
 
-    if let Some(dic_variant) = variant.try_to_dictionary() {
+    if let Ok(dic_variant) = variant.try_to::<Dictionary>() {
         assert!(dic_variant.len() == dict.len());
     } else {
         panic!("variant should be a Dictionary");

gdnative-core/src/core_types/string.rs

@@ -695,13 +695,13 @@ godot_test!(test_string {
     assert_eq!(index_string[1], 'a');
     assert_eq!(index_string[2], 'r');
 
-    let variant = Variant::from_godot_string(&foo);
+    let variant = Variant::new(&foo);
     assert!(variant.get_type() == VariantType::GodotString);
 
     let variant2: Variant = "foo".to_variant();
     assert!(variant == variant2);
 
-    if let Some(foo_variant) = variant.try_to_godot_string() {
+    if let Ok(foo_variant) = variant.try_to::<GodotString>() {
         assert!(foo_variant == foo);
     } else {
         panic!("variant should be a GodotString");