Merge #136

136: Virtual function dispatch r=Bromeon a=Mercerenies

See #134.

This MR replaces `GodotExt` with a new trait for each built-in Godot class. The trait is the name of the Godot class followed by the word "Virtual". So, for instance, a Rust class that extends `Node2D` would implement the trait called `godot::engine::Node2DVirtual`. This trait contains all of the virtual methods belonging to `Node2D` and its ancestors, as well as a few special-cased methods that used to be on `GodotExt` (`init`, `to_string`, and `register_class`).

All methods are optional. Any unimplemented methods call `unimplemented!()` on the Rust side and never get registered as virtual methods on the Godot side.

Internally, `ImplementsGodotExt` has been split out into `ImplementsGodotVirtual` (which *only* contains real Godot virtual methods), `GodotToString`, and `GodotRegisterClass`. All of these are automatically implemented when you implement the `ClassNameVirtual` trait, and *only* the ones that you actually define are implemented (so, for example, `GodotToString` is only implemented if you actually define a `to_string` function in your trait `impl`). The user never sees any of this. From the user's perspective, the expectation *used* to be "define an inherent impl and implement `GodotExt`". Now the expectation is "define an inherent impl and implement `ClassNameVirtual` for your class".

The tests and dodge-the-creeps example have been updated to reflect these changes.

Co-authored-by: Silvio Mayolo <mercerenies@comcast.net>

examples/dodge-the-creeps/rust/src/hud.rs

@@ -1,4 +1,4 @@
-use godot::engine::{Button, CanvasLayer, Label, Timer};
+use godot::engine::{Button, CanvasLayer, CanvasLayerVirtual, Label, Timer};
 use godot::prelude::*;
 
 #[derive(GodotClass)]
@@ -61,7 +61,7 @@ impl Hud {
 }
 
 #[godot_api]
-impl GodotExt for Hud {
+impl CanvasLayerVirtual for Hud {
     fn init(base: Base<Self::Base>) -> Self {
         Self { base }
     }

examples/dodge-the-creeps/rust/src/main_scene.rs

@@ -126,7 +126,7 @@ impl Main {
 }
 
 #[godot_api]
-impl GodotExt for Main {
+impl NodeVirtual for Main {
     fn init(base: Base<Node>) -> Self {
         Main {
             mob_scene: PackedScene::new(),

examples/dodge-the-creeps/rust/src/mob.rs

@@ -1,4 +1,4 @@
-use godot::engine::{AnimatedSprite2D, RigidBody2D};
+use godot::engine::{AnimatedSprite2D, RigidBody2D, RigidBody2DVirtual};
 use godot::prelude::*;
 use rand::seq::SliceRandom;
 
@@ -47,7 +47,7 @@ impl Mob {
 }
 
 #[godot_api]
-impl GodotExt for Mob {
+impl RigidBody2DVirtual for Mob {
     fn init(base: Base<RigidBody2D>) -> Self {
         Mob {
             min_speed: 150.0,

examples/dodge-the-creeps/rust/src/player.rs

@@ -1,4 +1,4 @@
-use godot::engine::{AnimatedSprite2D, Area2D, CollisionShape2D, PhysicsBody2D};
+use godot::engine::{AnimatedSprite2D, Area2D, Area2DVirtual, CollisionShape2D, PhysicsBody2D};
 use godot::prelude::*;
 
 #[derive(GodotClass)]
@@ -42,7 +42,7 @@ impl Player {
 }
 
 #[godot_api]
-impl GodotExt for Player {
+impl Area2DVirtual for Player {
     fn init(base: Base<Area2D>) -> Self {
         Player {
             speed: 400.0,

godot-codegen/src/api_parser.rs

@@ -167,7 +167,7 @@ pub struct BuiltinClassMethod {
     pub arguments: Option<Vec<MethodArg>>,
 }
 
-#[derive(DeJson)]
+#[derive(DeJson, Clone)]
 pub struct ClassMethod {
     pub name: String,
     pub is_const: bool,
@@ -200,7 +200,7 @@ pub struct MethodArg {
 }
 
 // Example: get_available_point_id -> {type: "int", meta: "int64"}
-#[derive(DeJson)]
+#[derive(DeJson, Clone)]
 pub struct MethodReturn {
     #[nserde(rename = "type")]
     pub type_: String,

godot-codegen/src/class_generator.rs

@@ -159,6 +159,7 @@ fn make_constructor(class: &Class, ctx: &Context) -> TokenStream {
 fn make_class(class: &Class, class_name: &TyName, ctx: &mut Context) -> GeneratedClass {
     // Strings
     let godot_class_str = &class_name.godot_ty;
+    let virtual_trait_str = class_name.virtual_trait_name();
 
     // Idents and tokens
     let base = match class.inherits.as_ref() {
@@ -174,6 +175,7 @@ fn make_class(class: &Class, class_name: &TyName, ctx: &mut Context) -> Generate
     let enums = make_enums(&class.enums, class_name, ctx);
     let inherits_macro = format_ident!("inherits_transitive_{}", class_name.rust_ty);
     let all_bases = ctx.inheritance_tree().collect_all_bases(class_name);
+    let virtual_trait = make_virtual_methods_trait(class, &all_bases, &virtual_trait_str, ctx);
 
     let memory = if class_name.rust_ty == "Object" {
         ident("DynamicRefCount")
@@ -199,6 +201,7 @@ fn make_class(class: &Class, class_name: &TyName, ctx: &mut Context) -> Generate
             pub struct #class_name {
                 object_ptr: sys::GDExtensionObjectPtr,
             }
+            #virtual_trait
             impl #class_name {
                 #constructor
                 #methods
@@ -323,12 +326,14 @@ fn make_module_file(classes_and_modules: Vec<GeneratedClassModule>) -> TokenStre
             is_pub,
             ..
         } = m;
+        let virtual_trait_name = ident(&class_name.virtual_trait_name());
 
         let vis = is_pub.then_some(quote! { pub });
 
         quote! {
             #vis mod #module_name;
             pub use #module_name::re_export::#class_name;
+            pub use #module_name::re_export::#virtual_trait_name;
         }
     });
 
@@ -463,12 +468,15 @@ fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool {
     }
 }
 
-fn is_method_excluded(method: &ClassMethod, #[allow(unused_variables)] ctx: &mut Context) -> bool {
+fn is_method_excluded(
+    method: &ClassMethod,
+    is_virtual_impl: bool,
+    #[allow(unused_variables)] ctx: &mut Context,
+) -> bool {
     // Currently excluded:
     //
-    // * Private virtual methods designed for override; skip for now
-    //   E.g.: AudioEffectInstance::_process(const void*, AudioFrame*, int)
-    //   TODO decide what to do with them, overriding in a type-safe way?
+    // * Private virtual methods are only included in a virtual
+    //   implementation.
     //
     // * Methods accepting pointers are often supplementary
     //   E.g.: TextServer::font_set_data_ptr() -- in addition to TextServer::font_set_data().
@@ -490,11 +498,14 @@ fn is_method_excluded(method: &ClassMethod, #[allow(unused_variables)] ctx: &mut
     }
     // -- end.
 
-    method.name.starts_with('_')
-        || method
-            .return_value
-            .as_ref()
-            .map_or(false, |ret| ret.type_.contains('*'))
+    if method.name.starts_with('_') && !is_virtual_impl {
+        return true;
+    }
+
+    method
+        .return_value
+        .as_ref()
+        .map_or(false, |ret| ret.type_.contains('*'))
         || method
             .arguments
             .as_ref()
@@ -523,7 +534,8 @@ fn make_method_definition(
     class_name: &TyName,
     ctx: &mut Context,
 ) -> TokenStream {
-    if is_method_excluded(method, ctx) || special_cases::is_deleted(class_name, &method.name) {
+    if is_method_excluded(method, false, ctx) || special_cases::is_deleted(class_name, &method.name)
+    {
         return TokenStream::new();
     }
     /*if method.map_args(|args| args.is_empty()) {
@@ -799,13 +811,7 @@ fn make_receiver(
     is_const: bool,
     receiver_arg: TokenStream,
 ) -> (TokenStream, TokenStream) {
-    let receiver = if is_static {
-        quote! {}
-    } else if is_const {
-        quote! { &self, }
-    } else {
-        quote! { &mut self, }
-    };
+    let receiver = make_receiver_self_param(is_static, is_const);
 
     let receiver_arg = if is_static {
         quote! { std::ptr::null_mut() }
@@ -816,6 +822,16 @@ fn make_receiver(
     (receiver, receiver_arg)
 }
 
+fn make_receiver_self_param(is_static: bool, is_const: bool) -> TokenStream {
+    if is_static {
+        quote! {}
+    } else if is_const {
+        quote! { &self, }
+    } else {
+        quote! { &mut self, }
+    }
+}
+
 fn make_params(
     method_args: &Option<Vec<MethodArg>>,
     is_varcall: bool,
@@ -930,3 +946,113 @@ fn make_return(
 
     (return_decl, call)
 }
+
+fn make_virtual_methods_trait(
+    class: &Class,
+    all_bases: &[TyName],
+    trait_name: &str,
+    ctx: &mut Context,
+) -> TokenStream {
+    let trait_name = ident(trait_name);
+
+    let virtual_method_fns = make_all_virtual_methods(class, all_bases, ctx);
+    let special_virtual_methods = special_virtual_methods();
+
+    quote! {
+        #[allow(unused_variables)]
+        #[allow(clippy::unimplemented)]
+        pub trait #trait_name: crate::private::You_forgot_the_attribute__godot_api + crate::obj::GodotClass {
+            #( #virtual_method_fns )*
+            #special_virtual_methods
+        }
+    }
+}
+
+fn special_virtual_methods() -> TokenStream {
+    quote! {
+        fn register_class(builder: &mut crate::builder::ClassBuilder<Self>) {
+            unimplemented!()
+        }
+        fn init(base: crate::obj::Base<Self::Base>) -> Self {
+            unimplemented!()
+        }
+        fn to_string(&self) -> crate::builtin::GodotString {
+            unimplemented!()
+        }
+    }
+}
+
+fn make_virtual_method(class_method: &ClassMethod, ctx: &mut Context) -> TokenStream {
+    let method_name = ident(virtual_method_name(class_method));
+
+    // Virtual methods are never static.
+    assert!(!class_method.is_static);
+
+    let receiver = make_receiver_self_param(false, class_method.is_const);
+    let [params, _, _, _] = make_params(&class_method.arguments, class_method.is_vararg, ctx);
+
+    quote! {
+        fn #method_name ( #receiver #( #params , )* ) {
+          unimplemented!()
+        }
+    }
+}
+
+fn make_all_virtual_methods(
+    class: &Class,
+    all_bases: &[TyName],
+    ctx: &mut Context,
+) -> Vec<TokenStream> {
+    let mut all_virtuals = vec![];
+    let mut extend_virtuals = |class| {
+        all_virtuals.extend(
+            get_methods_in_class(class)
+                .iter()
+                .cloned()
+                .filter(|m| m.is_virtual),
+        );
+    };
+
+    // Get virtuals defined on the current class.
+    extend_virtuals(class);
+    // Add virtuals from superclasses.
+    for base in all_bases {
+        let superclass = ctx.get_engine_class(base);
+        extend_virtuals(superclass);
+    }
+    all_virtuals
+        .into_iter()
+        .filter_map(|method| {
+            if is_method_excluded(&method, true, ctx) {
+                None
+            } else {
+                Some(make_virtual_method(&method, ctx))
+            }
+        })
+        .collect()
+}
+
+fn get_methods_in_class(class: &Class) -> &[ClassMethod] {
+    match &class.methods {
+        None => &[],
+        Some(methods) => methods,
+    }
+}
+
+fn virtual_method_name(class_method: &ClassMethod) -> &str {
+    // Matching the C++ convention, we remove the leading underscore
+    // from virtual method names.
+    let method_name = class_method
+        .name
+        .strip_prefix('_')
+        .unwrap_or(&class_method.name);
+
+    // As a special exception, a few classes define a virtual method
+    // called "_init" (distinct from the constructor), so we rename
+    // those to avoid a name conflict in our trait.
+    if method_name == "init" {
+        "init_ext"
+    } else {
+        method_name
+    }
+}

godot-codegen/src/context.rs

@@ -4,12 +4,13 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+use crate::api_parser::Class;
 use crate::{ExtensionApi, RustTy, TyName};
 use std::collections::{HashMap, HashSet};
 
 #[derive(Default)]
 pub(crate) struct Context<'a> {
-    engine_classes: HashSet<TyName>,
+    engine_classes: HashMap<TyName, &'a Class>,
     builtin_types: HashSet<&'a str>,
     singletons: HashSet<&'a str>,
     inheritance_tree: InheritanceTree,
@@ -39,7 +40,7 @@ impl<'a> Context<'a> {
             }
 
             println!("-- add engine class {}", class_name.description());
-            ctx.engine_classes.insert(class_name.clone());
+            ctx.engine_classes.insert(class_name.clone(), class);
 
             if let Some(base) = class.inherits.as_ref() {
                 let base_name = TyName::from_godot(base);
@@ -50,6 +51,10 @@ impl<'a> Context<'a> {
         ctx
     }
 
+    pub fn get_engine_class(&self, class_name: &TyName) -> &Class {
+        self.engine_classes.get(class_name).unwrap()
+    }
+
     // pub fn is_engine_class(&self, class_name: &str) -> bool {
     //     self.engine_classes.contains(class_name)
     // }

godot-codegen/src/lib.rs

@@ -197,6 +197,10 @@ impl TyName {
             format!("{}  [renamed {}]", self.godot_ty, self.rust_ty)
         }
     }
+
+    fn virtual_trait_name(&self) -> String {
+        format!("{}Virtual", self.rust_ty)
+    }
 }
 
 impl ToTokens for TyName {