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

#[func(virtual)]: override virtual Rust functions in GDScript #606

Merged
merged 7 commits into from
Feb 13, 2024

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Feb 12, 2024

This adds support for godotengine/godot#87758 in Rust.
Since that PR is not yet merged, CI will fail.

Feature description

If you have a Rust method

#[godot_api]
impl MyClass {
    #[func(virtual)]
    fn greet(&self) -> GString {
        "Rust".into()
    }
}

, you can now attach a script to an instance of MyClass and override this method. This should work for all sorts of scripts, but most common is GDScript:

extends MyClass

func _greet() -> String:
    return "GDScript"

Calling self.greet() in Rust will now directly return either "Rust" (if no script is attached) or "GDScript" (if a script is attached).

greet() thus always implements late-binding, you cannot call it statically. If that is desired, a separate Rust method can be declared.

API Design

I deliberately chose the most lightweight syntactical option, since it's very easy to understand and use. Also, turning an existing method into a virtual one requires just adding one keyword.

A "more idiomatic" approach with traits and impls was also considered.
// Existing impl block for non-virtual methods.
#[godot_api]
impl MyClass {
    ...
}

// Trait to define the new virtual API.
#[godot_api(virtuals)]
trait MyClassVirtuals {
    fn virtual(&self, i: i32) { ... }
}

// Impl block without a body, to signal that gdext implements it.
#[godot_api(virtuals=MyClassVirtuals)]
impl MyClassVirtuals for MyClass {}

However, this:

  • comes with a ton of boilerplate for the user, at no added value
  • makes switching between virtual/non-virtual a significant refactoring exercise
  • requires two extra items to handle in the proc-macro API
  • needs explicit specification of relations (the virtuals= part), i.e. also error potential

The primary goal of gdext is to be ergonomic and useful in practice, and the trait/impl approach is pretty much the antithesis thereof. But I believe #[func(virtual)]'s late-binding semantics are simple enough for users to understand.

Future work

A few things are not done:

  1. Re-entrancy: we can see if this can be integrated with base/base_mut somehow, to allow the script to call back to the same object.

    • For now, #[func(gd_self)] can be used to achieve this.
  2. Required implementation: at the moment, the user must provide a default implementation. Sometimes, you'd want to force the script to override a method though.

    • Godot does not natively support that; GDScript parsing/compilation will always succeed.
    • For now, the Rust method can panic in such cases.
    • We could enable abstract / pure virtual methods in the future that auto-panic.
    • Maybe it's even possible to check after _ready() that a script is attached and the corresponding function is overridden. This would move the error from the time of call to the time of scene-entering.
  3. Calling super methods.

    • The script method cannot call the base method; I think this is not supported by GDExtension.
    • Something like this could maybe be emulated if there is demand, but we would need to use a different method. So it's questionable if that's even worth its own feature, as those semantics are trivial to achieve with a separate #[func].
  4. Dynamic dispatch from script side.

    • Rust calls to greet in the above example can either call the script implementation or the Rust fallback.
    • This is not possible when calling from GDScript: obj._greet() will either call the script implementation, or fail if there is no script.
    • This also seems to be a limitation in Godot. It can be worked around by having another #[func] -- just like for the super case. Concretely, this other #[func] would forward the call to Rust-side greet (thus dispatch dynamically), and would be accessible from GDScript.
    • Maybe we can establish conventions around those use cases and add library support to simplify them.
  5. Diagnostics

    • Godot already provides quite a good error on signature mismatch:

        SCRIPT ERROR: Parse Error: The function signature doesn't match the parent.
        Parent signature is "method_name(String) -> String".
      
    • Depending on user feedback, we can see if there are additional error messages/diagnostics we can provide.

@Bromeon Bromeon added feature Adds functionality to the library c: register Register classes, functions and other symbols to GDScript labels Feb 12, 2024
@Bromeon Bromeon force-pushed the feature/virtual-func branch 4 times, most recently from 3af3d82 to df9efaf Compare February 12, 2024 18:19
@Bromeon Bromeon force-pushed the feature/virtual-func branch from df9efaf to b1803bb Compare February 13, 2024 11:25
@Bromeon Bromeon marked this pull request as ready for review February 13, 2024 11:27
@Bromeon Bromeon force-pushed the feature/virtual-func branch 2 times, most recently from 2e46005 to 9514ea2 Compare February 13, 2024 11:51
@Bromeon Bromeon force-pushed the feature/virtual-func branch from 9514ea2 to 446e3b5 Compare February 13, 2024 12:09
@GodotRust
Copy link

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

@Bromeon Bromeon enabled auto-merge February 13, 2024 18:21
@Bromeon Bromeon added this pull request to the merge queue Feb 13, 2024
Merged via the queue into master with commit 4c9f7bc Feb 13, 2024
16 checks passed
@Bromeon Bromeon deleted the feature/virtual-func branch February 13, 2024 18:41
@Bromeon Bromeon mentioned this pull request May 1, 2024
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.

2 participants