Skip to content

Allow @export_range to support numeric types#105878

Open
lodetrick wants to merge 1 commit intogodotengine:masterfrom
lodetrick:vector-range-hint
Open

Allow @export_range to support numeric types#105878
lodetrick wants to merge 1 commit intogodotengine:masterfrom
lodetrick:vector-range-hint

Conversation

@lodetrick
Copy link
Contributor

@lodetrick lodetrick commented Apr 28, 2025

Fixes #105875, Fixes #112647

Currently, numeric inspector properties support PROPERTY_HINT_RANGE implicitly (and are used by many editor classes), but the @export_range does not recognize this. This PR allows @export_range annotation to use the following types:
Vector2, Vector2I, Rect2, Rect2I, Vector3, Vector3I, Vector4, Vector4I, Transform2D, Transform3D, Plane, Quaternion, AABB, Basis, and Projection

It does this by replacing the float and int type check done in master with a custom check that was already partially implemented.

@lodetrick lodetrick requested a review from a team as a code owner April 28, 2025 18:47
@dalexeev dalexeev added this to the 4.x milestone Apr 28, 2025
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it mostly works as expected.

Testing project: test_pr_105878.zip

However, the various property inspectors in editor_properties.cpp don't fully implement range hints for all types. Most of them such as Vector2 will ignore the minimum/maximum value when dragging or entering a value, even if no or_less/or_greater is used. Some of them also ignore the step snapping, regardless of the input method used.

This should be fixed before this PR can be merged.

Additionally, I noticed that using an invalid type like bool with a range will cause an error, but the error will be reported on the last line containing an @export_range annotation. This means the error is often reported on the wrong line, unless the bool property is the last property with @export_range in the script.

@Mickeon
Copy link
Member

Mickeon commented Apr 28, 2025

I have been thinking about implementing this, but what kept putting me off is the thought that...
most users would expect PROPERTY_HINT_RANGE on vector types to optionally allow the range to be chosen for every component individually. The problem is that there's no agreed way to split the string for every component, and using commas would not make this optional.

@lodetrick
Copy link
Contributor Author

I was looking through the code, and the vector (vector, transform, basis, etc) types simply set the allow_greater and allow_lesser to true, as a fix for them being capped arbitrarily. This can be fixed by adding more arguments to the setup methods but at some point I wonder if theres a better way. I'm eyeing the EditorPropertyRangeHint struct that already exists within the editor_properties.cpp file, although I suppose this would add yet another method call before being able to create the property. I don't think its ideal for setup to take in just the hint-string instead. This would probably break compatibility and might be out of the scope of this PR, but its definitely something to be aware of.

My main concern is that by adding yet another two arguments to the setup method would just clutter it up and confuse developers. I think that the best way (without thinking about compatibility) would be to use a struct as input to the setup instead.

@Mickeon
Copy link
Member

Mickeon commented Apr 28, 2025

I don't think its ideal for setup to take in just the hint-string instead. This would probably break compatibility and might be out of the scope of this PR, but its definitely something to be aware of.

I think that generally we are aware that the hint and hint_string system is struggling to exist and will be replaced in the future. For now it's worth respecting it and making use of it as much as possible.
If you're afraid this is too much for the hint string, take a look at how PROPERTY_HINT_TYPE_STRING needs to be set up.

@lodetrick
Copy link
Contributor Author

lodetrick commented Apr 28, 2025

I have been thinking about implementing this, but what kept putting me off is the thought that... most users would expect PROPERTY_HINT_RANGE on vector types to optionally allow the range to be chosen for every component individually. The problem is that there's no agreed way to split the string for every component, and using commas would not make this optional.

I'm also concerned as to how @export_range would work with this, would it have a variable number of arguments? I think with regards to the hint_string the '|' character is still available (ie not conflicting with how nested types store data). Were you more talking about what order they would occur in/what would happen if the user didn't give one or all of the range hints?

I think that generally we are aware that the hint and hint_string system is struggling to exist and will be replaced in the future. For now it's worth respecting it and making use of it as much as possible

My earlier comment was more talking about the construction of the specific EditorProperties (the setup methods) than the hint_string in general, but I see what you mean, the whole hint system feels off right now.

@Mickeon
Copy link
Member

Mickeon commented Apr 29, 2025

Were you more talking about what order they would occur in/what would happen if the user didn't give one or all of the range hints?

Yes. It's also true that we could decide on a separation character on the spot, albeit it may get confusing.

@lodetrick lodetrick requested review from a team as code owners April 30, 2025 03:10
@lodetrick lodetrick closed this Apr 30, 2025
@lodetrick lodetrick reopened this Apr 30, 2025
@lodetrick
Copy link
Contributor Author

Sorry, misclick.

However, the various property inspectors in editor_properties.cpp don't fully implement range hints for all types. Most of them such as Vector2 will ignore the minimum/maximum value when dragging or entering a value, even if no or_less/or_greater is used. Some of them also ignore the step snapping, regardless of the input method used.

This should be fixed now, although it needed for two more arguments to be added to each EditorProperty setup method.

Additionally, I noticed that using an invalid type like bool with a range will cause an error, but the error will be reported on the last line containing an @export_range annotation. This means the error is often reported on the wrong line, unless the bool property is the last property with @export_range in the script.

This should also be fixed now. The push_error method had an argument that I didn't realize that encodes the line number.

@lodetrick lodetrick requested a review from a team January 7, 2026 02:01
@lodetrick
Copy link
Contributor Author

Rebased and updated to include changes made in #108065

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PROPERTY_HINT_RANGE with TYPE_VECTOR3 does not block lesser and greater Vector types are supported by PROPERTY_HINT_RANGE but not @export_range

4 participants