Update to use UTC DateTime instead of Local DateTime

teshiba / AoE2.netDesktop

AoE2.netDesktop is a desktop application that makes viewing "Age of Empires II: Definitive Edition" play history easy.

MIT License

2 stars 2 forks source link

Update to use UTC DateTime instead of Local DateTime #39

Closed Bo-Jangles closed 2 years ago

Bo-Jangles commented 2 years ago

Describe the bug One of the test cases is failing because it is time zone dependent: PlotTest in PlayerRateFormsPlotTests.cs

To Reproduce Run the above test in a time zone other than GMT+9.

Screenshots before

Additional context Several changes are required to address time zone dependency:

DateTimeOffset.FromUnixTimeSeconds() returns a UTC date so using the UtcDateTime property would make the code timezone independent.
Similarly, the DateTime.Now.ToUniversalTime() method should be used instead of the DateTime.Now.ToLocalTime() method.

teshiba commented 2 years ago

@Bo-Jangles Could you run the test on the develop branch?

I recently found this fails on the GitHub Actions environment. On develop branch, I've fixed the issue and pushed the commits.

Thanks for reporting about the 'unit test' as well! Especially, it is hard to find localizing issues.

There are the latest codes on develop branch! If you could report unit test issues, please check it out!

I'll check PR #40 later as well.

Bo-Jangles commented 2 years ago

I've run the test on the develop branch. It passes! I will use this branch going forward.

As for PR #40 I think that my change to the following file is incorrect because local time is required here. AoE2.netDesktop/LibAoE2Net/Functions/MatchExt.cs

This PR can be ignored because it was based on outdated code.