AmbientRun / Ambient

The multiplayer game engine
https://ambient.run
Apache License 2.0
3.78k stars 122 forks source link

Use of chrono::offset::local::now() ignores the Windows 10's time zone #519

Open Clauvin opened 1 year ago

Clauvin commented 1 year ago

What's Happening

When I use chrono::offset::local::now() in an ambient program, it returns a date and time that does not have the time zone of the computer where the program runs, instead the time zone returned is +0.

Where's Happening

In a PC with Windows 10, in an Ambient program using as dependencies

ambient_api = "0.2.1" more-asserts = "0.3.1" chrono = "0.4.26"

AND it shows up at the first person camera example (see Extra Commentary, below). May be happening on other programs as well, more testing is required.

How to Reproduce

Extra Commentary

This (or a similar bug) can also be visible at the Ambient's log lines - each log line starts with a [, then the date, time and time zone of when it was printed - running the Ambient program where this bug appeared and also the first person camera example revealed the same problem regarding the wrong date and time at each line:

[2023-06-21T02:29:45Z INFO ambient] Building First person camera [2023-06-21T02:29:46Z INFO ambient] Done building First person camera [2023-06-21T02:29:46Z INFO ambient::server] Creating server

The three date and time above are using UTC+0, but it should be using UTC-3.

philpax commented 1 year ago

Thanks for reporting this!

It looks like it's two issues:

With 1), we'll have to research the going standard across applications to see which timezone our logs should be in. I suspect UTC is better for servers, but localtime is better for usability (i.e. I can see the mdbook server is giving me local time, but you wouldn't run that in production)

For 2), this appears to be a consequence of wasmtime-wasi not supporting timezones yet: https://github.com/bytecodealliance/wasmtime/blob/a43d1dc68fc24e71f726ec858db90f1731e96a11/crates/wasi/src/preview2/preview2/clocks.rs#L63-L80

I'm not sure when this will be fixed, but I believe this will just "start working" once that's available and the preview1-on-2 adapter is adapted for it. I'll keep this issue open until then 🙂

Clauvin commented 1 year ago

Right, thanks 🙂