Merge pull request #622 from godot-rust/qol/script-instance-cleanups

`ScriptInstance` cleanup + documentation

godot-codegen/src/generator/docs.rs

@@ -9,8 +9,8 @@
 //!
 //! Single module for documentation, rather than having it in each symbol-specific file, so it's easier to keep docs consistent.
 
-use crate::models::domain::ModName;
-use crate::models::domain::TyName;
+use crate::models::domain::{ModName, TyName};
+use crate::special_cases;
 use proc_macro2::Ident;
 
 pub fn make_class_doc(
@@ -49,6 +49,10 @@ pub fn make_class_doc(
 
     let trait_name = class_name.virtual_trait_name();
 
+    let notes = special_cases::get_class_extra_docs(class_name)
+        .map(|notes| format!("# Specific notes for this class\n\n{}", notes))
+        .unwrap_or_default();
+
     format!(
         "Godot class `{godot_ty}.`\n\n\
         \
@@ -59,24 +63,28 @@ pub fn make_class_doc(
         * [`{trait_name}`][crate::engine::{trait_name}]: virtual methods\n\
         {notify_line}\
         \n\n\
-        See also [Godot docs for `{godot_ty}`]({online_link}).\n\n",
+        See also [Godot docs for `{godot_ty}`]({online_link}).\n\n{notes}",
     )
 }
 
-pub fn make_virtual_trait_doc(class_name: &TyName) -> String {
+pub fn make_virtual_trait_doc(trait_name_str: &str, class_name: &TyName) -> String {
     let TyName { rust_ty, godot_ty } = class_name;
 
     let online_link = format!(
         "https://docs.godotengine.org/en/stable/classes/class_{}.html#methods",
         godot_ty.to_ascii_lowercase()
     );
 
+    let notes = special_cases::get_interface_extra_docs(trait_name_str)
+        .map(|notes| format!("# Specific notes for this interface\n\n{}", notes))
+        .unwrap_or_default();
+
     format!(
         "Virtual methods for class [`{rust_ty}`][crate::engine::{rust_ty}].\
         \n\n\
         These methods represent constructors (`init`) or callbacks invoked by the engine.\
         \n\n\
-        See also [Godot docs for `{godot_ty}` methods]({online_link}).\n\n"
+        See also [Godot docs for `{godot_ty}` methods]({online_link}).\n\n{notes}"
     )
 }
 

godot-codegen/src/generator/functions_common.rs

@@ -96,6 +96,11 @@ pub fn make_function_definition(
         make_vis(sig.is_private())
     };
 
+    // Functions are marked unsafe as soon as raw pointers are involved, irrespectively of whether they appear in parameter or return type
+    // position. In cases of virtual functions called by Godot, a returned pointer must be valid and of the expected type. It might be possible
+    // to only use `unsafe` for pointers in parameters (for outbound calls), and in return values (for virtual calls). Or technically more
+    // correct, make the entire trait unsafe as soon as one function can return pointers, but that's very unergonomic and non-local.
+    // Thus, let's keep things simple and more conservative.
     let (maybe_unsafe, safety_doc) = if let Some(safety_doc) = safety_doc {
         (quote! { unsafe }, safety_doc)
     } else if function_uses_pointers(sig) {

godot-codegen/src/generator/virtual_traits.rs

@@ -17,16 +17,16 @@ use quote::quote;
 pub fn make_virtual_methods_trait(
     class: &Class,
     all_base_names: &[TyName],
-    trait_name: &str,
+    trait_name_str: &str,
     notification_enum_name: &Ident,
     view: &ApiView,
 ) -> TokenStream {
-    let trait_name = ident(trait_name);
+    let trait_name = ident(trait_name_str);
 
     let virtual_method_fns = make_all_virtual_methods(class, all_base_names, view);
     let special_virtual_methods = special_virtual_methods(notification_enum_name);
 
-    let trait_doc = docs::make_virtual_trait_doc(class.name());
+    let trait_doc = docs::make_virtual_trait_doc(trait_name_str, class.name());
 
     quote! {
         #[doc = #trait_doc]

godot-codegen/src/special_cases/special_cases.rs

@@ -285,3 +285,26 @@ pub fn maybe_rename_virtual_method(rust_method_name: &str) -> &str {
         _ => rust_method_name,
     }
 }
+
+pub fn get_class_extra_docs(class_name: &TyName) -> Option<&'static str> {
+    match class_name.godot_ty.as_str() {
+        "FileAccess" => {
+            Some("The gdext library provides a higher-level abstraction, which should be preferred: [`GFile`][crate::engine::GFile].")
+        }
+        "ScriptExtension" => {
+            Some("Use this in combination with [`ScriptInstance`][crate::engine::ScriptInstance].")
+        }
+
+        _ => None,
+    }
+}
+
+pub fn get_interface_extra_docs(trait_name: &str) -> Option<&'static str> {
+    match trait_name {
+        "IScriptExtension" => {
+            Some("Use this in combination with [`ScriptInstance`][crate::engine::ScriptInstance].")
+        }
+
+        _ => None,
+    }
+}