Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It is tricky to figure out how to properly use WithBaseField generically #715

Closed
lilizoey opened this issue May 19, 2024 · 4 comments · Fixed by #716
Closed

It is tricky to figure out how to properly use WithBaseField generically #715

lilizoey opened this issue May 19, 2024 · 4 comments · Fixed by #716
Labels
documentation Improvements or additions to documentation

Comments

@lilizoey
Copy link
Member

lilizoey commented May 19, 2024

See this discord thread.

But basically there isn't any explicit documentation stating that in order to call self.base/base_mut(), the compiler must know the type of Self::Base. It could be useful to add an example somewhere of how to properly declare the bounds in the generic case.

The way to declare the bound in case you want the base to be Node3D is:

fn some_fn<T>(value: &T)
where
    T: WithBaseField + GodotClass<Base = Node3D>,
{
    let base = gd.base();
}
@lilizoey lilizoey added the documentation Improvements or additions to documentation label May 19, 2024
lokimckay added a commit to lokimckay-graveyard/gdext that referenced this issue May 19, 2024
lokimckay added a commit to lokimckay-graveyard/gdext that referenced this issue May 22, 2024
github-merge-queue bot pushed a commit that referenced this issue May 22, 2024
Add WithBaseField example to traits.rs (#715)
@bluenote10
Copy link
Contributor

bluenote10 commented Oct 30, 2024

Is it actually possible to make WithBaseField work with subclasses as well?

It looks like when using WithBaseField<Base = Node> I can only use it with classes that have exactly a base of Node, but not with with classes that are any subtype of Node, i.e., are Inherits<Node>. That feels unfortunate, because the implementation itself would make sense and theory would be usable for subclasses. Conceptually I'm trying something like this, but that doesn't work:

fn some_fn<T, C>(gd: &T)
where
    T: WithBaseField<Base = C>,
    C: Inherits<Node>
{
    let base = gd.base();
}

// or
impl<T> some_fn(gd: &T)
where
    T: WithBaseField,
    <T as GodotClass>::Base: Inherits<Node>,
{
    let base = gd.base();
}

EDIT: Actually after tinkering a bit more it looks like I got the second pattern to work. The trick was to use an explicit upcast first before using it let base = self.base().clone().upcast::<Node>().

@Bromeon
Copy link
Member

Bromeon commented Oct 31, 2024

In which context did this come up? Do you not have a Gd<T> around?

@bluenote10
Copy link
Contributor

My use case was: I noticed that I'm copy/pasting the same two functions into many class impls if they want to access the tree root e.g. for viewport size querying. So I wanted to introduce a convenience trait that I can mix in where needed instead of copy/pasting code. With the following I can now write self.get_root_viewport_size() (in any class inheriting from Node or any child) simply after importing the trait.

pub trait RootViewportConvenience {
    fn get_root_viewport(&self) -> Gd<Window>;
    fn get_root_viewport_size(&self) -> Vector2i;
}

impl<T> RootViewportConvenience for T
where
    T: WithBaseField,
    <T as GodotClass>::Base: Inherits<Node>,
{
    fn get_root_viewport(&self) -> Gd<Window> {
        let base = self.base().clone().upcast::<Node>();
        base.get_tree()
            .expect("Couldn't get tree of node")
            .get_root()
            .expect("Couldn't get root of tree")
    }

    fn get_root_viewport_size(&self) -> Vector2i {
        self.get_root_viewport().get_size()
    }
}

Of course this only really makes sense if it works for all classes inheriting from Node, not only classes that are directly a Node. The pattern that was mentioned in the docstring (WithBaseField<Base = Node>) would not allow using the trait in subclasses. It took me a while to figure out how to solve such a situation fully generically.

@Bromeon
Copy link
Member

Bromeon commented Oct 31, 2024

I see, so you want to write self.get_root_viewport_size() without mentioning self.base() anywhere. 👍

Btw, you should be able to use upcast_ref() instead of clone().upcast().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants