Page MenuHomeWildfire Games

Add datetimeToString and monthToString functions
Needs ReviewPublic

Authored by temple on Dec 6 2017, 4:45 AM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Add datetimeToString and monthToString functions.

Test Plan

monthToString is terrible, so it's a question of which to do first, this patch or D367.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4008
Build 7029: Vulcan BuildJenkins
Build 7028: arc lint + arc unit

Event Timeline

temple created this revision.Dec 6 2017, 4:45 AM
Owners added a subscriber: Restricted Owners Package.Dec 6 2017, 4:45 AM
Vulcan added a subscriber: Vulcan.Dec 6 2017, 4:47 AM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
elexis added a subscriber: elexis.Dec 7 2017, 2:59 PM

Patch correct and complete otherwise. All other occurrences of the Engine time format call are HH:mm.

binaries/data/mods/public/gui/common/functions_utility_loadsave.js
1

(I hate this file as much as functions_utilits.js. Could possibly exclude this from this directory one way or another.)

binaries/data/mods/public/gui/common/l10n.js
15 ↗(On Diff #4594)

By definition, gui/common/ files are possibly useful on any GUI page. Hence the mentioned GUI pages may only serve as examples (i.e. the second sentence, or the second part of the sentence of the comment. The first part should define the global use case.)

elexis added a comment.Dec 7 2017, 3:15 PM

monthToString is terrible, so it's a question of which to do first, this patch or D367.

So not that terrible and D367 is also not the most appealing implementation, at least in the current iteration IMO

binaries/data/mods/public/gui/common/l10n.js
15 ↗(On Diff #4594)

(Just in english there are several ways to interpret monthToString. For intsance Dec 17 and 2017-12. But pointing out the difference (that the one is a numeric format and there exists a more clear order) will probably lead to more confusion than clarification.)

temple added a comment.Dec 7 2017, 3:40 PM

I didn't update the links to go to the official site.
http://userguide.icu-project.org/formatparse/datetime#TOC-Date-Field-Symbol-Table

binaries/data/mods/public/gui/common/l10n.js
15 ↗(On Diff #4594)

So you'd prefer "...as shown for example in..."?

temple added inline comments.Dec 7 2017, 3:51 PM
binaries/data/mods/public/gui/common/functions_utility_loadsave.js
15

This includes the seconds :ss but I'm removing that because it seems unnecessary and so that we can have just one datetime format.

elexis added inline comments.Dec 7 2017, 6:46 PM
binaries/data/mods/public/gui/common/functions_utility_loadsave.js
15

Sometimes you have two savegames in the same minute, so it's not useless being able to distinguish them, nay?

binaries/data/mods/public/gui/common/l10n.js
15 ↗(On Diff #4594)

yeah

temple updated this revision to Diff 4643.Dec 8 2017, 4:41 AM
temple added inline comments.
binaries/data/mods/public/gui/common/functions_utility_loadsave.js
15

Okay. (I don't care. :) )

Vulcan added a comment.Dec 8 2017, 5:13 AM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
temple added a reviewer: Restricted Owners Package.Jan 17 2018, 1:21 AM