godot-rust / gdext

Rust bindings for Godot 4
https://mastodon.gamedev.place/@GodotRust
Mozilla Public License 2.0
2.97k stars 185 forks source link

Derive `GodotClass` for traits #426

Open lilizoey opened 11 months ago

lilizoey commented 11 months ago

similar to #334, a possible way to add restricted inheritance would be to allow traits to derive GodotClass. This would turn that trait into an abstract class. Then later we could allow people to inherit from this base class by implementing the trait.

i haven't thought out many details of this, but it has the advantage over doing it for enums that we dont need to enumerate all possible children immediately. it's also more similar in both godot and rust.

we could possibly integrate this with traits in gdscript when they're added: https://github.com/godotengine/godot-proposals/issues/6416

We'd probably need to make the trait a subtrait of Inherits<BaseClass>.

naelstrof commented 11 months ago

Both 334 and this are binding shortfalls of godot-rust that other languages currently don't suffer from.

Inherited Resources is how I implement the gang of four command pattern. It allows me to store context of the scene for things like achievements, game events, cheats, and context-sensitive data.

This feature would keep my workflow unharmed!

Here's a video of what the proposed feature would need to achieve:

output.webm

Enums might be a more rusty way to do it, though this was where I went first with my inheritance riddled workflow.

awestlake87 provided a close work-around here, but it doesn't support restricting the type, making it easy to put invalid data in.

Bromeon commented 11 months ago

Interesting idea, but there are some hurdles we need to address:

  1. Conceptually, traits are not abstract base classes, but rather interfaces.
    • A trait cannot store any state.
    • This potentially pushes duplication down to each implementor, if people want to use abstract base classes the same way as in GDScript.
  2. If we use traits polymorphically, we have two options:
    • fn accept(obj: Gd<T>) where T: Trait -- like Inherits<SomeBase> bound, but for custom bases.
    • fn accept(obj: Gd<dyn Trait>) -- dynamic dispatch needs to be paid on top of Godot's own dynamic dispatch. This is basically a more expensive upcast().
  3. Godot abstract base classes should not be seen as a replacement for proper Rust abstraction mechanisms.
    • Rust-only traits and composition
    • This feature is probably aimed at interop with GDScript and the Godot editor.

Both 334 and this are binding shortfalls of godot-rust that other languages currently don't suffer from.

Maybe other languages that have inheritance. Pretty sure Go bindings face the same issue.


Before looking at how we can implement this, we should decide what use case we actually want to solve. Because if we can't use traits to emulate full-blown abstract base classes due to lack of state, we need to see what remains.

One thing mentioned is:

The resources must be typed restricted within the editor, forcing myself to put the right data in helps me game dev faster!

That is, GDScript/editor type safety. Others?

naelstrof commented 11 months ago

Maybe other languages that have inheritance. Pretty sure Go bindings face the same issue.

Yeah you're right, honestly it seems like this is a failure on godot's part for not supporting traits or interfaces. C# also suffers from a similar problem, but is able to use inheritance to work around it. It doesn't seem like godot-rust's responsibility to make a fake inheritance just to play nice with the editor.

The most elegant solution seems to be to add traits to Godot itself, then Go, C#, and rust will bind better.

That is, GDScript/editor type safety. Others?

Specifically editor type safety, as in, the UI that allows me to drag and drop assets visually within the editor. Using inheritance means that the editor will smartly restrict what I can create or drag and drop into exported resource and node parameters.

I don't care for gdscript. Just want to work nicely with the editor.

PgBiel commented 10 months ago

I was thinking that it might not make much sense to use just traits for this. Rather, I think a struct/trait pair, much like the current ClassName / ClassNameVirtual system, would be the way to go. That would be something similar to how glib currently does it in Rust as well (and the way they do things could be used as inspiration). We might require a separate set of Inherits-like traits in order to deal with Rust-side traits (e.g. to statically verify that if you inherit a Rust class then you are implementing the companion trait), but otherwise this is probably much more viable from a technical standpoint. This could probably be made more ergonomic through macros as well.

Edit: this setup would also allow you to upcast types to the traits' "companion structs" (like in GLib) without resorting to dyn.

PgBiel commented 10 months ago

I made a small POC of having inheritance between Rust classes: https://github.com/PgBiel/gdext/commit/0f4131ca61e70b875a4ebd99582789a903412786 and https://github.com/PgBiel/gdext/commit/936dd1d67c0c353431117d2c36ae7df60bbfdd3e

It worked and I could instantiate Mob2 (which inherited from Mob) from within the editor (and use the parent Mob's methods and stuff), although there were a few quirks. In particular, some form of topological sort will be needed when registering classes as the order of declaration of Mob2 and Mob seemed to matter (Godot could complain with "parent class Mob not found" if Mob2 happened to be registered first).

So this seems to indicate that it's possible to make some form of "abstract/virtual class" within Rust. We'll likely need some trait work (using associated types) to indicate the companion/virtual trait for a particular class. We could also consider asking the abstract class' children to implement its parent's Virtual trait to override built-in virtual methods, and use the companion trait only to override Rust-side virtual methods. (For instance, if the abstract class inherited Node, its children would implement NodeVirtual to override built-in methods and TheAbstractClassCompanionTrait to override the abstract class' own virtual methods.)

Bromeon commented 10 months ago

Thanks, that looks very interesting!

Adding Rust inheritance will add quite a few things that we need to thoroughly consider:

Probably a lot of edge cases I'm currently not thinking of 🙂

PgBiel commented 10 months ago

Those are definitely important concerns; thanks for pointing them out.

Regarding Inherits<T> in particular, I gave a shot at this on my fork: https://github.com/PgBiel/gdext/commit/90cae572ea8f65d18b7e8ffb7d8222fe843d8802

In particular, we could try to do something like

impl<T> Inherits<T> for Child
where Parent: Inherits<T> {}

It did seem to compile, but I haven't tested any possible interaction with that yet (I could upcast the base, but haven't tested upcasting the child itself). Could be some food for thought for now.

StatisMike commented 8 months ago

Wouldn't it be possible to handle Rust traits for GodotClass structs in other way? I mean something like:

// trait declaration
#[godot_api_trait]
pub trait MyGodotTrait {
   #[func]
   fn my_trait_function_to_implement(&self) -> i32

   #[func]
   fn my_trait_function_with_default(&self) -> i32 {
     42
   }
}

// trait implementation
#[derive(GodotClass)]
#[class(base=Object, init)]
pub struct MyGodotClass;

#[godot_api_trait]
impl MyGodotTrait for MyGodotClass {
   #[func]
   fn my_trait_function_to_implement(&self) -> i32 {
     15
   }
}

This way we avoid the problem of Rust inheritance - we can use the regular Rust trait logic, just register the methods accordingly to struct, most notably its Godot signature. From Godot point of view the MyGodotClass won't inherit from any more GodotClasses, it would just be a class with passed methods from trait.

#[func] on trait with #[godot_api_trait] could transform and register the method signatures in gdext plugin macro (or similar one crafted for this purpose) and #[godot_api_trait] on trait implementation could look for the registered signatures (and default implementations), expand them in the implementation block and do the macro magic similar to regular #[godot_api].

The obvious red flag is of course more macro magic, unfortunately in current form, it seems inevitable. Another is the requirement for the trait declaration to be compiled before the trait implementation - I'm not exactly sure how the Rust compiler works in that regard, but in the worst-case scenario it could be possible to move the traits to a second library, which will be added as a dependency to the one making use of traits - the same it is done for procedural macros in regular Rust currently.

I would prefer this to go more in this way rather than trying to create an inheritance in Rust and between Rust-defined classes. Firstly - it is more Rusty this way, and Rust users coming in gdext territory won't be scratching their head. Secondly, it shouldn't introduce either complicated inheritance trees, eg. if user would like to implement more than one GodotClass trait.

Lastly - Godot can provide a trait system themselves in the future, as there are proposals for this already and they are discussed. Using Rust trait system in the way above shouldn't clash with Godot's traits in contrary with Inherits<T>, as their current proposals are currently debating using inheritance, but AFAIK aren't concrete yet.

EDIT: Additional hurdle is that venial in 0.5.0 doesn't support traits, though its version on GitHub does...

fpdotmonkey commented 8 months ago

What if exporting trait impls were done really naively, like just export the methods in the trait impl the same as for those in the struct impl. But then also export a method fn implements(&self, trait_name: &str) -> bool to allow for duck typing in GDScript.

mivort commented 3 days ago

Previously, in Godot 3 bindings, it was possible to something similar to what @StatisMike described by hand: it was possible to create custom register_trait function which was able to expose methods and properties defined by traits. I was using this pattern in gdnative bindings like this:

pub trait HasProperty {
    fn prop(&self) -> &GodotString;
    fn prop_mut(&mut self) -> &mut GodotString;
}

pub fn register_prop<N>(builder: &ClassBuilder<N>)
where N: HasProperty + NativeClass, N::UserData: UserDataMapMut
{
    builder.property("prop")
        .with_getter(|s, _| s.prop())
        .with_setter(|s, _, v| s.prop_mut() = v)
        .with_default("").done();

    // Similarly it's also possible to register methods, signals etc.
    // Sturcts which impl this trait will need to run this `register` method inside their own `register`s.
}

AFAIK there's currently no public API for manual registration of properties/methods/signals etc., so it's impossible to implement this pattern. With builder API (#4) it would be possible to do that again, but I also think this manual registration method can be provided by proc macro placed on top of trait definition as well: it can generate a hidden register method, which can be provided later as the option for #[godot_api] (or called manually), so the order of how classes will get registered in engine wouldn't matter.