From 364051cb0d279f054c755400ae0e615670dfc144 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Mon, 7 Jul 2025 23:12:31 +0200 Subject: [PATCH 1/6] Refactor: move enum MAX detection up into domain mapping --- godot-codegen/src/generator/enums.rs | 2 +- godot-codegen/src/models/domain/enums.rs | 13 +++++++++---- godot-codegen/src/models/domain_mapping.rs | 5 ++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/godot-codegen/src/generator/enums.rs b/godot-codegen/src/generator/enums.rs index 044a8080b..2bcc95b57 100644 --- a/godot-codegen/src/generator/enums.rs +++ b/godot-codegen/src/generator/enums.rs @@ -141,7 +141,7 @@ pub fn make_enum_definition_with( /// /// Returns `None` if `enum_` isn't an indexable enum. fn make_enum_index_impl(enum_: &Enum) -> Option { - let enum_max = enum_.find_index_enum_max()?; + let enum_max = enum_.max_index?; // Do nothing if enum isn't sequential with a MAX constant. let name = &enum_.name; Some(quote! { diff --git a/godot-codegen/src/models/domain/enums.rs b/godot-codegen/src/models/domain/enums.rs index d0237d5c2..c81d59dda 100644 --- a/godot-codegen/src/models/domain/enums.rs +++ b/godot-codegen/src/models/domain/enums.rs @@ -22,6 +22,8 @@ pub struct Enum { pub is_private: bool, pub is_exhaustive: bool, pub enumerators: Vec, + /// If the enum is sequential and has a `*_MAX` constant (Godot name), this is the index of it. + pub max_index: Option, } impl Enum { @@ -73,15 +75,18 @@ impl Enum { /// Returns the maximum index of an indexable enum. /// - /// Return `None` if `self` isn't an indexable enum. Meaning it is either a bitfield, or it is an enum that can't be used as an index. - pub fn find_index_enum_max(&self) -> Option { - if self.is_bitfield { + /// Returns `None` if this is a bitfield, or an enum that isn't sequential with a `*_MAX` enumerator. + pub fn find_index_enum_max_impl( + is_bitfield: bool, + enumerators: &[Enumerator], + ) -> Option { + if is_bitfield { return None; } // Sort by ordinal value. Allocates for every enum in the JSON, but should be OK (most enums are indexable). let enumerators = { - let mut enumerators = self.enumerators.clone(); + let mut enumerators = enumerators.to_vec(); enumerators.sort_by_key(|v| v.value.to_i64()); enumerators }; diff --git a/godot-codegen/src/models/domain_mapping.rs b/godot-codegen/src/models/domain_mapping.rs index 15175e551..bc5ffbf34 100644 --- a/godot-codegen/src/models/domain_mapping.rs +++ b/godot-codegen/src/models/domain_mapping.rs @@ -662,7 +662,7 @@ impl Enum { conv::make_enumerator_names(godot_class_name, &rust_enum_name, godot_enumerator_names) }; - let enumerators = json_enum + let enumerators: Vec = json_enum .values .iter() .zip(rust_enumerator_names) @@ -671,6 +671,8 @@ impl Enum { }) .collect(); + let max_index = Enum::find_index_enum_max_impl(is_bitfield, &enumerators); + Self { name: ident(&rust_enum_name), godot_name: godot_name.clone(), @@ -679,6 +681,7 @@ impl Enum { is_private, is_exhaustive, enumerators, + max_index, } } } From 6690631a86a0c2e3fc4bff81aa3b1a99df958688 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Mon, 7 Jul 2025 23:32:31 +0200 Subject: [PATCH 2/6] Slightly reformat #[itest(focus)] in comments, so it doesn't appear in search --- .github/composite/godot-itest/action.yml | 2 +- itest/rust/src/object_tests/dynamic_call_test.rs | 2 +- itest/rust/src/register_tests/constant_test.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/composite/godot-itest/action.yml b/.github/composite/godot-itest/action.yml index 434f779f8..4f46fed88 100644 --- a/.github/composite/godot-itest/action.yml +++ b/.github/composite/godot-itest/action.yml @@ -176,7 +176,7 @@ runs: # since it's not available on Windows, use taskkill in that case. # * exit: the terminated process would return 143, but this is more explicit and future-proof # - # --disallow-focus: fail if #[itest(focus)] is encountered, to prevent running only a few tests for full CI + # --disallow-focus: fail if #[itest (focus)] is encountered, to prevent running only a few tests for full CI run: | cd itest/godot echo "OUTCOME=itest" >> $GITHUB_ENV diff --git a/itest/rust/src/object_tests/dynamic_call_test.rs b/itest/rust/src/object_tests/dynamic_call_test.rs index d56961241..ebaa204a2 100644 --- a/itest/rust/src/object_tests/dynamic_call_test.rs +++ b/itest/rust/src/object_tests/dynamic_call_test.rs @@ -140,7 +140,7 @@ fn dynamic_call_parameter_mismatch() { obj.free(); } -// There seems to be a weird bug where running *only* this test with #[itest(focus)] causes panic, which then causes a +// There seems to be a weird bug where running *only* this test with #[itest (focus)] causes panic, which then causes a // follow-up failure of Gd::bind_mut(), preventing benchmarks from being run. Doesn't happen with #[itest], when running all. #[itest] fn dynamic_call_with_panic() { diff --git a/itest/rust/src/register_tests/constant_test.rs b/itest/rust/src/register_tests/constant_test.rs index f9d5969a8..916c26f9b 100644 --- a/itest/rust/src/register_tests/constant_test.rs +++ b/itest/rust/src/register_tests/constant_test.rs @@ -180,7 +180,7 @@ godot::sys::plugin_add!( macro_rules! test_enum_export { ( $class:ty, $enum_name:ident, [$($enumerators:ident),* $(,)?]; - // Include the `attr` here to, so we can easily do things like `#[itest(focus)]`. + // Include the `attr` here too, so we can easily do things like `#[itest (focus)]`. #$attr:tt fn $test_name:ident() { .. } ) => { From a2a08d2369cc9a4b046ca4fa5988507c79cd5e10 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 12 Jul 2025 13:24:14 +0200 Subject: [PATCH 3/6] EngineEnum: add values() + all_constants() functions --- godot-codegen/src/generator/enums.rs | 47 ++++++ godot-codegen/src/models/domain/enums.rs | 2 +- godot-core/src/builtin/vectors/vector_axis.rs | 21 +++ godot-core/src/meta/inspect.rs | 56 ++++++++ godot-core/src/meta/mod.rs | 1 + godot-core/src/obj/traits.rs | 52 ++++++- .../rust/src/engine_tests/engine_enum_test.rs | 135 +++++++++++++++++- itest/rust/src/object_tests/enum_test.rs | 2 +- 8 files changed, 307 insertions(+), 9 deletions(-) create mode 100644 godot-core/src/meta/inspect.rs diff --git a/godot-codegen/src/generator/enums.rs b/godot-codegen/src/generator/enums.rs index 2bcc95b57..24b91439f 100644 --- a/godot-codegen/src/generator/enums.rs +++ b/godot-codegen/src/generator/enums.rs @@ -13,6 +13,7 @@ use crate::models::domain::{Enum, Enumerator, EnumeratorValue, RustTy}; use crate::special_cases; use proc_macro2::TokenStream; use quote::{quote, ToTokens}; +use std::collections::HashSet; pub fn make_enums(enums: &[Enum], cfg_attributes: &TokenStream) -> TokenStream { let definitions = enums.iter().map(make_enum_definition); @@ -262,6 +263,7 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T }); let str_functions = make_enum_str_functions(enum_); + let values_and_constants_functions = make_enum_values_and_constants_functions(enum_); quote! { impl #engine_trait for #name { @@ -277,6 +279,7 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T } #str_functions + #values_and_constants_functions } } } else { @@ -288,6 +291,7 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T let unique_ords = enum_.unique_ords().expect("self is an enum"); let str_functions = make_enum_str_functions(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. // However, this is not forward compatible: if a new mask is added in a future API version, it wouldn't be removed, and the @@ -317,6 +321,49 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T } #str_functions + #values_and_constants_functions + } + } + } +} + +/// Creates both the `values()` and `all_constants()` implementations for the enum. +fn make_enum_values_and_constants_functions(enum_: &Enum) -> TokenStream { + let name = &enum_.name; + + let mut distinct_values = Vec::new(); + let mut all_constants = Vec::new(); + let mut seen_ordinals = HashSet::new(); + + for (index, enumerator) in enum_.enumerators.iter().enumerate() { + let constant = &enumerator.name; + let rust_name = enumerator.name.to_string(); + let godot_name = enumerator.godot_name.to_string(); + let ordinal = &enumerator.value; + + // all_constants() includes every constant unconditionally. + all_constants.push(quote! { + crate::meta::inspect::EnumConstant::new(#rust_name, #godot_name, #ordinal, #name::#constant) + }); + + // values() contains value only if distinct (first time seen) and not MAX. + if enum_.max_index != Some(index) && seen_ordinals.insert(ordinal.clone()) { + distinct_values.push(quote! { #name::#constant }); + } + } + + quote! { + fn values() -> &'static [Self] { + &[ + #( #distinct_values ),* + ] + } + + fn all_constants() -> &'static [crate::meta::inspect::EnumConstant<#name>] { + const { + &[ + #( #all_constants ),* + ] } } } diff --git a/godot-codegen/src/models/domain/enums.rs b/godot-codegen/src/models/domain/enums.rs index c81d59dda..57f369ff2 100644 --- a/godot-codegen/src/models/domain/enums.rs +++ b/godot-codegen/src/models/domain/enums.rs @@ -129,7 +129,7 @@ pub struct Enumerator { pub value: EnumeratorValue, } -#[derive(Clone)] +#[derive(Clone, Hash, Eq, PartialEq)] pub enum EnumeratorValue { Enum(i32), Bitfield(u64), diff --git a/godot-core/src/builtin/vectors/vector_axis.rs b/godot-core/src/builtin/vectors/vector_axis.rs index e47ffe262..051cc9dbe 100644 --- a/godot-core/src/builtin/vectors/vector_axis.rs +++ b/godot-core/src/builtin/vectors/vector_axis.rs @@ -54,6 +54,27 @@ macro_rules! impl_vector_axis_enum { )+ } } + + fn values() -> &'static [Self] { + // For vector axis enums, all values are distinct, so both are the same + &[ + $( $AxisEnum::$axis, )+ + ] + } + + fn all_constants() -> &'static [crate::meta::inspect::EnumConstant<$AxisEnum>] { + use crate::meta::inspect::EnumConstant; + const { &[ + $( + EnumConstant::new( + stringify!($axis), + concat!("AXIS_", stringify!($axis)), + $AxisEnum::$axis as i32, + $AxisEnum::$axis + ), + )+ + ] } + } } impl GodotConvert for $AxisEnum { diff --git a/godot-core/src/meta/inspect.rs b/godot-core/src/meta/inspect.rs new file mode 100644 index 000000000..9051a8a6a --- /dev/null +++ b/godot-core/src/meta/inspect.rs @@ -0,0 +1,56 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +//! Introspection metadata for Godot engine types. + +/// Metadata for a single enum constant. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct EnumConstant { + rust_name: &'static str, + godot_name: &'static str, + ord: i32, + value: T, +} + +impl EnumConstant { + /// Creates a new enum constant metadata entry. + pub(crate) const fn new( + rust_name: &'static str, + godot_name: &'static str, + ord: i32, + value: T, + ) -> Self { + Self { + rust_name, + godot_name, + ord, + value, + } + } + + /// Rust name of the enum variant, usually without Prefix (e.g. `"ESCAPE"` for `Key::ESCAPE`). + /// + /// This is returned by [`EngineEnum::as_str()`](crate::obj::EngineEnum::as_str()). + pub const fn rust_name(&self) -> &'static str { + self.rust_name + } + + /// Godot constant name (e.g. `"KEY_ESCAPE"` for `Key::ESCAPE`). + pub const fn godot_name(&self) -> &'static str { + self.godot_name + } + + /// Ordinal value of this enum variant. + pub const fn ord(&self) -> i32 { + self.ord + } + + /// The enum value itself. + pub const fn value(&self) -> T { + self.value + } +} diff --git a/godot-core/src/meta/mod.rs b/godot-core/src/meta/mod.rs index b9d132505..9efc49631 100644 --- a/godot-core/src/meta/mod.rs +++ b/godot-core/src/meta/mod.rs @@ -57,6 +57,7 @@ mod uniform_object_deref; pub(crate) mod sealed; pub mod error; +pub mod inspect; pub use args::*; pub use class_name::ClassName; diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index b0786cc15..787ecc387 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -8,6 +8,7 @@ use crate::builder::ClassBuilder; use crate::builtin::GString; use crate::init::InitLevel; +use crate::meta::inspect::EnumConstant; use crate::meta::ClassName; use crate::obj::{bounds, Base, BaseMut, BaseRef, Bounds, Gd}; #[cfg(since_api = "4.2")] @@ -187,15 +188,54 @@ pub trait EngineEnum: Copy { .unwrap_or_else(|| panic!("ordinal {ord} does not map to any enumerator")) } - // The name of the enumerator, as it appears in Rust. - // - // If the value does not match one of the known enumerators, the empty string is returned. + /// The name of the enumerator, as it appears in Rust. + /// + /// Note that **this may not match the Rust constant name!** In case of multiple constants with the same ordinal value, this method returns + /// the first one in the order of definition. For example, `LayoutDirection::LOCALE.as_str()` (ord 1) returns `"APPLICATION_LOCALE"`, because + /// that happens to be the first constant with ordinal `1`. See [`all_constants()`][Self::all_constants] for a more robust and general + /// approach to introspection of enum constants. + /// + /// If the value does not match one of the known enumerators, the empty string is returned. fn as_str(&self) -> &'static str; - // The equivalent name of the enumerator, as specified in Godot. - // - // If the value does not match one of the known enumerators, the empty string is returned. + /// The equivalent name of the enumerator, as specified in Godot. + /// + /// If the value does not match one of the known enumerators, the empty string is returned. fn godot_name(&self) -> &'static str; + + /// Returns a slice of distinct enum values. + /// + /// This excludes MAX constants and deduplicates aliases, providing only meaningful enum values. + /// + /// This enables iteration over distinct enum variants: + /// ```no_run + /// use godot::classes::window; + /// use godot::obj::EngineEnum; + /// + /// for mode in window::Mode::values() { + /// println!("* {}: {}", mode.as_str(), mode.ord()); + /// } + /// ``` + fn values() -> &'static [Self]; + + /// Returns metadata for all enum constants. + /// + /// This includes all constants as they appear in the enum definition, including duplicates and MAX constants. + /// + /// This enables complete introspection and debugging: + /// ```no_run + /// use godot::classes::window; + /// use godot::obj::EngineEnum; + /// + /// for constant in window::Mode::all_constants() { + /// println!("* {}: {} (ord: {})", + /// constant.rust_name(), + /// constant.godot_name(), + /// constant.ord() + /// ); + /// } + /// ``` + fn all_constants() -> &'static [EnumConstant]; } /// Auto-implemented for all engine-provided bitfields. diff --git a/itest/rust/src/engine_tests/engine_enum_test.rs b/itest/rust/src/engine_tests/engine_enum_test.rs index cbafefdbe..759709052 100644 --- a/itest/rust/src/engine_tests/engine_enum_test.rs +++ b/itest/rust/src/engine_tests/engine_enum_test.rs @@ -7,7 +7,8 @@ use crate::framework::itest; -use godot::global::{Key, KeyModifierMask}; +use godot::classes::{mesh, window}; +use godot::global::{InlineAlignment, Key, KeyModifierMask, Orientation}; use godot::obj::{EngineBitfield, EngineEnum}; #[itest] @@ -46,3 +47,135 @@ fn enum_with_masked_bitfield_from_ord() { // let back = Key::try_from_ord(31); // assert_eq!(back, None, "don't deserialize invalid bitmasked enum"); } + +#[itest] +fn enum_values_class() { + let expected_modes = [ + window::Mode::WINDOWED, + window::Mode::MINIMIZED, + window::Mode::MAXIMIZED, + window::Mode::FULLSCREEN, + window::Mode::EXCLUSIVE_FULLSCREEN, + ]; + + assert_eq!(window::Mode::values(), &expected_modes); +} + +#[itest] +fn enum_values_global() { + let expected_orientations = [Orientation::VERTICAL, Orientation::HORIZONTAL]; + + assert_eq!(Orientation::values(), &expected_orientations); +} + +#[itest] +fn enum_values_duplicates() { + // InlineAlignment has many duplicate ordinals, but values() should return only distinct ones + // The order matches the declaration order in the JSON API, not ordinal order + let expected = [ + (InlineAlignment::TOP_TO, 0, true), + (InlineAlignment::CENTER_TO, 1, true), + (InlineAlignment::BASELINE_TO, 3, true), + (InlineAlignment::BOTTOM_TO, 2, true), + (InlineAlignment::TO_TOP, 0, false), // duplicate of TOP_TO + (InlineAlignment::TO_CENTER, 4, true), + (InlineAlignment::TO_BASELINE, 8, true), + (InlineAlignment::TO_BOTTOM, 12, true), + (InlineAlignment::TOP, 0, false), // duplicate of TOP_TO + (InlineAlignment::CENTER, 5, true), + (InlineAlignment::BOTTOM, 14, true), + (InlineAlignment::IMAGE_MASK, 3, false), // duplicate of BASELINE_TO + (InlineAlignment::TEXT_MASK, 12, false), // duplicate of TO_BOTTOM + ]; + + let all_constants = InlineAlignment::all_constants(); + let mut expected_distinct_values = vec![]; + for ((value, ord, is_distinct), c) in expected.into_iter().zip(all_constants) { + if is_distinct { + assert_eq!(c.rust_name(), value.as_str()); // First distinct. + expected_distinct_values.push(value); + } + + assert_eq!(c.value(), value); + assert_eq!(c.ord(), ord); + } + + assert_eq!(InlineAlignment::values(), &expected_distinct_values); + + // Some known duplicates. + assert_eq!(InlineAlignment::TOP_TO, InlineAlignment::TO_TOP); // ord 0 + assert_eq!(InlineAlignment::TOP_TO, InlineAlignment::TOP); // ord 0 + assert_eq!(InlineAlignment::BASELINE_TO, InlineAlignment::IMAGE_MASK); // ord 3 + assert_eq!(InlineAlignment::TO_BOTTOM, InlineAlignment::TEXT_MASK); // ord 12 +} + +#[itest] +fn enum_values_max_excluded() { + let expected_array_types = [ + mesh::ArrayType::VERTEX, + mesh::ArrayType::NORMAL, + mesh::ArrayType::TANGENT, + mesh::ArrayType::COLOR, + mesh::ArrayType::TEX_UV, + mesh::ArrayType::TEX_UV2, + mesh::ArrayType::CUSTOM0, + mesh::ArrayType::CUSTOM1, + mesh::ArrayType::CUSTOM2, + mesh::ArrayType::CUSTOM3, + mesh::ArrayType::BONES, + mesh::ArrayType::WEIGHTS, + mesh::ArrayType::INDEX, + ]; + + let array_types = mesh::ArrayType::values(); + assert_eq!(array_types, &expected_array_types); + assert!( + !array_types.contains(&mesh::ArrayType::MAX), + "ArrayType::MAX should be excluded from values()" + ); + + // However, it should still be present in all_constants(). + let all_constants = mesh::ArrayType::all_constants(); + assert!( + all_constants + .iter() + .any(|c| c.value() == mesh::ArrayType::MAX), + "ArrayType::MAX should be present in all_constants()" + ); +} + +#[itest] +fn enum_all_constants() { + let constants = InlineAlignment::all_constants(); + assert!( + constants.len() > InlineAlignment::values().len(), + "all_constants() should include duplicates" + ); + + // Check one known constant. + let first = constants[0]; + assert_eq!(first.rust_name(), "TOP_TO"); + assert_eq!(first.godot_name(), "INLINE_ALIGNMENT_TOP_TO"); + assert_eq!(first.ord(), 0); + assert_eq!(first.value(), InlineAlignment::TOP_TO); + + // Check specific constants at known indices, with equal ordinals. + let known_a = constants[2]; + let known_b = constants[11]; + + assert_eq!(known_a.ord(), 3); + assert_eq!(known_a.rust_name(), "BASELINE_TO"); + assert_eq!(known_a.godot_name(), "INLINE_ALIGNMENT_BASELINE_TO"); + assert_eq!(known_a.value(), InlineAlignment::BASELINE_TO); + + assert_eq!(known_b.ord(), 3); + assert_eq!(known_b.rust_name(), "IMAGE_MASK"); + assert_eq!(known_b.godot_name(), "INLINE_ALIGNMENT_IMAGE_MASK"); + assert_eq!(known_b.value(), InlineAlignment::IMAGE_MASK); + + // "Front-end" values are equal, too. + assert_eq!( + InlineAlignment::IMAGE_MASK.ord(), + InlineAlignment::BASELINE_TO.ord() + ); +} diff --git a/itest/rust/src/object_tests/enum_test.rs b/itest/rust/src/object_tests/enum_test.rs index 99f4ada5e..75ac501d9 100644 --- a/itest/rust/src/object_tests/enum_test.rs +++ b/itest/rust/src/object_tests/enum_test.rs @@ -62,7 +62,7 @@ fn enum_hash() { months.insert(time::Month::NOVEMBER); months.insert(time::Month::DECEMBER); - assert_eq!(months.len(), 12); + assert_eq!(months.len(), 12, "hash collisions in constants"); } // Testing https://github.com/godot-rust/gdext/issues/335 From 3612631df512f7c6744acf06f304e2c6d7f350be Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 12 Jul 2025 15:26:33 +0200 Subject: [PATCH 4/6] EngineBitfield::all_constants() --- godot-codegen/src/generator/enums.rs | 38 +++++++++++++---- godot-core/src/builtin/vectors/vector_axis.rs | 1 - godot-core/src/meta/inspect.rs | 29 ++++++------- godot-core/src/obj/traits.rs | 41 ++++++++++++++----- .../rust/src/engine_tests/engine_enum_test.rs | 20 +++++++-- itest/rust/src/object_tests/enum_test.rs | 4 ++ 6 files changed, 92 insertions(+), 41 deletions(-) diff --git a/godot-codegen/src/generator/enums.rs b/godot-codegen/src/generator/enums.rs index 24b91439f..7135f9e2c 100644 --- a/godot-codegen/src/generator/enums.rs +++ b/godot-codegen/src/generator/enums.rs @@ -227,6 +227,8 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T if enum_.is_bitfield { // Bitfields: u64, assume any combination is valid. + let constants_function = make_all_constants_function(enum_); + quote! { // We may want to add this in the future. // @@ -242,6 +244,8 @@ fn make_enum_engine_trait_impl(enum_: &Enum, enum_bitmask: Option<&RustTy>) -> T fn ord(self) -> u64 { self.ord } + + #constants_function } } } else if enum_.is_exhaustive { @@ -332,33 +336,49 @@ fn make_enum_values_and_constants_functions(enum_: &Enum) -> TokenStream { let name = &enum_.name; let mut distinct_values = Vec::new(); - let mut all_constants = Vec::new(); let mut seen_ordinals = HashSet::new(); for (index, enumerator) in enum_.enumerators.iter().enumerate() { let constant = &enumerator.name; - let rust_name = enumerator.name.to_string(); - let godot_name = enumerator.godot_name.to_string(); let ordinal = &enumerator.value; - // all_constants() includes every constant unconditionally. - all_constants.push(quote! { - crate::meta::inspect::EnumConstant::new(#rust_name, #godot_name, #ordinal, #name::#constant) - }); - // values() contains value only if distinct (first time seen) and not MAX. if enum_.max_index != Some(index) && seen_ordinals.insert(ordinal.clone()) { distinct_values.push(quote! { #name::#constant }); } } - quote! { + let values_function = quote! { fn values() -> &'static [Self] { &[ #( #distinct_values ),* ] } + }; + + let all_constants_function = make_all_constants_function(enum_); + quote! { + #values_function + #all_constants_function + } +} + +/// Creates a shared `all_constants()` implementation for enums and bitfields. +fn make_all_constants_function(enum_: &Enum) -> TokenStream { + let name = &enum_.name; + + let all_constants = enum_.enumerators.iter().map(|enumerator| { + let ident = &enumerator.name; + let rust_name = enumerator.name.to_string(); + let godot_name = enumerator.godot_name.to_string(); + + quote! { + crate::meta::inspect::EnumConstant::new(#rust_name, #godot_name, #name::#ident) + } + }); + + quote! { fn all_constants() -> &'static [crate::meta::inspect::EnumConstant<#name>] { const { &[ diff --git a/godot-core/src/builtin/vectors/vector_axis.rs b/godot-core/src/builtin/vectors/vector_axis.rs index 051cc9dbe..d83a84092 100644 --- a/godot-core/src/builtin/vectors/vector_axis.rs +++ b/godot-core/src/builtin/vectors/vector_axis.rs @@ -69,7 +69,6 @@ macro_rules! impl_vector_axis_enum { EnumConstant::new( stringify!($axis), concat!("AXIS_", stringify!($axis)), - $AxisEnum::$axis as i32, $AxisEnum::$axis ), )+ diff --git a/godot-core/src/meta/inspect.rs b/godot-core/src/meta/inspect.rs index 9051a8a6a..2326214a0 100644 --- a/godot-core/src/meta/inspect.rs +++ b/godot-core/src/meta/inspect.rs @@ -7,34 +7,32 @@ //! Introspection metadata for Godot engine types. -/// Metadata for a single enum constant. +/// Metadata for a single enum or bitfield constant. +/// +/// Returned by [`EngineEnum::all_constants()`][crate::obj::EngineEnum::all_constants] and +/// [`EngineBitfield::all_constants()`][crate::obj::EngineBitfield::all_constants]. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct EnumConstant { rust_name: &'static str, godot_name: &'static str, - ord: i32, value: T, } impl EnumConstant { /// Creates a new enum constant metadata entry. - pub(crate) const fn new( - rust_name: &'static str, - godot_name: &'static str, - ord: i32, - value: T, - ) -> Self { + pub(crate) const fn new(rust_name: &'static str, godot_name: &'static str, value: T) -> Self { Self { rust_name, godot_name, - ord, value, } } - /// Rust name of the enum variant, usually without Prefix (e.g. `"ESCAPE"` for `Key::ESCAPE`). + /// Rust name of the constant, usually without prefix (e.g. `"ESCAPE"` for `Key::ESCAPE`). /// - /// This is returned by [`EngineEnum::as_str()`](crate::obj::EngineEnum::as_str()). + /// For enums, this is the value returned by [`EngineEnum::as_str()`](crate::obj::EngineEnum::as_str()) **if the value is unique.** + /// If multiple enum values share the same ordinal, then this function will return each one separately, while `as_str()` will return the + /// first one. pub const fn rust_name(&self) -> &'static str { self.rust_name } @@ -44,12 +42,9 @@ impl EnumConstant { self.godot_name } - /// Ordinal value of this enum variant. - pub const fn ord(&self) -> i32 { - self.ord - } - - /// The enum value itself. + /// The Rust value itself. + /// + /// Use `value().ord()` to get the ordinal value. pub const fn value(&self) -> T { self.value } diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index 787ecc387..57d7b1f12 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -190,10 +190,10 @@ pub trait EngineEnum: Copy { /// The name of the enumerator, as it appears in Rust. /// - /// Note that **this may not match the Rust constant name!** In case of multiple constants with the same ordinal value, this method returns - /// the first one in the order of definition. For example, `LayoutDirection::LOCALE.as_str()` (ord 1) returns `"APPLICATION_LOCALE"`, because - /// that happens to be the first constant with ordinal `1`. See [`all_constants()`][Self::all_constants] for a more robust and general - /// approach to introspection of enum constants. + /// Note that **this may not match the Rust constant name.** In case of multiple constants with the same ordinal value, this method returns + /// the first one in the order of definition. For example, [`LayoutDirection::LOCALE.as_str()`][crate::classes::window::LayoutDirection::LOCALE] + /// (ord 1) returns `"APPLICATION_LOCALE"`, because that happens to be the first constant with ordinal `1`. + /// See [`all_constants()`][Self::all_constants] for a more robust and general approach to introspection of enum constants. /// /// If the value does not match one of the known enumerators, the empty string is returned. fn as_str(&self) -> &'static str; @@ -205,9 +205,10 @@ pub trait EngineEnum: Copy { /// Returns a slice of distinct enum values. /// - /// This excludes MAX constants and deduplicates aliases, providing only meaningful enum values. + /// This excludes `MAX` constants at the end (existing only to express the number of enumerators) and deduplicates aliases, + /// providing only meaningful enum values. See [`all_constants()`][Self::all_constants] for a complete list of all constants. /// - /// This enables iteration over distinct enum variants: + /// Enables iteration over distinct enum variants: /// ```no_run /// use godot::classes::window; /// use godot::obj::EngineEnum; @@ -220,18 +221,19 @@ pub trait EngineEnum: Copy { /// Returns metadata for all enum constants. /// - /// This includes all constants as they appear in the enum definition, including duplicates and MAX constants. + /// This includes all constants as they appear in the enum definition, including duplicates and `MAX` constants. + /// For a list of useful, distinct values, use [`values()`][Self::values]. /// - /// This enables complete introspection and debugging: + /// Enables introspection of available constants: /// ```no_run /// use godot::classes::window; /// use godot::obj::EngineEnum; /// /// for constant in window::Mode::all_constants() { - /// println!("* {}: {} (ord: {})", + /// println!("* window::Mode.{} (original {}) has ordinal value {}.", /// constant.rust_name(), /// constant.godot_name(), - /// constant.ord() + /// constant.value().ord() /// ); /// } /// ``` @@ -254,6 +256,25 @@ pub trait EngineBitfield: Copy { fn is_set(self, flag: Self) -> bool { self.ord() & flag.ord() != 0 } + + /// Returns metadata for all bitfield constants. + /// + /// This includes all constants as they appear in the bitfield definition. + /// + /// Enables introspection of available constants: + /// ```no_run + /// use godot::global::KeyModifierMask; + /// use godot::obj::EngineBitfield; + /// + /// for constant in KeyModifierMask::all_constants() { + /// println!("* KeyModifierMask.{} (original {}) has ordinal value {}.", + /// constant.rust_name(), + /// constant.godot_name(), + /// constant.value().ord() + /// ); + /// } + /// ``` + fn all_constants() -> &'static [EnumConstant]; } /// Trait for enums that can be used as indices in arrays. diff --git a/itest/rust/src/engine_tests/engine_enum_test.rs b/itest/rust/src/engine_tests/engine_enum_test.rs index 759709052..05cb33a57 100644 --- a/itest/rust/src/engine_tests/engine_enum_test.rs +++ b/itest/rust/src/engine_tests/engine_enum_test.rs @@ -97,7 +97,7 @@ fn enum_values_duplicates() { } assert_eq!(c.value(), value); - assert_eq!(c.ord(), ord); + assert_eq!(c.value().ord(), ord); } assert_eq!(InlineAlignment::values(), &expected_distinct_values); @@ -156,22 +156,22 @@ fn enum_all_constants() { let first = constants[0]; assert_eq!(first.rust_name(), "TOP_TO"); assert_eq!(first.godot_name(), "INLINE_ALIGNMENT_TOP_TO"); - assert_eq!(first.ord(), 0); assert_eq!(first.value(), InlineAlignment::TOP_TO); + assert_eq!(first.value().ord(), 0); // Check specific constants at known indices, with equal ordinals. let known_a = constants[2]; let known_b = constants[11]; - assert_eq!(known_a.ord(), 3); assert_eq!(known_a.rust_name(), "BASELINE_TO"); assert_eq!(known_a.godot_name(), "INLINE_ALIGNMENT_BASELINE_TO"); assert_eq!(known_a.value(), InlineAlignment::BASELINE_TO); + assert_eq!(known_a.value().ord(), 3); - assert_eq!(known_b.ord(), 3); assert_eq!(known_b.rust_name(), "IMAGE_MASK"); assert_eq!(known_b.godot_name(), "INLINE_ALIGNMENT_IMAGE_MASK"); assert_eq!(known_b.value(), InlineAlignment::IMAGE_MASK); + assert_eq!(known_b.value().ord(), 3); // "Front-end" values are equal, too. assert_eq!( @@ -179,3 +179,15 @@ fn enum_all_constants() { InlineAlignment::BASELINE_TO.ord() ); } + +#[itest] +fn bitfield_all_constants() { + let shift_constant = KeyModifierMask::all_constants() + .iter() + .find(|c| c.rust_name() == "SHIFT") + .expect("SHIFT constant should exist"); + + assert_eq!(shift_constant.godot_name(), "KEY_MASK_SHIFT"); + assert_eq!(shift_constant.value(), KeyModifierMask::SHIFT); + assert_eq!(shift_constant.value().ord(), 1 << 25); +} diff --git a/itest/rust/src/object_tests/enum_test.rs b/itest/rust/src/object_tests/enum_test.rs index 75ac501d9..42892a055 100644 --- a/itest/rust/src/object_tests/enum_test.rs +++ b/itest/rust/src/object_tests/enum_test.rs @@ -9,6 +9,7 @@ use crate::framework::itest; use godot::builtin::varray; use godot::classes::input::CursorShape; use godot::classes::mesh::PrimitiveType; +use godot::classes::window::LayoutDirection; use godot::classes::{time, ArrayMesh}; use godot::global::{Key, Orientation}; use godot::obj::NewGd; @@ -84,6 +85,9 @@ fn enum_as_str() { assert_eq!(Key::ESCAPE.as_str(), "ESCAPE"); assert_eq!(Key::TAB.as_str(), "TAB"); assert_eq!(Key::A.as_str(), "A"); + + #[cfg(since_api = "4.4")] // Deprecated in Godot, LOCALE is now alias for APPLICATION_LOCALE. + assert_eq!(LayoutDirection::LOCALE.as_str(), "APPLICATION_LOCALE"); } #[itest] From b3f008b31b8ac4ce06408f20ecc5ca612d7fd144 Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 12 Jul 2025 17:09:29 +0200 Subject: [PATCH 5/6] Deprecate EngineEnum::godot_name() --- godot-core/src/meta/inspect.rs | 5 +++- godot-core/src/obj/traits.rs | 19 +++++++++++++ itest/rust/src/object_tests/enum_test.rs | 36 +++++++++++++++++++++++- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/godot-core/src/meta/inspect.rs b/godot-core/src/meta/inspect.rs index 2326214a0..089180d41 100644 --- a/godot-core/src/meta/inspect.rs +++ b/godot-core/src/meta/inspect.rs @@ -18,7 +18,10 @@ pub struct EnumConstant { value: T, } -impl EnumConstant { +impl EnumConstant +where + T: Copy + Eq + PartialEq + 'static, +{ /// Creates a new enum constant metadata entry. pub(crate) const fn new(rust_name: &'static str, godot_name: &'static str, value: T) -> Self { Self { diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index 57d7b1f12..dfbd7cb65 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -201,6 +201,25 @@ pub trait EngineEnum: Copy { /// The equivalent name of the enumerator, as specified in Godot. /// /// If the value does not match one of the known enumerators, the empty string is returned. + /// + /// # Deprecation + /// Design change is due to the fact that Godot enums may have multiple constants with the same ordinal value, and `godot_name()` cannot + /// always return a unique name for it. So there are cases where this method returns unexpected results. + /// + /// To keep the old -- possibly incorrect -- behavior, you can write the following function. However, it runs in linear rather than constant + /// time (which is often OK, given that there are very few constants per enum). + /// ``` + /// use godot::obj::EngineEnum; + /// + /// fn godot_name(value: T) -> &'static str { + /// T::all_constants() + /// .iter() + /// .find(|c| c.value() == value) + /// .map(|c| c.godot_name()) + /// .unwrap_or("") // Previous behavior. + /// } + /// ``` + #[deprecated = "Moved to introspection API, see `EngineEnum::all_constants()` and `EnumConstant::godot_name()`"] fn godot_name(&self) -> &'static str; /// Returns a slice of distinct enum values. diff --git a/itest/rust/src/object_tests/enum_test.rs b/itest/rust/src/object_tests/enum_test.rs index 42892a055..d806351e8 100644 --- a/itest/rust/src/object_tests/enum_test.rs +++ b/itest/rust/src/object_tests/enum_test.rs @@ -12,7 +12,7 @@ use godot::classes::mesh::PrimitiveType; use godot::classes::window::LayoutDirection; use godot::classes::{time, ArrayMesh}; use godot::global::{Key, Orientation}; -use godot::obj::NewGd; +use godot::obj::{EngineEnum, NewGd}; use std::collections::HashSet; #[itest] @@ -92,6 +92,29 @@ fn enum_as_str() { #[itest] fn enum_godot_name() { + use godot::obj::EngineEnum; + assert_eq!( + godot_name(Orientation::VERTICAL), + Orientation::VERTICAL.as_str() + ); + assert_eq!( + godot_name(Orientation::HORIZONTAL), + Orientation::HORIZONTAL.as_str() + ); + + assert_eq!(godot_name(Key::NONE), "KEY_NONE"); + assert_eq!(godot_name(Key::SPECIAL), "KEY_SPECIAL"); + assert_eq!(godot_name(Key::ESCAPE), "KEY_ESCAPE"); + assert_eq!(godot_name(Key::TAB), "KEY_TAB"); + assert_eq!(godot_name(Key::A), "KEY_A"); + + // Unknown enumerators (might come from the future). + assert_eq!(godot_name(Key::from_ord(1234)), ""); +} + +#[itest] +#[expect(deprecated)] +fn enum_godot_name_deprecated() { use godot::obj::EngineEnum; assert_eq!( Orientation::VERTICAL.godot_name(), @@ -107,4 +130,15 @@ fn enum_godot_name() { assert_eq!(Key::ESCAPE.godot_name(), "KEY_ESCAPE"); assert_eq!(Key::TAB.godot_name(), "KEY_TAB"); assert_eq!(Key::A.godot_name(), "KEY_A"); + + // Unknown enumerators (might come from the future). + assert_eq!(Key::from_ord(1234).godot_name(), ""); +} + +fn godot_name(value: T) -> &'static str { + T::all_constants() + .iter() + .find(|c| c.value() == value) + .map(|c| c.godot_name()) + .unwrap_or("") // Previous behavior. } From e5006b14c27f5adf11db64eb5b470b1838abdaca Mon Sep 17 00:00:00 2001 From: Jan Haller Date: Sat, 12 Jul 2025 18:03:32 +0200 Subject: [PATCH 6/6] Fix #[itest] not preserving attributes --- godot-macros/src/bench.rs | 6 +++++- godot-macros/src/itest.rs | 8 +++++++- godot-macros/src/util/mod.rs | 11 +++++++++++ itest/rust/src/engine_tests/codegen_test.rs | 8 ++++++++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/godot-macros/src/bench.rs b/godot-macros/src/bench.rs index 9a42b639a..29a2b43c9 100644 --- a/godot-macros/src/bench.rs +++ b/godot-macros/src/bench.rs @@ -8,7 +8,7 @@ use proc_macro2::TokenStream; use quote::quote; -use crate::util::{bail, KvParser}; +use crate::util::{bail, retain_attributes_except, KvParser}; use crate::ParseResult; const DEFAULT_REPETITIONS: usize = 100; @@ -42,7 +42,11 @@ pub fn attribute_bench(input_decl: venial::Item) -> ParseResult { let body = &func.body; + // Filter out #[bench] itself, but preserve other attributes like #[allow], #[expect], etc. + let other_attributes = retain_attributes_except(&func.attributes, "bench"); + Ok(quote! { + #(#other_attributes)* pub fn #bench_name() { for _ in 0..#repetitions { let __ret: #ret = #body; diff --git a/godot-macros/src/itest.rs b/godot-macros/src/itest.rs index 1bfd6f0b7..a4eb3b774 100644 --- a/godot-macros/src/itest.rs +++ b/godot-macros/src/itest.rs @@ -8,7 +8,9 @@ use proc_macro2::TokenStream; use quote::{quote, ToTokens}; -use crate::util::{bail, extract_typename, ident, path_ends_with, KvParser}; +use crate::util::{ + bail, extract_typename, ident, path_ends_with, retain_attributes_except, KvParser, +}; use crate::ParseResult; pub fn attribute_itest(input_item: venial::Item) -> ParseResult { @@ -85,7 +87,11 @@ pub fn attribute_itest(input_item: venial::Item) -> ParseResult { plugin_name = ident("__GODOT_ITEST"); }; + // Filter out #[itest] itself, but preserve other attributes like #[allow], #[expect], etc. + let other_attributes = retain_attributes_except(&func.attributes, "itest"); + Ok(quote! { + #(#other_attributes)* pub fn #test_name(#param) #return_tokens { #body } diff --git a/godot-macros/src/util/mod.rs b/godot-macros/src/util/mod.rs index 1f1f23f47..4ccdb60ec 100644 --- a/godot-macros/src/util/mod.rs +++ b/godot-macros/src/util/mod.rs @@ -85,6 +85,17 @@ pub(crate) use bail; pub(crate) use error; pub(crate) use require_api_version; +/// Keeps all attributes except the one specified (e.g. `"itest"`). +pub fn retain_attributes_except<'a>( + attributes: &'a [venial::Attribute], + macro_name: &'a str, +) -> impl Iterator { + attributes.iter().filter(move |attr| { + attr.get_single_path_segment() + .is_none_or(|segment| segment != macro_name) + }) +} + pub fn reduce_to_signature(function: &venial::Function) -> venial::Function { let mut reduced = function.clone(); reduced.vis_marker = None; // retained outside in the case of #[signal]. diff --git a/itest/rust/src/engine_tests/codegen_test.rs b/itest/rust/src/engine_tests/codegen_test.rs index 9a535d06b..3ab526736 100644 --- a/itest/rust/src/engine_tests/codegen_test.rs +++ b/itest/rust/src/engine_tests/codegen_test.rs @@ -190,3 +190,11 @@ trait TraitA { impl TraitA for CodegenTest3 { fn exit_tree(&mut self) {} } + +// Verifies that attributes (here #[expect]) are preserved by #[itest] macro. +// See retain_attributes_except() function. +#[itest] +#[expect(unused_variables)] +fn test_itest_macro_attribute_retention() { + let unused_var = 42; // Should not generate warning. +}