Skip to content

Commit

Permalink
Merge #819
Browse files Browse the repository at this point in the history
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 <chitoyuu@potatoes.gay>
  • Loading branch information
bors[bot] and chitoyuu committed Nov 25, 2021
2 parents deb9dbf + 9f434d0 commit 6332fab
Show file tree
Hide file tree
Showing 23 changed files with 310 additions and 589 deletions.
10 changes: 0 additions & 10 deletions bindings_generator/src/methods.rs
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion examples/rpc/src/client.rs
Expand Up @@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion examples/rpc/src/server.rs
Expand Up @@ -38,7 +38,7 @@ impl Server {
owner.rpc_id(
tree.get_rpc_sender_id(),
"return_greeting",
&[Variant::from_str("hello")],
&[Variant::new("hello")],
);
}
}
10 changes: 3 additions & 7 deletions examples/scene_create/src/lib.rs
Expand Up @@ -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."),
Expand Down
6 changes: 3 additions & 3 deletions examples/signals/src/lib.rs
Expand Up @@ -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,
}],
Expand All @@ -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)]);
}
}
}
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions gdnative-async/src/method.rs
Expand Up @@ -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()
}
}
}
2 changes: 1 addition & 1 deletion gdnative-async/src/rt/bridge.rs
Expand Up @@ -125,7 +125,7 @@ impl Method<SignalBridge> for OnSignalFn {
})
.unwrap();

Variant::new()
Variant::nil()
}

fn site() -> Option<gdnative_core::log::Site<'static>> {
Expand Down
2 changes: 1 addition & 1 deletion gdnative-async/src/rt/func_state.rs
Expand Up @@ -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,
}],
Expand Down
20 changes: 10 additions & 10 deletions gdnative-core/src/core_types/dictionary.rs
Expand Up @@ -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.
Expand Down Expand Up @@ -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();

Expand All @@ -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);

Expand All @@ -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");
Expand Down
4 changes: 2 additions & 2 deletions gdnative-core/src/core_types/string.rs
Expand Up @@ -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");
Expand Down

0 comments on commit 6332fab

Please sign in to comment.