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

Store Control constants as floats instead of integers #39013

Closed

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented May 24, 2020

This allows to store values like the editor scale in a constant. This way, we can eventually remove the need to include editor-only headers in source files in the scene/ folder.

The editor still appears to look correct, but please test for regressions anyway. I'd be surprised if everything worked as expected right now 🙂

This allows to store values like the editor scale in a constant.
This way, we can eventually remove the need to include editor-only
headers in source files in the `scene/` folder.
@aaronfranke
Copy link
Member

Perhaps double should be used instead, to allow for (reliable) integers higher than 16 million?

@Calinou
Copy link
Member Author

Calinou commented May 25, 2020

@aaronfranke Maybe, but I can't think of what you'd need such large integers for in an UI context. The memory usage difference will probably be quite small and acceptable for 4.0, but I'd need approval from reduz first.

That said, scalar floats in GDScript are already 64-bit, so this would make custom constants consistent with them.

@TechnoPorg
Copy link
Contributor

Is this PR still wanted, now that EDSCALE dependencies in scene (except for one in path_2d.cpp?) have been removed by #53341?

@YuriSizov
Copy link
Contributor

In general, it would be useful to have constants other than integers. While using ints as booleans is normal for a programmer, it doesn't lead to a great user experience. Floats can be useful for several things as well. However, I suspect this PR needs to be redone at this point, rather than rebased.

@reduz
Copy link
Member

reduz commented Jul 28, 2022

I think editor scale probably should be accessible from the viewport/window, not from a theme property.

@Calinou
Copy link
Member Author

Calinou commented Jul 28, 2022

I think editor scale probably should be accessible from the viewport/window, not from a theme property.

This was added in #53341.

I think float constants still have a few benefits, such as setting floating-point font sizes (even if the final rendering rounds them to integer sizes, at least for non-MSDF fonts). This gives you more control over a font's final size after oversampling.

@YuriSizov
Copy link
Contributor

I think that ideally theme constants could allow any Variant constant, so bools would be true bools, and numbers can be both int and float, and maybe someone naughty would store something else in there.

@YuriSizov YuriSizov modified the milestones: 4.0, 4.x Sep 8, 2022
@YuriSizov
Copy link
Contributor

I think we can have a more flexible solution here, replacing ints with floats has a few downsides too. We should come up with a non-breaking solution during the 4.x development.

@YuriSizov YuriSizov closed this Jan 25, 2023
@aaronfranke aaronfranke removed this from the 4.x milestone Feb 1, 2023
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.

5 participants