Allow `PhantomVar` to work with engine enums #1438

LviatYi · 2025-12-07T18:27:42Z

Background

Feature

This enables use cases like:

#[export]
#[var(get = get_vertical_alignment, set = set_vertical_alignment)]
vertical_alignment: PhantomVar<godot::global::VerticalAlignment>;

Key Changes

Modify:

In godot-core/src/registry/property/phantom_var.rs

pub struct PhantomVar<T: GodotConvert + Var>(PhantomData<T>);

This makes the requirements for PhantomVar more lenient, because GodotType is a metaphor for GodotConvert. For existing supported types where T: GodotType and Via = T, behavior remains the same.

Add

In godot-codegen/src/generator/enums.rs

impl crate::registry::property::Var for #name {
    fn get_property(&self) -> Self::Via{
        <Self as #engine_trait>::ord(*self)
    }
    
    #var_trait_set_property
    
    fn var_hint() -> crate::meta::PropertyHintInfo{
        crate::meta::PropertyHintInfo{
            hint: #property_hint,
            hint_string: #enum_hint_string.into(),
        }
    }
}

impl crate::registry::property::Export for #name {}

Concern

Clearly, not all enumerations will be edited as fields in the inspector. However, this PR extensively implements Var and Export for godot enums. Is it acceptable to treat all engine enums as "exportable as properties" by default, given that it's purely additive and only used when the user opts into exporting such fields?

Additionally, I've provided a make_enum_hint_string function:

/// Returns the hint string for the given enum.
///
/// Separate with commas, and remove the `<ENUM_NAME>_` prefix (if possible). 
/// e.g.: "Left,Center,Right,Fill"
fn make_enum_hint_string(enum_: &Enum) -> String {
    let upper_cast_enum_name = enum_.godot_name.to_shouty_snake_case() + "_";
    enum_.enumerators
         .iter()
         .map(|enumerator| {
             enumerator.godot_name
                       .clone()
                       .trim_start_matches(upper_cast_enum_name.as_str())
                       .to_pascal_case()
         })
         .collect::<Vec<String>>()
         .join(",")
}

It works fairly well, with one exception: for EularOrder, users might prefer to see values like XYZ rather than Xyz. Unfortunately, I cannot distinguish this through extension_api.json.

During my research of the Godot source code, I found this line:

ADD_PROPERTY(PropertyInfo(Variant::INT, "vertical_alignment", PROPERTY_HINT_ENUM, "Top,Center,Bottom,Fill"), "set_vertical_alignment", "get_vertical_alignment");

This means that Godot itself does not have a method to convert enum values to hint strings, it's entirely "manually" maintained in every component that uses enum types.

This PR can be interpreted as an "overstepping" of authority, adding extra behavior to gdext that doesn't exist in Godot, but the user experience might be more comfortable than developing components on native Godot.

But is this allowed?

Thanks for considering this!

Bromeon · 2025-12-07T18:37:39Z

Thanks a lot for the contribution! 👍

Is it acceptable to treat all engine enums as "exportable as properties" by default, given that it's purely additive and only used when the user opts into exporting such fields?

Yep, I think that's nice to have.

This PR can be interpreted as an "overstepping" of authority, adding extra behavior to gdext that doesn't exist in Godot, but the user experience might be more comfortable than developing components on native Godot.

But is this allowed?

It's fine, but can you not use the same logic as used in the codegen for introspection methods on EngineEnum? I don't mean to call those methods at runtime, but look how they're implemented and use the same identifiers generated for them.

I don't think there's a need to reimplement the enum-to-string logic.

Bromeon

Please also squash commits to 1 eventually, see Contributing guidelines.

Bromeon · 2025-12-07T18:38:38Z

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

 // Bounds for T are somewhat un-idiomatically directly on the type, rather than impls.
 // This improves error messages in IDEs when using the type as a field.
-pub struct PhantomVar<T: GodotType + Var>(PhantomData<T>);
+pub struct PhantomVar<T: GodotConvert + Var>(PhantomData<T>);


Why these bound changes everywhere?

Some are mandatory, such as Var, Export, and Default. Debug, Clone, and Copy might be needed, but they're not in my use case.

As for the rest... I think GodotConvert is more permissive than GodotType, and it only adds cost when actually called, so I modified it accordingly.

The changes might be finalized tomorrow; it's too late here. Sorry~

I think GodotConvert is more permissive than GodotType

Good point, makes sense 👍

Bromeon · 2025-12-07T18:39:55Z

godot-codegen/src/generator/enums.rs

+/// Returns the hint string for the given enum.
+///
+/// Separate with commas, and remove the `<ENUM_NAME>_` prefix (if possible). 
+/// e.g.: "Left,Center,Right,Fill"
+fn make_enum_hint_string(enum_: &Enum) -> String {
+    let upper_cast_enum_name = enum_.godot_name.to_shouty_snake_case() + "_";
+    enum_.enumerators
+         .iter()
+         .map(|enumerator| {
+             enumerator.godot_name
+                       .clone()
+                       .trim_start_matches(upper_cast_enum_name.as_str())
+                       .to_pascal_case()
+         })
+         .collect::<Vec<String>>()
+         .join(",")
+}


As mentioned, this logic should already exist in some form.

There's a small detail here, intended to maintain consistency with how enumeration values are rendered in the godot editor. Godot uses Pascal for display.

Perhaps it's a bit redundant XD.

I see. Can you reuse one of these though?

gdext/godot-codegen/src/conv/name_conversions.rs

Lines 57 to 105 in 0b6b792

/// Used for `PascalCase` identifiers: classes and enums.

pub fn to_pascal_case(ty_name: &str) -> String {

use heck::ToPascalCase;

assert!(

is_valid_ident(ty_name),

"invalid identifier for PascalCase conversion: {ty_name}"

);

// Special cases: reuse snake_case impl to ensure at least consistency between those 2.

if let Some(snake_special) = to_snake_special_case(ty_name) {

return snake_special.to_pascal_case();

}

ty_name

.to_pascal_case()

.replace("GdExtension", "GDExtension")

.replace("GdNative", "GDNative")

.replace("GdScript", "GDScript")

.replace("Vsync", "VSync")

.replace("Sdfgiy", "SdfgiY")

}

#[allow(dead_code)] // Keep around in case we need it later.

pub fn shout_to_pascal(shout_case: &str) -> String {

// TODO use heck?

assert!(

is_valid_shout_ident(shout_case),

"invalid identifier for SHOUT_CASE -> PascalCase conversion: {shout_case}"

);

let mut result = String::with_capacity(shout_case.len());

let mut next_upper = true;

for ch in shout_case.chars() {

if next_upper {

assert_ne!(ch, '_'); // no double underscore

result.push(ch); // unchanged

next_upper = false;

} else if ch == '_' {

next_upper = true;

} else {

result.push(ch.to_ascii_lowercase());

}

}

result

}

If not, it should at least become a dedicated conversion function in that file (ideally with a comment on how it differs from the others). Thanks! 🙂

Bromeon · 2025-12-07T18:40:26Z

godot-codegen/src/generator/enums.rs


+        let var_trait_set_property = if enum_.is_exhaustive {
+            quote! {
+                fn set_property(&mut self, value: Self::Via){


Suggested change

fn set_property(&mut self, value: Self::Via){

fn set_property(&mut self, value: Self::Via) {

Please fix this same formatting error everywhere 🙂

GodotRust · 2025-12-07T18:55:24Z

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1438

Bromeon · 2025-12-07T20:30:34Z

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

 // Bounds for T are somewhat un-idiomatically directly on the type, rather than impls.
 // This improves error messages in IDEs when using the type as a field.
-pub struct PhantomVar<T: GodotType + Var>(PhantomData<T>);
+pub struct PhantomVar<T: GodotConvert + Var>(PhantomData<T>);


I think GodotConvert is more permissive than GodotType

Good point, makes sense 👍

Bromeon · 2025-12-07T20:35:50Z

godot-codegen/src/generator/enums.rs

+/// Returns the hint string for the given enum.
+///
+/// Separate with commas, and remove the `<ENUM_NAME>_` prefix (if possible). 
+/// e.g.: "Left,Center,Right,Fill"
+fn make_enum_hint_string(enum_: &Enum) -> String {
+    let upper_cast_enum_name = enum_.godot_name.to_shouty_snake_case() + "_";
+    enum_.enumerators
+         .iter()
+         .map(|enumerator| {
+             enumerator.godot_name
+                       .clone()
+                       .trim_start_matches(upper_cast_enum_name.as_str())
+                       .to_pascal_case()
+         })
+         .collect::<Vec<String>>()
+         .join(",")
+}


I see. Can you reuse one of these though?

gdext/godot-codegen/src/conv/name_conversions.rs

Lines 57 to 105 in 0b6b792

/// Used for `PascalCase` identifiers: classes and enums.

pub fn to_pascal_case(ty_name: &str) -> String {

use heck::ToPascalCase;

assert!(

is_valid_ident(ty_name),

"invalid identifier for PascalCase conversion: {ty_name}"

);

// Special cases: reuse snake_case impl to ensure at least consistency between those 2.

if let Some(snake_special) = to_snake_special_case(ty_name) {

return snake_special.to_pascal_case();

}

ty_name

.to_pascal_case()

.replace("GdExtension", "GDExtension")

.replace("GdNative", "GDNative")

.replace("GdScript", "GDScript")

.replace("Vsync", "VSync")

.replace("Sdfgiy", "SdfgiY")

}

#[allow(dead_code)] // Keep around in case we need it later.

pub fn shout_to_pascal(shout_case: &str) -> String {

// TODO use heck?

assert!(

is_valid_shout_ident(shout_case),

"invalid identifier for SHOUT_CASE -> PascalCase conversion: {shout_case}"

);

let mut result = String::with_capacity(shout_case.len());

let mut next_upper = true;

for ch in shout_case.chars() {

if next_upper {

assert_ne!(ch, '_'); // no double underscore

result.push(ch); // unchanged

next_upper = false;

} else if ch == '_' {

next_upper = true;

} else {

result.push(ch.to_ascii_lowercase());

}

}

result

}

If not, it should at least become a dedicated conversion function in that file (ideally with a comment on how it differs from the others). Thanks! 🙂

Bromeon · 2025-12-07T20:36:52Z

godot-codegen/src/generator/enums.rs

+      fn var_hint() -> crate::meta::PropertyHintInfo{
+          crate::meta::PropertyHintInfo{
+              hint: #property_hint,
+              hint_string: #enum_hint_string.into(),


Please use explicit GString::from() here.

Suggested change

hint_string: #enum_hint_string.into(),

hint_string: GString::from(#enum_hint_string),

Bromeon · 2025-12-08T09:24:21Z

Thank you! What would be nice now is to add 1-2 small integration tests, to make sure enums work.
Maybe one with a regular #[var] field: Enum and one inside PhantomVar.

You could add #[itest] cases to phantom_var_test.rs, asserting:

get default value (initialized with #[init(val = ...)] matches expected value
set a different value, get value again matches the new value
calling Object.get("field") dynamically returns the integer for the corresponding enum

For inspiration how #[itest] works, you could check out property_test.rs.

LviatYi · 2025-12-08T09:39:16Z

No problem. But I haven't used the gdext testing framework before; I'll look into how to do this later.

Bromeon · 2025-12-08T09:45:46Z

We also have a guide:
https://godot-rust.github.io/book/contribute/dev-tools.html

… bindings

Bromeon added feature Adds functionality to the library c: engine Godot classes (nodes, resources, ...) labels Dec 7, 2025

Bromeon added this to the 0.5 milestone Dec 7, 2025

Bromeon reviewed Dec 7, 2025

View reviewed changes

LviatYi force-pushed the feature/phantomVar-with-godot-enums branch from 94eb91e to f76040c Compare December 7, 2025 19:29

Bromeon reviewed Dec 7, 2025

View reviewed changes

LviatYi force-pushed the feature/phantomVar-with-godot-enums branch 2 times, most recently from 4b2f968 to e537bc7 Compare December 8, 2025 05:03

LviatYi closed this Dec 8, 2025

LviatYi force-pushed the feature/phantomVar-with-godot-enums branch from e537bc7 to 3416265 Compare December 8, 2025 14:32

feat: support engine enums in PhantomVar and implement Var/Export for…

dc83e3c

… bindings

LviatYi reopened this Dec 8, 2025

	/// Used for `PascalCase` identifiers: classes and enums.
	pub fn to_pascal_case(ty_name: &str) -> String {
	use heck::ToPascalCase;

	assert!(
	is_valid_ident(ty_name),
	"invalid identifier for PascalCase conversion: {ty_name}"
	);

	// Special cases: reuse snake_case impl to ensure at least consistency between those 2.
	if let Some(snake_special) = to_snake_special_case(ty_name) {
	return snake_special.to_pascal_case();
	}

	ty_name
	.to_pascal_case()
	.replace("GdExtension", "GDExtension")
	.replace("GdNative", "GDNative")
	.replace("GdScript", "GDScript")
	.replace("Vsync", "VSync")
	.replace("Sdfgiy", "SdfgiY")
	}

	#[allow(dead_code)] // Keep around in case we need it later.
	pub fn shout_to_pascal(shout_case: &str) -> String {
	// TODO use heck?

	assert!(
	is_valid_shout_ident(shout_case),
	"invalid identifier for SHOUT_CASE -> PascalCase conversion: {shout_case}"
	);

	let mut result = String::with_capacity(shout_case.len());
	let mut next_upper = true;

	for ch in shout_case.chars() {
	if next_upper {
	assert_ne!(ch, '_'); // no double underscore
	result.push(ch); // unchanged
	next_upper = false;
	} else if ch == '_' {
	next_upper = true;
	} else {
	result.push(ch.to_ascii_lowercase());
	}
	}

	result
	}

	fn set_property(&mut self, value: Self::Via){
	fn set_property(&mut self, value: Self::Via) {

	hint_string: #enum_hint_string.into(),
	hint_string: GString::from(#enum_hint_string),

Uh oh!

Allow PhantomVar to work with engine enums #1438

Are you sure you want to change the base?

Allow PhantomVar to work with engine enums #1438

Conversation

LviatYi commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Feature

Key Changes

Concern

Uh oh!

Bromeon commented Dec 7, 2025

Uh oh!

Bromeon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GodotRust commented Dec 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bromeon commented Dec 8, 2025

Uh oh!

LviatYi commented Dec 8, 2025

Uh oh!

Bromeon commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Allow `PhantomVar` to work with engine enums #1438

Allow `PhantomVar` to work with engine enums #1438

LviatYi commented Dec 7, 2025 •

edited

Loading