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

Derive GodotClass for traits #426

Open
Tracked by #697
lilizoey opened this issue Sep 25, 2023 · 9 comments
Open
Tracked by #697

Derive GodotClass for traits #426

lilizoey opened this issue Sep 25, 2023 · 9 comments
Labels
c: register Register classes, functions and other symbols to GDScript feature Adds functionality to the library hard Opposite of "good first issue": needs deeper know-how and significant design work.

Comments

@lilizoey
Copy link
Member

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: godotengine/godot-proposals#6416

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

@lilizoey lilizoey added feature Adds functionality to the library c: register Register classes, functions and other symbols to GDScript hard Opposite of "good first issue": needs deeper know-how and significant design work. labels Sep 25, 2023
@naelstrof
Copy link

naelstrof commented Sep 25, 2023

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
  • The resources must be typed restricted within the editor, forcing myself to put the right data in helps me game dev faster!
  • The traits must be somehow recovered on the Rust side. I don't care what data was put in provided I can call some expected function on them.

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
Copy link
Member

Bromeon commented Sep 26, 2023

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
Copy link

naelstrof commented Sep 26, 2023

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
Copy link
Contributor

PgBiel commented Oct 22, 2023

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
Copy link
Contributor

PgBiel commented Nov 4, 2023

I made a small POC of having inheritance between Rust classes: PgBiel@0f4131c and PgBiel@936dd1d

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
Copy link
Member

Bromeon commented Nov 5, 2023

Thanks, that looks very interesting!

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

  • We would need to decide how multiple virtual traits interact. Probably, derived traits' methods would "override" base traits' ones.
  • Integrate with Inherits<T> trait and make casting work.
  • Registering order, as you say.
  • Symbol lookup (you mentioned rustbase, but the struct might be in another module).
  • Object identity (instance ID) must be consistent.
  • Exported properties, signals etc. must be consistent.
  • Hard: how does bind/bind_mut work when we have now 2 possible Rust classes? It's possible to access T as T::Base and get references to a different type. But can it also be accessed through #[base]?
  • Implicit Deref is almost certainly out of the question.
  • Related to binding, we need to check multithreading implications.

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

@PgBiel
Copy link
Contributor

PgBiel commented Nov 5, 2023

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

Regarding Inherits<T> in particular, I gave a shot at this on my fork: PgBiel@90cae57

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
Copy link
Contributor

StatisMike commented Dec 23, 2023

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: register Register classes, functions and other symbols to GDScript feature Adds functionality to the library hard Opposite of "good first issue": needs deeper know-how and significant design work.
Projects
None yet
Development

No branches or pull requests

6 participants