godotengine / godot

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

Time.get_time_zone_from_system() returns localized timezone name instead of standard abbrev #82361

Open kxn opened 1 year ago

kxn commented 1 year ago

Godot version

Godot v4.1.1.stable - Windows 10.0.22621 - Vulkan (Forward+) - dedicated AMD Radeon RX 5700 XT (Advanced Micro Devices, Inc.; 31.0.21001.45002) - Intel(R) Core(TM) i7-10700K CPU @ 3.80GHz (16 Threads)

System information

Godot v4.1.1.stable - Windows 10.0.22621 - Vulkan (Forward+) - dedicated AMD Radeon RX 5700 XT (Advanced Micro Devices, Inc.; 31.0.21001.45002) - Intel(R) Core(TM) i7-10700K CPU @ 3.80GHz (16 Threads)

Issue description

my system locale is simp chinese and timezone is CST(GMT+8) , and Time.get_time_zone_from_system()["name"] returns

"中国标准时间"

instead of "CST"

In my opinion, the localized timezone name is almost useless since it is hard to process, and there is almost no need to show this string to user in games. so it would be better to just provide the abbrev, e.g "CST" ,"PST", "UTC", etc

Steps to reproduce

print(Time.get_time_zone_from_system()["name"])

Minimal reproduction project

not needed

jsjtxietian commented 1 year ago

After some searching, I concluded that maybe there is no easy way of fixing this other than using Windows Registry. Also it maybe overkill to maintain a translation map for this issue.

Maybe we can use GetDynamicTimeZoneInformation, it has a TimeZoneKeyName which can return China Standard Time instead of "中国标准时间". But maybe that's a breaking change.

aaronfranke commented 1 year ago

What is your goal here? To allow the timezone to be processed? You should use Time.get_time_zone_from_system()["bias"] to get the timezone offset in minutes if you need a machine-readable value.

"CST" is also used for Central Standard Time in the USA, so it's not a unique identifier, and not helpful for the ability for your code to process the timezone.

jsjtxietian commented 1 year ago

You should use Time.get_time_zone_from_system()["bias"] to get the timezone offset in minutes if you need a machine-readable value.

Agreed. But does it imply that the localized timezone name is designed for displaying info rather than use it to calculate something?

aaronfranke commented 1 year ago

Yes, I don't think the name is useful for anything other than displaying.

jsjtxietian commented 1 year ago

I see, In this way I think return a localized timezone name is fine. Should we update the doc too ?

aaronfranke commented 1 year ago

Sure.

kxn commented 1 year ago

What is your goal here? To allow the timezone to be processed? You should use Time.get_time_zone_from_system()["bias"] to get the timezone offset in minutes if you need a machine-readable value.

"CST" is also used for Central Standard Time in the USA, so it's not a unique identifier, and not helpful for the ability for your code to process the timezone.

In fact, I need to send this to a legacy data analysys system, it needs this kind of string as input. No idea why they require such field instead of a timezone offset, but they need a timezone abbrev,

I would slightly argue that godot should provide a way to format time string with this abbrev information because it does show more information than the timezone offset, like daylight saving time.

Anyway, I can workaround this issue by hard coding a 1:1 list, treating all timezone with the same offset as the one I've picked, since I know the system mentioned only uses this information for timezone offset calcuation, not for things like whether it is in daylight saving.

YuriSizov commented 1 year ago

Since we return a dictionary, I don't see why we couldn't add a field with the abbreviation, if it's available, as reported by the host system.

aaronfranke commented 1 year ago

No idea why they require such field instead of a timezone offset, but they need a timezone abbrev,

@kxn But if they use CST for China Standard Time and CST for Central Standard Time then it's not a unique identifier, so your legacy data analysis system will malfunction in this case. I don't think it's meaningful for Godot to support inherently flawed use cases. Hacks for compatibility with your legacy software are fine in your project, but not in the engine.

show more information than the timezone offset, like daylight saving time.

Use Time.get_datetime_dict_from_system()["dst"].

bruvzg commented 1 year ago

ICU has code for getting non localized time name on Windows - https://github.com/godotengine/godot/blob/master/thirdparty/icu4c/common/wintz.cpp, so it (except the part for remapping to ICU name) probably could be copied to the get_time_zone_from_system implementation.

kxn commented 1 year ago

No idea why they require such field instead of a timezone offset, but they need a timezone abbrev,

@kxn But if they use CST for China Standard Time and CST for Central Standard Time then it's not a unique identifier, so your legacy data analysis system will malfunction in this case. I don't think it's meaningful for Godot to support inherently flawed use cases. Hacks for compatibility with your legacy software are fine in your project, but not in the engine.

Well, this could be a lengthy discussion if we talked about design philosphy. As a software engineer I agree with you that the core of software should be as clean as possible. As a person who codes for food, I just find it's not so easy to workaround this with gdscript only.

show more information than the timezone offset, like daylight saving time.

Use Time.get_datetime_dict_from_system()["dst"].