Merge pull request #1340 from godot-rust/qol/todos

Remove deprecated symbols, other small cleanups

Author: Bromeon

godot-codegen/src/conv/type_conversions.rs

@@ -308,9 +308,7 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream {
                 _ => panic!("null not representable in target type {ty:?}"),
             }
         }
-        // empty string appears only for Callable/Rid in 4.0; default ctor syntax in 4.1+
-        // TODO(v0.4): check if we can remove ""
-        "" | "RID()" | "Callable()" if !is_inner => {
+        "RID()" | "Callable()" if !is_inner => {
             return match ty {
                 RustTy::BuiltinIdent { ty: ident, .. } if ident == "Rid" => quote! { Rid::Invalid },
                 RustTy::BuiltinIdent { ty: ident, .. } if ident == "Callable" => {

godot-codegen/src/generator/enums.rs

@@ -268,7 +268,7 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T
             }
         });
 
-        let str_functions = make_enum_str_functions(enum_);
+        let str_functions = make_enum_as_str(enum_);
         let values_and_constants_functions = make_enum_values_and_constants_functions(enum_);
 
         quote! {
@@ -296,7 +296,7 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T
         // However, those with masks don't have strict validation when marshalling from integers, and a Debug repr which includes the mask.
 
         let unique_ords = enum_.unique_ords().expect("self is an enum");
-        let str_functions = make_enum_str_functions(enum_);
+        let str_functions = make_enum_as_str(enum_);
         let values_and_constants_functions = make_enum_values_and_constants_functions(enum_);
 
         // We can technically check against all possible mask values, remove each mask, and then verify it's a valid base-enum value.
@@ -391,49 +391,10 @@ fn make_all_constants_function(enum_: &Enum) -> TokenStream {
     }
 }
 
-/// Creates the `as_str` and `godot_name` implementations for the enum.
-fn make_enum_str_functions(enum_: &Enum) -> TokenStream {
+/// Creates the `as_str()` implementation for the enum.
+fn make_enum_as_str(enum_: &Enum) -> TokenStream {
     let as_str_enumerators = make_enum_to_str_cases(enum_);
 
-    // Only enumerations with different godot names are specified.
-    // `as_str` is called for the rest of them.
-    let godot_different_cases = {
-        let enumerators = enum_
-            .enumerators
-            .iter()
-            .filter(|enumerator| enumerator.name != enumerator.godot_name)
-            .map(|enumerator| {
-                let Enumerator {
-                    name, godot_name, ..
-                } = enumerator;
-                let godot_name_str = godot_name.to_string();
-                quote! {
-                    Self::#name => #godot_name_str,
-                }
-            });
-
-        quote! {
-            #( #enumerators )*
-        }
-    };
-
-    let godot_name_match = if godot_different_cases.is_empty() {
-        // If empty, all the Rust names match the Godot ones.
-        // Remove match statement to avoid `clippy::match_single_binding`.
-        quote! {
-            self.as_str()
-        }
-    } else {
-        quote! {
-            // Many enums have duplicates, thus allow unreachable.
-            #[allow(unreachable_patterns)]
-            match *self {
-                #godot_different_cases
-                _ => self.as_str(),
-            }
-        }
-    };
-
     quote! {
         #[inline]
         fn as_str(&self) -> &'static str {
@@ -444,10 +405,6 @@ fn make_enum_str_functions(enum_: &Enum) -> TokenStream {
                 _ => "",
             }
         }
-
-        fn godot_name(&self) -> &'static str {
-            #godot_name_match
-        }
     }
 }
 

godot-codegen/src/generator/extension_interface.rs

@@ -12,7 +12,7 @@ use proc_macro2::{Ident, Literal, TokenStream};
 use quote::quote;
 use regex::Regex;
 
-use crate::util::ident;
+use crate::util::{ident, make_load_safety_doc};
 use crate::SubmitFn;
 
 pub fn generate_sys_interface_file(
@@ -77,15 +77,14 @@ fn generate_proc_address_funcs(h_path: &Path) -> TokenStream {
     }
 
     // Do not derive Copy -- even though the struct is bitwise-copyable, this is rarely needed and may point to an error.
+    let safety_doc = make_load_safety_doc();
     let code = quote! {
         pub struct GDExtensionInterface {
             #( #fptr_decls )*
         }
 
         impl GDExtensionInterface {
-            // TODO: Figure out the right safety preconditions. This currently does not have any because incomplete safety docs
-            // can cause issues with people assuming they are sufficient.
-            #[allow(clippy::missing_safety_doc)]
+            #safety_doc
             pub(crate) unsafe fn load(
                 get_proc_address: crate::GDExtensionInterfaceGetProcAddress,
             ) -> Self {

godot-codegen/src/generator/method_tables.rs

@@ -16,7 +16,7 @@ use crate::models::domain::{
     BuiltinClass, BuiltinMethod, BuiltinVariant, Class, ClassCodegenLevel, ClassLike, ClassMethod,
     ExtensionApi, FnDirection, Function, TyName,
 };
-use crate::util::ident;
+use crate::util::{ident, make_load_safety_doc};
 use crate::{conv, generator, special_cases, util};
 
 pub fn make_builtin_lifecycle_table(api: &ExtensionApi) -> TokenStream {
@@ -276,6 +276,7 @@ fn make_named_method_table(info: NamedMethodTable) -> TokenStream {
 
     // Assumes that both decls and inits already have a trailing comma.
     // This is necessary because some generators emit multiple lines (statements) per element.
+    let safety_doc = make_load_safety_doc();
     quote! {
         #imports
 
@@ -288,9 +289,7 @@ fn make_named_method_table(info: NamedMethodTable) -> TokenStream {
             pub const CLASS_COUNT: usize = #class_count;
             pub const METHOD_COUNT: usize = #method_count;
 
-            // TODO: Figure out the right safety preconditions. This currently does not have any because incomplete safety docs
-            // can cause issues with people assuming they are sufficient.
-            #[allow(clippy::missing_safety_doc)]
+            #safety_doc
             pub unsafe fn load(
                 #ctor_parameters
             ) -> Self {
@@ -374,6 +373,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream {
 
     // Assumes that inits already have a trailing comma.
     // This is necessary because some generators emit multiple lines (statements) per element.
+    let safety_doc = make_load_safety_doc();
     quote! {
         #imports
 
@@ -387,9 +387,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream {
             pub const CLASS_COUNT: usize = #class_count;
             pub const METHOD_COUNT: usize = #method_count;
 
-            // TODO: Figure out the right safety preconditions. This currently does not have any because incomplete safety docs
-            // can cause issues with people assuming they are sufficient.
-            #[allow(clippy::missing_safety_doc)]
+            #safety_doc
             #unused_attr
             pub unsafe fn load(
                 #ctor_parameters
@@ -440,6 +438,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream {
 
     // Assumes that inits already have a trailing comma.
     // This is necessary because some generators emit multiple lines (statements) per element.
+    let safety_doc = make_load_safety_doc();
     quote! {
         #imports
         use crate::StringCache;
@@ -462,9 +461,7 @@ fn make_method_table(info: IndexedMethodTable) -> TokenStream {
             pub const CLASS_COUNT: usize = #class_count;
             pub const METHOD_COUNT: usize = #method_count;
 
-            // TODO: Figure out the right safety preconditions. This currently does not have any because incomplete safety docs
-            // can cause issues with people assuming they are sufficient.
-            #[allow(clippy::missing_safety_doc)]
+            #safety_doc
             #unused_attr
             pub unsafe fn load() -> Self {
                 // SAFETY: interface and lifecycle tables are initialized at this point, so we can get 'static references to them.

godot-codegen/src/util.rs

@@ -85,6 +85,14 @@ pub fn lifetime(s: &str) -> TokenStream {
     TokenStream::from_iter([tk_apostrophe, tk_lifetime])
 }
 
+pub fn make_load_safety_doc() -> TokenStream {
+    quote! {
+        /// # Safety
+        /// - Must be called exactly once during library initialization.
+        /// - All parameters (dependencies) must have been initialized and valid.
+    }
+}
+
 // This function is duplicated in godot-macros\src\util\mod.rs
 #[rustfmt::skip]
 pub fn safe_ident(s: &str) -> Ident {

godot-core/src/builtin/aabb.rs

@@ -28,8 +28,11 @@ use crate::builtin::{real, Plane, Vector3, Vector3Axis};
 /// [`Rect2`]: crate::builtin::Rect2
 /// [`Rect2i`]: crate::builtin::Rect2i
 ///
-/// # Godot docs
+/// # Soft invariants
+/// `Aabb` requires non-negative size for certain operations, which is validated only on a best-effort basis. Violations may
+/// cause panics in Debug mode. See also [_Builtin API design_](../__docs/index.html#6-public-fields-and-soft-invariants).
 ///
+/// # Godot docs
 /// [`AABB`](https://docs.godotengine.org/en/stable/classes/class_aabb.html)
 #[derive(Default, Copy, Clone, PartialEq, Debug)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -421,8 +424,7 @@ impl Aabb {
     ///
     /// Most functions will fail to give a correct result if the size is negative.
     #[inline]
-    /// TODO(v0.3): make private, change to debug_assert().
-    pub fn assert_nonnegative(self) {
+    fn assert_nonnegative(self) {
         assert!(
             self.size.x >= 0.0 && self.size.y >= 0.0 && self.size.z >= 0.0,
             "size {:?} is negative",

godot-core/src/builtin/callable.rs

@@ -615,7 +615,7 @@ mod custom_callable {
         #[allow(clippy::result_unit_err)] // TODO remove once there's a clear error type here.
         fn invoke(&mut self, args: &[&Variant]) -> Variant;
 
-        // TODO(v0.3): add object_id().
+        // TODO(v0.5): add object_id().
 
         /// Returns whether the callable is considered valid.
         ///

godot-core/src/builtin/collections/packed_array.rs

@@ -771,8 +771,6 @@ impl<T: PackedArrayElement + fmt::Display> fmt::Display for PackedArray<T> {
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Specific API for PackedByteArray
 
-impl_builtin_froms!(PackedByteArray; VariantArray => packed_byte_array_from_array);
-
 macro_rules! declare_encode_decode {
     // $Via could be inferred, but ensures we have the correct type expectations.
     ($Ty:ty, $bytes:literal, $encode_fn:ident, $decode_fn:ident, $Via:ty) => {

godot-core/src/builtin/plane.rs

@@ -23,8 +23,11 @@ use crate::builtin::{real, Vector3};
 /// unit length and will panic if this invariant is violated. This is not separately
 /// annotated for each method.
 ///
-/// # Godot docs
+/// # Soft invariants
+/// `Plane` requires that the normal vector has unit length for most operations, which is validated only on a best-effort basis. Violations may
+/// cause panics in Debug mode. See also [_Builtin API design_](../__docs/index.html#6-public-fields-and-soft-invariants).
 ///
+/// # Godot docs
 /// [Plane (stable)](https://docs.godotengine.org/en/stable/classes/class_plane.html)
 #[derive(Copy, Clone, PartialEq, Debug)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]

godot-core/src/builtin/rect2.rs

@@ -27,8 +27,11 @@ use crate::builtin::{real, Rect2i, Side, Vector2};
 ///
 /// [`Aabb`]: crate::builtin::Aabb
 ///
-/// # Godot docs
+/// # Soft invariants
+/// `Rect2` requires non-negative size for certain operations, which is validated only on a best-effort basis. Violations may
+/// cause panics in Debug mode. See also [_Builtin API design_](../__docs/index.html#6-public-fields-and-soft-invariants).
 ///
+/// # Godot docs
 /// [`Rect2` (stable)](https://docs.godotengine.org/en/stable/classes/class_rect2.html)
 #[derive(Default, Copy, Clone, PartialEq, Debug)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]