Skip to content

Commit 1e429c3

Browse files
committed
Export for i8, u32, etc: limit to valid inputs using @export_range
1 parent 6f3e671 commit 1e429c3

File tree

5 files changed

+89
-44
lines changed

5 files changed

+89
-44
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
use std::fmt;
99

1010
use godot_ffi as sys;
11-
use godot_ffi::{ffi_methods, ExtVariantType, GdextBuild, GodotFfi};
11+
use godot_ffi::{ffi_methods, ExtVariantType, GodotFfi};
1212

1313
use super::{GString, StringName};
1414
use crate::builtin::inner;
@@ -160,7 +160,7 @@ impl NodePath {
160160
pub fn subpath(&self, range: impl SignedRange) -> NodePath {
161161
let (from, exclusive_end) = range.signed();
162162
// Polyfill for bug https://github.com/godotengine/godot/pull/100954, fixed in 4.4.
163-
let begin = if GdextBuild::since_api("4.4") {
163+
let begin = if sys::GdextBuild::since_api("4.4") {
164164
from
165165
} else {
166166
let name_count = self.get_name_count() as i64;

godot-core/src/meta/property_info.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use crate::meta::{
1515
use crate::obj::{bounds, Bounds, EngineBitfield, EngineEnum, GodotClass};
1616
use crate::registry::class::get_dyn_property_hint_string;
1717
use crate::registry::property::{Export, Var};
18-
use crate::{classes, sys};
18+
use crate::{classes, godot_str, sys};
1919

2020
/// Describes a property in Godot.
2121
///
@@ -300,6 +300,13 @@ impl PropertyHintInfo {
300300
}
301301
}
302302

303+
pub fn type_name_range<T: GodotType + std::fmt::Display>(lower: T, upper: T) -> Self {
304+
Self {
305+
hint: PropertyHint::RANGE,
306+
hint_string: godot_str!("{lower},{upper}"), // also valid for <4.3.
307+
}
308+
}
309+
303310
/// Use for `#[var]` properties -- [`PROPERTY_HINT_ARRAY_TYPE`](PropertyHint::ARRAY_TYPE) with the type name as hint string.
304311
pub fn var_array_element<T: ArrayElement>() -> Self {
305312
Self {

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

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,8 @@ where
224224
/// Each function is named the same as the equivalent Godot annotation.
225225
/// For instance, `@export_range` in Godot is `fn export_range` here.
226226
pub mod export_info_functions {
227+
use std::fmt::Write;
228+
227229
use godot_ffi::VariantType;
228230

229231
use crate::builtin::GString;
@@ -232,7 +234,6 @@ pub mod export_info_functions {
232234
use crate::obj::EngineEnum;
233235
use crate::registry::property::Export;
234236
use crate::sys;
235-
use std::fmt::Write;
236237

237238
/// Turn a list of variables into a comma separated string containing only the identifiers corresponding
238239
/// to a true boolean variable.
@@ -539,6 +540,12 @@ mod export_impls {
539540
impl_property_by_godot_convert!(@property $Ty);
540541
};
541542

543+
($Ty:ty, limit_range) => {
544+
impl_property_by_godot_convert!(@property $Ty);
545+
impl_property_by_godot_convert!(@export_range $Ty);
546+
impl_property_by_godot_convert!(@builtin $Ty);
547+
};
548+
542549
($Ty:ty) => {
543550
impl_property_by_godot_convert!(@property $Ty);
544551
impl_property_by_godot_convert!(@export $Ty);
@@ -565,6 +572,14 @@ mod export_impls {
565572
}
566573
};
567574

575+
(@export_range $Ty:ty) => {
576+
impl Export for $Ty {
577+
fn export_hint() -> PropertyHintInfo {
578+
PropertyHintInfo::type_name_range(<$Ty>::MIN, <$Ty>::MAX)
579+
}
580+
}
581+
};
582+
568583
(@builtin $Ty:ty) => {
569584
impl BuiltinExport for $Ty {}
570585
}
@@ -613,16 +628,14 @@ mod export_impls {
613628
// then the property will just round the value or become inf.
614629
impl_property_by_godot_convert!(f32);
615630

616-
// Godot uses i64 internally for integers, and if Godot tries to pass an invalid integer into a property
617-
// accepting one of the below values then rust will panic. In the editor this will appear as the property
618-
// failing to be set to a value and an error printed in the console. During runtime this will crash the
619-
// program and print the panic from rust stating that the property cannot store the value.
620-
impl_property_by_godot_convert!(i32);
621-
impl_property_by_godot_convert!(i16);
622-
impl_property_by_godot_convert!(i8);
623-
impl_property_by_godot_convert!(u32);
624-
impl_property_by_godot_convert!(u16);
625-
impl_property_by_godot_convert!(u8);
631+
// 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
632+
// can only set the respective values. If values are assigned in other ways (e.g. GDScript), a panic may occur, causing a Godot error.
633+
impl_property_by_godot_convert!(i32, limit_range);
634+
impl_property_by_godot_convert!(i16, limit_range);
635+
impl_property_by_godot_convert!(i8, limit_range);
636+
impl_property_by_godot_convert!(u32, limit_range);
637+
impl_property_by_godot_convert!(u16, limit_range);
638+
impl_property_by_godot_convert!(u8, limit_range);
626639

627640
// Callables and Signals are useless when exported to the editor, so we only need to make them available as
628641
// properties.

itest/rust/build.rs

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,21 @@ use quote::{format_ident, quote};
1313

1414
type IoResult = std::io::Result<()>;
1515

16+
#[derive(Eq, PartialEq)]
17+
enum ExportKind {
18+
NoExport,
19+
Export,
20+
ExportRange { min: i64, max: i64 },
21+
}
22+
1623
struct Input {
1724
ident: String,
1825
gdscript_ty: &'static str,
1926
gdscript_val: &'static str,
2027
rust_ty: TokenStream,
2128
rust_val: TokenStream,
2229
is_property: bool,
23-
is_exportable: bool,
30+
export_kind: ExportKind,
2431
initializer: Option<TokenStream>,
2532
extra: TokenStream,
2633
}
@@ -34,7 +41,7 @@ macro_rules! pushs {
3441
$gdscript_val:expr,
3542
$rust_val:expr,
3643
$property:expr,
37-
$export:expr,
44+
$export_kind:expr,
3845
$initializer:expr
3946
$(; $($extra:tt)* )?
4047
) => {
@@ -48,21 +55,27 @@ macro_rules! pushs {
4855
rust_ty: quote! { $RustTy },
4956
rust_val: quote! { $rust_val },
5057
is_property: $property,
51-
is_exportable: $export,
58+
export_kind: $export_kind,
5259
initializer: $initializer,
5360
extra: quote! { $($($extra)*)? },
5461
});
5562
};
5663
}
5764

58-
/// Push simple GDScript expression, outside string
65+
/// Push simple GDScript expression, outside string.
5966
macro_rules! push {
6067
($inputs:ident; $GDScriptTy:expr, $RustTy:ty, $val:expr) => {
6168
push!($inputs; $GDScriptTy, $RustTy, $val, $val);
6269
};
6370

71+
($inputs:ident; $GDScriptTy:expr, $RustTy:ty, $val:expr, export_range) => {
72+
pushs!($inputs; $GDScriptTy, $RustTy, stringify!($val), $val, true,
73+
ExportKind::ExportRange { min: <$RustTy>::MIN as i64, max: <$RustTy>::MAX as i64 },
74+
None);
75+
};
76+
6477
($inputs:ident; $GDScriptTy:expr, $RustTy:ty, $gdscript_val:expr, $rust_val:expr) => {
65-
pushs!($inputs; $GDScriptTy, $RustTy, stringify!($gdscript_val), $rust_val, true, true, None);
78+
pushs!($inputs; $GDScriptTy, $RustTy, stringify!($gdscript_val), $rust_val, true, ExportKind::Export, None);
6679
};
6780
}
6881

@@ -77,7 +90,7 @@ macro_rules! push_newtype {
7790

7891
(@s $inputs:ident; $GDScriptTy:expr, $name:ident($T:ty), $gdscript_val:expr, $rust_val:expr) => {
7992
pushs!(
80-
$inputs; $GDScriptTy, $name, $gdscript_val, $rust_val, false, false, None;
93+
$inputs; $GDScriptTy, $name, $gdscript_val, $rust_val, false, NoExport, None;
8194

8295
#[derive(Clone, PartialEq, Debug)]
8396
pub struct $name($T);
@@ -106,41 +119,43 @@ macro_rules! push_newtype {
106119

107120
// Edit this to change involved types
108121
fn collect_inputs() -> Vec<Input> {
122+
use ExportKind::*;
123+
109124
let mut inputs = vec![];
110125

111126
// Scalar
112127
push!(inputs; int, i64, -922337203685477580);
113-
push!(inputs; int, i32, -2147483648);
114-
push!(inputs; int, u32, 4294967295);
115-
push!(inputs; int, i16, -32767);
116-
push!(inputs; int, u16, 65535);
117-
push!(inputs; int, i8, -128);
118-
push!(inputs; int, u8, 255);
128+
push!(inputs; int, i32, -2147483648, export_range);
129+
push!(inputs; int, u32, 4294967295, export_range);
130+
push!(inputs; int, i16, -32767, export_range);
131+
push!(inputs; int, u16, 65535, export_range);
132+
push!(inputs; int, i8, -128, export_range);
133+
push!(inputs; int, u8, 255, export_range);
119134
push!(inputs; float, f32, 12.5);
120135
push!(inputs; float, f64, 127.83156478);
121136
push!(inputs; bool, bool, true);
122137
push!(inputs; Color, Color, Color(0.7, 0.5, 0.3, 0.2), Color::from_rgba(0.7, 0.5, 0.3, 0.2));
123138
push!(inputs; String, GString, "hello", "hello".into());
124139
push!(inputs; StringName, StringName, &"hello", "hello".into());
125-
pushs!(inputs; NodePath, NodePath, r#"^"hello""#, "hello".into(), true, true, None);
140+
pushs!(inputs; NodePath, NodePath, r#"^"hello""#, "hello".into(), true, Export, None);
126141
push!(inputs; Vector2, Vector2, Vector2(12.5, -3.5), Vector2::new(12.5, -3.5));
127142
push!(inputs; Vector3, Vector3, Vector3(117.5, 100.0, -323.25), Vector3::new(117.5, 100.0, -323.25));
128143
push!(inputs; Vector4, Vector4, Vector4(-18.5, 24.75, -1.25, 777.875), Vector4::new(-18.5, 24.75, -1.25, 777.875));
129144
push!(inputs; Vector2i, Vector2i, Vector2i(-2147483648, 2147483647), Vector2i::new(-2147483648, 2147483647));
130145
push!(inputs; Vector3i, Vector3i, Vector3i(-1, -2147483648, 2147483647), Vector3i::new(-1, -2147483648, 2147483647));
131146
push!(inputs; Vector4i, Vector4i, Vector4i(-1, -2147483648, 2147483647, 1000), Vector4i::new(-1, -2147483648, 2147483647, 100));
132-
pushs!(inputs; Callable, Callable, "Callable()", Callable::invalid(), true, false, Some(quote! { Callable::invalid() }));
133-
pushs!(inputs; Signal, Signal, "Signal()", Signal::invalid(), true, false, Some(quote! { Signal::invalid() }));
147+
pushs!(inputs; Callable, Callable, "Callable()", Callable::invalid(), true, NoExport, Some(quote! { Callable::invalid() }));
148+
pushs!(inputs; Signal, Signal, "Signal()", Signal::invalid(), true, NoExport, Some(quote! { Signal::invalid() }));
134149
push!(inputs; Rect2, Rect2, Rect2(), Rect2::default());
135150
push!(inputs; Rect2i, Rect2i, Rect2i(), Rect2i::default());
136151
push!(inputs; Transform2D, Transform2D, Transform2D(), Transform2D::default());
137-
pushs!(inputs; Plane, Plane, "Plane()", Plane::new(Vector3::new(1.0, 0.0, 0.0), 0.0), true, true, Some(quote! { Plane::new(Vector3::new(1.0, 0.0, 0.0), 0.0) }));
152+
pushs!(inputs; Plane, Plane, "Plane()", Plane::new(Vector3::new(1.0, 0.0, 0.0), 0.0), true, Export, Some(quote! { Plane::new(Vector3::new(1.0, 0.0, 0.0), 0.0) }));
138153
push!(inputs; Quaternion, Quaternion, Quaternion(), Quaternion::default());
139154
push!(inputs; AABB, Aabb, AABB(), Aabb::default());
140155
push!(inputs; Basis, Basis, Basis(), Basis::default());
141156
push!(inputs; Transform3D, Transform3D, Transform3D(), Transform3D::default());
142157
push!(inputs; Projection, Projection, Projection(), Projection::default());
143-
pushs!(inputs; RID, Rid, "RID()", Rid::Invalid, true, false, Some(quote! { Rid::Invalid }));
158+
pushs!(inputs; RID, Rid, "RID()", Rid::Invalid, true, NoExport, Some(quote! { Rid::Invalid }));
144159
push!(inputs; Node, Option<Gd<Node>>, null, None);
145160
push!(inputs; Resource, Option<Gd<Resource>>, null, None);
146161
push!(inputs; PackedByteArray, PackedByteArray, PackedByteArray(), PackedByteArray::new());
@@ -195,16 +210,16 @@ fn collect_inputs() -> Vec<Input> {
195210
pushs!(inputs; Dictionary, Dictionary,
196211
r#"{"key": 83, -3: Vector2(1, 2), 0.03: true}"#,
197212
vdict! { "key": 83, (-3): Vector2::new(1.0, 2.0), 0.03: true },
198-
true, true, None
213+
true, Export, None
199214
);
200215

201216
// Composite
202-
pushs!(inputs; int, InstanceId, "-1", InstanceId::from_i64(0xFFFFFFFFFFFFFFF), false, false, None);
217+
pushs!(inputs; int, InstanceId, "-1", InstanceId::from_i64(0xFFFFFFFFFFFFFFF), false, NoExport, None);
203218
// TODO: should `Variant` implement property?
204-
pushs!(inputs; Variant, Variant, "123", 123i64.to_variant(), false, false, None);
219+
pushs!(inputs; Variant, Variant, "123", 123i64.to_variant(), false, NoExport, None);
205220

206221
// EngineEnum
207-
pushs!(inputs; int, Error, "0", Error::OK, false, false, None);
222+
pushs!(inputs; int, Error, "0", Error::OK, false, NoExport, None);
208223

209224
inputs
210225
}
@@ -419,7 +434,7 @@ fn generate_property_template(inputs: &[Input]) -> PropertyTests {
419434
gdscript_ty,
420435
rust_ty,
421436
is_property,
422-
is_exportable,
437+
export_kind,
423438
initializer,
424439
..
425440
} = input;
@@ -451,7 +466,7 @@ fn generate_property_template(inputs: &[Input]) -> PropertyTests {
451466
format!("var {var_array}: Array[{gdscript_ty}]"),
452467
]);
453468

454-
if *is_exportable {
469+
if *export_kind != ExportKind::NoExport {
455470
rust.extend([
456471
quote! {
457472
#[export]
@@ -461,8 +476,18 @@ fn generate_property_template(inputs: &[Input]) -> PropertyTests {
461476
quote! { #[export] #export_array: Array<#rust_ty> },
462477
]);
463478

479+
let export_single = match *export_kind {
480+
ExportKind::Export => {
481+
format!("@export var {export}: {gdscript_ty}")
482+
}
483+
ExportKind::ExportRange { min, max } => {
484+
format!("@export_range({min}, {max}) var {export}: {gdscript_ty}")
485+
}
486+
_ => unreachable!(),
487+
};
488+
464489
gdscript.extend([
465-
format!("@export var {export}: {gdscript_ty}"),
490+
export_single,
466491
format!("@export var {export_array}: Array[{gdscript_ty}]"),
467492
]);
468493
}

itest/rust/src/object_tests/property_template_test.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,19 @@ fn property_template_test(ctx: &TestContext) {
5656
continue;
5757
};
5858

59-
let mut rust_usage = rust_prop.at("usage").to::<i64>();
59+
let mut rust_usage = rust_prop.at("usage").to::<PropertyUsageFlags>();
6060

61-
// The GDSscript variables are script variables, and so have `PROPERTY_USAGE_SCRIPT_VARIABLE` set.
61+
// The GDScript variables are script variables, and so have `PROPERTY_USAGE_SCRIPT_VARIABLE` set.
6262
// Before 4.3, `PROPERTY_USAGE_SCRIPT_VARIABLE` did the same thing as `PROPERTY_USAGE_STORAGE` and
6363
// so GDScript didn't set both if it didn't need to.
6464
if GdextBuild::before_api("4.3") {
65-
if rust_usage == PropertyUsageFlags::STORAGE.ord() as i64 {
66-
rust_usage = PropertyUsageFlags::SCRIPT_VARIABLE.ord() as i64
65+
if rust_usage == PropertyUsageFlags::STORAGE {
66+
rust_usage = PropertyUsageFlags::SCRIPT_VARIABLE
6767
} else {
68-
rust_usage |= PropertyUsageFlags::SCRIPT_VARIABLE.ord() as i64;
68+
rust_usage |= PropertyUsageFlags::SCRIPT_VARIABLE;
6969
}
7070
} else {
71-
rust_usage |= PropertyUsageFlags::SCRIPT_VARIABLE.ord() as i64;
71+
rust_usage |= PropertyUsageFlags::SCRIPT_VARIABLE;
7272
}
7373

7474
rust_prop.set("usage", rust_usage);

0 commit comments

Comments
 (0)