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

Implement dynTrait for Gd<T: TraitA> #930

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Yarwin
Copy link
Contributor

@Yarwin Yarwin commented Oct 26, 2024

closes: #631
Adds an ability to cast Gd<T: Trait> to dyn Traitobject.

It is based on dynamic dispatch. Long story short, we create set of closures for every dynTrait Class which allow us to cast Gd<TraitBase> to Gd<dynTraitClass>, bind it and finally use given binded smart pointer gd as a dyn TraitObject.

I'm not sure if it is the best possible approach, albeit it is much better than anything else I've tried so far (this being – inheritance with custom-godot-api, hacky "codegen" (same as previous but with more errors), some unsafe magic with enum dispatch and just-let-godot-runtime-handle-it-all). Creating proper dynTraits might be a bit wonky due to Rust Object Safety.

Ergonomics wise it is… pretty good. I've been using it for some time (two months) and it handles all of my usecases (throwing gdext-rust-declared commands Objects from gdscript to rust dispatcher) without much issue.

I still have to do docs (ETA: 4 days) and tests (ETA: after initial review); Outside of that I would change declaration from

#[derive(GodotClass)]
#[class(init, base=…, dyn_trait = (TraitA, TraitB))]

to

#[derive(GodotClass)]
#[class(init, base=…)]
#[dyn_trait(traits=(TraitA, TraitB))]

let me know if it is alright.

Parts of the macro that check if given trait can be used to create trait objects are a bit wonky – I would gladly apply any suggestions. Same goes for checking if given method is marked as non-dispatchable.

Generated signatures in internal godot docs properly point to dynTraits Base object – it would be cool if we would be able to somehow specify groups or traits for exports and docs in the future.
image

I have no idea how to export dynTraitObjects with Arrays<…> and whatnot. It would be good idea to explore this topic in the future.

Demo:

use godot::prelude::*;

struct MyExtension;

#[gdextension]
unsafe impl ExtensionLibrary for MyExtension {}

#[derive(GodotClass)]
#[class(init, base=Node)]
struct BiggestBrother {
    #[init(val = 1)]
    brother_count: u32
}

#[godot_api]
impl BiggestBrother {
    #[func]
    fn check_brother(&mut self, mut brother: BrotherDispatch) {
        brother.greet();
        godot_print!("nice to see you, {}", brother.get_name());
        brother.return_greeting();
        godot_print!("here is your new greeting!");
        let new_greeting = GString::from(format!("Hello, I am {} no. {}", brother.get_name(), self.brother_count));
        self.brother_count += 1;
        brother.take_greeting(new_greeting);
        brother.greet();
    }
}

#[derive(GodotClass)]
#[class(init, base=Node, dyn_trait = (TraitA))]
struct BigBrother {
    greeting: GString,
    base: Base<Node>
}

impl TraitA for BigBrother {
    fn get_name(&self) -> GString {
        GString::from("Big Brother")
    }

    fn greet(&self) {
        godot_print!("{}", self.greeting)
    }

    fn return_greeting(&self) -> GString {
        GString::from("Hello to you as well")
    }

    fn take_greeting(&mut self, new_greeting: GString) {
        self.greeting = new_greeting;
    }

    fn non_dispatchable() -> Self where Self: Sized {
        unimplemented!()
    }
}

#[derive(GodotClass)]
#[class(init, base=Node, dyn_trait = (TraitA))]
struct SmallBrother {
    greeting: GString,
    base: Base<Node>
}

impl TraitA for SmallBrother {
    fn get_name(&self) -> GString {
        GString::from("small brother")
    }

    fn greet(&self) {
        godot_print!("{}", self.greeting)
    }

    fn return_greeting(&self) -> GString {
        GString::from("hello to you as well")
    }

    fn take_greeting(&mut self, new_greeting: GString) {
        self.greeting = GString::from(&new_greeting.to_string().to_lowercase());
    }

    fn non_dispatchable() -> Self where Self: Sized {
        unimplemented!()
    }
}


#[dyn_trait(name=BrotherDispatch, base=Node)]
#[allow(unused_variables)]
pub trait TraitA {
    fn get_name(&self) -> GString;
    fn greet(&self);
    fn return_greeting(&self) -> GString;
    fn take_greeting(&mut self, new_greeting: GString);
    fn non_dispatchable() -> Self where Self: Sized;
}
extends BiggestBrother

var current_brother = 0


func _on_timer_timeout() -> void:
	var brother: Node = get_child(current_brother)
	self.check_brother(brother)
	current_brother = (current_brother + 1) % (get_child_count() - 1)

Output (after calling it in a scene with n brothers):

nice to see you, small brother
here is your new greeting!
hello, i am small brother no. 1

nice to see you, Big Brother
here is your new greeting!
Hello, I am Big Brother no. 2

nice to see you, small brother
here is your new greeting!
hello, i am small brother no. 3
hello, i am small brother no. 1
nice to see you, small brother
here is your new greeting!
hello, i am small brother no. 4
(…)

@Bromeon Bromeon added feature Adds functionality to the library c: register Register classes, functions and other symbols to GDScript labels Oct 26, 2024
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-930

@Bromeon
Copy link
Member

Bromeon commented Oct 26, 2024

Thanks a lot! I'll review this after v0.2 release and once I've had a chance to drive my own implementation of this pattern a bit further -- would be cool to compare the two afterwards, probably there are some trade-offs in each! 🙂 As such, don't spend too much time on docs/tests/refinement just yet.

@Yarwin
Copy link
Contributor Author

Yarwin commented Oct 26, 2024

No problem – I'm going to use it on my own, so it shouldn't lag behind master too much.

@Yarwin Yarwin force-pushed the implement-gd-dyn branch 2 times, most recently from 9d7f14b to d292853 Compare October 27, 2024 11:10
@Bromeon Bromeon added this to the 0.2.x milestone Nov 9, 2024
@Bromeon
Copy link
Member

Bromeon commented Nov 24, 2024

I'll review this after v0.2 release and once I've had a chance to drive my own implementation of this pattern a bit further -- would be cool to compare the two afterwards, probably there are some trade-offs in each! 🙂

So, I finally found some time to wrap up my own approach in #953, took a few iterations.

I haven't had a chance to look deeply into this PR's implementation yet, but from a glance at your examples, it seems our approaches have some different goals:

This PR

  • Generates a new class that can be used in Gd, by downcasting a Godot base into a generated adapter that exposes the trait method.
  • Needs quite a bit of registration macros and is more invasive (I'm not sure if traits can be added without touching existing classes)
  • Registers trait methods with Godot
  • Reuses the existing Gd<T> facilities (no new smart pointer type)

My PR #953

  • Provides a new DynGd<Class, dyn Trait> as a general mechanism for dyn dispatch
  • Is lightweight and only needs AsDyn to link traits + classes (and can be generated with #[godot_dyn])
  • However Rust only, no registration of trait methods with Godot

Please correct if something is inaccurate!


From your comment

I've been using it for some time (two months) and it handles all of my usecases (throwing gdext-rust-declared commands Objects from gdscript to rust dispatcher) without much issue.

I believe that DynGd as proposed might not fit that, since it deliberately doesn't expose the trait API to Godot. My idea was to use "proper" inheritance for that, one day 😎

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add GdDyn<dyn Trait> type
3 participants