wip: allow custom derive to work on generics

[Gisleburt]

What

Gets any Generics and Where clauses on the type we're deriving additional functionality for and adds them to the derived code.

This needs checking over as I'm not 100% sure what I've done is right, however, what I'm aiming to do in this PR is allow the custom derive macros to work over generic items and where clauses.

Why

What I'd like to do is allow my code to be tested somewhat in isolation from the library code, see #36. One possible way to do this might be to allow Godotclass::base to be a generic item that does not need to be tied to the godot engine. Eg:

#[derive(GodotClass, Debug)]
#[class(base = Node)]
pub struct AppState<B: AbstractBase> {
    #[base]
    base: B,
    state: State,
}

AbstractBase can then be a trait that has only the methods I need for my code and that mirror the signature of Base.

use godot::prelude::*;
use thiserror::Error;
use godot::engine::global::Error as GodotError;

#[derive(Error, Debug)]
pub enum Error {
    #[error("Godot Error")]
    GodotError(GodotError),
}

impl From<GodotError> for Error {
    fn from(error: GodotError) -> Self {
        Error::GodotError(error)
    }
}

pub trait AbstractBase {
    fn emit_signal(&mut self, signal: &str, varargs: &[Variant]) -> Error;
}

impl<T: GodotClass> AbstractBase for Base<T> {
    fn emit_signal(&mut self, signal: &str, varargs: &[Variant]) -> Error {
        (self as Base<T>).emit_signal(signal, varargs)?
    }
}

[Gisleburt]

Sorry, got ahead of myself. Noticed some problems where its still not playing nicely.

[Gisleburt]

I've missed something 🤔

  --> src/state/app_state.rs:13:12
   |
13 | pub struct AppState<B> where B: AbstractBase {
   |            ^^^^^^^^ expected 1 generic argument
   |
note: struct defined here, with 1 generic parameter: `B`
  --> src/state/app_state.rs:13:12
   |
13 | pub struct AppState<B> where B: AbstractBase {
   |            ^^^^^^^^ -
help: add missing generic argument
   |
13 | pub struct AppState<B><B> where B: AbstractBase {
   |            ~~~~~~~~~~~

[Gisleburt]

I got stuck on the inherits_macro! as I'm not sure how to handle generics and where clauses inside it. :(

[Bromeon]

Thanks for this PR!

Making GodotClass derived types generic can indeed be useful, although maybe not for the reasons of mocking, see my corresponding comment. In fact, this feature has already been proposed for the GDNative binding: godot-rust/gdnative#601

Implementing this can be quite involved, since we need to explicitly instantiate each monomorphization (type instantiation for a certain T). This is needed because we can only register concrete types in Godot, not generics.

Maybe we should start from a very simple test case (in the itest module), to see what would be required for generic support?

[Gisleburt]

I added itest/rust/src/generic_struct_test.rs, however I'm not sure how to run it. I tried

$GODOT4_BIN --headless 2>&1 | tee >(grep "SCRIPT ERROR:" -q && {
    printf "\n -- Godot engine encountered error, abort...\n";
    pkill godot
    exit 2
})

from .github/composite/godot/action.yml but it just hangs (godot opens but doesn't do anything)

[Bromeon]

Sorry, this is a bit under-documented at the moment. Simply running $GODOT4_BIN --headless in the ./itest/godot directory should launch the integration tests.

The bash script from CI is an absolute hack to detect printed errors from Godot, took me forever to get working and is not recommended for general use 😬

[Gisleburt]

@Bromeon I've got the test to a point where it correctly fails to compile due to a lack of generic parameters.

   Compiling itest v0.1.0 (/Users/danielmason/projects/rust/gdextension/itest/rust)
error[E0107]: missing generics for struct `GenericStructTest`
  --> itest/rust/src/generic_struct_test.rs:28:8
   |
28 | struct GenericStructTest<T> where T: Abstraction + Debug {
   |        ^^^^^^^^^^^^^^^^^ expected 1 generic argument
   |
note: struct defined here, with 1 generic parameter: `T`
  --> itest/rust/src/generic_struct_test.rs:28:8
   |
28 | struct GenericStructTest<T> where T: Abstraction + Debug {
   |        ^^^^^^^^^^^^^^^^^ -
help: add missing generic argument
   |
28 | struct GenericStructTest<T><T> where T: Abstraction + Debug {
   |        ~~~~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0107`.
error: could not compile `itest` due to previous error

Using cargo expand shows that the inherits_macro! would need to add generics and where clauses, though it might be this isn't the only problem.

impl ::godot::obj::Inherits<::godot::engine::Node> for GenericStructTest {}
impl ::godot::obj::Inherits<::godot::engine::Object> for GenericStructTest {}

Aside: I assume if I did a more deeply nested Godot type it would generate more of these inherits impls 🤔

Unfortunately, it looks like this is extremely difficult (if not impossible) to do in macro_rules!. There's a good go at doing it for just generics here but it doesn't cover where

If we could move this functionality to the godot-macros crate though it might be possible to achieve this with a derive. I'm happy to attempt this but just want to check you'd actually want me to first. 😄

[Bromeon]

Maybe before we go head over heels, what are we exactly trying to achieve? 😉

Ultimately we will need to have explicit monomorphizations (generic instantiations) in order to register the types to Godot, so at which stage do we do that? Would it be possible to do that for now, before going all the way generic?

That is, like in the gdnative issue I linked previously in the thread, we could explicitly say for which T we want to instantiate a GodotClass. Then also the inherits relation could be set on those. I could imagine we would anyway run into orphan-rule/trait shenanigans with generic impls of Inherits.

[Gisleburt]

Oh! I'm sorry, I follow now. 🤦🏻‍♂️ Thanks for your patience.

Hmm 🤔 this might be a little beyond my current skill level, though thats how we learn. I might have a crack at the suggestion in #36 first as that seems a little more achievable and should get me going in the right direction.

From my current understanding of the project though, rather than having an implicit implementation for non-generics, it might be better to have a separate derive function that only does the Rust part on generics. This would allow you to keep the "this does not work on generics" message and add "you can use derive(GodotClassGeneric) with #[monomophized]" kind of helper. This would help avoid people being confused as to why their code compiles but doesn't show up in Godot... unless there's a way to check if GodotClass has been derived without a monomorphized version and generate an appropriate warning or error.

[Bromeon]

The original motivation

What I'd like to do is allow my code to be tested somewhat in isolation from the library code, see #36. One possible way to do this might be to allow Godotclass::base to be a generic item that does not need to be tied to the godot engine.

seems to have been achieved differently, through conditional compilation.

At the same time, I think it's too early to support generic classes in GDExtension-Rust. A lot of concepts still have to be fleshed out for simple types, and the added complexity in the proc-macro parsing (without full test coverage) might get in the way of faster refactorings. So I would close this for now.

@Gisleburt Would you be OK if your code ended up being reused at some point in the future (with you credited as co-author, of course)? Just asking, might also be that things diverge too much over time 🙂

[Gisleburt]

Yeap, feel free to use anything committed. I'm just getting around to looking at the test stuff now, will add to #36 with what I'm thinking there. I might come back to this at some point but don't wait on me.