sourcemod icon indicating copy to clipboard operation
sourcemod copied to clipboard

FormatTime() and time correcting #875

Open komashchenko opened this issue 6 years ago • 12 comments

Implementing time formatting without taking into account time zone changes compatibility old plugins not be broken

komashchenko avatar Oct 29 '18 22:10 komashchenko

Implementing time formatting without taking into account time zone changes compatibility old plugins not be broken

I would also suggest changing the commit description to something like this:

Implement the ability to use local time formatting without breaking compatibility with existing plugins.

And the commit title to something like this:

Added localtime parameter to FormatTime();

Other than that, I see no reason why we can't merge this.

sm9cc avatar Oct 31 '18 13:10 sm9cc

What's your use case?

peace-maker avatar Nov 02 '18 14:11 peace-maker

What's your use case?

Necessary to record time UTC in database.

komashchenko avatar Nov 04 '18 12:11 komashchenko

I would have a use for this one too

Kenzzer avatar Nov 04 '18 14:11 Kenzzer

It looks we don't have any way to bypass sm_time_adjustment, which makes this not-very-useful for people using that for timezone correction.

asherkin avatar Nov 11 '18 18:11 asherkin

I'm still struggling to see the usefulness of sm_time_adjustment. I'm not sure I've seen any other application out there have a similar mechanism that's actively in-use by people.

KyleSanderson avatar Nov 11 '18 18:11 KyleSanderson

It’s so people who rent severs from a GSP can set the timezone correctly.

asherkin avatar Nov 11 '18 21:11 asherkin

What's your use case?

Necessary to record time UTC in database.

MySQL/MariaDB UTC_TIMESTAMP(), maybe ?

It’s so people who rent severs from a GSP can set the timezone correctly.

If things should change, it would make more sense to make an entry in addons/sourcemod/configs/core.cfg for the server's timezone, similar to the Linux system's /etc/timezone, e.g. "America/New_York" or "Europe/London".

A similar per-player setting could then be introduced, so it can also be adjusted for each individual player.

The latest "localtime" bool variable of FormatTime could then be the client id, of which it should be changed according to, e.g. "0" (which would be default) would imply the server, and be relative to the setting from core.cfg, where anything else would point towards a specific client id.

Or, maybe for simplicity, the latest "localtime" variable could simply just be a text string, e.g. "America/New_York" or "Europe/London".

DarkDeviL avatar Nov 25 '18 17:11 DarkDeviL

~~I think it'd be more useful to have this option in GetTime, rather than in FormatTime.~~

Headline avatar Nov 27 '18 02:11 Headline

@asherkin before I ping komashchenko on this one - how do you feel about it?

I can see it being useful, but additionally with FormatTime I always included the TZ as end-consumers already had a hint about where things were hosted (per latency to the box). You're right for GSPs this scenario is useful.

Anyway, I'm conflicted because it's legitimately changing time underneath people. In a totally amateur plugin I wrote in 2009 I have FormatTime(Clock, sizeof(Clock), "%I:%M:%S%p %Z", TimeCache); which to me shows this is beyond user error.

KyleSanderson avatar Aug 16 '20 23:08 KyleSanderson

@KyleSanderson At the time of creation, I wanted to use it with TZD, since I was getting the player's local time, then when formatting via FormatTime it turned out to be wrong, so I just added myself the TZD_FormatTime function

komashchenko avatar Aug 17 '20 08:08 komashchenko

To reactivate this old PR: I would also have a use case for this: Using SourceTV Manager, I've written a plugin which automatically starts/stops recording of server-side demos and uploads them to a FTP server so they can be downloaded.

To avoid people having to know which time zone the server is in, I would like to store the recording start time as a UTC string in the filename, which currently is not possible, as FormatTime uses the server's timezone, which can either be EST or CE(S)T in my case. Moreover, I've also written a script which automatically links said demos to bans in Sourcebans for evidence purposes and which currently requires extra (and imo unnecessary) code for timezone adjustment per server.

Not sure what else is preventing this PR from being merged, doesn't seem like it breaks bcompat.

awillinger avatar Jul 22 '23 21:07 awillinger