Add type-checking to #[var(get, set)]

[Bromeon]

It was possible for user-defined getters and setters to return/accept types that are completely unrelated to the field type.

This PR introduces descriptive compile-time errors, with tweaked spans for beautiful error messages.

Based on #1466.

Example

// in struct:
#[var(get, set)]
field: GString,

// in impl:
#[func]
pub fn get_field(&self) -> i32 { 123 }

#[func]
pub fn set_field(&mut self, value: i32) {}

causes these errors:

_{(Yes, it's not quite symmetric, if someone wants to tweak this further, I'm happy for PRs 🙂)}

Breaking change

Previously, having wrong getter/setter types worked as long as they were compatible in Godot subtying terms, e.g. Gd<Derived> -> Gd<Base>, or i32 -> Variant, etc. The new typechecks are now stricter.

There are a few instances where such weak typing was used in itests, possibly deliberately, or possibly due to limitations in early godot-rust.

gdext/itest/rust/src/object_tests/property_test.rs

Lines 25 to 70 in 5a7b65c

	#[var(get = get_object_val, set = set_object_val)]
	object_val: Option<Gd<Object>>,
	#[var]
	resource_var: OnEditor<Gd<Resource>>,
	#[var(get = get_resource_rw, set = set_resource_rw, hint = RESOURCE_TYPE, hint_string = "Resource")]
	resource_rw: Option<Gd<Resource>>,
	#[var]
	packed_int_array: PackedInt32Array,
	#[var(pub, rename = renamed_variable)]
	unused_name: GString,
	}
	#[godot_api]
	impl HasProperty {
	#[func]
	pub fn get_object_val(&self) -> Variant {
	if let Some(object_val) = self.object_val.as_ref() {
	object_val.to_variant()
	} else {
	Variant::nil()
	}
	}
	#[func]
	pub fn set_object_val(&mut self, val: Gd<Object>) {
	self.object_val = Some(val);
	}
	#[func]
	pub fn get_resource_rw(&self) -> Variant {
	if let Some(resource) = self.resource_rw.as_ref() {
	resource.to_variant()
	} else {
	Variant::nil()
	}
	}
	#[func]
	pub fn set_resource_rw(&mut self, val: Gd<Resource>) {
	self.resource_rw = Some(val);
	}
	}

[GodotRust]

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1469

[Yarwin]

Both won't work in an editor – dereferencing OnEditor will result in panic if value is not set yet (and it can't be set because we can't dereference it...).

[Bromeon]

Oh, great remark! So it would also be Var::var_get() here...

I guess we shoukd document this 🤔

[Yarwin]

Yeah, and set should mirror it as well

[Yarwin]

I would change expect message (OnEditor field of given type is not nullable and mustn't be None?)

i.e. leave the information why this expect in var getter is fine.

[Yarwin]

Looks good to me;

PR in general looks intimidating but it is smaller than it seems – bulk of the changes are due to switching from &mut self to field: &mut Self in Var signature (+ blanket impl SimpleVar).

I don't get why docs for OnEditor about setters/getters has been changed though.

[Bromeon]

Thanks a lot for the great review! 👍

---

PR in general looks intimidating but it is smaller than it seems

Yep, as written this is based on #1466 -- maybe I should have linked only the 3 relevant commits (out of 6):

• https://github.com/godot-rust/gdext/pull/1469/changes/333ad817eb565c4f8ee159a1221e61d463a4ad22..7fc559315b0aa10e4039c5bfd1e3f706f2579b57

---

I don't get why docs for OnEditor about setters/getters has been changed though.

I wasn't thinking of the editor case, so I accidentally "simplified" the doctests (they kept passing in unit-tests). Will fix that + add documentation why going through Var is necessary 🧐

Regarding "/// Use the default OnEditor<T> implementation to set/retrieve the value." to me it wasn't clear what the "default implementation" refers to? Maybe just "use Var instead of the Deref" or so?

[Yarwin]

Regarding "/// Use the default OnEditor implementation to set/retrieve the value." to me it wasn't clear what the "default implementation" refers to? Maybe just "use Var instead of the Deref" or so?

I think Maybe just "use Var instead of the Deref" or so? should be OKay