Merge pull request #771 from TitanNano/jovan/required_virtuals

Required virtual methods should be required at compile-time
godot-rust · Aug 9, 2024 · b294625 · b294625
2 parents 558b29f + 367a928
commit b294625
Show file tree

Hide file tree

Showing 20 changed files with 515 additions and 99 deletions.
diff --git a/.github/composite/godot-itest/action.yml b/.github/composite/godot-itest/action.yml
@@ -142,7 +142,7 @@ runs:
           targetArgs="--target $TARGET"
         fi
         
-        cargo build -p itest ${{ inputs.rust-extra-args }} $targetArgs
+        cargo build -p itest --no-default-features ${{ inputs.rust-extra-args }} $targetArgs
         
         # Instead of modifying .gdextension, rename the output directory
         if [[ -n "$TARGET" ]]; then

diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml
@@ -310,15 +310,15 @@ jobs:
             os: ubuntu-20.04
             artifact-name: linux-nightly
             godot-binary: godot.linuxbsd.editor.dev.x86_64
-            rust-extra-args: --features godot/__codegen-full
+            rust-extra-args: --features itest/codegen-full
             with-hot-reload: true
 
           # Combines now a lot of features, but should be OK. lazy-function-tables doesn't work with experimental-threads.
           - name: linux-double-lazy
             os: ubuntu-20.04
             artifact-name: linux-double-nightly
             godot-binary: godot.linuxbsd.editor.dev.double.x86_64
-            rust-extra-args: --features godot/api-custom,godot/double-precision,godot/__codegen-full,godot/lazy-function-tables
+            rust-extra-args: --features godot/api-custom,godot/double-precision,itest/codegen-full,godot/lazy-function-tables
 
           - name: linux-features-experimental
             os: ubuntu-20.04

diff --git a/.github/workflows/minimal-ci.yml b/.github/workflows/minimal-ci.yml
@@ -157,7 +157,7 @@ jobs:
             os: ubuntu-20.04
             artifact-name: linux-nightly
             godot-binary: godot.linuxbsd.editor.dev.x86_64
-            rust-extra-args: --features godot/__codegen-full
+            rust-extra-args: --features itest/codegen-full
             with-hot-reload: true
 
           - name: linux-features-experimental

diff --git a/godot-codegen/src/generator/builtins.rs b/godot-codegen/src/generator/builtins.rs
@@ -250,6 +250,7 @@ fn make_builtin_method_definition(
             receiver,
             varcall_invocation,
             ptrcall_invocation,
+            is_virtual_required: false,
         },
         safety_doc,
         &TokenStream::new(),

diff --git a/godot-codegen/src/generator/classes.rs b/godot-codegen/src/generator/classes.rs
@@ -535,6 +535,7 @@ fn make_class_method_definition(
             receiver,
             varcall_invocation,
             ptrcall_invocation,
+            is_virtual_required: false,
         },
         None,
         cfg_attributes,

diff --git a/godot-codegen/src/generator/functions_common.rs b/godot-codegen/src/generator/functions_common.rs
@@ -37,6 +37,7 @@ pub struct FnCode {
     pub receiver: FnReceiver,
     pub varcall_invocation: TokenStream,
     pub ptrcall_invocation: TokenStream,
+    pub is_virtual_required: bool,
 }
 
 pub struct FnDefinition {
@@ -150,6 +151,11 @@ pub fn make_function_definition(
     };
 
     let return_decl = &sig.return_value().decl;
+    let fn_body = if code.is_virtual_required {
+        quote! { ; }
+    } else {
+        quote! { { unimplemented!() } }
+    };
 
     let receiver_param = &code.receiver.param;
     let primary_function = if sig.is_virtual() {
@@ -160,9 +166,7 @@ pub fn make_function_definition(
             #maybe_unsafe fn #primary_fn_name(
                 #receiver_param
                 #( #params, )*
-            ) #return_decl {
-                unimplemented!()
-            }
+            ) #return_decl #fn_body
         }
     } else if sig.is_vararg() {
         // Varargs (usually varcall, but not necessarily -- utilities use ptrcall)

diff --git a/godot-codegen/src/generator/utility_functions.rs b/godot-codegen/src/generator/utility_functions.rs
@@ -70,6 +70,7 @@ pub(crate) fn make_utility_function_definition(function: &UtilityFunction) -> To
             receiver: FnReceiver::global_function(),
             varcall_invocation,
             ptrcall_invocation,
+            is_virtual_required: false,
         },
         None,
         &TokenStream::new(),

diff --git a/godot-codegen/src/generator/virtual_traits.rs b/godot-codegen/src/generator/virtual_traits.rs
@@ -154,6 +154,7 @@ fn make_virtual_method(method: &ClassMethod) -> Option<TokenStream> {
             // make_return() requests following args, but they are not used for virtual methods. We can provide empty streams.
             varcall_invocation: TokenStream::new(),
             ptrcall_invocation: TokenStream::new(),
+            is_virtual_required: method.is_virtual_required(),
         },
         None,
         &TokenStream::new(),

diff --git a/godot-codegen/src/models/domain.rs b/godot-codegen/src/models/domain.rs
@@ -280,6 +280,7 @@ pub struct FunctionCommon {
     pub return_value: FnReturn,
     pub is_vararg: bool,
     pub is_private: bool,
+    pub is_virtual_required: bool,
     pub direction: FnDirection,
 }
 
@@ -314,6 +315,10 @@ pub trait Function: fmt::Display {
     fn direction(&self) -> FnDirection {
         self.common().direction
     }
+
+    fn is_virtual_required(&self) -> bool {
+        self.common().is_virtual_required
+    }
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------

diff --git a/godot-codegen/src/models/domain_mapping.rs b/godot-codegen/src/models/domain_mapping.rs
@@ -358,6 +358,7 @@ impl BuiltinMethod {
                 return_value: FnReturn::new(&return_value, ctx),
                 is_vararg: method.is_vararg,
                 is_private: special_cases::is_method_private(builtin_name, &method.name),
+                is_virtual_required: false,
                 direction: FnDirection::Outbound {
                     hash: method.hash.expect("hash absent for builtin method"),
                 },
@@ -463,6 +464,10 @@ impl ClassMethod {
                 return_value: FnReturn::new(&method.return_value, ctx),
                 is_vararg: method.is_vararg,
                 is_private,
+                is_virtual_required: special_cases::is_virtual_method_required(
+                    &class_name.rust_ty.to_string(),
+                    rust_method_name,
+                ),
                 direction,
             },
             qualifier,
@@ -511,6 +516,7 @@ impl UtilityFunction {
                 return_value: FnReturn::new(&return_value, ctx),
                 is_vararg: function.is_vararg,
                 is_private: false,
+                is_virtual_required: false,
                 direction: FnDirection::Outbound {
                     hash: function.hash,
                 },

diff --git a/godot-codegen/src/special_cases/special_cases.rs b/godot-codegen/src/special_cases/special_cases.rs
@@ -336,6 +336,190 @@ pub fn get_interface_extra_docs(trait_name: &str) -> Option<&'static str> {
     }
 }
 
+pub fn is_virtual_method_required(class_name: &str, method: &str) -> bool {
+    match (class_name, method) {
+        ("ScriptLanguageExtension", _) => method != "get_doc_comment_delimiters",
+
+        ("ScriptExtension", "editor_can_reload_from_file")
+        | ("ScriptExtension", "can_instantiate")
+        | ("ScriptExtension", "get_base_script")
+        | ("ScriptExtension", "get_global_name")
+        | ("ScriptExtension", "inherits_script")
+        | ("ScriptExtension", "get_instance_base_type")
+        | ("ScriptExtension", "instance_create")
+        | ("ScriptExtension", "placeholder_instance_create")
+        | ("ScriptExtension", "instance_has")
+        | ("ScriptExtension", "has_source_code")
+        | ("ScriptExtension", "get_source_code")
+        | ("ScriptExtension", "set_source_code")
+        | ("ScriptExtension", "reload")
+        | ("ScriptExtension", "get_documentation")
+        | ("ScriptExtension", "has_method")
+        | ("ScriptExtension", "has_static_method")
+        | ("ScriptExtension", "get_method_info")
+        | ("ScriptExtension", "is_tool")
+        | ("ScriptExtension", "is_valid")
+        | ("ScriptExtension", "get_language")
+        | ("ScriptExtension", "has_script_signal")
+        | ("ScriptExtension", "get_script_signal_list")
+        | ("ScriptExtension", "has_property_default_value")
+        | ("ScriptExtension", "get_property_default_value")
+        | ("ScriptExtension", "update_exports")
+        | ("ScriptExtension", "get_script_method_list")
+        | ("ScriptExtension", "get_script_property_list")
+        | ("ScriptExtension", "get_member_line")
+        | ("ScriptExtension", "get_constants")
+        | ("ScriptExtension", "get_members")
+        | ("ScriptExtension", "is_placeholder_fallback_enabled")
+        | ("ScriptExtension", "get_rpc_config")
+        | ("EditorExportPlugin", "customize_resource")
+        | ("EditorExportPlugin", "customize_scene")
+        | ("EditorExportPlugin", "get_customization_configuration_hash")
+        | ("EditorExportPlugin", "get_name")
+        | ("EditorVcsInterface", _)
+        | ("MovieWriter", _)
+        | ("TextServerExtension", "has_feature")
+        | ("TextServerExtension", "get_name")
+        | ("TextServerExtension", "get_features")
+        | ("TextServerExtension", "free_rid")
+        | ("TextServerExtension", "has")
+        | ("TextServerExtension", "create_font")
+        | ("TextServerExtension", "font_set_fixed_size")
+        | ("TextServerExtension", "font_get_fixed_size")
+        | ("TextServerExtension", "font_set_fixed_size_scale_mode")
+        | ("TextServerExtension", "font_get_fixed_size_scale_mode")
+        | ("TextServerExtension", "font_get_size_cache_list")
+        | ("TextServerExtension", "font_clear_size_cache")
+        | ("TextServerExtension", "font_remove_size_cache")
+        | ("TextServerExtension", "font_set_ascent")
+        | ("TextServerExtension", "font_get_ascent")
+        | ("TextServerExtension", "font_set_descent")
+        | ("TextServerExtension", "font_get_descent")
+        | ("TextServerExtension", "font_set_underline_position")
+        | ("TextServerExtension", "font_get_underline_position")
+        | ("TextServerExtension", "font_set_underline_thickness")
+        | ("TextServerExtension", "font_get_underline_thickness")
+        | ("TextServerExtension", "font_set_scale")
+        | ("TextServerExtension", "font_get_scale")
+        | ("TextServerExtension", "font_get_texture_count")
+        | ("TextServerExtension", "font_clear_textures")
+        | ("TextServerExtension", "font_remove_texture")
+        | ("TextServerExtension", "font_set_texture_image")
+        | ("TextServerExtension", "font_get_texture_image")
+        | ("TextServerExtension", "font_get_glyph_list")
+        | ("TextServerExtension", "font_clear_glyphs")
+        | ("TextServerExtension", "font_remove_glyph")
+        | ("TextServerExtension", "font_get_glyph_advance")
+        | ("TextServerExtension", "font_set_glyph_advance")
+        | ("TextServerExtension", "font_get_glyph_offset")
+        | ("TextServerExtension", "font_set_glyph_offset")
+        | ("TextServerExtension", "font_get_glyph_size")
+        | ("TextServerExtension", "font_set_glyph_size")
+        | ("TextServerExtension", "font_get_glyph_uv_rect")
+        | ("TextServerExtension", "font_set_glyph_uv_rect")
+        | ("TextServerExtension", "font_get_glyph_texture_idx")
+        | ("TextServerExtension", "font_set_glyph_texture_idx")
+        | ("TextServerExtension", "font_get_glyph_texture_rid")
+        | ("TextServerExtension", "font_get_glyph_texture_size")
+        | ("TextServerExtension", "font_get_glyph_index")
+        | ("TextServerExtension", "font_get_char_from_glyph_index")
+        | ("TextServerExtension", "font_has_char")
+        | ("TextServerExtension", "font_get_supported_chars")
+        | ("TextServerExtension", "font_draw_glyph")
+        | ("TextServerExtension", "font_draw_glyph_outline")
+        | ("TextServerExtension", "create_shaped_text")
+        | ("TextServerExtension", "shaped_text_clear")
+        | ("TextServerExtension", "shaped_text_add_string")
+        | ("TextServerExtension", "shaped_text_add_object")
+        | ("TextServerExtension", "shaped_text_resize_object")
+        | ("TextServerExtension", "shaped_get_span_count")
+        | ("TextServerExtension", "shaped_get_span_meta")
+        | ("TextServerExtension", "shaped_set_span_update_font")
+        | ("TextServerExtension", "shaped_text_substr")
+        | ("TextServerExtension", "shaped_text_get_parent")
+        | ("TextServerExtension", "shaped_text_shape")
+        | ("TextServerExtension", "shaped_text_is_ready")
+        | ("TextServerExtension", "shaped_text_get_glyphs")
+        | ("TextServerExtension", "shaped_text_sort_logical")
+        | ("TextServerExtension", "shaped_text_get_glyph_count")
+        | ("TextServerExtension", "shaped_text_get_range")
+        | ("TextServerExtension", "shaped_text_get_trim_pos")
+        | ("TextServerExtension", "shaped_text_get_ellipsis_pos")
+        | ("TextServerExtension", "shaped_text_get_ellipsis_glyphs")
+        | ("TextServerExtension", "shaped_text_get_ellipsis_glyph_count")
+        | ("TextServerExtension", "shaped_text_get_objects")
+        | ("TextServerExtension", "shaped_text_get_object_rect")
+        | ("TextServerExtension", "shaped_text_get_object_range")
+        | ("TextServerExtension", "shaped_text_get_object_glyph")
+        | ("TextServerExtension", "shaped_text_get_size")
+        | ("TextServerExtension", "shaped_text_get_ascent")
+        | ("TextServerExtension", "shaped_text_get_descent")
+        | ("TextServerExtension", "shaped_text_get_width")
+        | ("TextServerExtension", "shaped_text_get_underline_position")
+        | ("TextServerExtension", "shaped_text_get_underline_thickness")
+        | ("AudioStreamPlayback", "mix")
+        | ("AudioStreamPlaybackResampled", "mix_resampled")
+        | ("AudioStreamPlaybackResampled", "get_stream_sampling_rate")
+        | ("AudioEffectInstance", "process")
+        | ("AudioEffect", "instantiate")
+        | ("PhysicsDirectBodyState2DExtension", _)
+        | ("PhysicsDirectBodyState3DExtension", _)
+        | ("PhysicsDirectSpaceState2DExtension", _)
+        | ("PhysicsDirectSpaceState3DExtension", _)
+        | ("PhysicsServer3DExtension", _)
+        | ("PhysicsServer2DExtension", _)
+        | ("EditorScript", "run")
+        | ("VideoStreamPlayback", "update")
+        | ("EditorFileSystemImportFormatSupportQuery", _)
+        | ("Mesh", _)
+        | ("Texture2D", "get_width")
+        | ("Texture2D", "get_height")
+        | ("Texture3D", _)
+        | ("TextureLayered", _)
+        | ("StyleBox", "draw")
+        | ("Material", "get_shader_rid")
+        | ("Material", "get_shader_mode")
+        | ("PacketPeerExtension", "get_available_packet_count")
+        | ("PacketPeerExtension", "get_max_packet_size")
+        | ("StreamPeerExtension", "get_available_bytes")
+        | ("WebRtcDataChannelExtension", "poll")
+        | ("WebRtcDataChannelExtension", "close")
+        | ("WebRtcDataChannelExtension", "set_write_mode")
+        | ("WebRtcDataChannelExtension", "get_write_mode")
+        | ("WebRtcDataChannelExtension", "was_string_packet")
+        | ("WebRtcDataChannelExtension", "get_ready_state")
+        | ("WebRtcDataChannelExtension", "get_label")
+        | ("WebRtcDataChannelExtension", "is_ordered")
+        | ("WebRtcDataChannelExtension", "get_id")
+        | ("WebRtcDataChannelExtension", "get_max_packet_life_time")
+        | ("WebRtcDataChannelExtension", "get_max_retransmits")
+        | ("WebRtcDataChannelExtension", "get_protocol")
+        | ("WebRtcDataChannelExtension", "is_negotiated")
+        | ("WebRtcDataChannelExtension", "get_buffered_amount")
+        | ("WebRtcDataChannelExtension", "get_available_packet_count")
+        | ("WebRtcDataChannelExtension", "get_max_packet_size")
+        | ("WebRtcPeerConnectionExtension", _)
+        | ("MultiplayerPeerExtension", "get_available_packet_count")
+        | ("MultiplayerPeerExtension", "get_max_packet_size")
+        | ("MultiplayerPeerExtension", "set_transfer_channel")
+        | ("MultiplayerPeerExtension", "get_transfer_channel")
+        | ("MultiplayerPeerExtension", "set_transfer_mode")
+        | ("MultiplayerPeerExtension", "get_transfer_mode")
+        | ("MultiplayerPeerExtension", "set_target_peer")
+        | ("MultiplayerPeerExtension", "get_packet_peer")
+        | ("MultiplayerPeerExtension", "get_packet_mode")
+        | ("MultiplayerPeerExtension", "get_packet_channel")
+        | ("MultiplayerPeerExtension", "is_server")
+        | ("MultiplayerPeerExtension", "poll")
+        | ("MultiplayerPeerExtension", "close")
+        | ("MultiplayerPeerExtension", "disconnect_peer")
+        | ("MultiplayerPeerExtension", "get_unique_id")
+        | ("MultiplayerPeerExtension", "get_connection_status") => true,
+
+        (_, _) => false,
+    }
+}
+
 #[rustfmt::skip]
 pub fn is_class_level_server(class_name: &str) -> bool {
     // Unclear on if some of these classes should be registered earlier than `Scene`:

diff --git a/godot-core/src/meta/signature.rs b/godot-core/src/meta/signature.rs
@@ -654,9 +654,9 @@ impl<'a> fmt::Display for CallContext<'a> {
 // Trace diagnostics for integration tests
 #[cfg(feature = "trace")]
 pub mod trace {
-    use crate::meta::CallContext;
+    use std::cell::Cell;
 
-    use super::sys::Global;
+    use crate::meta::CallContext;
 
     /// Stores information about the current call for diagnostic purposes.
     pub struct CallReport {
@@ -667,7 +667,7 @@ pub mod trace {
     }
 
     pub fn pop() -> CallReport {
-        let lock = TRACE.lock().take();
+        let lock = TRACE.take();
         // let th = std::thread::current().id();
         // println!("trace::pop [{th:?}]...");
 
@@ -688,8 +688,10 @@ pub mod trace {
             is_ptrcall: ptrcall,
         };
 
-        *TRACE.lock() = Some(report);
+        TRACE.set(Some(report));
     }
 
-    static TRACE: Global<Option<CallReport>> = Global::default();
+    thread_local! {
+        static TRACE: Cell<Option<CallReport>> = Cell::default();
+    }
 }