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

Label.get_total_character_count returns too high value #23720

Closed
ttencate opened this issue Nov 15, 2018 · 3 comments
Closed

Label.get_total_character_count returns too high value #23720

ttencate opened this issue Nov 15, 2018 · 3 comments
Milestone

Comments

@ttencate
Copy link
Contributor

ttencate commented Nov 15, 2018

Godot version: 3.0.6

OS/device including version: Arch Linux

Issue description:

The get_total_character_count() function on Label returns strange values. For the empty string, it return 1. For a string of one non-space character, it returns 3. For increasingly longer strings, it seems increasingly far off the mark.

It's not clear from the documentation why there should ever be a difference between get_total_character_count() ("Returns the total length of the text.") and len(text). So at the very least there's a doc bug here.

Steps to reproduce:

Create a scene with a Label at the root. Attach the following script:

extends Label
func _ready():
	for t in ["", " ", "x", "hello", "hello world", "Hello world!", "Some longer phrase. With punctuation and stuff."]:
		text = t
		print("text = %-14s     len(text) = %2d  get_total_character_count() = %2d" %
			['"%s"' % text, len(text), get_total_character_count()])

Run the scene and observe the output:

text = ""                 len(text) =  0  get_total_character_count() =  1
text = " "                len(text) =  1  get_total_character_count() =  2
text = "x"                len(text) =  1  get_total_character_count() =  3
text = "hello"            len(text) =  5  get_total_character_count() =  7
text = "hello world"      len(text) = 11  get_total_character_count() = 12
text = "Hello world!"     len(text) = 12  get_total_character_count() = 13
text = "Some longer phrase. With punctuation and stuff."     len(text) = 47  get_total_character_count() = 43

Related issues:

#874 has @reduz saying he fixed it very hard. But that was probably back in the 2.0 days.

@akien-mga akien-mga added this to the 3.1 milestone Nov 15, 2018
@ttencate
Copy link
Contributor Author

Update: looking at the code (Label::regenerate_word_cache), it seems that get_total_character_count() returns:

  • the number of non-space, non-newline characters (0x20 and 0x0a only)
  • plus 2 if the string is nonempty, else plus 1

"Regular" characters are counted here. Non-newlines are counted here, and spaces are subtracted from the count here.

The reason we get the +2 is that the loop runs up to xl_text.size() + 1. This is usually 2 larger than xl_text.length() because size() includes the null terminator. The last two characters are interpreted as spaces here but thanks to this if they are not excluded from the count.

The reason we get only +1 for an empty string is, I guess, that xl_text.size() probably returns 0 in that case, as the String class doesn't bother to create an actual array for this case.

So I guess this is what should be done:

  • Document that get_total_character_count only includes "printable" characters.
  • Use xl_text.length() instead of xl_text.size() in the loop condition.
  • Make sure to also subtract the "terminator" space from the count (or just not count spaces in the first place).

I'll whip up a PR.

There's also a potential issue with other space-like characters, in ASCII but especially in Unicode, but I'll leave that for another day.

@akien-mga
Copy link
Member

Document that get_total_character_count only includes "printable" characters.

What's the use case for knowing the number of non-space characters though? i.e. when would one use the (fixed) get_total_character_count() instead of just text.length()?

@ttencate
Copy link
Contributor Author

ttencate commented Nov 15, 2018

It's what visible_characters is using to animate the text appearing gradually.

This bug is also affecting percent_visible. E.g. with the string "a b c", we set visible_characters to 3 and it's all visible, but percent_visible is only 0.6.

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.

2 participants