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

Modify float values to display with trailing .0 for clarity #45303

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Modify float values to display with trailing .0 for clarity #45303

wants to merge 1 commit into from

Conversation

NoahGori
Copy link

@NoahGori NoahGori commented Jan 19, 2021

Added toggle to keep a trailing zero when displaying whole number floats so that in
certain circumstances they can be differentiated from integers such as in
exported variables or when printing arrays
Before
Made to resolve godotengine/godot-proposals#1693

@aaronfranke
Copy link
Member

See also: #34668 (comment) CC @akien-mga

In general I think this is a good idea, but it should be done consistently throughout the engine.

@NoahGori
Copy link
Author

So the behavior is the same for all floats within all data types that are exported to the editor but only the arrays print floats with the trailing zero but I can add that to the other data types.

@gxhamster
Copy link

gxhamster commented Jan 22, 2021

I dont think we should add it as a an option
Beacause this will be much harder.
And this is not something that will break any code. I Think

@Calinou
Copy link
Member

Calinou commented Jan 22, 2021

I also think this should be done for all floats. In other words, don't make it optional.

@NoahGori
Copy link
Author

So its not an option available to users but is an option in the real to string function so that the behavior exists in places where it makes sense. Thats because there are several piece of the code that use those function to display numbers in the gui that don't make sense to have trailing zeroes. For example, in the code editor the line numbers would display with a trailing zero which seems odd. I could build a separate function to display the floats that need the trailing zero but that seems to be a lot of unnecessary work. The option just exists for development so that in the event that features are added that display floats they can be displayed with the trailing zero for clarity but not break all the other places where it doesn't make sense to.

@aaronfranke
Copy link
Member

For example, in the code editor the line numbers would display with a trailing zero which seems odd.

A line number should be an integer, in which case the trailing zero shouldn't be there. The idea is to do this for all floats.

@NoahGori
Copy link
Author

For some reason the editor line numbers (and I suspect other places that would typically be ints) are being represented as floats currently and use the same function so without making it a bool/option in the float to string function they would be displayed with the trailing zero. I agree that all floats should be displayed with the zero but if its implemented for all of them indiscriminately it causes a lot of weird behavior. So by default I have it set that everything is done like it was before without the zeroes and only changed it on places where floats are being displayed and its relevant to tell the difference between floats and ints like within the property editor or print statements although I assume its probably relevant in more places than that. Right now its just easier to make it an option because doing it for all floats requires a bigger change to the code that I'm not sure is necessary for an issue like this but I can go through and try if it's agreed that its very important

@aaronfranke
Copy link
Member

@NoahGori The ideal solution then is to change the script editor to use integers. This would probably require a separate PR which should be merged before this one. The upside is that this work will help us find places where floats are used by accident.

@NoahGori
Copy link
Author

Sounds good. Thats what I thought. I'll go ahead and make another commit to remove the option and make it mandatory

Floats appear with a trailing zero to differentiate between floats and
ints and modify tests to match
@YuriSizov
Copy link
Contributor

YuriSizov commented Jul 14, 2022

The proposal have been approved, can you update your PR against the most recent master, and make the changes if any are required here?

@akien-mga
Copy link
Member

I'm not sure this PR is comprehensive enough. Should be compared with #47502.

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.

Always display exported float variables with one decimal, even when whole numbers
6 participants