Merge with vcmi/develop branch

.github/ISSUE_TEMPLATE/bug_report.md

@@ -0,0 +1,40 @@
+---
+name: Bug report
+about: Report an issue to help us improve
+title: ''
+labels: ["bug"]
+assignees: ''
+
+---
+
+**Describe the bug**
+A clear and concise description of what the bug is.
+
+**Game logs**
+Please attach game logs: `VCMI_client.txt`, `VCMI_server.txt` etc.
+
+**To Reproduce**
+Steps to reproduce the behavior:
+1. Go to '...'
+2. Click on '....'
+3. Scroll down to '....'
+4. See error
+
+**Expected behavior**
+A clear and concise description of what you expected to happen.
+
+**Actual behavior**
+A clear description what is currently happening 
+
+**Did it work earlier?**
+If this something which worked well some time ago, please let us know about version where it works or at date when it worked.
+
+**Screenshots**
+If applicable, add screenshots to help explain your problem.
+
+**Version**
+ - OS: [e.g. Windows, macOS Intel, macOS ARM, Android, Linux, iOS]
+ - Version: [VCMI version]
+
+**Additional context**
+Add any other context about the problem here.

.github/ISSUE_TEMPLATE/feature_request.md

@@ -0,0 +1,14 @@
+---
+name: Feature request
+about: Suggest an improvement
+title: ''
+labels: ["enhancement"]
+assignees: ''
+
+---
+
+**Describe your proposal**
+Give us as many as possible details about your idea.
+
+**Additional context**
+Add any other context or screenshots about the feature request here.

AI/BattleAI/AttackPossibility.cpp

@@ -156,7 +156,6 @@ AttackPossibility AttackPossibility::evaluate(const BattleAttackInfo & attackInf
 
 				TDmgRange retaliation(0, 0);
 				auto attackDmg = state.battleEstimateDamage(ap.attack, &retaliation);
-				TDmgRange defenderDamageBeforeAttack = state.battleEstimateDamage(BattleAttackInfo(u, attacker, u->canShoot()));
 
 				vstd::amin(attackDmg.first, defenderState->getAvailableHealth());
 				vstd::amin(attackDmg.second, defenderState->getAvailableHealth());

AI/BattleAI/BattleAI.cpp

@@ -79,7 +79,7 @@ CBattleAI::~CBattleAI()
 	}
 }
 
-void CBattleAI::init(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB)
+void CBattleAI::initBattleInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB)
 {
 	setCbc(CB);
 	env = ENV;
@@ -186,7 +186,6 @@ BattleAction CBattleAI::activeStack( const CStack * stack )
 
 			if(evaluationResult.score > score)
 			{
-				auto & target = bestAttack;
 				score = evaluationResult.score;
 				std::string action;
 

AI/BattleAI/BattleAI.h

@@ -65,7 +65,7 @@ public:
 	CBattleAI();
 	~CBattleAI();
 
-	void init(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB) override;
+	void initBattleInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB) override;
 	void attemptCastingSpell();
 
 	void evaluateCreatureSpellcast(const CStack * stack, PossibleSpellcast & ps); //for offensive damaging spells only

AI/BattleAI/BattleExchangeVariant.cpp

@@ -215,8 +215,6 @@ MoveTarget BattleExchangeEvaluator::findMoveTowardsUnreachable(const battle::Uni
 
 	for(const battle::Unit * enemy : targets.unreachableEnemies)
 	{
-		int64_t stackScore = EvaluationResult::INEFFECTIVE_SCORE;
-
 		std::vector<const battle::Unit *> adjacentStacks = getAdjacentUnits(enemy);
 		auto closestStack = *vstd::minElementByFun(adjacentStacks, [&](const battle::Unit * u) -> int64_t
 			{

AI/EmptyAI/CEmptyAI.cpp

@@ -20,7 +20,7 @@ void CEmptyAI::loadGame(BinaryDeserializer & h, const int version)
 {
 }
 
-void CEmptyAI::init(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB)
+void CEmptyAI::initGameInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB)
 {
 	cb = CB;
 	env = ENV;

AI/EmptyAI/CEmptyAI.h

@@ -22,7 +22,7 @@ public:
 	virtual void saveGame(BinarySerializer & h, const int version) override;
 	virtual void loadGame(BinaryDeserializer & h, const int version) override;
 
-	void init(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB) override;
+	void initGameInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB) override;
 	void yourTurn() override;
 	void heroGotLevel(const CGHeroInstance *hero, PrimarySkill::PrimarySkill pskill, std::vector<SecondarySkill> &skills, QueryID queryID) override;
 	void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, QueryID queryID) override;

AI/Nullkiller/AIGateway.cpp

@@ -28,8 +28,8 @@ namespace NKAI
 {
 
 // our to enemy strength ratio constants
-const float SAFE_ATTACK_CONSTANT = 1.2;
-const float RETREAT_THRESHOLD = 0.3;
+const float SAFE_ATTACK_CONSTANT = 1.2f;
+const float RETREAT_THRESHOLD = 0.3f;
 const double RETREAT_ABSOLUTE_THRESHOLD = 10000.;
 
 //one thread may be turn of AI and another will be handling a side effect for AI2
@@ -92,8 +92,9 @@ void AIGateway::heroMoved(const TryMoveHero & details, bool verbose)
 	validateObject(details.id); //enemy hero may have left visible area
 	auto hero = cb->getHero(details.id);
 
-	const int3 from = CGHeroInstance::convertPosition(details.start, false);
-	const int3 to = CGHeroInstance::convertPosition(details.end, false);
+	const int3 from = hero ? hero->convertToVisitablePos(details.start) : (details.start - int3(0,1,0));;
+	const int3 to   = hero ? hero->convertToVisitablePos(details.end)   : (details.end   - int3(0,1,0));
+
 	const CGObjectInstance * o1 = vstd::frontOrNull(cb->getVisitableObjs(from, verbose));
 	const CGObjectInstance * o2 = vstd::frontOrNull(cb->getVisitableObjs(to, verbose));
 
@@ -514,7 +515,7 @@ boost::optional<BattleAction> AIGateway::makeSurrenderRetreatDecision(
 }
 
 
-void AIGateway::init(std::shared_ptr<Environment> env, std::shared_ptr<CCallback> CB)
+void AIGateway::initGameInterface(std::shared_ptr<Environment> env, std::shared_ptr<CCallback> CB)
 {
 	LOG_TRACE(logAi);
 	myCb = CB;
@@ -535,8 +536,7 @@ void AIGateway::yourTurn()
 	LOG_TRACE(logAi);
 	NET_EVENT_HANDLER;
 	status.startedTurn();
-
-	makingTurn = make_unique<boost::thread>(&AIGateway::makeTurn, this);
+	makingTurn = std::make_unique<boost::thread>(&AIGateway::makeTurn, this);
 }
 
 void AIGateway::heroGotLevel(const CGHeroInstance * hero, PrimarySkill::PrimarySkill pskill, std::vector<SecondarySkill> & skills, QueryID queryID)
@@ -595,7 +595,7 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 
 					logAi->trace("Guarded object query hook: %s by %s danger ratio %f", target.toString(), hero.name, ratio);
 
-					if(text.find("guarded") >= 0 && (dangerUnknown || dangerTooHigh))
+					if(text.find("guarded") != std::string::npos && (dangerUnknown || dangerTooHigh))
 						answer = 0; // no
 				}
 			}
@@ -732,7 +732,7 @@ bool AIGateway::makePossibleUpgrades(const CArmedInstance * obj)
 		if(const CStackInstance * s = obj->getStackPtr(SlotID(i)))
 		{
 			UpgradeInfo ui;
-			myCb->getUpgradeInfo(obj, SlotID(i), ui);
+			myCb->fillUpgradeInfo(obj, SlotID(i), ui);
 			if(ui.oldID >= 0 && nullkiller->getFreeResources().canAfford(ui.cost[0] * s->count))
 			{
 				myCb->upgradeCreature(obj, SlotID(i), ui.newID[0]);
@@ -1179,7 +1179,7 @@ bool AIGateway::moveHeroToTile(int3 dst, HeroPtr h)
 	{
 		//FIXME: this assertion fails also if AI moves onto defeated guarded object
 		assert(cb->getVisitableObjs(dst).size() > 1); //there's no point in revisiting tile where there is no visitable object
-		cb->moveHero(*h, CGHeroInstance::convertPosition(dst, true));
+		cb->moveHero(*h, h->convertFromVisitablePos(dst));
 		afterMovementCheck(); // TODO: is it feasible to hero get killed there if game work properly?
 		// If revisiting, teleport probing is never done, and so the entries into the list would remain unused and uncleared
 		teleportChannelProbingList.clear();
@@ -1233,14 +1233,14 @@ bool AIGateway::moveHeroToTile(int3 dst, HeroPtr h)
 
 		auto doMovement = [&](int3 dst, bool transit)
 		{
-			cb->moveHero(*h, CGHeroInstance::convertPosition(dst, true), transit);
+			cb->moveHero(*h, h->convertFromVisitablePos(dst), transit);
 		};
 
 		auto doTeleportMovement = [&](ObjectInstanceID exitId, int3 exitPos)
 		{
 			destinationTeleport = exitId;
 			if(exitPos.valid())
-				destinationTeleportPos = CGHeroInstance::convertPosition(exitPos, true);
+				destinationTeleportPos = h->convertFromVisitablePos(exitPos);
 			cb->moveHero(*h, h->pos);
 			destinationTeleport = ObjectInstanceID();
 			destinationTeleportPos = int3(-1);
@@ -1249,7 +1249,7 @@ bool AIGateway::moveHeroToTile(int3 dst, HeroPtr h)
 
 		auto doChannelProbing = [&]() -> void
 		{
-			auto currentPos = CGHeroInstance::convertPosition(h->pos, false);
+			auto currentPos = h->visitablePos();
 			auto currentExit = getObj(currentPos, true)->id;
 
 			status.setChannelProbing(true);
@@ -1266,7 +1266,7 @@ bool AIGateway::moveHeroToTile(int3 dst, HeroPtr h)
 			int3 currentCoord = path.nodes[i].coord;
 			int3 nextCoord = path.nodes[i - 1].coord;
 
-			auto currentObject = getObj(currentCoord, currentCoord == CGHeroInstance::convertPosition(h->pos, false));
+			auto currentObject = getObj(currentCoord, currentCoord == h->visitablePos());
 			auto nextObjectTop = getObj(nextCoord, false);
 			auto nextObject = getObj(nextCoord, true);
 			auto destTeleportObj = getDestTeleportObj(currentObject, nextObjectTop, nextObject);

AI/Nullkiller/AIGateway.h

@@ -110,7 +110,7 @@ public:
 
 	std::string getBattleAIName() const override;
 
-	void init(std::shared_ptr<Environment> env, std::shared_ptr<CCallback> CB) override;
+	void initGameInterface(std::shared_ptr<Environment> env, std::shared_ptr<CCallback> CB) override;
 	void yourTurn() override;
 
 	void heroGotLevel(const CGHeroInstance * hero, PrimarySkill::PrimarySkill pskill, std::vector<SecondarySkill> & skills, QueryID queryID) override; //pskill is gained primary skill, interface has to choose one of given skills and call callback with selection id

AI/Nullkiller/AIUtility.h

@@ -329,7 +329,7 @@ public:
 
 		if(!poolIsEmpty) pool.pop_back();
 
-		return std::move(tmp);
+		return tmp;
 	}
 
 	bool empty() const

AI/Nullkiller/Engine/FuzzyHelper.cpp

@@ -122,10 +122,8 @@ ui64 FuzzyHelper::evaluateDanger(const CGObjectInstance * obj)
 	case Obj::RESOURCE:
 	{
 		if(!vstd::contains(ai->memory->alreadyVisited, obj))
-		{
 			return 0;
-		}
-		// passthrough
+		FALLTHROUGH;
 	}
 	case Obj::MONSTER:
 	case Obj::HERO:

AI/Nullkiller/Engine/Nullkiller.cpp

@@ -50,7 +50,7 @@ void Nullkiller::init(std::shared_ptr<CCallback> cb, PlayerColor playerID)
 		new SharedPool<PriorityEvaluator>(
 			[&]()->std::unique_ptr<PriorityEvaluator>
 			{
-				return make_unique<PriorityEvaluator>(this);
+				return std::make_unique<PriorityEvaluator>(this);
 			}));
 
 	dangerHitMap.reset(new DangerHitMapAnalyzer(this));

AI/Nullkiller/Goals/ExecuteHeroChain.cpp

@@ -127,7 +127,7 @@ void ExecuteHeroChain::accept(AIGateway * ai)
 							continue;
 						}
 					}
-					catch(cannotFulfillGoalException &)
+					catch(const cannotFulfillGoalException &)
 					{
 						if(!heroPtr.validAndSet())
 						{
@@ -173,7 +173,7 @@ void ExecuteHeroChain::accept(AIGateway * ai)
 			ai->nullkiller->lockHero(hero, HeroLockedReason::HERO_CHAIN);
 			blockedIndexes.insert(node.parentIndex);
 		}
-		catch(goalFulfilledException &)
+		catch(const goalFulfilledException &)
 		{
 			if(!heroPtr.validAndSet())
 			{

AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -401,6 +401,9 @@ public:
 
 	void execute(const blocked_range<size_t>& r)
 	{
+		std::random_device randomDevice;
+		std::mt19937 randomEngine(randomDevice());
+
 		for(int i = r.begin(); i != r.end(); i++)
 		{
 			auto & pos = tiles[i];
@@ -422,7 +425,7 @@ public:
 						existingChains.push_back(&node);
 				}
 
-				std::random_shuffle(existingChains.begin(), existingChains.end());
+				std::shuffle(existingChains.begin(), existingChains.end(), randomEngine);
 
 				for(AIPathNode * node : existingChains)
 				{
@@ -480,6 +483,9 @@ public:
 
 bool AINodeStorage::calculateHeroChain()
 {
+	std::random_device randomDevice;
+	std::mt19937 randomEngine(randomDevice());
+
 	heroChainPass = EHeroChainPass::CHAIN;
 	heroChain.clear();
 
@@ -489,7 +495,7 @@ bool AINodeStorage::calculateHeroChain()
 	{
 		boost::mutex resultMutex;
 
-		std::random_shuffle(data.begin(), data.end());
+		std::shuffle(data.begin(), data.end(), randomEngine);
 
 		parallel_for(blocked_range<size_t>(0, data.size()), [&](const blocked_range<size_t>& r)
 		{

AI/StupidAI/StupidAI.cpp

@@ -28,7 +28,7 @@ CStupidAI::~CStupidAI()
 	print("destroyed");
 }
 
-void CStupidAI::init(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB)
+void CStupidAI::initBattleInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB)
 {
 	print("init called, saving ptr to IBattleCallback");
 	env = ENV;

AI/StupidAI/StupidAI.h

@@ -25,7 +25,7 @@ public:
 	CStupidAI();
 	~CStupidAI();
 
-	void init(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB) override;
+	void initBattleInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB) override;
 	void actionFinished(const BattleAction &action) override;//occurs AFTER every action taken by any stack or by the hero
 	void actionStarted(const BattleAction &action) override;//occurs BEFORE every action taken by any stack or by the hero
 	BattleAction activeStack(const CStack * stack) override; //called when it's turn of that stack

AI/VCAI/Goals/Explore.cpp

@@ -50,7 +50,7 @@ namespace Goals
 			sightRadius = hero->getSightRadius();
 			bestGoal = sptr(Goals::Invalid());
 			bestValue = 0;
-			ourPos = h->convertPosition(h->pos, false);
+			ourPos = h->visitablePos();
 			allowDeadEndCancellation = true;
 			allowGatherArmy = gatherArmy;
 		}

AI/VCAI/Pathfinding/AINodeStorage.cpp

@@ -107,7 +107,7 @@ boost::optional<AIPathNode *> AINodeStorage::getOrCreateNode(const int3 & pos, c
 
 std::vector<CGPathNode *> AINodeStorage::getInitialNodes()
 {
-	auto hpos = hero->getPosition(false);
+	auto hpos = hero->visitablePos();
 	auto initialNode =
 		getOrCreateNode(hpos, hero->boat ? EPathfindingLayer::SAIL : EPathfindingLayer::LAND, NORMAL_CHAIN)
 		.get();
@@ -211,7 +211,7 @@ std::vector<CGPathNode *> AINodeStorage::calculateTeleportations(
 		}
 	}
 
-	if(hero->getPosition(false) == source.coord)
+	if(hero->visitablePos() == source.coord)
 	{
 		calculateTownPortalTeleportations(source, neighbours);
 	}

AI/VCAI/VCAI.cpp

@@ -98,11 +98,13 @@ void VCAI::heroMoved(const TryMoveHero & details, bool verbose)
 	LOG_TRACE(logAi);
 	NET_EVENT_HANDLER;
 
-	validateObject(details.id); //enemy hero may have left visible area
+	//enemy hero may have left visible area
+	validateObject(details.id);
 	auto hero = cb->getHero(details.id);
 
-	const int3 from = CGHeroInstance::convertPosition(details.start, false);
-	const int3 to = CGHeroInstance::convertPosition(details.end, false);
+	const int3 from = hero ? hero->convertToVisitablePos(details.start) : (details.start - int3(0,1,0));;
+	const int3 to   = hero ? hero->convertToVisitablePos(details.end)   : (details.end   - int3(0,1,0));
+
 	const CGObjectInstance * o1 = vstd::frontOrNull(cb->getVisitableObjs(from, verbose));
 	const CGObjectInstance * o2 = vstd::frontOrNull(cb->getVisitableObjs(to, verbose));
 
@@ -583,7 +585,7 @@ void VCAI::showWorldViewEx(const std::vector<ObjectPosInfo> & objectPositions)
 	NET_EVENT_HANDLER;
 }
 
-void VCAI::init(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB)
+void VCAI::initGameInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB)
 {
 	LOG_TRACE(logAi);
 	env = ENV;
@@ -608,7 +610,7 @@ void VCAI::yourTurn()
 	LOG_TRACE(logAi);
 	NET_EVENT_HANDLER;
 	status.startedTurn();
-	makingTurn = make_unique<boost::thread>(&VCAI::makeTurn, this);
+	makingTurn = std::make_unique<boost::thread>(&VCAI::makeTurn, this);
 }
 
 void VCAI::heroGotLevel(const CGHeroInstance * hero, PrimarySkill::PrimarySkill pskill, std::vector<SecondarySkill> & skills, QueryID queryID)
@@ -756,7 +758,7 @@ void makePossibleUpgrades(const CArmedInstance * obj)
 		if(const CStackInstance * s = obj->getStackPtr(SlotID(i)))
 		{
 			UpgradeInfo ui;
-			cb->getUpgradeInfo(obj, SlotID(i), ui);
+			cb->fillUpgradeInfo(obj, SlotID(i), ui);
 			if(ui.oldID >= 0 && cb->getResourceAmount().canAfford(ui.cost[0] * s->count))
 			{
 				cb->upgradeCreature(obj, SlotID(i), ui.newID[0]);
@@ -1813,7 +1815,7 @@ bool VCAI::moveHeroToTile(int3 dst, HeroPtr h)
 	{
 		//FIXME: this assertion fails also if AI moves onto defeated guarded object
 		assert(cb->getVisitableObjs(dst).size() > 1); //there's no point in revisiting tile where there is no visitable object
-		cb->moveHero(*h, CGHeroInstance::convertPosition(dst, true));
+		cb->moveHero(*h, h->convertFromVisitablePos(dst));
 		afterMovementCheck(); // TODO: is it feasible to hero get killed there if game work properly?
 		// If revisiting, teleport probing is never done, and so the entries into the list would remain unused and uncleared
 		teleportChannelProbingList.clear();
@@ -1867,14 +1869,14 @@ bool VCAI::moveHeroToTile(int3 dst, HeroPtr h)
 
 		auto doMovement = [&](int3 dst, bool transit)
 		{
-			cb->moveHero(*h, CGHeroInstance::convertPosition(dst, true), transit);
+			cb->moveHero(*h, h->convertFromVisitablePos(dst), transit);
 		};
 
 		auto doTeleportMovement = [&](ObjectInstanceID exitId, int3 exitPos)
 		{
 			destinationTeleport = exitId;
 			if(exitPos.valid())
-				destinationTeleportPos = CGHeroInstance::convertPosition(exitPos, true);
+				destinationTeleportPos = h->convertFromVisitablePos(exitPos);
 			cb->moveHero(*h, h->pos);
 			destinationTeleport = ObjectInstanceID();
 			destinationTeleportPos = int3(-1);
@@ -1883,7 +1885,7 @@ bool VCAI::moveHeroToTile(int3 dst, HeroPtr h)
 
 		auto doChannelProbing = [&]() -> void
 		{
-			auto currentPos = CGHeroInstance::convertPosition(h->pos, false);
+			auto currentPos = h->visitablePos();
 			auto currentExit = getObj(currentPos, true)->id;
 
 			status.setChannelProbing(true);
@@ -1900,7 +1902,7 @@ bool VCAI::moveHeroToTile(int3 dst, HeroPtr h)
 			int3 currentCoord = path.nodes[i].coord;
 			int3 nextCoord = path.nodes[i - 1].coord;
 
-			auto currentObject = getObj(currentCoord, currentCoord == CGHeroInstance::convertPosition(h->pos, false));
+			auto currentObject = getObj(currentCoord, currentCoord == h->visitablePos());
 			auto nextObjectTop = getObj(nextCoord, false);
 			auto nextObject = getObj(nextCoord, true);
 			auto destTeleportObj = getDestTeleportObj(currentObject, nextObjectTop, nextObject);

AI/VCAI/VCAI.h

@@ -143,7 +143,7 @@ public:
 
 	std::string getBattleAIName() const override;
 
-	void init(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB) override;
+	void initGameInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB) override;
 	void yourTurn() override;
 
 	void heroGotLevel(const CGHeroInstance * hero, PrimarySkill::PrimarySkill pskill, std::vector<SecondarySkill> & skills, QueryID queryID) override; //pskill is gained primary skill, interface has to choose one of given skills and call callback with selection id

CCallback.cpp

@@ -335,11 +335,6 @@ int3 CCallback::getGuardingCreaturePosition(int3 tile)
 	return gs->map->guardingCreaturePositions[tile.z][tile.x][tile.y];
 }
 
-void CCallback::calculatePaths( const CGHeroInstance *hero, CPathsInfo &out)
-{
-	gs->calculatePaths(hero, out);
-}
-
 void CCallback::dig( const CGObjectInstance *hero )
 {
 	DigWithHero dwh;
@@ -400,4 +395,4 @@ boost::optional<BattleAction> CBattleCallback::makeSurrenderRetreatDecision(
 	const BattleStateInfoForRetreat & battleState)
 {
 	return cl->playerint[getPlayerID().get()]->makeSurrenderRetreatDecision(battleState);
-}
+}

CCallback.h

@@ -133,8 +133,6 @@ public:
 	virtual int3 getGuardingCreaturePosition(int3 tile);
 	virtual std::shared_ptr<const CPathsInfo> getPathsInfo(const CGHeroInstance * h);
 
-	virtual void calculatePaths(const CGHeroInstance *hero, CPathsInfo &out);
-
 	//Set of metrhods that allows adding more interfaces for this player that'll receive game event call-ins.
 	void registerBattleInterface(std::shared_ptr<IBattleEventsReceiver> battleEvents);
 	void unregisterBattleInterface(std::shared_ptr<IBattleEventsReceiver> battleEvents);

CMakeLists.txt

@@ -60,6 +60,7 @@ if(NOT ${CMAKE_VERSION} VERSION_LESS "3.16.0")
 endif(NOT ${CMAKE_VERSION} VERSION_LESS "3.16.0")
 option(ENABLE_GITVERSION "Enable Version.cpp with Git commit hash" ON)
 option(ENABLE_DEBUG_CONSOLE "Enable debug console for Windows builds" ON)
+option(ENABLE_STRICT_COMPILATION "Treat all compiler warnings as errors" OFF)
 option(ENABLE_MULTI_PROCESS_BUILDS "Enable /MP flag for MSVS solution" ON)
 option(ENABLE_SINGLE_APP_BUILD "Builds client and server as single executable" OFF)
 option(COPY_CONFIG_ON_BUILD "Copies config folder into output directory at building phase" ON)
@@ -211,10 +212,18 @@ if(MINGW OR MSVC)
 		# Suppress warnings
 		add_definitions(-D_CRT_SECURE_NO_WARNINGS)
 		add_definitions(-D_SCL_SECURE_NO_WARNINGS)
-		# 4250: 'class1' : inherits 'class2::member' via dominance
-		# 4251: class 'xxx' needs to have dll-interface to be used by clients of class 'yyy'
-		# 4275: non dll-interface class 'xxx' used as base for dll-interface class
-		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /bigobj /wd4250 /wd4251 /wd4275")
+
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /bigobj")
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4250") # 4250: 'class1' : inherits 'class2::member' via dominance
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4251") # 4251: class 'xxx' needs to have dll-interface to be used by clients of class 'yyy'
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4244") # 4244: conversion from 'xxx' to 'yyy', possible loss of data
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4267") # 4267: conversion from 'xxx' to 'yyy', possible loss of data
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4275") # 4275: non dll-interface class 'xxx' used as base for dll-interface class
+		#set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4800") # 4800: implicit conversion from 'xxx' to bool. Possible information loss
+
+		if(ENABLE_STRICT_COMPILATION)
+			set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wx") # Treats all compiler warnings as errors
+		endif()
 
 		if(ENABLE_MULTI_PROCESS_BUILDS)
 			set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /MP")
@@ -249,19 +258,30 @@ if(MINGW OR MSVC)
 	endif(MINGW)
 endif(MINGW OR MSVC)
 
-if(CMAKE_COMPILER_IS_GNUCXX OR NOT WIN32) #so far all *nix compilers support such parameters
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wpointer-arith -Wuninitialized")
+if(CMAKE_COMPILER_IS_GNUCXX OR NOT WIN32)
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra")
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wpointer-arith")
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wuninitialized")
+
 	if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 10.0 OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang" )
 		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wmismatched-tags")
 	endif()
 
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-strict-aliasing -Wno-switch -Wno-sign-compare -Wno-unused-local-typedefs")
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-parameter -Wno-overloaded-virtual -Wno-type-limits -Wno-unknown-pragmas")
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-reorder")
-	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-varargs") # fuzzylite - Operation.h
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-parameter")   # low chance of valid reports, a lot of emitted warnings
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-switch")             # large number of false-positives, disabled
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-reorder")            # large number of noise, low chance of any significant issues
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-sign-compare")       # low chance of any significant issues
+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-varargs")            # emitted in fuzzylite headers, disabled
+
+	if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 6.0)
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-pragmas") # emitted only by ancient gcc 5.5 in MXE build, remove after upgrade
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable") # emitted only by ancient gcc 5.5 in MXE build, remove after upgrade
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-maybe-uninitialized") # emitted only by ancient gcc 5.5 in MXE build, remove after upgrade
+	endif()
 
-	if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-warning-option")
+	if(ENABLE_STRICT_COMPILATION)
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
+		set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=array-bounds") # false positives in boost::multiarray during release build, keep as warning-only
 	endif()
 
 	if(UNIX)

Global.h

@@ -15,13 +15,6 @@
 // Fixed width bool data type is important for serialization
 static_assert(sizeof(bool) == 1, "Bool needs to be 1 byte in size.");
 
-/* ---------------------------------------------------------------------------- */
-/* Suppress some compiler warnings */
-/* ---------------------------------------------------------------------------- */
-#ifdef _MSC_VER
-#  pragma warning (disable : 4800 ) /* disable conversion to bool warning -- I think it's intended in all places */
-#endif
-
 /* ---------------------------------------------------------------------------- */
 /* System detection. */
 /* ---------------------------------------------------------------------------- */
@@ -71,6 +64,7 @@ static_assert(sizeof(bool) == 1, "Bool needs to be 1 byte in size.");
 #endif
 
 // Each compiler uses own way to supress fall through warning. Try to find it.
+// TODO: replace with c++17 [[fallthrough]]
 #ifdef __has_cpp_attribute
 #  if __has_cpp_attribute(fallthrough)
 #    define FALLTHROUGH [[fallthrough]];
@@ -89,9 +83,15 @@ static_assert(sizeof(bool) == 1, "Bool needs to be 1 byte in size.");
 /* Commonly used C++, Boost headers */
 /* ---------------------------------------------------------------------------- */
 #ifdef VCMI_WINDOWS
-#  define WIN32_LEAN_AND_MEAN		// Exclude rarely-used stuff from Windows headers - delete this line if something is missing.
-#  define NOMINMAX					// Exclude min/max macros from <Windows.h>. Use std::[min/max] from <algorithm> instead.
-#  define _NO_W32_PSEUDO_MODIFIERS  // Exclude more macros for compiling with MinGW on Linux.
+#  ifndef WIN32_LEAN_AND_MEAN
+#    define WIN32_LEAN_AND_MEAN		 // Exclude rarely-used stuff from Windows headers - delete this line if something is missing.
+#  endif
+#  ifndef NOMINMAX
+#    define NOMINMAX				 // Exclude min/max macros from <Windows.h>. Use std::[min/max] from <algorithm> instead.
+#  endif
+#  ifndef _NO_W32_PSEUDO_MODIFIERS
+#    define _NO_W32_PSEUDO_MODIFIERS // Exclude more macros for compiling with MinGW on Linux.
+#  endif
 #endif
 
 #ifdef VCMI_ANDROID
@@ -249,7 +249,8 @@ template<typename T, size_t N> char (&_ArrayCountObj(const T (&)[N]))[N];
 #define ARRAY_COUNT(arr)    (sizeof(_ArrayCountObj(arr)))
 
 // should be used for variables that becomes unused in release builds (e.g. only used for assert checks)
-#define UNUSED(VAR) ((void)VAR)
+// TODO: replace with c++17 [[maybe_unused]]
+#define MAYBE_UNUSED(VAR) ((void)VAR)
 
 // old iOS SDKs compatibility
 #ifdef VCMI_IOS
@@ -490,32 +491,6 @@ namespace vstd
 		ptr = nullptr;
 	}
 
-	template<typename T>
-	std::unique_ptr<T> make_unique()
-	{
-		return std::unique_ptr<T>(new T());
-	}
-	template<typename T, typename Arg1>
-	std::unique_ptr<T> make_unique(Arg1 &&arg1)
-	{
-		return std::unique_ptr<T>(new T(std::forward<Arg1>(arg1)));
-	}
-	template<typename T, typename Arg1, typename Arg2>
-	std::unique_ptr<T> make_unique(Arg1 &&arg1, Arg2 &&arg2)
-	{
-		return std::unique_ptr<T>(new T(std::forward<Arg1>(arg1), std::forward<Arg2>(arg2)));
-	}
-	template<typename T, typename Arg1, typename Arg2, typename Arg3>
-	std::unique_ptr<T> make_unique(Arg1 &&arg1, Arg2 &&arg2, Arg3 &&arg3)
-	{
-		return std::unique_ptr<T>(new T(std::forward<Arg1>(arg1), std::forward<Arg2>(arg2), std::forward<Arg3>(arg3)));
-	}
-	template<typename T, typename Arg1, typename Arg2, typename Arg3, typename Arg4>
-	std::unique_ptr<T> make_unique(Arg1 &&arg1, Arg2 &&arg2, Arg3 &&arg3, Arg4 &&arg4)
-	{
-		return std::unique_ptr<T>(new T(std::forward<Arg1>(arg1), std::forward<Arg2>(arg2), std::forward<Arg3>(arg3), std::forward<Arg4>(arg4)));
-	}
-
 	template <typename Container>
 	typename Container::const_reference circularAt(const Container &r, size_t index)
 	{
@@ -753,7 +728,6 @@ namespace vstd
 	using boost::math::round;
 }
 using vstd::operator-=;
-using vstd::make_unique;
 
 #ifdef NO_STD_TOSTRING
 namespace std

client/CMT.cpp

@@ -878,7 +878,7 @@ void processCommand(const std::string &message)
 	{
 		std::string URI;
 		readed >> URI;
-		std::unique_ptr<CAnimation> anim = make_unique<CAnimation>(URI);
+		std::unique_ptr<CAnimation> anim = std::make_unique<CAnimation>(URI);
 		anim->preload();
 		anim->exportBitmaps(VCMIDirs::get().userExtractedPath());
 	}
@@ -1079,7 +1079,8 @@ static bool recreateWindow(int w, int h, int bpp, bool fullscreen, int displayIn
 		if (displayIndex < 0)
 			displayIndex = 0;
 	}
-#ifdef VCMI_IOS
+
+#if defined(VCMI_ANDROID) || defined(VCMI_IOS)
 	SDL_GetWindowSize(mainWindow, &w, &h);
 #else
 	if(!checkVideoMode(displayIndex, w, h))

client/CMessage.cpp

@@ -71,7 +71,7 @@ void CMessage::init()
 {
 	for(int i=0; i<PlayerColor::PLAYER_LIMIT_I; i++)
 	{
-		dialogBorders[i] = make_unique<CAnimation>("DIALGBOX");
+		dialogBorders[i] = std::make_unique<CAnimation>("DIALGBOX");
 		dialogBorders[i]->preload();
 
 		for(int j=0; j < dialogBorders[i]->size(0); j++)

client/CMusicHandler.cpp

@@ -409,6 +409,8 @@ void CMusicHandler::release()
 
 void CMusicHandler::playMusic(const std::string & musicURI, bool loop, bool fromStart)
 {
+	boost::mutex::scoped_lock guard(mutex);
+
 	if (current && current->isPlaying() && current->isTrack(musicURI))
 		return;
 
@@ -422,6 +424,8 @@ void CMusicHandler::playMusicFromSet(const std::string & musicSet, const std::st
 
 void CMusicHandler::playMusicFromSet(const std::string & whichSet, bool loop, bool fromStart)
 {
+	boost::mutex::scoped_lock guard(mutex);
+
 	auto selectedSet = musicsSet.find(whichSet);
 	if (selectedSet == musicsSet.end())
 	{
@@ -441,8 +445,6 @@ void CMusicHandler::queueNext(std::unique_ptr<MusicEntry> queued)
 	if (!initialized)
 		return;
 
-	boost::mutex::scoped_lock guard(mutex);
-
 	next = std::move(queued);
 
 	if (current.get() == nullptr || !current->stop(1000))
@@ -456,7 +458,7 @@ void CMusicHandler::queueNext(CMusicHandler *owner, const std::string & setName,
 {
 	try
 	{
-		queueNext(make_unique<MusicEntry>(owner, setName, musicURI, looped, fromStart));
+		queueNext(std::make_unique<MusicEntry>(owner, setName, musicURI, looped, fromStart));
 	}
 	catch(std::exception &e)
 	{
@@ -487,13 +489,32 @@ void CMusicHandler::setVolume(ui32 percent)
 
 void CMusicHandler::musicFinishedCallback()
 {
-	boost::mutex::scoped_lock guard(mutex);
+	// boost::mutex::scoped_lock guard(mutex);
+	// FIXME: WORKAROUND FOR A POTENTIAL DEADLOCK
+	// It is possible for:
+	// 1) SDL thread to call this method on end of playback
+	// 2) VCMI code to call queueNext() method to queue new file
+	// this leads to:
+	// 1) SDL thread waiting to acquire music lock in this method (while keeping internal SDL mutex locked)
+	// 2) VCMI thread waiting to acquire internal SDL mutex (while keeping music mutex locked)
+	// Because of that (and lack of clear way to fix that)
+	// We will try to acquire lock here and if failed - do nothing
+	// This may break music playback till next song is enqued but won't deadlock the game
+
+	if (!mutex.try_lock())
+	{
+		logGlobal->error("Failed to acquire mutex! Unable to restart music!");
+		return;
+	}
 
 	if (current.get() != nullptr)
 	{
 		// if music is looped, play it again
 		if (current->play())
+		{
+			mutex.unlock();
 			return;
+		}
 		else
 			current.reset();
 	}
@@ -503,6 +524,7 @@ void CMusicHandler::musicFinishedCallback()
 		current.reset(next.release());
 		current->play();
 	}
+	mutex.unlock();
 }
 
 MusicEntry::MusicEntry(CMusicHandler *owner, std::string setName, std::string musicURI, bool looped, bool fromStart):
@@ -597,7 +619,7 @@ bool MusicEntry::play()
 
 bool MusicEntry::stop(int fade_ms)
 {
-	if (Mix_PlayingMusic())
+	if (playing)
 	{
 		playing = false;
 		loop = 0;

client/CPlayerInterface.cpp

@@ -149,7 +149,7 @@ CPlayerInterface::~CPlayerInterface()
 	if (LOCPLINT == this)
 		LOCPLINT = nullptr;
 }
-void CPlayerInterface::init(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB)
+void CPlayerInterface::initGameInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB)
 {
 	cb = CB;
 	env = ENV;
@@ -267,8 +267,8 @@ void CPlayerInterface::heroMoved(const TryMoveHero & details, bool verbose)
 				assert(adventureInt->terrain.currentPath->nodes.size() >= 2);
 				std::vector<CGPathNode>::const_iterator nodesIt = adventureInt->terrain.currentPath->nodes.end() - 1;
 
-				if((nodesIt)->coord == CGHeroInstance::convertPosition(details.start, false)
-					&& (nodesIt - 1)->coord == CGHeroInstance::convertPosition(details.end, false))
+				if((nodesIt)->coord == hero->convertToVisitablePos(details.start)
+					&& (nodesIt - 1)->coord == hero->convertToVisitablePos(details.end))
 				{
 					//path was between entrance and exit of teleport -> OK, erase node as usual
 					removeLastNodeFromPath(hero);
@@ -692,7 +692,7 @@ void CPlayerInterface::battleStart(const CCreatureSet *army1, const CCreatureSet
 	if (settings["adventure"]["quickCombat"].Bool())
 	{
 		autofightingAI = CDynLibHandler::getNewBattleAI(settings["server"]["friendlyAI"].String());
-		autofightingAI->init(env, cb);
+		autofightingAI->initBattleInterface(env, cb);
 		autofightingAI->battleStart(army1, army2, int3(0,0,0), hero1, hero2, side);
 		isAutoFightOn = true;
 		cb->registerBattleInterface(autofightingAI);
@@ -1492,7 +1492,7 @@ void CPlayerInterface::newObject( const CGObjectInstance * obj )
 	//we might have built a boat in shipyard in opened town screen
 	if (obj->ID == Obj::BOAT
 		&& LOCPLINT->castleInt
-		&&  obj->pos-obj->getVisitableOffset() == LOCPLINT->castleInt->town->bestLocation())
+		&&  obj->visitablePos() == LOCPLINT->castleInt->town->bestLocation())
 	{
 		CCS->soundh->playSound(soundBase::newBuilding);
 		LOCPLINT->castleInt->addBuilding(BuildingID::SHIP);
@@ -1927,7 +1927,7 @@ CGPath * CPlayerInterface::getAndVerifyPath(const CGHeroInstance * h)
 		}
 		else
 		{
-			assert(h->getPosition(false) == path.startPos());
+			assert(h->visitablePos() == path.startPos());
 			//update the hero path in case of something has changed on map
 			if (LOCPLINT->cb->getPathsInfo(h)->getPath(path, path.endPos()))
 				return &path;
@@ -2031,7 +2031,7 @@ void CPlayerInterface::acceptTurn()
 void CPlayerInterface::tryDiggging(const CGHeroInstance * h)
 {
 	int msgToShow = -1;
-	const bool isBlocked = CGI->mh->hasObjectHole(h->getPosition(false)); // Don't dig in the pit.
+	const bool isBlocked = CGI->mh->hasObjectHole(h->visitablePos()); // Don't dig in the pit.
 
 	const auto diggingStatus = isBlocked
 		? EDiggingStatus::TILE_OCCUPIED
@@ -2192,6 +2192,8 @@ void CPlayerInterface::artifactRemoved(const ArtifactLocation &al)
 		if (artWin)
 			artWin->artifactRemoved(al);
 	}
+
+	waitWhileDialog();
 }
 
 void CPlayerInterface::artifactMoved(const ArtifactLocation &src, const ArtifactLocation &dst)
@@ -2206,6 +2208,8 @@ void CPlayerInterface::artifactMoved(const ArtifactLocation &src, const Artifact
 	}
 	if(!GH.objsToBlit.empty())
 		GH.objsToBlit.back()->redraw();
+
+	waitWhileDialog();
 }
 
 void CPlayerInterface::artifactPossibleAssembling(const ArtifactLocation & dst)
@@ -2324,7 +2328,7 @@ void CPlayerInterface::doMoveHero(const CGHeroInstance * h, CGPath path)
 	int i = 1;
 	auto getObj = [&](int3 coord, bool ignoreHero)
 	{
-		return cb->getTile(CGHeroInstance::convertPosition(coord,false))->topVisitableObj(ignoreHero);
+		return cb->getTile(h->convertToVisitablePos(coord))->topVisitableObj(ignoreHero);
 	};
 
 	auto isTeleportAction = [&](CGPathNode::ENodeAction action) -> bool
@@ -2363,7 +2367,9 @@ void CPlayerInterface::doMoveHero(const CGHeroInstance * h, CGPath path)
 	};
 
 	{
-		path.convert(0);
+		for (auto & elem : path.nodes)
+			elem.coord = h->convertFromVisitablePos(elem.coord);
+
 		TerrainId currentTerrain = Terrain::BORDER; // not init yet
 		TerrainId newTerrain;
 		int sh = -1;
@@ -2420,7 +2426,7 @@ void CPlayerInterface::doMoveHero(const CGHeroInstance * h, CGPath path)
 				sh = CCS->soundh->playSound(soundBase::horseFlying, -1);
 #endif
 			{
-				newTerrain = cb->getTile(CGHeroInstance::convertPosition(currentCoord, false))->terType->id;
+				newTerrain = cb->getTile(h->convertToVisitablePos(currentCoord))->terType->id;
 				if(newTerrain != currentTerrain)
 				{
 					CCS->soundh->stopSound(sh);

client/CPlayerInterface.h

@@ -220,7 +220,7 @@ public:
 	void openTownWindow(const CGTownInstance * town); //shows townscreen
 	void openHeroWindow(const CGHeroInstance * hero); //shows hero window with given hero
 	void updateInfo(const CGObjectInstance * specific);
-	void init(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB) override;
+	void initGameInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB) override;
 	int3 repairScreenPos(int3 pos); //returns position closest to pos we can center screen on
 	void activateForSpectator(); // TODO: spectator probably need own player interface class
 

client/CServerHandler.cpp

@@ -132,7 +132,7 @@ void CServerHandler::resetStateForLobby(const StartInfo::EMode mode, const std::
 {
 	hostClientId = -1;
 	state = EClientState::NONE;
-	th = make_unique<CStopWatch>();
+	th = std::make_unique<CStopWatch>();
 	packsForLobbyScreen.clear();
 	c.reset();
 	si = std::make_shared<StartInfo>();

client/Client.cpp

@@ -501,7 +501,7 @@ void CClient::installNewPlayerInterface(std::shared_ptr<CGameInterface> gameInte
 	logGlobal->trace("\tInitializing the interface for player %s", color.getStr());
 	auto cb = std::make_shared<CCallback>(gs, color, this);
 	battleCallbacks[color] = cb;
-	gameInterface->init(playerEnvironments.at(color), cb);
+	gameInterface->initGameInterface(playerEnvironments.at(color), cb);
 
 	installNewBattleInterface(gameInterface, color, battlecb);
 }
@@ -517,7 +517,7 @@ void CClient::installNewBattleInterface(std::shared_ptr<CBattleGameInterface> ba
 		logGlobal->trace("\tInitializing the battle interface for player %s", color.getStr());
 		auto cbc = std::make_shared<CBattleCallback>(color, this);
 		battleCallbacks[color] = cbc;
-		battleInterface->init(playerEnvironments.at(color), cbc);
+		battleInterface->initBattleInterface(playerEnvironments.at(color), cbc);
 	}
 }
 
@@ -755,7 +755,7 @@ scripting::Pool * CClient::getContextPool() const
 
 void CClient::reinitScripting()
 {
-	clientEventBus = make_unique<events::EventBus>();
+	clientEventBus = std::make_unique<events::EventBus>();
 #if SCRIPTING_ENABLED
 	clientScripts.reset(new scripting::PoolImpl(this));
 #endif

client/battle/BattleInterface.cpp

@@ -320,6 +320,8 @@ void BattleInterface::battleFinished(const BattleResult& br)
 	stacksController->setActiveStack(nullptr);
 
 	CCS->curh->set(Cursor::Map::POINTER);
+	curInt->waitWhileDialog();
+
 	if(settings["session"]["spectate"].Bool() && settings["session"]["spectate-skip-battle-result"].Bool())
 	{
 		windowObject->close();
@@ -680,7 +682,7 @@ void BattleInterface::requestAutofightingAIToTakeAction()
 
 	boost::thread aiThread([&]()
 	{
-		auto ba = make_unique<BattleAction>(curInt->autofightingAI->activeStack(stacksController->getActiveStack()));
+		auto ba = std::make_unique<BattleAction>(curInt->autofightingAI->activeStack(stacksController->getActiveStack()));
 
 		if(curInt->cb->battleIsFinished())
 		{

client/battle/BattleWindow.cpp

@@ -294,7 +294,7 @@ void BattleWindow::bAutofightf()
 		blockUI(true);
 
 		auto ai = CDynLibHandler::getNewBattleAI(settings["server"]["friendlyAI"].String());
-		ai->init(owner.curInt->env, owner.curInt->cb);
+		ai->initBattleInterface(owner.curInt->env, owner.curInt->cb);
 		ai->battleStart(owner.army1, owner.army2, int3(0,0,0), owner.attackingHeroInstance, owner.defendingHeroInstance, owner.curInt->cb->battleGetMySide());
 		owner.curInt->autofightingAI = ai;
 		owner.curInt->cb->registerBattleInterface(ai);

client/gui/CCursorHandler.cpp

@@ -54,10 +54,10 @@ CCursorHandler::CCursorHandler()
 
 	cursors =
 	{
-		make_unique<CAnimImage>("CRADVNTR", 0),
-		make_unique<CAnimImage>("CRCOMBAT", 0),
-		make_unique<CAnimImage>("CRDEFLT",  0),
-		make_unique<CAnimImage>("CRSPELL",  0)
+		std::make_unique<CAnimImage>("CRADVNTR", 0),
+		std::make_unique<CAnimImage>("CRCOMBAT", 0),
+		std::make_unique<CAnimImage>("CRDEFLT",  0),
+		std::make_unique<CAnimImage>("CRSPELL",  0)
 	};
 
 	currentCursor = cursors.at(static_cast<size_t>(Cursor::Type::DEFAULT)).get();

client/lobby/RandomMapTab.cpp

@@ -132,7 +132,7 @@ void RandomMapTab::updateMapInfoByHost()
 	// Generate header info
 	mapInfo = std::make_shared<CMapInfo>();
 	mapInfo->isRandomMap = true;
-	mapInfo->mapHeader = make_unique<CMapHeader>();
+	mapInfo->mapHeader = std::make_unique<CMapHeader>();
 	mapInfo->mapHeader->version = EMapFormat::SOD;
 	mapInfo->mapHeader->name = CGI->generaltexth->allTexts[740];
 	mapInfo->mapHeader->description = CGI->generaltexth->allTexts[741];

client/mainmenu/CMainMenu.cpp

@@ -235,7 +235,11 @@ std::shared_ptr<CButton> CMenuEntry::createButton(CMenuScreen * parent, const Js
 	if(posy < 0)
 		posy = pos.h + posy;
 
-	return std::make_shared<CButton>(Point(posx, posy), button["name"].String(), help, command, (int)button["hotkey"].Float());
+	auto result = std::make_shared<CButton>(Point(posx, posy), button["name"].String(), help, command, (int)button["hotkey"].Float());
+
+	if (button["center"].Bool())
+		result->moveBy(Point(-result->pos.w/2, -result->pos.h/2));
+	return result;
 }
 
 CMenuEntry::CMenuEntry(CMenuScreen * parent, const JsonNode & config)

client/mapHandler.cpp

@@ -140,7 +140,7 @@ void CMapHandler::initTerrainGraphics()
 		//no rotation and basic setup
 		for(auto & type : files)
 		{
-			animation[type.first][0] = make_unique<CAnimation>(type.second);
+			animation[type.first][0] = std::make_unique<CAnimation>(type.second);
 			animation[type.first][0]->preload();
 			const size_t views = animation[type.first][0]->size(0);
 			cache[type.first].resize(views);
@@ -153,7 +153,7 @@ void CMapHandler::initTerrainGraphics()
 		{
 			for(auto & type : files)
 			{
-				animation[type.first][rotation] = make_unique<CAnimation>(type.second);
+				animation[type.first][rotation] = std::make_unique<CAnimation>(type.second);
 				animation[type.first][rotation]->preload();
 				const size_t views = animation[type.first][rotation]->size(0);
 
@@ -569,7 +569,7 @@ void CMapHandler::CMapWorldViewBlitter::drawOverlayEx(SDL_Surface * targetSurf)
 			continue;
 
 		realPos.x = initPos.x + (iconInfo.pos.x - topTile.x) * tileSize;
-		realPos.y = initPos.x + (iconInfo.pos.y - topTile.y) * tileSize;
+		realPos.y = initPos.y + (iconInfo.pos.y - topTile.y) * tileSize;
 
 		auto wvIcon = this->objectToIcon(iconInfo.id, iconInfo.subId, iconInfo.owner);
 
@@ -1347,7 +1347,7 @@ CMapHandler::CMapHandler()
 	tilesW = tilesH = 0;
 	offsetX = offsetY = 0;
 
-	egdeAnimation = make_unique<CAnimation>("EDG");
+	egdeAnimation = std::make_unique<CAnimation>("EDG");
 	egdeAnimation->preload();
 }
 

client/widgets/AdventureMapClasses.cpp

@@ -486,7 +486,7 @@ void CMinimapInstance::showAll(SDL_Surface * to)
 	std::vector <const CGHeroInstance *> heroes = LOCPLINT->cb->getHeroesInfo(false); //TODO: do we really need separate function for drawing heroes?
 	for(auto & hero : heroes)
 	{
-		int3 position = hero->getPosition(false);
+		int3 position = hero->visitablePos();
 		if(position.z == level)
 		{
 			const SDL_Color & color = graphics->playerColors[hero->getOwner().getNum()];

client/widgets/CArtifactHolder.cpp

@@ -257,7 +257,7 @@ void CHeroArtPlace::clickRight(tribool down, bool previousState)
 void CArtifactsOfHero::activate()
 {
 	if (commonInfo->src.AOH == this && commonInfo->src.art)
-		CCS->curh->dragAndDropCursor(make_unique<CAnimImage>("artifact", commonInfo->src.art->artType->getIconIndex()));
+		CCS->curh->dragAndDropCursor(std::make_unique<CAnimImage>("artifact", commonInfo->src.art->artType->getIconIndex()));
 
 	CIntObject::activate();
 }
@@ -290,7 +290,7 @@ void CHeroArtPlace::select ()
 		}
 	}
 
-	CCS->curh->dragAndDropCursor(make_unique<CAnimImage>("artifact", ourArt->artType->getIconIndex()));
+	CCS->curh->dragAndDropCursor(std::make_unique<CAnimImage>("artifact", ourArt->artType->getIconIndex()));
 	ourOwner->commonInfo->src.setTo(this, false);
 	ourOwner->markPossibleSlots(ourArt);
 
@@ -763,7 +763,7 @@ void CArtifactsOfHero::artifactMoved(const ArtifactLocation &src, const Artifact
 			commonInfo->src.art = dst.getArt();
 			commonInfo->src.slotID = dst.slot;
 			assert(commonInfo->src.AOH);
-			CCS->curh->dragAndDropCursor(make_unique<CAnimImage>("artifact", dst.getArt()->artType->getIconIndex()));
+			CCS->curh->dragAndDropCursor(std::make_unique<CAnimImage>("artifact", dst.getArt()->artType->getIconIndex()));
 			markPossibleSlots(dst.getArt());
 		}
 	}

client/widgets/CGarrisonInt.cpp

@@ -163,7 +163,7 @@ std::function<void()> CGarrisonSlot::getDismiss() const
 bool CGarrisonSlot::viewInfo()
 {
 	UpgradeInfo pom;
-	LOCPLINT->cb->getUpgradeInfo(getObj(), ID, pom);
+	LOCPLINT->cb->fillUpgradeInfo(getObj(), ID, pom);
 
 	bool canUpgrade = getObj()->tempOwner == LOCPLINT->playerID && pom.oldID>=0; //upgrade is possible
 	std::function<void(CreatureID)> upgr = nullptr;

client/widgets/TextControls.cpp

@@ -594,6 +594,11 @@ void CTextInput::keyPressed(const SDL_KeyboardEvent & key)
 	}
 }
 
+void CTextInput::setText(const std::string & nText)
+{
+	setText(nText, false);
+}
+
 void CTextInput::setText(const std::string & nText, bool callCb)
 {
 	CLabel::setText(nText);

client/widgets/TextControls.h

@@ -218,7 +218,8 @@ protected:
 public:
 	CFunctionList<void(const std::string &)> cb;
 	CFunctionList<void(std::string &, const std::string &)> filters;
-	void setText(const std::string & nText, bool callCb = false);
+	void setText(const std::string & nText) override;
+	void setText(const std::string & nText, bool callCb);
 
 	CTextInput(const Rect & Pos, EFonts font, const CFunctionList<void(const std::string &)> & CB);
 	CTextInput(const Rect & Pos, const Point & bgOffset, const std::string & bgName, const CFunctionList<void(const std::string &)> & CB);

client/windows/CAdvmapInterface.cpp

@@ -54,10 +54,6 @@
 #include "../../lib/StartInfo.h"
 #include "../../lib/mapping/CMapInfo.h"
 
-#ifdef _MSC_VER
-#pragma warning (disable : 4355)
-#endif
-
 #define ADVOPT (conf.go()->ac)
 using namespace CSDL_Ext;
 
@@ -1361,7 +1357,7 @@ void CAdvMapInt::keyPressed(const SDL_KeyboardEvent & key)
 
 			CGPath &path = LOCPLINT->paths[h];
 			terrain.currentPath = &path;
-			int3 dst = h->getPosition(false) + dir;
+			int3 dst = h->visitablePos() + dir;
 			if(dst != verifyPos(dst) || !LOCPLINT->cb->getPathsInfo(h)->getPath(path, dst))
 			{
 				terrain.currentPath = nullptr;

client/windows/CHeroWindow.cpp

@@ -77,7 +77,7 @@ void CHeroSwitcher::clickLeft(tribool down, bool previousState)
 		#if 0
 		owner->update(hero, true);
 		#else
-		UNUSED(owner);
+		MAYBE_UNUSED(owner);
 		const CGHeroInstance * buf = hero;
 		GH.popInts(1);
 		GH.pushIntT<CHeroWindow>(buf);

client/windows/CTradeWindow.cpp

@@ -188,7 +188,7 @@ void CTradeWindow::CTradeableItem::clickLeft(tribool down, bool previousState)
 				aw->arts->markPossibleSlots(art);
 
 				//aw->arts->commonInfo->dst.AOH = aw->arts;
-				CCS->curh->dragAndDropCursor(make_unique<CAnimImage>("artifact", art->artType->iconIndex));
+				CCS->curh->dragAndDropCursor(std::make_unique<CAnimImage>("artifact", art->artType->iconIndex));
 
 				aw->arts->artifactsOnAltar.erase(art);
 				setID(-1);

client/windows/GUIClasses.cpp

@@ -917,7 +917,7 @@ std::function<void()> CExchangeController::onSwapArmy()
 	{
 		GsThread::run([=]
 		{
-			if(right->tempOwner != cb->getMyColor()
+			if(left->tempOwner != cb->getMyColor()
 				|| right->tempOwner != cb->getMyColor())
 			{
 				return;
@@ -1763,7 +1763,7 @@ void CHillFortWindow::updateGarrisons()
 		if(newState != -1)
 		{
 			UpgradeInfo info;
-			LOCPLINT->cb->getUpgradeInfo(hero, SlotID(i), info);
+			LOCPLINT->cb->fillUpgradeInfo(hero, SlotID(i), info);
 			if(info.newID.size())//we have upgrades here - update costs
 			{
 				costs[i] = info.cost[0] * hero->getStackCount(SlotID(i));
@@ -1868,7 +1868,7 @@ void CHillFortWindow::makeDeal(SlotID slot)
 				if(slot.getNum() ==i || ( slot.getNum() == slotsCount && currState[i] == 2 ))//this is activated slot or "upgrade all"
 				{
 					UpgradeInfo info;
-					LOCPLINT->cb->getUpgradeInfo(hero, SlotID(i), info);
+					LOCPLINT->cb->fillUpgradeInfo(hero, SlotID(i), info);
 					LOCPLINT->cb->upgradeCreature(hero, SlotID(i), info.newID[0]);
 				}
 			}
@@ -1899,7 +1899,7 @@ int CHillFortWindow::getState(SlotID slot)
 		return -1;
 
 	UpgradeInfo info;
-	LOCPLINT->cb->getUpgradeInfo(hero, slot, info);
+	LOCPLINT->cb->fillUpgradeInfo(hero, slot, info);
 	if(!info.newID.size())//already upgraded
 		return 1;
 

config/filesystem.json

@@ -11,12 +11,16 @@
 		[
 			{"type" : "lod", "path" : "Data/H3ab_bmp.lod"},
 			{"type" : "lod", "path" : "Data/H3bitmap.lod"},
+			{"type" : "lod", "path" : "Data/h3abp_bm.lod"}, // Polish version of H3 only
+			{"type" : "lod", "path" : "Data/H3pbitma.lod"}, // Polish version of H3 only
 			{"type" : "dir",  "path" : "Data"}
 		],
 		"SPRITES/":
 		[
 			{"type" : "lod", "path" : "Data/H3ab_spr.lod"},
 			{"type" : "lod", "path" : "Data/H3sprite.lod"},
+			{"type" : "lod", "path" : "Data/h3abp_sp.lod"}, // Polish version of H3 only
+			{"type" : "lod", "path" : "Data/H3psprit.lod"}, // Polish version of H3 only
 			{"type" : "dir",  "path" : "Sprites"}
 		],
 		"SOUNDS/":

config/mainmenu.json

@@ -10,29 +10,29 @@
 		"background" : "gamselbk",
 		//"scalable" : true, //background will be scaled to screen size
 		//"video" :    {"x": 8, "y": 105, "name":"CREDITS.SMK" },//Floating WoG logo. Disabled due to different position in various versions of H3.
-		//"images" : [],//Optioal, contains any additional images in the same format as video
+		//"images" : [],//Optional, contains any additional images in the same format as video
 		"items" : 
 		[
 			{
 				"name" : "main",
 				"buttons":
 				[
-					{"x": 540, "y": 10,  "name":"MMENUNG", "hotkey" : 110, "help": 3, "command": "to new"},
-					{"x": 532, "y": 132, "name":"MMENULG", "hotkey" : 108, "help": 4, "command": "to load"},
-					{"x": 524, "y": 251, "name":"MMENUHS", "hotkey" : 104, "help": 5, "command": "highscores"},
-					{"x": 557, "y": 359, "name":"MMENUCR", "hotkey" : 99,  "help": 6, "command": "to credits"},
-					{"x": 586, "y": 468, "name":"MMENUQT", "hotkey" : 27,  "help": 7, "command": "exit"}
+					{"x": 644, "y":  70, "center" : true, "name":"MMENUNG", "hotkey" : 110, "help": 3, "command": "to new"},
+					{"x": 645, "y": 192, "center" : true, "name":"MMENULG", "hotkey" : 108, "help": 4, "command": "to load"},
+					{"x": 643, "y": 296, "center" : true, "name":"MMENUHS", "hotkey" : 104, "help": 5, "command": "highscores"},
+					{"x": 643, "y": 414, "center" : true, "name":"MMENUCR", "hotkey" : 99,  "help": 6, "command": "to credits"},
+					{"x": 643, "y": 520, "center" : true, "name":"MMENUQT", "hotkey" : 27,  "help": 7, "command": "exit"}
 				]
 			},
 			{
 				"name" : "new",
 				"buttons":
 				[
-					{"x": 545, "y": 4,   "name":"GTSINGL", "hotkey" : 115, "help": 10, "command": "start single"},
-					{"x": 568, "y": 120, "name":"GTMULTI", "hotkey" : 109, "help": 12, "command": "start multi"},
-					{"x": 541, "y": 233, "name":"GTCAMPN", "hotkey" : 99,  "help": 11, "command": "to campaign"},
-					{"x": 545, "y": 358, "name":"GTTUTOR", "hotkey" : 116, "help": 13, "command": "start tutorial"},
-					{"x": 582, "y": 464, "name":"GTBACK",  "hotkey" : 27,  "help": 14, "command": "to main"}
+					{"x": 649, "y":  65, "center" : true, "name":"GTSINGL", "hotkey" : 115, "help": 10, "command": "start single"},
+					{"x": 649, "y": 180, "center" : true, "name":"GTMULTI", "hotkey" : 109, "help": 12, "command": "start multi"},
+					{"x": 646, "y": 298, "center" : true, "name":"GTCAMPN", "hotkey" : 99,  "help": 11, "command": "to campaign"},
+					{"x": 647, "y": 412, "center" : true, "name":"GTTUTOR", "hotkey" : 116, "help": 13, "command": "start tutorial"},
+					{"x": 645, "y": 517, "center" : true, "name":"GTBACK",  "hotkey" : 27,  "help": 14, "command": "to main"}
 				],
 				"images": [ {"x": 114, "y": 312, "name":"NEWGAME"} ]
 			},
@@ -40,11 +40,11 @@
 				"name" : "load",
 				"buttons":
 				[
-					{"x": 545, "y": 8,   "name":"GTSINGL", "hotkey" : 115, "help": 10, "command": "load single"},
-					{"x": 568, "y": 120, "name":"GTMULTI", "hotkey" : 109, "help": 12, "command": "load multi"},
-					{"x": 541, "y": 233, "name":"GTCAMPN", "hotkey" : 99,  "help": 11, "command": "load campaign"},
-					{"x": 545, "y": 358, "name":"GTTUTOR", "hotkey" : 116, "help": 13, "command": "load tutorial"},
-					{"x": 582, "y": 464, "name":"GTBACK",  "hotkey" : 27,  "help": 14, "command": "to main"}
+					{"x": 649, "y":  65, "center" : true, "name":"GTSINGL", "hotkey" : 115, "help": 10, "command": "load single"},
+					{"x": 649, "y": 180, "center" : true, "name":"GTMULTI", "hotkey" : 109, "help": 12, "command": "load multi"},
+					{"x": 646, "y": 298, "center" : true, "name":"GTCAMPN", "hotkey" : 99,  "help": 11, "command": "load campaign"},
+					{"x": 647, "y": 412, "center" : true, "name":"GTTUTOR", "hotkey" : 116, "help": 13, "command": "load tutorial"},
+					{"x": 645, "y": 517, "center" : true, "name":"GTBACK",  "hotkey" : 27,  "help": 14, "command": "to main"}
 				],
 				"images": [ {"x": 114, "y": 312, "name":"LOADGAME"} ]
 			},
@@ -52,11 +52,11 @@
 				"name" : "campaign",
 				"buttons":
 				[
-					{"x": 535, "y": 4,   "name":"CSSSOD", "hotkey" : 119, "command": "campaigns sod"},
-					{"x": 494, "y": 117, "name":"CSSROE", "hotkey" : 114, "command": "campaigns roe"},
-					{"x": 486, "y": 241, "name":"CSSARM", "hotkey" : 97,  "command": "campaigns ab"},
-					{"x": 550, "y": 358, "name":"CSSCUS", "hotkey" : 99,  "command": "start campaign"},
-					{"x": 582, "y": 464, "name":"GTBACK", "hotkey" : 27,  "command": "to new"}
+					{"x": 634, "y":  67, "center" : true, "name":"CSSSOD", "hotkey" : 119, "command": "campaigns sod"},
+					{"x": 637, "y": 181, "center" : true, "name":"CSSROE", "hotkey" : 114, "command": "campaigns roe"},
+					{"x": 638, "y": 301, "center" : true, "name":"CSSARM", "hotkey" : 97,  "command": "campaigns ab"},
+					{"x": 638, "y": 413, "center" : true, "name":"CSSCUS", "hotkey" : 99,  "command": "start campaign"},
+					{"x": 639, "y": 518, "center" : true, "name":"CSSEXIT", "hotkey" : 27,  "command": "to new"}
 				],
 			}
 		]

debian/rules

@@ -8,6 +8,7 @@ override_dh_auto_configure:
 	dh_auto_configure -- \
 		-DCMAKE_INSTALL_RPATH_USE_LINK_PATH=ON \
 		-DCMAKE_INSTALL_RPATH=/usr/lib/$(DEB_HOST_MULTIARCH)/vcmi \
+		-DCMAKE_BUILD_TYPE=RelWithDebInfo \
 		-DBIN_DIR=games \
 		-DFORCE_BUNDLED_FL=OFF \
 		-DENABLE_TEST=0

include/vcmi/events/SubscriptionRegistry.h

@@ -40,7 +40,7 @@ public:
 
 		auto storage = std::make_shared<PreHandlerStorage>(std::move(handler));
 		preHandlers[tag].push_back(storage);
-		return make_unique<PreSubscription>(tag, storage);
+		return std::make_unique<PreSubscription>(tag, storage);
 	}
 
 	std::unique_ptr<EventSubscription> subscribeAfter(BusTag tag, PostHandler && handler)
@@ -49,7 +49,7 @@ public:
 
 		auto storage = std::make_shared<PostHandlerStorage>(std::move(handler));
 		postHandlers[tag].push_back(storage);
-		return make_unique<PostSubscription>(tag, storage);
+		return std::make_unique<PostSubscription>(tag, storage);
 	}
 
 	void executeEvent(const EventBus * bus, E & event, const ExecHandler & execHandler)

include/vstd/CLoggerBase.h

@@ -175,7 +175,7 @@ private:
 #define RAII_TRACE(logger, onEntry, onLeave)			\
 	std::unique_ptr<vstd::CTraceLogger> ctl00;						\
 	if(logger->isTraceEnabled())						\
-		ctl00 = make_unique<vstd::CTraceLogger>(logger, onEntry, onLeave);
+		ctl00 = std::make_unique<vstd::CTraceLogger>(logger, onEntry, onLeave);
 
 #define LOG_TRACE(logger) RAII_TRACE(logger,								\
 		boost::str(boost::format("Entering %s.") % BOOST_CURRENT_FUNCTION),	\

launcher/lobby/lobby_moc.cpp

@@ -202,10 +202,12 @@ void Lobby::serverCommand(const ServerCommand & command) try
 				ui->playersList->addItem(new QListWidgetItem(QIcon("icons:mod-disabled.png"), args[tagPoint]));
 			
 			if(args[tagPoint] == username)
+			{
 				if(args[tagPoint + 1] == "True")
 					ui->buttonReady->setText("Not ready");
 				else
 					ui->buttonReady->setText("Ready");
+			}
 		}
 		break;
 

launcher/main.cpp

@@ -48,9 +48,11 @@ void startGame(const QStringList & args)
 	logGlobal->warn("Starting game with the arguments: %s", args.join(" ").toStdString());
 
 #ifdef Q_OS_IOS
+	static const char clientName[] = "vcmiclient";
 	argcForClient = args.size() + 1; //first argument is omitted
-    argvForClient = new char*[argcForClient];
-	argvForClient[0] = "vcmiclient";
+	argvForClient = new char*[argcForClient];
+	argvForClient[0] = new char[strlen(clientName)+1];
+	strcpy(argvForClient[0], clientName);
 	for(int i = 1; i < argcForClient; ++i)
 	{
         std::string s = args.at(i - 1).toStdString();

launcher/modManager/cmodlist.cpp

@@ -18,7 +18,6 @@ namespace
 {
 bool isCompatible(const QString & verMin, const QString & verMax)
 {
-	const int maxSections = 3; // versions consist from up to 3 sections, major.minor.patch
 	QVersionNumber vcmiVersion(VCMI_VERSION_MAJOR,
 							   VCMI_VERSION_MINOR,
 							   VCMI_VERSION_PATCH);
@@ -26,8 +25,10 @@ bool isCompatible(const QString & verMin, const QString & verMax)
 	auto versionMin = QVersionNumber::fromString(verMin);
 	auto versionMax = QVersionNumber::fromString(verMax);
 	
-	auto buildVersion = [maxSections](QVersionNumber & ver)
+	auto buildVersion = [](QVersionNumber & ver)
 	{
+		const int maxSections = 3; // versions consist from up to 3 sections, major.minor.patch
+
 		if(ver.segmentCount() < maxSections)
 		{
 			auto segments = ver.segments();

launcher/modManager/cmodlistview_moc.cpp

@@ -28,7 +28,7 @@
 void CModListView::setupModModel()
 {
 	modModel = new CModListModel(this);
-	manager = vstd::make_unique<CModManager>(modModel);
+	manager = std::make_unique<CModManager>(modModel);
 
 	connect(manager.get(), &CModManager::extraResolutionsEnabledChanged,
 		this, &CModListView::extraResolutionsEnabledChanged);

launcher/settingsView/csettingsview_moc.cpp

@@ -135,6 +135,8 @@ void CSettingsView::fillValidResolutionsForScreen(int screenIndex)
 	const auto screens = qGuiApp->screens();
 	const auto currentScreen = screenIndex < screens.size() ? screens[screenIndex] : qGuiApp->primaryScreen();
 	const auto screenSize = currentScreen->size();
+	MAYBE_UNUSED(screenSize);
+
 	for(const auto & entry : resolutions)
 	{
 		const auto resolutionMap = entry.toMap().value(QLatin1String{"resolution"}).toMap();

lib/CCreatureHandler.cpp

@@ -299,7 +299,7 @@ TerrainId CCreature::getNativeTerrain() const
 	//this code is used in the CreatureTerrainLimiter::limit to setup battle bonuses
 	//and in the CGHeroInstance::getNativeTerrain() to setup mevement bonuses or/and penalties.
 	return hasBonus(selectorNoTerrainPenalty, selectorNoTerrainPenalty)
-		? Terrain::ANY_TERRAIN
+		? TerrainId(Terrain::ANY_TERRAIN)
 		: (*VLC->townh)[faction]->nativeTerrain;
 }
 

lib/CCreatureSet.cpp

@@ -507,7 +507,7 @@ void CCreatureSet::joinStack(SlotID slot, CStackInstance * stack)
 	const CCreature *c = getCreature(slot);
 	assert(c == stack->type);
 	assert(c);
-	UNUSED(c);
+	MAYBE_UNUSED(c);
 
 	//TODO move stuff
 	changeStackCount(slot, stack->count);

lib/CGameInfoCallback.cpp

@@ -166,13 +166,13 @@ const CGTownInstance* CGameInfoCallback::getTown(ObjectInstanceID objid) const
 		return nullptr;
 }
 
-void CGameInfoCallback::getUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out) const
+void CGameInfoCallback::fillUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out) const
 {
 	//boost::shared_lock<boost::shared_mutex> lock(*gs->mx);
 	ERROR_RET_IF(!canGetFullInfo(obj), "Cannot get info about not owned object!");
 	ERROR_RET_IF(!obj->hasStackAtSlot(stackPos), "There is no such stack!");
-	out = gs->getUpgradeInfo(obj->getStack(stackPos));
-	//return gs->getUpgradeInfo(obj->getStack(stackPos));
+	gs->fillUpgradeInfo(obj, stackPos, out);
+	//return gs->fillUpgradeInfo(obj->getStack(stackPos));
 }
 
 const StartInfo * CGameInfoCallback::getStartInfo(bool beforeRandomization) const
@@ -294,7 +294,7 @@ bool CGameInfoCallback::getHeroInfo(const CGObjectInstance * hero, InfoAboutHero
 		if(gs->curB && gs->curB->playerHasAccessToHeroInfo(*player, h)) //if it's battle we can get enemy hero full data
 			infoLevel = InfoAboutHero::EInfoLevel::INBATTLE;
 		else
-			ERROR_RET_VAL_IF(!isVisible(h->getPosition(false)), "That hero is not visible!", false);
+			ERROR_RET_VAL_IF(!isVisible(h->visitablePos()), "That hero is not visible!", false);
 	}
 
 	if( (infoLevel == InfoAboutHero::EInfoLevel::BASIC) && nullptr != selectedObject)
@@ -402,7 +402,7 @@ int CGameInfoCallback::getDate(Date::EDateType mode) const
 bool CGameInfoCallback::isVisible(int3 pos, boost::optional<PlayerColor> Player) const
 {
 	//boost::shared_lock<boost::shared_mutex> lock(*gs->mx);
-	return gs->map->isInTheMap(pos) && (!Player || gs->isVisible(pos, *Player));
+	return gs->isVisible(pos, Player);
 }
 
 bool CGameInfoCallback::isVisible(int3 pos) const
@@ -734,7 +734,7 @@ std::vector < const CGHeroInstance *> CPlayerSpecificInfoCallback::getHeroesInfo
 	{
 		// !player || // - why would we even get access to hero not owned by any player?
 		if((hero->tempOwner == *player) ||
-			(isVisible(hero->getPosition(false), player) && !onlyOur)	)
+			(isVisible(hero->visitablePos(), player) && !onlyOur)	)
 		{
 			ret.push_back(hero);
 		}
@@ -933,6 +933,12 @@ void CGameInfoCallback::calculatePaths(std::shared_ptr<PathfinderConfig> config)
 	gs->calculatePaths(config);
 }
 
+void CGameInfoCallback::calculatePaths( const CGHeroInstance *hero, CPathsInfo &out)
+{
+	gs->calculatePaths(hero, out);
+}
+
+
 const CArtifactInstance * CGameInfoCallback::getArtInstance( ArtifactInstanceID aid ) const
 {
 	return gs->map->artInstances[aid.num];

lib/CGameInfoCallback.h

@@ -25,6 +25,7 @@ struct TerrainTile;
 struct PlayerState;
 class CTown;
 struct StartInfo;
+struct CPathsInfo;
 
 struct InfoAboutHero;
 struct InfoAboutTown;
@@ -68,7 +69,7 @@ public:
 
 
 //	//armed object
-//	void getUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out)const;
+//	void fillUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out)const;
 
 	//hero
 	virtual const CGHeroInstance * getHero(ObjectInstanceID objid) const = 0;
@@ -134,9 +135,6 @@ protected:
 	CGameInfoCallback();
 	CGameInfoCallback(CGameState *GS, boost::optional<PlayerColor> Player);
 	bool hasAccess(boost::optional<PlayerColor> playerId) const;
-	bool isVisible(int3 pos, boost::optional<PlayerColor> Player) const;
-	bool isVisible(const CGObjectInstance *obj, boost::optional<PlayerColor> Player) const;
-	bool isVisible(const CGObjectInstance *obj) const;
 
 	bool canGetFullInfo(const CGObjectInstance *obj) const; //true we player owns obj or ally owns obj or privileged mode
 	bool isOwnedOrVisited(const CGObjectInstance *obj) const;
@@ -151,7 +149,6 @@ public:
 	const Player * getPlayer(PlayerColor color) const override;
 	virtual const PlayerState * getPlayerState(PlayerColor color, bool verbose = true) const;
 	virtual int getResource(PlayerColor Player, Res::ERes which) const;
-	virtual bool isVisible(int3 pos) const;
 	virtual PlayerRelations::PlayerRelations getPlayerRelations(PlayerColor color1, PlayerColor color2) const;
 	virtual void getThievesGuildInfo(SThievesGuildInfo & thi, const CGObjectInstance * obj); //get thieves' guild info obtainable while visiting given object
 	virtual EPlayerStatus::EStatus getPlayerStatus(PlayerColor player, bool verbose = true) const; //-1 if no such player
@@ -159,8 +156,15 @@ public:
 	PlayerColor getLocalPlayer() const override; //player that is currently owning given client (if not a client, then returns current player)
 	virtual const PlayerSettings * getPlayerSettings(PlayerColor color) const;
 
+	//map
+	virtual bool isVisible(int3 pos, boost::optional<PlayerColor> Player) const;
+	virtual bool isVisible(const CGObjectInstance *obj, boost::optional<PlayerColor> Player) const;
+	virtual bool isVisible(const CGObjectInstance *obj) const;
+	virtual bool isVisible(int3 pos) const;
+
+
 	//armed object
-	virtual void getUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out)const;
+	virtual void fillUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out)const;
 
 	//hero
 	virtual const CGHeroInstance * getHero(ObjectInstanceID objid) const override;
@@ -192,6 +196,7 @@ public:
 	virtual bool isInTheMap(const int3 &pos) const;
 	virtual void getVisibleTilesInRange(std::unordered_set<int3, ShashInt3> &tiles, int3 pos, int radious, int3::EDistanceFormula distanceFormula = int3::DIST_2D) const;
 	virtual void calculatePaths(std::shared_ptr<PathfinderConfig> config);
+	virtual void calculatePaths(const CGHeroInstance *hero, CPathsInfo &out);
 
 	//town
 	virtual const CGTownInstance* getTown(ObjectInstanceID objid) const;
@@ -222,6 +227,9 @@ public:
 class DLL_LINKAGE CPlayerSpecificInfoCallback : public CGameInfoCallback
 {
 public:
+	// keep player-specific override in scope
+	using CGameInfoCallback::howManyTowns;
+
 	virtual int howManyTowns() const;
 	virtual int howManyHeroes(bool includeGarrisoned = true) const;
 	virtual int3 getGrailPos(double *outKnownRatio);
@@ -242,5 +250,4 @@ public:
 	//virtual const PlayerSettings * getPlayerSettings(PlayerColor color) const;
 };
 
-
 VCMI_LIB_NAMESPACE_END

lib/CGameInterface.cpp

@@ -170,7 +170,7 @@ void CAdventureAI::battleStart(const CCreatureSet * army1, const CCreatureSet *
 	assert(!battleAI);
 	assert(cbc);
 	battleAI = CDynLibHandler::getNewBattleAI(getBattleAIName());
-	battleAI->init(env, cbc);
+	battleAI->initBattleInterface(env, cbc);
 	battleAI->battleStart(army1, army2, tile, hero1, hero2, side);
 }
 
@@ -262,7 +262,7 @@ void CAdventureAI::loadGame(BinaryDeserializer & h, const int version) /*loading
 		h & dllName;
 		battleAI = CDynLibHandler::getNewBattleAI(dllName);
 		assert(cbc); //it should have been set by the one who new'ed us
-		battleAI->init(env, cbc);
+		battleAI->initBattleInterface(env, cbc);
 	}
 }
 

lib/CGameInterface.h

@@ -52,7 +52,6 @@ struct StackLocation;
 class CStackInstance;
 class CCommanderInstance;
 class CStack;
-struct CPathsInfo;
 class CCreature;
 class CLoadFile;
 class CSaveFile;
@@ -78,7 +77,7 @@ public:
 	std::string dllName;
 
 	virtual ~CBattleGameInterface() {};
-	virtual void init(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB){};
+	virtual void initBattleInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB){};
 
 	//battle call-ins
 	virtual BattleAction activeStack(const CStack * stack)=0; //called when it's turn of that stack
@@ -90,7 +89,7 @@ class DLL_LINKAGE CGameInterface : public CBattleGameInterface, public IGameEven
 {
 public:
 	virtual ~CGameInterface() = default;
-	virtual void init(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB){};
+	virtual void initGameInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CCallback> CB){};
 	virtual void yourTurn(){}; //called AFTER playerStartsTurn(player)
 
 	//pskill is gained primary skill, interface has to choose one of given skills and call callback with selection id

lib/CGameState.cpp

@@ -1768,7 +1768,7 @@ void CGameState::initTowns()
 		for(auto building : vti->builtBuildings)
 		{
 			assert(vti->town->buildings.at(building) != nullptr);
-			UNUSED(building);
+			MAYBE_UNUSED(building);
 		}
 
 		//town events
@@ -1880,8 +1880,7 @@ void CGameState::placeHeroesInTowns()
 				// assume that this hero should be visiting the town (H3M format quirk) and move hero to correct position
 				if (heroOnTownBlockableTile)
 				{
-					int3 townVisitablePos = t->visitablePos();
-					int3 correctedPos = townVisitablePos + h->getVisitableOffset();
+					int3 correctedPos = h->convertFromVisitablePos(t->visitablePos());
 
 					map->removeBlockVisTiles(h);
 					h->pos = correctedPos;
@@ -1957,7 +1956,16 @@ BattleField CGameState::battleGetBattlefieldType(int3 tile, CRandomGenerator & r
 		*RandomGeneratorUtil::nextItem(t.terType->battleFields, rand));
 }
 
-UpgradeInfo CGameState::getUpgradeInfo(const CStackInstance &stack)
+
+void CGameState::fillUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out) const
+{
+	assert(obj);
+	assert(obj->hasStackAtSlot(stackPos));
+
+	out = fillUpgradeInfo(obj->getStack(stackPos));
+}
+
+UpgradeInfo CGameState::fillUpgradeInfo(const CStackInstance &stack) const
 {
 	UpgradeInfo ret;
 	const CCreature *base = stack.type;
@@ -2021,7 +2029,7 @@ UpgradeInfo CGameState::getUpgradeInfo(const CStackInstance &stack)
 	return ret;
 }
 
-PlayerRelations::PlayerRelations CGameState::getPlayerRelations( PlayerColor color1, PlayerColor color2 )
+PlayerRelations::PlayerRelations CGameState::getPlayerRelations( PlayerColor color1, PlayerColor color2 ) const
 {
 	if ( color1 == color2 )
 		return PlayerRelations::SAME_PLAYER;
@@ -2042,8 +2050,7 @@ void CGameState::apply(CPack *pack)
 
 void CGameState::calculatePaths(const CGHeroInstance *hero, CPathsInfo &out)
 {
-	CPathfinder pathfinder(out, this, hero);
-	pathfinder.calculatePaths();
+	calculatePaths(std::make_shared<SingleHeroPathfinderConfig>(out, this, hero));
 }
 
 void CGameState::calculatePaths(std::shared_ptr<PathfinderConfig> config)
@@ -2187,17 +2194,21 @@ void CGameState::updateRumor()
 	while(!rumor.update(rumorId, rumorExtra));
 }
 
-bool CGameState::isVisible(int3 pos, PlayerColor player)
+bool CGameState::isVisible(int3 pos, boost::optional<PlayerColor> player) const
 {
+	if (!map->isInTheMap(pos))
+		return false;
+	if (!player)
+		return true;
 	if(player == PlayerColor::NEUTRAL)
 		return false;
-	if(player.isSpectator())
+	if(player->isSpectator())
 		return true;
 
-	return (*getPlayerTeam(player)->fogOfWarMap)[pos.z][pos.x][pos.y];
+	return (*getPlayerTeam(*player)->fogOfWarMap)[pos.z][pos.x][pos.y];
 }
 
-bool CGameState::isVisible( const CGObjectInstance *obj, boost::optional<PlayerColor> player )
+bool CGameState::isVisible( const CGObjectInstance *obj, boost::optional<PlayerColor> player ) const
 {
 	if(!player)
 		return true;

lib/CGameState.h

@@ -178,10 +178,11 @@ public:
 
 	void apply(CPack *pack);
 	BattleField battleGetBattlefieldType(int3 tile, CRandomGenerator & rand);
-	UpgradeInfo getUpgradeInfo(const CStackInstance &stack);
-	PlayerRelations::PlayerRelations getPlayerRelations(PlayerColor color1, PlayerColor color2);
+
+	void fillUpgradeInfo(const CArmedInstance *obj, SlotID stackPos, UpgradeInfo &out) const override;
+	PlayerRelations::PlayerRelations getPlayerRelations(PlayerColor color1, PlayerColor color2) const override;
 	bool checkForVisitableDir(const int3 & src, const int3 & dst) const; //check if src tile is visitable from dst tile
-	void calculatePaths(const CGHeroInstance *hero, CPathsInfo &out); //calculates possible paths for hero, by default uses current hero position and movement left; returns pointer to newly allocated CPath or nullptr if path does not exists
+	void calculatePaths(const CGHeroInstance *hero, CPathsInfo &out) override; //calculates possible paths for hero, by default uses current hero position and movement left; returns pointer to newly allocated CPath or nullptr if path does not exists
 	void calculatePaths(std::shared_ptr<PathfinderConfig> config) override;
 	int3 guardingCreaturePosition (int3 pos) const override;
 	std::vector<CGObjectInstance*> guardingCreatures (int3 pos) const;
@@ -197,8 +198,9 @@ public:
 	void obtainPlayersStats(SThievesGuildInfo & tgi, int level); //fills tgi with info about other players that is available at given level of thieves' guild
 	std::map<ui32, ConstTransitivePtr<CGHeroInstance> > unusedHeroesFromPool(); //heroes pool without heroes that are available in taverns
 
-	bool isVisible(int3 pos, PlayerColor player);
-	bool isVisible(const CGObjectInstance *obj, boost::optional<PlayerColor> player);
+
+	bool isVisible(int3 pos, boost::optional<PlayerColor> player) const override;
+	bool isVisible(const CGObjectInstance *obj, boost::optional<PlayerColor> player) const override;
 
 	int getDate(Date::EDateType mode=Date::DAY) const override; //mode=0 - total days in game, mode=1 - day of week, mode=2 - current week, mode=3 - current month
 
@@ -292,6 +294,7 @@ private:
 	std::pair<Obj,int> pickObject(CGObjectInstance *obj); //chooses type of object to be randomized, returns <type, subtype>
 	int pickUnusedHeroTypeRandomly(PlayerColor owner); // picks a unused hero type randomly
 	int pickNextHeroType(PlayerColor owner); // picks next free hero type of the H3 hero init sequence -> chosen starting hero, then unused hero type randomly
+	UpgradeInfo fillUpgradeInfo(const CStackInstance &stack) const;
 
 	// ---- data -----
 	std::shared_ptr<CApplier<CBaseForGSApply>> applier;

lib/CModHandler.cpp

@@ -582,7 +582,7 @@ CModInfo::Version CModInfo::Version::fromString(std::string from)
 				patch = std::stoi(from.substr(pointPos + 1));
 		}
 	}
-	catch(const std::invalid_argument & e)
+	catch(const std::invalid_argument &)
 	{
 		return Version();
 	}

lib/CPathfinder.cpp

@@ -141,7 +141,7 @@ NodeStorage::NodeStorage(CPathsInfo & pathsInfo, const CGHeroInstance * hero)
 	:out(pathsInfo)
 {
 	out.hero = hero;
-	out.hpos = hero->getPosition(false);
+	out.hpos = hero->visitablePos();
 }
 
 void NodeStorage::resetTile(
@@ -254,16 +254,6 @@ PathfinderConfig::PathfinderConfig(
 {
 }
 
-CPathfinder::CPathfinder(
-	CPathsInfo & _out,
-	CGameState * _gs,
-	const CGHeroInstance * _hero)
-	: CPathfinder(
-		_gs,
-		std::make_shared<SingleHeroPathfinderConfig>(_out, _gs, _hero))
-{
-}
-
 std::vector<std::shared_ptr<IPathfindingRule>> SingleHeroPathfinderConfig::buildRuleSet()
 {
 	return std::vector<std::shared_ptr<IPathfindingRule>>{
@@ -289,7 +279,7 @@ CPathfinderHelper * SingleHeroPathfinderConfig::getOrCreatePathfinderHelper(cons
 CPathfinder::CPathfinder(
 	CGameState * _gs,
 	std::shared_ptr<PathfinderConfig> config)
-	: CGameInfoCallback(_gs, boost::optional<PlayerColor>())
+	: gamestate(_gs)
 	, config(config)
 	, source()
 	, destination()
@@ -329,14 +319,14 @@ void CPathfinder::calculatePaths()
 
 	for(auto initialNode : initialNodes)
 	{
-		if(!isInTheMap(initialNode->coord)/* || !gs->map->isInTheMap(dest)*/) //check input
+		if(!gamestate->isInTheMap(initialNode->coord)/* || !gs->map->isInTheMap(dest)*/) //check input
 		{
 			logGlobal->error("CGameState::calculatePaths: Hero outside the gs->map? How dare you...");
 			throw std::runtime_error("Wrong checksum");
 		}
 
-		source.setNode(gs, initialNode);
-		auto hlp = config->getOrCreatePathfinderHelper(source, gs);
+		source.setNode(gamestate, initialNode);
+		auto hlp = config->getOrCreatePathfinderHelper(source, gamestate);
 
 		if(hlp->isHeroPatrolLocked())
 			continue;
@@ -349,14 +339,14 @@ void CPathfinder::calculatePaths()
 		counter++;
 		auto node = topAndPop();
 
-		source.setNode(gs, node);
+		source.setNode(gamestate, node);
 		source.node->locked = true;
 
 		int movement = source.node->moveRemains;
 		uint8_t turn = source.node->turns;
 		float cost = source.node->getCost();
 
-		auto hlp = config->getOrCreatePathfinderHelper(source, gs);
+		auto hlp = config->getOrCreatePathfinderHelper(source, gamestate);
 
 		hlp->updateTurnInfo(turn);
 		if(!movement)
@@ -368,7 +358,7 @@ void CPathfinder::calculatePaths()
 		}
 
 		source.isInitialPosition = source.nodeHero == hlp->hero;
-		source.updateInfo(hlp, gs);
+		source.updateInfo(hlp, gamestate);
 
 		//add accessible neighbouring nodes to the queue
 		auto neighbourNodes = config->nodeStorage->calculateNeighbours(source, config.get(), hlp);
@@ -380,8 +370,8 @@ void CPathfinder::calculatePaths()
 			if(!hlp->isLayerAvailable(neighbour->layer))
 				continue;
 
-			destination.setNode(gs, neighbour);
-			hlp = config->getOrCreatePathfinderHelper(destination, gs);
+			destination.setNode(gamestate, neighbour);
+			hlp = config->getOrCreatePathfinderHelper(destination, gamestate);
 
 			if(!hlp->isPatrolMovementAllowed(neighbour->coord))
 				continue;
@@ -393,7 +383,7 @@ void CPathfinder::calculatePaths()
 			destination.turn = turn;
 			destination.movementLeft = movement;
 			destination.cost = cost;
-			destination.updateInfo(hlp, gs);
+			destination.updateInfo(hlp, gamestate);
 			destination.isGuardianTile = destination.guarded && isDestinationGuardian();
 
 			for(auto rule : config->rules)
@@ -410,7 +400,7 @@ void CPathfinder::calculatePaths()
 		} //neighbours loop
 
 		//just add all passable teleport exits
-		hlp = config->getOrCreatePathfinderHelper(source, gs);
+		hlp = config->getOrCreatePathfinderHelper(source, gamestate);
 
 		/// For now we disable teleports usage for patrol movement
 		/// VCAI not aware about patrol and may stuck while attempt to use teleport
@@ -430,7 +420,7 @@ void CPathfinder::calculatePaths()
 			if(teleportNode->accessible == CGPathNode::BLOCKED)
 				continue;
 
-			destination.setNode(gs, teleportNode);
+			destination.setNode(gamestate, teleportNode);
 			destination.turn = turn;
 			destination.movementLeft = movement;
 			destination.cost = cost;
@@ -903,7 +893,7 @@ CGPathNode::ENodeAction CPathfinder::getTeleportDestAction() const
 
 bool CPathfinder::isDestinationGuardian() const
 {
-	return gs->guardingCreaturePosition(destination.node->coord) == destination.node->coord;
+	return gamestate->guardingCreaturePosition(destination.node->coord) == destination.node->coord;
 }
 
 void CPathfinderHelper::initializePatrol()
@@ -927,7 +917,7 @@ void CPathfinderHelper::initializePatrol()
 void CPathfinder::initializeGraph()
 {
 	INodeStorage * nodeStorage = config->nodeStorage.get();
-	nodeStorage->initialize(config->options, gs);
+	nodeStorage->initialize(config->options, gamestate);
 }
 
 bool CPathfinderHelper::canMoveBetween(const int3 & a, const int3 & b) const
@@ -1028,7 +1018,7 @@ TurnInfo::TurnInfo(const CGHeroInstance * Hero, const int turn)
 	: hero(Hero), maxMovePointsLand(-1), maxMovePointsWater(-1)
 {
 	bonuses = hero->getAllBonuses(Selector::days(turn), Selector::all, nullptr, "");
-	bonusCache = make_unique<BonusCache>(bonuses);
+	bonusCache = std::make_unique<BonusCache>(bonuses);
 	nativeTerrain = hero->getNativeTerrain();
 }
 
@@ -1230,7 +1220,7 @@ int CPathfinderHelper::getMovementCost(
 	/// TODO: by the original game rules hero shouldn't be affected by terrain penalty while flying.
 	/// Also flying movement only has penalty when player moving over blocked tiles.
 	/// So if you only have base flying with 40% penalty you can still ignore terrain penalty while having zero flying penalty.
-	ui32 ret = hero->getTileCost(*dt, *ct, ti);
+	int ret = hero->getTileCost(*dt, *ct, ti);
 	/// Unfortunately this can't be implemented yet as server don't know when player flying and when he's not.
 	/// Difference in cost calculation on client and server is much worse than incorrect cost.
 	/// So this one is waiting till server going to use pathfinder rules for path validation.
@@ -1292,17 +1282,6 @@ int3 CGPath::endPos() const
 	return nodes[0].coord;
 }
 
-void CGPath::convert(ui8 mode)
-{
-	if(mode==0)
-	{
-		for(auto & elem : nodes)
-		{
-			elem.coord = CGHeroInstance::convertPosition(elem.coord,true);
-		}
-	}
-}
-
 CPathsInfo::CPathsInfo(const int3 & Sizes, const CGHeroInstance * hero_)
 	: sizes(Sizes), hero(hero_)
 {

lib/CPathfinder.h

@@ -170,7 +170,6 @@ struct DLL_LINKAGE CGPath
 
 	int3 startPos() const; // start point
 	int3 endPos() const; //destination point
-	void convert(ui8 mode); //mode=0 -> from 'manifest' to 'object'
 };
 
 struct DLL_LINKAGE CPathsInfo
@@ -472,12 +471,11 @@ public:
 	static std::vector<std::shared_ptr<IPathfindingRule>> buildRuleSet();
 };
 
-class CPathfinder : private CGameInfoCallback
+class CPathfinder
 {
 public:
 	friend class CPathfinderHelper;
 
-	CPathfinder(CPathsInfo & _out, CGameState * _gs, const CGHeroInstance * _hero);
 	CPathfinder(
 		CGameState * _gs,
 		std::shared_ptr<PathfinderConfig> config);
@@ -485,6 +483,8 @@ public:
 	void calculatePaths(); //calculates possible paths for hero, uses current hero position and movement left; returns pointer to newly allocated CPath or nullptr if path does not exists
 
 private:
+	CGameState * gamestate;
+
 	typedef EPathfindingLayer ELayer;
 
 	std::shared_ptr<PathfinderConfig> config;

lib/CStack.cpp

@@ -308,7 +308,7 @@ std::vector<BattleHex> CStack::meleeAttackHexes(const battle::Unit * attacker, c
 			res.push_back(otherDefenderPos);
 		}
 	}
-	UNUSED(mask);
+	MAYBE_UNUSED(mask);
 
 	return res;
 }

lib/HeroBonus.cpp

@@ -663,9 +663,7 @@ int IBonusBearer::valOfBonuses(Bonus::BonusType type, const CSelector &selector)
 int IBonusBearer::valOfBonuses(Bonus::BonusType type, int subtype) const
 {
 	//This part is performance-critical
-
-	char cachingStr[20] = {};
-	std::sprintf(cachingStr, "type_%ds_%d", (int)type, subtype);
+	std::string cachingStr = "type_" + std::to_string(int(type)) + "_" + std::to_string(subtype);
 
 	CSelector s = Selector::type()(type);
 	if(subtype != -1)
@@ -694,8 +692,7 @@ bool IBonusBearer::hasBonus(const CSelector &selector, const CSelector &limit, c
 bool IBonusBearer::hasBonusOfType(Bonus::BonusType type, int subtype) const
 {
 	//This part is performance-ciritcal
-	char cachingStr[20] = {};
-	std::sprintf(cachingStr, "type_%ds_%d", (int)type, subtype);
+	std::string cachingStr = "type_" + std::to_string(int(type)) + "_" + std::to_string(subtype);
 
 	CSelector s = Selector::type()(type);
 	if(subtype != -1)

lib/IGameCallback.h

@@ -116,6 +116,16 @@ public:
 class DLL_LINKAGE CNonConstInfoCallback : public CPrivilegedInfoCallback
 {
 public:
+	//keep const version of callback accessible
+	using CGameInfoCallback::getPlayerState;
+	using CGameInfoCallback::getTeam;
+	using CGameInfoCallback::getPlayerTeam;
+	using CGameInfoCallback::getHero;
+	using CGameInfoCallback::getTown;
+	using CGameInfoCallback::getTile;
+	using CGameInfoCallback::getArtInstance;
+	using CGameInfoCallback::getObjInstance;
+
 	PlayerState * getPlayerState(PlayerColor color, bool verbose = true);
 	TeamState *getTeam(TeamID teamID);//get team by team ID
 	TeamState *getPlayerTeam(PlayerColor color);// get team by player color

lib/JsonNode.cpp

@@ -1028,6 +1028,12 @@ void JsonUtils::merge(JsonNode & dest, JsonNode & source, bool ignoreOverride, b
 	bool destNumeric = dest.getType() == JsonNode::JsonType::DATA_FLOAT || dest.getType() == JsonNode::JsonType::DATA_INTEGER;
 	bool bothNumeric = sourceNumeric && destNumeric;
 
+	MAYBE_UNUSED(hasNull);
+	MAYBE_UNUSED(sameType);
+	MAYBE_UNUSED(sourceNumeric);
+	MAYBE_UNUSED(destNumeric);
+	MAYBE_UNUSED(bothNumeric);
+
 	assert( hasNull || sameType || bothNumeric );
 
 	switch (source.getType())

lib/NetPacksLib.cpp

@@ -550,7 +550,7 @@ void TryMoveHero::applyGs(CGameState *gs)
 
 	if(result == EMBARK) //hero enters boat at destination tile
 	{
-		const TerrainTile &tt = gs->map->getTile(CGHeroInstance::convertPosition(end, false));
+		const TerrainTile &tt = gs->map->getTile(h->convertToVisitablePos(end));
 		assert(tt.visitableObjects.size() >= 1  &&  tt.visitableObjects.back()->ID == Obj::BOAT); //the only visitable object at destination is Boat
 		CGBoat *boat = static_cast<CGBoat*>(tt.visitableObjects.back());
 
@@ -704,14 +704,13 @@ DLL_LINKAGE void GiveHero::applyGs(CGameState *gs)
 	h->detachFrom(gs->globalEffects);
 	h->attachTo(*gs->getPlayerState(player));
 
-	auto oldOffset = h->getVisitableOffset();
+	auto oldVisitablePos = h->visitablePos();
 	gs->map->removeBlockVisTiles(h,true);
 	h->appearance = VLC->objtypeh->getHandlerFor(Obj::HERO, h->type->heroClass->getIndex())->getTemplates().front();
-	auto newOffset = h->getVisitableOffset();
 
 	h->setOwner(player);
 	h->movement =  h->maxMovePoints(true);
-	h->pos = h->pos - oldOffset + newOffset;
+	h->pos = h->convertFromVisitablePos(oldVisitablePos);
 	gs->map->heroesOnMap.push_back(h);
 	gs->getPlayerState(h->getOwner())->heroes.push_back(h);
 
@@ -731,7 +730,7 @@ DLL_LINKAGE void NewObject::applyGs(CGameState *gs)
 
 		const int3 previousXAxisTile = int3(pos.x - 1, pos.y, pos.z);
 		assert(gs->isInTheMap(previousXAxisTile) && (testObject.visitablePos() == previousXAxisTile));
-		UNUSED(previousXAxisTile);
+		MAYBE_UNUSED(previousXAxisTile);
 	}
 	else
 	{
@@ -957,7 +956,7 @@ DLL_LINKAGE void RebalanceStacks::applyGs(CGameState * gs)
 		if(const CCreature *c = dst.army->getCreature(dst.slot)) //stack at dest -> merge
 		{
 			assert(c == srcType);
-			UNUSED(c);
+			MAYBE_UNUSED(c);
 			auto alHere = ArtifactLocation (src.getStack(), ArtifactPosition::CREATURE_SLOT);
 			auto alDest = ArtifactLocation (dst.getStack(), ArtifactPosition::CREATURE_SLOT);
 			auto artHere = alHere.getArt();
@@ -1008,7 +1007,7 @@ DLL_LINKAGE void RebalanceStacks::applyGs(CGameState * gs)
 		if(const CCreature *c = dst.army->getCreature(dst.slot)) //stack at dest -> rebalance
 		{
 			assert(c == srcType);
-			UNUSED(c);
+			MAYBE_UNUSED(c);
 			if (stackExp)
 			{
 				ui64 totalExp = srcCount * src.army->getStackExperience(src.slot) + dst.army->getStackCount(dst.slot) * dst.army->getStackExperience(dst.slot);
@@ -1176,13 +1175,11 @@ DLL_LINKAGE void AssembledArtifact::applyGs(CGameState *gs)
 	const CArtifactInstance *transformedArt = al.getArt();
 	assert(transformedArt);
 	bool combineEquipped = !ArtifactUtils::isSlotBackpack(al.slot);
-
 	assert(vstd::contains_if(transformedArt->assemblyPossibilities(artSet, combineEquipped), [=](const CArtifact * art)->bool
 		{
 			return art->id == builtArt->id;
 		}));
-
-	UNUSED(transformedArt);
+	MAYBE_UNUSED(transformedArt);
 
 	auto combinedArt = new CCombinedArtifactInstance(builtArt);
 	gs->map->addNewArtifactInstance(combinedArt);

lib/VCMIDirs.cpp

@@ -46,8 +46,8 @@ std::string IVCMIDirs::genHelpString() const
 		"  user cache:		" + userCachePath().string() + "\n"
 		"  user config:		" + userConfigPath().string() + "\n"
 		"  user logs:		" + userLogsPath().string() + "\n"
-		"  user saves:		" + userSavePath().string() + "\n";
-		"  user extracted:	" + userExtractedPath().string() + "\n"; // Should end without new-line?
+		"  user saves:		" + userSavePath().string() + "\n"
+		"  user extracted:	" + userExtractedPath().string() + "\n";
 }
 
 void IVCMIDirs::init()

lib/battle/CBattleInfoCallback.cpp

@@ -1245,7 +1245,7 @@ std::pair<const battle::Unit *, BattleHex> CBattleInfoCallback::getNearestStack(
 	// I hate std::pairs with their undescriptive member names first / second
 	struct DistStack
 	{
-		int distanceToPred;
+		uint32_t distanceToPred;
 		BattleHex destination;
 		const battle::Unit * stack;
 	};

lib/battle/ReachabilityInfo.cpp

@@ -43,17 +43,17 @@ bool ReachabilityInfo::isReachable(BattleHex hex) const
 	return distances[hex] < INFINITE_DIST;
 }
 
-int ReachabilityInfo::distToNearestNeighbour(
+uint32_t ReachabilityInfo::distToNearestNeighbour(
 	const std::vector<BattleHex> & targetHexes,
 	BattleHex * chosenHex) const
 {
-	int ret = 1000000;
+	uint32_t ret = 1000000;
 
 	for(auto targetHex : targetHexes)
 	{
 		for(auto & n : targetHex.neighbouringTiles())
 		{
-			if(distances[n] >= 0 && distances[n] < ret)
+			if(distances[n] < ret)
 			{
 				ret = distances[n];
 				if(chosenHex)
@@ -65,7 +65,7 @@ int ReachabilityInfo::distToNearestNeighbour(
 	return ret;
 }
 
-int ReachabilityInfo::distToNearestNeighbour(
+uint32_t ReachabilityInfo::distToNearestNeighbour(
 	const battle::Unit * attacker,
 	const battle::Unit * defender,
 	BattleHex * chosenHex) const

lib/battle/ReachabilityInfo.h

@@ -18,7 +18,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 // startPosition and perpective.
 struct DLL_LINKAGE ReachabilityInfo
 {
-	typedef std::array<int, GameConstants::BFIELD_SIZE> TDistances;
+	typedef std::array<uint32_t, GameConstants::BFIELD_SIZE> TDistances;
 	typedef std::array<BattleHex, GameConstants::BFIELD_SIZE> TPredecessors;
 
 	enum { INFINITE_DIST = 1000000 };
@@ -46,16 +46,14 @@ struct DLL_LINKAGE ReachabilityInfo
 
 	bool isReachable(BattleHex hex) const;
 
-	int distToNearestNeighbour(
+	uint32_t distToNearestNeighbour(
 		const std::vector<BattleHex> & targetHexes,
 		BattleHex * chosenHex = nullptr) const;
 
-	int distToNearestNeighbour(
+	uint32_t distToNearestNeighbour(
 		const battle::Unit * attacker,
 		const battle::Unit * defender,
 		BattleHex * chosenHex = nullptr) const;
 };
 
-
-
 VCMI_LIB_NAMESPACE_END

lib/events/ApplyDamage.cpp

@@ -22,7 +22,7 @@ namespace events
 
 SubscriptionRegistry<ApplyDamage> * ApplyDamage::getRegistry()
 {
-	static std::unique_ptr<SubscriptionRegistry<ApplyDamage>> Instance = make_unique<SubscriptionRegistry<ApplyDamage>>();
+	static std::unique_ptr<SubscriptionRegistry<ApplyDamage>> Instance = std::make_unique<SubscriptionRegistry<ApplyDamage>>();
 	return Instance.get();
 }
 

lib/events/GameResumed.cpp

@@ -20,7 +20,7 @@ namespace events
 
 SubscriptionRegistry<GameResumed> * GameResumed::getRegistry()
 {
-	static std::unique_ptr<SubscriptionRegistry<GameResumed>> Instance = make_unique<SubscriptionRegistry<GameResumed>>();
+	static std::unique_ptr<SubscriptionRegistry<GameResumed>> Instance = std::make_unique<SubscriptionRegistry<GameResumed>>();
 	return Instance.get();
 }
 

lib/events/ObjectVisitEnded.cpp

@@ -21,7 +21,7 @@ namespace events
 
 SubscriptionRegistry<ObjectVisitEnded> * ObjectVisitEnded::getRegistry()
 {
-	static std::unique_ptr<Sub> Instance = make_unique<Sub>();
+	static std::unique_ptr<Sub> Instance = std::make_unique<Sub>();
 	return Instance.get();
 }
 

lib/events/ObjectVisitStarted.cpp

@@ -21,7 +21,7 @@ namespace events
 
 SubscriptionRegistry<ObjectVisitStarted> * ObjectVisitStarted::getRegistry()
 {
-	static std::unique_ptr<Sub> Instance = make_unique<Sub>();
+	static std::unique_ptr<Sub> Instance = std::make_unique<Sub>();
 	return Instance.get();
 }
 

lib/events/PlayerGotTurn.cpp

@@ -20,7 +20,7 @@ namespace events
 
 SubscriptionRegistry<PlayerGotTurn> * PlayerGotTurn::getRegistry()
 {
-	static std::unique_ptr<SubscriptionRegistry<PlayerGotTurn>> Instance = make_unique<SubscriptionRegistry<PlayerGotTurn>>();
+	static std::unique_ptr<SubscriptionRegistry<PlayerGotTurn>> Instance = std::make_unique<SubscriptionRegistry<PlayerGotTurn>>();
 	return Instance.get();
 }
 

lib/events/TurnStarted.cpp

@@ -20,7 +20,7 @@ namespace events
 
 SubscriptionRegistry<TurnStarted> * TurnStarted::getRegistry()
 {
-	static std::unique_ptr<SubscriptionRegistry<TurnStarted>> Instance = make_unique<SubscriptionRegistry<TurnStarted>>();
+	static std::unique_ptr<SubscriptionRegistry<TurnStarted>> Instance = std::make_unique<SubscriptionRegistry<TurnStarted>>();
 	return Instance.get();
 }
 

lib/filesystem/CArchiveLoader.cpp

@@ -152,12 +152,12 @@ void CArchiveLoader::initSNDArchive(const std::string &mountPoint, CFileInputStr
 		char filename[40];
 		reader.read(reinterpret_cast<ui8*>(filename), 40);
 
-		//for some reason entries in snd have format NAME\0WAVRUBBISH....
-		//we need to replace first \0 with dot and take the 3 chars with extension (and drop the rest)
+		// for some reason entries in snd have format NAME\0WAVRUBBISH....
+		// and Polish version does not have extension at all
+		// we need to replace first \0 with dot and add wav extension manuall - we don't expect other types here anyway
 		ArchiveEntry entry;
 		entry.name  = filename; // till 1st \0
-		entry.name += '.';
-		entry.name += std::string(filename + entry.name.size(), 3);
+		entry.name += ".wav";
 
 		entry.offset = reader.readInt32();
 		entry.fullSize = reader.readInt32();
@@ -177,13 +177,13 @@ std::unique_ptr<CInputStream> CArchiveLoader::load(const ResourceID & resourceNa
 
 	if (entry.compressedSize != 0) //compressed data
 	{
-		auto fileStream = make_unique<CFileInputStream>(archive, entry.offset, entry.compressedSize);
+		auto fileStream = std::make_unique<CFileInputStream>(archive, entry.offset, entry.compressedSize);
 
-		return make_unique<CCompressedStream>(std::move(fileStream), false, entry.fullSize);
+		return std::make_unique<CCompressedStream>(std::move(fileStream), false, entry.fullSize);
 	}
 	else
 	{
-		return make_unique<CFileInputStream>(archive, entry.offset, entry.fullSize);
+		return std::make_unique<CFileInputStream>(archive, entry.offset, entry.fullSize);
 	}
 }
 

lib/filesystem/CFilesystemLoader.cpp

@@ -31,7 +31,7 @@ std::unique_ptr<CInputStream> CFilesystemLoader::load(const ResourceID & resourc
 	assert(fileList.count(resourceName));
 	bfs::path file = baseDirectory / fileList.at(resourceName);
 	logGlobal->trace("loading %s", file.string());
-	return make_unique<CFileInputStream>(file);
+	return std::make_unique<CFileInputStream>(file);
 }
 
 bool CFilesystemLoader::existsResource(const ResourceID & resourceName) const

lib/filesystem/CInputStream.h

@@ -45,7 +45,7 @@ public:
 		seek(0);
 		auto readSize = read(data.get(), getSize());
 		assert(readSize == getSize());
-		UNUSED(readSize);
+		MAYBE_UNUSED(readSize);
 
 		return std::make_pair(std::move(data), getSize());
 	}

lib/filesystem/FileStream.h

@@ -40,7 +40,14 @@ struct zlib_filefunc64_def_s;
 typedef zlib_filefunc64_def_s zlib_filefunc64_def;
 
 #ifdef VCMI_DLL
+#ifdef _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4910)
+#endif
 extern template struct DLL_LINKAGE boost::iostreams::stream<VCMI_LIB_WRAP_NAMESPACE(FileBuf)>;
+#ifdef _MSC_VER
+#pragma warning (pop)
+#endif
 #endif
 
 VCMI_LIB_NAMESPACE_BEGIN

lib/filesystem/Filesystem.cpp

@@ -178,7 +178,7 @@ void CResourceHandler::initialize()
 	if (globalResourceHandler.rootLoader)
 		return;
 
-	globalResourceHandler.rootLoader = vstd::make_unique<CFilesystemList>();
+	globalResourceHandler.rootLoader = std::make_unique<CFilesystemList>();
 	knownLoaders["root"] = globalResourceHandler.rootLoader.get();
 	knownLoaders["saves"] = new CFilesystemLoader("SAVES/", VCMIDirs::get().userSavePath());
 	knownLoaders["config"] = new CFilesystemLoader("CONFIG/", VCMIDirs::get().userConfigPath());

lib/int3.h

@@ -161,10 +161,12 @@ public:
 	std::string toString() const
 	{
 		//Performance is important here
-		char str[16] = {};
-		std::sprintf(str, "(%d %d %d)", x, y, z);
+		std::string result = "(" +
+				std::to_string(x) + " " +
+				std::to_string(y) + " " +
+				std::to_string(z) + ")";
 
-		return std::string(str);
+		return result;
 	}
 
 	bool valid() const //Should be named "isValid"?

lib/logging/CBasicLogConfigurator.cpp

@@ -20,8 +20,8 @@ CBasicLogConfigurator::CBasicLogConfigurator(boost::filesystem::path filePath, C
 
 void CBasicLogConfigurator::configureDefault()
 {
-	CLogger::getGlobalLogger()->addTarget(make_unique<CLogConsoleTarget>(console));
-	CLogger::getGlobalLogger()->addTarget(make_unique<CLogFileTarget>(filePath, appendToLogFile));
+	CLogger::getGlobalLogger()->addTarget(std::make_unique<CLogConsoleTarget>(console));
+	CLogger::getGlobalLogger()->addTarget(std::make_unique<CLogFileTarget>(filePath, appendToLogFile));
 	appendToLogFile = true;
 }
 
@@ -52,7 +52,7 @@ void CBasicLogConfigurator::configure()
 		CLogger::getGlobalLogger()->clearTargets();
 
 		// Add console target
-		auto consoleTarget = make_unique<CLogConsoleTarget>(console);
+		auto consoleTarget = std::make_unique<CLogConsoleTarget>(console);
 		const JsonNode & consoleNode = loggingNode["console"];
 		if(!consoleNode.isNull())
 		{
@@ -80,7 +80,7 @@ void CBasicLogConfigurator::configure()
 		CLogger::getGlobalLogger()->addTarget(std::move(consoleTarget));
 
 		// Add file target
-		auto fileTarget = make_unique<CLogFileTarget>(filePath, appendToLogFile);
+		auto fileTarget = std::make_unique<CLogFileTarget>(filePath, appendToLogFile);
 		const JsonNode & fileNode = loggingNode["file"];
 		if(!fileNode.isNull())
 		{

lib/mapObjects/CGHeroInstance.cpp

@@ -122,37 +122,11 @@ TerrainId CGHeroInstance::getNativeTerrain() const
 	return nativeTerrain;
 }
 
-int3 CGHeroInstance::convertPosition(int3 src, bool toh3m) //toh3m=true: manifest->h3m; toh3m=false: h3m->manifest
-{
-	if (toh3m)
-	{
-		src.x+=1;
-		return src;
-	}
-	else
-	{
-		src.x-=1;
-		return src;
-	}
-}
-
 BattleField CGHeroInstance::getBattlefield() const
 {
 	return BattleField::NONE;
 }
 
-int3 CGHeroInstance::getPosition(bool h3m) const //h3m=true - returns position of hero object; h3m=false - returns position of hero 'manifestation'
-{
-	if (h3m)
-	{
-		return pos;
-	}
-	else
-	{
-		return convertPosition(pos,false);
-	}
-}
-
 ui8 CGHeroInstance::getSecSkillLevel(SecondarySkill skill) const
 {
 	for(auto & elem : secSkills)
@@ -190,6 +164,16 @@ void CGHeroInstance::setSecSkillLevel(SecondarySkill which, int val, bool abs)
 	}
 }
 
+int3 CGHeroInstance::convertToVisitablePos(const int3 & position) const
+{
+	return position - getVisitableOffset();
+}
+
+int3 CGHeroInstance::convertFromVisitablePos(const int3 & position) const
+{
+	return position + getVisitableOffset();
+}
+
 bool CGHeroInstance::canLearnSkill() const
 {
 	return secSkills.size() < GameConstants::SKILL_PER_HERO;
@@ -1121,7 +1105,7 @@ EDiggingStatus CGHeroInstance::diggingStatus() const
 	if((int)movement < maxMovePoints(true))
 		return EDiggingStatus::LACK_OF_MOVEMENT;
 
-	return cb->getTile(getPosition(false))->getDiggingStatus();
+	return cb->getTile(visitablePos())->getDiggingStatus();
 }
 
 ArtBearer::ArtBearer CGHeroInstance::bearerType() const
@@ -1375,7 +1359,7 @@ bool CGHeroInstance::hasVisions(const CGObjectInstance * target, const int subty
 	if (visionsMultiplier > 0)
 		vstd::amax(visionsRange, 3); //minimum range is 3 tiles, but only if VISIONS bonus present
 
-	const int distance = static_cast<int>(target->pos.dist2d(getPosition(false)));
+	const int distance = static_cast<int>(target->pos.dist2d(visitablePos()));
 
 	//logGlobal->debug(boost::to_string(boost::format("Visions: dist %d, mult %d, range %d") % distance % visionsMultiplier % visionsRange));
 
@@ -1611,7 +1595,7 @@ void CGHeroInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 		if(!handler.saving)
 		{
 			patrol.patrolling = (rawPatrolRadius > NO_PATROLING);
-			patrol.initialPos = convertPosition(pos, false);
+			patrol.initialPos = visitablePos();
 			patrol.patrolRadius = (rawPatrolRadius > NO_PATROLING) ? rawPatrolRadius : 0;
 		}
 	}

lib/mapObjects/CGHeroInstance.h

@@ -159,7 +159,6 @@ public:
 	ui32 getTileCost(const TerrainTile &dest, const TerrainTile &from, const TurnInfo * ti) const; //move cost - applying pathfinding skill, road and terrain modifiers. NOT includes diagonal move penalty, last move levelling
 	TerrainId getNativeTerrain() const;
 	ui32 getLowestCreatureSpeed() const;
-	int3 getPosition(bool h3m = false) const; //h3m=true - returns position of hero object; h3m=false - returns position of hero 'manifestation'
 	si32 manaRegain() const; //how many points of mana can hero regain "naturally" in one day
 	si32 getManaNewTurn() const; //calculate how much mana this hero is going to have the next day
 	int getCurrentLuck(int stack=-1, bool town=false) const;
@@ -168,6 +167,9 @@ public:
 	bool canLearnSpell(const spells::Spell * spell) const;
 	bool canCastThisSpell(const spells::Spell * spell) const; //determines if this hero can cast given spell; takes into account existing spell in spellbook, existing spellbook and artifact bonuses
 
+	/// convert given position between map position (CGObjectInstance::pos) and visitable position used for hero interactions
+	int3 convertToVisitablePos(const int3 & position) const;
+	int3 convertFromVisitablePos(const int3 & position) const;
 
 	// ----- primary and secondary skill, experience, level handling -----
 
@@ -199,7 +201,6 @@ public:
 
 	int movementPointsAfterEmbark(int MPsBefore, int basicCost, bool disembark = false, const TurnInfo * ti = nullptr) const;
 
-	static int3 convertPosition(int3 src, bool toh3m); //toh3m=true: manifest->h3m; toh3m=false: h3m->manifest
 	double getFightingStrength() const; // takes attack / defense skill into account
 	double getMagicStrength() const; // takes knowledge / spell power skill into account
 	double getHeroStrength() const; // includes fighting and magic strength

lib/mapObjects/CRewardableConstructor.cpp

@@ -126,7 +126,7 @@ void CRandomRewardObjectInfo::configureObject(CRewardableObject * object, CRando
 	object->canRefuse = parameters["canRefuse"].Bool();
 	
 	auto visitMode = parameters["visitMode"].String();
-	for(int i = 0; Rewardable::VisitModeString.size(); ++i)
+	for(int i = 0; i < Rewardable::VisitModeString.size(); ++i)
 	{
 		if(Rewardable::VisitModeString[i] == visitMode)
 		{
@@ -136,7 +136,7 @@ void CRandomRewardObjectInfo::configureObject(CRewardableObject * object, CRando
 	}
 	
 	auto selectMode = parameters["selectMode"].String();
-	for(int i = 0; Rewardable::SelectModeString.size(); ++i)
+	for(int i = 0; i < Rewardable::SelectModeString.size(); ++i)
 	{
 		if(Rewardable::SelectModeString[i] == selectMode)
 		{

lib/mapObjects/CRewardableObject.cpp

@@ -1007,7 +1007,9 @@ void CGVisitableOPH::initObj(CRandomGenerator & rand)
 			info.resize(2);
 			info[0].reward.primary[PrimarySkill::ATTACK] = 1;
 			info[1].reward.primary[PrimarySkill::DEFENSE] = 1;
+			info[0].limiter.resources[Res::GOLD] = 1000;
 			info[0].reward.resources[Res::GOLD] = -1000;
+			info[1].limiter.resources[Res::GOLD] = 1000;
 			info[1].reward.resources[Res::GOLD] = -1000;
 			onSelect.addTxt(MetaString::ADVOB_TXT, 158);
 			onVisited.addTxt(MetaString::ADVOB_TXT, 159);
@@ -1157,7 +1159,7 @@ std::vector<ui32> CGMagicSpring::getAvailableRewards(const CGHeroInstance * hero
 	auto tiles = getVisitableOffsets();
 	for (size_t i=0; i<tiles.size(); i++)
 	{
-		if (pos - tiles[i] == hero->getPosition() && info[i].numOfGrants == 0)
+		if (pos - tiles[i] == hero->visitablePos() && info[i].numOfGrants == 0)
 		{
 			return std::vector<ui32>(1, (ui32)i);
 		}

lib/mapObjects/MiscObjects.cpp

@@ -75,6 +75,11 @@ bool CTeamVisited::wasVisited(PlayerColor player) const
 	return wasVisited(cb->getPlayerState(player)->team);
 }
 
+bool CTeamVisited::wasVisited(const CGHeroInstance * h) const
+{
+	return wasVisited(h->tempOwner);
+}
+
 bool CTeamVisited::wasVisited(TeamID team) const
 {
 	for(auto i : players)
@@ -1069,7 +1074,7 @@ void CGMonolith::onHeroVisit( const CGHeroInstance * h ) const
 			auto exits = cb->getTeleportChannelExits(channel);
 			for(auto exit : exits)
 			{
-				td.exits.push_back(std::make_pair(exit, CGHeroInstance::convertPosition(cb->getObj(exit)->visitablePos(), true)));
+				td.exits.push_back(std::make_pair(exit, h->convertFromVisitablePos(cb->getObj(exit)->visitablePos())));
 			}
 		}
 
@@ -1101,7 +1106,7 @@ void CGMonolith::teleportDialogAnswered(const CGHeroInstance *hero, ui32 answer,
 	else if(vstd::isValidIndex(exits, answer))
 		dPos = exits[answer].second;
 	else
-		dPos = CGHeroInstance::convertPosition(cb->getObj(randomExit)->visitablePos(), true);
+		dPos = hero->convertFromVisitablePos(cb->getObj(randomExit)->visitablePos());
 
 	cb->moveHero(hero->id, dPos, true);
 }
@@ -1145,7 +1150,7 @@ void CGSubterraneanGate::onHeroVisit( const CGHeroInstance * h ) const
 	else
 	{
 		auto exit = getRandomExit(h);
-		td.exits.push_back(std::make_pair(exit, CGHeroInstance::convertPosition(cb->getObj(exit)->visitablePos(), true)));
+		td.exits.push_back(std::make_pair(exit, h->convertFromVisitablePos(cb->getObj(exit)->visitablePos())));
 	}
 
 	cb->showTeleportDialog(&td);
@@ -1254,7 +1259,7 @@ void CGWhirlpool::onHeroVisit( const CGHeroInstance * h ) const
 		{
 			auto blockedPosList = cb->getObj(exit)->getBlockedPos();
 			for(auto bPos : blockedPosList)
-				td.exits.push_back(std::make_pair(exit, CGHeroInstance::convertPosition(bPos, true)));
+				td.exits.push_back(std::make_pair(exit, h->convertFromVisitablePos(bPos)));
 		}
 	}
 
@@ -1278,7 +1283,7 @@ void CGWhirlpool::teleportDialogAnswered(const CGHeroInstance *hero, ui32 answer
 
 		auto obj = cb->getObj(exit);
 		std::set<int3> tiles = obj->getBlockedPos();
-		dPos = CGHeroInstance::convertPosition(*RandomGeneratorUtil::nextItem(tiles, CRandomGenerator::getDefault()), true);
+		dPos = hero->convertFromVisitablePos(*RandomGeneratorUtil::nextItem(tiles, CRandomGenerator::getDefault()));
 	}
 
 	cb->moveHero(hero->id, dPos, true);
@@ -1915,7 +1920,7 @@ void CGMagi::onHeroVisit(const CGHeroInstance * h) const
 
 				cb->sendAndApply(&cv);
 			}
-			cv.pos = h->getPosition(false);
+			cv.pos = h->visitablePos();
 			cv.focusTime = 0;
 			cb->sendAndApply(&cv);
 		}

lib/mapObjects/MiscObjects.h

@@ -23,6 +23,7 @@ class DLL_LINKAGE CTeamVisited: public CGObjectInstance
 public:
 	std::set<PlayerColor> players; //players that visited this object
 
+	bool wasVisited (const CGHeroInstance * h) const override;
 	bool wasVisited(PlayerColor player) const override;
 	bool wasVisited(TeamID team) const;
 	void setPropertyDer(ui8 what, ui32 val) override;

lib/mapping/CCampaignHandler.cpp

@@ -70,7 +70,7 @@ CCampaignHeader CCampaignHandler::getHeader( const std::string & name)
 
 std::unique_ptr<CCampaign> CCampaignHandler::getCampaign( const std::string & name )
 {
-	auto ret = make_unique<CCampaign>();
+	auto ret = std::make_unique<CCampaign>();
 
 	std::vector<std::vector<ui8>> file = getFile(name, false);
 

lib/mapping/CMap.cpp

@@ -712,7 +712,7 @@ void CMap::initTerrain()
 
 CMapEditManager * CMap::getEditManager()
 {
-	if(!editManager) editManager = make_unique<CMapEditManager>(this);
+	if(!editManager) editManager = std::make_unique<CMapEditManager>(this);
 	return editManager.get();
 }
 

lib/mapping/CMapEditManager.cpp

@@ -123,24 +123,24 @@ CMap * CMapEditManager::getMap()
 
 void CMapEditManager::clearTerrain(CRandomGenerator * gen)
 {
-	execute(make_unique<CClearTerrainOperation>(map, gen ? gen : &(this->gen)));
+	execute(std::make_unique<CClearTerrainOperation>(map, gen ? gen : &(this->gen)));
 }
 
 void CMapEditManager::drawTerrain(TerrainId terType, CRandomGenerator * gen)
 {
-	execute(make_unique<CDrawTerrainOperation>(map, terrainSel, terType, gen ? gen : &(this->gen)));
+	execute(std::make_unique<CDrawTerrainOperation>(map, terrainSel, terType, gen ? gen : &(this->gen)));
 	terrainSel.clearSelection();
 }
 
 void CMapEditManager::drawRoad(RoadId roadType, CRandomGenerator* gen)
 {
-	execute(make_unique<CDrawRoadsOperation>(map, terrainSel, roadType, gen ? gen : &(this->gen)));
+	execute(std::make_unique<CDrawRoadsOperation>(map, terrainSel, roadType, gen ? gen : &(this->gen)));
 	terrainSel.clearSelection();
 }
 
 void CMapEditManager::drawRiver(RiverId riverType, CRandomGenerator* gen)
 {
-	execute(make_unique<CDrawRiversOperation>(map, terrainSel, riverType, gen ? gen : &(this->gen)));
+	execute(std::make_unique<CDrawRiversOperation>(map, terrainSel, riverType, gen ? gen : &(this->gen)));
 	terrainSel.clearSelection();
 }
 
@@ -148,35 +148,35 @@ void CMapEditManager::drawRiver(RiverId riverType, CRandomGenerator* gen)
 
 void CMapEditManager::insertObject(CGObjectInstance * obj)
 {
-	execute(make_unique<CInsertObjectOperation>(map, obj));
+	execute(std::make_unique<CInsertObjectOperation>(map, obj));
 }
 
 void CMapEditManager::insertObjects(std::set<CGObjectInstance*>& objects)
 {
-	auto composedOperation = make_unique<CComposedOperation>(map);
+	auto composedOperation = std::make_unique<CComposedOperation>(map);
 	for (auto obj : objects)
 	{
-		composedOperation->addOperation(make_unique<CInsertObjectOperation>(map, obj));
+		composedOperation->addOperation(std::make_unique<CInsertObjectOperation>(map, obj));
 	}
 	execute(std::move(composedOperation));
 }
 
 void CMapEditManager::moveObject(CGObjectInstance * obj, const int3 & pos)
 {
-	execute(make_unique<CMoveObjectOperation>(map, obj, pos));
+	execute(std::make_unique<CMoveObjectOperation>(map, obj, pos));
 }
 
 void CMapEditManager::removeObject(CGObjectInstance * obj)
 {
-	execute(make_unique<CRemoveObjectOperation>(map, obj));
+	execute(std::make_unique<CRemoveObjectOperation>(map, obj));
 }
 
 void CMapEditManager::removeObjects(std::set<CGObjectInstance*> & objects)
 {
-	auto composedOperation = make_unique<CComposedOperation>(map);
+	auto composedOperation = std::make_unique<CComposedOperation>(map);
 	for (auto obj : objects)
 	{
-		composedOperation->addOperation(make_unique<CRemoveObjectOperation>(map, obj));
+		composedOperation->addOperation(std::make_unique<CRemoveObjectOperation>(map, obj));
 	}
 	execute(std::move(composedOperation));
 }

lib/mapping/CMapInfo.cpp

@@ -49,7 +49,7 @@ void CMapInfo::saveInit(ResourceID file)
 	CLoadFile lf(*CResourceHandler::get()->getResourceName(file), MINIMAL_SERIALIZATION_VERSION);
 	lf.checkMagicBytes(SAVEGAME_MAGIC);
 
-	mapHeader = make_unique<CMapHeader>();
+	mapHeader = std::make_unique<CMapHeader>();
 	lf >> *(mapHeader.get()) >> scenarioOptionsOfSave;
 	fileURI = file.getName();
 	countPlayers();