Skip to content

Fix ScriptEditor inline colors float handling #107904

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

Merged

Conversation

kitbdev
Copy link
Contributor

@kitbdev kitbdev commented Jun 23, 2025

Mostly whitespace changes since I tried to make the function easier to read.

split_floats just gives 0.0 when it encounters non-numbers, so I removed it and now check is_valid_float for each parameter, with a maximum of 4.
It must be a float literal, variables and expressions are not allowed here. It may be possible to use Expression to handle them, but it would still need to be readonly.

Also added checks when typing a string parameter to only show the color if it is valid and to prevent some invalid color errors (not all, see #89357).

This function will need to be moved to be language independent in the future, then it can be parsed better with GDScript.

@kitbdev kitbdev added this to the 4.5 milestone Jun 23, 2025
@kitbdev kitbdev requested a review from a team as a code owner June 23, 2025 17:58
@akien-mga akien-mga requested a review from KoBeWi June 25, 2025 16:24
@kitbdev kitbdev force-pushed the fix-script-inline-color-require-floats branch from 16c3fc5 to 1472b80 Compare July 4, 2025 15:54
@kitbdev
Copy link
Contributor Author

kitbdev commented Jul 4, 2025

Updated to show default color if the string isn't valid so it doesn't keep dissapearing and reappearing while typing.
Also now works with single quote strings. Though it will be changed to double quote when changing color.
Added more checks to prevent these cases being considered valid:

Color(")
Color("aaa)
Color(0xawaaaaaa)
Color(0x0 + aaaa)
Color("ff"+red)

There are still some colors that should be considered valid (like hex without alpha and more static methods) but those can be done later.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Works fine now.
Needs rebase.

@kitbdev kitbdev force-pushed the fix-script-inline-color-require-floats branch from 1472b80 to d8e6381 Compare July 5, 2025 17:02
@kitbdev kitbdev force-pushed the fix-script-inline-color-require-floats branch from d8e6381 to 9184256 Compare July 8, 2025 16:02
@Repiteo Repiteo merged commit 539cb03 into godotengine:master Jul 9, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jul 9, 2025

Thanks!

@kitbdev kitbdev deleted the fix-script-inline-color-require-floats branch July 9, 2025 16:53
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.

Inline color picker doesn't work well with non-constant colors
4 participants