code review

Mods/vcmi/config/vcmi/english.json

@@ -162,7 +162,7 @@
 	"vcmi.systemOptions.otherGroup" : "Other Settings", // unused right now
 	"vcmi.systemOptions.townsGroup" : "Town Screen",
 
-	"vcmi.statisticWindow.statistic" : "Statistic",
+	"vcmi.statisticWindow.statistics" : "Statistics",
 	"vcmi.statisticWindow.tsvCopy" : "Data to clipboard",
 	"vcmi.statisticWindow.selectView" : "Select view",
 	"vcmi.statisticWindow.value" : "Value",

Mods/vcmi/config/vcmi/german.json

@@ -162,7 +162,7 @@
 	"vcmi.systemOptions.otherGroup" : "Andere Einstellungen", // unused right now
 	"vcmi.systemOptions.townsGroup" : "Stadt-Bildschirm",
 
-	"vcmi.statisticWindow.statistic" : "Statistik",
+	"vcmi.statisticWindow.statistics" : "Statistik",
 	"vcmi.statisticWindow.tsvCopy" : "Daten in Zwischenabl.",
 	"vcmi.statisticWindow.selectView" : "Ansicht wählen",
 	"vcmi.statisticWindow.value" : "Wert",

client/CServerHandler.cpp

@@ -673,7 +673,7 @@ void CServerHandler::startGameplay(VCMI_LIB_WRAP_NAMESPACE(CGameState) * gameSta
 	setState(EClientState::GAMEPLAY);
 }
 
-void CServerHandler::showHighScoresAndEndGameplay(PlayerColor player, bool victory, StatisticDataSet statistic)
+void CServerHandler::showHighScoresAndEndGameplay(PlayerColor player, bool victory, const StatisticDataSet & statistic)
 {
 	HighScoreParameter param = HighScore::prepareHighScores(client->gameState(), player, victory);
 
@@ -722,7 +722,7 @@ void CServerHandler::restartGameplay()
 	logicConnection->enterLobbyConnectionMode();
 }
 
-void CServerHandler::startCampaignScenario(HighScoreParameter param, std::shared_ptr<CampaignState> cs, StatisticDataSet statistic)
+void CServerHandler::startCampaignScenario(HighScoreParameter param, std::shared_ptr<CampaignState> cs, const StatisticDataSet & statistic)
 {
 	std::shared_ptr<CampaignState> ourCampaign = cs;
 

client/CServerHandler.h

@@ -205,11 +205,11 @@ public:
 	void debugStartTest(std::string filename, bool save = false);
 
 	void startGameplay(VCMI_LIB_WRAP_NAMESPACE(CGameState) * gameState = nullptr);
-	void showHighScoresAndEndGameplay(PlayerColor player, bool victory, StatisticDataSet statistic);
+	void showHighScoresAndEndGameplay(PlayerColor player, bool victory, const StatisticDataSet & statistic);
 	void endNetwork();
 	void endGameplay();
 	void restartGameplay();
-	void startCampaignScenario(HighScoreParameter param, std::shared_ptr<CampaignState> cs, StatisticDataSet statistic);
+	void startCampaignScenario(HighScoreParameter param, std::shared_ptr<CampaignState> cs, const StatisticDataSet & statistic);
 	void showServerError(const std::string & txt) const;
 
 	// TODO: LobbyState must be updated within game so we should always know how many player interfaces our client handle

client/gui/Shortcut.h

@@ -74,7 +74,7 @@ enum class EShortcut
 	HIGH_SCORES_CAMPAIGNS,
 	HIGH_SCORES_SCENARIOS,
 	HIGH_SCORES_RESET,
-	HIGH_SCORES_STATISTIC,
+	HIGH_SCORES_STATISTICS,
 
 	// Game lobby / scenario selection
 	LOBBY_BEGIN_STANDARD_GAME, // b

client/gui/ShortcutHandler.cpp

@@ -290,7 +290,7 @@ EShortcut ShortcutHandler::findShortcut(const std::string & identifier ) const
 		{"highScoresCampaigns",      EShortcut::HIGH_SCORES_CAMPAIGNS     },
 		{"highScoresScenarios",      EShortcut::HIGH_SCORES_SCENARIOS     },
 		{"highScoresReset",          EShortcut::HIGH_SCORES_RESET         },
-		{"highScoresStatistic",      EShortcut::HIGH_SCORES_STATISTIC     },
+		{"highScoresStatistics",     EShortcut::HIGH_SCORES_STATISTICS    },
 		{"lobbyReplayVideo",         EShortcut::LOBBY_REPLAY_VIDEO        },
 		{"lobbyExtraOptions",        EShortcut::LOBBY_EXTRA_OPTIONS       },
 		{"lobbyTurnOptions",         EShortcut::LOBBY_TURN_OPTIONS        },

client/mainmenu/CStatisticScreen.cpp

@@ -44,35 +44,11 @@ CStatisticScreen::CStatisticScreen(StatisticDataSet stat)
 	filledBackground->setPlayerColor(PlayerColor(1));
 
 	contentArea = Rect(10, 40, 780, 510);
-	layout.push_back(std::make_shared<CLabel>(400, 20, FONT_BIG, ETextAlignment::CENTER, Colors::YELLOW, CGI->generaltexth->translate("vcmi.statisticWindow.statistic")));
+	layout.push_back(std::make_shared<CLabel>(400, 20, FONT_BIG, ETextAlignment::CENTER, Colors::YELLOW, CGI->generaltexth->translate("vcmi.statisticWindow.statistics")));
 	layout.push_back(std::make_shared<TransparentFilledRectangle>(contentArea, ColorRGBA(0, 0, 0, 128), ColorRGBA(64, 80, 128, 255), 1));
 	layout.push_back(std::make_shared<CButton>(Point(725, 558), AnimationPath::builtin("MUBCHCK"), CButton::tooltip(), [this](){ close(); }, EShortcut::GLOBAL_ACCEPT));
 
-	buttonSelect = std::make_shared<CToggleButton>(Point(10, 564), AnimationPath::builtin("GSPBUT2"), CButton::tooltip(), [this](bool on){
-		std::vector<std::string> texts;
-		for(auto & val : contentInfo)
-			texts.push_back(CGI->generaltexth->translate(std::get<0>(val.second)));
-		GH.windows().createAndPushWindow<StatisticSelector>(texts, [this](int selectedIndex)
-		{
-			OBJECT_CONSTRUCTION;
-			if(!std::get<1>(contentInfo[(Content)selectedIndex]))
-				mainContent = getContent((Content)selectedIndex, EGameResID::NONE);
-			else
-			{
-				auto content = (Content)selectedIndex;
-				auto possibleRes = std::vector<EGameResID>{EGameResID::GOLD, EGameResID::WOOD, EGameResID::MERCURY, EGameResID::ORE, EGameResID::SULFUR, EGameResID::CRYSTAL, EGameResID::GEMS};
-				std::vector<std::string> resourceText;
-				for(auto & res : possibleRes)
-					resourceText.push_back(CGI->generaltexth->translate(TextIdentifier("core.restypes", res.getNum()).get()));
-				
-				GH.windows().createAndPushWindow<StatisticSelector>(resourceText, [this, content, possibleRes](int selectedIndex)
-				{
-					OBJECT_CONSTRUCTION;
-					mainContent = getContent(content, possibleRes[selectedIndex]);
-				});
-			}
-		});
-	});
+	buttonSelect = std::make_shared<CToggleButton>(Point(10, 564), AnimationPath::builtin("GSPBUT2"), CButton::tooltip(), [this](bool on){ onSelectButton(); });
 	buttonSelect->setTextOverlay(CGI->generaltexth->translate("vcmi.statisticWindow.selectView"), EFonts::FONT_SMALL, Colors::YELLOW);
 
 	buttonCsvSave = std::make_shared<CToggleButton>(Point(150, 564), AnimationPath::builtin("GSPBUT2"), CButton::tooltip(), [this](bool on){ GH.input().copyToClipBoard(statistic.toCsv("\t"));	});
@@ -81,6 +57,33 @@ CStatisticScreen::CStatisticScreen(StatisticDataSet stat)
 	mainContent = getContent(OVERVIEW, EGameResID::NONE);
 }
 
+void CStatisticScreen::onSelectButton()
+{
+	std::vector<std::string> texts;
+	for(auto & val : contentInfo)
+		texts.push_back(CGI->generaltexth->translate(std::get<0>(val.second)));
+	GH.windows().createAndPushWindow<StatisticSelector>(texts, [this](int selectedIndex)
+	{
+		OBJECT_CONSTRUCTION;
+		if(!std::get<1>(contentInfo[(Content)selectedIndex]))
+			mainContent = getContent((Content)selectedIndex, EGameResID::NONE);
+		else
+		{
+			auto content = (Content)selectedIndex;
+			auto possibleRes = std::vector<EGameResID>{EGameResID::GOLD, EGameResID::WOOD, EGameResID::MERCURY, EGameResID::ORE, EGameResID::SULFUR, EGameResID::CRYSTAL, EGameResID::GEMS};
+			std::vector<std::string> resourceText;
+			for(auto & res : possibleRes)
+				resourceText.push_back(CGI->generaltexth->translate(TextIdentifier("core.restypes", res.getNum()).get()));
+			
+			GH.windows().createAndPushWindow<StatisticSelector>(resourceText, [this, content, possibleRes](int selectedIndex)
+			{
+				OBJECT_CONSTRUCTION;
+				mainContent = getContent(content, possibleRes[selectedIndex]);
+			});
+		}
+	});
+}
+
 TData CStatisticScreen::extractData(StatisticDataSet stat, std::function<float(StatisticDataSetEntry val)> selector)
 {
 	auto tmpData = stat.data;

client/mainmenu/CStatisticScreen.h

@@ -66,6 +66,7 @@ class CStatisticScreen : public CWindowObject
 
 	TData extractData(StatisticDataSet stat, std::function<float(StatisticDataSetEntry val)> selector);
 	std::shared_ptr<CIntObject> getContent(Content c, EGameResID res);
+	void onSelectButton();
 public:
 	CStatisticScreen(StatisticDataSet stat);
 };

config/shortcutsConfig.json

@@ -137,7 +137,7 @@
 		"heroToggleTactics":        "B",
 		"highScoresCampaigns":      "C",
 		"highScoresReset":          "R",
-		"highScoresStatistic":      ".",
+		"highScoresStatistics":     ".",
 		"highScoresScenarios":      "S",
 		"kingdomHeroesTab":         "H",
 		"kingdomTownsTab":          "T",

lib/gameState/GameStatistics.h

@@ -64,7 +64,8 @@ struct DLL_LINKAGE StatisticDataSetEntry
 		h & timestamp;
 		h & day;
 		h & player;
-		h & playerName;
+		if(h.version >= Handler::Version::STATISTICS_SCREEN)
+			h & playerName;
 		h & team;
 		h & isHuman;
 		h & status;
@@ -92,9 +93,12 @@ struct DLL_LINKAGE StatisticDataSetEntry
 		h & spentResourcesForArmy;
 		h & spentResourcesForBuildings;
 		h & tradeVolume;
-		h & eventCapturedTown;
-		h & eventDefeatedStrongestHero;
-		h & eventRansackingDragonUtopia;
+		if(h.version >= Handler::Version::STATISTICS_SCREEN)
+		{
+			h & eventCapturedTown;
+			h & eventDefeatedStrongestHero;
+			h & eventRansackingDragonUtopia;
+		}
 		h & movementPointsUsed;
 	}
 };
@@ -135,9 +139,12 @@ public:
 			h & spentResourcesForBuildings;
 			h & tradeVolume;
 			h & movementPointsUsed;
-			h & lastCapturedTownDay;
-			h & lastDefeatedStrongestHeroDay;
-			h & lastRansackingDragonUtopiaDay;
+			if(h.version >= Handler::Version::STATISTICS_SCREEN)
+			{
+				h & lastCapturedTownDay;
+				h & lastDefeatedStrongestHeroDay;
+				h & lastRansackingDragonUtopiaDay;
+			}
 		}
 	};
 	std::vector<StatisticDataSetEntry> data;

lib/networkPacks/PacksForClient.h

@@ -444,7 +444,7 @@ struct DLL_LINKAGE PlayerEndsGame : public CPackForClient
 	{
 		h & player;
 		h & victoryLossCheckResult;
-		if (h.version >= Handler::Version::STATISTICS)
+		if (h.version >= Handler::Version::STATISTICS_SCREEN)
 			h & statistic;
 	}
 };

lib/serializer/ESerializationVersion.h

@@ -63,6 +63,7 @@ enum class ESerializationVersion : int32_t
 	STATISTICS, // 852 - removed random number generators from library classes
 	CAMPAIGN_REGIONS, // 853 - configurable campaign regions
 	EVENTS_PLAYER_SET, // 854 - map & town events use std::set instead of bitmask to store player list
+	STATISTICS_SCREEN, // 855 - extent statistic functions
 
-	CURRENT = EVENTS_PLAYER_SET
+	CURRENT = STATISTICS_SCREEN
 };

server/battles/BattleResultProcessor.cpp

@@ -480,6 +480,7 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 	// Remove beaten hero
 	if(finishingBattle->loserHero)
 	{
+		//add statistics
 		if(!finishingBattle->isDraw())
 		{
 			ConstTransitivePtr<CGHeroInstance> strongestHero = nullptr;