Var now supports #[var(pub)]; add SimpleVar to reuse existing Godot conversions#1466
Var now supports #[var(pub)]; add SimpleVar to reuse existing Godot conversions#1466
Var now supports #[var(pub)]; add SimpleVar to reuse existing Godot conversions#1466Conversation
Var refactor: add VarPub + VarByGodotConvert traits
|
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1466 |
1e879e5 to
a99e9b9
Compare
…Var` Also add new `Var` impls: `InstanceId`, `AnyArray`
Design iteration 1Original design in this PR, ready in a99e9b9.
|
Design iteration 2I tried a different, more modular approach: pub trait VarAccess<T> {
type Value;
fn var_get(field: &T) -> Self::Value;
fn var_set(field: &mut T, value: Self::Value);
}
pub trait Var: GodotConvert {
/// Used for internal conversion -- replaces `get_property`/`set_property` impls.
type GodotAccess: VarAccess<Self, Value = Self::Via>;
/// Type used in generated getter/setter (`#[var(pub)]`).
type PubAccess: VarAccess<Self>;
/// Unchanged.
fn var_hint() -> PropertyHintInfo { ... }
}
/// Utility methods and types (maybe hidden):
pub type VarGodotAccess<T> = <<T as Var>::GodotAccess as VarAccess<T>>::Value;
pub type VarPubAccess<T> = <<T as Var>::PubAccess as VarAccess<T>>::Value;
pub fn var_godot_get<T: Var>(field: &T) -> VarGodotAccess<T> { T::GodotAccess::var_get(field) }
pub fn var_godot_set<T: Var>(field: &mut T, v: VarGodotAccess<T>) { T::GodotAccess::var_set(field, v) }
pub fn var_pub_get<T: Var>(field: &T) -> VarPubAccess<T> { T::PubAccess::var_get(field) }
pub fn var_pub_set<T: Var>(field: &mut T, v: VarPubAccess<T>) { T::PubAccess::var_set(field, v) }Then we would have some building blocks for the "access" strategies: /// Access that reuses existing Godot conversion: returns `Via`, uses `ToGodot`/`FromGodot`.
pub struct VarAccessVia;
impl<T> VarAccess<T> for VarAccessVia
where
T: ToGodot + FromGodot,
T::Via: Clone,
{
type Value = T::Via;
fn var_get(field: &T) -> Self::Value {
field.to_godot_owned()
}
fn var_set(field: &mut T, value: Self::Value) {
*field = FromGodot::from_godot(value);
}
}/// Access by using `Self` directly (not through `Via`).
pub struct VarAccessSelf;
impl<T: Clone> VarAccess<T> for VarAccessSelf {
type Value = T;
fn var_get(field: &T) -> Self::Value {
field.clone()
}
fn var_set(field: &mut T, value: Self::Value) {
*field = value;
}
}Most types would then simply need something like this -- Which also covers enums (one of the reasons for this separation), which have impl Var for MyType {
type GodotAccess = VarAccessVia;
type PubAccess = VarAccessSelf;
}However, the custom impls are quite a bit more involved...impl<T: Var> Var for PhantomVar<T> {
type GodotAccess = ...;
type PubAccess = VarAccessNone;
}
pub struct VarAccessNone;
impl<T> VarAccess<T> for VarAccessNone {
type Value = std::convert::Infallible;
fn var_get(_field: &T) -> Self::Value {
unreachable!("VarAccessNone should not generate public getters")
}
fn var_set(_field: &mut T, _value: Self::Value) {
unreachable!("VarAccessNone should not generate public setters")
}
}pub struct VarAccessOnEditor<T>(PhantomData<T>);
impl<T: Var + FromGodot + PartialEq> VarAccess<OnEditor<T>> for VarAccessOnEditor<T>
where
T::Via: BuiltinExport,
{
type Value = T::Via;
fn var_get(field: &OnEditor<T>) -> Self::Value {
field.get_property_inner().expect("...")
}
fn var_set(field: &mut OnEditor<T>, value: Self::Value) {
field.set_property_inner(Some(value));
}
}Overall, the indirection is flexible but makes things quite a bit more complex -- in the cases where people do need to customize I'll look further... |
a99e9b9 to
66ff746
Compare
Var refactor: add VarPub + VarByGodotConvert traitsVar now supports #[var(pub)]; add SimpleVar to reuse existing Godot conversions
90dcb60 to
9adaa6b
Compare
9adaa6b to
333ad81
Compare
Design iteration 3At last, I converged towards an API with which I'm somewhat happy. Explanation in the initial post 🙂 |
godot-core/src/obj/on_editor.rs
Outdated
| /// | ||
| /// Custom setters and/or getters for `OnEditor` are declared as for any other `#[var]`. | ||
| /// Use the default `OnEditor<T>` implementation to set/retrieve the value. | ||
| /// Access the value through dereferencing, since `OnEditor<T>` implements `Deref` and `DerefMut`. |
There was a problem hiding this comment.
Similarly as in follow-up PR – OnEditor panics on deref (semantically – this value can't be None/null on runtime, trust me bro) thus setters/getters must delegate this functionality to Var (deref will panic).
|
I can't say much without testing it thoroughly in the practice but I think it is fine? |
9dd08fe to
aea7a44
Compare
Changes `get_property`/`set_property` methods to associated functions `var_get`/`var_set`, to reduce namespace pollution for "extension" symbols. Adds `var_pub_set`/`var_pub_get` to customize `#[var(pub)]` access. Integrates `SimpleVar` with new methods for generated get/set.
aea7a44 to
1cde4b0
Compare
|
One thing that's not great is that That was already somewhat the case before:
Potential solutions -- likely (1) is the winner for near future as it's not a huge deal in practice.
|
User-facing API
Changes the
Vartrait from:to:
Adds
SimpleVartrait. Types that already supportToGodot/FromGodotcan benefit from a fast-trackVarimpl reusing existing conversions:Semantic changes
#[var],#[func],#[signal]#1172.#[var(pub)]attribute with two new methods inVar.{get,set}_propertytovar_{get,set}and turn methods into associated functions..get_property()/.set_property()in IDE although that's almost never what you want. It was also an overly generic name.#[var(pub)]fields are missing required trait bounds.InstanceIdandAnyArrayimpls toVar.Refactors
Varimpls, reducing a lot of code duplication. It turns out that 90% of ourVars were simply delegating toFromGodot+ToGodot. The marker traitSimpleVarreduces a lot of repetition here.GetterSetterImpl::from_generated_impl()logic.