Skip to content
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

Recognize inf, nan, and inf_neg constants (note: lower case) #3390

Closed
theraot opened this issue Oct 5, 2021 · 5 comments · May be fixed by godotengine/godot#53698
Closed

Recognize inf, nan, and inf_neg constants (note: lower case) #3390

theraot opened this issue Oct 5, 2021 · 5 comments · May be fixed by godotengine/godot#53698
Milestone

Comments

@theraot
Copy link

theraot commented Oct 5, 2021

Describe the project you are working on

This is how I stumbled upon this: I was making a custom node that would have an optional maximum distance check. So I exported a variable for the maximum distance. Note: Godot does not have a nullable float type. And I figured that a good way to disable a maximum distance check is to use INF. In fact, I can check for INF in the code to short circuit it.

Describe the problem or limitation you are having in your project

This is purely an usability concern.

Most float literals can be typed verbatim in the code inspector. Except for inf, nan, and inf_neg. Instead we have to use INF, NAN and -INF.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Considering that godotengine/godot#47497 / godotengine/godot#47497 is merged.

The code inspector uses GDScript expressions (this is how -INF works, it is the negative of the constant INF). Thus the evident solution would be to add new inf, inf_neg and nan constants to GDScripts.

Honestly, I don't think GDScript needs those constants. And my concern is with the inspector panel.

The alternative is, of course, change the Variant serialization. Which is a breaking change.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

With the new inf, inf_neg and nan constants in GDScripts, any float literal can be written verbatim in the inspector the same way they are serialized / represented in the inspector. Being the inspector input is a GDScript expression, it would recognize those proposed GDScript constants.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Writing INF or NAN in the inspector has no precedence… Since it didn't work. Now it is bound to happen, and if anything you can point people here.

However, honestly, it probably isn't a common use case, since a lot of things won't work with those values. For example RayCasts don't work #3389.

And yes, we have a workaround. We can use INF and NAN.

Is there a reason why this should be core and not an add-on in the asset library?

Cannot be done with an asset.

@Calinou Calinou added this to the 4.0 milestone Oct 5, 2021
@aaronfranke
Copy link
Member

aaronfranke commented Oct 6, 2021

Instead of allowing inf in GDScript, why not do the opposite, and allow INF etc in the inspector?

The naming convention in GDScript is that constants are uppercase.

@theraot
Copy link
Author

theraot commented Oct 6, 2021

@aaronfranke I would prefer that, yes.

As far as I can tell, the inspector shows the serialized value.

Are you suggesting to make an special case for the inspector or to change the serialization?

The serialization could have been INF instead of inf but… Well, you can read how that played at godotengine/godot#47497 - hence this proposal.

@dalexeev
Copy link
Member

dalexeev commented Oct 7, 2021

I also think we'd better fix the output of the values to match the value literals, but not vice versa.

See also godotengine/godot#47502 (and a few more, but I'm too lazy to search).

@theraot
Copy link
Author

theraot commented Oct 7, 2021

@dalexeev @aaronfranke What do you think of #3398?

@Calinou
Copy link
Member

Calinou commented May 11, 2022

Closing in favor of #3398, which seems to have more community support.

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

Successfully merging a pull request may close this issue.

4 participants