godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.17k stars 20.21k forks source link

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

Open DasCapschen opened 3 months ago

DasCapschen commented 3 months ago

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

Minimal reproduction project (MRP)

n.a.

DasCapschen commented 3 months ago

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.

Calinou commented 3 months ago

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 commented 3 months ago

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

DasCapschen commented 3 months ago

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

I'll attach a Screencast:

Screencast_20240601_125532.webm

Riteo commented 3 months ago

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:

https://github.com/godotengine/godot/blob/705b7a0b0bd535c95e4e8fb439f3d84b3fb4f427/core/string/ustring.cpp#L1878-L1912

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:

https://github.com/godotengine/godot/blob/705b7a0b0bd535c95e4e8fb439f3d84b3fb4f427/platform/linuxbsd/godot_linuxbsd.cpp#L65

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 commented 3 months ago

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:

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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.

Riteo commented 1 month ago

Oh no... I think I found something: https://x.org/releases/current/doc/libX11/libX11/libX11.html#X_Locale_Management

This is still speculation, but judging from this line I fear that xlib might actually be setting the locale in weird ways:

On implementations that conform to the ANSI C library, the locale announcement method is setlocale. This function configures the locale operation of both the host C library and Xlib. The operation of Xlib is governed by the LC_CTYPE category; this is called the current locale.

(emphasis mine)

If setting LC_NUMERIC=C does not mess localization up, we can make a PR for 4.3.1.

I should've probably looked into this earlier, sorry.

Edit: nevermind, it talks about the libc setlocale... I'll have to look deeper if I want to find what's going on.