- random number generation refactoring

- fixed mantis #1743

.gitignore

@@ -7,3 +7,4 @@
 *.layout
 *.pro.user
 *.pro.user.*
+/CMakeLists.txt.user

AI/EmptyAI/CEmptyAI.cpp

@@ -1,6 +1,8 @@
 #include "StdInc.h"
 #include "CEmptyAI.h"
 
+#include "CRandomGenerator.h"
+
 void CEmptyAI::init(shared_ptr<CCallback> CB)
 {
 	cb = CB;
@@ -15,12 +17,12 @@ void CEmptyAI::yourTurn()
 
 void CEmptyAI::heroGotLevel(const CGHeroInstance *hero, PrimarySkill::PrimarySkill pskill, std::vector<SecondarySkill> &skills, QueryID queryID)
 {
-	cb->selectionMade(rand() % skills.size(), queryID);
+	cb->selectionMade(CRandomGenerator::getDefault().nextInt(skills.size() - 1), queryID);
 }
 
 void CEmptyAI::commanderGotLevel(const CCommanderInstance * commander, std::vector<ui32> skills, QueryID queryID)
 {
-	cb->selectionMade(rand() % skills.size(), queryID);
+	cb->selectionMade(CRandomGenerator::getDefault().nextInt(skills.size() - 1), queryID);
 }
 
 void CEmptyAI::showBlockingDialog(const std::string &text, const std::vector<Component> &components, QueryID askID, const int soundID, bool selection, bool cancel)

AI/StupidAI/StupidAI.cpp

@@ -104,8 +104,9 @@ BattleAction CStupidAI::activeStack( const CStack * stack )
 	if(stack->type->idNumber == CreatureID::CATAPULT)
 	{
 		BattleAction attack;
-		static const int wallHexes[] = {50, 183, 182, 130, 62, 29, 12, 95};
-		attack.destinationTile = wallHexes[ rand()%ARRAY_COUNT(wallHexes) ];
+		static const std::vector<int> wallHexes = boost::assign::list_of(50)(183)(182)(130)(62)(29)(12)(95);
+
+		attack.destinationTile = *RandomGeneratorUtil::nextItem(wallHexes, CRandomGenerator::getDefault());
 		attack.actionType = Battle::CATAPULT;
 		attack.additionalInfo = 0;
 		attack.side = side;

Global.h

@@ -553,19 +553,6 @@ namespace vstd
 		});
 	}
 
-	static inline int retreiveRandNum(const std::function<int()> &randGen)
-	{
-		if (randGen)
-			return randGen();
-		else
-			return rand();
-	}
-
-	template <typename T> const T & pickRandomElementOf(const std::vector<T> &v, const std::function<int()> &randGen)
-	{
-		return v.at(retreiveRandNum(randGen) % v.size());
-	}
-
 	template<typename T>
 	void advance(T &obj, int change)
 	{

client/CAnimation.cpp

@@ -4,6 +4,7 @@
 #include "../lib/filesystem/Filesystem.h"
 #include "../lib/filesystem/ISimpleResourceLoader.h"
 #include "../lib/JsonNode.h"
+#include "../lib/CRandomGenerator.h"
 
 #include "CBitmapHandler.h"
 #include "Graphics.h"
@@ -1432,7 +1433,7 @@ void CCreatureAnim::loopPreview(bool warMachine)
 		if (anim.size(elem))
 			available.push_back(elem);
 
-	size_t rnd = rand()%(available.size()*2);
+	size_t rnd = CRandomGenerator::getDefault().nextInt(available.size() * 2 - 1);
 
 	if (rnd >= available.size())
 	{

client/CMusicHandler.cpp

@@ -9,6 +9,7 @@
 #include "../lib/GameConstants.h"
 #include "../lib/filesystem/Filesystem.h"
 #include "../lib/StringConstants.h"
+#include "../lib/CRandomGenerator.h"
 
 /*
  * CMusicHandler.cpp, part of VCMI engine
@@ -230,7 +231,7 @@ int CSoundHandler::playSound(std::string sound, int repeats)
 // Helper. Randomly select a sound from an array and play it
 int CSoundHandler::playSoundFromSet(std::vector<soundBase::soundID> &sound_vec)
 {
-	return playSound(sound_vec[rand() % sound_vec.size()]);
+	return playSound(*RandomGeneratorUtil::nextItem(sound_vec, CRandomGenerator::getDefault()));
 }
 
 void CSoundHandler::stopSound( int handler )
@@ -504,10 +505,7 @@ bool MusicEntry::play()
 	if (!setName.empty())
 	{
 		auto set = owner->musicsSet[setName];
-		size_t entryID = rand() % set.size();
-		auto iterator = set.begin();
-		std::advance(iterator, entryID);
-		load(iterator->second);
+		load(RandomGeneratorUtil::nextItem(set, CRandomGenerator::getDefault())->second);
 	}
 
     logGlobal->traceStream()<<"Playing music file "<<currentName;

client/CPreGame.cpp

@@ -43,6 +43,7 @@
 #include "gui/CIntObjectClasses.h"
 #include "../lib/mapping/CMapService.h"
 #include "../lib/mapping/CMap.h"
+#include "../lib/CRandomGenerator.h"
 
 /*
  * CPreGame.cpp, part of VCMI engine
@@ -607,7 +608,7 @@ CSelectionScreen::CSelectionScreen(CMenuScreen::EState Type, CMenuScreen::EMulti
 		bordered = true;
 		//load random background
 		const JsonVector & bgNames = CGPreGameConfig::get().getConfig()["game-select"].Vector();
-		bg = new CPicture(bgNames[rand() % bgNames.size()].String(), 0, 0);
+		bg = new CPicture(RandomGeneratorUtil::nextItem(bgNames, CRandomGenerator::getDefault())->String(), 0, 0);
 		pos = bg->center();
 	}
 
@@ -4136,7 +4137,7 @@ std::string CLoadingScreen::getBackground()
 	}
 	else
 	{
-		return conf[ rand() % conf.size() ].String();
+		return RandomGeneratorUtil::nextItem(conf, CRandomGenerator::getDefault())->String();
 	}
 }
 

client/battle/CBattleInterface.cpp

@@ -26,6 +26,7 @@
 #include "../CVideoHandler.h"
 #include "../../lib/CTownHandler.h"
 #include "../../lib/mapping/CMap.h"
+#include "../../lib/CRandomGenerator.h"
 
 #include "CBattleAnimations.h"
 #include "CBattleInterfaceClasses.h"
@@ -59,7 +60,7 @@ static void onAnimationFinished(const CStack *stack, CCreatureAnimation * anim)
 
 		if (anim->framesInGroup(CCreatureAnim::MOUSEON) > 0)
 		{
-			if (float(rand() % 100) < creature->animation.timeBetweenFidgets * 10)
+			if (CRandomGenerator::getDefault().nextDouble(99.0) < creature->animation.timeBetweenFidgets * 10)
 				anim->playOnce(CCreatureAnim::MOUSEON);
 			else
 				anim->setType(CCreatureAnim::HOLDING);
@@ -194,7 +195,7 @@ CBattleInterface::CBattleInterface(const CCreatureSet * army1, const CCreatureSe
 			logGlobal->errorStream() << bfieldType << " battlefield type does not have any backgrounds!";
 		else
 		{
-			const std::string bgName = vstd::pickRandomElementOf(graphics->battleBacks[bfieldType], rand);
+			const std::string bgName = *RandomGeneratorUtil::nextItem(graphics->battleBacks[bfieldType], CRandomGenerator::getDefault());
 			background = BitmapHandler::loadBitmap(bgName, false);
 		}
 	}

client/mapHandler.cpp

@@ -17,6 +17,7 @@
 #include "../lib/GameConstants.h"
 #include "../lib/CStopWatch.h"
 #include "CMT.h"
+#include "../lib/CRandomGenerator.h"
 
 /*
  * mapHandler.cpp, part of VCMI engine
@@ -129,7 +130,7 @@ void CMapHandler::prepareFOWDefs()
 			elem[j].resize(sizes.z);
 			for(int k = 0; k < sizes.z; ++k)
 			{
-				elem[j][k] = rand()%graphics->FoWfullHide->ourImages.size();
+				elem[j][k] = CRandomGenerator::getDefault().nextInt(graphics->FoWfullHide->ourImages.size() - 1);
 			}
 		}
 	}
@@ -199,6 +200,8 @@ void CMapHandler::borderAndTerrainBitmapInit()
 				{
 					int terBitmapNum = -1;
 
+					auto & rand = CRandomGenerator::getDefault();
+
 					if(i==-1 && j==-1)
 						terBitmapNum = 16;
 					else if(i==-1 && j==(sizes.y))
@@ -208,15 +211,15 @@ void CMapHandler::borderAndTerrainBitmapInit()
 					else if(i==(sizes.x) && j==(sizes.y))
 						terBitmapNum = 18;
 					else if(j == -1 && i > -1 && i < sizes.y)
-						terBitmapNum = 22+rand()%2;
+						terBitmapNum = rand.nextInt(22, 23);
 					else if(i == -1 && j > -1 && j < sizes.y)
-						terBitmapNum = 33+rand()%2;
+						terBitmapNum = rand.nextInt(33, 34);
 					else if(j == sizes.y && i >-1 && i < sizes.x)
-						terBitmapNum = 29+rand()%2;
+						terBitmapNum = rand.nextInt(29, 30);
 					else if(i == sizes.x && j > -1 && j < sizes.y)
-						terBitmapNum = 25+rand()%2;
+						terBitmapNum = rand.nextInt(25, 26);
 					else
-						terBitmapNum = rand()%16;
+						terBitmapNum = rand.nextInt(15);
 
 					if(terBitmapNum != -1)
 					{
@@ -1083,7 +1086,7 @@ ui8 CMapHandler::getPhaseShift(const CGObjectInstance *object) const
 	auto i = animationPhase.find(object);
 	if(i == animationPhase.end())
 	{
-		ui8 ret = rand() % 255;
+		ui8 ret = CRandomGenerator::getDefault().nextInt(254);
 		animationPhase[object] = ret;
 		return ret;
 	}

lib/BattleState.cpp

@@ -87,7 +87,7 @@ std::pair< std::vector<BattleHex>, int > BattleInfo::getPath(BattleHex start, Ba
 }
 
 ui32 BattleInfo::calculateDmg( const CStack* attacker, const CStack* defender, const CGHeroInstance * attackerHero, const CGHeroInstance * defendingHero,
-	bool shooting, ui8 charge, bool lucky, bool unlucky, bool deathBlow, bool ballistaDoubleDmg )
+	bool shooting, ui8 charge, bool lucky, bool unlucky, bool deathBlow, bool ballistaDoubleDmg, CRandomGenerator & rand )
 {
 	TDmgRange range = calculateDmgRange(attacker, defender, shooting, charge, lucky, unlucky, deathBlow, ballistaDoubleDmg);
 
@@ -97,7 +97,7 @@ ui32 BattleInfo::calculateDmg( const CStack* attacker, const CStack* defender, c
 		int howManyToAv = std::min<ui32>(10, attacker->count);
 		for (int g=0; g<howManyToAv; ++g)
 		{
-			valuesToAverage[g] = range.first  +  rand() % (range.second - range.first + 1);
+			valuesToAverage[g] = rand.nextInt(range.first, range.second);
 		}
 
 		return std::accumulate(valuesToAverage, valuesToAverage + howManyToAv, 0) / howManyToAv;
@@ -723,7 +723,9 @@ const CGHeroInstance * BattleInfo::getHero( PlayerColor player ) const
 	return nullptr;
 }
 
-std::vector<ui32> BattleInfo::calculateResistedStacks(const CSpell * sp, const CGHeroInstance * caster, const CGHeroInstance * hero2, const std::vector<const CStack*> & affectedCreatures, PlayerColor casterSideOwner, ECastingMode::ECastingMode mode, int usedSpellPower, int spellLevel) const
+std::vector<ui32> BattleInfo::calculateResistedStacks(const CSpell * sp, const CGHeroInstance * caster, const CGHeroInstance * hero2,
+	const std::vector<const CStack*> & affectedCreatures, PlayerColor casterSideOwner, ECastingMode::ECastingMode mode,
+	int usedSpellPower, int spellLevel, CRandomGenerator & rand) const
 {
 	std::vector<ui32> ret;
 	for(auto & affectedCreature : affectedCreatures)
@@ -749,8 +751,11 @@ std::vector<ui32> BattleInfo::calculateResistedStacks(const CSpell * sp, const C
 
 		if(prob > 100) prob = 100;
 
-		if(rand()%100 < prob) //immunity from resistance
+		//immunity from resistance
+		if(rand.nextInt(99) < prob)
+		{
 			ret.push_back((affectedCreature)->ID);
+		}
 
 	}
 

lib/BattleState.h

@@ -129,7 +129,7 @@ struct DLL_LINKAGE BattleInfo : public CBonusSystemNode, public CBattleInfoCallb
 	shared_ptr<CObstacleInstance> getObstacleOnTile(BattleHex tile) const;
 	std::set<BattleHex> getStoppers(bool whichSidePerspective) const;
 
-	ui32 calculateDmg(const CStack* attacker, const CStack* defender, const CGHeroInstance * attackerHero, const CGHeroInstance * defendingHero, bool shooting, ui8 charge, bool lucky, bool unlucky, bool deathBlow, bool ballistaDoubleDmg); //charge - number of hexes travelled before attack (for champion's jousting)
+	ui32 calculateDmg(const CStack* attacker, const CStack* defender, const CGHeroInstance * attackerHero, const CGHeroInstance * defendingHero, bool shooting, ui8 charge, bool lucky, bool unlucky, bool deathBlow, bool ballistaDoubleDmg, CRandomGenerator & rand); //charge - number of hexes travelled before attack (for champion's jousting)
 	void calculateCasualties(std::map<ui32,si32> *casualties) const; //casualties are array of maps size 2 (attacker, defeneder), maps are (crid => amount)
 
 	//void getPotentiallyAttackableHexes(AttackableTiles &at, const CStack* attacker, BattleHex destinationTile, BattleHex attackerPos); //hexes around target that could be attacked in melee
@@ -144,7 +144,7 @@ struct DLL_LINKAGE BattleInfo : public CBonusSystemNode, public CBattleInfoCallb
 
 	const CGHeroInstance * getHero(PlayerColor player) const; //returns fighting hero that belongs to given player
 
-	std::vector<ui32> calculateResistedStacks(const CSpell * sp, const CGHeroInstance * caster, const CGHeroInstance * hero2, const std::vector<const CStack*> & affectedCreatures, PlayerColor casterSideOwner, ECastingMode::ECastingMode mode, int usedSpellPower, int spellLevel) const;
+	std::vector<ui32> calculateResistedStacks(const CSpell * sp, const CGHeroInstance * caster, const CGHeroInstance * hero2, const std::vector<const CStack*> & affectedCreatures, PlayerColor casterSideOwner, ECastingMode::ECastingMode mode, int usedSpellPower, int spellLevel, CRandomGenerator & rand) const;
 
 	const CStack * battleGetStack(BattleHex pos, bool onlyAlive); //returns stack at given tile
 	const CGHeroInstance * battleGetOwner(const CStack * stack) const; //returns hero that owns given stack; nullptr if none

lib/CBattleCallback.cpp

@@ -2251,10 +2251,14 @@ SpellID CBattleInfoCallback::getRandomBeneficialSpell(const CStack * subject) co
 		}
 	}
 
-	if (possibleSpells.size())
-		return possibleSpells[rand() % possibleSpells.size()];
+	if(!possibleSpells.empty())
+	{
+		return *RandomGeneratorUtil::nextItem(possibleSpells, gs->getRandomGenerator());
+	}
 	else
+	{
 		return SpellID::NONE;
+	}
 }
 
 SpellID CBattleInfoCallback::getRandomCastedSpell(const CStack * caster) const
@@ -2269,7 +2273,7 @@ SpellID CBattleInfoCallback::getRandomCastedSpell(const CStack * caster) const
 	{
 		totalWeight += std::max(b->additionalInfo, 1); //minimal chance to cast is 1
 	}
-	int randomPos = rand() % totalWeight;
+	int randomPos = gs->getRandomGenerator().nextInt(totalWeight - 1);
 	for(Bonus * b : *bl)
 	{
 		randomPos -= std::max(b->additionalInfo, 1);

lib/CGameState.h

@@ -458,7 +458,7 @@ public:
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & scenarioOps & initialOpts & currentPlayer & day & map & players & teams & hpool & globalEffects;
+		h & scenarioOps & initialOpts & currentPlayer & day & map & players & teams & hpool & globalEffects & rand;
 		BONUS_TREE_DESERIALIZATION_FIX
 	}
 

lib/CHeroHandler.cpp

@@ -23,7 +23,7 @@
  *
  */
 
-SecondarySkill CHeroClass::chooseSecSkill(const std::set<SecondarySkill> & possibles, std::minstd_rand & distr) const //picks secondary skill out from given possibilities
+SecondarySkill CHeroClass::chooseSecSkill(const std::set<SecondarySkill> & possibles, CRandomGenerator & rand) const //picks secondary skill out from given possibilities
 {
 	int totalProb = 0;
 	for(auto & possible : possibles)
@@ -32,12 +32,14 @@ SecondarySkill CHeroClass::chooseSecSkill(const std::set<SecondarySkill> & possi
 	}
 	if (totalProb != 0) // may trigger if set contains only banned skills (0 probability)
 	{
-		int ran = distr()%totalProb;
+		auto ran = rand.nextInt(totalProb - 1);
 		for(auto & possible : possibles)
 		{
 			ran -= secSkillProbability[possible];
-			if(ran<0)
+			if(ran < 0)
+			{
 				return possible;
+			}
 		}
 	}
 	// FIXME: select randomly? How H3 handles such rare situation?

lib/CHeroHandler.h

@@ -21,6 +21,7 @@ class CGameInfo;
 class CGHeroInstance;
 struct BattleHex;
 class JsonNode;
+class CRandomGenerator;
 
 struct SSpecialtyInfo
 {	si32 type;
@@ -132,7 +133,7 @@ public:
 	CHeroClass();
 
 	bool isMagicHero() const;
-	SecondarySkill chooseSecSkill(const std::set<SecondarySkill> & possibles, std::minstd_rand & distr) const; //picks secondary skill out from given possibilities
+	SecondarySkill chooseSecSkill(const std::set<SecondarySkill> & possibles, CRandomGenerator & rand) const; //picks secondary skill out from given possibilities
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{

lib/CMakeLists.txt

@@ -68,6 +68,7 @@ set(lib_SRCS
 		CHeroHandler.cpp
 		CModHandler.cpp
 		CObstacleInstance.cpp
+		CRandomGenerator.cpp
 		CSpellHandler.cpp
 		CThreadHelper.cpp
 		CTownHandler.cpp
@@ -91,7 +92,6 @@ set(lib_HEADERS
 		CondSh.h
 		ConstTransitivePtr.h
 		CBonusTypeHandler.h
-		CRandomGenerator.h
 		CScriptingModule.h
 		CStopWatch.h
 		FunctionList.h

lib/CObjectHandler.cpp

@@ -767,17 +767,14 @@ void CGHeroInstance::initHero()
 	}
 	assert(validTypes());
 
-	if (exp == 0xffffffff)
+	level = 1;
+	if(exp == 0xffffffff)
 	{
 		initExp();
 	}
-	else if (ID != Obj::PRISON)
-	{
-		level = VLC->heroh->level(exp);
-	}
-	else //warp hero at the beginning of next turn
+	else
 	{
-		level = 1;
+		levelUpAutomatically();
 	}
 
 	if (VLC->modh->modules.COMMANDERS && !commander)
@@ -953,7 +950,7 @@ void CGHeroInstance::initObj()
 	if(!type)
 		initHero(); //TODO: set up everything for prison before specialties are configured
 
-	skillsInfo.distribution.seed(rand());
+	skillsInfo.rand.setSeed(cb->gameState()->getRandomGenerator().nextInt());
 	skillsInfo.resetMagicSchoolCounter();
 	skillsInfo.resetWisdomCounter();
 
@@ -1552,7 +1549,6 @@ EAlignment::EAlignment CGHeroInstance::getAlignment() const
 void CGHeroInstance::initExp()
 {
 	exp = cb->gameState()->getRandomGenerator().nextInt(40, 89);
-	level = 1;
 }
 
 std::string CGHeroInstance::nodeName() const
@@ -1630,7 +1626,7 @@ ArtBearer::ArtBearer CGHeroInstance::bearerType() const
 	return ArtBearer::HERO;
 }
 
-std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
+std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills() const
 {
 	std::vector<SecondarySkill> obligatorySkills; //hero is offered magic school or wisdom if possible
 	if (!skillsInfo.wisdomCounter)
@@ -1643,11 +1639,7 @@ std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
 		std::vector<SecondarySkill> ss;
 		ss += SecondarySkill::FIRE_MAGIC, SecondarySkill::AIR_MAGIC, SecondarySkill::WATER_MAGIC, SecondarySkill::EARTH_MAGIC;
 
-		auto rng = [=](ui32 val) mutable -> ui32
-		{
-			return skillsInfo.distribution() % val; //must be determined
-		};
-		std::random_shuffle(ss.begin(), ss.end(), rng);
+		std::shuffle(ss.begin(), ss.end(), skillsInfo.rand.getStdGenerator());
 
 		for (auto skill : ss)
 		{
@@ -1691,12 +1683,12 @@ std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
 	}
 	else if(none.size() && canLearnSkill()) //hero have free skill slot
 	{
-		skills.push_back(type->heroClass->chooseSecSkill(none, skillsInfo.distribution)); //new skill
+		skills.push_back(type->heroClass->chooseSecSkill(none, skillsInfo.rand)); //new skill
 		none.erase(skills.back());
 	}
 	else if(!basicAndAdv.empty())
 	{
-		skills.push_back(type->heroClass->chooseSecSkill(basicAndAdv, skillsInfo.distribution)); //upgrade existing
+		skills.push_back(type->heroClass->chooseSecSkill(basicAndAdv, skillsInfo.rand)); //upgrade existing
 		basicAndAdv.erase(skills.back());
 	}
 
@@ -1706,7 +1698,7 @@ std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
 	//3) give any other new skill
 	if(!basicAndAdv.empty())
 	{
-		SecondarySkill s = type->heroClass->chooseSecSkill(basicAndAdv, skillsInfo.distribution);//upgrade existing
+		SecondarySkill s = type->heroClass->chooseSecSkill(basicAndAdv, skillsInfo.rand);//upgrade existing
 		skills.push_back(s);
 		basicAndAdv.erase(s);
 	}
@@ -1716,18 +1708,147 @@ std::vector<SecondarySkill> CGHeroInstance::levelUpProposedSkills() const
 	}
 	else if(none.size() && canLearnSkill())
 	{
-		skills.push_back(type->heroClass->chooseSecSkill(none, skillsInfo.distribution)); //give new skill
+		skills.push_back(type->heroClass->chooseSecSkill(none, skillsInfo.rand)); //give new skill
 		none.erase(skills.back());
 	}
 
 	return skills;
 }
 
+PrimarySkill::PrimarySkill CGHeroInstance::nextPrimarySkill() const
+{
+	assert(gainsLevel());
+	int randomValue = cb->gameState()->getRandomGenerator().nextInt(99), pom = 0, primarySkill = 0;
+	const auto & skillChances = (level > 9) ? type->heroClass->primarySkillLowLevel : type->heroClass->primarySkillHighLevel;
+
+	for(; primarySkill < GameConstants::PRIMARY_SKILLS; ++primarySkill)
+	{
+		pom += skillChances[primarySkill];
+		if(randomValue < pom)
+		{
+			break;
+		}
+	}
+
+	logGlobal->traceStream() << "The hero gets the primary skill " << primarySkill << " with a probability of " << randomValue << "%.";
+	return static_cast<PrimarySkill::PrimarySkill>(primarySkill);
+}
+
+boost::optional<SecondarySkill> CGHeroInstance::nextSecondarySkill() const
+{
+	assert(gainsLevel());
+
+	boost::optional<SecondarySkill> chosenSecondarySkill;
+	const auto proposedSecondarySkills = getLevelUpProposedSecondarySkills();
+	if(!proposedSecondarySkills.empty())
+	{
+		std::vector<SecondarySkill> learnedSecondarySkills;
+		for(auto secondarySkill : proposedSecondarySkills)
+		{
+			if(getSecSkillLevel(secondarySkill) > 0)
+			{
+				learnedSecondarySkills.push_back(secondarySkill);
+			}
+		}
+
+		auto & rand = cb->gameState()->getRandomGenerator();
+		if(learnedSecondarySkills.empty())
+		{
+			// there are only new skills to learn, so choose anyone of them
+			chosenSecondarySkill = *RandomGeneratorUtil::nextItem(proposedSecondarySkills, rand);
+		}
+		else
+		{
+			// preferably upgrade a already learned secondary skill
+			chosenSecondarySkill = *RandomGeneratorUtil::nextItem(learnedSecondarySkills, rand);
+		}
+	}
+	return chosenSecondarySkill;
+}
+
+void CGHeroInstance::setPrimarySkill(PrimarySkill::PrimarySkill primarySkill, si64 value, ui8 abs)
+{
+	if(primarySkill < PrimarySkill::EXPERIENCE)
+	{
+		Bonus * skill = getBonusLocalFirst(Selector::type(Bonus::PRIMARY_SKILL)
+											.And(Selector::subtype(primarySkill))
+											.And(Selector::sourceType(Bonus::HERO_BASE_SKILL)));
+		assert(skill);
+
+		if(abs)
+		{
+			skill->val = value;
+		}
+		else
+		{
+			skill->val += value;
+		}
+	}
+	else if(primarySkill == PrimarySkill::EXPERIENCE)
+	{
+		if(abs)
+		{
+			exp = value;
+		}
+		else
+		{
+			exp += value;
+		}
+	}
+}
+
 bool CGHeroInstance::gainsLevel() const
 {
 	return exp >= VLC->heroh->reqExp(level+1);
 }
 
+void CGHeroInstance::levelUp(std::vector<SecondarySkill> skills)
+{
+	++level;
+
+	//deterministic secondary skills
+	skillsInfo.magicSchoolCounter = (skillsInfo.magicSchoolCounter + 1) % maxlevelsToMagicSchool();
+	skillsInfo.wisdomCounter = (skillsInfo.wisdomCounter + 1) % maxlevelsToWisdom();
+	if(vstd::contains(skills, SecondarySkill::WISDOM))
+	{
+		skillsInfo.resetWisdomCounter();
+	}
+
+	SecondarySkill spellSchools[] = {
+		SecondarySkill::FIRE_MAGIC, SecondarySkill::AIR_MAGIC, SecondarySkill::WATER_MAGIC, SecondarySkill::EARTH_MAGIC};
+	for(auto skill : spellSchools)
+	{
+		if(vstd::contains(skills, skill))
+		{
+			skillsInfo.resetMagicSchoolCounter();
+			break;
+		}
+	}
+
+	//specialty
+	Updatespecialty();
+}
+
+void CGHeroInstance::levelUpAutomatically()
+{
+	while(gainsLevel())
+	{
+		const auto primarySkill = nextPrimarySkill();
+		setPrimarySkill(primarySkill, 1, false);
+
+		auto proposedSecondarySkills = getLevelUpProposedSecondarySkills();
+
+		const auto secondarySkill = nextSecondarySkill();
+		if(secondarySkill)
+		{
+			setSecSkillLevel(*secondarySkill, 1, false);
+		}
+
+		//TODO why has the secondary skills to be passed to the method?
+		levelUp(proposedSecondarySkills);
+	}
+}
+
 void CGDwelling::initObj()
 {
 	switch(ID)
@@ -2303,13 +2424,15 @@ void CGTownInstance::newTurn() const
 {
 	if (cb->getDate(Date::DAY_OF_WEEK) == 1) //reset on new week
 	{
+		auto & rand = cb->gameState()->getRandomGenerator();
+
 		//give resources for Rampart, Mystic Pond
 		if (hasBuilt(BuildingID::MYSTIC_POND, ETownType::RAMPART)
 			&& cb->getDate(Date::DAY) != 1 && (tempOwner < PlayerColor::PLAYER_LIMIT))
 		{
-			int resID = rand()%4+2;//bonus to random rare resource
+			int resID = rand.nextInt(2, 5); //bonus to random rare resource
 			resID = (resID==2)?1:resID;
-			int resVal = rand()%4+1;//with size 1..4
+			int resVal = rand.nextInt(1, 4);//with size 1..4
 			cb->giveResource(tempOwner, static_cast<Res::ERes>(resID), resVal);
 			cb->setObjProperty (id, ObjProperty::BONUS_VALUE_FIRST, resID);
 			cb->setObjProperty (id, ObjProperty::BONUS_VALUE_SECOND, resVal);
@@ -2334,11 +2457,11 @@ void CGTownInstance::newTurn() const
 				}
 				if (nativeCrits.size())
 				{
-					SlotID pos = nativeCrits[rand() % nativeCrits.size()];
+					SlotID pos = *RandomGeneratorUtil::nextItem(nativeCrits, rand);
 					StackLocation sl(this, pos);
 
 					const CCreature *c = getCreature(pos);
-					if (rand()%100 < 90 || c->upgrades.empty()) //increase number if no upgrade available
+					if (rand.nextInt(99) < 90 || c->upgrades.empty()) //increase number if no upgrade available
 					{
 						cb->changeStackCount(sl, c->growth);
 					}
@@ -2347,9 +2470,9 @@ void CGTownInstance::newTurn() const
 						cb->changeStackType(sl, VLC->creh->creatures[*c->upgrades.begin()]);
 					}
 				}
-				if ((stacksCount() < GameConstants::ARMY_SIZE && rand()%100 < 25) || Slots().empty()) //add new stack
+				if ((stacksCount() < GameConstants::ARMY_SIZE && rand.nextInt(99) < 25) || Slots().empty()) //add new stack
 				{
-					int i = rand() % std::min (GameConstants::CREATURES_PER_TOWN, cb->getDate(Date::MONTH)<<1);
+					int i = rand.nextInt(std::min(GameConstants::CREATURES_PER_TOWN, cb->getDate(Date::MONTH) << 1) - 1);
 					if (!town->creatures[i].empty())
 					{
 						CreatureID c = town->creatures[i][0];
@@ -3316,7 +3439,7 @@ void CGCreature::initObj()
 			amount = 1;
 		}
 	}
-	formation.randomFormation = rand();
+	formation.randomFormation = cb->gameState()->getRandomGenerator().nextInt();
 
 	temppower = stacks[SlotID(0)]->count * 1000;
 	refusedJoining = false;
@@ -3532,10 +3655,10 @@ void CGCreature::fight( const CGHeroInstance *h ) const
 		if (formation.randomFormation % 100 < 50) //upgrade
 		{
 			SlotID slotId = SlotID(stacks.size() / 2);
-			if(ui32 upgradesSize = getStack(slotId).type->upgrades.size())
+			const auto & upgrades = getStack(slotId).type->upgrades;
+			if(!upgrades.empty())
 			{
-				auto it = getStack(slotId).type->upgrades.cbegin(); //pick random in case there are more
-				std::advance (it, rand() % upgradesSize);
+				auto it = RandomGeneratorUtil::nextItem(upgrades, cb->gameState()->getRandomGenerator());
 				cb->changeStackType(StackLocation(this, slotId), VLC->creh->creatures[*it]);
 			}
 		}
@@ -3869,11 +3992,12 @@ void CGVisitableOPW::onHeroVisit( const CGHeroInstance * h ) const
 		Component::EComponentType type = Component::RESOURCE;
 		Res::ERes sub=Res::WOOD;
 		int val=0;
+		auto & rand = cb->gameState()->getRandomGenerator();
 
 		switch (ID)
 		{
 		case Obj::MYSTICAL_GARDEN:
-			if (rand()%2)
+			if(rand.nextInt(1) == 1)
 			{
 				sub = Res::GEMS;
 				val = 5;
@@ -3886,8 +4010,8 @@ void CGVisitableOPW::onHeroVisit( const CGHeroInstance * h ) const
 			break;
 		case Obj::WINDMILL:
 			mid = 170;
-			sub = static_cast<Res::ERes>((rand() % 5) + 1);
-			val = (rand() % 4) + 3;
+			sub = static_cast<Res::ERes>(rand.nextInt(1, 5));
+			val = rand.nextInt(3, 6);
 			break;
 		case Obj::WATER_WHEEL:
 			mid = 164;
@@ -3923,16 +4047,27 @@ void CGTeleport::onHeroVisit( const CGHeroInstance * h ) const
 	switch(ID)
 	{
 	case Obj::MONOLITH1: //one way - find corresponding exit monolith
+	{
 		if(vstd::contains(objs,Obj::MONOLITH2) && vstd::contains(objs[Obj::MONOLITH2],subID) && objs[Obj::MONOLITH2][subID].size())
-			destinationid = objs[Obj::MONOLITH2][subID][rand()%objs[Obj::MONOLITH2][subID].size()];
+		{
+			destinationid = *RandomGeneratorUtil::nextItem(objs[Obj::MONOLITH2][subID], cb->gameState()->getRandomGenerator());
+		}
 		else
+		{
             logGlobal->warnStream() << "Cannot find corresponding exit monolith for "<< id;
+		}
 		break;
+	}
 	case Obj::MONOLITH3://two way monolith - pick any other one
 	case Obj::WHIRLPOOL: //Whirlpool
 		if(vstd::contains(objs,ID) && vstd::contains(objs[ID],subID) && objs[ID][subID].size()>1)
 		{
-			while ((destinationid = objs[ID][subID][rand()%objs[ID][subID].size()]) == id); //choose another exit
+			//choose another exit
+			do
+			{
+				destinationid = *RandomGeneratorUtil::nextItem(objs[ID][subID], cb->gameState()->getRandomGenerator());
+			} while(destinationid == id);
+
 			if (ID == Obj::WHIRLPOOL)
 			{
 				if (!h->hasBonusOfType(Bonus::WHIRLPOOL_PROTECTION))
@@ -3983,9 +4118,8 @@ void CGTeleport::onHeroVisit( const CGHeroInstance * h ) const
 	if (ID == Obj::WHIRLPOOL)
 	{
 		std::set<int3> tiles = cb->getObj(destinationid)->getBlockedPos();
-		auto it = tiles.begin();
-		std::advance (it, rand() % tiles.size()); //picking random element of set is tiring
-		cb->moveHero (h->id, *it + int3(1,0,0), true);
+		auto & tile = *RandomGeneratorUtil::nextItem(tiles, cb->gameState()->getRandomGenerator());
+		cb->moveHero(h->id, tile + int3(1,0,0), true);
 	}
 	else
 		cb->moveHero (h->id,CGHeroInstance::convertPosition(cb->getObj(destinationid)->pos,true) - getVisitableOffset(), true);
@@ -5156,7 +5290,7 @@ void CGBonusingObject::onHeroVisit( const CGHeroInstance * h ) const
 		messageID = 55;
 		iw.soundID = soundBase::LUCK;
 		gbonus.bonus.type = Bonus::LUCK;
-		gbonus.bonus.val = rand()%5 - 1;
+		gbonus.bonus.val = cb->gameState()->getRandomGenerator().nextInt(-1, 3);
 		descr_id = 69;
 		gbonus.bdescr.addReplacement((gbonus.bonus.val<0 ? "-" : "+") + boost::lexical_cast<std::string>(gbonus.bonus.val));
 		break;

lib/CObjectHandler.h

@@ -8,6 +8,7 @@
 #include "int3.h"
 #include "GameConstants.h"
 #include "ResourceSet.h"
+#include "CRandomGenerator.h"
 
 /*
  * CObjectHandler.h, part of VCMI engine
@@ -387,8 +388,8 @@ public:
 	struct DLL_LINKAGE SecondarySkillsInfo
 	{
 		//skills are determined, initialized at map start
-		//FIXME: remove mutable?
-		mutable std::minstd_rand distribution;
+		//FIXME remove mutable
+		mutable CRandomGenerator rand;
 		ui8 magicSchoolCounter;
 		ui8 wisdomCounter;
 
@@ -397,39 +398,10 @@ public:
 
 		template <typename Handler> void serialize(Handler &h, const int version)
 		{
-			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;
-			}
+			h & magicSchoolCounter & wisdomCounter & rand;
 		}
 	} skillsInfo;
 
-	//////////////////////////////////////////////////////////////////////////
-
-
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & static_cast<CArmedInstance&>(*this);
-		h & static_cast<CArtifactSet&>(*this);
-		h & exp & level & name & biography & portrait & mana & secSkills & movement
-			& sex & inTownGarrison & spells & patrol & moveDir & skillsInfo;
-		h & visitedTown & boat;
-		h & type & specialty & commander;
-		BONUS_TREE_DESERIALIZATION_FIX
-		//visitied town pointer will be restored by map serialization method
-	}
-	//////////////////////////////////////////////////////////////////////////
 	int3 getSightCenter() const; //"center" tile from which the sight distance is calculated
 	int getSightRadious() const; //sight distance (should be used if player-owned structure)
 	//////////////////////////////////////////////////////////////////////////
@@ -451,9 +423,28 @@ public:
 	int getCurrentLuck(int stack=-1, bool town=false) const;
 	int getSpellCost(const CSpell *sp) const; //do not use during battles -> bonuses from army would be ignored
 
+	// ----- primary and secondary skill, experience, level handling -----
+
+	/// Returns true if hero has lower level than should upon his experience.
+	bool gainsLevel() const;
+
+	/// Returns the next primary skill on level up. Can only be called if hero can gain a level up.
+	PrimarySkill::PrimarySkill nextPrimarySkill() const;
+
+	/// Returns the next secondary skill randomly on level up. Can only be called if hero can gain a level up.
+	boost::optional<SecondarySkill> nextSecondarySkill() const;
+
+	/// Gets 0, 1 or 2 secondary skills which are proposed on hero level up.
+	std::vector<SecondarySkill> getLevelUpProposedSecondarySkills() const;
+
 	ui8 getSecSkillLevel(SecondarySkill skill) const; //0 - no skill
+
+	/// Returns true if hero has free secondary skill slot.
+	bool canLearnSkill() const;
+
+	void setPrimarySkill(PrimarySkill::PrimarySkill primarySkill, si64 value, ui8 abs);
 	void setSecSkillLevel(SecondarySkill which, int val, bool abs);// abs == 0 - changes by value; 1 - sets to value
-	bool canLearnSkill() const; ///true if hero has free secondary skill slot
+	void levelUp(std::vector<SecondarySkill> skills);
 
 	int maxMovePoints(bool onLand) const;
 	int movementPointsAfterEmbark(int MPsBefore, int basicCost, bool disembark = false) const;
@@ -470,8 +461,6 @@ public:
 	CStackBasicDescriptor calculateNecromancy (const BattleResult &battleResult) const;
 	void showNecromancyDialog(const CStackBasicDescriptor &raisedStack) const;
 	ECanDig diggingStatus() const; //0 - can dig; 1 - lack of movement; 2 -
-	std::vector<SecondarySkill> levelUpProposedSkills() const;
-	bool gainsLevel() const; //true if hero has lower level than should upon his experience
 
 	//////////////////////////////////////////////////////////////////////////
 
@@ -506,6 +495,22 @@ public:
 	const std::string & getHoverText() const override;
 protected:
 	void setPropertyDer(ui8 what, ui32 val) override;//synchr
+
+private:
+	void levelUpAutomatically();
+
+public:
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		h & static_cast<CArmedInstance&>(*this);
+		h & static_cast<CArtifactSet&>(*this);
+		h & exp & level & name & biography & portrait & mana & secSkills & movement
+			& sex & inTownGarrison & spells & patrol & moveDir & skillsInfo;
+		h & visitedTown & boat;
+		h & type & specialty & commander;
+		BONUS_TREE_DESERIALIZATION_FIX
+		//visitied town pointer will be restored by map serialization method
+	}
 };
 
 class DLL_LINKAGE CSpecObjInfo

lib/CRandomGenerator.cpp

@@ -0,0 +1,86 @@
+
+/*
+ * CRandomGenerator.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 "CRandomGenerator.h"
+
+boost::thread_specific_ptr<CRandomGenerator> CRandomGenerator::defaultRand;
+
+CRandomGenerator::CRandomGenerator()
+{
+	resetSeed();
+}
+
+void CRandomGenerator::setSeed(int seed)
+{
+	rand.seed(seed);
+}
+
+void CRandomGenerator::resetSeed()
+{
+	boost::hash<std::string> stringHash;
+	auto threadIdHash = stringHash(boost::lexical_cast<std::string>(boost::this_thread::get_id()));
+	setSeed(threadIdHash * std::time(nullptr));
+}
+
+TRandI CRandomGenerator::getIntRange(int lower, int upper)
+{
+	return boost::bind(TIntDist(lower, upper), boost::ref(rand));
+}
+
+int CRandomGenerator::nextInt(int upper)
+{
+	return getIntRange(0, upper)();
+}
+
+int CRandomGenerator::nextInt(int lower, int upper)
+{
+	return getIntRange(lower, upper)();
+}
+
+int CRandomGenerator::nextInt()
+{
+	return TIntDist()(rand);
+}
+
+TRand CRandomGenerator::getDoubleRange(double lower, double upper)
+{
+	return boost::bind(TRealDist(lower, upper), boost::ref(rand));
+}
+
+double CRandomGenerator::nextDouble(double upper)
+{
+	return getDoubleRange(0, upper)();
+}
+
+double CRandomGenerator::nextDouble(double lower, double upper)
+{
+	return getDoubleRange(lower, upper)();
+}
+
+double CRandomGenerator::nextDouble()
+{
+	return TRealDist()(rand);
+}
+
+CRandomGenerator & CRandomGenerator::getDefault()
+{
+	if(!defaultRand.get())
+	{
+		defaultRand.reset(new CRandomGenerator());
+	}
+	return *defaultRand.get();
+}
+
+TGenerator & CRandomGenerator::getStdGenerator()
+{
+	return rand;
+}

lib/CRandomGenerator.h

@@ -19,77 +19,80 @@ typedef std::function<double()> TRand;
 
 /// The random generator randomly generates integers and real numbers("doubles") between
 /// a given range. This is a header only class and mainly a wrapper for
-/// convenient usage of the standard random API.
-class CRandomGenerator : boost::noncopyable
+/// convenient usage of the standard random API. An instance of this RNG is not thread safe.
+class DLL_LINKAGE CRandomGenerator : boost::noncopyable
 {
 public:
-	/// Seeds the generator with the current time by default.
-	CRandomGenerator() 
-	{
-		rand.seed(static_cast<unsigned long>(std::time(nullptr)));
-	}
+	/// Seeds the generator by default with the product of the current time in milliseconds and the
+	/// current thread ID.
+	CRandomGenerator();
 
-	void setSeed(int seed)
-	{
-		rand.seed(seed);
-	}
+	void setSeed(int seed);
+
+	/// Resets the seed to the product of the current time in milliseconds and the
+	/// current thread ID.
+	void resetSeed();
 
 	/// Generate several integer numbers within the same range.
 	/// e.g.: auto a = gen.getIntRange(0,10); a(); a(); a();
 	/// requires: lower <= upper
-	TRandI getIntRange(int lower, int upper)
-	{
-		return boost::bind(TIntDist(lower, upper), boost::ref(rand));
-	}
+	TRandI getIntRange(int lower, int upper);
 	
 	/// Generates an integer between 0 and upper.
 	/// requires: 0 <= upper
-	int nextInt(int upper)
-	{
-		return getIntRange(0, upper)();
-	}
+	int nextInt(int upper);
 
 	/// requires: lower <= upper
-	int nextInt(int lower, int upper)
-	{
-		return getIntRange(lower, upper)();
-	}
+	int nextInt(int lower, int upper);
 	
 	/// Generates an integer between 0 and the maximum value it can hold.
-	int nextInt()
-	{
-		return TIntDist()(rand);
-	}
+	int nextInt();
 
 	/// Generate several double/real numbers within the same range.
 	/// e.g.: auto a = gen.getDoubleRange(4.5,10.2); a(); a(); a();
 	/// requires: lower <= upper
-	TRand getDoubleRange(double lower, double upper)
-	{
-		return boost::bind(TRealDist(lower, upper), boost::ref(rand));
-	}
+	TRand getDoubleRange(double lower, double upper);
 	
 	/// Generates a double between 0 and upper.
 	/// requires: 0 <= upper
-	double nextDouble(double upper)
-	{
-		return getDoubleRange(0, upper)();
-	}
+	double nextDouble(double upper);
 
 	/// requires: lower <= upper
-	double nextDouble(double lower, double upper)
-	{
-		return getDoubleRange(lower, upper)();
-	}
+	double nextDouble(double lower, double upper);
 
 	/// Generates a double between 0.0 and 1.0.
-	double nextDouble()
-	{
-		return TRealDist()(rand);
-	}
+	double nextDouble();
+
+	/// Gets a globally accessible RNG which will be constructed once per thread. For the
+	/// seed a combination of the thread ID and current time in milliseconds will be used.
+	static CRandomGenerator & getDefault();
+
+	/// Provide method so that this RNG can be used with legacy std:: API
+	TGenerator & getStdGenerator();
 
 private:
 	TGenerator rand;
+	static boost::thread_specific_ptr<CRandomGenerator> defaultRand;
+
+public:
+	template <typename Handler>
+	void serialize(Handler & h, const int version)
+	{
+		if(h.saving)
+		{
+			std::ostringstream stream;
+			stream << rand;
+			std::string str = stream.str();
+			h & str;
+		}
+		else
+		{
+			std::string str;
+			h & str;
+			std::istringstream stream(str);
+			stream >> rand;
+		}
+	}
 };
 
 namespace RandomGeneratorUtil

lib/NetPacks.h

@@ -1068,14 +1068,13 @@ struct HeroLevelUp : public Query//2000
 
 	const CGHeroInstance *hero;
 	PrimarySkill::PrimarySkill primskill;
-	ui8 level;
 	std::vector<SecondarySkill> skills;
 
 	HeroLevelUp(){type = 2000;};
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & queryID & hero & primskill & level & skills;
+		h & queryID & hero & primskill & skills;
 	}
 };
 

lib/NetPacksLib.cpp

@@ -49,28 +49,9 @@ DLL_LINKAGE void SetResources::applyGs( CGameState *gs )
 
 DLL_LINKAGE void SetPrimSkill::applyGs( CGameState *gs )
 {
-	CGHeroInstance *hero = gs->getHero(id);
+	CGHeroInstance * hero = gs->getHero(id);
 	assert(hero);
-
-	if(which < PrimarySkill::EXPERIENCE)
-	{
-		Bonus *skill = hero->getBonusLocalFirst(Selector::type(Bonus::PRIMARY_SKILL)
-											.And(Selector::subtype(which))
-											.And(Selector::sourceType(Bonus::HERO_BASE_SKILL)));
-		assert(skill);
-
-		if(abs)
-			skill->val = val;
-		else
-			skill->val += val;
-	}
-	else if(which == PrimarySkill::EXPERIENCE)
-	{
-		if(abs)
-			hero->exp = val;
-		else
-			hero->exp += val;
-	}
+	hero->setPrimarySkill(which, val, abs);
 }
 
 DLL_LINKAGE void SetSecSkill::applyGs( CGameState *gs )
@@ -1020,25 +1001,8 @@ DLL_LINKAGE void SetHoverName::applyGs( CGameState *gs )
 
 DLL_LINKAGE void HeroLevelUp::applyGs( CGameState *gs )
 {
-	CGHeroInstance* h = gs->getHero(hero->id);
-	h->level = level;
-	//deterministic secondary skills
-	h->skillsInfo.magicSchoolCounter = (h->skillsInfo.magicSchoolCounter + 1) % h->maxlevelsToMagicSchool();
-	h->skillsInfo.wisdomCounter = (h->skillsInfo.wisdomCounter + 1) % h->maxlevelsToWisdom();
-	if (vstd::contains(skills, SecondarySkill::WISDOM))
-		h->skillsInfo.resetWisdomCounter();
-	SecondarySkill spellSchools[] = {
-		SecondarySkill::FIRE_MAGIC, SecondarySkill::AIR_MAGIC, SecondarySkill::WATER_MAGIC, SecondarySkill::EARTH_MAGIC};
-	for (auto skill : spellSchools)
-	{
-		if (vstd::contains(skills, skill))
-		{
-			h->skillsInfo.resetMagicSchoolCounter();
-			break;
-		}
-	}
-	//specialty
-	h->Updatespecialty();
+	CGHeroInstance * h = gs->getHero(hero->id);
+	h->levelUp(skills);
 }
 
 DLL_LINKAGE void CommanderLevelUp::applyGs (CGameState *gs)

lib/VCMI_lib.cbp

@@ -107,6 +107,7 @@
 		<Unit filename="CObjectHandler.h" />
 		<Unit filename="CObstacleInstance.cpp" />
 		<Unit filename="CObstacleInstance.h" />
+		<Unit filename="CRandomGenerator.cpp" />
 		<Unit filename="CRandomGenerator.h" />
 		<Unit filename="CScriptingModule.h" />
 		<Unit filename="CSpellHandler.cpp" />

lib/VCMI_lib.vcxproj

@@ -185,6 +185,7 @@
     <ClCompile Include="CSpellHandler.cpp" />
     <ClCompile Include="CThreadHelper.cpp" />
     <ClCompile Include="CTownHandler.cpp" />
+    <ClCompile Include="CRandomGenerator.cpp" />
     <ClCompile Include="filesystem\AdapterLoaders.cpp" />
     <ClCompile Include="filesystem\CArchiveLoader.cpp" />
     <ClCompile Include="filesystem\CBinaryReader.cpp" />
@@ -323,4 +324,4 @@
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
-</Project>
+</Project>

lib/VCMI_lib.vcxproj.filters

@@ -39,6 +39,7 @@
     <ClCompile Include="CCreatureSet.cpp" />
     <ClCompile Include="CGameState.cpp" />
     <ClCompile Include="Connection.cpp" />
+    <ClCompile Include="CRandomGenerator.cpp" />
     <ClCompile Include="HeroBonus.cpp" />
     <ClCompile Include="IGameCallback.cpp" />
     <ClCompile Include="NetPacksLib.cpp" />
@@ -393,4 +394,4 @@
       <Filter>Header Files</Filter>
     </ClInclude>
   </ItemGroup>
-</Project>
+</Project>

server/CGameHandler.cpp

@@ -183,31 +183,21 @@ void CGameHandler::levelUpHero(const CGHeroInstance * hero)
 		return;
 	}
 
-	//give prim skill
-    logGlobal->traceStream() << hero->name <<" got level "<<hero->level;
-	int r = rand()%100, pom=0, x=0;
+	// give primary skill
+	logGlobal->traceStream() << hero->name << " got level "<< hero->level;
+	auto primarySkill = hero->nextPrimarySkill();
 
-	auto & skillChances = (hero->level>9) ? hero->type->heroClass->primarySkillLowLevel : hero->type->heroClass->primarySkillHighLevel;
-
-	for(;x<GameConstants::PRIMARY_SKILLS;x++)
-	{
-		pom += skillChances[x];
-		if(r<pom)
-			break;
-	}
-    logGlobal->traceStream() << "The hero gets the primary skill with the no. " << x << " with a probability of " << r << "%.";
 	SetPrimSkill sps;
 	sps.id = hero->id;
-	sps.which = static_cast<PrimarySkill::PrimarySkill>(x);
+	sps.which = primarySkill;
 	sps.abs = false;
 	sps.val = 1;
 	sendAndApply(&sps);
 
 	HeroLevelUp hlu;
 	hlu.hero = hero;
-	hlu.primskill = static_cast<PrimarySkill::PrimarySkill>(x);
-	hlu.level = hero->level+1;
-	hlu.skills = hero->levelUpProposedSkills();
+	hlu.primskill = primarySkill;
+	hlu.skills = hero->getLevelUpProposedSecondarySkills();
 
 	if(hlu.skills.size() == 0)
 	{
@@ -217,11 +207,7 @@ void CGameHandler::levelUpHero(const CGHeroInstance * hero)
 	else if(hlu.skills.size() == 1  ||  hero->tempOwner == PlayerColor::NEUTRAL)  //choose skill automatically
 	{
 		sendAndApply(&hlu);
-		auto rng = [&]() mutable -> ui32
-		{
-			return hero->skillsInfo.distribution(); //must be determined
-		};
-		levelUpHero(hero, vstd::pickRandomElementOf (hlu.skills, rng));
+		levelUpHero(hero, *RandomGeneratorUtil::nextItem(hlu.skills, hero->skillsInfo.rand));
 	}
 	else if(hlu.skills.size() > 1)
 	{
@@ -359,7 +345,7 @@ void CGameHandler::levelUpCommander(const CCommanderInstance * c)
 	else if(skillAmount == 1  ||  hero->tempOwner == PlayerColor::NEUTRAL) //choose skill automatically
 	{
 		sendAndApply(&clu);
-		levelUpCommander(c, vstd::pickRandomElementOf (clu.skills, rand));
+		levelUpCommander(c, *RandomGeneratorUtil::nextItem(clu.skills, gs->getRandomGenerator()));
 	}
 	else if(skillAmount > 1) //apply and ask for secondary skill
 	{
@@ -520,7 +506,7 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance *hero1, const CGHer
 			int maxLevel = eagleEyeLevel + 1;
 			double eagleEyeChance = finishingBattle->winnerHero->valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, SecondarySkill::EAGLE_EYE);
 			for(const CSpell *sp : gs->curB->sides.at(!battleResult.data->winner).usedSpellsHistory)
-				if(sp->level <= maxLevel && !vstd::contains(finishingBattle->winnerHero->spells, sp->id) && rand() % 100 < eagleEyeChance)
+				if(sp->level <= maxLevel && !vstd::contains(finishingBattle->winnerHero->spells, sp->id) && gs->getRandomGenerator().nextInt(99) < eagleEyeChance)
 					cs.spells.insert(sp->id);
 		}
 	}
@@ -754,20 +740,20 @@ void CGameHandler::prepareAttack(BattleAttack &bat, const CStack *att, const CSt
 
 	if(!vstd::contains_if(gs->curB->sides, sideHeroBlocksLuck))
 	{
-		if(attackerLuck > 0  && rand()%24 < attackerLuck)
+		if(attackerLuck > 0  && gs->getRandomGenerator().nextInt(23) < attackerLuck)
 		{
 			bat.flags |= BattleAttack::LUCKY;
 		}
 		if (VLC->modh->settings.data["hardcodedFeatures"]["NEGATIVE_LUCK"].Bool()) // negative luck enabled
 		{
-			if (attackerLuck < 0 && rand()%24 < abs(attackerLuck))
+			if (attackerLuck < 0 && gs->getRandomGenerator().nextInt(23) < abs(attackerLuck))
 			{
 				bat.flags |= BattleAttack::UNLUCKY;
 			}
 		}
 	}
 
-	if (rand()%100 < att->valOfBonuses(Bonus::DOUBLE_DAMAGE_CHANCE))
+	if(gs->getRandomGenerator().nextInt(99) < att->valOfBonuses(Bonus::DOUBLE_DAMAGE_CHANCE))
 	{
 		bat.flags |= BattleAttack::DEATH_BLOW;
 	}
@@ -777,7 +763,7 @@ void CGameHandler::prepareAttack(BattleAttack &bat, const CStack *att, const CSt
 		static const int artilleryLvlToChance[] = {0, 50, 75, 100};
 		const CGHeroInstance * owner = gs->curB->getHero(att->owner);
 		int chance = artilleryLvlToChance[owner->getSecSkillLevel(SecondarySkill::ARTILLERY)];
-		if(chance > rand() % 100)
+		if(chance > gs->getRandomGenerator().nextInt(99))
 		{
 			bat.flags |= BattleAttack::BALLISTA_DOUBLE_DMG;
 		}
@@ -823,8 +809,9 @@ void CGameHandler::applyBattleEffects(BattleAttack &bat, const CStack *att, cons
 		bsa.flags |= BattleStackAttacked::SECONDARY; //all other targets do not suffer from spells & spell-like abilities
 	bsa.attackerID = att->ID;
 	bsa.stackAttacked = def->ID;
-	bsa.damageAmount = gs->curB->calculateDmg(att, def, gs->curB->battleGetOwner(att), gs->curB->battleGetOwner(def), bat.shot(), distance, bat.lucky(), bat.unlucky(), bat.deathBlow(), bat.ballistaDoubleDmg());
-	def->prepareAttacked(bsa, gameState()->getRandomGenerator()); //calculate casualties
+	bsa.damageAmount = gs->curB->calculateDmg(att, def, gs->curB->battleGetOwner(att), gs->curB->battleGetOwner(def),
+		bat.shot(), distance, bat.lucky(), bat.unlucky(), bat.deathBlow(), bat.ballistaDoubleDmg(), gs->getRandomGenerator());
+	def->prepareAttacked(bsa, gs->getRandomGenerator()); //calculate casualties
 
 	//life drain handling
 	if (att->hasBonusOfType(Bonus::LIFE_DRAIN) && def->isLiving())
@@ -865,7 +852,6 @@ void CGameHandler::applyBattleEffects(BattleAttack &bat, const CStack *att, cons
 void CGameHandler::handleConnection(std::set<PlayerColor> players, CConnection &c)
 {
 	setThreadName("CGameHandler::handleConnection");
-	srand(time(nullptr));
 
 	try
 	{
@@ -1089,6 +1075,9 @@ void CGameHandler::init(StartInfo *si)
 	gs->init(si);
     logGlobal->infoStream() << "Gamestate initialized!";
 
+	// reset seed, so that clients can't predict any following random values
+	gs->getRandomGenerator().resetSeed();
+
 	for(auto & elem : gs->players)
 	{
 		states.addPlayer(elem.first);
@@ -1123,15 +1112,20 @@ void CGameHandler::setPortalDwelling(const CGTownInstance * town, bool forced=fa
 				return;
 			}
 
-			ui32 dwellpos = rand()%dwellings.size();//take random dwelling
-			ui32 creapos = rand()%dwellings.at(dwellpos)->creatures.size();//for multi-creature dwellings like Golem Factory
-			CreatureID creature = dwellings.at(dwellpos)->creatures.at(creapos).second[0];
+			auto dwelling = *RandomGeneratorUtil::nextItem(dwellings, gs->getRandomGenerator());
+
+			// for multi-creature dwellings like Golem Factory
+			auto creatureId = RandomGeneratorUtil::nextItem(dwelling->creatures, gs->getRandomGenerator())->second[0];
 
-			if (clear)
-				ssi.creatures[GameConstants::CREATURES_PER_TOWN].first = std::max((ui32)1, (VLC->creh->creatures.at(creature)->growth)/2);
+			if(clear)
+			{
+				ssi.creatures[GameConstants::CREATURES_PER_TOWN].first = std::max((ui32)1, (VLC->creh->creatures.at(creatureId)->growth)/2);
+			}
 			else
-				ssi.creatures[GameConstants::CREATURES_PER_TOWN].first = VLC->creh->creatures.at(creature)->growth;
-			ssi.creatures[GameConstants::CREATURES_PER_TOWN].second.push_back(creature);
+			{
+				ssi.creatures[GameConstants::CREATURES_PER_TOWN].first = VLC->creh->creatures.at(creatureId)->growth;
+			}
+			ssi.creatures[GameConstants::CREATURES_PER_TOWN].second.push_back(creatureId);
 			sendAndApply(&ssi);
 		}
 }
@@ -1149,7 +1143,6 @@ void CGameHandler::newTurn()
 	bool newMonth = getDate(Date::DAY_OF_MONTH) == 28;
 
 	std::map<PlayerColor, si32> hadGold;//starting gold - for buildings like dwarven treasury
-	srand(time(nullptr));
 
 	if (firstTurn)
 	{
@@ -1182,7 +1175,7 @@ void CGameHandler::newTurn()
 		}
 		else
 		{
-			int monthType = rand()%100;
+			int monthType = gs->getRandomGenerator().nextInt(99);
 			if(newMonth) //new month
 			{
 				if (monthType < 40) //double growth
@@ -1196,7 +1189,7 @@ void CGameHandler::newTurn()
 					else if(VLC->creh->doubledCreatures.size())
 					{
 						const std::vector<CreatureID> doubledCreatures (VLC->creh->doubledCreatures.begin(), VLC->creh->doubledCreatures.end());
-						n.creatureid = vstd::pickRandomElementOf(doubledCreatures, []{ return rand(); });
+						n.creatureid = *RandomGeneratorUtil::nextItem(doubledCreatures, gs->getRandomGenerator());
 					}
 					else
 					{
@@ -1427,12 +1420,12 @@ void CGameHandler::newTurn()
 					if (newMonth)
 					{
 						iw.text.addTxt(MetaString::ARRAY_TXT, (130));
-						iw.text.addReplacement(MetaString::ARRAY_TXT, 32 + rand()%10);
+						iw.text.addReplacement(MetaString::ARRAY_TXT, gs->getRandomGenerator().nextInt(32, 41));
 					}
 					else
 					{
 						iw.text.addTxt(MetaString::ARRAY_TXT, (133));
-						iw.text.addReplacement(MetaString::ARRAY_TXT, 43 + rand()%15);
+						iw.text.addReplacement(MetaString::ARRAY_TXT, gs->getRandomGenerator().nextInt(43, 57));
 					}
 			}
 			for (auto & elem : gs->players)
@@ -3592,7 +3585,7 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 				{
 					if(currentHP.at(attackedPart) != EWallState::DESTROYED && // this part can be hit
 					   currentHP.at(attackedPart) != EWallState::NONE &&
-					   rand()%100 < getCatapultHitChance(attackedPart, sbi))//hit is successful
+					   gs->getRandomGenerator().nextInt(99) < getCatapultHitChance(attackedPart, sbi))//hit is successful
 					{
 						hitSuccessfull = true;
 					}
@@ -3607,7 +3600,7 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 						}
 						if (allowedTargets.empty())
 							break;
-						attackedPart = allowedTargets.at(rand()%allowedTargets.size());
+						attackedPart = *RandomGeneratorUtil::nextItem(allowedTargets, gs->getRandomGenerator());
 					}
 				}
 				while (!hitSuccessfull);
@@ -3623,7 +3616,7 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 
 				int dmgChance[] = { sbi.noDmg, sbi.oneDmg, sbi.twoDmg }; //dmgChance[i] - chance for doing i dmg when hit is successful
 
-				int dmgRand = rand()%100;
+				int dmgRand = gs->getRandomGenerator().nextInt(99);
 				//accumulating dmgChance
 				dmgChance[1] += dmgChance[0];
 				dmgChance[2] += dmgChance[1];
@@ -4071,7 +4064,7 @@ void CGameHandler::handleSpellCasting( SpellID spellID, int spellLvl, BattleHex
 	}
 
 	//checking if creatures resist
-	sc.resisted = gs->curB->calculateResistedStacks(spell, caster, secHero, attackedCres, casterColor, mode, usedSpellPower, spellLvl);
+	sc.resisted = gs->curB->calculateResistedStacks(spell, caster, secHero, attackedCres, casterColor, mode, usedSpellPower, spellLvl, gs->getRandomGenerator());
 
 	//calculating dmg to display
 	if (spellID == SpellID::DEATH_STARE || spellID == SpellID::ACID_BREATH_DAMAGE)
@@ -4465,7 +4458,7 @@ void CGameHandler::handleSpellCasting( SpellID spellID, int spellLvl, BattleHex
 		for(auto & attackedCre : attackedCres)
 		{
 			int mirrorChance = (attackedCre)->valOfBonuses(Bonus::MAGIC_MIRROR);
-			if(mirrorChance > rand()%100)
+			if(mirrorChance > gs->getRandomGenerator().nextInt(99))
 			{
 				std::vector<CStack *> mirrorTargets;
 				std::vector<CStack *> & battleStacks = gs->curB->stacks;
@@ -4479,7 +4472,7 @@ void CGameHandler::handleSpellCasting( SpellID spellID, int spellLvl, BattleHex
 				}
 				if (!mirrorTargets.empty())
 				{
-					int targetHex = mirrorTargets.at(rand() % mirrorTargets.size())->position;
+					int targetHex = (*RandomGeneratorUtil::nextItem(mirrorTargets, gs->getRandomGenerator()))->position;
 					handleSpellCasting(spellID, 0, targetHex, 1 - casterSide, (attackedCre)->owner, nullptr, (caster ? caster : nullptr), usedSpellPower, ECastingMode::MAGIC_MIRROR, (attackedCre));
 				}
 			}
@@ -4652,7 +4645,7 @@ void CGameHandler::stackTurnTrigger(const CStack * st)
 			}
 			if (fearsomeCreature)
 			{
-				if (rand() % 100 < 10) //fixed 10%
+				if (gs->getRandomGenerator().nextInt(99) < 10) //fixed 10%
 				{
 					bte.effect = Bonus::FEAR;
 					sendAndApply(&bte);
@@ -4663,18 +4656,18 @@ void CGameHandler::stackTurnTrigger(const CStack * st)
 		int side = gs->curB->whatSide(st->owner);
 		if (bl.size() && st->casts && !gs->curB->sides.at(side).enchanterCounter)
 		{
-			int index = rand() % bl.size();
-			SpellID spellID = SpellID(bl[index]->subtype);
+			auto bonus = *RandomGeneratorUtil::nextItem(bl, gs->getRandomGenerator());
+			auto spellID = SpellID(bonus->subtype);
 			if (gs->curB->battleCanCastThisSpell(st->owner, SpellID(spellID).toSpell(), ECastingMode::ENCHANTER_CASTING) == ESpellCastProblem::OK) //TODO: select another available?
 			{
-				int spellLeveL = bl[index]->val; //spell level
+				int spellLeveL = bonus->val; //spell level
 				const CGHeroInstance * enemyHero = gs->curB->getHero(gs->curB->theOtherPlayer(st->owner));
 				handleSpellCasting(spellID, spellLeveL, -1, side, st->owner, nullptr, enemyHero, 0, ECastingMode::ENCHANTER_CASTING, st);
 
 				BattleSetStackProperty ssp;
 				ssp.which = BattleSetStackProperty::ENCHANTER_COUNTER;
 				ssp.absolute = false;
-				ssp.val = bl[index]->additionalInfo; //increase cooldown counter
+				ssp.val = bonus->additionalInfo; //increase cooldown counter
 				ssp.stackID = st->ID;
 				sendAndApply(&ssp);
 			}
@@ -5345,7 +5338,7 @@ void CGameHandler::attackCasting(const BattleAttack & bat, Bonus::BonusType atta
 				continue;
 
 			//check if spell should be casted (probability handling)
-			if(rand()%100 >= chance)
+			if(gs->getRandomGenerator().nextInt(99) >= chance)
 				continue;
 
 			//casting //TODO: check if spell can be blocked or target is immune
@@ -5383,10 +5376,10 @@ void CGameHandler::handleAfterAttackCasting( const BattleAttack & bat )
 		double chanceToKill = attacker->valOfBonuses(Bonus::DEATH_STARE, 0) / 100.0f;
 		vstd::amin(chanceToKill, 1); //cap at 100%
 
-		std::binomial_distribution<> distr(attacker->count, chanceToKill);
-		std::mt19937 rng(rand());
+		std::binomial_distribution<> distribution(attacker->count, chanceToKill);
+		std::mt19937 rng(std::time(nullptr));
 
-		int staredCreatures = distr(rng);
+		int staredCreatures = distribution(rng);
 
 		double cap = 1 / std::max(chanceToKill, (double)(0.01));//don't divide by 0
 		int maxToKill = (attacker->count + cap - 1) / cap; //not much more than chance * count
@@ -5405,7 +5398,7 @@ void CGameHandler::handleAfterAttackCasting( const BattleAttack & bat )
 	TBonusListPtr acidBreath = attacker->getBonuses(Selector::type(Bonus::ACID_BREATH));
 	for(const Bonus *b : *acidBreath)
 	{
-		if (b->additionalInfo > rand()%100)
+		if (b->additionalInfo > gs->getRandomGenerator().nextInt(99))
 			acidDamage += b->val;
 	}
 	if (acidDamage)
@@ -5439,7 +5432,7 @@ bool CGameHandler::castSpell(const CGHeroInstance *h, SpellID spellID, const int
 	case SpellID::SUMMON_BOAT:
 		{
 			//check if spell works at all
-			if(rand() % 100 >= s->getPower(schoolLevel)) //power is % chance of success
+			if(gs->getRandomGenerator().nextInt(99) >= s->getPower(schoolLevel)) //power is % chance of success
 			{
 				InfoWindow iw;
 				iw.player = h->tempOwner;
@@ -5501,7 +5494,7 @@ bool CGameHandler::castSpell(const CGHeroInstance *h, SpellID spellID, const int
 	case SpellID::SCUTTLE_BOAT:
 		{
 			//check if spell works at all
-			if(rand() % 100 >= s->getPower(schoolLevel)) //power is % chance of success
+			if(gs->getRandomGenerator().nextInt(99) >= s->getPower(schoolLevel)) //power is % chance of success
 			{
 				InfoWindow iw;
 				iw.player = h->tempOwner;
@@ -5918,7 +5911,7 @@ void CGameHandler::runBattle()
 				   || NBonus::hasOfType(gs->curB->battleGetFightingHero(1), Bonus::BLOCK_MORALE)) //checking if gs->curB->heroes have (or don't have) morale blocking bonuses)
 				)
 			{
-				if( rand()%24   <   -2 * nextStackMorale)
+				if(gs->getRandomGenerator().nextInt(23) < -2 * nextStackMorale)
 				{
 					//unit loses its turn - empty freeze action
 					BattleAction ba;
@@ -5983,7 +5976,7 @@ void CGameHandler::runBattle()
 				if(!attackableBattleHexes.empty())
 				{
 					BattleAction attack;
-					attack.destinationTile = attackableBattleHexes[rand() % attackableBattleHexes.size()];
+					attack.destinationTile = *RandomGeneratorUtil::nextItem(attackableBattleHexes, gs->getRandomGenerator());
 					attack.actionType = Battle::CATAPULT;
 					attack.additionalInfo = 0;
 					attack.side = !next->attackerOwned;
@@ -6077,7 +6070,7 @@ void CGameHandler::runBattle()
 						|| NBonus::hasOfType(gs->curB->battleGetFightingHero(1), Bonus::BLOCK_MORALE)) //checking if gs->curB->heroes have (or don't have) morale blocking bonuses
 					)
 				{
-					if(rand()%24 < nextStackMorale) //this stack hasn't got morale this turn
+					if(gs->getRandomGenerator().nextInt(23) < nextStackMorale) //this stack hasn't got morale this turn
 
 						{
 							BattleTriggerEffect bte;

server/StdInc.h

@@ -8,7 +8,6 @@
 //#include <boost/interprocess/shared_memory_object.hpp>
 #include <boost/random/linear_congruential.hpp>
 #include <boost/random/mersenne_twister.hpp>
-#include <boost/random/poisson_distribution.hpp>
 #include <boost/random/variate_generator.hpp>
 #include <boost/system/system_error.hpp>
 //#include <boost/asio.hpp>