Implement various todo's and review suggestions

client/GameChatHandler.cpp

@@ -26,15 +26,6 @@
 #include "../lib/CConfigHandler.h"
 #include "../lib/MetaString.h"
 
-static std::string getCurrentTimeFormatted(int timeOffsetSeconds = 0)
-{
-	// FIXME: better/unified way to format date
-	auto timeNowChrono = std::chrono::system_clock::now();
-	timeNowChrono += std::chrono::seconds(timeOffsetSeconds);
-
-	return TextOperations::getFormattedTimeLocal(std::chrono::system_clock::to_time_t(timeNowChrono));
-}
-
 const std::vector<GameChatMessage> & GameChatHandler::getChatHistory() const
 {
 	return chatHistory;
@@ -77,7 +68,7 @@ void GameChatHandler::onNewLobbyMessageReceived(const std::string & senderName,
 	if(lobby && lobby->card)
 	{
 		MetaString formatted = MetaString::createFromRawString("[%s] %s: %s");
-		formatted.replaceRawString(getCurrentTimeFormatted());
+		formatted.replaceRawString(TextOperations::getCurrentFormattedTimeLocal());
 		formatted.replaceRawString(senderName);
 		formatted.replaceRawString(messageText);
 
@@ -86,13 +77,20 @@ void GameChatHandler::onNewLobbyMessageReceived(const std::string & senderName,
 				lobby->toggleChat();
 	}
 
-	chatHistory.push_back({senderName, messageText, getCurrentTimeFormatted()});
+	chatHistory.push_back({senderName, messageText, TextOperations::getCurrentFormattedTimeLocal()});
 }
 
 void GameChatHandler::onNewGameMessageReceived(PlayerColor sender, const std::string & messageText)
 {
-	std::string timeFormatted = getCurrentTimeFormatted();
+
-	std::string playerName = sender.isSpectator() ? "Spectator" : sender.toString(); //FIXME: should actually be player nickname, at least in MP
+	std::string timeFormatted = TextOperations::getCurrentFormattedTimeLocal();
+	std::string playerName = "<UNKNOWN>";
+
+	if (sender.isValidPlayer())
+		playerName = LOCPLINT->cb->getStartInfo()->playerInfos.at(sender).name;
+
+	if (sender.isSpectator())
+		playerName = "Spectator"; // FIXME: translate? Provide nickname somewhere?
 
 	chatHistory.push_back({playerName, messageText, timeFormatted});
 
@@ -101,9 +99,9 @@ void GameChatHandler::onNewGameMessageReceived(PlayerColor sender, const std::st
 
 void GameChatHandler::onNewSystemMessageReceived(const std::string & messageText)
 {
-	chatHistory.push_back({"System", messageText, getCurrentTimeFormatted()});
+	chatHistory.push_back({"System", messageText, TextOperations::getCurrentFormattedTimeLocal()});
 
 	if(LOCPLINT && !settings["session"]["hideSystemMessages"].Bool())
-		LOCPLINT->cingconsole->addMessage(getCurrentTimeFormatted(), "System", messageText);
+		LOCPLINT->cingconsole->addMessage(TextOperations::getCurrentFormattedTimeLocal(), "System", messageText);
 }
 

client/adventureMap/CInGameConsole.cpp

@@ -66,8 +66,7 @@ void CInGameConsole::show(Canvas & to)
 void CInGameConsole::tick(uint32_t msPassed)
 {
 	// Check whether text input is active - we want to keep recent messages visible during this period
-	// FIXME: better check?
+	if(isEnteringText())
-	if(enteredText != "")
 		return;
 
 	size_t sizeBefore = texts.size();
@@ -125,7 +124,7 @@ void CInGameConsole::addMessage(const std::string & timeFormatted, const std::st
 
 bool CInGameConsole::captureThisKey(EShortcut key)
 {
-	if (enteredText.empty())
+	if (!isEnteringText())
 		return false;
 
 	switch (key)
@@ -148,7 +147,7 @@ void CInGameConsole::keyPressed (EShortcut key)
 	if (LOCPLINT->cingconsole != this)
 		return;
 
-	if(enteredText.empty() && key != EShortcut::GAME_ACTIVATE_CONSOLE)
+	if(!isEnteringText() && key != EShortcut::GAME_ACTIVATE_CONSOLE)
 		return; //because user is not entering any text
 
 	switch(key)
@@ -230,7 +229,7 @@ void CInGameConsole::textInputed(const std::string & inputtedText)
 	if (LOCPLINT->cingconsole != this)
 		return;
 
-	if(enteredText.empty())
+	if(!isEnteringText())
 		return;
 
 	enteredText.resize(enteredText.size()-1);
@@ -266,7 +265,7 @@ void CInGameConsole::startEnteringText()
 	if (!isActive())
 		return;
 
-	if(enteredText != "")
+	if(isEnteringText())
 		return;
 		
 	assert(currentStatusBar.expired());//effectively, nullptr check
@@ -325,3 +324,7 @@ void CInGameConsole::refreshEnteredText()
 		statusbar->setEnteredText(enteredText);
 }
 
+bool CInGameConsole::isEnteringText() const
+{
+	return !enteredText.empty();
+}

client/adventureMap/CInGameConsole.h

@@ -38,6 +38,9 @@ private:
 	std::weak_ptr<IStatusBar> currentStatusBar;
 	std::string enteredText;
 
+	/// Returns true if console is active and player is currently entering text
+	bool isEnteringText() const;
+
 	void showRecentChatHistory();
 	void addMessageSilent(const std::string & timeFormatted, const std::string & senderName, const std::string & messageText);
 public:

client/globalLobby/GlobalLobbyClient.cpp

@@ -38,24 +38,6 @@ GlobalLobbyClient::GlobalLobbyClient()
 
 GlobalLobbyClient::~GlobalLobbyClient() = default;
 
-static std::string getCurrentTimeFormatted(int timeOffsetSeconds = 0)
-{
-	// FIXME: better/unified way to format date
-	auto timeNowChrono = std::chrono::system_clock::now();
-	timeNowChrono += std::chrono::seconds(timeOffsetSeconds);
-
-	return TextOperations::getFormattedTimeLocal(std::chrono::system_clock::to_time_t(timeNowChrono));
-}
-
-static std::string getCurrentDateTimeFormatted(int timeOffsetSeconds = 0)
-{
-	// FIXME: better/unified way to format date
-	auto timeNowChrono = std::chrono::system_clock::now();
-	timeNowChrono += std::chrono::seconds(timeOffsetSeconds);
-
-	return TextOperations::getFormattedDateTimeLocal(std::chrono::system_clock::to_time_t(timeNowChrono));
-}
-
 void GlobalLobbyClient::onPacketReceived(const std::shared_ptr<INetworkConnection> &, const std::vector<std::byte> & message)
 {
 	boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
@@ -162,8 +144,8 @@ void GlobalLobbyClient::receiveChatHistory(const JsonNode & json)
 		message.accountID = entry["accountID"].String();
 		message.displayName = entry["displayName"].String();
 		message.messageText = entry["messageText"].String();
-		int ageSeconds = entry["ageSeconds"].Integer();
+		std::chrono::seconds ageSeconds (entry["ageSeconds"].Integer());
-		message.timeFormatted = getCurrentTimeFormatted(-ageSeconds);
+		message.timeFormatted = TextOperations::getCurrentFormattedTimeLocal(-ageSeconds);
 
 		chatHistory[channelKey].push_back(message);
 
@@ -179,7 +161,7 @@ void GlobalLobbyClient::receiveChatMessage(const JsonNode & json)
 	message.accountID = json["accountID"].String();
 	message.displayName = json["displayName"].String();
 	message.messageText = json["messageText"].String();
-	message.timeFormatted = getCurrentTimeFormatted();
+	message.timeFormatted = TextOperations::getCurrentFormattedTimeLocal();
 
 	std::string channelType = json["channelType"].String();
 	std::string channelName = json["channelName"].String();
@@ -227,8 +209,8 @@ void GlobalLobbyClient::receiveActiveGameRooms(const JsonNode & json)
 		room.hostAccountDisplayName = jsonEntry["hostAccountDisplayName"].String();
 		room.description = jsonEntry["description"].String();
 		room.statusID = jsonEntry["status"].String();
-		int ageSeconds = jsonEntry["ageSeconds"].Integer();
+		std::chrono::seconds ageSeconds (jsonEntry["ageSeconds"].Integer());
-		room.startDateFormatted = getCurrentDateTimeFormatted(-ageSeconds);
+		room.startDateFormatted = TextOperations::getCurrentFormattedDateTimeLocal(-ageSeconds);
 
 		for(const auto & jsonParticipant : jsonEntry["participants"].Vector())
 		{
@@ -260,8 +242,8 @@ void GlobalLobbyClient::receiveMatchesHistory(const JsonNode & json)
 		room.hostAccountDisplayName = jsonEntry["hostAccountDisplayName"].String();
 		room.description = jsonEntry["description"].String();
 		room.statusID = jsonEntry["status"].String();
-		int ageSeconds = jsonEntry["ageSeconds"].Integer();
+		std::chrono::seconds ageSeconds (jsonEntry["ageSeconds"].Integer());
-		room.startDateFormatted = getCurrentDateTimeFormatted(-ageSeconds);
+		room.startDateFormatted = TextOperations::getCurrentFormattedDateTimeLocal(-ageSeconds);
 
 		for(const auto & jsonParticipant : jsonEntry["participants"].Vector())
 		{
@@ -290,7 +272,7 @@ void GlobalLobbyClient::receiveInviteReceived(const JsonNode & json)
 	if(lobbyWindowPtr)
 	{
 		std::string message = MetaString::createFromTextID("vcmi.lobby.invite.notification").toString();
-		std::string time = getCurrentTimeFormatted();
+		std::string time = TextOperations::getCurrentFormattedTimeLocal();
 
 		lobbyWindowPtr->onGameChatMessage("System", message, time, "player", accountID);
 		lobbyWindowPtr->onInviteReceived(gameRoomID);

client/globalLobby/GlobalLobbyWindow.cpp

@@ -39,7 +39,7 @@ GlobalLobbyWindow::GlobalLobbyWindow()
 	widget->getChannelListHeader()->setText(MetaString::createFromTextID("vcmi.lobby.header.channels").toString());
 }
 
-bool GlobalLobbyWindow::isChannelOpen(const std::string & testChannelType, const std::string & testChannelName)
+bool GlobalLobbyWindow::isChannelOpen(const std::string & testChannelType, const std::string & testChannelName) const
 {
 	return testChannelType == currentChannelType && testChannelName == currentChannelName;
 }
@@ -133,7 +133,7 @@ void GlobalLobbyWindow::onGameChatMessage(const std::string & sender, const std:
 	widget->getGameChat()->setText(chatHistory);
 }
 
-bool GlobalLobbyWindow::isChannelUnread(const std::string & channelType, const std::string & channelName)
+bool GlobalLobbyWindow::isChannelUnread(const std::string & channelType, const std::string & channelName) const
 {
 	return unreadChannels.count(channelType + "_" + channelName) > 0;
 }
@@ -180,7 +180,7 @@ void GlobalLobbyWindow::onInviteReceived(const std::string & invitedRoomID)
 	widget->getRoomList()->reset();
 }
 
-bool GlobalLobbyWindow::isInviteUnread(const std::string & gameRoomID)
+bool GlobalLobbyWindow::isInviteUnread(const std::string & gameRoomID) const
 {
 	return unreadInvites.count(gameRoomID) > 0;
 }

client/globalLobby/GlobalLobbyWindow.h

@@ -37,9 +37,9 @@ public:
 	void doJoinRoom(const std::string & roomID);
 
 	/// Returns true if provided chat channel is the one that is currently open in UI
-	bool isChannelOpen(const std::string & channelType, const std::string & channelName);
+	bool isChannelOpen(const std::string & channelType, const std::string & channelName) const;
-	bool isChannelUnread(const std::string & channelType, const std::string & channelName);
+	bool isChannelUnread(const std::string & channelType, const std::string & channelName) const;
-	bool isInviteUnread(const std::string & gameRoomID);
+	bool isInviteUnread(const std::string & gameRoomID) const;
 
 	// Callbacks for network packs
 

lib/Languages.h

@@ -120,7 +120,7 @@ inline const Options & getLanguageOptions(const std::string & language)
 		if(entry.identifier == language)
 			return entry;
 
-	throw std::runtime_error("Language " + language + " does not exists!");
+	throw std::out_of_range("Language " + language + " does not exists!");
 }
 
 template<typename Numeric>

lib/TextOperations.cpp

@@ -224,5 +224,17 @@ std::string TextOperations::getFormattedTimeLocal(std::time_t dt)
 	return vstd::getFormattedDateTime(dt, "%H:%M");
 }
 
+std::string TextOperations::getCurrentFormattedTimeLocal(std::chrono::seconds timeOffset)
+{
+	auto timepoint = std::chrono::system_clock::now() + timeOffset;
+	return TextOperations::getFormattedTimeLocal(std::chrono::system_clock::to_time_t(timepoint));
+}
+
+std::string TextOperations::getCurrentFormattedDateTimeLocal(std::chrono::seconds timeOffset)
+{
+	auto timepoint = std::chrono::system_clock::now() + timeOffset;
+	return TextOperations::getFormattedDateTimeLocal(std::chrono::system_clock::to_time_t(timepoint));
+}
+
 
 VCMI_LIB_NAMESPACE_END

lib/TextOperations.h

@@ -60,8 +60,16 @@ namespace TextOperations
 	/// get formatted DateTime depending on the language selected
 	DLL_LINKAGE std::string getFormattedDateTimeLocal(std::time_t dt);
 
+	/// get formatted current DateTime depending on the language selected
+	/// timeOffset - optional parameter to modify current time by specified time in seconds
+	DLL_LINKAGE std::string getCurrentFormattedDateTimeLocal(std::chrono::seconds timeOffset = {});
+
 	/// get formatted time (without date)
 	DLL_LINKAGE std::string getFormattedTimeLocal(std::time_t dt);
+
+	/// get formatted time (without date)
+	/// timeOffset - optional parameter to modify current time by specified time in seconds
+	DLL_LINKAGE std::string getCurrentFormattedTimeLocal(std::chrono::seconds timeOffset = {});
 };
 
 

lobby/LobbyDatabase.cpp

@@ -89,7 +89,6 @@ void LobbyDatabase::clearOldData()
 		WHERE online <> 0
 	)";
 
-	//FIXME: set different status for rooms that never reached in game state
 	static const std::string removeActiveLobbyRooms = R"(
 		UPDATE gameRooms
 		SET status = 4

lobby/LobbyServer.cpp

@@ -12,22 +12,27 @@
 
 #include "LobbyDatabase.h"
 
+#include "../lib/Languages.h"
+#include "../lib/TextOperations.h"
 #include "../lib/json/JsonFormatException.h"
 #include "../lib/json/JsonNode.h"
 #include "../lib/json/JsonUtils.h"
-#include "../lib/Languages.h"
 
 #include <boost/uuid/uuid_generators.hpp>
 #include <boost/uuid/uuid_io.hpp>
 
 bool LobbyServer::isAccountNameValid(const std::string & accountName) const
 {
+	// Arbitrary limit on account name length.
+	// Can be extended if there are no issues with UI space
 	if(accountName.size() < 4)
 		return false;
 
 	if(accountName.size() > 20)
 		return false;
 
+	// For now permit only latin alphabet and numbers
+	// Can be extended, but makes sure that such symbols will be present in all H3 fonts
 	for(const auto & c : accountName)
 		if(!std::isalnum(c))
 			return false;
@@ -37,11 +42,23 @@ bool LobbyServer::isAccountNameValid(const std::string & accountName) const
 
 std::string LobbyServer::sanitizeChatMessage(const std::string & inputString) const
 {
-	// TODO: sanitize message and remove any "weird" symbols from it:
+	static const std::string blacklist = "{}";
-	// - control characters ('\0' ... ' ')
+	std::string sanitized;
-	// - '{' and '}' symbols to avoid formatting
+
-	// - other non-printable characters?
+	for(const auto & ch : inputString)
-	return boost::trim_copy(inputString);
+	{
+		// Remove all control characters
+		if (ch >= '\0' && ch < ' ')
+			continue;
+
+		// Remove blacklisted characters such as brackets that are used for text formatting
+		if (blacklist.find(ch) != std::string::npos)
+			continue;
+
+		sanitized += ch;
+	}
+
+	return boost::trim_copy(sanitized);
 }
 
 NetworkConnectionPtr LobbyServer::findAccount(const std::string & accountID) const
@@ -195,7 +212,7 @@ static JsonNode loadLobbyGameRoomToJson(const LobbyGameRoom & gameRoom)
 	jsonEntry["playerLimit"].Integer() = gameRoom.playerLimit;
 	jsonEntry["ageSeconds"].Integer() = gameRoom.age.count();
 
-	for (auto const & account : gameRoom.participants)
+	for(const auto & account : gameRoom.participants)
 		jsonEntry["participants"].Vector().push_back(loadLobbyAccountToJson(account));
 
 	return jsonEntry;
@@ -470,9 +487,12 @@ void LobbyServer::receiveSendChatMessage(const NetworkConnectionPtr & connection
 	std::string messageText = json["messageText"].String();
 	std::string channelType = json["channelType"].String();
 	std::string channelName = json["channelName"].String();
-	std::string messageTextClean = sanitizeChatMessage(messageText);
 	std::string displayName = database->getAccountDisplayName(senderAccountID);
 
+	if(TextOperations::isValidUnicodeString(messageText))
+		return sendOperationFailed(connection, "String contains invalid characters!");
+
+	std::string messageTextClean = sanitizeChatMessage(messageText);
 	if(messageTextClean.empty())
 		return sendOperationFailed(connection, "No printable characters in sent message!");
 
@@ -482,7 +502,7 @@ void LobbyServer::receiveSendChatMessage(const NetworkConnectionPtr & connection
 		{
 			Languages::getLanguageOptions(channelName);
 		}
-		catch (const std::runtime_error &)
+		catch (const std::out_of_range &)
 		{
 			return sendOperationFailed(connection, "Unknown language!");
 		}
@@ -499,11 +519,17 @@ void LobbyServer::receiveSendChatMessage(const NetworkConnectionPtr & connection
 
 		database->insertChatMessage(senderAccountID, channelType, channelName, messageText);
 
-		// TODO: Don't report match messages if room is still active - players in room will receive these messages via match server
+		LobbyRoomState roomStatus = database->getGameRoomStatus(channelName);
-		for(const auto & otherConnection : activeAccounts)
+
+		// Broadcast chat message only if it being sent to already closed match
+		// Othervice it will be handled by match server
+		if (roomStatus == LobbyRoomState::CLOSED)
 		{
-			if (database->isPlayerInGameRoom(otherConnection.second, channelName))
+			for(const auto & otherConnection : activeAccounts)
-				sendChatMessage(otherConnection.first, channelType, channelName, senderAccountID, displayName, messageText);
+			{
+				if (database->isPlayerInGameRoom(otherConnection.second, channelName))
+					sendChatMessage(otherConnection.first, channelType, channelName, senderAccountID, displayName, messageText);
+			}
 		}
 	}
 

lobby/LobbyServer.h

@@ -44,7 +44,9 @@ class LobbyServer final : public INetworkServerListener
 	std::unique_ptr<INetworkHandler> networkHandler;
 	std::unique_ptr<INetworkServer> networkServer;
 
+	/// removes any "weird" symbols from chat message that might break UI
 	std::string sanitizeChatMessage(const std::string & inputString) const;
+
 	bool isAccountNameValid(const std::string & accountName) const;
 
 	NetworkConnectionPtr findAccount(const std::string & accountID) const;