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

Add GUILabel (colour code + currency icon support) #251

Merged
merged 4 commits into from
Aug 30, 2024
Merged

Conversation

Hop311
Copy link
Contributor

@Hop311 Hop311 commented Aug 15, 2024

No description provided.

@Hop311 Hop311 added the enhancement New feature or request label Aug 15, 2024
@schombert
Copy link

I don't know if you ever plan on supporting unicode text, but if you are thinking about it static constexpr char COLOUR_CHAR = '\xA7'; // § will give you a headache, since that character will end up as more than one byte if it goes through a win1252 to utf8 routine first (since you would probably end up using utf8 as your internal representation and then converting existing text from game files to that on load, rather than having two code paths for all text)

@Spartan322
Copy link
Member

I don't know if you ever plan on supporting unicode text, but if you are thinking about it static constexpr char COLOUR_CHAR = '\xA7'; // § will give you a headache, since that character will end up as more than one byte if it goes through a win1252 to utf8 routine first (since you would probably end up using utf8 as your internal representation and then converting existing text from game files to that on load, rather than having two code paths for all text)

He needs to use string_view, not char, we already support utf8 and the conversion happens all the way back in the dataloader.

@schombert
Copy link

He needs to use string_view, not char, we already support utf8 and the conversion happens all the way back in the dataloader.

Ah, so then it is only working by accident. '\xA7' from your ascii code page becomes the two-byte sequence C2 A7 in utf8, and so this char happens, by accident, to match the last byte in that sequence and you haven't happened across any other text that contains a different multi-byte sequence ending in A7

@schombert
Copy link

schombert commented Aug 19, 2024

I imagine that embedding strings like _balance_label.text = "§%s%s¤" are going to be pretty fragile then since your source code doesn't specify an encoding, and the C++ compiler is free IIRC to use either an ASCII code page or utf8 or really whatever makes it happy.

@schombert
Copy link

same also holds for static constexpr char CURRENCY_CHAR = '\xA4'

@Spartan322
Copy link
Member

Spartan322 commented Aug 20, 2024

Well Godot strings are inherently utf8, they'll try to read it as ISO 8859-1 if you just shove the single byte values beyond ASCII in without the utf functions, however Godot strings themselves store everything as utf8 and all strings made in GDScript are utf8. And in my experience so far no compiler has caused any issues storing utf8 in string_view. (the closest I think of is the GA's Ubuntu distribution of GCC would hang when trying to store a specific utf8 encoded multi-byte character in our testing platform to the dataloader, but wouldn't have any issue storing it in the normal path of the program, it works fine when used, but the test somehow borks GCC when non-ascii characters were there)

@Hop311
Copy link
Contributor Author

Hop311 commented Aug 22, 2024

@schombert thanks for the advice, I was using Windows-1252 character codes from previous Vic2 localisation analysis code, changed it all now to use godot::Strings, created from unicode code points in cases where the string literal may be unreliable.

@Hop311 Hop311 changed the title Add GUITextLabel (colour code + currency icon support) Add GUILabel (colour code + currency icon support) Aug 26, 2024
Copy link
Member

@Spartan322 Spartan322 left a comment

Choose a reason for hiding this comment

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

Works on Linux

@Hop311 Hop311 merged commit f54e454 into master Aug 30, 2024
18 checks passed
@Hop311 Hop311 deleted the gui-text-label branch August 30, 2024 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants