Rework campaign bonuses storage in type-safe form

Replaced campaign bonuses from using 3 integers to store anything with
type-safe version that uses std::variant that ensures that all bonuses
are in correct state.

Also removed "interesting" solutions like storing primary skills using
bit shifts.

Prerequirement for HotA campaign support

client/lobby/CBonusSelection.cpp

@@ -173,149 +173,174 @@ void CBonusSelection::createBonusesIcons()
 
 	for(int i = 0; i < bonDescs.size(); i++)
 	{
-		int bonusType = static_cast<size_t>(bonDescs[i].type);
-		std::string picName = bonusPics[bonusType];
-		size_t picNumber = bonDescs[i].info2;
+		const CampaignBonus & bonus	= bonDescs[i];
+		CampaignBonusType bonusType = bonus.getType();
+		std::string picName = bonusPics[static_cast<int>(bonusType)];
+		size_t picNumber = -1;
 
 		MetaString desc;
-		switch(bonDescs[i].type)
+		switch(bonusType)
 		{
-		case CampaignBonusType::SPELL:
-			desc.appendLocalString(EMetaText::GENERAL_TXT, 715);
-			desc.replaceName(SpellID(bonDescs[i].info2));
-			break;
-		case CampaignBonusType::MONSTER:
-			picNumber = bonDescs[i].info2 + 2;
-			desc.appendLocalString(EMetaText::GENERAL_TXT, 717);
-			desc.replaceNumber(bonDescs[i].info3);
-			desc.replaceNamePlural(bonDescs[i].info2);
-			break;
-		case CampaignBonusType::BUILDING:
-		{
-			FactionID faction;
-			for(auto & elem : GAME->server().si->playerInfos)
+			case CampaignBonusType::SPELL:
+			{
+				const auto & bonusValue = bonus.getValue<CampaignBonusSpell>();
+				picNumber = bonusValue.spell.getNum();
+				desc.appendLocalString(EMetaText::GENERAL_TXT, 715);
+				desc.replaceName(bonusValue.spell);
+				break;
+			}
+			case CampaignBonusType::MONSTER:
+			{
+				const auto & bonusValue = bonus.getValue<CampaignBonusCreatures>();
+				picNumber = bonusValue.creature.getNum() + 2;
+				desc.appendLocalString(EMetaText::GENERAL_TXT, 717);
+				desc.replaceNumber(bonusValue.amount);
+				desc.replaceNamePlural(bonusValue.creature);
+				break;
+			}
+			case CampaignBonusType::BUILDING:
 			{
-				if(elem.second.isControlledByHuman())
+				const auto & bonusValue = bonus.getValue<CampaignBonusBuilding>();
+				FactionID faction;
+				for(auto & elem : GAME->server().si->playerInfos)
 				{
-					faction = elem.second.castle;
-					break;
+					if(elem.second.isControlledByHuman())
+					{
+						faction = elem.second.castle;
+						break;
+					}
 				}
+				assert(faction.hasValue());
+
+				BuildingID buildID;
+				if(getCampaign()->formatVCMI())
+					buildID = bonusValue.building;
+				else
+					buildID = CBuildingHandler::campToERMU(bonusValue.building, faction, std::set<BuildingID>());
+				picName = graphics->ERMUtoPicture[faction.getNum()][buildID.getNum()];
+				picNumber = -1;
 
+				if(vstd::contains((*LIBRARY->townh)[faction]->town->buildings, buildID))
+					desc.appendTextID((*LIBRARY->townh)[faction]->town->buildings.find(buildID)->second->getNameTextID());
+				break;
 			}
-			assert(faction.hasValue());
-
-			BuildingID buildID;
-			if(getCampaign()->formatVCMI())
-				buildID = BuildingID(bonDescs[i].info1);
-			else
-				buildID = CBuildingHandler::campToERMU(bonDescs[i].info1, faction, std::set<BuildingID>());
-			picName = graphics->ERMUtoPicture[faction.getNum()][buildID.getNum()];
-			picNumber = -1;
-
-			if(vstd::contains((*LIBRARY->townh)[faction]->town->buildings, buildID))
-				desc.appendTextID((*LIBRARY->townh)[faction]->town->buildings.find(buildID)->second->getNameTextID());
-			break;
-		}
-		case CampaignBonusType::ARTIFACT:
-			desc.appendLocalString(EMetaText::GENERAL_TXT, 715);
-			desc.replaceName(ArtifactID(bonDescs[i].info2));
-			break;
-		case CampaignBonusType::SPELL_SCROLL:
-			desc.appendLocalString(EMetaText::GENERAL_TXT, 716);
-			desc.replaceName(SpellID(bonDescs[i].info2));
-			break;
-		case CampaignBonusType::PRIMARY_SKILL:
-		{
-			int leadingSkill = -1;
-			std::vector<std::pair<int, int>> toPrint; //primary skills to be listed <num, val>
-			const ui8 * ptr = reinterpret_cast<const ui8 *>(&bonDescs[i].info2);
-			for(int g = 0; g < GameConstants::PRIMARY_SKILLS; ++g)
+			case CampaignBonusType::ARTIFACT:
+			{
+				const auto & bonusValue = bonus.getValue<CampaignBonusArtifact>();
+				picNumber = bonusValue.artifact;
+				desc.appendLocalString(EMetaText::GENERAL_TXT, 715);
+				desc.replaceName(bonusValue.artifact);
+				break;
+			}
+			case CampaignBonusType::SPELL_SCROLL:
+			{
+				const auto & bonusValue = bonus.getValue<CampaignBonusSpellScroll>();
+				picNumber = bonusValue.spell;
+				desc.appendLocalString(EMetaText::GENERAL_TXT, 716);
+				desc.replaceName(bonusValue.spell);
+				break;
+			}
+			case CampaignBonusType::PRIMARY_SKILL:
 			{
-				if(leadingSkill == -1 || ptr[g] > ptr[leadingSkill])
+				const auto & bonusValue = bonus.getValue<CampaignBonusPrimarySkill>();
+				int leadingSkill = -1;
+				std::vector<std::pair<int, int>> toPrint; //primary skills to be listed <num, val>
+				for(int g = 0; g < bonusValue.amounts.size(); ++g)
 				{
-					leadingSkill = g;
+					if(leadingSkill == -1 || bonusValue.amounts[g] > bonusValue.amounts[leadingSkill])
+					{
+						leadingSkill = g;
+					}
+					if(bonusValue.amounts[g] != 0)
+					{
+						toPrint.push_back(std::make_pair(g, bonusValue.amounts[g]));
+					}
 				}
-				if(ptr[g] != 0)
+				picNumber = leadingSkill;
+				desc.appendLocalString(EMetaText::GENERAL_TXT, 715);
+
+				std::string substitute; //text to be printed instead of %s
+				for(int v = 0; v < toPrint.size(); ++v)
 				{
-					toPrint.push_back(std::make_pair(g, ptr[g]));
+					substitute += std::to_string(toPrint[v].second);
+					substitute += " " + LIBRARY->generaltexth->primarySkillNames[toPrint[v].first];
+					if(v != toPrint.size() - 1)
+					{
+						substitute += ", ";
+					}
 				}
+
+				desc.replaceRawString(substitute);
+				break;
 			}
-			picNumber = leadingSkill;
-			desc.appendLocalString(EMetaText::GENERAL_TXT, 715);
+			case CampaignBonusType::SECONDARY_SKILL:
+			{
+				const auto & bonusValue = bonus.getValue<CampaignBonusSecondarySkill>();
+				desc.appendLocalString(EMetaText::GENERAL_TXT, 718);
+				desc.replaceTextID(TextIdentifier("core", "skilllev", bonusValue.mastery - 1).get());
+				desc.replaceName(bonusValue.skill);
+				picNumber = bonusValue.skill.getNum() * 3 + bonusValue.mastery - 1;
 
-			std::string substitute; //text to be printed instead of %s
-			for(int v = 0; v < toPrint.size(); ++v)
+				break;
+			}
+			case CampaignBonusType::RESOURCE:
 			{
-				substitute += std::to_string(toPrint[v].second);
-				substitute += " " + LIBRARY->generaltexth->primarySkillNames[toPrint[v].first];
-				if(v != toPrint.size() - 1)
+				const auto & bonusValue = bonus.getValue<CampaignBonusStartingResources>();
+				desc.appendLocalString(EMetaText::GENERAL_TXT, 717);
+
+				switch(bonusValue.resource)
 				{
-					substitute += ", ";
+					case EGameResID::COMMON: //wood + ore
+					{
+						desc.replaceLocalString(EMetaText::GENERAL_TXT, 721);
+						picNumber = 7;
+						break;
+					}
+					case EGameResID::RARE: //mercury + sulfur + crystal + gems
+					{
+						desc.replaceLocalString(EMetaText::GENERAL_TXT, 722);
+						picNumber = 8;
+						break;
+					}
+					default:
+					{
+						desc.replaceName(bonusValue.resource);
+						picNumber = bonusValue.resource.getNum();
+					}
 				}
-			}
 
-			desc.replaceRawString(substitute);
-			break;
-		}
-		case CampaignBonusType::SECONDARY_SKILL:
-			desc.appendLocalString(EMetaText::GENERAL_TXT, 718);
-			desc.replaceTextID(TextIdentifier("core", "skilllev", bonDescs[i].info3 - 1).get());
-			desc.replaceName(SecondarySkill(bonDescs[i].info2));
-			picNumber = bonDescs[i].info2 * 3 + bonDescs[i].info3 - 1;
-
-			break;
-		case CampaignBonusType::RESOURCE:
-		{
-			desc.appendLocalString(EMetaText::GENERAL_TXT, 717);
+				desc.replaceNumber(bonusValue.amount);
+				break;
+			}
+			case CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO:
+			{
+				const auto & bonusValue = bonus.getValue<CampaignBonusHeroesFromScenario>();
+				auto superhero = getCampaign()->strongestHero(bonusValue.scenario, bonusValue.startingPlayer);
+				if(!superhero)
+					logGlobal->warn("No superhero! How could it be transferred?");
+				picNumber = superhero ? superhero->getIconIndex() : 0;
+				desc.appendLocalString(EMetaText::GENERAL_TXT, 719);
+				desc.replaceRawString(getCampaign()->scenario(bonusValue.scenario).scenarioName.toString());
+				break;
+			}
 
-			switch(bonDescs[i].info1)
+			case CampaignBonusType::HERO:
 			{
-				case EGameResID::COMMON: //wood + ore
+				const auto & bonusValue = bonus.getValue<CampaignBonusStartingHero>();
+				if(bonusValue.hero == HeroTypeID::CAMP_RANDOM.getNum())
 				{
-					desc.replaceLocalString(EMetaText::GENERAL_TXT, 721);
-					picNumber = 7;
-					break;
+					desc.appendLocalString(EMetaText::GENERAL_TXT, 720); // Start with random hero
+					picNumber = -1;
+					picName = "CBONN1A3.BMP";
 				}
-				case EGameResID::RARE : //mercury + sulfur + crystal + gems
+				else
 				{
-					desc.replaceLocalString(EMetaText::GENERAL_TXT, 722);
-					picNumber = 8;
-					break;
+					desc.appendLocalString(EMetaText::GENERAL_TXT, 715); // Start with %s
+					desc.replaceTextID(bonusValue.hero.toHeroType()->getNameTextID());
+					picNumber = bonusValue.hero.getNum();
 				}
-				default:
-				{
-					desc.replaceName(GameResID(bonDescs[i].info1));
-					picNumber = bonDescs[i].info1;
-				}
-			}
-
-			desc.replaceNumber(bonDescs[i].info2);
-			break;
-		}
-		case CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO:
-		{
-			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->getIconIndex() : 0;
-			desc.appendLocalString(EMetaText::GENERAL_TXT, 719);
-			desc.replaceRawString(getCampaign()->scenario(static_cast<CampaignScenarioID>(bonDescs[i].info2)).scenarioName.toString());
-			break;
-		}
-
-		case CampaignBonusType::HERO:
-			if(bonDescs[i].info2 == HeroTypeID::CAMP_RANDOM.getNum())
-			{
-				desc.appendLocalString(EMetaText::GENERAL_TXT, 720); // Start with random hero
-				picNumber = -1;
-				picName = "CBONN1A3.BMP";
-			}
-			else
-			{
-				desc.appendLocalString(EMetaText::GENERAL_TXT, 715); // Start with %s
-				desc.replaceTextID(LIBRARY->heroh->objects[bonDescs[i].info2]->getNameTextID());
+				break;
 			}
-			break;
 		}
 
 		std::shared_ptr<CToggleButton> bonusButton = std::make_shared<CToggleButton>(Point(475 + i * 68, 455), AnimationPath::builtin("campaignBonusSelection"), CButton::tooltip(desc.toString(), desc.toString()), nullptr, EShortcut::NONE, false, [this](){

lib/CMakeLists.txt

@@ -86,6 +86,7 @@ set(lib_MAIN_SRCS
 	callback/CPlayerSpecificInfoCallback.cpp
 	callback/GameRandomizer.cpp
 
+	campaign/CampaignBonus.cpp
 	campaign/CampaignHandler.cpp
 	campaign/CampaignState.cpp
 
@@ -480,7 +481,7 @@ set(lib_MAIN_HEADERS
 	callback/IGameRandomizer.h
 	callback/GameRandomizer.h
 
-
+	campaign/CampaignBonus.h
 	campaign/CampaignConstants.h
 	campaign/CampaignHandler.h
 	campaign/CampaignScenarioPrologEpilog.h

lib/campaign/CampaignBonus.cpp

@@ -0,0 +1,352 @@
+/*
+ * CampaignBonus.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#include "StdInc.h"
+#include "CampaignBonus.h"
+
+#include "../filesystem/CBinaryReader.h"
+#include "../json/JsonNode.h"
+#include "../constants/StringConstants.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+static const std::map<std::string, CampaignBonusType> bonusTypeMap = {
+	{"spell", CampaignBonusType::SPELL},
+	{"creature", CampaignBonusType::MONSTER},
+	{"building", CampaignBonusType::BUILDING},
+	{"artifact", CampaignBonusType::ARTIFACT},
+	{"scroll", CampaignBonusType::SPELL_SCROLL},
+	{"primarySkill", CampaignBonusType::PRIMARY_SKILL},
+	{"secondarySkill", CampaignBonusType::SECONDARY_SKILL},
+	{"resource", CampaignBonusType::RESOURCE},
+	{"prevHero", CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO},
+	{"hero", CampaignBonusType::HERO},
+};
+
+static const std::map<std::string, ui16> heroSpecialMap = {
+	{"strongest", HeroTypeID::CAMP_STRONGEST},
+	{"generated", HeroTypeID::CAMP_GENERATED},
+	{"random", HeroTypeID::CAMP_RANDOM}
+};
+
+static const std::map<std::string, ui8> resourceTypeMap = {
+	{"wood", EGameResID::WOOD},
+	{"mercury", EGameResID::MERCURY},
+	{"ore", EGameResID::ORE},
+	{"sulfur", EGameResID::SULFUR},
+	{"crystal", EGameResID::CRYSTAL},
+	{"gems", EGameResID::GEMS},
+	{"gold", EGameResID::GOLD},
+	{"common", EGameResID::COMMON},
+	{"rare", EGameResID::RARE}
+};
+
+CampaignBonus::CampaignBonus(CBinaryReader & reader, CampaignStartOptions mode)
+{
+	switch(mode)
+	{
+		case CampaignStartOptions::NONE:
+			// in this mode game should not attempt to create instances of bonuses
+			throw std::runtime_error("Attempt to create empty campaign bonus");
+
+		case CampaignStartOptions::START_BONUS:
+		{
+			auto bonusType = static_cast<CampaignBonusType>(reader.readUInt8());
+
+			switch(bonusType)
+			{
+				case CampaignBonusType::SPELL:
+				{
+					HeroTypeID hero(reader.readUInt16());
+					SpellID spell(reader.readUInt8());
+					data = CampaignBonusSpell{hero, spell};
+					break;
+				}
+				case CampaignBonusType::MONSTER:
+				{
+					HeroTypeID hero(reader.readUInt16());
+					CreatureID creature(reader.readUInt16());
+					int32_t amount = reader.readUInt16();
+					data = CampaignBonusCreatures{hero, creature, amount};
+					break;
+				}
+				case CampaignBonusType::BUILDING:
+				{
+					BuildingID building(reader.readUInt8());
+					data = CampaignBonusBuilding{building};
+					break;
+				}
+				case CampaignBonusType::ARTIFACT:
+				{
+					HeroTypeID hero(reader.readUInt16());
+					ArtifactID artifact(reader.readUInt16());
+					data = CampaignBonusArtifact{hero, artifact};
+					break;
+				}
+				case CampaignBonusType::SPELL_SCROLL:
+				{
+					HeroTypeID hero(reader.readUInt16());
+					SpellID spell(reader.readUInt8());
+					data = CampaignBonusSpellScroll{hero, spell};
+					break;
+				}
+				case CampaignBonusType::PRIMARY_SKILL:
+				{
+					HeroTypeID hero(reader.readUInt16());
+					std::array<uint8_t, 4> amounts = {};
+					for(auto & value : amounts)
+						value = reader.readUInt8();
+
+					data = CampaignBonusPrimarySkill{hero, amounts};
+					break;
+				}
+				case CampaignBonusType::SECONDARY_SKILL:
+				{
+					HeroTypeID hero(reader.readUInt16());
+					SecondarySkill skill(reader.readUInt8());
+					int32_t skillMastery(reader.readUInt8());
+					data = CampaignBonusSecondarySkill{hero, skill, skillMastery};
+					break;
+				}
+				case CampaignBonusType::RESOURCE:
+				{
+					GameResID resource(reader.readInt8());
+					int32_t amount(reader.readInt32());
+					data = CampaignBonusStartingResources{resource, amount};
+					break;
+				}
+				default:
+					throw std::runtime_error("Corrupted or unsupported h3c file");
+			}
+			break;
+		}
+		case CampaignStartOptions::HERO_CROSSOVER: //reading of players (colors / scenarios ?) player can choose
+		{
+			PlayerColor player(reader.readUInt8());
+			CampaignScenarioID scenario(reader.readUInt8());
+			data = CampaignBonusHeroesFromScenario{player, scenario};
+			break;
+		}
+		case CampaignStartOptions::HERO_OPTIONS: //heroes player can choose between
+		{
+			PlayerColor player(reader.readUInt8());
+			HeroTypeID hero(reader.readInt16());
+			data = CampaignBonusStartingHero{player, hero};
+			break;
+		}
+		default:
+		{
+			throw std::runtime_error("Corrupted or unsupported h3c file");
+		}
+	}
+}
+
+CampaignBonus::CampaignBonus(const JsonNode & bjson, CampaignStartOptions mode)
+{
+	CampaignBonusType bonusType;
+
+	if (!bjson["what"].isNull())
+	{
+		bonusType = bonusTypeMap.at(bjson["what"].String());
+	}
+	else if (mode == CampaignStartOptions::HERO_CROSSOVER)
+	{
+		bonusType = CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO;
+	}
+	else if (mode == CampaignStartOptions::HERO_OPTIONS)
+	{
+		bonusType = CampaignBonusType::HERO;
+	}
+	else
+	{
+		throw std::runtime_error("Corrupted or unsupported vcmp file");
+	}
+
+	const auto & decodeHeroTypeID = [](JsonNode heroType) -> HeroTypeID
+	{
+		if(vstd::contains(heroSpecialMap, heroType.String()))
+			return HeroTypeID(heroSpecialMap.at(heroType.String()));
+		else
+			return HeroTypeID(HeroTypeID::decode(heroType.String()));
+	};
+
+	switch(bonusType)
+	{
+		case CampaignBonusType::SPELL:
+		{
+			HeroTypeID hero(decodeHeroTypeID(bjson["hero"]));
+			SpellID spell(SpellID::decode(bjson["spellType"].String()));
+			data = CampaignBonusSpell{hero, spell};
+			break;
+		}
+		case CampaignBonusType::MONSTER:
+		{
+			HeroTypeID hero(decodeHeroTypeID(bjson["hero"]));
+			CreatureID creature(CreatureID::decode(bjson["creatureType"].String()));
+			int32_t amount = bjson["creatureAmount"].Integer();
+			data = CampaignBonusCreatures{hero, creature, amount};
+			break;
+		}
+		case CampaignBonusType::BUILDING:
+		{
+			BuildingID building(vstd::find_pos(EBuildingType::names, bjson["buildingType"].String()));
+			data = CampaignBonusBuilding{building};
+			break;
+		}
+		case CampaignBonusType::ARTIFACT:
+		{
+			HeroTypeID hero(decodeHeroTypeID(bjson["hero"]));
+			ArtifactID artifact(ArtifactID::decode(bjson["artifactType"].String()));
+			data = CampaignBonusArtifact{hero, artifact};
+			break;
+		}
+		case CampaignBonusType::SPELL_SCROLL:
+		{
+			HeroTypeID hero(decodeHeroTypeID(bjson["hero"]));
+			SpellID spell(SpellID::decode(bjson["spellType"].String()));
+			data = CampaignBonusSpellScroll{hero, spell};
+			break;
+		}
+		case CampaignBonusType::PRIMARY_SKILL:
+		{
+			HeroTypeID hero(decodeHeroTypeID(bjson["hero"]));
+			std::array<uint8_t, 4> amounts = {};
+			for(size_t i = 0; i < amounts.size(); ++i)
+				amounts[i] = bjson[NPrimarySkill::names[i]].Integer();
+
+			data = CampaignBonusPrimarySkill{hero, amounts};
+			break;
+		}
+		case CampaignBonusType::SECONDARY_SKILL:
+		{
+			HeroTypeID hero(decodeHeroTypeID(bjson["hero"]));
+			SecondarySkill skill(SecondarySkill::decode(bjson["skillType"].String()));
+			int32_t skillMastery = bjson["skillMastery"].Integer();
+			data = CampaignBonusSecondarySkill{hero, skill, skillMastery};
+			break;
+		}
+		case CampaignBonusType::RESOURCE:
+		{
+			GameResID resource(resourceTypeMap.at(bjson["resourceType"].String()));
+			int32_t resourceAmount(bjson["resourceAmount"].Integer());
+			data = CampaignBonusStartingResources{resource, resourceAmount};
+			break;
+		}
+		case CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO: //reading of players (colors / scenarios ?) player can choose
+		{
+			PlayerColor player(SecondarySkill::decode(bjson["playerColor"].String()));
+			CampaignScenarioID scenario(bjson["scenario"].Integer());
+			data = CampaignBonusHeroesFromScenario{player, scenario};
+			break;
+		}
+		case CampaignBonusType::HERO: //heroes player can choose between
+		{
+			PlayerColor player(SecondarySkill::decode(bjson["playerColor"].String()));
+			HeroTypeID hero(decodeHeroTypeID(bjson["hero"]));
+			data = CampaignBonusStartingHero{player, hero};
+			break;
+		}
+		default:
+			throw std::runtime_error("Corrupted or unsupported h3c file");
+	}
+}
+
+JsonNode CampaignBonus::toJson() const
+{
+	JsonNode bnode;
+
+	const auto & encodeHeroTypeID = [](HeroTypeID heroType) -> JsonNode
+	{
+		if(vstd::contains(vstd::reverseMap(heroSpecialMap), heroType))
+			return JsonNode(vstd::reverseMap(heroSpecialMap)[heroType]);
+		else
+			return JsonNode(HeroTypeID::encode(heroType));
+	};
+
+	bnode["what"].String() = vstd::reverseMap(bonusTypeMap).at(getType());
+
+	switch (getType())
+	{
+		case CampaignBonusType::SPELL:
+		{
+			const auto & bonusValue = getValue<CampaignBonusSpell>();
+			bnode["hero"] = encodeHeroTypeID(bonusValue.hero);
+			bnode["spellType"].String() = SpellID::encode(bonusValue.spell.getNum());
+			break;
+		}
+		case CampaignBonusType::MONSTER:
+		{
+			const auto & bonusValue = getValue<CampaignBonusCreatures>();
+			bnode["hero"] = encodeHeroTypeID(bonusValue.hero);
+			bnode["creatureType"].String() = CreatureID::encode(bonusValue.creature.getNum());
+			bnode["creatureAmount"].Integer() = bonusValue.amount;
+			break;
+		}
+		case CampaignBonusType::BUILDING:
+		{
+			const auto & bonusValue = getValue<CampaignBonusBuilding>();
+			bnode["buildingType"].String() = EBuildingType::names[bonusValue.building.getNum()];
+			break;
+		}
+		case CampaignBonusType::ARTIFACT:
+		{
+			const auto & bonusValue = getValue<CampaignBonusArtifact>();
+			bnode["hero"] = encodeHeroTypeID(bonusValue.hero);
+			bnode["artifactType"].String() = ArtifactID::encode(bonusValue.artifact.getNum());
+			break;
+		}
+		case CampaignBonusType::SPELL_SCROLL:
+		{
+			const auto & bonusValue = getValue<CampaignBonusSpellScroll>();
+			bnode["hero"] = encodeHeroTypeID(bonusValue.hero);
+			bnode["spellType"].String() = SpellID::encode(bonusValue.spell.getNum());
+			break;
+		}
+		case CampaignBonusType::PRIMARY_SKILL:
+		{
+			const auto & bonusValue = getValue<CampaignBonusPrimarySkill>();
+			bnode["hero"] = encodeHeroTypeID(bonusValue.hero);
+			for(size_t i = 0; i < bonusValue.amounts.size(); ++i)
+				bnode[NPrimarySkill::names[i]].Integer() = bonusValue.amounts[i];
+			break;
+		}
+		case CampaignBonusType::SECONDARY_SKILL:
+		{
+			const auto & bonusValue = getValue<CampaignBonusSecondarySkill>();
+			bnode["hero"] = encodeHeroTypeID(bonusValue.hero);
+			bnode["skillType"].String() = SecondarySkill::encode(bonusValue.skill.getNum());
+			bnode["skillMastery"].Integer() = bonusValue.mastery;
+			break;
+		}
+		case CampaignBonusType::RESOURCE:
+		{
+			const auto & bonusValue = getValue<CampaignBonusStartingResources>();
+			bnode["resourceType"].String() = vstd::reverseMap(resourceTypeMap)[bonusValue.resource.getNum()];
+			bnode["resourceAmount"].Integer() = bonusValue.amount;
+			break;
+		}
+		case CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO:
+		{
+			const auto & bonusValue = getValue<CampaignBonusHeroesFromScenario>();
+			bnode["playerColor"].String() = PlayerColor::encode(bonusValue.startingPlayer);
+			bnode["scenario"].Integer() = bonusValue.scenario.getNum();
+			break;
+		}
+		case CampaignBonusType::HERO:
+		{
+			const auto & bonusValue = getValue<CampaignBonusStartingHero>();
+			bnode["playerColor"].String() = PlayerColor::encode(bonusValue.startingPlayer);
+			bnode["hero"] = encodeHeroTypeID(bonusValue.hero);
+			break;
+		}
+	}
+	return bnode;
+}
+
+VCMI_LIB_NAMESPACE_END

lib/campaign/CampaignBonus.h

@@ -0,0 +1,220 @@
+/*
+ * CampaignBonus.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+#include "CampaignConstants.h"
+#include "../constants/EntityIdentifiers.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+class CBinaryReader;
+class JsonNode;
+
+struct CampaignBonusSpell
+{
+	HeroTypeID hero;
+	SpellID spell;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & hero;
+		h & spell;
+	}
+};
+
+struct CampaignBonusCreatures
+{
+	HeroTypeID hero;
+	CreatureID creature;
+	int32_t amount = 0;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & hero;
+		h & creature;
+		h & amount;
+	}
+};
+
+struct CampaignBonusBuilding
+{
+	BuildingID building;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & building;
+	}
+};
+
+struct CampaignBonusArtifact
+{
+	HeroTypeID hero;
+	ArtifactID artifact;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & hero;
+		h & artifact;
+	}
+};
+
+struct CampaignBonusSpellScroll
+{
+	HeroTypeID hero;
+	SpellID spell;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & hero;
+		h & spell;
+	}
+};
+
+struct CampaignBonusPrimarySkill
+{
+	HeroTypeID hero;
+	std::array<uint8_t, 4> amounts = {};
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & hero;
+		h & amounts;
+	}
+};
+
+struct CampaignBonusSecondarySkill
+{
+	HeroTypeID hero;
+	SecondarySkill skill;
+	int32_t mastery = 0;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & hero;
+		h & skill;
+		h & mastery;
+	}
+};
+
+struct CampaignBonusStartingResources
+{
+	GameResID resource;
+	int32_t amount = 0;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & resource;
+		h & amount;
+	}
+};
+
+struct CampaignBonusHeroesFromScenario
+{
+	PlayerColor startingPlayer;
+	CampaignScenarioID scenario;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & startingPlayer;
+		h & scenario;
+	}
+};
+
+struct CampaignBonusStartingHero
+{
+	PlayerColor startingPlayer;
+	HeroTypeID hero;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & startingPlayer;
+		h & hero;
+	}
+};
+
+class CampaignBonus
+{
+	using Variant = std::variant<
+		CampaignBonusSpell,
+		CampaignBonusCreatures,
+		CampaignBonusBuilding,
+		CampaignBonusArtifact,
+		CampaignBonusSpellScroll,
+		CampaignBonusPrimarySkill,
+		CampaignBonusSecondarySkill,
+		CampaignBonusStartingResources,
+		CampaignBonusHeroesFromScenario,
+		CampaignBonusStartingHero>;
+
+	Variant data;
+
+	template<typename T, typename = void>
+	struct HasHero : std::false_type { };
+
+	template<typename T>
+	struct HasHero<T, decltype(std::declval<T>().hero, void())> : std::true_type { };
+
+public:
+	CampaignBonus() = default;
+
+	template<typename BonusType>
+	CampaignBonus(const BonusType & value)
+		:data(value)
+	{}
+
+	DLL_LINKAGE CampaignBonus(CBinaryReader & reader, CampaignStartOptions mode);
+	DLL_LINKAGE CampaignBonus(const JsonNode & json, CampaignStartOptions mode);
+
+	template<typename T>
+	const T & getValue() const
+	{
+		return std::get<T>(data);
+	}
+
+	HeroTypeID getTargetedHero() const
+	{
+		HeroTypeID result;
+
+		std::visit([&result](const auto & bonusValue){
+			if constexpr (HasHero<decltype(bonusValue)>::value)
+			{
+				result = bonusValue.hero;
+			}
+			throw std::runtime_error("Attempt to get targeted hero on invalid type!");
+		}, data);
+
+		return result;
+	}
+
+	bool isBonusForHero() const
+	{
+		auto type = getType();
+		return type == CampaignBonusType::SPELL ||
+			   type == CampaignBonusType::MONSTER ||
+			   type == CampaignBonusType::ARTIFACT ||
+			   type == CampaignBonusType::SPELL_SCROLL ||
+			   type == CampaignBonusType::PRIMARY_SKILL ||
+			   type == CampaignBonusType::SECONDARY_SKILL;
+	}
+
+	CampaignBonusType getType() const
+	{
+		return static_cast<CampaignBonusType>(data.index());
+	}
+
+	DLL_LINKAGE JsonNode toJson() const;
+
+	template <typename Handler> void serialize(Handler &h)
+	{
+		h & data;
+	}
+};
+
+VCMI_LIB_NAMESPACE_END

lib/campaign/CampaignConstants.h

@@ -27,7 +27,7 @@ enum class CampaignVersion : uint8_t
 	VCMI_MAX = 1,
 };
 
-enum class CampaignStartOptions: int8_t
+enum class CampaignStartOptions : int8_t
 {
 	NONE = 0,
 	START_BONUS,
@@ -37,7 +37,6 @@ enum class CampaignStartOptions: int8_t
 
 enum class CampaignBonusType : int8_t
 {
-	NONE = -1,
 	SPELL,
 	MONSTER,
 	BUILDING,

lib/campaign/CampaignHandler.cpp

@@ -18,7 +18,6 @@
 #include "../filesystem/CBinaryReader.h"
 #include "../filesystem/CZipLoader.h"
 #include "../GameLibrary.h"
-#include "../constants/StringConstants.h"
 #include "../mapping/CMapHeader.h"
 #include "../mapping/CMapService.h"
 #include "../modding/CModHandler.h"
@@ -29,7 +28,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-void CampaignHandler::readCampaign(Campaign * ret, const std::vector<ui8> & input, std::string filename, std::string modName, std::string encoding)
+void CampaignHandler::readCampaign(Campaign * ret, const std::vector<ui8> & input, const std::string & filename, const std::string & modName, const std::string & encoding)
 {
 	if (input.front() < uint8_t(' ')) // binary format
 	{
@@ -77,8 +76,8 @@ std::unique_ptr<Campaign> CampaignHandler::getHeader( const std::string & name)
 std::shared_ptr<CampaignState> CampaignHandler::getCampaign( const std::string & name )
 {
 	ResourcePath resourceID(name, EResType::CAMPAIGN);
-	std::string modName = LIBRARY->modh->findResourceOrigin(resourceID);
-	std::string encoding = LIBRARY->modh->findResourceEncoding(resourceID);
+	const std::string & modName = LIBRARY->modh->findResourceOrigin(resourceID);
+	const std::string & encoding = LIBRARY->modh->findResourceEncoding(resourceID);
 	
 	auto ret = std::make_unique<CampaignState>();
 	
@@ -124,14 +123,14 @@ static std::string convertMapName(std::string input)
 	return input;
 }
 
-std::string CampaignHandler::readLocalizedString(CampaignHeader & target, CBinaryReader & reader, std::string filename, std::string modName, std::string encoding, std::string identifier)
+std::string CampaignHandler::readLocalizedString(CampaignHeader & target, CBinaryReader & reader, const std::string & filename, const std::string & modName, const std::string & encoding, const std::string & identifier)
 {
-	std::string input = TextOperations::toUnicode(reader.readBaseString(), encoding);
+	const std::string & input = TextOperations::toUnicode(reader.readBaseString(), encoding);
 
 	return readLocalizedString(target, input, filename, modName, identifier);
 }
 
-std::string CampaignHandler::readLocalizedString(CampaignHeader & target, std::string text, std::string filename, std::string modName, std::string identifier)
+std::string CampaignHandler::readLocalizedString(CampaignHeader & target, const std::string & text, const std::string & filename, const std::string & modName, const std::string & identifier)
 {
 	TextIdentifier stringID( "campaign", convertMapName(filename), identifier);
 
@@ -142,7 +141,7 @@ std::string CampaignHandler::readLocalizedString(CampaignHeader & target, std::s
 	return stringID.get();
 }
 
-void CampaignHandler::readHeaderFromJson(CampaignHeader & ret, JsonNode & reader, std::string filename, std::string modName, std::string encoding)
+void CampaignHandler::readHeaderFromJson(CampaignHeader & ret, JsonNode & reader, const std::string & filename, const std::string & modName, const std::string & encoding)
 {
 	ret.version = static_cast<CampaignVersion>(reader["version"].Integer());
 	if(ret.version < CampaignVersion::VCMI_MIN || ret.version > CampaignVersion::VCMI_MAX)
@@ -260,44 +259,6 @@ static const std::map<std::string, CampaignStartOptions> startOptionsMap = {
 	{"hero", CampaignStartOptions::HERO_OPTIONS}
 };
 
-static const std::map<std::string, CampaignBonusType> bonusTypeMap = {
-	{"spell", CampaignBonusType::SPELL},
-	{"creature", CampaignBonusType::MONSTER},
-	{"building", CampaignBonusType::BUILDING},
-	{"artifact", CampaignBonusType::ARTIFACT},
-	{"scroll", CampaignBonusType::SPELL_SCROLL},
-	{"primarySkill", CampaignBonusType::PRIMARY_SKILL},
-	{"secondarySkill", CampaignBonusType::SECONDARY_SKILL},
-	{"resource", CampaignBonusType::RESOURCE},
-	//{"prevHero", CScenarioTravel::STravelBonus::EBonusType::HEROES_FROM_PREVIOUS_SCENARIO},
-	//{"hero", CScenarioTravel::STravelBonus::EBonusType::HERO},
-};
-
-static const std::map<std::string, ui32> primarySkillsMap = {
-	{"attack", 0},
-	{"defence", 8},
-	{"spellpower", 16},
-	{"knowledge", 24},
-};
-
-static const std::map<std::string, ui16> heroSpecialMap = {
-	{"strongest", HeroTypeID::CAMP_STRONGEST},
-	{"generated", HeroTypeID::CAMP_GENERATED},
-	{"random", HeroTypeID::CAMP_RANDOM}
-};
-
-static const std::map<std::string, ui8> resourceTypeMap = {
-	{"wood", EGameResID::WOOD},
-	{"mercury", EGameResID::MERCURY},
-	{"ore", EGameResID::ORE},
-	{"sulfur", EGameResID::SULFUR},
-	{"crystal", EGameResID::CRYSTAL},
-	{"gems", EGameResID::GEMS},
-	{"gold", EGameResID::GOLD},
-	{"common", EGameResID::COMMON},
-	{"rare", EGameResID::RARE}
-};
-
 CampaignTravel CampaignHandler::readScenarioTravelFromJson(JsonNode & reader)
 {
 	CampaignTravel ret;
@@ -327,117 +288,12 @@ CampaignTravel CampaignHandler::readScenarioTravelFromJson(JsonNode & reader)
 	}
 
 	ret.startOptions = startOptionsMap.at(reader["startOptions"].String());
-	switch(ret.startOptions)
-	{
-	case CampaignStartOptions::NONE:
-		//no bonuses. Seems to be OK
-		break;
-	case CampaignStartOptions::START_BONUS: //reading of bonuses player can choose
-		{
-			ret.playerColor = PlayerColor(PlayerColor::decode(reader["playerColor"].String()));
-			for(auto & bjson : reader["bonuses"].Vector())
-			{
-				CampaignBonus bonus;
-				bonus.type = bonusTypeMap.at(bjson["what"].String());
-				switch (bonus.type)
-				{
-					case CampaignBonusType::RESOURCE:
-						bonus.info1 = resourceTypeMap.at(bjson["type"].String());
-						bonus.info2 = bjson["amount"].Integer();
-						break;
-						
-					case CampaignBonusType::BUILDING:
-						bonus.info1 = vstd::find_pos(EBuildingType::names, bjson["type"].String());
-						if(bonus.info1 == -1)
-							logGlobal->warn("VCMP Loading: unresolved building identifier %s", bjson["type"].String());
-						break;
-						
-					default:
-						auto heroIdentifier = bjson["hero"].String();
-						auto it = heroSpecialMap.find(heroIdentifier);
-						if(it != heroSpecialMap.end())
-							bonus.info1 = it->second;
-						else
-							if(auto identifier = LIBRARY->identifiers()->getIdentifier(ModScope::scopeMap(), "hero", heroIdentifier))
-								bonus.info1 = identifier.value();
-							else
-								logGlobal->warn("VCMP Loading: unresolved hero identifier %s", heroIdentifier);
-	
-						bonus.info3 = bjson["amount"].Integer();
-						
-						switch(bonus.type)
-						{
-							case CampaignBonusType::SPELL:
-							case CampaignBonusType::MONSTER:
-							case CampaignBonusType::SECONDARY_SKILL:
-							case CampaignBonusType::ARTIFACT:
-								if(auto identifier  = LIBRARY->identifiers()->getIdentifier(ModScope::scopeMap(), bjson["what"].String(), bjson["type"].String()))
-									bonus.info2 = identifier.value();
-								else
-									logGlobal->warn("VCMP Loading: unresolved %s identifier %s", bjson["what"].String(), bjson["type"].String());
-								break;
-								
-							case CampaignBonusType::SPELL_SCROLL:
-								if(auto Identifier = LIBRARY->identifiers()->getIdentifier(ModScope::scopeMap(), "spell", bjson["type"].String()))
-									bonus.info2 = Identifier.value();
-								else
-									logGlobal->warn("VCMP Loading: unresolved spell scroll identifier %s", bjson["type"].String());
-								break;
-								
-							case CampaignBonusType::PRIMARY_SKILL:
-								for(auto & ps : primarySkillsMap)
-									bonus.info2 |= bjson[ps.first].Integer() << ps.second;
-								break;
-								
-							default:
-								bonus.info2 = bjson["type"].Integer();
-						}
-						break;
-				}
-				ret.bonusesToChoose.push_back(bonus);
-			}
-			break;
-		}
-	case CampaignStartOptions::HERO_CROSSOVER: //reading of players (colors / scenarios ?) player can choose
-		{
-			for(auto & bjson : reader["bonuses"].Vector())
-			{
-				CampaignBonus bonus;
-				bonus.type = CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO;
-				bonus.info1 = bjson["playerColor"].Integer(); //player color
-				bonus.info2 = bjson["scenario"].Integer(); //from what scenario
-				ret.bonusesToChoose.push_back(bonus);
-			}
-			break;
-		}
-	case CampaignStartOptions::HERO_OPTIONS: //heroes player can choose between
-		{
-			for(auto & bjson : reader["bonuses"].Vector())
-			{
-				CampaignBonus bonus;
-				bonus.type = CampaignBonusType::HERO;
-				bonus.info1 = bjson["playerColor"].Integer(); //player color
-
-				auto heroIdentifier = bjson["hero"].String();
-				auto it = heroSpecialMap.find(heroIdentifier);
-				if(it != heroSpecialMap.end())
-					bonus.info2 = it->second;
-				else
-					if (auto identifier = LIBRARY->identifiers()->getIdentifier(ModScope::scopeMap(), "hero", heroIdentifier))
-						bonus.info2 = identifier.value();
-					else
-						logGlobal->warn("VCMP Loading: unresolved hero identifier %s", heroIdentifier);
-			
-				ret.bonusesToChoose.push_back(bonus);
-			}
-			break;
-		}
-	default:
-		{
-			logGlobal->warn("VCMP Loading: Unsupported start options value");
-			break;
-		}
-	}
+
+	if (!reader["playerColor"].isNull())
+		ret.playerColor = PlayerColor(PlayerColor::decode(reader["playerColor"].String()));
+
+	for(auto & bjson : reader["bonuses"].Vector())
+		ret.bonusesToChoose.emplace_back(bjson, ret.startOptions);
 
 	return ret;
 }
@@ -454,99 +310,19 @@ void CampaignHandler::writeScenarioTravelToJson(JsonNode & node, const CampaignT
 		node["heroKeeps"].Vector().push_back(JsonNode("spells"));
 	if(travel.whatHeroKeeps.artifacts)
 		node["heroKeeps"].Vector().push_back(JsonNode("artifacts"));
-	for(auto & c : travel.monstersKeptByHero)
+	for(const auto & c : travel.monstersKeptByHero)
 		node["keepCreatures"].Vector().push_back(JsonNode(CreatureID::encode(c)));
-	for(auto & a : travel.artifactsKeptByHero)
+	for(const auto & a : travel.artifactsKeptByHero)
 		node["keepArtifacts"].Vector().push_back(JsonNode(ArtifactID::encode(a)));
-	node["startOptions"].String() = vstd::reverseMap(startOptionsMap)[travel.startOptions];
 
-	switch(travel.startOptions)
-	{
-	case CampaignStartOptions::NONE:
-		break;
-	case CampaignStartOptions::START_BONUS:
-		{
-			node["playerColor"].String() = PlayerColor::encode(travel.playerColor);
-			for(auto & bonus : travel.bonusesToChoose)
-			{
-				JsonNode bnode;
-				bnode["what"].String() = vstd::reverseMap(bonusTypeMap)[bonus.type];
-				switch (bonus.type)
-				{
-					case CampaignBonusType::RESOURCE:
-						bnode["type"].String() = vstd::reverseMap(resourceTypeMap)[bonus.info1];
-						bnode["amount"].Integer() = bonus.info2;
-						break;
-					case CampaignBonusType::BUILDING:
-						bnode["type"].String() = EBuildingType::names[bonus.info1];
-						break;
-					default:
-						if(vstd::contains(vstd::reverseMap(heroSpecialMap), bonus.info1))
-							bnode["hero"].String() = vstd::reverseMap(heroSpecialMap)[bonus.info1];
-						else
-							bnode["hero"].String() = HeroTypeID::encode(bonus.info1);
-						bnode["amount"].Integer() = bonus.info3;
-						switch(bonus.type)
-						{
-							case CampaignBonusType::SPELL:
-								bnode["type"].String() = SpellID::encode(bonus.info2);
-								break;
-							case CampaignBonusType::MONSTER:
-								bnode["type"].String() = CreatureID::encode(bonus.info2);
-								break;
-							case CampaignBonusType::SECONDARY_SKILL:
-								bnode["type"].String() = SecondarySkill::encode(bonus.info2);
-								break;
-							case CampaignBonusType::ARTIFACT:
-								bnode["type"].String() = ArtifactID::encode(bonus.info2);
-								break;
-							case CampaignBonusType::SPELL_SCROLL:
-								bnode["type"].String() = SpellID::encode(bonus.info2);
-								break;
-							case CampaignBonusType::PRIMARY_SKILL:
-								for(auto & ps : primarySkillsMap)
-									bnode[ps.first].Integer() = (bonus.info2 >> ps.second) & 0xff;
-								break;
-							default:
-								bnode["type"].Integer() = bonus.info2;
-						}
-						break;
-				}
-				node["bonuses"].Vector().push_back(bnode);
-			}
-			break;
-		}
-	case CampaignStartOptions::HERO_CROSSOVER:
-		{
-			for(auto & bonus : travel.bonusesToChoose)
-			{
-				JsonNode bnode;
-				bnode["playerColor"].Integer() = bonus.info1;
-				bnode["scenario"].Integer() = bonus.info2;
-				node["bonuses"].Vector().push_back(bnode);
-			}
-			break;
-		}
-	case CampaignStartOptions::HERO_OPTIONS:
-		{
-			for(auto & bonus : travel.bonusesToChoose)
-			{
-				JsonNode bnode;
-				bnode["playerColor"].Integer() = bonus.info1;
-
-				if(vstd::contains(vstd::reverseMap(heroSpecialMap), bonus.info2))
-					bnode["hero"].String() = vstd::reverseMap(heroSpecialMap)[bonus.info2];
-				else
-					bnode["hero"].String() = HeroTypeID::encode(bonus.info2);
-				
-				node["bonuses"].Vector().push_back(bnode);
-			}
-			break;
-		}
-	}
+	if (travel.playerColor.isValidPlayer())
+		node["playerColor"].String() = PlayerColor::encode(travel.playerColor.getNum());
+
+	for (const auto & bonus : travel.bonusesToChoose)
+		node["bonuses"].Vector().push_back(bonus.toJson());
 }
 
-void CampaignHandler::readHeaderFromMemory( CampaignHeader & ret, CBinaryReader & reader, std::string filename, std::string modName, std::string encoding )
+void CampaignHandler::readHeaderFromMemory( CampaignHeader & ret, CBinaryReader & reader, const std::string & filename, const std::string & modName, const std::string & encoding )
 {
 	ret.version = static_cast<CampaignVersion>(reader.readUInt32());
 
@@ -684,114 +460,14 @@ CampaignTravel CampaignHandler::readScenarioTravelFromMemory(CBinaryReader & rea
 
 	ret.startOptions = static_cast<CampaignStartOptions>(reader.readUInt8());
 
-	switch(ret.startOptions)
-	{
-	case CampaignStartOptions::NONE:
-		//no bonuses. Seems to be OK
-		break;
-	case CampaignStartOptions::START_BONUS: //reading of bonuses player can choose
-		{
-			ret.playerColor.setNum(reader.readUInt8());
-			ui8 numOfBonuses = reader.readUInt8();
-			for (int g=0; g<numOfBonuses; ++g)
-			{
-				CampaignBonus bonus;
-				bonus.type = static_cast<CampaignBonusType>(reader.readUInt8());
-				//hero: FFFD means 'most powerful' and FFFE means 'generated'
-				switch(bonus.type)
-				{
-				case CampaignBonusType::SPELL:
-					{
-						bonus.info1 = reader.readUInt16(); //hero
-						bonus.info2 = reader.readUInt8(); //spell ID
-						break;
-					}
-				case CampaignBonusType::MONSTER:
-					{
-						bonus.info1 = reader.readUInt16(); //hero
-						bonus.info2 = reader.readUInt16(); //monster type
-						bonus.info3 = reader.readUInt16(); //monster count
-						break;
-					}
-				case CampaignBonusType::BUILDING:
-					{
-						bonus.info1 = reader.readUInt8(); //building ID (0 - town hall, 1 - city hall, 2 - capitol, etc)
-						break;
-					}
-				case CampaignBonusType::ARTIFACT:
-					{
-						bonus.info1 = reader.readUInt16(); //hero
-						bonus.info2 = reader.readUInt16(); //artifact ID
-						break;
-					}
-				case CampaignBonusType::SPELL_SCROLL:
-					{
-						bonus.info1 = reader.readUInt16(); //hero
-						bonus.info2 = reader.readUInt8(); //spell ID
-						break;
-					}
-				case CampaignBonusType::PRIMARY_SKILL:
-					{
-						bonus.info1 = reader.readUInt16(); //hero
-						bonus.info2 = reader.readUInt32(); //bonuses (4 bytes for 4 skills)
-						break;
-					}
-				case CampaignBonusType::SECONDARY_SKILL:
-					{
-						bonus.info1 = reader.readUInt16(); //hero
-						bonus.info2 = reader.readUInt8(); //skill ID
-						bonus.info3 = reader.readUInt8(); //skill level
-						break;
-					}
-				case CampaignBonusType::RESOURCE:
-					{
-						bonus.info1 = reader.readUInt8(); //type
-						//FD - wood+ore
-						//FE - mercury+sulfur+crystal+gem
-						bonus.info2 = reader.readUInt32(); //count
-						break;
-					}
-				default:
-					logGlobal->warn("Corrupted h3c file");
-					break;
-				}
-				ret.bonusesToChoose.push_back(bonus);
-			}
-			break;
-		}
-	case CampaignStartOptions::HERO_CROSSOVER: //reading of players (colors / scenarios ?) player can choose
-		{
-			ui8 numOfBonuses = reader.readUInt8();
-			for (int g=0; g<numOfBonuses; ++g)
-			{
-				CampaignBonus bonus;
-				bonus.type = CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO;
-				bonus.info1 = reader.readUInt8(); //player color
-				bonus.info2 = reader.readUInt8(); //from what scenario
-
-				ret.bonusesToChoose.push_back(bonus);
-			}
-			break;
-		}
-	case CampaignStartOptions::HERO_OPTIONS: //heroes player can choose between
-		{
-			ui8 numOfBonuses = reader.readUInt8();
-			for (int g=0; g<numOfBonuses; ++g)
-			{
-				CampaignBonus bonus;
-				bonus.type = CampaignBonusType::HERO;
-				bonus.info1 = reader.readUInt8(); //player color
-				bonus.info2 = reader.readUInt16(); //hero, FF FF - random
+	if (ret.startOptions == CampaignStartOptions::START_BONUS)
+		ret.playerColor.setNum(reader.readUInt8());
 
-				ret.bonusesToChoose.push_back(bonus);
-			}
-			break;
-		}
-	default:
-		{
-			logGlobal->warn("Corrupted h3c file");
-			break;
-		}
+	if (ret.startOptions != CampaignStartOptions::NONE)
+	{
+		ui8 numOfBonuses = reader.readUInt8();
+		for (int g=0; g<numOfBonuses; ++g)
+			ret.bonusesToChoose.emplace_back(reader, ret.startOptions);
 	}
 
 	return ret;

lib/campaign/CampaignHandler.h

@@ -16,13 +16,13 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 class DLL_LINKAGE CampaignHandler
 {
-	static std::string readLocalizedString(CampaignHeader & target, CBinaryReader & reader, std::string filename, std::string modName, std::string encoding, std::string identifier);
-	static std::string readLocalizedString(CampaignHeader & target, std::string text, std::string filename, std::string modName, std::string identifier);
+	static std::string readLocalizedString(CampaignHeader & target, CBinaryReader & reader, const std::string & filename, const std::string & modName, const std::string & encoding, const std::string & identifier);
+	static std::string readLocalizedString(CampaignHeader & target, const std::string & text, const std::string & filename, const std::string & modName, const std::string & identifier);
 
-	static void readCampaign(Campaign * target, const std::vector<ui8> & stream, std::string filename, std::string modName, std::string encoding);
+	static void readCampaign(Campaign * target, const std::vector<ui8> & stream, const std::string & filename, const std::string & modName, const std::string & encoding);
 
 	//parsers for VCMI campaigns (*.vcmp)
-	static void readHeaderFromJson(CampaignHeader & target, JsonNode & reader, std::string filename, std::string modName, std::string encoding);
+	static void readHeaderFromJson(CampaignHeader & target, JsonNode & reader, const std::string & filename, const std::string & modName, const std::string & encoding);
 	static CampaignScenario readScenarioFromJson(JsonNode & reader);
 	static CampaignTravel readScenarioTravelFromJson(JsonNode & reader);
 
@@ -30,7 +30,7 @@ class DLL_LINKAGE CampaignHandler
 	static void writeScenarioTravelToJson(JsonNode & node, const CampaignTravel & travel);
 
 	//parsers for original H3C campaigns
-	static void readHeaderFromMemory(CampaignHeader & target, CBinaryReader & reader, std::string filename, std::string modName, std::string encoding);
+	static void readHeaderFromMemory(CampaignHeader & target, CBinaryReader & reader, const std::string & filename, const std::string & modName, const std::string & encoding);
 	static CampaignScenario readScenarioFromMemory(CBinaryReader & reader, CampaignHeader & header);
 	static CampaignTravel readScenarioTravelFromMemory(CBinaryReader & reader, CampaignVersion version);
 	/// returns h3c split in parts. 0 = h3c header, 1-end - maps (binary h3m)

lib/campaign/CampaignState.cpp

@@ -80,7 +80,7 @@ JsonNode CampaignRegions::toJson(CampaignRegions cr)
 	JsonNode node;
 	node["prefix"].String() = cr.campPrefix;
 	node["colorSuffixLength"].Float() = cr.colorSuffixLength;
-	if(!cr.campSuffix.size())
+	if(cr.campSuffix.empty())
 		node["suffix"].clear();
 	else
 		node["suffix"].Vector() = JsonVector{ JsonNode(cr.campSuffix[0]), JsonNode(cr.campSuffix[1]), JsonNode(cr.campSuffix[2]) };
@@ -127,7 +127,7 @@ std::optional<Point> CampaignRegions::getLabelPosition(CampaignScenarioID which)
 	return region.labelPos;
 }
 
-ImagePath CampaignRegions::getNameFor(CampaignScenarioID which, int colorIndex, std::string type) const
+ImagePath CampaignRegions::getNameFor(CampaignScenarioID which, int colorIndex, const std::string & type) const
 {
 	auto const & region = regions[which.getNum()];
 
@@ -166,24 +166,13 @@ ImagePath CampaignRegions::getConqueredName(CampaignScenarioID which, int color)
 		return getNameFor(which, color, campSuffix[2]);
 }
 
-
-bool CampaignBonus::isBonusForHero() const
-{
-	return type == CampaignBonusType::SPELL ||
-		   type == CampaignBonusType::MONSTER ||
-		   type == CampaignBonusType::ARTIFACT ||
-		   type == CampaignBonusType::SPELL_SCROLL ||
-		   type == CampaignBonusType::PRIMARY_SKILL ||
-		   type == CampaignBonusType::SECONDARY_SKILL;
-}
-
 void CampaignHeader::loadLegacyData(ui8 campId)
 {
 	campaignRegions = CampaignRegions::getLegacy(campId);
 	numberOfScenarios = LIBRARY->generaltexth->getCampaignLength(campId);
 }
 
-void CampaignHeader::loadLegacyData(CampaignRegions regions, int numOfScenario)
+void CampaignHeader::loadLegacyData(const CampaignRegions & regions, int numOfScenario)
 {
 	campaignRegions = regions;
 	numberOfScenarios = numOfScenario;

lib/campaign/CampaignState.h

@@ -9,11 +9,10 @@
  */
 #pragma once
 
-#include "../GameConstants.h"
 #include "../filesystem/ResourcePath.h"
 #include "../serializer/Serializeable.h"
 #include "../texts/TextLocalizationContainer.h"
-#include "CampaignConstants.h"
+#include "CampaignBonus.h"
 #include "CampaignScenarioPrologEpilog.h"
 #include "../gameState/HighScore.h"
 #include "../Point.h"
@@ -61,7 +60,7 @@ class DLL_LINKAGE CampaignRegions
 
 	std::vector<RegionDescription> regions;
 
-	ImagePath getNameFor(CampaignScenarioID which, int color, std::string type) const;
+	ImagePath getNameFor(CampaignScenarioID which, int color, const std::string & type) const;
 
 public:
 	ImagePath getBackgroundName() const;
@@ -116,7 +115,7 @@ class DLL_LINKAGE CampaignHeader : public boost::noncopyable
 	bool difficultyChosenByPlayer = false;
 
 	void loadLegacyData(ui8 campId);
-	void loadLegacyData(CampaignRegions regions, int numOfScenario);
+	void loadLegacyData(const CampaignRegions & regions, int numOfScenario);
 
 	TextContainerRegistrable textContainer;
 
@@ -166,26 +165,6 @@ public:
 	}
 };
 
-struct DLL_LINKAGE CampaignBonus
-{
-	CampaignBonusType type = CampaignBonusType::NONE;
-
-	//purpose depends on type
-	int32_t info1 = 0;
-	int32_t info2 = 0;
-	int32_t info3 = 0;
-
-	bool isBonusForHero() const;
-
-	template <typename Handler> void serialize(Handler &h)
-	{
-		h & type;
-		h & info1;
-		h & info2;
-		h & info3;
-	}
-};
-
 struct DLL_LINKAGE CampaignTravel
 {
 	struct DLL_LINKAGE WhatHeroKeeps
@@ -221,7 +200,30 @@ struct DLL_LINKAGE CampaignTravel
 		h & artifactsKeptByHero;
 		h & startOptions;
 		h & playerColor;
-		h & bonusesToChoose;
+		if (h.hasFeature(Handler::Version::CAMPAIGN_BONUSES))
+		{
+			h & bonusesToChoose;
+		}
+		else
+		{
+			struct OldBonus{
+				CampaignBonusType type = {};
+				int32_t info1 = 0;
+				int32_t info2 = 0;
+				int32_t info3 = 0;
+
+				void serialize(Handler &h)
+				{
+					h & type;
+					h & info1;
+					h & info2;
+					h & info3;
+				}
+			};
+
+			std::vector<OldBonus> oldBonuses;
+			h & oldBonuses;
+		}
 	}
 };
 

lib/gameState/CGameStateCampaign.cpp

@@ -66,8 +66,8 @@ std::optional<CampaignScenarioID> CGameStateCampaign::getHeroesSourceScenario()
 	auto campaignState = gameState->scenarioOps->campState;
 	auto bonus = currentBonus();
 
-	if(bonus && bonus->type == CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO)
-		return static_cast<CampaignScenarioID>(bonus->info2);
+	if(bonus && bonus->getType() == CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO)
+		return bonus->getValue<CampaignBonusHeroesFromScenario>().scenario;
 
 	return campaignState->lastScenario();
 }
@@ -211,17 +211,18 @@ void CGameStateCampaign::trimCrossoverHeroesParameters(vstd::RNG & randomGenerat
 void CGameStateCampaign::placeCampaignHeroes(vstd::RNG & randomGenerator)
 {
 	// place bonus hero
-	auto campaignState = gameState->scenarioOps->campState;
-	auto campaignBonus = campaignState->getBonus(*campaignState->currentScenario());
-	bool campaignGiveHero = campaignBonus && campaignBonus->type == CampaignBonusType::HERO;
+	const auto & campaignState = gameState->scenarioOps->campState;
+	const auto & campaignBonus = campaignState->getBonus(*campaignState->currentScenario());
+	bool campaignGiveHero = campaignBonus && campaignBonus->getType() == CampaignBonusType::HERO;
 
 	if(campaignGiveHero)
 	{
-		auto playerColor = PlayerColor(campaignBonus->info1);
-		auto it = gameState->scenarioOps->playerInfos.find(playerColor);
+		const auto & campaignBonusValue = campaignBonus->getValue<CampaignBonusStartingHero>();
+		const auto & playerColor = campaignBonusValue.startingPlayer;
+		const auto & it = gameState->scenarioOps->playerInfos.find(playerColor);
 		if(it != gameState->scenarioOps->playerInfos.end())
 		{
-			HeroTypeID heroTypeId = HeroTypeID(campaignBonus->info2);
+			HeroTypeID heroTypeId = campaignBonusValue.hero;
 			if(heroTypeId == HeroTypeID::CAMP_RANDOM) // random bonus hero
 			{
 				heroTypeId = gameState->pickUnusedHeroTypeRandomly(randomGenerator, playerColor);
@@ -309,20 +310,22 @@ void CGameStateCampaign::giveCampaignBonusToHero(CGHeroInstance * hero)
 	assert(curBonus->isBonusForHero());
 
 	//apply bonus
-	switch(curBonus->type)
+	switch(curBonus->getType())
 	{
 		case CampaignBonusType::SPELL:
 		{
-			hero->addSpellToSpellbook(SpellID(curBonus->info2));
+			const auto & bonusValue = curBonus->getValue<CampaignBonusSpell>();
+			hero->addSpellToSpellbook(bonusValue.spell);
 			break;
 		}
 		case CampaignBonusType::MONSTER:
 		{
+			const auto & bonusValue = curBonus->getValue<CampaignBonusCreatures>();
 			for(int i = 0; i < GameConstants::ARMY_SIZE; i++)
 			{
 				if(hero->slotEmpty(SlotID(i)))
 				{
-					hero->addToSlot(SlotID(i), CreatureID(curBonus->info2), curBonus->info3);
+					hero->addToSlot(SlotID(i), bonusValue.creature, bonusValue.amount);
 					break;
 				}
 			}
@@ -330,13 +333,15 @@ void CGameStateCampaign::giveCampaignBonusToHero(CGHeroInstance * hero)
 		}
 		case CampaignBonusType::ARTIFACT:
 		{
-			if(!gameState->giveHeroArtifact(hero, static_cast<ArtifactID>(curBonus->info2)))
+			const auto & bonusValue = curBonus->getValue<CampaignBonusArtifact>();
+			if(!gameState->giveHeroArtifact(hero, bonusValue.artifact))
 				logGlobal->error("Cannot give starting artifact - no free slots!");
 			break;
 		}
 		case CampaignBonusType::SPELL_SCROLL:
 		{
-			const auto scroll = gameState->createScroll(SpellID(curBonus->info2));
+			const auto & bonusValue = curBonus->getValue<CampaignBonusSpellScroll>();
+			const auto scroll = gameState->createScroll(bonusValue.spell);
 			const auto slot = ArtifactUtils::getArtAnyPosition(hero, scroll->getTypeId());
 			if(ArtifactUtils::isSlotEquipment(slot) || ArtifactUtils::isSlotBackpack(slot))
 				gameState->map->putArtifactInstance(*hero, scroll->getId(), slot);
@@ -346,10 +351,10 @@ void CGameStateCampaign::giveCampaignBonusToHero(CGHeroInstance * hero)
 		}
 		case CampaignBonusType::PRIMARY_SKILL:
 		{
-			const ui8 * ptr = reinterpret_cast<const ui8 *>(&curBonus->info2);
+			const auto & bonusValue = curBonus->getValue<CampaignBonusPrimarySkill>();
 			for(auto skill : PrimarySkill::ALL_SKILLS())
 			{
-				int val = ptr[skill.getNum()];
+				int val = bonusValue.amounts[skill.getNum()];
 				if(val == 0)
 					continue;
 
@@ -361,7 +366,8 @@ void CGameStateCampaign::giveCampaignBonusToHero(CGHeroInstance * hero)
 		}
 		case CampaignBonusType::SECONDARY_SKILL:
 		{
-			hero->setSecSkillLevel(SecondarySkill(curBonus->info2), curBonus->info3, ChangeValueMode::ABSOLUTE);
+			const auto & bonusValue = curBonus->getValue<CampaignBonusSecondarySkill>();
+			hero->setSecSkillLevel(bonusValue.skill, bonusValue.mastery, ChangeValueMode::ABSOLUTE);
 			break;
 		}
 	}
@@ -526,7 +532,7 @@ void CGameStateCampaign::generateCampaignHeroesToReplace()
 void CGameStateCampaign::initHeroes()
 {
 	auto chosenBonus = currentBonus();
-	if (chosenBonus && chosenBonus->isBonusForHero() && chosenBonus->info1 != HeroTypeID::CAMP_GENERATED.getNum()) //exclude generated heroes
+	if (chosenBonus && chosenBonus->isBonusForHero() && chosenBonus->getTargetedHero() != HeroTypeID::CAMP_GENERATED.getNum()) //exclude generated heroes
 	{
 		//find human player
 		PlayerColor humanPlayer=PlayerColor::NEUTRAL;
@@ -542,7 +548,7 @@ void CGameStateCampaign::initHeroes()
 
 		const auto & heroes = gameState->players.at(humanPlayer).getHeroes();
 
-		if (chosenBonus->info1 == HeroTypeID::CAMP_STRONGEST.getNum()) //most powerful
+		if (chosenBonus->getTargetedHero() == HeroTypeID::CAMP_STRONGEST.getNum()) //most powerful
 		{
 			int maxB = -1;
 			for (int b=0; b<heroes.size(); ++b)
@@ -561,7 +567,7 @@ void CGameStateCampaign::initHeroes()
 		{
 			for (auto & hero : heroes)
 			{
-				if (hero->getHeroTypeID().getNum() == chosenBonus->info1)
+				if (hero->getHeroTypeID().getNum() == chosenBonus->getTargetedHero())
 				{
 					giveCampaignBonusToHero(hero);
 					break;
@@ -595,18 +601,17 @@ void CGameStateCampaign::initStartingResources()
 		return ret;
 	};
 
-	auto chosenBonus = currentBonus();
-	if(chosenBonus && chosenBonus->type == CampaignBonusType::RESOURCE)
+	const auto & chosenBonus = currentBonus();
+	if(chosenBonus && chosenBonus->getType() == CampaignBonusType::RESOURCE)
 	{
+		const auto & bonusValue = chosenBonus->getValue<CampaignBonusStartingResources>();
+
 		std::vector<const PlayerSettings *> people = getHumanPlayerInfo(); //players we will give resource bonus
 		for(const PlayerSettings *ps : people)
 		{
 			std::vector<GameResID> res; //resources we will give
-			switch (chosenBonus->info1)
+			switch (bonusValue.resource.toEnum())
 			{
-				case 0: case 1: case 2: case 3: case 4: case 5: case 6:
-					res.push_back(chosenBonus->info1);
-					break;
 				case EGameResID::COMMON: //wood+ore
 					res.push_back(GameResID(EGameResID::WOOD));
 					res.push_back(GameResID(EGameResID::ORE));
@@ -618,14 +623,12 @@ void CGameStateCampaign::initStartingResources()
 					res.push_back(GameResID(EGameResID::GEMS));
 					break;
 				default:
-					assert(0);
+					res.push_back(bonusValue.resource);
 					break;
 			}
-			//increasing resource quantity
+
 			for (auto & re : res)
-			{
-				gameState->players.at(ps->color).resources[re] += chosenBonus->info2;
-			}
+				gameState->players.at(ps->color).resources[re] += bonusValue.amount;
 		}
 	}
 }
@@ -637,9 +640,11 @@ void CGameStateCampaign::initTowns()
 	if (!chosenBonus)
 		return;
 
-	if (chosenBonus->type != CampaignBonusType::BUILDING)
+	if (chosenBonus->getType() != CampaignBonusType::BUILDING)
 		return;
 
+	const auto & bonusValue = chosenBonus->getValue<CampaignBonusBuilding>();
+
 	for (const auto & townID : gameState->map->getAllTowns())
 	{
 		auto town = gameState->getTown(townID);
@@ -658,9 +663,9 @@ void CGameStateCampaign::initTowns()
 
 		BuildingID newBuilding;
 		if(gameState->scenarioOps->campState->formatVCMI())
-			newBuilding = BuildingID(chosenBonus->info1);
+			newBuilding = bonusValue.building;
 		else
-			newBuilding = CBuildingHandler::campToERMU(chosenBonus->info1, town->getFactionID(), town->getBuildings());
+			newBuilding = CBuildingHandler::campToERMU(bonusValue.building, town->getFactionID(), town->getBuildings());
 
 		// Build granted building & all prerequisites - e.g. Mages Guild Lvl 3 should also give Mages Guild Lvl 1 & 2
 		while(true)
@@ -687,7 +692,7 @@ bool CGameStateCampaign::playerHasStartingHero(PlayerColor playerColor) const
 	if (!campaignBonus)
 		return false;
 
-	if(campaignBonus->type == CampaignBonusType::HERO && playerColor == PlayerColor(campaignBonus->info1))
+	if(campaignBonus->getType() == CampaignBonusType::HERO && playerColor == PlayerColor(campaignBonus->getValue<CampaignBonusStartingHero>().startingPlayer))
 		return true;
 	return false;
 }

lib/gameState/CGameStateCampaign.h

@@ -15,7 +15,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-struct CampaignBonus;
+class CampaignBonus;
 struct CampaignTravel;
 class CGHeroInstance;
 class CGameState;

lib/serializer/ESerializationVersion.h

@@ -46,8 +46,9 @@ enum class ESerializationVersion : int32_t
 	SERVER_STATISTICS, // statistics now only saved on server
 	OPPOSITE_SIDE_LIMITER_OWNER, // opposite side limiter no longer stores owner in itself
 	UNIVERSITY_CONFIG, // town university is configurable
+	CAMPAIGN_BONUSES, // new format for scenario bonuses in campaigns
 
-	CURRENT = UNIVERSITY_CONFIG,
+	CURRENT = CAMPAIGN_BONUSES,
 };
 
 static_assert(ESerializationVersion::MINIMAL <= ESerializationVersion::CURRENT, "Invalid serialization version definition!");

mapeditor/campaigneditor/scenarioproperties.cpp

@@ -218,11 +218,13 @@ void ScenarioProperties::reloadMapRelatedUi()
 			for(int i = 0; i < ui->comboBoxStartingBonusPlayerPosition->count(); ++i) // copy from player dropdown
 				comboBoxPlayer->addItem(ui->comboBoxStartingBonusPlayerPosition->itemText(i), ui->comboBoxStartingBonusPlayerPosition->itemData(i));
 
+			const auto & bonusValue = bonus.getValue<CampaignBonusHeroesFromScenario>();
+
 			// set selected
-			int index = comboBoxPlayer->findData(bonus.info1);
+			int index = comboBoxPlayer->findData(bonusValue.startingPlayer.getNum());
 			if(index != -1)
 				comboBoxPlayer->setCurrentIndex(index);
-			index = comboBoxOption->findData(bonus.info2);
+			index = comboBoxOption->findData(bonusValue.scenario.getNum());
 			if(index != -1)
 				comboBoxOption->setCurrentIndex(index);
 
@@ -337,12 +339,24 @@ void ScenarioProperties::on_buttonBox_clicked(QAbstractButton * button)
 		{
 			for (int i = 0; i < ui->tableWidgetStartingCrossover->rowCount(); ++i)
 			{
-				CampaignBonus bonus;
-				bonus.type = ui->radioButtonStartingOptionHeroCrossover->isChecked() ? CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO : CampaignBonusType::HERO;
 				QComboBox* comboBoxOption = qobject_cast<QComboBox*>(ui->tableWidgetStartingCrossover->cellWidget(i, 0));
 				QComboBox* comboBoxPlayer = qobject_cast<QComboBox*>(ui->tableWidgetStartingCrossover->cellWidget(i, 1));
-				bonus.info1 = comboBoxPlayer->currentData().toInt();
-				bonus.info2 = comboBoxOption->currentData().toInt();
+				CampaignBonus bonus;
+
+				if (ui->radioButtonStartingOptionHeroCrossover->isChecked())
+				{
+					bonus = CampaignBonusHeroesFromScenario{
+						PlayerColor(comboBoxPlayer->currentData().toInt()),
+						CampaignScenarioID(comboBoxOption->currentData().toInt())
+					};
+				}
+				else
+				{
+					bonus = CampaignBonusStartingHero{
+						PlayerColor(comboBoxPlayer->currentData().toInt()),
+						HeroTypeID(comboBoxOption->currentData().toInt())
+					};
+				}
 				campaignState->scenarios.at(scenario).travelOptions.bonusesToChoose.push_back(bonus);
 			}
 		}
@@ -476,8 +490,8 @@ void ScenarioProperties::on_pushButtonStartingAdd_clicked()
 	}
 	else
 	{
-		CampaignBonus bonus;
-		bonus.type = CampaignBonusType::SPELL;
+		CampaignBonus bonus = CampaignBonusSpell{ HeroTypeID(), SpellID() };
+
 		if(StartingBonus::showStartingBonus(PlayerColor(ui->comboBoxStartingBonusPlayerPosition->currentData().toInt()), map, bonus))
 		{
 			QListWidgetItem * item = new QListWidgetItem(StartingBonus::getBonusListTitle(bonus, map));

mapeditor/campaigneditor/startingbonus.cpp

@@ -139,131 +139,138 @@ void StartingBonus::loadBonus()
 			comboBox->setCurrentIndex(index);
 	};
 
-	switch (bonus.type)
+	switch(bonus.getType())
 	{
-	case CampaignBonusType::SPELL:
-		ui->radioButtonSpell->setChecked(true);
-		on_radioButtonSpell_toggled();
-		setComboBoxValue(ui->comboBoxSpellRecipient, bonus.info1);
-		setComboBoxValue(ui->comboBoxSpellSpell, bonus.info2);
-		break;
-	case CampaignBonusType::MONSTER:
-		ui->radioButtonCreature->setChecked(true);
-		on_radioButtonCreature_toggled();
-		setComboBoxValue(ui->comboBoxCreatureRecipient, bonus.info1);
-		setComboBoxValue(ui->comboBoxCreatureCreatureType, bonus.info2);
-		ui->spinBoxCreatureQuantity->setValue(bonus.info3);
-		break;
-	case CampaignBonusType::BUILDING:
-		ui->radioButtonBuilding->setChecked(true);
-		on_radioButtonBuilding_toggled();
-		setComboBoxValue(ui->comboBoxBuildingBuilding, bonus.info1);
-		break;
-	case CampaignBonusType::ARTIFACT:
-		ui->radioButtonArtifact->setChecked(true);
-		on_radioButtonArtifact_toggled();
-		setComboBoxValue(ui->comboBoxArtifactRecipient, bonus.info1);
-		setComboBoxValue(ui->comboBoxArtifactArtifact, bonus.info2);
-		break;
-	case CampaignBonusType::SPELL_SCROLL:
-		ui->radioButtonSpellScroll->setChecked(true);
-		on_radioButtonSpellScroll_toggled();
-		setComboBoxValue(ui->comboBoxSpellScrollRecipient, bonus.info1);
-		setComboBoxValue(ui->comboBoxSpellScrollSpell, bonus.info2);
-		break;
-	case CampaignBonusType::PRIMARY_SKILL:
-		ui->radioButtonPrimarySkill->setChecked(true);
-		on_radioButtonPrimarySkill_toggled();
-		setComboBoxValue(ui->comboBoxPrimarySkillRecipient, bonus.info1);
-		ui->spinBoxPrimarySkillAttack->setValue((bonus.info2 >> 0) & 0xff);
-		ui->spinBoxPrimarySkillDefense->setValue((bonus.info2 >> 8) & 0xff);
-		ui->spinBoxPrimarySkillSpell->setValue((bonus.info2 >> 16) & 0xff);
-		ui->spinBoxPrimarySkillKnowledge->setValue((bonus.info2 >> 24) & 0xff);
-		break;
-	case CampaignBonusType::SECONDARY_SKILL:
-		ui->radioButtonSecondarySkill->setChecked(true);
-		on_radioButtonSecondarySkill_toggled();
-		setComboBoxValue(ui->comboBoxSecondarySkillRecipient, bonus.info1);
-		setComboBoxValue(ui->comboBoxSecondarySkillSecondarySkill, bonus.info2);
-		setComboBoxValue(ui->comboBoxSecondarySkillMastery, bonus.info3);
-		break;
-	case CampaignBonusType::RESOURCE:
-		ui->radioButtonResource->setChecked(true);
-		on_radioButtonResource_toggled();
-		setComboBoxValue(ui->comboBoxResourceResourceType, bonus.info1);
-		ui->spinBoxResourceQuantity->setValue(bonus.info2);
-		break;
-	
-	default:
-		break;
+		case CampaignBonusType::SPELL:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusSpell>();
+			ui->radioButtonSpell->setChecked(true);
+			on_radioButtonSpell_toggled();
+			setComboBoxValue(ui->comboBoxSpellRecipient, bonusValue.hero.getNum());
+			setComboBoxValue(ui->comboBoxSpellSpell, bonusValue.spell.getNum());
+			break;
+		}
+		case CampaignBonusType::MONSTER:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusCreatures>();
+			ui->radioButtonCreature->setChecked(true);
+			on_radioButtonCreature_toggled();
+			setComboBoxValue(ui->comboBoxCreatureRecipient, bonusValue.hero.getNum());
+			setComboBoxValue(ui->comboBoxCreatureCreatureType, bonusValue.creature.getNum());
+			ui->spinBoxCreatureQuantity->setValue(bonusValue.amount);
+			break;
+		}
+		case CampaignBonusType::BUILDING:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusBuilding>();
+			ui->radioButtonBuilding->setChecked(true);
+			on_radioButtonBuilding_toggled();
+			setComboBoxValue(ui->comboBoxBuildingBuilding, bonusValue.building.getNum());
+			break;
+		}
+		case CampaignBonusType::ARTIFACT:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusArtifact>();
+			ui->radioButtonArtifact->setChecked(true);
+			on_radioButtonArtifact_toggled();
+			setComboBoxValue(ui->comboBoxArtifactRecipient, bonusValue.hero.getNum());
+			setComboBoxValue(ui->comboBoxArtifactArtifact, bonusValue.artifact.getNum());
+			break;
+		}
+		case CampaignBonusType::SPELL_SCROLL:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusSpellScroll>();
+			ui->radioButtonSpellScroll->setChecked(true);
+			on_radioButtonSpellScroll_toggled();
+			setComboBoxValue(ui->comboBoxSpellScrollRecipient, bonusValue.hero.getNum());
+			setComboBoxValue(ui->comboBoxSpellScrollSpell, bonusValue.spell.getNum());
+			break;
+		}
+		case CampaignBonusType::PRIMARY_SKILL:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusPrimarySkill>();
+			ui->radioButtonPrimarySkill->setChecked(true);
+			on_radioButtonPrimarySkill_toggled();
+			setComboBoxValue(ui->comboBoxPrimarySkillRecipient, bonusValue.hero.getNum());
+			ui->spinBoxPrimarySkillAttack->setValue(bonusValue.amounts[0]);
+			ui->spinBoxPrimarySkillDefense->setValue(bonusValue.amounts[0]);
+			ui->spinBoxPrimarySkillSpell->setValue(bonusValue.amounts[0]);
+			ui->spinBoxPrimarySkillKnowledge->setValue(bonusValue.amounts[0]);
+			break;
+		}
+		case CampaignBonusType::SECONDARY_SKILL:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusSecondarySkill>();
+			ui->radioButtonSecondarySkill->setChecked(true);
+			on_radioButtonSecondarySkill_toggled();
+			setComboBoxValue(ui->comboBoxSecondarySkillRecipient, bonusValue.hero.getNum());
+			setComboBoxValue(ui->comboBoxSecondarySkillSecondarySkill, bonusValue.skill.getNum());
+			setComboBoxValue(ui->comboBoxSecondarySkillMastery, bonusValue.mastery);
+			break;
+		}
+		case CampaignBonusType::RESOURCE:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusStartingResources>();
+			ui->radioButtonResource->setChecked(true);
+			on_radioButtonResource_toggled();
+			setComboBoxValue(ui->comboBoxResourceResourceType, bonusValue.resource.getNum());
+			ui->spinBoxResourceQuantity->setValue(bonusValue.amount);
+			break;
+		}
+
+		default:
+			break;
 	}
 }
 
 void StartingBonus::saveBonus()
 {
 	if(ui->radioButtonSpell->isChecked())
-		bonus.type = CampaignBonusType::SPELL;
+		bonus = CampaignBonusSpell{
+			HeroTypeID(ui->comboBoxSpellRecipient->currentData().toInt()),
+			SpellID(ui->comboBoxSpellSpell->currentData().toInt())
+		};
 	else if(ui->radioButtonCreature->isChecked())
-		bonus.type = CampaignBonusType::MONSTER;
+		bonus = CampaignBonusCreatures{
+			HeroTypeID(ui->comboBoxCreatureRecipient->currentData().toInt()),
+			CreatureID(ui->comboBoxCreatureCreatureType->currentData().toInt()),
+			int32_t(ui->spinBoxCreatureQuantity->value())
+		};
 	else if(ui->radioButtonBuilding->isChecked())
-		bonus.type = CampaignBonusType::BUILDING;
+		bonus = CampaignBonusBuilding{
+			BuildingID(ui->comboBoxBuildingBuilding->currentData().toInt())
+		};
 	else if(ui->radioButtonArtifact->isChecked())
-		bonus.type = CampaignBonusType::ARTIFACT;
+		bonus = CampaignBonusArtifact{
+			HeroTypeID(ui->comboBoxCreatureRecipient->currentData().toInt()),
+			ArtifactID(ui->comboBoxArtifactArtifact->currentData().toInt())
+		};
 	else if(ui->radioButtonSpellScroll->isChecked())
-		bonus.type = CampaignBonusType::SPELL_SCROLL;
+		bonus = CampaignBonusSpellScroll{
+			HeroTypeID(ui->comboBoxCreatureRecipient->currentData().toInt()),
+			SpellID(ui->comboBoxSpellScrollSpell->currentData().toInt())
+		};
 	else if(ui->radioButtonPrimarySkill->isChecked())
-		bonus.type = CampaignBonusType::PRIMARY_SKILL;
+		bonus = CampaignBonusPrimarySkill{
+			HeroTypeID(ui->comboBoxCreatureRecipient->currentData().toInt()),
+			{
+				uint8_t(ui->spinBoxPrimarySkillAttack->value()),
+				uint8_t(ui->spinBoxPrimarySkillDefense->value()),
+				uint8_t(ui->spinBoxPrimarySkillSpell->value()),
+				uint8_t(ui->spinBoxPrimarySkillKnowledge->value()),
+			}
+		};
 	else if(ui->radioButtonSecondarySkill->isChecked())
-		bonus.type = CampaignBonusType::SECONDARY_SKILL;
+		bonus = CampaignBonusSecondarySkill{
+			HeroTypeID(ui->comboBoxCreatureRecipient->currentData().toInt()),
+			SecondarySkill(ui->comboBoxSecondarySkillSecondarySkill->currentData().toInt()),
+			int32_t(ui->comboBoxSecondarySkillMastery->currentData().toInt())
+		};
 	else if(ui->radioButtonResource->isChecked())
-		bonus.type = CampaignBonusType::RESOURCE;
-
-	bonus.info1 = 0;
-	bonus.info2 = 0;
-	bonus.info3 = 0;
-
-	switch (bonus.type)
-	{
-	case CampaignBonusType::SPELL:
-		bonus.info1 = ui->comboBoxSpellRecipient->currentData().toInt();
-		bonus.info2 = ui->comboBoxSpellSpell->currentData().toInt();
-		break;
-	case CampaignBonusType::MONSTER:
-		bonus.info1 = ui->comboBoxCreatureRecipient->currentData().toInt();
-		bonus.info2 = ui->comboBoxCreatureCreatureType->currentData().toInt();
-		bonus.info3 = ui->spinBoxCreatureQuantity->value();
-		break;
-	case CampaignBonusType::BUILDING:
-		bonus.info1 = ui->comboBoxBuildingBuilding->currentData().toInt();
-		break;
-	case CampaignBonusType::ARTIFACT:
-		bonus.info1 = ui->comboBoxArtifactRecipient->currentData().toInt();
-		bonus.info2 = ui->comboBoxArtifactArtifact->currentData().toInt();
-		break;
-	case CampaignBonusType::SPELL_SCROLL:
-		bonus.info1 = ui->comboBoxSpellScrollRecipient->currentData().toInt();
-		bonus.info2 = ui->comboBoxSpellScrollSpell->currentData().toInt();
-		break;
-	case CampaignBonusType::PRIMARY_SKILL:
-		bonus.info1 = ui->comboBoxPrimarySkillRecipient->currentData().toInt();
-		bonus.info2 |= ui->spinBoxPrimarySkillAttack->value() << 0;
-		bonus.info2 |= ui->spinBoxPrimarySkillDefense->value() << 8;
-		bonus.info2 |= ui->spinBoxPrimarySkillSpell->value() << 16;
-		bonus.info2 |= ui->spinBoxPrimarySkillKnowledge->value() << 24;
-		break;
-	case CampaignBonusType::SECONDARY_SKILL:
-		bonus.info1 = ui->comboBoxSecondarySkillRecipient->currentData().toInt();
-		bonus.info2 = ui->comboBoxSecondarySkillSecondarySkill->currentData().toInt();
-		bonus.info3 = ui->comboBoxSecondarySkillMastery->currentData().toInt();
-		break;
-	case CampaignBonusType::RESOURCE:
-		bonus.info1 = ui->comboBoxResourceResourceType->currentData().toInt();
-		bonus.info2 = ui->spinBoxResourceQuantity->value();
-		break;
-	
-	default:
-		break;
-	}
+		bonus = CampaignBonusStartingResources{
+			GameResID(ui->comboBoxResourceResourceType->currentData().toInt()),
+			int32_t(ui->spinBoxResourceQuantity->value())
+		};
 }
 
 void StartingBonus::on_buttonBox_clicked(QAbstractButton * button)
@@ -309,40 +316,66 @@ QString StartingBonus::getBonusListTitle(CampaignBonus bonus, std::shared_ptr<CM
 		}
 		return QString::fromStdString(tmp.toString());
 	};
-	auto getSpellName = [](int id){
+	auto getSpellName = [](SpellID id){
 		MetaString tmp;
-		tmp.appendName(SpellID(id));
+		tmp.appendName(id);
 		return QString::fromStdString(tmp.toString());
 	};
-	auto getMonsterName = [](int id, int amount){
+	auto getMonsterName = [](CreatureID id, int amount){
 		MetaString tmp;
-		tmp.appendName(CreatureID(id), amount);
+		tmp.appendName(id, amount);
 		return QString::fromStdString(tmp.toString());
 	};
-	auto getArtifactName = [](int id){
+	auto getArtifactName = [](ArtifactID id){
 		MetaString tmp;
-		tmp.appendName(ArtifactID(id));
+		tmp.appendName(id);
 		return QString::fromStdString(tmp.toString());
 	};
 
-	switch (bonus.type)
+	switch(bonus.getType())
 	{
-	case CampaignBonusType::SPELL:
-		return tr("%1 spell for %2").arg(getSpellName(bonus.info2)).arg(getHeroName(bonus.info1));
-	case CampaignBonusType::MONSTER:
-		return tr("%1 %2 for %3").arg(bonus.info3).arg(getMonsterName(bonus.info2, bonus.info3)).arg(getHeroName(bonus.info1));
-	case CampaignBonusType::BUILDING:
-		return tr("Building");
-	case CampaignBonusType::ARTIFACT:
-		return tr("%1 artifact for %2").arg(getArtifactName(bonus.info2)).arg(getHeroName(bonus.info1));
-	case CampaignBonusType::SPELL_SCROLL:
-		return tr("%1 spell scroll for %2").arg(getSpellName(bonus.info2)).arg(getHeroName(bonus.info1));
-	case CampaignBonusType::PRIMARY_SKILL:
-		return tr("Primary skill (Attack: %1, Defense: %2, Spell: %3, Knowledge: %4) for %5").arg((bonus.info2 >> 0) & 0xff).arg((bonus.info2 >> 8) & 0xff).arg((bonus.info2 >> 16) & 0xff).arg((bonus.info2 >> 24) & 0xff).arg(getHeroName(bonus.info1));
-	case CampaignBonusType::SECONDARY_SKILL:
-		return tr("Secondary skill");
-	case CampaignBonusType::RESOURCE:
-		return tr("Resource");
+		case CampaignBonusType::SPELL:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusSpell>();
+			return tr("%1 spell for %2").arg(getSpellName(bonusValue.spell)).arg(getHeroName(bonusValue.hero));
+		}
+		case CampaignBonusType::MONSTER:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusCreatures>();
+			return tr("%1 %2 for %3").arg(bonusValue.amount).arg(getMonsterName(bonusValue.creature, bonusValue.amount)).arg(getHeroName(bonusValue.hero));
+		}
+		case CampaignBonusType::BUILDING:
+		{
+			return tr("Building");
+		}
+		case CampaignBonusType::ARTIFACT:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusArtifact>();
+			return tr("%1 artifact for %2").arg(getArtifactName(bonusValue.artifact)).arg(getHeroName(bonusValue.hero));
+		}
+		case CampaignBonusType::SPELL_SCROLL:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusSpellScroll>();
+			return tr("%1 spell scroll for %2").arg(getSpellName(bonusValue.spell)).arg(getHeroName(bonusValue.hero));
+		}
+		case CampaignBonusType::PRIMARY_SKILL:
+		{
+			const auto & bonusValue = bonus.getValue<CampaignBonusPrimarySkill>();
+			return tr("Primary skill (Attack: %1, Defense: %2, Spell: %3, Knowledge: %4) for %5")
+				.arg(bonusValue.amounts[0])
+				.arg(bonusValue.amounts[1])
+				.arg(bonusValue.amounts[2])
+				.arg(bonusValue.amounts[3])
+				.arg(getHeroName(bonusValue.hero));
+		}
+		case CampaignBonusType::SECONDARY_SKILL:
+		{
+			return tr("Secondary skill");
+		}
+		case CampaignBonusType::RESOURCE:
+		{
+			return tr("Resource");
+		}
 	}
 	return {};
 }

mapeditor/campaigneditor/startingbonus.h

@@ -13,7 +13,7 @@
 #include "lib/constants/EntityIdentifiers.h"
 #include "lib/campaign/CampaignState.h"
 
-struct CampaignBonus;
+class CampaignBonus;
 class CMap;
 
 namespace Ui {

server/CVCMIServer.cpp

@@ -832,12 +832,16 @@ void CVCMIServer::setCampaignBonus(int bonusId)
 	campaignBonus = bonusId;
 
 	const CampaignScenario & scenario = si->campState->scenario(campaignMap);
-	const std::vector<CampaignBonus> & bonDescs = scenario.travelOptions.bonusesToChoose;
-	if(bonDescs[bonusId].type == CampaignBonusType::HERO || bonDescs[bonusId].type == CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO)
+	const CampaignBonus & bonus = scenario.travelOptions.bonusesToChoose.at(bonusId);
+	if(bonus.getType() == CampaignBonusType::HERO || bonus.getType() == CampaignBonusType::HEROES_FROM_PREVIOUS_SCENARIO)
 	{
+		PlayerColor startingPlayer = bonus.getType() == CampaignBonusType::HERO ?
+			bonus.getValue<CampaignBonusStartingHero>().startingPlayer :
+			bonus.getValue<CampaignBonusHeroesFromScenario>().startingPlayer;
+
 		for(auto & elem : si->playerInfos)
 		{
-			if(elem.first == PlayerColor(bonDescs[bonusId].info1))
+			if(elem.first == startingPlayer)
 				setPlayerConnectedId(elem.second, 1);
 			else
 				setPlayerConnectedId(elem.second, PlayerSettings::PLAYER_AI);