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

[4.3 beta 1] Godot Wayland produces corrupt config if language does not use '.' as decimal separator #92594

Open
Tracked by #88346
DasCapschen opened this issue May 31, 2024 · 10 comments

Comments

@DasCapschen
Copy link

Tested versions

System information

Godot v4.3.beta1 - openSUSE Tumbleweed 20240523 - Wayland - Vulkan (Forward+) - dedicated AMD Radeon RX 580 Series (RADV POLARIS10) () - Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz (8 Threads)

Issue description

Godot writes invalid editor_settings-4.3.tres and invalid project.godot files, when the User's language does not use a period as a decimal separator.
Floats get represented like 0,57 instead of 0.57, causing issues like Parse Error: Expected 4 arguments for constructor.

This prevents Godot from starting until editor_settings file is fixed.

Starting Godot with LANG=C ./godot fixes the problem.

Steps to reproduce

  • run Godot 4.3 Beta 1 with PC language set to German
  • change colors in Theme settings of the Editor
  • close Godot
  • start Godot
  • crashes immediately, Parse Error: Expected 4 arguments for constructor

Minimal reproduction project (MRP)

n.a.

@DasCapschen
Copy link
Author

Addition:
This might be related to Wayland. After starting the Editor for the first time, I immediately turned on Wayland and clicked "save & restart". The engine froze. I killed the process, restarted, and the config file was invalid.

@akien-mga akien-mga added this to the 4.3 milestone May 31, 2024
@Calinou
Copy link
Member

Calinou commented Jun 1, 2024

Can you reproduce this in 4.2.2? I've never heard of this issue before, as Godot does not use language-specific decimal separators when printing numbers in the first place.

@DasCapschen
Copy link
Author

No, I've been using 4.2.2 (v4.2.2.stable.openSUSE [15073af] to be exact) the whole time and never had this issue.

@DasCapschen
Copy link
Author

I just tried again, and it only happens with prefer_wayland = true.

I'll attach a Screencast:

Screencast_20240601_125532.webm

@akien-mga akien-mga changed the title [4.3 beta 1] Godot produces corrupt config if language does not use '.' as decimal separator [4.3 beta 1] Godot Wayland produces corrupt config if language does not use '.' as decimal separator Jun 2, 2024
@Riteo
Copy link
Contributor

Riteo commented Jun 2, 2024

Hi, thanks for the report!

I'm not aware of any code on Wayland that would directly interfere with string printing. The core string logic uses snprintf or sprintf depending on the current libc:

godot/core/string/ustring.cpp

Lines 1878 to 1912 in 705b7a0

String String::num_scientific(double p_num) {
if (Math::is_nan(p_num)) {
return "nan";
}
if (Math::is_inf(p_num)) {
if (signbit(p_num)) {
return "-inf";
} else {
return "inf";
}
}
char buf[256];
#if defined(__GNUC__) || defined(_MSC_VER)
#if defined(__MINGW32__) && defined(_TWO_DIGIT_EXPONENT) && !defined(_UCRT)
// MinGW requires _set_output_format() to conform to C99 output for printf
unsigned int old_exponent_format = _set_output_format(_TWO_DIGIT_EXPONENT);
#endif
snprintf(buf, 256, "%lg", p_num);
#if defined(__MINGW32__) && defined(_TWO_DIGIT_EXPONENT) && !defined(_UCRT)
_set_output_format(old_exponent_format);
#endif
#else
sprintf(buf, "%.16lg", p_num);
#endif
buf[255] = 0;
return buf;
}

The only weird thing to me, which has been there for years though, is that we set the user locale in the Linux main to an empty string, which means the user's preferred locale:

setlocale(LC_CTYPE, "");

I wonder if removing it, or forcing it to "C" just in case would change things. This does not make much sense though :/

Further help would be appreciated.

@DasCapschen
Copy link
Author

Well, the Man-Page for sprintf says the decimal separator depends on LC_NUMERIC. So setting LC_CTYPE should not change that. But maybe it is worth giving that a try anyways:

  • the Unix Env Var Spec says that it will use LANG in absence of LC_ALL and other LC_* variables. I wonder if this is true for all the LC_* variables. So perhaps if LC_NUMERIC is unset, it uses uses the next best thing (perhaps LC_CTYPE, if set).

Other than that, I guess the only reason it happens with Wayland is that Wayland sets (or doesn't set) these Variables differently from X11.

@Riteo
Copy link
Contributor

Riteo commented Jun 3, 2024

@DasCapschen uh interesting. Perhaps XWayland (or the X.org server in general) generates a full locale'd environment. Testing this should not even need changing the code.

If you happen to try running the engine with LC_NUMERIC=C in the environment, please update us :D

@DasCapschen
Copy link
Author

DasCapschen commented Jun 3, 2024

Well, I just inspected /proc/<pid>/environ to find differences between running with prefer_wayland = true and false, and... there is absolutely no difference... if I set LANG=C, the only difference is that LANG=C instead of LANG=de_DE.UTF-8 ...
Interestingly, apart from LANG, no other LC_* variables are set at all!

Setting LC_NUMERIC=C does solve the problem!
Setting LC_CTYPE=C does not solve the problem.

@DasCapschen
Copy link
Author

It definitely happens inside snprintf
Here's an ltrace of Godot without prefer_wayland:

[pid 21555] Godot_v4.3-beta1_linux.x86_64->snprintf("0.56", 256, "%lg", 0,560000) = 4

And now with prefer_wayland...

[pid 21740] Godot_v4.3-beta1_linux.x86_64->snprintf("0,56", 256, "%lg", 0,560000) = 4

As you can see, the output with prefer_wayland takes locale into account, while without it, it does not 🤔
So... is something weird going on inside libc?!

@Riteo
Copy link
Contributor

Riteo commented Jun 3, 2024

@DasCapschen

Setting LC_CTYPE=C does not solve the problem.

That's to be expected, as the generic linuxbsd code overwrites it.

But the rest's just baffling...

The fact that LC_NUMERIC=C fixes the problem is nice (and probably something we had to do in the first place), but how did it work all along on the X11 backend, since the only thing that Godot does is explicitly set a non-portable locale?!

As you can see, the output with prefer_wayland takes locale into account, while without it, it does not 🤔
So... is something weird going on inside libc?!

That's the only thing I can think of, but still, if the environment has no other stuff... Oh, glibc... I wonder if there might be any other condition, or some weird non-standard thread-local stuff messing this stuff up.

Perhaps either platform lib is doing something behind our backs? Gosh, what a mess.

At this point I'd just try forcing LC_NUMERIC=C at startup, unless it messes up the UI's numeric localization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants