Skip to content

Commit 090843d

Browse files
committed
Implement correct property hints for @export_file etc. on Array<GString> + PackedStringArray
Also cause error if another field type is used.
1 parent 45d3c40 commit 090843d

File tree

4 files changed

+90
-10
lines changed

4 files changed

+90
-10
lines changed

godot-core/src/meta/property_info.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ use godot_ffi::VariantType;
2323
/// Keeps the actual allocated values (the `sys` equivalent only keeps pointers, which fall out of scope).
2424
#[derive(Debug, Clone)]
2525
// Note: is not #[non_exhaustive], so adding fields is a breaking change. Mostly used internally at the moment though.
26+
// Note: There was an idea of a high-level representation of the following, but it's likely easier and more efficient to use introspection
27+
// APIs like `is_array_of_elem()`, unless there's a real user-facing need.
28+
// pub(crate) enum SimplePropertyType {
29+
// Variant { ty: VariantType },
30+
// Array { elem_ty: VariantType },
31+
// Object { class_name: ClassName },
32+
// }
2633
pub struct PropertyInfo {
2734
/// Which type this property has.
2835
///
@@ -134,6 +141,21 @@ impl PropertyInfo {
134141
}
135142
}
136143

144+
// ------------------------------------------------------------------------------------------------------------------------------------------
145+
// Introspection API -- could be made public in the future
146+
147+
pub(crate) fn is_array_of_elem<T>(&self) -> bool
148+
where
149+
T: ArrayElement,
150+
{
151+
self.variant_type == VariantType::ARRAY
152+
&& self.hint_info.hint == PropertyHint::ARRAY_TYPE
153+
&& self.hint_info.hint_string == T::Via::godot_type_name().into()
154+
}
155+
156+
// ------------------------------------------------------------------------------------------------------------------------------------------
157+
// FFI conversion functions
158+
137159
/// Converts to the FFI type. Keep this object allocated while using that!
138160
pub fn property_sys(&self) -> sys::GDExtensionPropertyInfo {
139161
use crate::obj::EngineBitfield as _;

godot-core/src/meta/traits.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@ pub trait GodotType: GodotConvert<Via = Self> + sealed::Sealed + Sized + 'static
118118
))
119119
}
120120

121+
/// Returns a string representation of the Godot type name, as it is used in several property hint contexts.
122+
///
123+
/// Examples:
124+
/// - `MyClass` for objects
125+
/// - `StringName`, `AABB` or `int` for builtins
126+
/// - `Array` for arrays
121127
#[doc(hidden)]
122128
fn godot_type_name() -> String;
123129

@@ -165,7 +171,9 @@ pub trait ArrayElement: ToGodot + FromGodot + sealed::Sealed + meta::ParamType {
165171
// Note: several indirections in ArrayElement and the global `element_*` functions go through `GodotConvert::Via`,
166172
// to not require Self: GodotType. What matters is how array elements map to Godot on the FFI level (GodotType trait).
167173

168-
/// Returns the representation of this type as a type string.
174+
/// Returns the representation of this type as a type string, e.g. `"4:"` for string, or `"24:34/MyClass"` for objects.
175+
///
176+
/// (`4` and `24` are variant type ords; `34` is `PropertyHint::NODE_TYPE` ord).
169177
///
170178
/// Used for elements in arrays (the latter despite `ArrayElement` not having a direct relation).
171179
///

godot-core/src/registry/property.rs

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,10 @@ where
226226
pub mod export_info_functions {
227227
use crate::builtin::GString;
228228
use crate::global::PropertyHint;
229-
use crate::meta::PropertyHintInfo;
229+
use crate::meta::{GodotType, PropertyHintInfo, PropertyInfo};
230+
use crate::obj::EngineEnum;
230231
use crate::registry::property::Export;
232+
use godot_ffi::VariantType;
231233

232234
/// Turn a list of variables into a comma separated string containing only the identifiers corresponding
233235
/// to a true boolean variable.
@@ -416,19 +418,67 @@ pub mod export_info_functions {
416418
is_global: bool,
417419
filter: impl AsRef<str>,
418420
) -> PropertyHintInfo {
421+
let field_ty = T::Via::property_info("");
419422
let filter = filter.as_ref();
420423
debug_assert!(is_file || filter.is_empty()); // Dir never has filter.
421424

425+
export_file_or_dir_inner(&field_ty, is_file, is_global, filter)
426+
}
427+
428+
pub fn export_file_or_dir_inner(
429+
field_ty: &PropertyInfo,
430+
is_file: bool,
431+
is_global: bool,
432+
filter: &str,
433+
) -> PropertyHintInfo {
422434
let hint = match (is_file, is_global) {
423435
(true, true) => PropertyHint::GLOBAL_FILE,
424436
(true, false) => PropertyHint::FILE,
425437
(false, true) => PropertyHint::GLOBAL_DIR,
426438
(false, false) => PropertyHint::DIR,
427439
};
428440

441+
// Returned value depends on field type.
442+
match field_ty.variant_type {
443+
// GString field:
444+
// { "type": 4, "hint": 13, "hint_string": "*.png" }
445+
VariantType::STRING => PropertyHintInfo {
446+
hint,
447+
hint_string: GString::from(filter),
448+
},
449+
450+
// Array<GString> or PackedStringArray field:
451+
// { "type": 28, "hint": 23, "hint_string": "4/13:*.png" }
452+
VariantType::PACKED_STRING_ARRAY => to_string_array_hint(hint, filter),
453+
VariantType::ARRAY if field_ty.is_array_of_elem::<GString>() => {
454+
to_string_array_hint(hint, filter)
455+
}
456+
457+
_ => {
458+
// E.g. `global_file`.
459+
let attribute_name = hint.as_str().to_lowercase();
460+
461+
// TODO nicer error handling.
462+
// Compile time may be difficult (at least without extra traits... maybe const fn?). But at least more context info, field name etc.
463+
panic!(
464+
"#[export({attribute_name})] only supports GString, Array<String> or PackedStringArray field types\n\
465+
encountered: {field_ty:?}"
466+
);
467+
}
468+
}
469+
}
470+
471+
/// For `Array<GString>` and `PackedStringArray` fields using one of the `@export[_global]_{file|dir}` annotations.
472+
///
473+
/// Formats: `"4/13:"`, `"4/15:*.png"`, ...
474+
fn to_string_array_hint(hint: PropertyHint, filter: &str) -> PropertyHintInfo {
475+
let variant_ord = VariantType::STRING.ord(); // "4"
476+
let hint_ord = hint.ord();
477+
let hint_string = format!("{variant_ord}/{hint_ord}");
478+
429479
PropertyHintInfo {
430-
hint,
431-
hint_string: GString::from(filter),
480+
hint: PropertyHint::TYPE_STRING,
481+
hint_string: format!("{hint_string}:{filter}").into(),
432482
}
433483
}
434484

@@ -603,9 +653,9 @@ pub(crate) fn builtin_type_string<T: GodotType>() -> String {
603653

604654
// Godot 4.3 changed representation for type hints, see https://github.com/godotengine/godot/pull/90716.
605655
if sys::GdextBuild::since_api("4.3") {
606-
format!("{}:", variant_type.sys())
656+
format!("{}:", variant_type.ord())
607657
} else {
608-
format!("{}:{}", variant_type.sys(), T::godot_type_name())
658+
format!("{}:{}", variant_type.ord(), T::godot_type_name())
609659
}
610660
}
611661

itest/rust/src/object_tests/property_template_test.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use crate::framework::TestContext;
2121

2222
use crate::register_tests::gen_ffi::PropertyTestsRust;
2323

24-
#[itest(focus)]
24+
#[itest]
2525
fn property_template_test(ctx: &TestContext) {
2626
let rust_properties = PropertyTestsRust::new_alloc();
2727
let gdscript_properties = ctx.property_tests.clone();
@@ -92,9 +92,9 @@ fn property_template_test(ctx: &TestContext) {
9292
errors.push(format!(
9393
"mismatch in property {name}:\n GDScript: {gdscript_prop:?}\n Rust: {rust_prop:?}"
9494
));
95-
} else {
96-
println!("matching property {name}\n GDScript: {gdscript_prop:?}\n Rust: {rust_prop:?}");
97-
}
95+
} /*else {
96+
println!("matching property {name}\n GDScript: {gdscript_prop:?}\n Rust: {rust_prop:?}");
97+
}*/
9898
}
9999

100100
assert!(

0 commit comments

Comments
 (0)