jpcsupplies / Economy_mod

Basic Economy System for Space Engineers
13 stars 12 forks source link

DateTime #54

Closed midspace closed 8 years ago

midspace commented 8 years ago

DateTime needs to be stored in UTC. I haven't dealt much with it, because most of the time we deal with local date and time when dealing with apps, but because of how many people from different time zones may connect we need to make sure we get this right.

We need to treat DateTime like another form of Localization. On the server, it is in it's own timezone. when displayed to a user, it should be in their TimeZone.

1. Replace all instances of DateTime.Now with DateTime.UtcNow. (Exclude the TextLogger from these changes, as it is a local logger.)

  1. Examine sending data back instead of messages, so it can be localized. (take MessageListAccounts for example.)

Updated for relevance.

midspace commented 8 years ago
  1. Might not be as critical as I thought, as timezone is stored in DateTime, just never surfaced. See below, with +10:00 and +11:00.
  2. Becomes more important, so DateTime is displayed correctly for individual users.
    <BankAccountStruct>
      <SteamId>888</SteamId>
      <BankBalance>100</BankBalance>
      <NickName>Barney</NickName>
      <Date>2015-09-11T21:17:51.7538783+10:00</Date>
      <OpenedDate>0001-01-01T00:00:00</OpenedDate>
      <Language>0</Language>
    </BankAccountStruct>
    <BankAccountStruct>
      <SteamId>76561197961224864</SteamId>
      <BankBalance>6296.08821818</BankBalance>
      <NickName>Screaming Angels</NickName>
      <Date>2015-10-04T11:18:39.5220149+11:00</Date>
      <OpenedDate>0001-01-01T00:00:00</OpenedDate>
      <Language>0</Language>
    </BankAccountStruct>
jpcsupplies commented 8 years ago

Hmmm... Possible workaround short term - Use date at server, but for all player time checks display as "days/hours/minutes since" too Eg. Last seen stores 1/1/15 or however it is stored server side. When seen command is invoked we see - Player nobody last seen 4/10/15 11:00 (2 hours, 28 minutes, 30 seconds ago)

That way alien time formats still give a useful timeframe no matter what timezone they are in.

Long term we could also attempt to localise the time to match their local timezone and time format, but that seems like a lot of extra work for a basically debugging data value.

Another approach to avoid localised formats is explicity display the time eg 12 January 2016 which should make sense in any country.. this doesnt however resolve the issue of locally adjusting timezone so they see a time that is local.

jpcsupplies commented 8 years ago

Oh, so its basically doing it already?

midspace commented 8 years ago

Yeah, I keep forgetting that .Net has these smarts built in.

I blame work for my confusion. 25 year old system we've progressively upgraded which now is service enabled and can work in the cloud, except it still writes with the local time and not UTC, and does not pass any time zone information. It needs to be fixed. Back on topic.

The display format isn't an issue. .Net will use the user defined format from your OS. As long as the string to display is generated on your system, and not the server.

i'm kind if mixed mind now to change to UTC. It removes the potential for confusion, but make it harder to read the content for debugging.

jpcsupplies commented 8 years ago

How about we just use the work around.. in user displayed content - just break it up into (x number of days hours minutes ago) and not worry about it too much.

jpcsupplies commented 8 years ago

can we close this issue? or is it worth adding the x days y hours z minutes extension to seen command?

midspace commented 8 years ago

For the moment. Until Keen proplerly support resources in mods for localization, its kind of pointless to continue discussion.