Try to convert OS::execute() output to Unicode on Windows
#60920
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timothyqiu
added
bug
platform:windows
topic:core
cherrypick:3.x
bruvzg
reviewed
May 10, 2022
There was a problem hiding this comment.
MultiByteToWideChar probably will work for most cases
But I'm not sure about reading everything to the buffer and converting it at once. This will prevent receiving long output in portions (e.g., from the other thread).
Breaking it at the arbitrary points might break multibyte conversion, but the same is true for old code and Unix version. So might be better to look for the new like characters and do converting/adding to string for a single line at a time.
|
Updated to convert available lines at each iteration. |
|
Thanks! |
|
Cherry-picked for 3.5. |
akien-mga
removed
the
cherrypick:3.x
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #60897
Windows defaults to ANSI instead of 8-bit ASCII. Full characters are sent but using different encodings according to the code page. This is why godot prints lots of "Unicode parsing error" when trying to interpret these bytes as UTF-8 which is most of the time an incorrect encoding (but compatible with the ASCII part).
This PR uses
MultiByteToWideCharto convert the output bytes from system default ANSI encoding to Unicode (wchar_t) so that aStringcan be constructed correctly. If the API fails, we fallback to the original "treat as UTF-8" approach.I only tested this on a Simplified Chinese version of Windows. CC @bruvzg
For cherry-pick:
is_empty()→empty().