Added convertToVisitablePos/convertFromVisitablePos to CGHeroInstance

This method replaces old convertPosition, but with more clear names and
without hardcoded magic constants.

AI/Nullkiller/AIGateway.cpp

@@ -92,8 +92,8 @@ 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 = details.start - hero->getVisitableOffset();
-	const int3 to = details.end - hero->getVisitableOffset();
+	const int3 from = hero->convertToVisitablePos(details.start);
+	const int3 to = hero->convertToVisitablePos(details.end);
 	const CGObjectInstance * o1 = vstd::frontOrNull(cb->getVisitableObjs(from, verbose));
 	const CGObjectInstance * o2 = vstd::frontOrNull(cb->getVisitableObjs(to, verbose));
 
@@ -1178,7 +1178,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, dst + h->getVisitableOffset());
+		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();
@@ -1232,14 +1232,14 @@ bool AIGateway::moveHeroToTile(int3 dst, HeroPtr h)
 
 		auto doMovement = [&](int3 dst, bool transit)
 		{
-			cb->moveHero(*h, dst + h->getVisitableOffset(), transit);
+			cb->moveHero(*h, h->convertFromVisitablePos(dst), transit);
 		};
 
 		auto doTeleportMovement = [&](ObjectInstanceID exitId, int3 exitPos)
 		{
 			destinationTeleport = exitId;
 			if(exitPos.valid())
-				destinationTeleportPos = exitPos + h->getVisitableOffset();
+				destinationTeleportPos = h->convertFromVisitablePos(exitPos);
 			cb->moveHero(*h, h->pos);
 			destinationTeleport = ObjectInstanceID();
 			destinationTeleportPos = int3(-1);

AI/VCAI/VCAI.cpp

@@ -101,8 +101,8 @@ void VCAI::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 = details.start - hero->getVisitableOffset();
-	const int3 to = details.end - hero->getVisitableOffset();
+	const int3 from = hero->convertToVisitablePos(details.start);
+	const int3 to = hero->convertToVisitablePos(details.end);
 	const CGObjectInstance * o1 = vstd::frontOrNull(cb->getVisitableObjs(from, verbose));
 	const CGObjectInstance * o2 = vstd::frontOrNull(cb->getVisitableObjs(to, verbose));
 
@@ -1813,7 +1813,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, dst + h->getVisitableOffset());
+		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 +1867,14 @@ bool VCAI::moveHeroToTile(int3 dst, HeroPtr h)
 
 		auto doMovement = [&](int3 dst, bool transit)
 		{
-			cb->moveHero(*h, dst + h->getVisitableOffset(), transit);
+			cb->moveHero(*h, h->convertFromVisitablePos(dst), transit);
 		};
 
 		auto doTeleportMovement = [&](ObjectInstanceID exitId, int3 exitPos)
 		{
 			destinationTeleport = exitId;
 			if(exitPos.valid())
-				destinationTeleportPos = exitPos + h->getVisitableOffset();
+				destinationTeleportPos = h->convertFromVisitablePos(exitPos);
 			cb->moveHero(*h, h->pos);
 			destinationTeleport = ObjectInstanceID();
 			destinationTeleportPos = int3(-1);

client/CPlayerInterface.cpp

@@ -264,8 +264,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 == details.start - hero->getVisitableOffset()
-					&& (nodesIt - 1)->coord == details.end - hero->getVisitableOffset())
+				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);
@@ -1573,7 +1573,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);
@@ -2405,7 +2405,7 @@ void CPlayerInterface::doMoveHero(const CGHeroInstance * h, CGPath path)
 	int i = 1;
 	auto getObj = [&](int3 coord, bool ignoreHero)
 	{
-		return cb->getTile(coord - h->getVisitableOffset())->topVisitableObj(ignoreHero);
+		return cb->getTile(h->convertToVisitablePos(coord))->topVisitableObj(ignoreHero);
 	};
 
 	auto isTeleportAction = [&](CGPathNode::ENodeAction action) -> bool
@@ -2445,7 +2445,7 @@ void CPlayerInterface::doMoveHero(const CGHeroInstance * h, CGPath path)
 
 	{
 		for (auto & elem : path.nodes)
-			elem.coord = elem.coord + h->getVisitableOffset();
+			elem.coord = h->convertFromVisitablePos(elem.coord);
 
 		TerrainId currentTerrain = Terrain::BORDER; // not init yet
 		TerrainId newTerrain;
@@ -2503,7 +2503,7 @@ void CPlayerInterface::doMoveHero(const CGHeroInstance * h, CGPath path)
 				sh = CCS->soundh->playSound(soundBase::horseFlying, -1);
 #endif
 			{
-				newTerrain = cb->getTile(currentCoord - h->getVisitableOffset())->terType->id;
+				newTerrain = cb->getTile(h->convertToVisitablePos(currentCoord))->terType->id;
 				if(newTerrain != currentTerrain)
 				{
 					CCS->soundh->stopSound(sh);

lib/CGameState.cpp

@@ -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;

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(end - h->getVisitableOffset());
+		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);
 

lib/mapObjects/CGHeroInstance.cpp

@@ -164,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;
@@ -1585,7 +1595,7 @@ void CGHeroInstance::serializeJsonOptions(JsonSerializeFormat & handler)
 		if(!handler.saving)
 		{
 			patrol.patrolling = (rawPatrolRadius > NO_PATROLING);
-			patrol.initialPos = pos - getVisitableOffset();
+			patrol.initialPos = visitablePos();
 			patrol.patrolRadius = (rawPatrolRadius > NO_PATROLING) ? rawPatrolRadius : 0;
 		}
 	}

lib/mapObjects/CGHeroInstance.h

@@ -167,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 -----
 

lib/mapObjects/MiscObjects.cpp

@@ -1074,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, cb->getObj(exit)->visitablePos() + h->getVisitableOffset()));
+				td.exits.push_back(std::make_pair(exit, h->convertFromVisitablePos(cb->getObj(exit)->visitablePos())));
 			}
 		}
 
@@ -1106,7 +1106,7 @@ void CGMonolith::teleportDialogAnswered(const CGHeroInstance *hero, ui32 answer,
 	else if(vstd::isValidIndex(exits, answer))
 		dPos = exits[answer].second;
 	else
-		dPos = cb->getObj(randomExit)->visitablePos() + hero->getVisitableOffset();
+		dPos = hero->convertFromVisitablePos(cb->getObj(randomExit)->visitablePos());
 
 	cb->moveHero(hero->id, dPos, true);
 }
@@ -1150,7 +1150,7 @@ void CGSubterraneanGate::onHeroVisit( const CGHeroInstance * h ) const
 	else
 	{
 		auto exit = getRandomExit(h);
-		td.exits.push_back(std::make_pair(exit, cb->getObj(exit)->visitablePos() + h->getVisitableOffset()));
+		td.exits.push_back(std::make_pair(exit, h->convertFromVisitablePos(cb->getObj(exit)->visitablePos())));
 	}
 
 	cb->showTeleportDialog(&td);
@@ -1259,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, bPos + h->getVisitableOffset()));
+				td.exits.push_back(std::make_pair(exit, h->convertFromVisitablePos(bPos)));
 		}
 	}
 
@@ -1283,7 +1283,7 @@ void CGWhirlpool::teleportDialogAnswered(const CGHeroInstance *hero, ui32 answer
 
 		auto obj = cb->getObj(exit);
 		std::set<int3> tiles = obj->getBlockedPos();
-		dPos = *RandomGeneratorUtil::nextItem(tiles, CRandomGenerator::getDefault()) + hero->getVisitableOffset();
+		dPos = hero->convertFromVisitablePos(*RandomGeneratorUtil::nextItem(tiles, CRandomGenerator::getDefault()));
 	}
 
 	cb->moveHero(hero->id, dPos, true);

lib/mapping/MapFormatH3M.cpp

@@ -1664,7 +1664,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(ObjectInstanceID idToBeGiven, const i
 	else
 	{
 		nhi->patrol.patrolling = true;
-		nhi->patrol.initialPos = initialPos - nhi->getVisitableOffset();
+		nhi->patrol.initialPos = nhi->convertToVisitablePos(initialPos);
 	}
 
 	if(map->version > EMapFormat::ROE)

lib/spells/AdventureSpellMechanics.cpp

@@ -324,7 +324,7 @@ ESpellCastResult DimensionDoorMechanics::applyAdventureEffects(SpellCastEnvironm
 		iw.text.addTxt(MetaString::GENERAL_TXT, 70); //Dimension Door failed!
 		env->apply(&iw);
 	}
-	else if(env->moveHero(parameters.caster->id, parameters.pos + parameters.caster->getVisitableOffset(), true))
+	else if(env->moveHero(parameters.caster->id, parameters.caster->convertFromVisitablePos(parameters.pos), true))
 	{
 		SetMovePoints smp;
 		smp.hid = parameters.caster->id;
@@ -424,7 +424,7 @@ ESpellCastResult TownPortalMechanics::applyAdventureEffects(SpellCastEnvironment
 		return ESpellCastResult::ERROR;
 	}
 
-	if(env->moveHero(parameters.caster->id, destination->visitablePos() + parameters.caster->getVisitableOffset(), true))
+	if(env->moveHero(parameters.caster->id, parameters.caster->convertFromVisitablePos(destination->visitablePos()), true))
 	{
 		SetMovePoints smp;
 		smp.hid = parameters.caster->id;

server/CGameHandler.cpp

@@ -2326,7 +2326,7 @@ bool CGameHandler::moveHero(ObjectInstanceID hid, int3 dst, ui8 teleporting, boo
 	}
 
 	logGlobal->trace("Player %d (%s) wants to move hero %d from %s to %s", asker, asker.getStr(), hid.getNum(), h->pos.toString(), dst.toString());
-	const int3 hmpos = dst - h->getVisitableOffset();
+	const int3 hmpos = h->convertToVisitablePos(dst);
 
 	if (!gs->map->isInTheMap(hmpos))
 	{
@@ -2526,8 +2526,8 @@ bool CGameHandler::teleportHero(ObjectInstanceID hid, ObjectInstanceID dstid, ui
 	|| (!t->hasBuilt(BuildingSubID::CASTLE_GATE)
 		&& complain("Cannot teleport hero to town without Castle gate in it")))
 			return false;
-	int3 pos = t->visitablePos();
-	pos += h->getVisitableOffset();
+
+	int3 pos = h->convertFromVisitablePos(t->visitablePos());
 	moveHero(hid,pos,1);
 	return true;
 }
@@ -4353,7 +4353,7 @@ bool CGameHandler::hireHero(const CGObjectInstance *obj, ui8 hid, PlayerColor pl
 	hr.tid = obj->id;
 	hr.hid = nh->subID;
 	hr.player = player;
-	hr.tile = obj->visitablePos() + nh->getVisitableOffset();
+	hr.tile = nh->convertFromVisitablePos(obj->visitablePos());
 	sendAndApply(&hr);
 
 	std::map<ui32, ConstTransitivePtr<CGHeroInstance> > pool = gs->unusedHeroesFromPool();

server/CQuery.cpp

@@ -489,7 +489,7 @@ void CHeroMovementQuery::onExposure(QueryPtr topQuery)
 		logGlobal->trace("Hero %s after victory over guard finishes visit to %s", hero->name, tmh.end.toString());
 		//finish movement
 		visitDestAfterVictory = false;
-		gh->visitObjectOnTile(*gh->getTile(tmh.end - hero->getVisitableOffset()), hero);
+		gh->visitObjectOnTile(*gh->getTile(hero->convertToVisitablePos(tmh.end)), hero);
 	}
 
 	owner->popIfTop(*this);