Multiple minor fixes:

- fixed starting armies of some heroes, including Bron #1602
- (debian) added vcmi-dbg package with debug information
- proper randomization of hero secondary skills, fixes #1603
- second capitol will show up as disabled immediately #1604
- proper deserialization of boost::variant
- empty file will no longer crash JsonNode parser
- fixed some compiler warnings

config/heroes/fortress.json

@@ -12,6 +12,21 @@
 		"specialties":
 		[
 			{ "type":10, "val": 1, "subtype": 5, "info": 0 }
+		],
+		"army" :
+		[
+			{
+				"min" : 10, "max" : 20,
+				"creature" : "gnoll"
+			},
+			{
+				"min" : 4, "max" : 7,
+				"creature" : "basilisk"
+			},
+			{
+				"min" : 2, "max" : 4,
+				"creature" : "serpentFly"
+			}
 		]
 	},
 	"drakon":

config/heroes/special.json

@@ -170,12 +170,15 @@
 		"army" :
 		[
 			{
+				"min" : 10, "max" : 20,
 				"creature" : "troglodyte"
 			},
 			{
+				"min" : 4, "max" : 7,
 				"creature" : "harpy"
 			},
 			{
+				"min" : 2, "max" : 3,
 				"creature" : "evilEye"
 			}
 		]
@@ -198,12 +201,15 @@
 		"army" :
 		[
 			{
+				"min" : 10, "max" : 20,
 				"creature" : "goblin"
 			},
 			{
+				"min" : 4, "max" : 7,
 				"creature" : "goblinWolfRider"
 			},
 			{
+				"min" : 2, "max" : 3,
 				"creature" : "orc"
 			}
 		]
@@ -228,12 +234,15 @@
 		"army" :
 		[
 			{
+				"min" : 10, "max" : 20,
 				"creature" : "imp"
 			},
 			{
+				"min" : 4, "max" : 7,
 				"creature" : "hellHound"
 			},
 			{
+				"min" : 2, "max" : 3,
 				"creature" : "hellHound"
 			}
 		]

debian/control

@@ -17,3 +17,9 @@ Description: Rewrite of the Heroes of Might and Magic 3 engine
  .
  In its current state it already supports maps of any sizes, higher
  resolutions and extended engine limits.
+
+Package: vcmi-dbg
+Architecture: any
+Section: debug
+Depends: vcmi (= ${binary:Version}),    ${misc:Depends} 
+Description: Debug symbols for vcmi package

debian/rules

@@ -6,3 +6,11 @@
 # override disabled by default rpath - we need to find libvcmi.so with it:
 override_dh_auto_configure:
 	dh_auto_configure -- -DCMAKE_SKIP_RPATH=OFF -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBIN_DIR=games
+.PHONY: override_dh_strip
+override_dh_strip:
+	dh_strip --dbg-package=vcmi-dbg
+override_dh_auto_install:
+	dh_auto_install --destdir=debian/vcmi
+override_dh_installdocs:
+	dh_installdocs --link-doc=vcmi
+

lib/CGameState.cpp

@@ -2957,8 +2957,8 @@ DuelParameters DuelParameters::fromJSON(const std::string &fname)
 		else
 			ss.heroId = -1;
 
-		for(const JsonNode &n : n["heroPrimSkills"].Vector())
-			ss.heroPrimSkills.push_back(n.Float());
+		for(const JsonNode &entry : n["heroPrimSkills"].Vector())
+			ss.heroPrimSkills.push_back(entry.Float());
 
 		for(const JsonNode &skillNode : n["heroSecSkills"].Vector())
 		{

lib/CHeroHandler.cpp

@@ -22,7 +22,7 @@
  *
  */
 
-SecondarySkill CHeroClass::chooseSecSkill(const std::set<SecondarySkill> & possibles) const //picks secondary skill out from given possibilities
+SecondarySkill CHeroClass::chooseSecSkill(const std::set<SecondarySkill> & possibles, std::minstd_rand & distr) const //picks secondary skill out from given possibilities
 {
 	int totalProb = 0;
 	for(auto & possible : possibles)
@@ -31,7 +31,7 @@ SecondarySkill CHeroClass::chooseSecSkill(const std::set<SecondarySkill> & possi
 	}
 	if (totalProb != 0) // may trigger if set contains only banned skills (0 probability)
 	{
-		int ran = rand()%totalProb;
+		int ran = distr()%totalProb;
 		for(auto & possible : possibles)
 		{
 			ran -= secSkillProbability[possible];

lib/CHeroHandler.h

@@ -121,7 +121,7 @@ public:
 	std::string imageMapFemale;
 
 	bool isMagicHero() const;
-	SecondarySkill chooseSecSkill(const std::set<SecondarySkill> & possibles) const; //picks secondary skill out from given possibilities
+	SecondarySkill chooseSecSkill(const std::set<SecondarySkill> & possibles, std::minstd_rand & distr) const; //picks secondary skill out from given possibilities
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{

lib/CObjectHandler.cpp

@@ -1016,7 +1016,7 @@ void CGHeroInstance::initObj()
 	if(!type)
 		initHero(); //TODO: set up everything for prison before specialties are configured
 
-	skillsInfo.randomSeed = rand();
+	skillsInfo.distribution.seed(rand());
 	skillsInfo.resetMagicSchoolCounter();
 	skillsInfo.resetWisdomCounter();
 
@@ -1661,14 +1661,19 @@ ArtBearer::ArtBearer CGHeroInstance::bearerType() const
 std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
 {
 	std::vector<SecondarySkill> obligatorySkills; //hero is offered magic school or wisdom if possible
+	if (!skillsInfo.wisdomCounter)
+	{
+		if (cb->isAllowed(2, SecondarySkill::WISDOM) && !getSecSkillLevel(SecondarySkill::WISDOM))
+			obligatorySkills.push_back(SecondarySkill::WISDOM);
+	}
 	if (!skillsInfo.magicSchoolCounter)
 	{
 		std::vector<SecondarySkill> ss;
 		ss += SecondarySkill::FIRE_MAGIC, SecondarySkill::WATER_MAGIC, SecondarySkill::EARTH_MAGIC, SecondarySkill::EARTH_MAGIC;
 
-		auto rng = [=](ui32 val)-> ui32
+		auto rng = [=](ui32 val) mutable -> ui32
 		{
-			return skillsInfo.randomSeed % val; //must be determined
+			return skillsInfo.distribution() % val; //must be determined
 		};
 		std::random_shuffle(ss.begin(), ss.end(), rng);
 
@@ -1681,11 +1686,6 @@ std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
 			}
 		}
 	}
-	if (!skillsInfo.wisdomCounter)
-	{
-		if (cb->isAllowed(2, SecondarySkill::WISDOM) && !getSecSkillLevel(SecondarySkill::WISDOM))
-			obligatorySkills.push_back(SecondarySkill::WISDOM);
-	}
 
 	std::vector<SecondarySkill> skills;
 	//picking sec. skills for choice
@@ -1696,7 +1696,7 @@ std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
 
 	for(auto & elem : secSkills)
 	{
-		if(elem.second < 3)
+		if(elem.second < SecSkillLevel::EXPERT)
 			basicAndAdv.insert(elem.first);
 		else
 			expert.insert(elem.first);
@@ -1709,35 +1709,41 @@ std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
 		expert.erase (s);
 	}
 
-	//first offered skill
-	if (canLearnSkill() && obligatorySkills.size()) //offer always if possible
+	//first offered skill:
+	// 1) give obligatory skill
+	// 2) give any other new skill
+	// 3) upgrade existing
+	if (canLearnSkill() && obligatorySkills.size() > 0)
 	{
 		skills.push_back (obligatorySkills[0]);
 	}
-	else if(!basicAndAdv.empty())
+	else if(none.size() && canLearnSkill()) //hero have free skill slot
 	{
-		SecondarySkill s = type->heroClass->chooseSecSkill(basicAndAdv);//upgrade existing
-		skills.push_back(s);
-		basicAndAdv.erase(s);
+		skills.push_back(type->heroClass->chooseSecSkill(none, skillsInfo.distribution)); //new skill
 	}
-	else if(none.size() && canLearnSkill())
+	else if(!basicAndAdv.empty())
 	{
-		skills.push_back(type->heroClass->chooseSecSkill(none)); //give new skill
-		none.erase(skills.back());
+		skills.push_back(type->heroClass->chooseSecSkill(basicAndAdv, skillsInfo.distribution)); //upgrade existing
 	}
 
-	//second offered skill
-	if (canLearnSkill() && obligatorySkills.size() > 1)
+	//second offered skill:
+	//1) upgrade existing
+	//2) give obligatory skill
+	//3) give any other new skill
+	if(!basicAndAdv.empty())
 	{
-		skills.push_back (obligatorySkills[1]);
+		SecondarySkill s = type->heroClass->chooseSecSkill(basicAndAdv, skillsInfo.distribution);//upgrade existing
+		skills.push_back(s);
+		basicAndAdv.erase(s);
 	}
-	else if(none.size() && canLearnSkill()) //hero have free skill slot
+	else if (canLearnSkill() && obligatorySkills.size() > 1)
 	{
-		skills.push_back(type->heroClass->chooseSecSkill(none)); //new skill
+		skills.push_back (obligatorySkills[1]);
 	}
-	else if(!basicAndAdv.empty())
+	else if(none.size() && canLearnSkill())
 	{
-		skills.push_back(type->heroClass->chooseSecSkill(basicAndAdv)); //upgrade existing
+		skills.push_back(type->heroClass->chooseSecSkill(none, skillsInfo.distribution)); //give new skill
+		none.erase(skills.back());
 	}
 
 	return skills;

lib/CObjectHandler.h

@@ -350,7 +350,9 @@ public:
 
 	struct DLL_LINKAGE SecondarySkillsInfo
 	{
-		ui32 randomSeed; //skills are determined, initialized at map start
+		//skills are determined, initialized at map start
+		//FIXME: remove mutable?
+		mutable std::minstd_rand distribution;
 		ui8 magicSchoolCounter;
 		ui8 wisdomCounter;
 
@@ -359,7 +361,21 @@ public:
 
 		template <typename Handler> void serialize(Handler &h, const int version)
 		{
-			h & randomSeed & magicSchoolCounter & wisdomCounter;
+			h & magicSchoolCounter & wisdomCounter;
+			if (h.saving)
+			{
+				std::ostringstream stream;
+				stream << distribution;
+				std::string str = stream.str();
+				h & str;
+			}
+			else
+			{
+				std::string str;
+				h & str;
+				std::istringstream stream(str);
+				stream >> distribution;
+			}
 		}
 	} skillsInfo;
 

lib/Connection.h

@@ -19,6 +19,7 @@
 #include <boost/mpl/equal_to.hpp>
 #include <boost/mpl/int.hpp>
 #include <boost/mpl/identity.hpp>
+#include <boost/mpl/for_each.hpp>
 #include <boost/any.hpp>
 
 #include "ConstTransitivePtr.h"
@@ -248,6 +249,30 @@ struct LoadWrong
 	}
 };
 
+template<typename Variant, typename Source>
+struct VariantLoaderHelper
+{
+	Source & source;
+	std::vector<std::function<Variant()>> funcs;
+
+	VariantLoaderHelper(Source & source):
+		source(source)
+	{
+		mpl::for_each<typename Variant::types>(std::ref(*this));
+	}
+
+	template<typename Type>
+	void operator()(Type)
+	{
+		funcs.push_back([&]() -> Variant
+		{
+			Type obj;
+			source >> obj;
+			return Variant(obj);
+		});
+	}
+};
+
 template<typename T>
 struct SerializationLevel
 {
@@ -1179,27 +1204,20 @@ public:
 		data.resize(length);
 		this->This()->read((void*)data.c_str(),length);
 	}
+
 	template <BOOST_VARIANT_ENUM_PARAMS(typename T)>
 	void loadSerializable(boost::variant<BOOST_VARIANT_ENUM_PARAMS(T)> &data)
 	{
+		typedef boost::variant<BOOST_VARIANT_ENUM_PARAMS(T)> TVariant;
+
+		VariantLoaderHelper<TVariant, CISer> loader(*this);
+
 		si32 which;
 		*this >> which;
-		if(which == 0)
-		{
-			T0 obj;
-			*this >> obj;
-			data = obj;
-		}
-		else if(which == 1)
-		{
-			T1 obj;
-			*this >> obj;
-			data = obj;
-		}
-		else
-			assert(0);
-		//TODO write more if needed, general solution would be much longer
+		assert(which < loader.funcs.size());
+		data = loader.funcs.at(which)();
 	}
+
 	template <typename T>
 	void loadSerializable(boost::optional<T> & data)
 	{

lib/IGameCallback.cpp

@@ -580,20 +580,6 @@ EBuildingState::EBuildingState CGameInfoCallback::canBuildStructure( const CGTow
 	if(vstd::contains(t->forbiddenBuildings, ID))
 		return EBuildingState::FORBIDDEN; //forbidden
 
-	auto buildTest = [&](const BuildingID & id)
-	{
-		return t->hasBuilt(id);
-	};
-
-	if(t->builded >= VLC->modh->settings.MAX_BUILDING_PER_TURN)
-		return EBuildingState::CANT_BUILD_TODAY; //building limit
-
-	if (!building->requirements.test(buildTest))
-		return EBuildingState::PREREQUIRES;
-
-	if (building->upgrade != BuildingID::NONE && !t->hasBuilt(building->upgrade))
-		return EBuildingState::MISSING_BASE;
-
 	if(ID == BuildingID::CAPITOL)
 	{
 		const PlayerState *ps = getPlayer(t->tempOwner);
@@ -612,10 +598,24 @@ EBuildingState::EBuildingState CGameInfoCallback::canBuildStructure( const CGTow
 	{
 		const TerrainTile *tile = getTile(t->bestLocation(), false);
 
-        if(!tile || tile->terType != ETerrainType::WATER)
+		if(!tile || tile->terType != ETerrainType::WATER)
 			return EBuildingState::NO_WATER; //lack of water
 	}
 
+	auto buildTest = [&](const BuildingID & id)
+	{
+		return t->hasBuilt(id);
+	};
+
+	if(t->builded >= VLC->modh->settings.MAX_BUILDING_PER_TURN)
+		return EBuildingState::CANT_BUILD_TODAY; //building limit
+
+	if (!building->requirements.test(buildTest))
+		return EBuildingState::PREREQUIRES;
+
+	if (building->upgrade != BuildingID::NONE && !t->hasBuilt(building->upgrade))
+		return EBuildingState::MISSING_BASE;
+
 	//checking resources
 	if(!building->resources.canBeAfforded(getPlayer(t->tempOwner)->resources))
 		return EBuildingState::NO_RESOURCES; //lack of res

lib/JsonDetail.cpp

@@ -131,15 +131,22 @@ JsonNode JsonParser::parse(std::string fileName)
 {
 	JsonNode root;
 
-	if (!Unicode::isValidString(&input[0], input.size()))
-		error("Not a valid UTF-8 file", false);
+	if (input.size() == 0)
+	{
+		error("File is empty", false);
+	}
+	else
+	{
+		if (!Unicode::isValidString(&input[0], input.size()))
+			error("Not a valid UTF-8 file", false);
 
-	extractValue(root);
-	extractWhitespace(false);
+		extractValue(root);
+		extractWhitespace(false);
 
-	//Warn if there are any non-whitespace symbols left
-	if (pos < input.size())
-		error("Not all file was parsed!", true);
+		//Warn if there are any non-whitespace symbols left
+		if (pos < input.size())
+			error("Not all file was parsed!", true);
+	}
 
 	if (!errors.empty())
 	{

server/CGameHandler.cpp

@@ -205,9 +205,9 @@ void CGameHandler::levelUpHero(const CGHeroInstance * hero)
 	else if(hlu.skills.size() == 1  ||  hero->tempOwner == PlayerColor::NEUTRAL)  //choose skill automatically
 	{
 		sendAndApply(&hlu);
-		auto rng = [&]()-> ui32
+		auto rng = [&]() mutable -> ui32
 		{
-			return hero->skillsInfo.randomSeed; //must be determined
+			return hero->skillsInfo.distribution(); //must be determined
 		};
 		levelUpHero(hero, vstd::pickRandomElementOf (hlu.skills, rng));
 	}