Correct lobby chat timestamps.
rP19801
Actions

Description

Correct lobby chat timestamps.

Only historic messages were timestamped in C++ prior.
Other messages showed the timestamp when they were displayed in JS, which is wrong in case of returning to the lobby from a game.

Differential Revision: https://code.wildfiregames.com/D514
Fixes #3832.
Reverts the revert of rP17928 in rP17941.
Patch By: Josh
Tested By: Itms

Details

Committed

elexis

Jun 18 2017, 8:48 PM

Differential Revision

D514: Store lobby timestamps in C++

Parents

rP19800: Fix Valgrind complaints in CRenderer by properly intializing some members…

Branches

Unknown

Tags

Unknown

Build Status

Buildable 2334
Build 3880: Trigger Windows Autobuild
Build 3879: Post-Commit Build	Jenkins

Event Timeline

elexis committed rP19801: Correct lobby chat timestamps..Jun 18 2017, 8:48 PM

Harbormaster completed building B2334: rP19801: Correct lobby chat timestamps..Jun 18 2017, 9:37 PM

elexis mentioned this in rP19873: Fix a reference error in rP19801 / D514 reported by causative..Jul 6 2017, 3:05 AM

• leper raised a concern with this commit.Jul 28 2017, 6:37 PM

• leper added subscribers: Itms, • leper.

• leper added inline comments.

/ps/trunk/source/lobby/XmppClient.cpp
958	`timezone` is not required by any standard, but is an XSI extension. It is not present on BSD (or if one can trust the man pages Apple provides) OSX. Both FreeBSD and OSX also have timezone(3) which is a function that doesn't do what this code wants, and also has a different return type. (OpenBSD seems to have removed this one.) Also using `timezone` without any previous call to tzset(3) is likely to leave the contents of `timezone` to be something unspecified (though given how things work, and other programs calling it) possibly 0 or the correct value, but this is not in any way ensured by the code. TL;DR: This code doesn't conform to standards supported by all platforms we support, is broken in the current state, and prevents builds on at least a few platforms currently.

This commit now has outstanding concerns.Jul 28 2017, 6:37 PM

• leper added inline comments.Jul 29 2017, 10:04 PM

/ps/trunk/source/lobby/XmppClient.cpp
958	OSX removed the BSD/AT&T Unix Version 7 timezone(3) sometime before 10.6 (I stopped digging at some point, because who cares), but after 10.0, and also introduced the XSI `timezone` var (behind some UNIX_03 define, that is mostly likely on by default. They however didn't update the documentation to reflect any of that.