Improvements to type safety of Identifier class

- Constructor of Identifier from integer is now explicit
- Lobby hero/town selection now uses Identifiers instead of int's
- Removed serialization workaround for hero portraits
- Added dummy objects for custom heroes portraits for ID resolver to use
- HeroInstance now stores portrait ID only in case of custom portrait
- Fixed loading of campaign heroes portraits on RoE maps

Mods/vcmi/Sprites/PortraitsLarge.json

@@ -156,13 +156,6 @@
 		{ "frame" : 152, "file" : "HPL009SH.bmp"},
 		{ "frame" : 153, "file" : "HPL008SH.bmp"},
 		{ "frame" : 154, "file" : "HPL001SH.bmp"},
-		{ "frame" : 155, "file" : "HPL131DM.bmp"},
-		{ "frame" : 156, "file" : "HPL129MK.bmp"},
-		{ "frame" : 157, "file" : "HPL002SH.bmp"},
-		{ "frame" : 158, "file" : "HPL132Wl.bmp"},
-		{ "frame" : 159, "file" : "HPL133Nc.bmp"},
-		{ "frame" : 160, "file" : "HPL134Nc.bmp"},
-		{ "frame" : 161, "file" : "HPL135Wi.bmp"},
-		{ "frame" : 162, "file" : "HPL136Wi.bmp"}
+		{ "frame" : 155, "file" : "HPL131DM.bmp"}
 	]
 }

Mods/vcmi/Sprites/PortraitsSmall.json

@@ -155,16 +155,9 @@
 		{ "frame" : 151, "file" : "HPS007SH.bmp"},
 		{ "frame" : 152, "file" : "HPS009SH.bmp"},
 		{ "frame" : 153, "file" : "HPS008SH.bmp"},
-		{ "frame" : 154, "file" : "HPS001SH.bmp"},
-		{ "frame" : 155, "file" : "HPS131DM.bmp"},
-		{ "frame" : 156, "file" : "HPS129MK.bmp"},
-		{ "frame" : 157, "file" : "HPS002SH.bmp"},
-		{ "frame" : 158, "file" : "HPS132Wl.bmp"},
-		{ "frame" : 159, "file" : "HPS133Nc.bmp"},
-		{ "frame" : 160, "file" : "HPS134Nc.bmp"},
-		{ "frame" : 161, "file" : "HPS135Wi.bmp"},
-		{ "frame" : 162, "file" : "HPS136Wi.bmp"},
-		{ "frame" : 163, "file" : "HPSRAND1.bmp"}, //random hero
-		{ "frame" : 164, "file" : "HPSRAND6.bmp"} //no hero
+		{ "frame" : 154, "file" : "HPS001SH.bmp"}, 
+		{ "frame" : 155, "file" : "HPS131DM.bmp"}, 
+		{ "frame" : 156, "file" : "HPSRAND1.bmp"}, // random hero
+		{ "frame" : 157, "file" : "HPSRAND6.bmp"}  // no hero
 	]
 }

client/ClientCommandManager.cpp

@@ -466,7 +466,7 @@ void ClientCommandManager::giveTurn(const PlayerColor &colorIdentifier)
 {
 	PlayerStartsTurn yt;
 	yt.player = colorIdentifier;
-	yt.queryID = -1;
+	yt.queryID = QueryID::NONE;
 
 	ApplyClientNetPackVisitor visitor(*CSH->client, *CSH->client->gameState());
 	yt.visit(visitor);

client/NetPacksClient.cpp

@@ -403,7 +403,7 @@ void ApplyClientNetPackVisitor::visitPlayerReinitInterface(PlayerReinitInterface
 			if (cl.gameState()->isPlayerMakingTurn(player))
 			{
 				callAllInterfaces(cl, &IGameEventsReceiver::playerStartsTurn, player);
-				callOnlyThatInterface(cl, player, &CGameInterface::yourTurn, -1);
+				callOnlyThatInterface(cl, player, &CGameInterface::yourTurn, QueryID::NONE);
 			}
 		}
 	};

client/adventureMap/CList.cpp

@@ -220,7 +220,7 @@ CHeroList::CHeroItem::CHeroItem(CHeroList *parent, const CGHeroInstance * Hero)
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
 	movement = std::make_shared<CAnimImage>(AnimationPath::builtin("IMOBIL"), 0, 0, 0, 1);
-	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsSmall"), hero->portrait, 0, movement->pos.w + 1);
+	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsSmall"), hero->getIconIndex(), 0, movement->pos.w + 1);
 	mana = std::make_shared<CAnimImage>(AnimationPath::builtin("IMANA"), 0, 0, movement->pos.w + portrait->pos.w + 2, 1);
 
 	pos.w = mana->pos.w + mana->pos.x - pos.x;

client/battle/BattleInterfaceClasses.cpp

@@ -407,7 +407,7 @@ void HeroInfoBasicPanel::initializeData(const InfoAboutHero & hero)
 	auto currentSpellPoints = hero.details->mana;
 	auto maxSpellPoints = hero.details->manaLimit;
 
-	icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero.portrait, 0, 10, 6));
+	icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero.getIconIndex(), 0, 10, 6));
 
 	//primary stats
 	labels.push_back(std::make_shared<CLabel>(9, 75, EFonts::FONT_TINY, ETextAlignment::TOPLEFT, Colors::WHITE, CGI->generaltexth->allTexts[380] + ":"));
@@ -506,9 +506,9 @@ BattleResultWindow::BattleResultWindow(const BattleResult & br, CPlayerInterface
 		auto heroInfo = owner.cb->getBattle(br.battleID)->battleGetHeroInfo(i);
 		const int xs[] = {21, 392};
 
-		if(heroInfo.portrait >= 0) //attacking hero
+		if(heroInfo.portraitSource.isValid()) //attacking hero
 		{
-			icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), heroInfo.portrait, 0, xs[i], 38));
+			icons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), heroInfo.getIconIndex(), 0, xs[i], 38));
 			sideNames[i] = heroInfo.name;
 		}
 		else

client/gui/InterfaceObjectConfigurable.cpp

@@ -240,7 +240,7 @@ PlayerColor InterfaceObjectConfigurable::readPlayerColor(const JsonNode & config
 {
 	logGlobal->debug("Reading PlayerColor");
 	if(!config.isNull() && config.isString())
-		return PlayerColor::decode(config.String());
+		return PlayerColor(PlayerColor::decode(config.String()));
 	
 	logGlobal->debug("Unknown PlayerColor attribute");
 	return PlayerColor::CANNOT_DETERMINE;

client/lobby/CBonusSelection.cpp

@@ -274,7 +274,7 @@ void CBonusSelection::createBonusesIcons()
 			auto superhero = getCampaign()->strongestHero(static_cast<CampaignScenarioID>(bonDescs[i].info2), PlayerColor(bonDescs[i].info1));
 			if(!superhero)
 				logGlobal->warn("No superhero! How could it be transferred?");
-			picNumber = superhero ? superhero->portrait : 0;
+			picNumber = superhero ? superhero->getIconIndex() : 0;
 			desc.appendLocalString(EMetaText::GENERAL_TXT, 719);
 			desc.replaceRawString(getCampaign()->scenario(static_cast<CampaignScenarioID>(bonDescs[i].info2)).scenarioName.toString());
 			break;

client/lobby/CSelectionBase.cpp

@@ -68,9 +68,9 @@ int ISelectionScreenInfo::getCurrentDifficulty()
 	return getStartInfo()->difficulty;
 }
 
-PlayerInfo ISelectionScreenInfo::getPlayerInfo(int color)
+PlayerInfo ISelectionScreenInfo::getPlayerInfo(PlayerColor color)
 {
-	return getMapInfo()->mapHeader->players[color];
+	return getMapInfo()->mapHeader->players[color.getNum()];
 }
 
 CSelectionBase::CSelectionBase(ESelectionScreen type)
@@ -398,8 +398,9 @@ CFlagBox::CFlagBoxTooltipBox::CFlagBoxTooltipBox(std::shared_ptr<CAnimation> ico
 	labelTeamAlignment = std::make_shared<CLabel>(128, 30, FONT_MEDIUM, ETextAlignment::CENTER, Colors::YELLOW, CGI->generaltexth->allTexts[657]);
 	labelGroupTeams = std::make_shared<CLabelGroup>(FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE);
 
-	std::vector<std::set<ui8>> teams(PlayerColor::PLAYER_LIMIT_I);
-	for(ui8 j = 0; j < PlayerColor::PLAYER_LIMIT_I; j++)
+	std::vector<std::set<PlayerColor>> teams(PlayerColor::PLAYER_LIMIT_I);
+
+	for(PlayerColor j(0); j < PlayerColor::PLAYER_LIMIT; j++)
 	{
 		if(SEL->getPlayerInfo(j).canHumanPlay || SEL->getPlayerInfo(j).canComputerPlay)
 		{

client/lobby/CSelectionBase.h

@@ -44,7 +44,7 @@ public:
 	virtual const StartInfo * getStartInfo() = 0;
 
 	virtual int getCurrentDifficulty();
-	virtual PlayerInfo getPlayerInfo(int color);
+	virtual PlayerInfo getPlayerInfo(PlayerColor color);
 
 };
 

client/lobby/OptionsTab.cpp

@@ -291,48 +291,49 @@ size_t OptionsTab::CPlayerSettingsHelper::getImageIndex(bool big)
 		WOOD = 0,       ORE     = 0,    MITHRIL = 10, // resources unavailable in bonuses file
 
 		TOWN_RANDOM = 38,  TOWN_NONE = 39, // Special frames in ITPA
-		HERO_RANDOM = 163, HERO_NONE = 164 // Special frames in PortraitsSmall
+		HERO_RANDOM = 156, HERO_NONE = 157 // Special frames in PortraitsSmall
 	};
-	auto factionIndex = playerSettings.castle.getNum() >= CGI->townh->size() ? 0 : playerSettings.castle.getNum();
+	auto factionIndex = playerSettings.getCastleValidated();
 
-	switch(type)
+	switch(selectionType)
 	{
 	case TOWN:
-		switch(playerSettings.castle)
-		{
-		case PlayerSettings::NONE:
+	{
+		if (playerSettings.castle == FactionID::NONE)
 			return TOWN_NONE;
-		case PlayerSettings::RANDOM:
+
+		if (playerSettings.castle == FactionID::RANDOM)
 			return TOWN_RANDOM;
-		default:
-			return (*CGI->townh)[factionIndex]->town->clientInfo.icons[true][false] + (big ? 0 : 2);
-		}
+
+		return (*CGI->townh)[factionIndex]->town->clientInfo.icons[true][false] + (big ? 0 : 2);
+	}
+
 	case HERO:
-		switch(playerSettings.hero)
-		{
-		case PlayerSettings::NONE:
+	{
+		if (playerSettings.hero == HeroTypeID::NONE)
 			return HERO_NONE;
-		case PlayerSettings::RANDOM:
+
+		if (playerSettings.hero == HeroTypeID::RANDOM)
 			return HERO_RANDOM;
-		default:
-		{
-			if(playerSettings.heroPortrait != HeroTypeID::NONE)
-				return playerSettings.heroPortrait;
-			auto index = playerSettings.hero.getNum() >= CGI->heroh->size() ? 0 : playerSettings.hero.getNum();
-			return (*CGI->heroh)[index]->imageIndex;
-		}
-		}
+
+		if(playerSettings.heroPortrait != HeroTypeID::NONE)
+			return playerSettings.heroPortrait;
+
+		auto index = playerSettings.getHeroValidated();
+		return (*CGI->heroh)[index]->imageIndex;
+	}
+
 	case BONUS:
 	{
 		switch(playerSettings.bonus)
 		{
-		case PlayerSettings::RANDOM:
+		case PlayerStartingBonus::RANDOM:
 			return RANDOM;
-		case PlayerSettings::ARTIFACT:
+		case PlayerStartingBonus::ARTIFACT:
 			return ARTIFACT;
-		case PlayerSettings::GOLD:
+		case PlayerStartingBonus::GOLD:
 			return GOLD;
-		case PlayerSettings::RESOURCE:
+		case PlayerStartingBonus::RESOURCE:
 		{
 			switch((*CGI->townh)[factionIndex]->town->primaryRes.toEnum())
 			{
@@ -364,7 +365,7 @@ size_t OptionsTab::CPlayerSettingsHelper::getImageIndex(bool big)
 
 AnimationPath OptionsTab::CPlayerSettingsHelper::getImageName(bool big)
 {
-	switch(type)
+	switch(selectionType)
 	{
 	case OptionsTab::TOWN:
 		return AnimationPath::builtin(big ? "ITPt": "ITPA");
@@ -378,74 +379,63 @@ AnimationPath OptionsTab::CPlayerSettingsHelper::getImageName(bool big)
 
 std::string OptionsTab::CPlayerSettingsHelper::getName()
 {
-	switch(type)
-	{
-	case TOWN:
+	switch(selectionType)
 	{
-		switch(playerSettings.castle)
-		{
-		case PlayerSettings::NONE:
-			return CGI->generaltexth->allTexts[523];
-		case PlayerSettings::RANDOM:
-			return CGI->generaltexth->allTexts[522];
-		default:
+		case TOWN:
 		{
-			auto factionIndex = playerSettings.castle.getNum() >= CGI->townh->size() ? 0 : playerSettings.castle.getNum();
+			if (playerSettings.castle == FactionID::NONE)
+				return CGI->generaltexth->allTexts[523];
+
+			if (playerSettings.castle == FactionID::RANDOM)
+				return CGI->generaltexth->allTexts[522];
+
+			auto factionIndex = playerSettings.getCastleValidated();
 			return (*CGI->townh)[factionIndex]->getNameTranslated();
 		}
-	}
-	}
-	case HERO:
-	{
-		switch(playerSettings.hero)
-		{
-		case PlayerSettings::NONE:
-			return CGI->generaltexth->allTexts[523];
-		case PlayerSettings::RANDOM:
-			return CGI->generaltexth->allTexts[522];
-		default:
+		case HERO:
 		{
+			if (playerSettings.hero == HeroTypeID::NONE)
+				return CGI->generaltexth->allTexts[523];
+
+			if (playerSettings.hero == HeroTypeID::RANDOM)
+					return CGI->generaltexth->allTexts[522];
+
 			if(!playerSettings.heroNameTextId.empty())
 				return playerSettings.heroNameTextId;
-			auto index = playerSettings.hero.getNum() >= CGI->heroh->size() ? 0 : playerSettings.hero.getNum();
+			auto index = playerSettings.getHeroValidated();
 			return (*CGI->heroh)[index]->getNameTranslated();
 		}
-		}
-	}
-	case BONUS:
-	{
-		switch(playerSettings.bonus)
+		case BONUS:
 		{
-		case PlayerSettings::RANDOM:
-			return CGI->generaltexth->allTexts[522];
-		default:
-			return CGI->generaltexth->arraytxt[214 + playerSettings.bonus];
+			if (playerSettings.bonus == PlayerStartingBonus::RANDOM)
+					return CGI->generaltexth->allTexts[522];
+
+			return CGI->generaltexth->arraytxt[214 + static_cast<int>(playerSettings.bonus)];
 		}
 	}
-	}
 	return "";
 }
 
 
 std::string OptionsTab::CPlayerSettingsHelper::getTitle()
 {
-	switch(type)
+	switch(selectionType)
 	{
 	case OptionsTab::TOWN:
-		return (playerSettings.castle.getNum() < 0) ? CGI->generaltexth->allTexts[103] : CGI->generaltexth->allTexts[80];
+		return playerSettings.castle.isValid() ? CGI->generaltexth->allTexts[80] : CGI->generaltexth->allTexts[103];
 	case OptionsTab::HERO:
-		return (playerSettings.hero.getNum() < 0) ? CGI->generaltexth->allTexts[101] : CGI->generaltexth->allTexts[77];
+		return playerSettings.hero.isValid() ? CGI->generaltexth->allTexts[77] : CGI->generaltexth->allTexts[101];
 	case OptionsTab::BONUS:
 	{
 		switch(playerSettings.bonus)
 		{
-		case PlayerSettings::RANDOM:
+		case PlayerStartingBonus::RANDOM:
 			return CGI->generaltexth->allTexts[86]; //{Random Bonus}
-		case PlayerSettings::ARTIFACT:
+		case PlayerStartingBonus::ARTIFACT:
 			return CGI->generaltexth->allTexts[83]; //{Artifact Bonus}
-		case PlayerSettings::GOLD:
+		case PlayerStartingBonus::GOLD:
 			return CGI->generaltexth->allTexts[84]; //{Gold Bonus}
-		case PlayerSettings::RESOURCE:
+		case PlayerStartingBonus::RESOURCE:
 			return CGI->generaltexth->allTexts[85]; //{Resource Bonus}
 		}
 	}
@@ -454,16 +444,16 @@ std::string OptionsTab::CPlayerSettingsHelper::getTitle()
 }
 std::string OptionsTab::CPlayerSettingsHelper::getSubtitle()
 {
-	auto factionIndex = playerSettings.castle.getNum() >= CGI->townh->size() ? 0 : playerSettings.castle.getNum();
-	auto heroIndex = playerSettings.hero.getNum() >= CGI->heroh->size() ? 0 : playerSettings.hero.getNum();
+	auto factionIndex = playerSettings.getCastleValidated();
+	auto heroIndex = playerSettings.getHeroValidated();
 
-	switch(type)
+	switch(selectionType)
 	{
 	case TOWN:
 		return getName();
 	case HERO:
 	{
-		if(playerSettings.hero.getNum() >= 0)
+		if(playerSettings.hero.isValid())
 			return getName() + " - " + (*CGI->heroh)[heroIndex]->heroClass->getNameTranslated();
 		return getName();
 	}
@@ -472,9 +462,9 @@ std::string OptionsTab::CPlayerSettingsHelper::getSubtitle()
 	{
 		switch(playerSettings.bonus)
 		{
-		case PlayerSettings::GOLD:
+		case PlayerStartingBonus::GOLD:
 			return CGI->generaltexth->allTexts[87]; //500-1000
-		case PlayerSettings::RESOURCE:
+		case PlayerStartingBonus::RESOURCE:
 		{
 			switch((*CGI->townh)[factionIndex]->town->primaryRes.toEnum())
 			{
@@ -498,9 +488,9 @@ std::string OptionsTab::CPlayerSettingsHelper::getSubtitle()
 
 std::string OptionsTab::CPlayerSettingsHelper::getDescription()
 {
-	auto factionIndex = playerSettings.castle.getNum() >= CGI->townh->size() ? 0 : playerSettings.castle.getNum();
+	auto factionIndex = playerSettings.getCastleValidated();
 
-	switch(type)
+	switch(selectionType)
 	{
 	case TOWN:
 		return CGI->generaltexth->allTexts[104];
@@ -510,13 +500,13 @@ std::string OptionsTab::CPlayerSettingsHelper::getDescription()
 	{
 		switch(playerSettings.bonus)
 		{
-		case PlayerSettings::RANDOM:
+		case PlayerStartingBonus::RANDOM:
 			return CGI->generaltexth->allTexts[94]; //Gold, wood and ore, or an artifact is randomly chosen as your starting bonus
-		case PlayerSettings::ARTIFACT:
+		case PlayerStartingBonus::ARTIFACT:
 			return CGI->generaltexth->allTexts[90]; //An artifact is randomly chosen and equipped to your starting hero
-		case PlayerSettings::GOLD:
+		case PlayerStartingBonus::GOLD:
 			return CGI->generaltexth->allTexts[92]; //At the start of the game, 500-1000 gold is added to your Kingdom's resource pool
-		case PlayerSettings::RESOURCE:
+		case PlayerStartingBonus::RESOURCE:
 		{
 			switch((*CGI->townh)[factionIndex]->town->primaryRes.toEnum())
 			{
@@ -543,30 +533,19 @@ OptionsTab::CPlayerOptionTooltipBox::CPlayerOptionTooltipBox(CPlayerSettingsHelp
 {
 	OBJ_CONSTRUCTION_CAPTURING_ALL_NO_DISPOSE;
 
-	int value = PlayerSettings::NONE;
-
-	switch(CPlayerSettingsHelper::type)
+	switch(selectionType)
 	{
-		break;
-	case TOWN:
-		value = playerSettings.castle;
-		break;
-	case HERO:
-		value = playerSettings.hero;
-		break;
-	case BONUS:
-		value = playerSettings.bonus;
+		case TOWN:
+			genTownWindow();
+			break;
+		case HERO:
+			genHeroWindow();
+			break;
+		case BONUS:
+			genBonusWindow();
+			break;
 	}
 
-	if(value == PlayerSettings::RANDOM)
-		genBonusWindow();
-	else if(CPlayerSettingsHelper::type == BONUS)
-		genBonusWindow();
-	else if(CPlayerSettingsHelper::type == HERO)
-		genHeroWindow();
-	else if(CPlayerSettingsHelper::type == TOWN)
-		genTownWindow();
-
 	center();
 }
 
@@ -585,7 +564,7 @@ void OptionsTab::CPlayerOptionTooltipBox::genTownWindow()
 	pos = Rect(0, 0, 228, 290);
 	genHeader();
 	labelAssociatedCreatures = std::make_shared<CLabel>(pos.w / 2 + 8, 122, FONT_MEDIUM, ETextAlignment::CENTER, Colors::YELLOW, CGI->generaltexth->allTexts[79]);
-	auto factionIndex = playerSettings.castle.getNum() >= CGI->townh->size() ? 0 : playerSettings.castle.getNum();
+	auto factionIndex = playerSettings.getCastleValidated();
 	std::vector<std::shared_ptr<CComponent>> components;
 	const CTown * town = (*CGI->townh)[factionIndex]->town;
 
@@ -602,7 +581,7 @@ void OptionsTab::CPlayerOptionTooltipBox::genHeroWindow()
 	pos = Rect(0, 0, 292, 226);
 	genHeader();
 	labelHeroSpeciality = std::make_shared<CLabel>(pos.w / 2 + 4, 117, FONT_MEDIUM, ETextAlignment::CENTER, Colors::YELLOW, CGI->generaltexth->allTexts[78]);
-	auto heroIndex = playerSettings.hero.getNum() >= CGI->heroh->size() ? 0 : playerSettings.hero.getNum();
+	auto heroIndex = playerSettings.getHeroValidated();
 
 	imageSpeciality = std::make_shared<CAnimImage>(AnimationPath::builtin("UN44"), (*CGI->heroh)[heroIndex]->imageIndex, 0, pos.w / 2 - 22, 134);
 	labelSpecialityName = std::make_shared<CLabel>(pos.w / 2, 188, FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE, (*CGI->heroh)[heroIndex]->getSpecialtyNameTranslated());
@@ -630,7 +609,7 @@ OptionsTab::SelectionWindow::SelectionWindow(PlayerColor _color, SelType _type)
 	selectedFaction = initialFaction;
 	selectedHero = initialHero;
 	selectedBonus = initialBonus;
-	allowedFactions = SEL->getPlayerInfo(color.getNum()).allowedFactions;
+	allowedFactions = SEL->getPlayerInfo(color).allowedFactions;
 	std::vector<bool> allowedHeroesFlag = SEL->getMapInfo()->mapHeader->allowedHeroes;
 	for(int i = 0; i < allowedHeroesFlag.size(); i++)
 		if(allowedHeroesFlag[i])
@@ -638,16 +617,19 @@ OptionsTab::SelectionWindow::SelectionWindow(PlayerColor _color, SelType _type)
 
 	for(auto & player : SEL->getStartInfo()->playerInfos)
 	{
-		if(player.first != color && (int)player.second.hero > PlayerSettings::RANDOM)
+		if(player.first != color && (int)player.second.hero > HeroTypeID::RANDOM)
 			unusableHeroes.insert(player.second.hero);
 	}
 
-	allowedBonus.push_back(-1); // random
-	if(initialHero.getNum() >= -1 || SEL->getPlayerInfo(color.getNum()).heroesNames.size() > 0)
-		allowedBonus.push_back(0); // artifact
-	allowedBonus.push_back(1); // gold
-	if(initialFaction.getNum() >= 0)
-		allowedBonus.push_back(2); // resource
+	allowedBonus.push_back(PlayerStartingBonus::RANDOM);
+
+	if(initialHero != HeroTypeID::NONE|| SEL->getPlayerInfo(color).heroesNames.size() > 0)
+		allowedBonus.push_back(PlayerStartingBonus::ARTIFACT);
+
+	allowedBonus.push_back(PlayerStartingBonus::GOLD);
+
+	if(initialFaction.isValid())
+		allowedBonus.push_back(PlayerStartingBonus::RESOURCE);
 
 	recreate();
 }
@@ -656,7 +638,7 @@ int OptionsTab::SelectionWindow::calcLines(FactionID faction)
 {
 	double additionalItems = 1; // random
 
-	if(faction.getNum() < 0)
+	if(!faction.isValid())
 		return std::ceil(((double)allowedFactions.size() + additionalItems) / elementsPerLine);
 
 	int count = 0;
@@ -692,7 +674,7 @@ void OptionsTab::SelectionWindow::setSelection()
 		CSH->setPlayerOption(LobbyChangePlayerOption::HERO_ID, selectedHero, color);
 
 	if(selectedBonus != initialBonus)
-		CSH->setPlayerOption(LobbyChangePlayerOption::BONUS_ID, selectedBonus, color);
+		CSH->setPlayerOption(LobbyChangePlayerOption::BONUS_ID, static_cast<int>(selectedBonus), color);
 }
 
 void OptionsTab::SelectionWindow::reopen()
@@ -728,7 +710,7 @@ void OptionsTab::SelectionWindow::recreate()
 			elementsPerLine = floor(sqrt(count));
 		}
 
-		amountLines = calcLines((type > SelType::TOWN) ? selectedFaction : static_cast<FactionID>(PlayerSettings::RANDOM));
+		amountLines = calcLines((type > SelType::TOWN) ? selectedFaction : FactionID::RANDOM);
 	}
 
 	int x = (elementsPerLine) * (ICON_BIG_WIDTH-1);
@@ -777,11 +759,11 @@ void OptionsTab::SelectionWindow::genContentFactions()
 
 	// random
 	PlayerSettings set = PlayerSettings();
-	set.castle = PlayerSettings::RANDOM;
+	set.castle = FactionID::RANDOM;
 	CPlayerSettingsHelper helper = CPlayerSettingsHelper(set, SelType::TOWN);
 	components.push_back(std::make_shared<CAnimImage>(helper.getImageName(), helper.getImageIndex(), 0, 6, (ICON_SMALL_HEIGHT/2)));
-	drawOutlinedText(TEXT_POS_X, TEXT_POS_Y, (selectedFaction.getNum() == PlayerSettings::RANDOM) ? Colors::YELLOW : Colors::WHITE, helper.getName());
-	if(selectedFaction.getNum() == PlayerSettings::RANDOM)
+	drawOutlinedText(TEXT_POS_X, TEXT_POS_Y, (selectedFaction == FactionID::RANDOM) ? Colors::YELLOW : Colors::WHITE, helper.getName());
+	if(selectedFaction == FactionID::RANDOM)
 		components.push_back(std::make_shared<CPicture>(ImagePath::builtin("lobby/townBorderSmallActivated"), 6, (ICON_SMALL_HEIGHT/2)));
 
 	for(auto & elem : allowedFactions)
@@ -809,11 +791,11 @@ void OptionsTab::SelectionWindow::genContentHeroes()
 
 	// random
 	PlayerSettings set = PlayerSettings();
-	set.hero = PlayerSettings::RANDOM;
+	set.hero = HeroTypeID::RANDOM;
 	CPlayerSettingsHelper helper = CPlayerSettingsHelper(set, SelType::HERO);
 	components.push_back(std::make_shared<CAnimImage>(helper.getImageName(), helper.getImageIndex(), 0, 6, (ICON_SMALL_HEIGHT/2)));
-	drawOutlinedText(TEXT_POS_X, TEXT_POS_Y, (selectedHero.getNum() == PlayerSettings::RANDOM) ? Colors::YELLOW : Colors::WHITE, helper.getName());
-	if(selectedHero.getNum() == PlayerSettings::RANDOM)
+	drawOutlinedText(TEXT_POS_X, TEXT_POS_Y, (selectedHero == HeroTypeID::RANDOM) ? Colors::YELLOW : Colors::WHITE, helper.getName());
+	if(selectedHero == HeroTypeID::RANDOM)
 		components.push_back(std::make_shared<CPicture>(ImagePath::builtin("lobby/townBorderSmallActivated"), 6, (ICON_SMALL_HEIGHT/2)));
 
 	for(auto & elem : allowedHeroes)
@@ -856,7 +838,7 @@ void OptionsTab::SelectionWindow::genContentBonus()
 		int x = i;
 		int y = 0;
 
-		set.bonus = static_cast<PlayerSettings::Ebonus>(elem);
+		set.bonus = elem;
 		CPlayerSettingsHelper helper = CPlayerSettingsHelper(set, SelType::BONUS);
 		components.push_back(std::make_shared<CAnimImage>(helper.getImageName(), helper.getImageIndex(), 0, x * (ICON_BIG_WIDTH-1) + 6, y * (ICON_BIG_HEIGHT-1) + (ICON_SMALL_HEIGHT/2)));
 		drawOutlinedText(x * (ICON_BIG_WIDTH-1) + TEXT_POS_X, y * (ICON_BIG_HEIGHT-1) + TEXT_POS_Y, Colors::WHITE , helper.getName());
@@ -892,9 +874,9 @@ void OptionsTab::SelectionWindow::setElement(int elem, bool doApply)
 		}
 		else
 		{
-			set.castle = PlayerSettings::RANDOM;
+			set.castle = FactionID::RANDOM;
 		}
-		if(set.castle.getNum() != PlayerSettings::NONE)
+		if(set.castle != FactionID::NONE)
 		{
 			if(!doApply)
 			{
@@ -916,18 +898,18 @@ void OptionsTab::SelectionWindow::setElement(int elem, bool doApply)
 		}
 		else
 		{
-			set.hero = PlayerSettings::RANDOM;
+			set.hero = HeroTypeID::RANDOM;
 		}
 
 		if(doApply && unusableHeroes.count(heroes[elem]))
 			return;
 
-		if(set.hero.getNum() != PlayerSettings::NONE)
+		if(set.hero != HeroTypeID::NONE)
 		{
 			if(!doApply)
 			{
 				CPlayerSettingsHelper helper = CPlayerSettingsHelper(set, SelType::HERO);
-				if(settings["general"]["enableUiEnhancements"].Bool() && helper.playerSettings.hero.getNum() > PlayerSettings::RANDOM && helper.playerSettings.heroNameTextId.empty())
+				if(settings["general"]["enableUiEnhancements"].Bool() && helper.playerSettings.hero.isValid() && helper.playerSettings.heroNameTextId.empty())
 					GH.windows().createAndPushWindow<CHeroOverview>(helper.playerSettings.hero);
 				else
 					GH.windows().createAndPushWindow<CPlayerOptionTooltipBox>(helper);
@@ -940,17 +922,15 @@ void OptionsTab::SelectionWindow::setElement(int elem, bool doApply)
 	{
 		if(elem >= 4)
 			return;
-		set.bonus = static_cast<PlayerSettings::Ebonus>(allowedBonus[elem]);
-		if(set.bonus != PlayerSettings::NONE)
+		set.bonus = static_cast<PlayerStartingBonus>(allowedBonus[elem]);
+
+		if(!doApply)
 		{
-			if(!doApply)
-			{
-				CPlayerSettingsHelper helper = CPlayerSettingsHelper(set, SelType::BONUS);
-				GH.windows().createAndPushWindow<CPlayerOptionTooltipBox>(helper);
-			}
-			else
-				selectedBonus = set.bonus;
+			CPlayerSettingsHelper helper = CPlayerSettingsHelper(set, SelType::BONUS);
+			GH.windows().createAndPushWindow<CPlayerOptionTooltipBox>(helper);
 		}
+		else
+			selectedBonus = set.bonus;
 	}
 
 	if(doApply)
@@ -1008,12 +988,12 @@ void OptionsTab::SelectedBox::update()
 void OptionsTab::SelectedBox::showPopupWindow(const Point & cursorPosition)
 {
 	// cases when we do not need to display a message
-	if(playerSettings.castle.getNum() == PlayerSettings::NONE && CPlayerSettingsHelper::type == TOWN)
+	if(playerSettings.castle == FactionID::NONE && CPlayerSettingsHelper::selectionType == TOWN)
 		return;
-	if(playerSettings.hero.getNum() == PlayerSettings::NONE && !SEL->getPlayerInfo(playerSettings.color.getNum()).hasCustomMainHero() && CPlayerSettingsHelper::type == HERO)
+	if(playerSettings.hero == HeroTypeID::NONE && !SEL->getPlayerInfo(playerSettings.color).hasCustomMainHero() && CPlayerSettingsHelper::selectionType == HERO)
 		return;
 
-	if(settings["general"]["enableUiEnhancements"].Bool() && CPlayerSettingsHelper::type == HERO && playerSettings.hero.getNum() > PlayerSettings::RANDOM && playerSettings.heroNameTextId.empty())
+	if(settings["general"]["enableUiEnhancements"].Bool() && CPlayerSettingsHelper::selectionType == HERO && playerSettings.hero.isValid() && playerSettings.heroNameTextId.empty())
 		GH.windows().createAndPushWindow<CHeroOverview>(playerSettings.hero);
 	else
 		GH.windows().createAndPushWindow<CPlayerOptionTooltipBox>(*this);
@@ -1021,20 +1001,20 @@ void OptionsTab::SelectedBox::showPopupWindow(const Point & cursorPosition)
 
 void OptionsTab::SelectedBox::clickReleased(const Point & cursorPosition)
 {
-	PlayerInfo pi = SEL->getPlayerInfo(playerSettings.color.getNum());
+	PlayerInfo pi = SEL->getPlayerInfo(playerSettings.color);
 	const bool foreignPlayer = CSH->isGuest() && !CSH->isMyColor(playerSettings.color);
 
-	if(type == SelType::TOWN && ((pi.allowedFactions.size() < 2 && !pi.isFactionRandom) || foreignPlayer))
+	if(selectionType == SelType::TOWN && ((pi.allowedFactions.size() < 2 && !pi.isFactionRandom) || foreignPlayer))
 		return;
 
-	if(type == SelType::HERO && ((pi.defaultHero() != -1 || playerSettings.castle.getNum() < 0) || foreignPlayer))
+	if(selectionType == SelType::HERO && ((pi.defaultHero() == HeroTypeID::NONE || !playerSettings.castle.isValid() || foreignPlayer)))
 		return;
 
-	if(type == SelType::BONUS && foreignPlayer)
+	if(selectionType == SelType::BONUS && foreignPlayer)
 		return;
 
 	GH.input().hapticFeedback();
-	GH.windows().createAndPushWindow<SelectionWindow>(playerSettings.color, type);
+	GH.windows().createAndPushWindow<SelectionWindow>(playerSettings.color, selectionType);
 }
 
 void OptionsTab::SelectedBox::scrollBy(int distance)
@@ -1044,7 +1024,7 @@ void OptionsTab::SelectedBox::scrollBy(int distance)
 	// so, currently, gesture will always move selection only by 1, and then wait for recreation from server info
 	distance = std::clamp(distance, -1, 1);
 
-	switch(CPlayerSettingsHelper::type)
+	switch(CPlayerSettingsHelper::selectionType)
 	{
 		case TOWN:
 			CSH->setPlayerOption(LobbyChangePlayerOption::TOWN, distance, playerSettings.color);
@@ -1061,7 +1041,7 @@ void OptionsTab::SelectedBox::scrollBy(int distance)
 }
 
 OptionsTab::PlayerOptionsEntry::PlayerOptionsEntry(const PlayerSettings & S, const OptionsTab & parent)
-	: pi(std::make_unique<PlayerInfo>(SEL->getPlayerInfo(S.color.getNum())))
+	: pi(std::make_unique<PlayerInfo>(SEL->getPlayerInfo(S.color)))
 	, s(std::make_unique<PlayerSettings>(S))
 	, parentTab(parent)
 {
@@ -1069,7 +1049,7 @@ OptionsTab::PlayerOptionsEntry::PlayerOptionsEntry(const PlayerSettings & S, con
 	defActions |= SHARE_POS;
 
 	int serial = 0;
-	for(int g = 0; g < s->color.getNum(); ++g)
+	for(PlayerColor g = PlayerColor(0); g < s->color; ++g)
 	{
 		auto itred = SEL->getPlayerInfo(g);
 		if(itred.canComputerPlay || itred.canHumanPlay)
@@ -1080,7 +1060,7 @@ OptionsTab::PlayerOptionsEntry::PlayerOptionsEntry(const PlayerSettings & S, con
 	pos.y += 128 + serial * 50;
 
 	assert(CSH->mi && CSH->mi->mapHeader);
-	const PlayerInfo & p = SEL->getPlayerInfo(s->color.getNum());
+	const PlayerInfo & p = SEL->getPlayerInfo(s->color);
 	assert(p.canComputerPlay || p.canHumanPlay); //someone must be able to control this player
 	if(p.canHumanPlay && p.canComputerPlay)
 		whoCanPlay = HUMAN_OR_CPU;
@@ -1100,7 +1080,7 @@ OptionsTab::PlayerOptionsEntry::PlayerOptionsEntry(const PlayerSettings & S, con
 		"ADOPOPNL.bmp", "ADOPPPNL.bmp", "ADOPTPNL.bmp", "ADOPSPNL.bmp"
 	}};
 
-	background = std::make_shared<CPicture>(ImagePath::builtin(bgs[s->color.getNum()]), 0, 0);
+	background = std::make_shared<CPicture>(ImagePath::builtin(bgs[s->color]), 0, 0);
 	labelPlayerName = std::make_shared<CLabel>(55, 10, EFonts::FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE, s->name);
 	labelWhoCanPlay = std::make_shared<CMultiLineLabel>(Rect(6, 23, 45, (int)graphics->fonts[EFonts::FONT_TINY]->getLineHeight()*2), EFonts::FONT_TINY, ETextAlignment::CENTER, Colors::WHITE, CGI->generaltexth->arraytxt[206 + whoCanPlay]);
 
@@ -1116,7 +1096,7 @@ OptionsTab::PlayerOptionsEntry::PlayerOptionsEntry(const PlayerSettings & S, con
 
 	hideUnavailableButtons();
 
-	if(SEL->screenType != ESelectionScreen::scenarioInfo && SEL->getPlayerInfo(s->color.getNum()).canHumanPlay)
+	if(SEL->screenType != ESelectionScreen::scenarioInfo && SEL->getPlayerInfo(s->color).canHumanPlay)
 	{
 		flag = std::make_shared<CButton>(
 			Point(-43, 2),
@@ -1159,7 +1139,7 @@ void OptionsTab::PlayerOptionsEntry::hideUnavailableButtons()
 		buttonTownRight->enable();
 	}
 
-	if((pi->defaultHero() != -1 || s->castle.getNum() < 0) //fixed hero
+	if((pi->defaultHero() != HeroTypeID::RANDOM || !s->castle.isValid()) //fixed hero
 		|| foreignPlayer) //or not our player
 	{
 		buttonHeroLeft->disable();

client/lobby/OptionsTab.h

@@ -16,6 +16,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 struct PlayerSettings;
 struct PlayerInfo;
+enum class PlayerStartingBonus : int8_t;
 VCMI_LIB_NAMESPACE_END
 
 class CLabel;
@@ -54,10 +55,10 @@ private:
 	struct CPlayerSettingsHelper
 	{
 		const PlayerSettings & playerSettings;
-		const SelType type;
+		const SelType selectionType;
 
 		CPlayerSettingsHelper(const PlayerSettings & playerSettings, SelType type)
-			: playerSettings(playerSettings), type(type)
+			: playerSettings(playerSettings), selectionType(type)
 		{}
 
 		/// visible image settings
@@ -118,14 +119,14 @@ private:
 
 		FactionID initialFaction;
 		HeroTypeID initialHero;
-		int initialBonus;
+		PlayerStartingBonus initialBonus;
 		FactionID selectedFaction;
 		HeroTypeID selectedHero;
-		int selectedBonus;
+		PlayerStartingBonus selectedBonus;
 
 		std::set<FactionID> allowedFactions;
 		std::set<HeroTypeID> allowedHeroes;
-		std::vector<int> allowedBonus;
+		std::vector<PlayerStartingBonus> allowedBonus;
 
 		void genContentGrid(int lines);
 		void genContentFactions();

client/widgets/CComponent.cpp

@@ -30,6 +30,7 @@
 
 #include "../../lib/ArtifactUtils.h"
 #include "../../lib/CTownHandler.h"
+#include "../../lib/CHeroHandler.h"
 #include "../../lib/spells/CSpellHandler.h"
 #include "../../lib/CCreatureHandler.h"
 #include "../../lib/CSkillHandler.h"
@@ -153,7 +154,7 @@ size_t CComponent::getIndex()
 	case morale:     return val+3;
 	case luck:       return val+3;
 	case building:   return val;
-	case hero:       return subtype;
+	case hero:       return CGI->heroTypes()->getByIndex(subtype)->getIconIndex();
 	case flag:       return subtype;
 	}
 	assert(0);

client/widgets/MiscWidgets.cpp

@@ -131,7 +131,7 @@ CHeroArea::CHeroArea(int x, int y, const CGHeroInstance * hero)
 
 	if(hero)
 	{
-		portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero->portrait);
+		portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero->getIconIndex());
 		clickFunctor = [hero]() -> void
 		{
 			LOCPLINT->openHeroWindow(hero);
@@ -291,7 +291,7 @@ CArmyTooltip::CArmyTooltip(Point pos, const CArmedInstance * army):
 void CHeroTooltip::init(const InfoAboutHero & hero)
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
-	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero.portrait, 0, 3, 2);
+	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero.getIconIndex(), 0, 3, 2);
 
 	if(hero.details)
 	{
@@ -329,7 +329,7 @@ CInteractableHeroTooltip::CInteractableHeroTooltip(Point pos, const CGHeroInstan
 void CInteractableHeroTooltip::init(const InfoAboutHero & hero)
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
-	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero.portrait, 0, 3, 2);
+	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero.getIconIndex(), 0, 3, 2);
 	title = std::make_shared<CLabel>(66, 2, FONT_SMALL, ETextAlignment::TOPLEFT, Colors::WHITE, hero.name);
 
 	if(hero.details)
@@ -607,4 +607,4 @@ SimpleLine::SimpleLine(Point pos1, Point pos2, ColorRGBA color) :
 void SimpleLine::showAll(Canvas & to) 
 {
 	to.drawLine(pos1 + pos.topLeft(), pos2 + pos.topLeft(), color, color);
-}
+}

client/windows/CCastleInterface.cpp

@@ -450,7 +450,7 @@ void CHeroGSlot::set(const CGHeroInstance * newHero)
 	if(newHero)
 	{
 		portrait->visible = true;
-		portrait->setFrame(newHero->portrait);
+		portrait->setFrame(newHero->getIconIndex());
 	}
 	else if(!upg && owner->showEmpty) //up garrison
 	{

client/windows/CHeroWindow.cpp

@@ -64,7 +64,7 @@ CHeroSwitcher::CHeroSwitcher(CHeroWindow * owner_, Point pos_, const CGHeroInsta
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
 	pos += pos_;
 
-	image = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsSmall"), hero->portrait);
+	image = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsSmall"), hero->getIconIndex());
 	pos.w = image->pos.w;
 	pos.h = image->pos.h;
 }
@@ -202,7 +202,7 @@ void CHeroWindow::update(const CGHeroInstance * hero, bool redrawNeeded)
 	dismissButton->addHoverText(CButton::NORMAL, boost::str(boost::format(CGI->generaltexth->heroscrn[16]) % curHero->getNameTranslated() % curHero->type->heroClass->getNameTranslated()));
 	portraitArea->hoverText = boost::str(boost::format(CGI->generaltexth->allTexts[15]) % curHero->getNameTranslated() % curHero->type->heroClass->getNameTranslated());
 	portraitArea->text = curHero->getBiographyTranslated();
-	portraitImage->setFrame(curHero->portrait);
+	portraitImage->setFrame(curHero->getIconIndex());
 
 	{
 		OBJECT_CONSTRUCTION_CUSTOM_CAPTURING(255-DISPOSE);

client/windows/CKingdomInterface.cpp

@@ -915,7 +915,7 @@ CHeroItem::CHeroItem(const CGHeroInstance * Hero)
 
 	garr = std::make_shared<CGarrisonInt>(Point(6, 78), 4, Point(), hero, nullptr, true, true);
 
-	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero->portrait, 0, 5, 6);
+	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero->getIconIndex(), 0, 5, 6);
 	heroArea = std::make_shared<CHeroArea>(5, 6, hero);
 
 	name = std::make_shared<CLabel>(73, 7, FONT_SMALL, ETextAlignment::TOPLEFT, Colors::WHITE, hero->getNameTranslated());

client/windows/GUIClasses.cpp

@@ -401,7 +401,7 @@ CLevelWindow::CLevelWindow(const CGHeroInstance * hero, PrimarySkill pskill, std
 		box = std::make_shared<CComponentBox>(comps, Rect(75, 300, pos.w - 150, 100));
 	}
 
-	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero->portrait, 0, 170, 66);
+	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero->getIconIndex(), 0, 170, 66);
 	ok = std::make_shared<CButton>(Point(297, 413), AnimationPath::builtin("IOKAY"), CButton::tooltip(), std::bind(&CLevelWindow::close, this), EShortcut::GLOBAL_ACCEPT);
 
 	//%s has gained a level.
@@ -580,7 +580,7 @@ CTavernWindow::HeroPortrait::HeroPortrait(int & sel, int id, int x, int y, const
 		boost::algorithm::replace_first(description, "%s", h->type->heroClass->getNameTranslated());
 		boost::algorithm::replace_first(description, "%d", std::to_string(artifs));
 
-		portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("portraitsLarge"), h->portrait);
+		portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("portraitsLarge"), h->getIconIndex());
 	}
 }
 
@@ -1210,7 +1210,7 @@ CGarrisonWindow::CGarrisonWindow(const CArmedInstance * up, const CGHeroInstance
 	title = std::make_shared<CLabel>(275, 30, FONT_BIG, ETextAlignment::CENTER, Colors::YELLOW, titleText);
 
 	banner = std::make_shared<CAnimImage>(AnimationPath::builtin("CREST58"), up->getOwner().getNum(), 0, 28, 124);
-	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), down->portrait, 0, 29, 222);
+	portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), down->getIconIndex(), 0, 29, 222);
 }
 
 void CGarrisonWindow::updateGarrisons()
@@ -1501,9 +1501,9 @@ CThievesGuildWindow::CThievesGuildWindow(const CGObjectInstance * _owner):
 	for(auto & iter : tgi.colorToBestHero)
 	{
 		banners.push_back(std::make_shared<CPicture>(ImagePath::builtin(colorToBox[iter.first.getNum()]), 253 + 66 * counter, 334));
-		if(iter.second.portrait >= 0)
+		if(iter.second.portraitSource.isValid())
 		{
-			bestHeroes.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsSmall"), iter.second.portrait, 0, 260 + 66 * counter, 360));
+			bestHeroes.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsSmall"), iter.second.getIconIndex(), 0, 260 + 66 * counter, 360));
 			//TODO: r-click info:
 			// - r-click on hero
 			// - r-click on primary skill label

config/gameConfig.json

@@ -42,7 +42,8 @@
 		"config/heroes/stronghold.json",
 		"config/heroes/fortress.json",
 		"config/heroes/conflux.json",
-		"config/heroes/special.json"
+		"config/heroes/special.json",
+		"config/heroes/portraits.json"
 	],
 
 	"objects" :
@@ -218,6 +219,11 @@
 						"special2" : 19, // bloodObelisk
 						"special3" : 18  // glyphsOfFear
 					}
+				},
+
+				"portraits" : {
+					"catherine" : 128, // In "RoE" Catherine only has portrait
+					"portraitGeneralKendal" : 129
 				}
 			},
 			"armageddonsBlade" : {
@@ -228,11 +234,27 @@
 						"special1" : 10, // artifactMerchants
 						"special2" : 18  // magicUniversity
 					}
+				},
+				
+				"portraits" : {
+					"portraitGeneralKendal" : 156,
+					"portraitYoungCristian" : 157,
+					"portraitOrdwald" : 158
 				}
 			},
 			"shadowOfDeath" : { 
 				"supported" : true,
-				"iconIndex" : 2
+				"iconIndex" : 2,
+				
+				"portraits" : {
+					"portraitGeneralKendal" : 156,
+					"portraitYoungCristian" : 157,
+					"portraitOrdwald" : 158,
+					"portraitFinneas" : 159,
+					"portraitYoungGem" : 160,
+					"portraitYoungSandro" : 161,
+					"portraitYoungYog" : 162
+				}
 			},
 			"jsonVCMI" : { 
 				"supported" : true,

config/heroes/portraits.json

@@ -0,0 +1,206 @@
+{
+	// additional empty heroes for correct loading of hero portraits set in map editor
+	"portraitGeneralKendal" :
+	{
+		"class" : "knight",
+		"special" : true,
+		"images": {
+			"large" : "HPL129MK",
+			"small" : "HPS129MK",
+			"specialtySmall" : "default",
+			"specialtyLarge" : "default"
+		},
+		"texts" : {
+			"name" : "",
+			"biography" : "",
+			"specialty" : {
+				"description" : "",
+				"tooltip" : "",
+				"name" : ""
+			}
+		},
+		"army" : [
+			{
+				"creature" : "pikeman",
+				"min" : 1,
+				"max" : 1
+			}
+		],
+		"skills" : [],
+		"specialty" : {}
+	},
+	"portraitYoungCristian" :
+	{
+		"class" : "knight",
+		"special" : true,
+		"images": {
+			"large" : "HPL002SH",
+			"small" : "HPS002SH",
+			"specialtySmall" : "default",
+			"specialtyLarge" : "default"
+		},
+		"texts" : {
+			"name" : "",
+			"biography" : "",
+			"specialty" : {
+				"description" : "",
+				"tooltip" : "",
+				"name" : ""
+			}
+		},
+		"army" : [
+			{
+				"creature" : "pikeman",
+				"min" : 1,
+				"max" : 1
+			}
+		],
+		"skills" : [],
+		"specialty" : {}
+	},
+	"portraitOrdwald" :
+	{
+		"class" : "druid",
+		"special" : true,
+		"images": {
+			"large" : "HPL132Wl",
+			"small" : "HPS132Wl",
+			"specialtySmall" : "default",
+			"specialtyLarge" : "default"
+		},
+		"texts" : {
+			"name" : "",
+			"biography" : "",
+			"specialty" : {
+				"description" : "",
+				"tooltip" : "",
+				"name" : ""
+			}
+		},
+		"army" : [
+			{
+				"creature" : "centaur",
+				"min" : 1,
+				"max" : 1
+			}
+		],
+		"skills" : [],
+		"specialty" : {}
+	},
+	"portraitFinneas" :
+	{
+		"class" : "necromancer",
+		"special" : true,
+		"images": {
+			"large" : "HPL133Nc",
+			"small" : "HPS133Nc",
+			"specialtySmall" : "default",
+			"specialtyLarge" : "default"
+		},
+		"texts" : {
+			"name" : "",
+			"biography" : "",
+			"specialty" : {
+				"description" : "",
+				"tooltip" : "",
+				"name" : ""
+			}
+		},
+		"army" : [
+			{
+				"creature" : "skeleton",
+				"min" : 1,
+				"max" : 1
+			}
+		],
+		"skills" : [],
+		"specialty" : {}
+	},
+	"portraitYoungGem" :
+	{
+		"class" : "druid",
+		"special" : true,
+		"images": {
+			"large" : "HPL134Nc",
+			"small" : "HPS134Nc",
+			"specialtySmall" : "default",
+			"specialtyLarge" : "default"
+		},
+		"texts" : {
+			"name" : "",
+			"biography" : "",
+			"specialty" : {
+				"description" : "",
+				"tooltip" : "",
+				"name" : ""
+			}
+		},
+		"army" : [
+			{
+				"creature" : "centaur",
+				"min" : 1,
+				"max" : 1
+			}
+		],
+		"skills" : [],
+		"specialty" : {}
+	},
+	"portraitYoungSandro" :
+	{
+		"class" : "necromancer",
+		"special" : true,
+		"images": {
+			"large" : "HPL135Wi",
+			"small" : "HPS135Wi",
+			"specialtySmall" : "default",
+			"specialtyLarge" : "default"
+		},
+		"texts" : {
+			"name" : "",
+			"biography" : "",
+			"specialty" : {
+				"description" : "",
+				"tooltip" : "",
+				"name" : ""
+			}
+		},
+		"army" : [
+			{
+				"creature" : "skeleton",
+				"min" : 1,
+				"max" : 1
+			}
+		],
+		"skills" : [],
+		"specialty" : {}
+	},
+	"portraitYoungYog" :
+	{
+		"class" : "wizard",
+		"special" : true,
+		"images": {
+			"large" : "HPL136Wi",
+			"small" : "HPS136Wi",
+			"specialtySmall" : "default",
+			"specialtyLarge" : "default"
+		},
+		"texts" : {
+			"name" : "",
+			"biography" : "",
+			"specialty" : {
+				"description" : "",
+				"tooltip" : "",
+				"name" : ""
+			}
+		},
+		"army" : [
+			{
+				"creature" : "gremlin",
+				"min" : 1,
+				"max" : 1
+			}
+		],
+		"skills" : [],
+		"specialty" : {}
+	}
+}

lib/CHeroHandler.cpp

@@ -298,7 +298,7 @@ CHeroClass * CHeroClassHandler::loadFromJson(const std::string & scope, const Js
 	VLC->identifiers()->requestIdentifier("faction", node["faction"],
 	[=](si32 factionID)
 	{
-		heroClass->faction = factionID;
+		heroClass->faction.setNum(factionID);
 	});
 
 	VLC->identifiers()->requestIdentifier(scope, "object", "hero", [=](si32 index)
@@ -722,8 +722,9 @@ std::vector<JsonNode> CHeroHandler::loadLegacyData()
 void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNode & data)
 {
 	size_t index = objects.size();
+	static const int specialFramesCount = 2; // reserved for 2 special frames
 	auto * object = loadFromJson(scope, data, name, index);
-	object->imageIndex = static_cast<si32>(index) + GameConstants::HERO_PORTRAIT_SHIFT; // 2 special frames + some extra portraits
+	object->imageIndex = static_cast<si32>(index) + specialFramesCount;
 
 	objects.emplace_back(object);
 

lib/StartInfo.cpp

@@ -11,6 +11,8 @@
 #include "StartInfo.h"
 
 #include "CGeneralTextHandler.h"
+#include "CTownHandler.h"
+#include "CHeroHandler.h"
 #include "VCMI_Lib.h"
 #include "rmg/CMapGenOptions.h"
 #include "mapping/CMapInfo.h"
@@ -22,9 +24,28 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 PlayerSettings::PlayerSettings()
-	: bonus(RANDOM), castle(NONE), hero(RANDOM), heroPortrait(RANDOM), color(0), handicap(NO_HANDICAP), compOnly(false)
+	: bonus(PlayerStartingBonus::RANDOM), color(0), handicap(NO_HANDICAP), compOnly(false)
 {
+}
+
+FactionID PlayerSettings::getCastleValidated() const
+{
+	if (!castle.isValid())
+		return FactionID(0);
+	if (castle.getNum() < VLC->townh->size())
+		return castle;
+
+	return FactionID(0);
+}
+
+HeroTypeID PlayerSettings::getHeroValidated() const
+{
+	if (!hero.isValid())
+		return HeroTypeID(0);
+	if (hero.getNum() < VLC->heroh->size())
+		return hero;
 
+	return HeroTypeID(0);
 }
 
 bool PlayerSettings::isControlledByAI() const
@@ -196,15 +217,15 @@ ui8 LobbyInfo::clientFirstId(int clientId) const
 	return 0;
 }
 
-PlayerInfo & LobbyInfo::getPlayerInfo(int color)
+PlayerInfo & LobbyInfo::getPlayerInfo(PlayerColor color)
 {
-	return mi->mapHeader->players[color];
+	return mi->mapHeader->players[color.getNum()];
 }
 
 TeamID LobbyInfo::getPlayerTeamId(const PlayerColor & color)
 {
 	if(color.isValidPlayer())
-		return getPlayerInfo(color.getNum()).team;
+		return getPlayerInfo(color).team;
 	else
 		return TeamID::NO_TEAM;
 }

lib/StartInfo.h

@@ -41,20 +41,20 @@ struct DLL_LINKAGE SimturnsInfo
 	}
 };
 
+enum class PlayerStartingBonus : int8_t
+{
+	RANDOM   = -1,
+	ARTIFACT =  0,
+	GOLD     =  1,
+	RESOURCE =  2
+};
+
 /// Struct which describes the name, the color, the starting bonus of a player
 struct DLL_LINKAGE PlayerSettings
 {
 	enum { PLAYER_AI = 0 }; // for use in playerID
 
-	enum Ebonus {
-		NONE     = -2,
-		RANDOM   = -1,
-		ARTIFACT =  0,
-		GOLD     =  1,
-		RESOURCE =  2
-	};
-
-	Ebonus bonus;
+	PlayerStartingBonus bonus;
 	FactionID castle;
 	HeroTypeID hero;
 	HeroTypeID heroPortrait; //-1 if default, else ID
@@ -85,6 +85,9 @@ struct DLL_LINKAGE PlayerSettings
 	PlayerSettings();
 	bool isControlledByAI() const;
 	bool isControlledByHuman() const;
+
+	FactionID getCastleValidated() const;
+	HeroTypeID getHeroValidated() const;
 };
 
 /// Struct which describes the difficulty, the turn time,.. of a heroes match.
@@ -197,7 +200,7 @@ struct DLL_LINKAGE LobbyInfo : public LobbyState
 	PlayerColor clientFirstColor(int clientId) const;
 	bool isClientColor(int clientId, const PlayerColor & color) const;
 	ui8 clientFirstId(int clientId) const; // Used by chat only!
-	PlayerInfo & getPlayerInfo(int color);
+	PlayerInfo & getPlayerInfo(PlayerColor color);
 	TeamID getPlayerTeamId(const PlayerColor & color);
 };
 

lib/campaign/CampaignHandler.cpp

@@ -273,7 +273,7 @@ CampaignTravel CampaignHandler::readScenarioTravelFromJson(JsonNode & reader)
 		break;
 	case CampaignStartOptions::START_BONUS: //reading of bonuses player can choose
 		{
-			ret.playerColor = reader["playerColor"].Integer();
+			ret.playerColor = PlayerColor(PlayerColor::decode(reader["playerColor"].String()));
 			for(auto & bjson : reader["bonuses"].Vector())
 			{
 				CampaignBonus bonus;
@@ -472,7 +472,7 @@ CampaignTravel CampaignHandler::readScenarioTravelFromMemory(CBinaryReader & rea
 		break;
 	case CampaignStartOptions::START_BONUS: //reading of bonuses player can choose
 		{
-			ret.playerColor = reader.readUInt8();
+			ret.playerColor.setNum(reader.readUInt8());
 			ui8 numOfBonuses = reader.readUInt8();
 			for (int g=0; g<numOfBonuses; ++g)
 			{

lib/constants/EntityIdentifiers.cpp

@@ -38,60 +38,60 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-const BattleID BattleID::NONE = BattleID(-1);
-const QueryID QueryID::NONE = QueryID(-1);
-const HeroTypeID HeroTypeID::NONE = HeroTypeID(-1);
-const ObjectInstanceID ObjectInstanceID::NONE = ObjectInstanceID(-1);
-
-const SlotID SlotID::COMMANDER_SLOT_PLACEHOLDER = SlotID(-2);
-const SlotID SlotID::SUMMONED_SLOT_PLACEHOLDER = SlotID(-3);
-const SlotID SlotID::WAR_MACHINES_SLOT = SlotID(-4);
-const SlotID SlotID::ARROW_TOWERS_SLOT = SlotID(-5);
-
-const PlayerColor PlayerColor::SPECTATOR = PlayerColor(-4);
-const PlayerColor PlayerColor::CANNOT_DETERMINE = PlayerColor(-3);
-const PlayerColor PlayerColor::UNFLAGGABLE = PlayerColor(-2);
-const PlayerColor PlayerColor::NEUTRAL = PlayerColor(-1);
-const PlayerColor PlayerColor::PLAYER_LIMIT = PlayerColor(PLAYER_LIMIT_I);
-const TeamID TeamID::NO_TEAM = TeamID(-1);
-
-const SpellSchool SpellSchool::ANY = -1;
-const SpellSchool SpellSchool::AIR = 0;
-const SpellSchool SpellSchool::FIRE = 1;
-const SpellSchool SpellSchool::WATER = 2;
-const SpellSchool SpellSchool::EARTH = 3;
-
-const FactionID FactionID::NONE = -2;
-const FactionID FactionID::DEFAULT = -1;
-const FactionID FactionID::RANDOM = -1;
-const FactionID FactionID::ANY = -1;
-const FactionID FactionID::CASTLE = 0;
-const FactionID FactionID::RAMPART = 1;
-const FactionID FactionID::TOWER = 2;
-const FactionID FactionID::INFERNO = 3;
-const FactionID FactionID::NECROPOLIS = 4;
-const FactionID FactionID::DUNGEON = 5;
-const FactionID FactionID::STRONGHOLD = 6;
-const FactionID FactionID::FORTRESS = 7;
-const FactionID FactionID::CONFLUX = 8;
-const FactionID FactionID::NEUTRAL = 9;
-
-const BoatId BoatId::NONE = -1;
-const BoatId BoatId::NECROPOLIS = 0;
-const BoatId BoatId::CASTLE = 1;
-const BoatId BoatId::FORTRESS = 2;
-
-const RiverId RiverId::NO_RIVER = 0;
-const RiverId RiverId::WATER_RIVER = 1;
-const RiverId RiverId::ICY_RIVER = 2;
-const RiverId RiverId::MUD_RIVER = 3;
-const RiverId RiverId::LAVA_RIVER = 4;
-
-const RoadId RoadId::NO_ROAD = 0;
-const RoadId RoadId::DIRT_ROAD = 1;
-const RoadId RoadId::GRAVEL_ROAD = 2;
-const RoadId RoadId::COBBLESTONE_ROAD = 3;
-
+const BattleID BattleID::NONE(-1);
+const QueryID QueryID::NONE(-1);
+const HeroTypeID HeroTypeID::NONE(-1);
+const HeroTypeID HeroTypeID::RANDOM(-2);
+const ObjectInstanceID ObjectInstanceID::NONE(-1);
+
+const SlotID SlotID::COMMANDER_SLOT_PLACEHOLDER(-2);
+const SlotID SlotID::SUMMONED_SLOT_PLACEHOLDER(-3);
+const SlotID SlotID::WAR_MACHINES_SLOT(-4);
+const SlotID SlotID::ARROW_TOWERS_SLOT(-5);
+
+const PlayerColor PlayerColor::SPECTATOR(-4);
+const PlayerColor PlayerColor::CANNOT_DETERMINE(-3);
+const PlayerColor PlayerColor::UNFLAGGABLE(-2);
+const PlayerColor PlayerColor::NEUTRAL(-1);
+const PlayerColor PlayerColor::PLAYER_LIMIT(PLAYER_LIMIT_I);
+const TeamID TeamID::NO_TEAM(-1);
+
+const SpellSchool SpellSchool::ANY(-1);
+const SpellSchool SpellSchool::AIR(0);
+const SpellSchool SpellSchool::FIRE(1);
+const SpellSchool SpellSchool::WATER(2);
+const SpellSchool SpellSchool::EARTH(3);
+
+const FactionID FactionID::NONE(-2);
+const FactionID FactionID::DEFAULT(-1);
+const FactionID FactionID::RANDOM(-1);
+const FactionID FactionID::ANY(-1);
+const FactionID FactionID::CASTLE(0);
+const FactionID FactionID::RAMPART(1);
+const FactionID FactionID::TOWER(2);
+const FactionID FactionID::INFERNO(3);
+const FactionID FactionID::NECROPOLIS(4);
+const FactionID FactionID::DUNGEON(5);
+const FactionID FactionID::STRONGHOLD(6);
+const FactionID FactionID::FORTRESS(7);
+const FactionID FactionID::CONFLUX(8);
+const FactionID FactionID::NEUTRAL(9);
+
+const BoatId BoatId::NONE(-1);
+const BoatId BoatId::NECROPOLIS(0);
+const BoatId BoatId::CASTLE(1);
+const BoatId BoatId::FORTRESS(2);
+
+const RiverId RiverId::NO_RIVER(0);
+const RiverId RiverId::WATER_RIVER(1);
+const RiverId RiverId::ICY_RIVER(2);
+const RiverId RiverId::MUD_RIVER(3);
+const RiverId RiverId::LAVA_RIVER(4);
+
+const RoadId RoadId::NO_ROAD(0);
+const RoadId RoadId::DIRT_ROAD(1);
+const RoadId RoadId::GRAVEL_ROAD(2);
+const RoadId RoadId::COBBLESTONE_ROAD(3);
 
 namespace GameConstants
 {

lib/constants/EntityIdentifiers.h

@@ -38,7 +38,11 @@ class CNonConstInfoCallback;
 class IdentifierBase
 {
 protected:
-	constexpr IdentifierBase(int32_t value = -1 ):
+	constexpr IdentifierBase():
+		num(-1)
+	{}
+
+	explicit constexpr IdentifierBase(int32_t value):
 		num(value)
 	{}
 
@@ -51,6 +55,11 @@ public:
 		return num;
 	}
 
+	constexpr void setNum(int32_t value)
+	{
+		num = value;
+	}
+
 	struct hash
 	{
 		size_t operator()(const IdentifierBase & id) const
@@ -88,8 +97,11 @@ class Identifier : public IdentifierBase
 {
 	using BaseClass = IdentifierBase;
 public:
-	constexpr Identifier(int32_t _num = -1)
-		:IdentifierBase(_num)
+	constexpr Identifier()
+	{}
+
+	explicit constexpr Identifier(int32_t value):
+		IdentifierBase(value)
 	{}
 
 	constexpr bool operator == (const Identifier & b) const { return BaseClass::num == b.num; }
@@ -105,6 +117,19 @@ public:
 		return static_cast<FinalClass&>(*this);
 	}
 
+	constexpr FinalClass & operator--()
+	{
+		--BaseClass::num;
+		return static_cast<FinalClass&>(*this);
+	}
+
+	constexpr FinalClass operator--(int)
+	{
+		FinalClass ret(num);
+		--BaseClass::num;
+		return ret;
+	}
+
 	constexpr FinalClass operator++(int)
 	{
 		FinalClass ret(num);
@@ -209,6 +234,12 @@ public:
 	static std::string entityType();
 
 	DLL_LINKAGE static const HeroTypeID NONE;
+	DLL_LINKAGE static const HeroTypeID RANDOM;
+
+	bool isValid() const
+	{
+		return getNum() >= 0;
+	}
 };
 
 class SlotID : public Identifier<SlotID>
@@ -336,6 +367,11 @@ public:
 	static si32 decode(const std::string& identifier);
 	static std::string encode(const si32 index);
 	static std::string entityType();
+
+	bool isValid() const
+	{
+		return getNum() >= 0;
+	}
 };
 
 class BuildingIDBase : public IdentifierBase

lib/constants/NumericConstants.h

@@ -50,8 +50,6 @@ namespace GameConstants
 	constexpr int CREATURES_COUNT = 197;
 
 	constexpr ui32 BASE_MOVEMENT_COST = 100; //default cost for non-diagonal movement
-
-	constexpr int HERO_PORTRAIT_SHIFT = 9;// 2 special frames + 7 extra portraits
 }
 
 VCMI_LIB_NAMESPACE_END

lib/gameState/CGameState.cpp

@@ -824,7 +824,7 @@ void CGameState::placeStartingHeroes()
 			if (campaign && campaign->playerHasStartingHero(playerColor))
 				continue;
 
-			int heroTypeId = pickNextHeroType(playerColor);
+			HeroTypeID heroTypeId = pickNextHeroType(playerColor);
 			if(playerSettingPair.second.hero == HeroTypeID::NONE)
 				playerSettingPair.second.hero = heroTypeId;
 
@@ -959,14 +959,15 @@ void CGameState::initStartingBonus()
 	for(auto & elem : players)
 	{
 		//starting bonus
-		if(scenarioOps->playerInfos[elem.first].bonus==PlayerSettings::RANDOM)
-			scenarioOps->playerInfos[elem.first].bonus = static_cast<PlayerSettings::Ebonus>(getRandomGenerator().nextInt(2));
+		if(scenarioOps->playerInfos[elem.first].bonus == PlayerStartingBonus::RANDOM)
+			scenarioOps->playerInfos[elem.first].bonus = static_cast<PlayerStartingBonus>(getRandomGenerator().nextInt(2));
+
 		switch(scenarioOps->playerInfos[elem.first].bonus)
 		{
-		case PlayerSettings::GOLD:
+		case PlayerStartingBonus::GOLD:
 			elem.second.resources[EGameResID::GOLD] += getRandomGenerator().nextInt(5, 10) * 100;
 			break;
-		case PlayerSettings::RESOURCE:
+		case PlayerStartingBonus::RESOURCE:
 			{
 				auto res = (*VLC->townh)[scenarioOps->playerInfos[elem.first].castle]->town->primaryRes;
 				if(res == EGameResID::WOOD_AND_ORE)
@@ -981,7 +982,7 @@ void CGameState::initStartingBonus()
 				}
 				break;
 			}
-		case PlayerSettings::ARTIFACT:
+		case PlayerStartingBonus::ARTIFACT:
 			{
 				if(elem.second.heroes.empty())
 				{
@@ -2117,7 +2118,7 @@ std::set<HeroTypeID> CGameState::getUnusedAllowedHeroes(bool alsoIncludeNotAllow
 
 	for(const auto & playerSettingPair : scenarioOps->playerInfos) //remove uninitialized yet heroes picked for start by other players
 	{
-		if(playerSettingPair.second.hero.getNum() != PlayerSettings::RANDOM)
+		if(playerSettingPair.second.hero != HeroTypeID::RANDOM)
 			ret -= HeroTypeID(playerSettingPair.second.hero);
 	}
 

lib/gameState/CGameStateCampaign.cpp

@@ -233,7 +233,7 @@ void CGameStateCampaign::placeCampaignHeroes()
 	// now add removed heroes again with unused type ID
 	for(auto * hero : removedHeroes)
 	{
-		si32 heroTypeId = 0;
+		HeroTypeID heroTypeId;
 		if(hero->ID == Obj::HERO)
 		{
 			heroTypeId = gameState->pickUnusedHeroTypeRandomly(hero->tempOwner);
@@ -243,7 +243,7 @@ void CGameStateCampaign::placeCampaignHeroes()
 			auto unusedHeroTypeIds = gameState->getUnusedAllowedHeroes();
 			if(!unusedHeroTypeIds.empty())
 			{
-				heroTypeId = (*RandomGeneratorUtil::nextItem(unusedHeroTypeIds, gameState->getRandomGenerator())).getNum();
+				heroTypeId = (*RandomGeneratorUtil::nextItem(unusedHeroTypeIds, gameState->getRandomGenerator()));
 			}
 			else
 			{
@@ -257,7 +257,6 @@ void CGameStateCampaign::placeCampaignHeroes()
 		}
 
 		hero->subID = heroTypeId;
-		hero->portrait = hero->subID;
 		gameState->map->getEditManager()->insertObject(hero);
 	}
 }

lib/gameState/InfoAboutArmy.cpp

@@ -73,18 +73,18 @@ void InfoAboutHero::assign(const InfoAboutHero & iah)
 
 	details = (iah.details ? new Details(*iah.details) : nullptr);
 	hclass = iah.hclass;
-	portrait = iah.portrait;
+	portraitSource = iah.portraitSource;
 }
 
-InfoAboutHero::InfoAboutHero(): portrait(-1) {}
+InfoAboutHero::InfoAboutHero()
+{}
 
 InfoAboutHero::InfoAboutHero(const InfoAboutHero & iah): InfoAboutArmy(iah)
 {
 	assign(iah);
 }
 
-InfoAboutHero::InfoAboutHero(const CGHeroInstance * h, InfoAboutHero::EInfoLevel infoLevel):
-	portrait(-1)
+InfoAboutHero::InfoAboutHero(const CGHeroInstance * h, InfoAboutHero::EInfoLevel infoLevel)
 {
 	initFromHero(h, infoLevel);
 }
@@ -100,6 +100,11 @@ InfoAboutHero & InfoAboutHero::operator=(const InfoAboutHero & iah)
 	return *this;
 }
 
+int32_t InfoAboutHero::getIconIndex() const
+{
+	return VLC->heroTypes()->getById(portraitSource)->getIconIndex();
+}
+
 void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLevel infoLevel)
 {
 	vstd::clear_pointer(details);
@@ -112,7 +117,7 @@ void InfoAboutHero::initFromHero(const CGHeroInstance *h, InfoAboutHero::EInfoLe
 
 	hclass = h->type->heroClass;
 	name = h->getNameTranslated();
-	portrait = h->portrait;
+	portraitSource = h->getPortraitSource();
 
 	if(detailed)
 	{

lib/gameState/InfoAboutArmy.h

@@ -55,7 +55,7 @@ public:
 	Details * details = nullptr;
 
 	const CHeroClass *hclass;
-	int portrait;
+	HeroTypeID portraitSource;
 
 	enum EInfoLevel
 	{
@@ -72,6 +72,7 @@ public:
 	InfoAboutHero & operator=(const InfoAboutHero & iah);
 
 	void initFromHero(const CGHeroInstance *h, EInfoLevel infoLevel);
+	int32_t getIconIndex() const;
 };
 
 /// Struct which holds a int information about a town

lib/mapObjects/CGHeroInstance.cpp

@@ -246,7 +246,6 @@ CGHeroInstance::CGHeroInstance():
 	moveDir(4),
 	mana(UNINITIALIZED_MANA),
 	movement(UNINITIALIZED_MOVEMENT),
-	portrait(UNINITIALIZED_PORTRAIT),
 	level(1),
 	exp(UNINITIALIZED_EXPERIENCE),
 	gender(EHeroGender::DEFAULT),
@@ -273,7 +272,7 @@ void CGHeroInstance::setType(si32 ID, si32 subID)
 {
 	assert(ID == Obj::HERO); // just in case
 	type = VLC->heroh->objects[subID];
-	portrait = type->imageIndex;
+
 	CGObjectInstance::setType(ID, type->heroClass->getIndex()); // to find object handler we must use heroClass->id
 	this->subID = subID; // after setType subID used to store unique hero identify id. Check issue 2277 for details
 	randomizeArmy(type->heroClass->faction);
@@ -309,8 +308,6 @@ void CGHeroInstance::initHero(CRandomGenerator & rand)
 	if(!getArt(ArtifactPosition::MACH4))
 		putArtifact(ArtifactPosition::MACH4, ArtifactUtils::createNewArtifactInstance(ArtifactID::CATAPULT)); //everyone has a catapult
 
-	if(portrait < 0 || portrait == 255)
-		portrait = type->imageIndex;
 	if(!hasBonus(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)))
 	{
 		for(int g=0; g<GameConstants::PRIMARY_SKILLS; ++g)
@@ -1028,6 +1025,19 @@ si32 CGHeroInstance::manaLimit() const
 		* (valOfBonuses(BonusType::MANA_PER_KNOWLEDGE)));
 }
 
+HeroTypeID CGHeroInstance::getPortraitSource() const
+{
+	if (customPortraitSource.isValid())
+		return customPortraitSource;
+	else
+		return HeroTypeID(subID);
+}
+
+int32_t CGHeroInstance::getIconIndex() const
+{
+	return VLC->heroTypes()->getById(getPortraitSource())->getIconIndex();
+}
+
 std::string CGHeroInstance::getNameTranslated() const
 {
 	return VLC->generaltexth->translate(getNameTextID());
@@ -1528,40 +1538,7 @@ void CGHeroInstance::serializeCommonOptions(JsonSerializeFormat & handler)
 
 	handler.serializeString("name", nameCustomTextId);
 	handler.serializeInt("gender", gender, 0);
-
-	{
-		const int legacyHeroes = VLC->settings()->getInteger(EGameSettings::TEXTS_HERO);
-		const int moddedStart = legacyHeroes + GameConstants::HERO_PORTRAIT_SHIFT;
-
-		if(handler.saving)
-		{
-			if(portrait >= 0)
-			{
-				if(portrait < legacyHeroes || portrait >= moddedStart)
-				{
-					int tempPortrait = portrait >= moddedStart
-						? portrait - GameConstants::HERO_PORTRAIT_SHIFT
-						: portrait;
-					handler.serializeId<si32, si32, HeroTypeID>("portrait", tempPortrait, -1);
-				}
-				else
-					handler.serializeInt("portrait", portrait, -1);
-			}
-		}
-		else
-		{
-			const JsonNode & portraitNode = handler.getCurrent()["portrait"];
-
-			if(portraitNode.getType() == JsonNode::JsonType::DATA_STRING)
-			{
-				handler.serializeId<si32, si32, HeroTypeID>("portrait", portrait, -1);
-				if(portrait >= legacyHeroes)
-					portrait += GameConstants::HERO_PORTRAIT_SHIFT;
-			}
-			else
-				handler.serializeInt("portrait", portrait, -1);
-		}
-	}
+	handler.serializeId("portrait", customPortraitSource, HeroTypeID::NONE);
 
 	//primary skills
 	if(handler.saving)

lib/mapObjects/CGHeroInstance.h

@@ -69,7 +69,9 @@ public:
 	ConstTransitivePtr<CHero> type;
 	TExpType exp; //experience points
 	ui32 level; //current level of hero
-	si32 portrait; //may be custom
+
+	/// If not NONE - then hero should use portrait from referenced hero type
+	HeroTypeID customPortraitSource;
 	si32 mana; // remaining spell points
 	std::vector<std::pair<SecondarySkill,ui8> > secSkills; //first - ID of skill, second - level of skill (1 - basic, 2 - adv., 3 - expert); if hero has ability (-1, -1) it meansthat it should have default secondary abilities
 	EHeroGender gender;
@@ -82,7 +84,6 @@ public:
 	ConstTransitivePtr<CCommanderInstance> commander;
 	const CGBoat * boat = nullptr; //set to CGBoat when sailing
 
-	static constexpr si32 UNINITIALIZED_PORTRAIT = -1;
 	static constexpr si32 UNINITIALIZED_MANA = -1;
 	static constexpr ui32 UNINITIALIZED_MOVEMENT = -1;
 	static constexpr TExpType UNINITIALIZED_EXPERIENCE = std::numeric_limits<TExpType>::max();
@@ -144,6 +145,9 @@ public:
 	std::string getBiographyTranslated() const;
 	std::string getNameTranslated() const;
 
+	HeroTypeID getPortraitSource() const;
+	int32_t getIconIndex() const;
+
 private:
 	std::string getNameTextID() const;
 	std::string getBiographyTextID() const;
@@ -321,7 +325,7 @@ public:
 		h & level;
 		h & nameCustomTextId;
 		h & biographyCustomTextId;
-		h & portrait;
+		h & customPortraitSource;
 		h & mana;
 		h & secSkills;
 		h & movement;

lib/mapObjects/CQuest.cpp

@@ -44,7 +44,6 @@ CQuest::CQuest():
 	textOption(0),
 	completedOption(0),
 	stackDirection(0),
-	heroPortrait(-1),
 	isCustomFirst(false),
 	isCustomNext(false),
 	isCustomComplete(false)
@@ -555,7 +554,7 @@ void CGSeerHut::setObjToKill()
 	else if(quest->missionType == CQuest::MISSION_KILL_HERO)
 	{
 		quest->heroName = getHeroToKill(false)->getNameTranslated();
-		quest->heroPortrait = getHeroToKill(false)->portrait;
+		quest->heroPortrait = getHeroToKill(false)->getPortraitSource();
 	}
 
 	quest->getCompletionText(configuration.onSelect);

lib/mapObjects/CQuest.h

@@ -69,7 +69,7 @@ public:
 	CStackBasicDescriptor stackToKill;
 	ui8 stackDirection;
 	std::string heroName; //backup of hero name
-	si32 heroPortrait;
+	HeroTypeID heroPortrait;
 
 	MetaString firstVisitText, nextVisitText, completedText;
 	bool isCustomFirst;

lib/mapping/CMapHeader.cpp

@@ -32,29 +32,29 @@ PlayerInfo::PlayerInfo(): canHumanPlay(false), canComputerPlay(false),
 	allowedFactions = VLC->townh->getAllowedFactions();
 }
 
-si8 PlayerInfo::defaultCastle() const
+FactionID PlayerInfo::defaultCastle() const
 {
 	//if random allowed set it as default
 	if(isFactionRandom)
-		return -1;
+		return FactionID::RANDOM;
 
 	if(!allowedFactions.empty())
 		return *allowedFactions.begin();
 
 	// fall back to random
-	return -1;
+	return FactionID::RANDOM;
 }
 
-si8 PlayerInfo::defaultHero() const
+HeroTypeID PlayerInfo::defaultHero() const
 {
 	// we will generate hero in front of main town
 	if((generateHeroAtMainTown && hasMainTown) || hasRandomHero)
 	{
 		//random hero
-		return -1;
+		return HeroTypeID::RANDOM;
 	}
 
-	return -2;
+	return HeroTypeID::NONE;
 }
 
 bool PlayerInfo::canAnyonePlay() const
@@ -64,7 +64,7 @@ bool PlayerInfo::canAnyonePlay() const
 
 bool PlayerInfo::hasCustomMainHero() const
 {
-	return !mainCustomHeroNameTextId.empty() && mainCustomHeroPortrait != -1;
+	return mainCustomHeroId.isValid();
 }
 
 EventCondition::EventCondition(EWinLoseType condition):

lib/mapping/CMapHeader.h

@@ -27,7 +27,7 @@ struct DLL_LINKAGE SHeroName
 {
 	SHeroName();
 
-	int heroId;
+	HeroTypeID heroId;
 	std::string heroName;
 
 	template <typename Handler>
@@ -45,9 +45,9 @@ struct DLL_LINKAGE PlayerInfo
 	PlayerInfo();
 
 	/// Gets the default faction id or -1 for a random faction.
-	si8 defaultCastle() const;
+	FactionID defaultCastle() const;
 	/// Gets the default hero id or -1 for a random hero.
-	si8 defaultHero() const;
+	HeroTypeID defaultHero() const;
 	bool canAnyonePlay() const;
 	bool hasCustomMainHero() const;
 
@@ -63,10 +63,10 @@ struct DLL_LINKAGE PlayerInfo
 	/// Player has a random main hero
 	bool hasRandomHero;
 	/// The default value is -1.
-	si32 mainCustomHeroPortrait;
+	HeroTypeID mainCustomHeroPortrait;
 	std::string mainCustomHeroNameTextId;
 	/// ID of custom hero (only if portrait and hero name are set, otherwise unpredicted value), -1 if none (not always -1)
-	si32 mainCustomHeroId;
+	HeroTypeID mainCustomHeroId;
 
 	std::vector<SHeroName> heroesNames; /// list of placed heroes on the map
 	bool hasMainTown; /// The default value is false.

lib/mapping/MapFeaturesH3M.cpp

@@ -80,7 +80,7 @@ MapFormatFeaturesH3M MapFormatFeaturesH3M::getFeaturesAB()
 	result.creaturesCount = 145; // + Conflux and new neutrals
 
 	result.heroesCount = 156; // + Conflux and campaign heroes
-	result.heroesPortraitsCount = 163;
+	result.heroesPortraitsCount = 159; // +Kendal, +young Cristian, +Ordwald
 	result.heroesBytes = 20;
 
 	result.artifactsCount = 129; // + Armaggedon Blade and Vial of Dragon Blood
@@ -100,6 +100,8 @@ MapFormatFeaturesH3M MapFormatFeaturesH3M::getFeaturesSOD()
 	result.artifactsCount = 141; // + Combined artifacts
 	result.artifactsBytes = 18;
 
+	result.heroesPortraitsCount = 163; // +Finneas +young Gem +young Sandro +young Yog
+
 	result.artifactSlotsCount = 19; // + MISC_5 slot
 
 	return result;

lib/mapping/MapFormatH3M.cpp

@@ -248,9 +248,9 @@ void CMapLoaderH3M::readPlayerInfo()
 		}
 
 		playerInfo.hasRandomHero = reader->readBool();
-		playerInfo.mainCustomHeroId = reader->readHero().getNum();
+		playerInfo.mainCustomHeroId = reader->readHero();
 
-		if(playerInfo.mainCustomHeroId != -1)
+		if(playerInfo.mainCustomHeroId != HeroTypeID::NONE)
 		{
 			playerInfo.mainCustomHeroPortrait = reader->readHeroPortrait();
 			playerInfo.mainCustomHeroNameTextId = readLocalizedString(TextIdentifier("header", "player", i, "mainHeroName"));
@@ -263,7 +263,7 @@ void CMapLoaderH3M::readPlayerInfo()
 			for(int pp = 0; pp < heroCount; ++pp)
 			{
 				SHeroName vv;
-				vv.heroId = reader->readUInt8();
+				vv.heroId = reader->readHero();
 				vv.heroName = readLocalizedString(TextIdentifier("header", "heroNames", vv.heroId));
 
 				playerInfo.heroesNames.push_back(vv);
@@ -547,7 +547,6 @@ void CMapLoaderH3M::readVictoryLossConditions()
 				}
 			);
 
-			assert(playersOnMap > 1);
 			if(playersOnMap == 1)
 			{
 				logGlobal->warn("Map %s: Only one player exists, but normal victory allowed!", mapName);
@@ -703,8 +702,8 @@ void CMapLoaderH3M::readDisposedHeroes()
 		map->disposedHeroes.resize(disp);
 		for(int g = 0; g < disp; ++g)
 		{
-			map->disposedHeroes[g].heroId = reader->readHero().getNum();
-			map->disposedHeroes[g].portrait = reader->readHeroPortrait();
+			map->disposedHeroes[g].heroId = reader->readHero();
+			map->disposedHeroes[g].portrait.setNum(reader->readHeroPortrait());
 			map->disposedHeroes[g].name = readLocalizedString(TextIdentifier("header", "heroes", map->disposedHeroes[g].heroId));
 			reader->readBitmaskPlayers(map->disposedHeroes[g].players, false);
 		}
@@ -1679,14 +1678,13 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 	}
 
 	setOwnerAndValidate(mapPosition, object, owner);
-	object->portrait = CGHeroInstance::UNINITIALIZED_PORTRAIT;
 
 	for(auto & elem : map->disposedHeroes)
 	{
 		if(elem.heroId.getNum() == object->subID)
 		{
 			object->nameCustomTextId = elem.name;
-			object->portrait = elem.portrait;
+			object->customPortraitSource = elem.portrait;
 			break;
 		}
 	}
@@ -1712,7 +1710,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 
 	bool hasPortrait = reader->readBool();
 	if(hasPortrait)
-		object->portrait = reader->readHeroPortrait();
+		object->customPortraitSource = reader->readHeroPortrait();
 
 	bool hasSecSkills = reader->readBool();
 	if(hasSecSkills)

lib/mapping/MapFormatJson.cpp

@@ -562,11 +562,11 @@ void CMapFormatJson::serializePlayerInfo(JsonSerializeFormat & handler)
 				const std::string instanceName = hero.first;
 
 				SHeroName hname;
-				hname.heroId = -1;
+				hname.heroId = HeroTypeID::NONE;
 				std::string rawId = data["type"].String();
 
 				if(!rawId.empty())
-					hname.heroId = HeroTypeID::decode(rawId);
+					hname.heroId = HeroTypeID(HeroTypeID::decode(rawId));
 
 				hname.heroName = data["name"].String();
 
@@ -574,9 +574,9 @@ void CMapFormatJson::serializePlayerInfo(JsonSerializeFormat & handler)
 				{
 					//this is main hero
 					info.mainCustomHeroNameTextId = hname.heroName;
-					info.hasRandomHero = (hname.heroId == -1);
+					info.hasRandomHero = (hname.heroId == HeroTypeID::NONE);
 					info.mainCustomHeroId = hname.heroId;
-					info.mainCustomHeroPortrait = -1;
+					info.mainCustomHeroPortrait = HeroTypeID::NONE;
 					//todo:mainHeroPortrait
 				}
 
@@ -744,7 +744,7 @@ void CMapFormatJson::readDisposedHeroes(JsonSerializeFormat & handler)
 		{
 			DisposedHero hero;
 
-			hero.heroId = type.getNum();
+			hero.heroId = type;
 			hero.players = mask;
 			//name and portrait are not used
 

lib/mapping/MapIdentifiersH3M.cpp

@@ -87,15 +87,7 @@ void MapIdentifiersH3M::loadMapping(const JsonNode & mapping)
 		}
 	}
 
-	for (auto entry : mapping["portraits"].Struct())
-	{
-		int32_t sourceID = entry.second.Integer();
-		int32_t targetID = *VLC->identifiers()->getIdentifier(entry.second.meta, "hero", entry.first);
-		int32_t iconID = VLC->heroTypes()->getByIndex(targetID)->getIconIndex();
-
-		mappingHeroPortrait[sourceID] = iconID;
-	}
-
+	loadMapping(mappingHeroPortrait, mapping["portraits"], "hero");
 	loadMapping(mappingBuilding, mapping["buildingsCommon"], "building.core:random");
 	loadMapping(mappingFaction, mapping["factions"], "faction");
 	loadMapping(mappingCreature, mapping["creatures"], "creature");
@@ -168,7 +160,7 @@ HeroTypeID MapIdentifiersH3M::remap(HeroTypeID input) const
 	return input;
 }
 
-int32_t MapIdentifiersH3M::remapPortrrait(int32_t input) const
+HeroTypeID MapIdentifiersH3M::remapPortrait(HeroTypeID input) const
 {
 	if (mappingHeroPortrait.count(input))
 		return mappingHeroPortrait.at(input);

lib/mapping/MapIdentifiersH3M.h

@@ -38,7 +38,7 @@ class MapIdentifiersH3M
 	std::map<FactionID, FactionID> mappingFaction;
 	std::map<CreatureID, CreatureID> mappingCreature;
 	std::map<HeroTypeID, HeroTypeID> mappingHeroType;
-	std::map<int32_t, int32_t> mappingHeroPortrait;
+	std::map<HeroTypeID, HeroTypeID> mappingHeroPortrait;
 	std::map<HeroClassID, HeroClassID> mappingHeroClass;
 	std::map<TerrainId, TerrainId> mappingTerrain;
 	std::map<ArtifactID, ArtifactID> mappingArtifact;
@@ -55,7 +55,7 @@ public:
 	void remapTemplate(ObjectTemplate & objectTemplate);
 
 	BuildingID remapBuilding(std::optional<FactionID> owner, BuildingID input) const;
-	int32_t remapPortrrait(int32_t input) const;
+	HeroTypeID remapPortrait(HeroTypeID input) const;
 	FactionID remap(FactionID input) const;
 	CreatureID remap(CreatureID input) const;
 	HeroTypeID remap(HeroTypeID input) const;

lib/mapping/MapReaderH3M.cpp

@@ -106,20 +106,20 @@ HeroTypeID MapReaderH3M::readHero()
 	return remapIdentifier(result);
 }
 
-int32_t MapReaderH3M::readHeroPortrait()
+HeroTypeID MapReaderH3M::readHeroPortrait()
 {
 	HeroTypeID result(reader->readUInt8());
 
 	if(result.getNum() == features.heroIdentifierInvalid)
-		return int32_t(-1);
+		return HeroTypeID::NONE;
 
 	if (result.getNum() >= features.heroesPortraitsCount)
 	{
 		logGlobal->warn("Map contains invalid hero portrait ID %d. Will be reset!", result.getNum() );
-		return int32_t(-1);
+		return HeroTypeID::NONE;
 	}
 
-	return remapper.remapPortrrait(result);
+	return remapper.remapPortrait(result);
 }
 
 CreatureID MapReaderH3M::readCreature()

lib/mapping/MapReaderH3M.h

@@ -35,7 +35,7 @@ public:
 	ArtifactID readArtifact32();
 	CreatureID readCreature();
 	HeroTypeID readHero();
-	int32_t readHeroPortrait();
+	HeroTypeID readHeroPortrait();
 	TerrainId readTerrain();
 	RoadId readRoad();
 	RiverId readRiver();

mapeditor/inspector/inspector.cpp

@@ -158,7 +158,6 @@ void Initializer::initialize(CGHeroInstance * o)
 		//	o->type = VLC->heroh->objects.at(o->subID);
 		
 		o->gender = o->type->gender;
-		o->portrait = o->type->imageIndex;
 		o->randomizeArmy(o->type->heroClass->faction);
 	}
 }
@@ -279,7 +278,7 @@ void Inspector::updateProperties(CGHeroInstance * o)
 	}
 	addProperty("Name", o->getNameTranslated(), false);
 	addProperty("Biography", o->getBiographyTranslated(), new MessageDelegate, false);
-	addProperty("Portrait", o->portrait, false);
+	addProperty("Portrait", o->customPortraitSource, false);
 	
 	auto * delegate = new HeroSkillsDelegate(*o);
 	addProperty("Skills", PropertyEditorPlaceholder(), delegate, false);
@@ -622,7 +621,6 @@ void Inspector::setProperty(CGHeroInstance * o, const QString & key, const QVari
 				o->type = t.get();
 		}
 		o->gender = o->type->gender;
-		o->portrait = o->type->imageIndex;
 		o->randomizeArmy(o->type->heroClass->faction);
 		updateProperties(); //updating other properties after change
 	}

mapeditor/mapcontroller.cpp

@@ -184,9 +184,6 @@ void MapController::repairMap(CMap * map) const
 					nih->putArtifact(ArtifactPosition::SPELLBOOK, ArtifactUtils::createNewArtifactInstance(ArtifactID::SPELLBOOK));
 			}
 			
-			//fix portrait
-			if(nih->portrait < 0 || nih->portrait == 255)
-				nih->portrait = type->imageIndex;
 		}
 		//fix town instance
 		if(auto * tnh = dynamic_cast<CGTownInstance*>(obj.get()))

mapeditor/playerparams.cpp

@@ -172,7 +172,7 @@ void PlayerParams::on_mainTown_currentIndexChanged(int index)
 
 void PlayerParams::on_teamId_activated(int index)
 {
-	playerInfo.team = ui->teamId->currentData().toInt();
+	playerInfo.team.setNum(ui->teamId->currentData().toInt());
 }
 
 

server/CVCMIServer.cpp

@@ -729,7 +729,7 @@ void CVCMIServer::updateStartInfoOnMapChange(std::shared_ptr<CMapInfo> mapInfo,
 			pset.castle = pinfo.defaultCastle();
 			pset.hero = pinfo.defaultHero();
 
-			if(pset.hero.getNum() != PlayerSettings::RANDOM && pinfo.hasCustomMainHero())
+			if(pset.hero != HeroTypeID::RANDOM && pinfo.hasCustomMainHero())
 			{
 				pset.hero = pinfo.mainCustomHeroId;
 				pset.heroNameTextId = pinfo.mainCustomHeroNameTextId;
@@ -839,13 +839,13 @@ void CVCMIServer::optionNextCastle(PlayerColor player, int dir)
 {
 	PlayerSettings & s = si->playerInfos[player];
 	FactionID & cur = s.castle;
-	auto & allowed = getPlayerInfo(player.getNum()).allowedFactions;
-	const bool allowRandomTown = getPlayerInfo(player.getNum()).isFactionRandom;
+	auto & allowed = getPlayerInfo(player).allowedFactions;
+	const bool allowRandomTown = getPlayerInfo(player).isFactionRandom;
 
-	if(cur.getNum() == PlayerSettings::NONE) //no change
+	if(cur == FactionID::NONE) //no change
 		return;
 
-	if(cur.getNum() == PlayerSettings::RANDOM) //first/last available
+	if(cur == FactionID::RANDOM) //first/last available
 	{
 		if(dir > 0)
 			cur = *allowed.begin(); //id of first town
@@ -859,7 +859,7 @@ void CVCMIServer::optionNextCastle(PlayerColor player, int dir)
 		{
 			if(allowRandomTown)
 			{
-				cur = PlayerSettings::RANDOM;
+				cur = FactionID::RANDOM;
 			}
 			else
 			{
@@ -878,34 +878,34 @@ void CVCMIServer::optionNextCastle(PlayerColor player, int dir)
 		}
 	}
 
-	if(s.hero.getNum() >= 0 && !getPlayerInfo(player.getNum()).hasCustomMainHero()) // remove hero unless it set to fixed one in map editor
+	if(s.hero.isValid() && !getPlayerInfo(player).hasCustomMainHero()) // remove hero unless it set to fixed one in map editor
 	{
-		s.hero = PlayerSettings::RANDOM;
+		s.hero = HeroTypeID::RANDOM;
 	}
-	if(cur.getNum() < 0 && s.bonus == PlayerSettings::RESOURCE)
-		s.bonus = PlayerSettings::RANDOM;
+	if(!cur.isValid() && s.bonus == PlayerStartingBonus::RESOURCE)
+		s.bonus = PlayerStartingBonus::RANDOM;
 }
 
-void CVCMIServer::optionSetCastle(PlayerColor player, int id)
+void CVCMIServer::optionSetCastle(PlayerColor player, FactionID id)
 {
 	PlayerSettings & s = si->playerInfos[player];
 	FactionID & cur = s.castle;
-	auto & allowed = getPlayerInfo(player.getNum()).allowedFactions;
+	auto & allowed = getPlayerInfo(player).allowedFactions;
 
-	if(cur.getNum() == PlayerSettings::NONE) //no change
+	if(cur == FactionID::NONE) //no change
 		return;
 
-	if(allowed.find(static_cast<FactionID>(id)) == allowed.end() && id != PlayerSettings::RANDOM) // valid id
+	if(allowed.find(id) == allowed.end() && id != FactionID::RANDOM) // valid id
 		return;
 
 	cur = static_cast<FactionID>(id);
 
-	if(s.hero.getNum() >= 0 && !getPlayerInfo(player.getNum()).hasCustomMainHero()) // remove hero unless it set to fixed one in map editor
+	if(s.hero.isValid() && !getPlayerInfo(player).hasCustomMainHero()) // remove hero unless it set to fixed one in map editor
 	{
-		s.hero = PlayerSettings::RANDOM;
+		s.hero = HeroTypeID::RANDOM;
 	}
-	if(cur.getNum() < 0 && s.bonus == PlayerSettings::RESOURCE)
-		s.bonus = PlayerSettings::RANDOM;
+	if(!cur.isValid() && s.bonus == PlayerStartingBonus::RESOURCE)
+		s.bonus = PlayerStartingBonus::RANDOM;
 }
 
 void CVCMIServer::setCampaignMap(CampaignScenarioID mapId)
@@ -937,105 +937,105 @@ void CVCMIServer::setCampaignBonus(int bonusId)
 void CVCMIServer::optionNextHero(PlayerColor player, int dir)
 {
 	PlayerSettings & s = si->playerInfos[player];
-	if(s.castle.getNum() < 0 || s.hero.getNum() == PlayerSettings::NONE)
+	if(!s.castle.isValid() || s.hero == HeroTypeID::NONE)
 		return;
 
-	if(s.hero.getNum() == PlayerSettings::RANDOM) // first/last available
+	if(s.hero == HeroTypeID::RANDOM) // first/last available
 	{
-		int max = static_cast<int>(VLC->heroh->size()),
-			min = 0;
-		s.hero = nextAllowedHero(player, min, max, 0, dir);
+		if (dir > 0)
+			s.hero = nextAllowedHero(player, HeroTypeID(-1), dir);
+		else
+			s.hero = nextAllowedHero(player, HeroTypeID(VLC->heroh->size()), dir);
 	}
 	else
 	{
-		if(dir > 0)
-			s.hero = nextAllowedHero(player, s.hero, (int)VLC->heroh->size(), 1, dir);
-		else
-			s.hero = nextAllowedHero(player, -1, s.hero, 1, dir); // min needs to be -1 -- hero at index 0 would be skipped otherwise
+		s.hero = nextAllowedHero(player, s.hero, dir);
 	}
 }
 
-void CVCMIServer::optionSetHero(PlayerColor player, int id)
+void CVCMIServer::optionSetHero(PlayerColor player, HeroTypeID id)
 {
 	PlayerSettings & s = si->playerInfos[player];
-	if(s.castle.getNum() < 0 || s.hero.getNum() == PlayerSettings::NONE)
+	if(!s.castle.isValid() || s.hero == HeroTypeID::NONE)
 		return;
 
-	if(id == PlayerSettings::RANDOM)
+	if(id == HeroTypeID::RANDOM)
 	{
-		s.hero = PlayerSettings::RANDOM;
+		s.hero = HeroTypeID::RANDOM;
 	}
 	if(canUseThisHero(player, id))
 		s.hero = static_cast<HeroTypeID>(id);
 }
 
-HeroTypeID CVCMIServer::nextAllowedHero(PlayerColor player, int min, int max, int incl, int dir)
+HeroTypeID CVCMIServer::nextAllowedHero(PlayerColor player, HeroTypeID initial, int direction)
 {
-	if(dir > 0)
+	HeroTypeID first(initial.getNum() + direction);
+
+	if(direction > 0)
 	{
-		for(int i = min + incl; i <= max - incl; i++)
+		for (auto i = first; i.getNum() < VLC->heroh->size(); ++i)
 			if(canUseThisHero(player, i))
-				return HeroTypeID(i);
+				return i;
 	}
 	else
 	{
-		for(int i = max - incl; i >= min + incl; i--)
+		for (auto i = first; i.getNum() >= 0; --i)
 			if(canUseThisHero(player, i))
-				return HeroTypeID(i);
+				return i;
 	}
-	return -1;
+	return HeroTypeID::RANDOM;
 }
 
 void CVCMIServer::optionNextBonus(PlayerColor player, int dir)
 {
 	PlayerSettings & s = si->playerInfos[player];
-	PlayerSettings::Ebonus & ret = s.bonus = static_cast<PlayerSettings::Ebonus>(static_cast<int>(s.bonus) + dir);
+	PlayerStartingBonus & ret = s.bonus = static_cast<PlayerStartingBonus>(static_cast<int>(s.bonus) + dir);
 
-	if(s.hero.getNum() == PlayerSettings::NONE &&
-		!getPlayerInfo(player.getNum()).heroesNames.size() &&
-		ret == PlayerSettings::ARTIFACT) //no hero - can't be artifact
+	if(s.hero == HeroTypeID::NONE &&
+		!getPlayerInfo(player).heroesNames.size() &&
+		ret == PlayerStartingBonus::ARTIFACT) //no hero - can't be artifact
 	{
 		if(dir < 0)
-			ret = PlayerSettings::RANDOM;
+			ret = PlayerStartingBonus::RANDOM;
 		else
-			ret = PlayerSettings::GOLD;
+			ret = PlayerStartingBonus::GOLD;
 	}
 
-	if(ret > PlayerSettings::RESOURCE)
-		ret = PlayerSettings::RANDOM;
-	if(ret < PlayerSettings::RANDOM)
-		ret = PlayerSettings::RESOURCE;
+	if(ret > PlayerStartingBonus::RESOURCE)
+		ret = PlayerStartingBonus::RANDOM;
+	if(ret < PlayerStartingBonus::RANDOM)
+		ret = PlayerStartingBonus::RESOURCE;
 
-	if(s.castle.getNum() == PlayerSettings::RANDOM && ret == PlayerSettings::RESOURCE) //random castle - can't be resource
+	if(s.castle == FactionID::RANDOM && ret == PlayerStartingBonus::RESOURCE) //random castle - can't be resource
 	{
 		if(dir < 0)
-			ret = PlayerSettings::GOLD;
+			ret = PlayerStartingBonus::GOLD;
 		else
-			ret = PlayerSettings::RANDOM;
+			ret = PlayerStartingBonus::RANDOM;
 	}
 }
 
-void CVCMIServer::optionSetBonus(PlayerColor player, int id)
+void CVCMIServer::optionSetBonus(PlayerColor player, PlayerStartingBonus id)
 {
 	PlayerSettings & s = si->playerInfos[player];
 
-	if(s.hero.getNum() == PlayerSettings::NONE &&
-		!getPlayerInfo(player.getNum()).heroesNames.size() &&
-		id == PlayerSettings::ARTIFACT) //no hero - can't be artifact
+	if(s.hero == HeroTypeID::NONE &&
+		!getPlayerInfo(player).heroesNames.size() &&
+		id == PlayerStartingBonus::ARTIFACT) //no hero - can't be artifact
 			return;
 
-	if(id > PlayerSettings::RESOURCE)
+	if(id > PlayerStartingBonus::RESOURCE)
 		return;
-	if(id < PlayerSettings::RANDOM)
+	if(id < PlayerStartingBonus::RANDOM)
 		return;
 
-	if(s.castle.getNum() == PlayerSettings::RANDOM && id == PlayerSettings::RESOURCE) //random castle - can't be resource
+	if(s.castle == FactionID::RANDOM && id == PlayerStartingBonus::RESOURCE) //random castle - can't be resource
 		return;
 
-	s.bonus = static_cast<PlayerSettings::Ebonus>(static_cast<int>(id));
+	s.bonus = id;;
 }
 
-bool CVCMIServer::canUseThisHero(PlayerColor player, int ID)
+bool CVCMIServer::canUseThisHero(PlayerColor player, HeroTypeID ID)
 {
 	return VLC->heroh->size() > ID
 		&& si->playerInfos[player].castle == VLC->heroh->objects[ID]->heroClass->faction
@@ -1043,17 +1043,17 @@ bool CVCMIServer::canUseThisHero(PlayerColor player, int ID)
 		&& mi->mapHeader->allowedHeroes[ID];
 }
 
-std::vector<int> CVCMIServer::getUsedHeroes()
+std::vector<HeroTypeID> CVCMIServer::getUsedHeroes()
 {
-	std::vector<int> heroIds;
+	std::vector<HeroTypeID> heroIds;
 	for(auto & p : si->playerInfos)
 	{
-		const auto & heroes = getPlayerInfo(p.first.getNum()).heroesNames;
+		const auto & heroes = getPlayerInfo(p.first).heroesNames;
 		for(auto & hero : heroes)
 			if(hero.heroId >= 0) //in VCMI map format heroId = -1 means random hero
 				heroIds.push_back(hero.heroId);
 
-		if(p.second.hero.getNum() != PlayerSettings::RANDOM)
+		if(p.second.hero != HeroTypeID::RANDOM)
 			heroIds.push_back(p.second.hero);
 	}
 	return heroIds;

server/CVCMIServer.h

@@ -108,14 +108,14 @@ public:
 	// Work with LobbyInfo
 	void setPlayer(PlayerColor clickedColor);
 	void optionNextHero(PlayerColor player, int dir); //dir == -1 or +1
-	void optionSetHero(PlayerColor player, int id);
-	HeroTypeID nextAllowedHero(PlayerColor player, int min, int max, int incl, int dir);
-	bool canUseThisHero(PlayerColor player, int ID);
-	std::vector<int> getUsedHeroes();
+	void optionSetHero(PlayerColor player, HeroTypeID id);
+	HeroTypeID nextAllowedHero(PlayerColor player, HeroTypeID id, int direction);
+	bool canUseThisHero(PlayerColor player, HeroTypeID ID);
+	std::vector<HeroTypeID> getUsedHeroes();
 	void optionNextBonus(PlayerColor player, int dir); //dir == -1 or +1
-	void optionSetBonus(PlayerColor player, int id);
+	void optionSetBonus(PlayerColor player, PlayerStartingBonus id);
 	void optionNextCastle(PlayerColor player, int dir); //dir == -1 or +
-	void optionSetCastle(PlayerColor player, int id);
+	void optionSetCastle(PlayerColor player, FactionID id);
 
 	// Campaigns
 	void setCampaignMap(CampaignScenarioID mapId);

server/NetPacksLobbyServer.cpp

@@ -369,19 +369,19 @@ void ApplyOnServerNetPackVisitor::visitLobbyChangePlayerOption(LobbyChangePlayer
 	switch(pack.what)
 	{
 	case LobbyChangePlayerOption::TOWN_ID:
-		srv.optionSetCastle(pack.color, pack.value);
+		srv.optionSetCastle(pack.color, FactionID(pack.value));
 		break;
 	case LobbyChangePlayerOption::TOWN:
 		srv.optionNextCastle(pack.color, pack.value);
 		break;
 	case LobbyChangePlayerOption::HERO_ID:
-		srv.optionSetHero(pack.color, pack.value);
+		srv.optionSetHero(pack.color, HeroTypeID(pack.value));
 		break;
 	case LobbyChangePlayerOption::HERO:
 		srv.optionNextHero(pack.color, pack.value);
 		break;
 	case LobbyChangePlayerOption::BONUS_ID:
-		srv.optionSetBonus(pack.color, pack.value);
+		srv.optionSetBonus(pack.color, PlayerStartingBonus(pack.value));
 		break;
 	case LobbyChangePlayerOption::BONUS:
 		srv.optionNextBonus(pack.color, pack.value);

server/battles/BattleResultProcessor.cpp

@@ -277,7 +277,7 @@ void BattleResultProcessor::endBattle(const CBattleInfoCallback & battle)
 		gameHandler->queries->addQuery(battleDialogQuery);
 	}
 	else
-		battleResult->queryID = -1;
+		battleResult->queryID = QueryID::NONE;
 
 	//set same battle result for all gameHandler->queries
 	for(auto q : gameHandler->queries->allQueries())

server/processors/HeroPoolProcessor.cpp

@@ -72,7 +72,7 @@ void HeroPoolProcessor::onHeroSurrendered(const PlayerColor & color, const CGHer
 
 	sah.slotID = selectSlotForRole(color, sah.roleID);
 	sah.player = color;
-	sah.hid = hero->subID;
+	sah.hid.setNum(hero->subID);
 	gameHandler->sendAndApply(&sah);
 }
 
@@ -83,7 +83,7 @@ void HeroPoolProcessor::onHeroEscaped(const PlayerColor & color, const CGHeroIns
 
 	sah.slotID = selectSlotForRole(color, sah.roleID);
 	sah.player = color;
-	sah.hid = hero->subID;
+	sah.hid.setNum(hero->subID);
 	sah.army.clearSlots();
 	sah.army.setCreature(SlotID(0), hero->type->initialArmy.at(0).creature, 1);
 
@@ -110,7 +110,7 @@ void HeroPoolProcessor::selectNewHeroForSlot(const PlayerColor & color, TavernHe
 
 	if (newHero)
 	{
-		sah.hid = newHero->subID;
+		sah.hid.setNum(newHero->subID);
 
 		if (giveArmy)
 		{
@@ -126,7 +126,7 @@ void HeroPoolProcessor::selectNewHeroForSlot(const PlayerColor & color, TavernHe
 	}
 	else
 	{
-		sah.hid = -1;
+		sah.hid = HeroTypeID::NONE;
 	}
 	gameHandler->sendAndApply(&sah);
 }
@@ -205,7 +205,7 @@ bool HeroPoolProcessor::hireHero(const ObjectInstanceID & objectID, const HeroTy
 
 	HeroRecruited hr;
 	hr.tid = mapObject->id;
-	hr.hid = recruitedHero->subID;
+	hr.hid.setNum(recruitedHero->subID);
 	hr.player = player;
 	hr.tile = recruitedHero->convertFromVisitablePos(targetPos );
 	if(gameHandler->getTile(targetPos)->isWater() && !recruitedHero->boat)

test/game/CGameStateTest.cpp

@@ -167,11 +167,11 @@ public:
 			pset.castle = pinfo.defaultCastle();
 			pset.hero = pinfo.defaultHero();
 
-			if(pset.hero.getNum() != PlayerSettings::RANDOM && pinfo.hasCustomMainHero())
+			if(pset.hero != HeroTypeID::RANDOM && pinfo.hasCustomMainHero())
 			{
 				pset.hero = pinfo.mainCustomHeroId;
 				pset.heroNameTextId = pinfo.mainCustomHeroNameTextId;
-				pset.heroPortrait = pinfo.mainCustomHeroPortrait;
+				pset.heroPortrait = HeroTypeID(pinfo.mainCustomHeroPortrait);
 			}
 
 			pset.handicap = PlayerSettings::NO_HANDICAP;