-
-
Notifications
You must be signed in to change notification settings - Fork 23.9k
Allow using any-case constants in Expression #100395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
f53f9d3 to
096d2f9
Compare
core/math/expression.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about if (id.nocasecmp_to("pi") == 0) ? Would allow Pi, TaU or iNf as well, not sure if it's desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! Actually, I think an even better option would be to support this for all constants / operators, so Python users will feel at home writing True and such, SQL users will feel at home writing AND/OR/NOT, C# users can write Tau etc, and users won't have to remember the capitalization.
096d2f9 to
9fe6c82
Compare
9fe6c82 to
6019cfb
Compare
|
I don't really see why Expression should be special cased like this. Its parser is meant to be as close to GDScript as possible, so allowing expressions which aren't supported in GDScript seems weird to me. |
|
Is that a stated goal of Expression? The docs don't explicitly mention that: https://docs.godotengine.org/en/stable/classes/class_expression.html Expression is used by the Godot editor for user input in Range, in which case |
6019cfb to
2de71dd
Compare
2de71dd to
13b7afa
Compare
7dd07bc to
801fb4f
Compare
|
This is likely best implemented at a local level in Range/SpinBox/EditorSpinSlider, as per Prefer local solutions. I also agree with Akien that Expression should remain as close as possible to GDScript's behavior for ease of learning (and porting code from one to the other). |
|
True, it would be simple for SpinBox and EditorSpinSlider to capitalize the input text before parsing it with Expression. |
|
Re-opening because PR #105330 is infeasible and the problem still needs to be solved. |
801fb4f to
eb23ced
Compare
eb23ced to
3065ea1
Compare
3065ea1 to
daeb0ba
Compare
daeb0ba to
a1ce0d3
Compare
a1ce0d3 to
39efbcf
Compare
This is a general usability improvement for people using the Expression class with user-supplied input, such as the Godot editor inspector. This way, users will not have to capitalize things like
pi/2, it will just work. Without this change, users would have to writePI/2,PI / 2, or similar.This won't break user code unless they were using
pi,tau,inf, ornanas variable names, but I doubt that.EDIT: Updated the PR to support any casing.
pi,PI,Pi,pI,true,True,TRUE, etc.