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

MSBuild output garbled #64276

Open
timothyqiu opened this issue Aug 11, 2022 · 14 comments
Open

MSBuild output garbled #64276

timothyqiu opened this issue Aug 11, 2022 · 14 comments

Comments

@timothyqiu
Copy link
Member

timothyqiu commented Aug 11, 2022

Godot version

3.5.stable.mono

System information

Windows 10 & 11

Issue description

I've got a feedback that the build output in MSBuild panel is garbled when there are Chinese characters:

3 5

Works fine on 3.4.5:

3 4 5

  • This is only reproducible if godot is started via .exe. Starting via the .cmd works fine.
  • Regular editor output (GD.Print() to the Output panel) works fine.

Steps to reproduce

Start godot by double clicking the .exe, not the .cmd file.

Build anything in C# in Chinese (probably also any non-English) environment.

Minimal reproduction project

No response

@raulsntos
Copy link
Member

I can't reproduce in Linux, is this only reproducible in Windows?

@timothyqiu
Copy link
Member Author

I think so. Other operating systems usually use UTF-8 for all languages.

@raulsntos
Copy link
Member

C# uses UTF-16 for strings, to set the value in the log (a TextEdit) it marshals the C# string into a Godot String.
There was a PR in 3.5 that touched the marshaling code (#53942) but it shouldn't have affected strings.
Could you check if GD.Print("你好世界") also prints garbled output? Also, do you know if this affects master too?

@timothyqiu
Copy link
Member Author

timothyqiu commented Aug 12, 2022

GD.Print() is good.

I found that starting Godot with _console.cmd works fine. The grabled build output only exists if I start Godot with the .exe directly. (3.4.5 only provides a .exe and it works fine.)

master does not have C# support yet.

@raulsntos
Copy link
Member

C# is supported in master but there have been no alphas released with C# support so you'd have to build it yourself. However, judging from what you are saying I don't think the issue is caused by marshaling or the mono module.

If the issue is related to the changes that were made to support the new Windows Terminal, then I believe the relevant PRs are #55925 and #55967. @bruvzg might know more about it.

@bruvzg
Copy link
Member

bruvzg commented Aug 12, 2022

I was not able to get any non-English output from msbuild (it seems to be ignoring system UI language settings).

But with a few custom apps that's printing wide strings, I get incorrect output when using both wrapper script and executable.

Seems like a regression from #60920, Windows console encoding handling is extremely inconsistent, so it might fix in on a one system and break on another.

@timothyqiu
Copy link
Member Author

I reverted ddb7774 locally and tested, the issue is not resolved.

I found out that msbuild is executed directly with C# API, so it should not be related to OS::execute() modifications.

var startInfo = new ProcessStartInfo(msbuildPath, compilerArgs);
string launchMessage = $"Running: \"{startInfo.FileName}\" {startInfo.Arguments}";
stdOutHandler?.Invoke(launchMessage);
if (Godot.OS.IsStdoutVerbose())
Console.WriteLine(launchMessage);
startInfo.RedirectStandardOutput = true;
startInfo.RedirectStandardError = true;
startInfo.UseShellExecute = false;
startInfo.CreateNoWindow = true;
if (UsingMonoMsBuildOnWindows)
{
// These environment variables are required for Mono's MSBuild to find the compilers.
// We use the batch files in Mono's bin directory to make sure the compilers are executed with mono.
string monoWinBinDir = MonoWindowsBinDir;
startInfo.EnvironmentVariables.Add("CscToolExe", Path.Combine(monoWinBinDir, "csc.bat"));
startInfo.EnvironmentVariables.Add("VbcToolExe", Path.Combine(monoWinBinDir, "vbc.bat"));
startInfo.EnvironmentVariables.Add("FscToolExe", Path.Combine(monoWinBinDir, "fsharpc.bat"));
}
// Needed when running from Developer Command Prompt for VS
RemovePlatformVariable(startInfo.EnvironmentVariables);
var process = new Process {StartInfo = startInfo};
if (stdOutHandler != null)
process.OutputDataReceived += (s, e) => stdOutHandler.Invoke(e.Data);
if (stdErrHandler != null)
process.ErrorDataReceived += (s, e) => stdErrHandler.Invoke(e.Data);
process.Start();

@raulsntos
Copy link
Member

@bruvzg If you are using dotnet CLI as the build tool, you can set the language by setting an environment variable DOTNET_CLI_UI_LANGUAGE=zh-cn (I think that's the locale for Chinese).

@timothyqiu The only thing that changed between 3.4.5 and 3.5 in BuildSystem was the addition of startInfo.CreateNoWindow = true in #60956 (since .NET Core does not support creating windows on Unix-like platforms, this property only affects Windows). I don't think that'd be causing the issue though.

@timothyqiu
Copy link
Member Author

The only thing that changed between 3.4.5 and 3.5 in BuildSystem was the addition of startInfo.CreateNoWindow = true in #60956 (since .NET Core does not support creating windows on Unix-like platforms, this property only affects Windows). I don't think that'd be causing the issue though.

Yeah. I removed CreateNoWindows = true and it's still garbled when running the exe directly.

@raulsntos
Copy link
Member

raulsntos commented Aug 12, 2022

Since I'm out of ideas I think then the best way to find the commit that caused the regression is to test with the 3.5 betas and RCs to narrow the range of possible commits and then git bisect them.

@timothyqiu
Copy link
Member Author

timothyqiu commented Aug 13, 2022

Bisected and the first bad commit is from #55987 CC @bruvzg

The issue is gone if I change the linker flags back (/SUBSYSTEM and /ENTRY).

@DmitriySalnikov
Copy link
Contributor

4.0 still uses the wrong encoding

If you change the dotnet language for example to Russian (set DOTNET_CLI_UI_LANGUAGE=ru) and then run the build of the project, only ╨╡ ╨┐╤А╨╛╨╡╨ will be printed..

image
v4.0.beta16.mono.official [518b9e5]

@nagilson
Copy link

@DmitriySalnikov Note that this was an issue independent of Godot in the .NET SDK as well, which should be fixed in 8.0 preview 1 and 7.0.300 dotnet/sdk#29755

@yurit85
Copy link

yurit85 commented Dec 13, 2023

An example of what might be wrong: OS_Windows::execute uses MultiByteToWideChar with CP_ACP. CP_ACP defaults to 125x.
https://en.wikipedia.org/wiki/Windows_code_page
Windows defaults to 125x for the window and DOS code page for the console.
You need to take CP_OEMCP for the console. And determine which application you are running, console or window)) Most often, pipes are used only by console and window ones are silent.
More info:
https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getoemcp
https://learn.microsoft.com/en-us/windows/win32/api/winnls/nf-winnls-getacp
https://learn.microsoft.com/en-us/windows/console/getconsoleoutputcp
https://learn.microsoft.com/en-us/windows/console/getconsolecp

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.

7 participants