Merge pull request #1320 from godot-rust/feature/export-limits

Numeric `#[export]` limits and type checks

Author: Bromeon

godot-core/src/builtin/string/node_path.rs

@@ -8,7 +8,7 @@
 use std::fmt;
 
 use godot_ffi as sys;
-use godot_ffi::{ffi_methods, ExtVariantType, GdextBuild, GodotFfi};
+use godot_ffi::{ffi_methods, ExtVariantType, GodotFfi};
 
 use super::{GString, StringName};
 use crate::builtin::inner;
@@ -160,7 +160,7 @@ impl NodePath {
     pub fn subpath(&self, range: impl SignedRange) -> NodePath {
         let (from, exclusive_end) = range.signed();
         // Polyfill for bug https://github.com/godotengine/godot/pull/100954, fixed in 4.4.
-        let begin = if GdextBuild::since_api("4.4") {
+        let begin = if sys::GdextBuild::since_api("4.4") {
             from
         } else {
             let name_count = self.get_name_count() as i64;

godot-core/src/meta/property_info.rs

@@ -13,7 +13,7 @@ use crate::meta::{element_godot_type_name, ArrayElement, ClassId, GodotType, Pac
 use crate::obj::{bounds, Bounds, EngineBitfield, EngineEnum, GodotClass};
 use crate::registry::class::get_dyn_property_hint_string;
 use crate::registry::property::{Export, Var};
-use crate::{classes, sys};
+use crate::{classes, godot_str, sys};
 
 /// Describes a property in Godot.
 ///
@@ -298,6 +298,13 @@ impl PropertyHintInfo {
         }
     }
 
+    pub fn type_name_range<T: GodotType + std::fmt::Display>(lower: T, upper: T) -> Self {
+        Self {
+            hint: PropertyHint::RANGE,
+            hint_string: godot_str!("{lower},{upper}"), // also valid for <4.3.
+        }
+    }
+
     /// Use for `#[var]` properties -- [`PROPERTY_HINT_ARRAY_TYPE`](PropertyHint::ARRAY_TYPE) with the type name as hint string.
     pub fn var_array_element<T: ArrayElement>() -> Self {
         Self {

godot-core/src/registry/property/mod.rs

@@ -17,6 +17,9 @@ use crate::global::PropertyHint;
 use crate::meta::{ClassId, FromGodot, GodotConvert, GodotType, PropertyHintInfo, ToGodot};
 use crate::obj::{EngineEnum, GodotClass};
 
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Child modules
+
 mod phantom_var;
 
 pub use phantom_var::PhantomVar;
@@ -229,18 +232,22 @@ where
 /// Each function is named the same as the equivalent Godot annotation.  
 /// For instance, `@export_range` in Godot is `fn export_range` here.
 pub mod export_info_functions {
+    use std::fmt;
+    use std::fmt::Write;
+
     use godot_ffi::VariantType;
 
     use crate::builtin::GString;
     use crate::global::PropertyHint;
     use crate::meta::{GodotType, PropertyHintInfo, PropertyInfo};
     use crate::obj::EngineEnum;
     use crate::registry::property::Export;
+    use crate::{godot_str, sys};
 
     /// Turn a list of variables into a comma separated string containing only the identifiers corresponding
     /// to a true boolean variable.
     macro_rules! comma_separate_boolean_idents {
-        ($( $ident:ident),* $(,)?) => {
+        ($( $ident:ident ),* $(,)?) => {
             {
                 let mut strings = Vec::new();
 
@@ -261,24 +268,26 @@ pub mod export_info_functions {
     /// You'll never call this function itself, but will instead use the macro `#[export(range=(...))]`, as below.  The syntax is
     /// very similar to Godot's [`@export_range`](https://docs.godotengine.org/en/stable/classes/class_%40gdscript.html#class-gdscript-annotation-export-range).
     /// `min`, `max`, and `step` are `f32` positional arguments, with `step` being optional and defaulting to `1.0`.  The rest of
-    /// the arguments can be written in any order.  The symbols of type `bool` just need to have those symbols written, and those of type `Option<T>` will be written as `{KEY}={VALUE}`, e.g. `suffix="px"`.
+    /// the arguments can be written in any order.  The symbols of type `bool` just need to have those symbols written, and those of type
+    /// `Option<T>` will be written as `{KEY}={VALUE}`, e.g. `suffix="px"`.
     ///
     /// ```
     /// # use godot::prelude::*;
     /// #[derive(GodotClass)]
     /// #[class(init, base=Node)]
     /// struct MyClassWithRangedValues {
-    ///     #[export(range=(0.0, 400.0, 1.0, or_greater, suffix="px"))]
+    ///     #[export(range=(0, 400, 1, or_greater, suffix="px"))]
     ///     icon_width: i32,
+    ///
     ///     #[export(range=(-180.0, 180.0, degrees))]
     ///     angle: f32,
     /// }
     /// ```
     #[allow(clippy::too_many_arguments)]
-    pub fn export_range(
-        min: f64,
-        max: f64,
-        step: Option<f64>,
+    pub fn export_range<T: Export + fmt::Display>(
+        min: T,
+        max: T,
+        step: Option<T>,
         or_greater: bool,
         or_less: bool,
         exp: bool,
@@ -307,10 +316,10 @@ pub mod export_info_functions {
 
         let mut hint_string = hint_beginning;
         if !rest.is_empty() {
-            hint_string.push_str(&format!(",{rest}"));
+            write!(hint_string, ",{rest}").unwrap();
         }
         if let Some(suffix) = suffix {
-            hint_string.push_str(&format!(",suffix:{suffix}"));
+            write!(hint_string, ",suffix:{suffix}").unwrap();
         }
 
         PropertyHintInfo {
@@ -319,6 +328,7 @@ pub mod export_info_functions {
         }
     }
 
+    // See also godot-macros > c_style_enum.rs; some code duplication.
     #[doc(hidden)]
     pub struct ExportValueWithKey<T> {
         variant: String,
@@ -339,23 +349,19 @@ pub mod export_info_functions {
         where
             for<'a> &'a V: Into<Self>,
         {
-            let values = values
-                .iter()
-                .map(|v| v.into().as_hint_string())
-                .collect::<Vec<_>>();
-
-            values.join(",")
+            sys::join_with(values.iter(), ",", |v| (*v).into().as_hint_string())
         }
     }
 
+    /// Allows syntax `("name", None)` and `("name", Some(10))`.
     impl<T, S> From<&(S, Option<T>)> for ExportValueWithKey<T>
     where
         T: Clone,
         S: AsRef<str>,
     {
         fn from((variant, key): &(S, Option<T>)) -> Self {
             Self {
-                variant: variant.as_ref().into(),
+                variant: variant.as_ref().to_string(),
                 key: key.clone(),
             }
         }
@@ -493,14 +499,14 @@ pub mod export_info_functions {
 
         PropertyHintInfo {
             hint: PropertyHint::TYPE_STRING,
-            hint_string: GString::from(&format!("{hint_string}:{filter}")),
+            hint_string: godot_str!("{hint_string}:{filter}"),
         }
     }
 
-    pub fn export_placeholder<S: AsRef<str>>(placeholder: S) -> PropertyHintInfo {
+    pub fn export_placeholder(placeholder: &str) -> PropertyHintInfo {
         PropertyHintInfo {
             hint: PropertyHint::PLACEHOLDER_TEXT,
-            hint_string: GString::from(placeholder.as_ref()),
+            hint_string: GString::from(placeholder),
         }
     }
 
@@ -544,6 +550,12 @@ mod export_impls {
             impl_property_by_godot_convert!(@property $Ty);
         };
 
+        ($Ty:ty, limit_range) => {
+            impl_property_by_godot_convert!(@property $Ty);
+            impl_property_by_godot_convert!(@export_range $Ty);
+            impl_property_by_godot_convert!(@builtin $Ty);
+        };
+
         ($Ty:ty) => {
             impl_property_by_godot_convert!(@property $Ty);
             impl_property_by_godot_convert!(@export $Ty);
@@ -570,6 +582,14 @@ mod export_impls {
             }
         };
 
+        (@export_range $Ty:ty) => {
+            impl Export for $Ty {
+                fn export_hint() -> PropertyHintInfo {
+                    PropertyHintInfo::type_name_range(<$Ty>::MIN, <$Ty>::MAX)
+                }
+            }
+        };
+
         (@builtin $Ty:ty) => {
             impl BuiltinExport for $Ty {}
         }
@@ -618,16 +638,14 @@ mod export_impls {
     // then the property will just round the value or become inf.
     impl_property_by_godot_convert!(f32);
 
-    // Godot uses i64 internally for integers, and if Godot tries to pass an invalid integer into a property
-    // accepting one of the below values then rust will panic. In the editor this will appear as the property
-    // failing to be set to a value and an error printed in the console. During runtime this will crash the
-    // program and print the panic from rust stating that the property cannot store the value.
-    impl_property_by_godot_convert!(i32);
-    impl_property_by_godot_convert!(i16);
-    impl_property_by_godot_convert!(i8);
-    impl_property_by_godot_convert!(u32);
-    impl_property_by_godot_convert!(u16);
-    impl_property_by_godot_convert!(u8);
+    // Godot uses i64 internally for integers. We use the @export_range hints to limit the integers to a sub-range of i64, so the editor UI
+    // can only set the respective values. If values are assigned in other ways (e.g. GDScript), a panic may occur, causing a Godot error.
+    impl_property_by_godot_convert!(i32, limit_range);
+    impl_property_by_godot_convert!(i16, limit_range);
+    impl_property_by_godot_convert!(i8, limit_range);
+    impl_property_by_godot_convert!(u32, limit_range);
+    impl_property_by_godot_convert!(u16, limit_range);
+    impl_property_by_godot_convert!(u8, limit_range);
 
     // Callables and Signals are useless when exported to the editor, so we only need to make them available as
     // properties.

godot-ffi/src/toolbox.rs

@@ -177,10 +177,11 @@ where
         // String formatting by itself is an infallible operation.
         // Read more at: https://doc.rust-lang.org/stable/std/fmt/index.html#formatting-traits
         write!(&mut result, "{first}", first = format_elem(&first))
-            .expect("Formatter should not fail!");
+            .expect("formatter should not fail");
+
         for item in iter {
             write!(&mut result, "{sep}{item}", item = format_elem(&item))
-                .expect("Formatter should not fail!");
+                .expect("formatter should not fail");
         }
     }
     result

godot-macros/src/class/data_models/field_export.rs

@@ -12,16 +12,18 @@ use quote::quote;
 
 use crate::util::{ident, KvParser, ListParser};
 use crate::ParseResult;
-
 pub struct FieldExport {
     pub export_type: ExportType,
     pub span: Span,
 }
 
 impl FieldExport {
-    pub(crate) fn new_from_kv(parser: &mut KvParser) -> ParseResult<Self> {
+    pub(crate) fn new_from_kv(
+        parser: &mut KvParser,
+        field_ty: venial::TypeExpr,
+    ) -> ParseResult<Self> {
         let span = parser.span();
-        let export_type = ExportType::new_from_kv(parser)?;
+        let export_type = ExportType::new_from_kv(parser, field_ty)?;
         Ok(Self { export_type, span })
     }
 
@@ -65,6 +67,7 @@ pub enum ExportType {
     /// ### Property hints
     /// - `RANGE`
     Range {
+        field_ty: venial::TypeExpr,
         min: TokenStream,
         max: TokenStream,
         step: TokenStream,
@@ -168,13 +171,15 @@ impl ExportType {
     /// - `@export_{flags/enum}("elem1", "elem2:key2", ...)`
     ///   becomes
     ///   `#[export(flags/enum = (elem1, elem2 = key2, ...))]`
-    pub(crate) fn new_from_kv(parser: &mut KvParser) -> ParseResult<Self> {
+    pub(crate) fn new_from_kv(
+        parser: &mut KvParser,
+        field_ty: venial::TypeExpr,
+    ) -> ParseResult<Self> {
         if parser.handle_alone("storage")? {
             return Self::new_storage();
         }
-
         if let Some(list_parser) = parser.handle_list("range")? {
-            return Self::new_range_list(list_parser);
+            return Self::new_range_list(list_parser, field_ty);
         }
 
         if let Some(list_parser) = parser.handle_list("enum")? {
@@ -300,7 +305,7 @@ impl ExportType {
         Ok(Self::Storage)
     }
 
-    fn new_range_list(mut parser: ListParser) -> ParseResult<Self> {
+    fn new_range_list(mut parser: ListParser, field_ty: venial::TypeExpr) -> ParseResult<Self> {
         const FLAG_OPTIONS: [&str; 7] = [
             "or_greater",
             "or_less",
@@ -314,6 +319,7 @@ impl ExportType {
 
         let min = parser.next_expr()?;
         let max = parser.next_expr()?;
+
         // If there is a next element, and it is a literal, we take its tokens directly.
         let step = if parser.peek().is_some_and(|kv| kv.as_literal().is_ok()) {
             let value = parser
@@ -330,6 +336,7 @@ impl ExportType {
         loop {
             let key_maybe_value =
                 parser.next_allowed_key_optional_value(&FLAG_OPTIONS, &KV_OPTIONS)?;
+
             match key_maybe_value {
                 Some((option, None)) => {
                     flags.insert(option.to_string());
@@ -344,6 +351,7 @@ impl ExportType {
         parser.finish()?;
 
         Ok(Self::Range {
+            field_ty,
             min,
             max,
             step,
@@ -411,12 +419,19 @@ impl ExportType {
 }
 
 macro_rules! quote_export_func {
-    ($function_name:ident($($tt:tt)*)) => {
+    ($function_name:ident ($($tt:tt)*) ) => {
         Some(quote! {
             ::godot::register::property::export_info_functions::$function_name($($tt)*)
         })
     };
 
+    // Use [ ] for generic args due to parsing ambiguity with ::< > turbofish.
+    ($function_name:ident [ $($generic_args:tt)* ] ($($tt:tt)*) ) => {
+        Some(quote! {
+            ::godot::register::property::export_info_functions::$function_name::< $($generic_args)* >($($tt)*)
+        })
+    };
+
     // Passes in a previously declared local `type FieldType = ...` as first generic argument.
     // Doesn't work if function takes other generic arguments -- in that case it could be converted to a Type<...> parameter.
     ($function_name:ident < T > ($($tt:tt)*)) => {
@@ -434,6 +449,7 @@ impl ExportType {
             Self::Storage => quote_export_func! { export_storage() },
 
             Self::Range {
+                field_ty,
                 min,
                 max,
                 step,
@@ -451,9 +467,11 @@ impl ExportType {
                 } else {
                     quote! { None }
                 };
+
                 let export_func = quote_export_func! {
-                    export_range(#min, #max, #step, #or_greater, #or_less, #exp, #radians_as_degrees || #radians, #degrees, #hide_slider, #suffix)
+                    export_range [ #field_ty ] (#min, #max, #step, #or_greater, #or_less, #exp, #radians_as_degrees || #radians, #degrees, #hide_slider, #suffix)
                 }?;
+
                 let deprecation_warning = if *radians {
                     // For some reason, rustfmt formatting like this.  Probably a bug.
                     // See https://github.com/godot-rust/gdext/pull/783#discussion_r1669105958 and
@@ -465,6 +483,7 @@ impl ExportType {
                 } else {
                     quote! { #export_func }
                 };
+
                 Some(quote! {
                     #deprecation_warning
                 })

godot-macros/src/class/derive_godot_class.rs

@@ -680,7 +680,7 @@ fn parse_fields(
 
         // #[export]
         if let Some(mut parser) = KvParser::parse(&named_field.attributes, "export")? {
-            let export = FieldExport::new_from_kv(&mut parser)?;
+            let export = FieldExport::new_from_kv(&mut parser, named_field.ty.clone())?;
             field.export = Some(export);
             parser.finish()?;
         }

godot-macros/src/derive/data_models/c_style_enum.rs

@@ -122,6 +122,8 @@ impl CStyleEnum {
 
     /// Return a hint string for use with `PropertyHint::ENUM` where the variants are just kept as strings.
     pub fn to_string_hint(&self) -> TokenStream {
+        // See also export_info_functions::slice_as_hint_string(), some code duplication.
+
         let hint_string = self
             .enumerator_names
             .iter()