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

[Docs] Clarify that String parsing methods don't support num separators #97332

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

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Sep 22, 2024

Even if we chose to implement this in the future it'd be good to clear this up in 4.2 and 4.3, might easily be confusing as:

  • to_int does ignore these
  • GDScript allows them

Can explicitly clarify the case in Expression but I think that class needs a more general clarification on how it works and what it does actually support so leaving it for now.

See:

@AThousandShips AThousandShips added enhancement documentation cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Sep 22, 2024
@AThousandShips AThousandShips added this to the 4.4 milestone Sep 22, 2024
@AThousandShips AThousandShips requested a review from a team as a code owner September 22, 2024 16:49
Comment on lines 895 to 927
var a = "12.35".to_float() # a is 12.35
var b = "1.2.3".to_float() # b is 1.2
var c = "12xy3".to_float() # c is 12.0
var d = "1e3".to_float() # d is 1000.0
var e = "Hello!".to_float() # e is 0.0
var f = "1_2".to_float() # f is 1.0 (number separators are not supported)
Copy link
Member Author

Choose a reason for hiding this comment

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

This part was using to_int incorrectly so copied over the String version for correctness while at it

@Mickeon
Copy link
Contributor

Mickeon commented Sep 22, 2024

Not sure this actually solves the mentioned issue, as this information is still hidden behind the String methods, which are not directly tied to the Expression class, at least not documented as such.

@AThousandShips
Copy link
Member Author

It doesn't, hence why not linked, it's related and what inspired this change

@Mickeon
Copy link
Contributor

Mickeon commented Sep 22, 2024

Also this should probably either be a note, the example should speak for itself without additional remarks in the same line, or both. As by the rest of this class reference page, you shouldn't need the code blocks to understand the methods.

@AThousandShips
Copy link
Member Author

If says it only supports digits, so this is just "hey keep this one in mind" for those who miss that, it doesn't add any information the description doesn't have, just makes it clearer (it doesn't explicitly say "you can't put letters in a number")

@AThousandShips AThousandShips modified the milestones: 4.4, 4.x Jan 23, 2025
@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 19, 2025

Verified

This commit was signed with the committer’s verified signature.
AThousandShips A Thousand Ships
…tors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants