- updated icons + psd file

 - better log messages if server failed to open port
 - 1148 should be fixed. Cleanup in CGameHandler::moveHero()
 - compile fixes

AI/VCAI/VCAI.cpp

@@ -3,7 +3,7 @@
 #include "../../lib/UnlockGuard.h"
 #include "../../lib/CObjectHandler.h"
 #include "../../lib/CConfigHandler.h"
-#include "../../lib/CHerohandler.h"
+#include "../../lib/CHeroHandler.h"
 
 #define I_AM_ELEMENTAR return CGoal(*this).setisElementar(true)
 CLogger &aiLogger = tlog6;
@@ -103,9 +103,9 @@ std::string CGoal::name() const
 		case GATHER_TROOPS:
 			return "GATHER TROOPS";
 		case GET_OBJ:
-			return "GET OBJECT " + objid;
+			return "GET OBJECT " + boost::lexical_cast<std::string>(objid);
 		case FIND_OBJ:
-			return "FIND OBJECT " + objid;
+			return "FIND OBJECT " + boost::lexical_cast<std::string>(objid);
 		case VISIT_HERO:
 			return "VISIT HERO " + VLC->heroh->heroes[objid]->name;
 		case GET_ART_TYPE:

client/CPlayerInterface.cpp

@@ -286,7 +286,7 @@ void CPlayerInterface::heroMoved(const TryMoveHero & details)
 		if (ho->pos != details.end //hero didn't change tile but visit succeeded
 			|| directlyAttackingCreature) // or creature was attacked from endangering tile.
 		{
-			eraseCurrentPathOf(ho);
+			eraseCurrentPathOf(ho, false);
 		}
 		else if(adventureInt->terrain.currentPath  &&  ho->pos == details.end) //&& hero is moving
 		{

client/Client.cpp

@@ -751,7 +751,11 @@ void CServerHandler::callServer()
 	if (result == 0)
 		tlog1 << "Server closed correctly\n";
 	else
+	{
 		tlog0 << "Error: server failed to close correctly or crashed!\n";
+		tlog0 << "Check " << logName << " for more info\n";
+		exit(1);// exit in case of error. Othervice without working server VCMI will hang
+	}
 }
 
 CConnection * CServerHandler::justConnectToServer(const std::string &host, const std::string &port)

server/CGameHandler.cpp

@@ -1635,7 +1635,6 @@ void CGameHandler::setAmount(int objid, ui32 val)
 
 bool CGameHandler::moveHero( si32 hid, int3 dst, ui8 instant, ui8 asker /*= 255*/ )
 {
-	bool blockvis = false;
 	const CGHeroInstance *h = getHero(hid);
 
 	if(!h  || (asker != 255 && (instant  ||   h->getOwner() != gs->currentPlayer)) //not turn of that hero or player can't simply teleport hero (at least not with this function)
@@ -1645,7 +1644,6 @@ bool CGameHandler::moveHero( si32 hid, int3 dst, ui8 instant, ui8 asker /*= 255*
 		return false;
 	}
 
-
 	tlog5 << "Player " <<int(asker) << " wants to move hero "<< hid << " from "<< h->pos << " to " << dst << std::endl;
 	int3 hmpos = dst + int3(-1,0,0);
 
@@ -1671,12 +1669,14 @@ bool CGameHandler::moveHero( si32 hid, int3 dst, ui8 instant, ui8 asker /*= 255*
 
 	//it's a rock or blocked and not visitable tile
 	//OR hero is on land and dest is water and (there is not present only one object - boat)
-    if(((t.terType == ETerrainType::ROCK  ||  (t.blocked && !t.visitable && !h->hasBonusOfType(Bonus::FLYING_MOVEMENT) ))
+	if(((t.terType == ETerrainType::ROCK  ||  (t.blocked && !t.visitable && !h->hasBonusOfType(Bonus::FLYING_MOVEMENT) ))
 			&& complain("Cannot move hero, destination tile is blocked!"))
-        || ((!h->boat && !h->canWalkOnSea() && t.terType == ETerrainType::WATER && (t.visitableObjects.size() < 1 ||  (t.visitableObjects.back()->ID != 8 && t.visitableObjects.back()->ID != Obj::HERO)))  //hero is not on boat/water walking and dst water tile doesn't contain boat/hero (objs visitable from land) -> we test back cause boat may be on top of another object (#276)
+		|| ((!h->boat && !h->canWalkOnSea() && t.terType == ETerrainType::WATER && (t.visitableObjects.size() < 1 ||  (t.visitableObjects.back()->ID != 8 && t.visitableObjects.back()->ID != Obj::HERO)))  //hero is not on boat/water walking and dst water tile doesn't contain boat/hero (objs visitable from land) -> we test back cause boat may be on top of another object (#276)
 			&& complain("Cannot move hero, destination tile is on water!"))
-        || ((h->boat && t.terType != ETerrainType::WATER && t.blocked)
+		|| ((h->boat && t.terType != ETerrainType::WATER && t.blocked)
 			&& complain("Cannot disembark hero, tile is blocked!"))
+		|| ( (distance(h->pos, dst) >= 1.5 && !instant)
+			&& complain("Tiles are not neighboring!"))
 		|| ((h->movement < cost  &&  dst != h->pos  &&  !instant)
 			&& complain("Hero doesn't have any movement points left!"))
 		|| (states.checkFlag(h->tempOwner, &PlayerStatus::engagedIntoBattle)
@@ -1687,105 +1687,104 @@ bool CGameHandler::moveHero( si32 hid, int3 dst, ui8 instant, ui8 asker /*= 255*
 		return false;
 	}
 
+	//several generic blocks of code
+
+	// should be called if hero changes tile but before applying TryMoveHero package
+	auto leaveTile = [&]()
+	{
+		BOOST_FOREACH(CGObjectInstance *obj, gs->map->terrain[h->pos.x-1][h->pos.y][h->pos.z].visitableObjects)
+		{
+			obj->onHeroLeave(h);
+		}
+		getTilesInRange(tmh.fowRevealed, h->getSightCenter()+(tmh.end-tmh.start), h->getSightRadious(), h->tempOwner, 1);
+	};
+
+	//interaction with blocking object (like resources)
+	auto blockingVisit = [&]() -> bool
+	{
+		BOOST_FOREACH(CGObjectInstance *obj, t.visitableObjects)
+		{
+			if(obj != h  &&  obj->blockVisit  &&  !(obj->getPassableness() & 1<<h->tempOwner))
+			{
+				//can't move to that tile but we visit object
+				objectVisited(t.visitableObjects.back(), h);
+
+				tlog5 << "Blocking visit at " << hmpos << std::endl;
+				return true;
+			}
+		}
+		return false;
+	};
+
+	auto applyWithResult = [&](TryMoveHero::EResult result) -> bool
+	{
+		tmh.result = result;
+		sendAndApply(&tmh);
+		return result != TryMoveHero::FAILED;
+	};
+
 	//hero enters the boat
-	if(!h->boat && t.visitableObjects.size() && t.visitableObjects.back()->ID == Obj::BOAT)
+	if(!h->boat && !t.visitableObjects.empty() && t.visitableObjects.back()->ID == Obj::BOAT)
 	{
-		tmh.result = TryMoveHero::EMBARK;
+		// blockingVisit test?
+		leaveTile();
 		tmh.movePoints = h->movementPointsAfterEmbark(h->movement, cost, false);
-		getTilesInRange(tmh.fowRevealed,h->getSightCenter()+(tmh.end-tmh.start),h->getSightRadious(),h->tempOwner,1);
-		sendAndApply(&tmh);
-		return true;
+		return applyWithResult(TryMoveHero::EMBARK);
+		//attack guards on embarking? In H3 creatures on water had no zone of control at all
 	}
+
 	//hero leaves the boat
-    else if(h->boat && t.terType != ETerrainType::WATER && !t.blocked)
+	if(h->boat && t.terType != ETerrainType::WATER && !t.blocked)
 	{
-		//TODO? code similarity with the block above
-		tmh.result = TryMoveHero::DISEMBARK;
+		// blockingVisit test?
+		leaveTile();
+		tmh.attackedFrom = guardPos;
 		tmh.movePoints = h->movementPointsAfterEmbark(h->movement, cost, true);
-		getTilesInRange(tmh.fowRevealed,h->getSightCenter()+(tmh.end-tmh.start),h->getSightRadious(),h->tempOwner,1);
-		sendAndApply(&tmh);
-		tryAttackingGuard(guardPos, h);
+
+		applyWithResult(TryMoveHero::DISEMBARK);
+		if (!tryAttackingGuard(guardPos, h))
+			visitObjectOnTile(t, h);
 		return true;
 	}
 
-	//checks for standard movement
+	//standard movement
 	if(!instant)
 	{
-		if( (distance(h->pos,dst) >= 1.5  &&  complain("Tiles are not neighboring!"))
-			|| (h->movement < cost  &&  h->movement < 100  &&  complain("Not enough move points!")))
-		{
-			sendAndApply(&tmh);
-			return false;
-		}
+		tmh.movePoints = std::max(ui32(0), h->movement - cost);
 
-		//check if there is blocking visitable object
-		blockvis = false;
-		tmh.movePoints = std::max((ui32)(0),h->movement-cost); //take move points
-		BOOST_FOREACH(CGObjectInstance *obj, t.visitableObjects)
-		{
-			if(obj != h  &&  obj->blockVisit  &&  !(obj->getPassableness() & 1<<h->tempOwner))
-			{
-				blockvis = true;
-				break;
-			}
-		}
-		//we start moving
-		if(blockvis)//interaction with blocking object (like resources)
-		{
-			tmh.result = TryMoveHero::BLOCKING_VISIT;
-			sendAndApply(&tmh);
-			//failed to move to that tile but we visit object
-			if(t.visitableObjects.size())
-				objectVisited(t.visitableObjects.back(), h);
+		if(blockingVisit())
+			return applyWithResult(TryMoveHero::BLOCKING_VISIT);
 
-			tlog5 << "Blocking visit at " << hmpos << std::endl;
-			return true;
-		}
-		else //normal move
-		{
-			BOOST_FOREACH(CGObjectInstance *obj, gs->map->terrain[h->pos.x-1][h->pos.y][h->pos.z].visitableObjects)
-			{
-				obj->onHeroLeave(h);
-			}
-			getTilesInRange(tmh.fowRevealed,h->getSightCenter()+(tmh.end-tmh.start),h->getSightRadious(),h->tempOwner,1);
+		leaveTile();
 
-			tmh.result = TryMoveHero::SUCCESS;
-			tmh.attackedFrom = guardPos;
-			sendAndApply(&tmh);
-			tlog5 << "Moved to " <<tmh.end<<std::endl;
+		tmh.attackedFrom = guardPos;
+		applyWithResult(TryMoveHero::SUCCESS);
 
-			// If a creature guards the tile, block visit.
-			const bool fightingGuard = tryAttackingGuard(guardPos, h);
+		tlog5 << "Moved to " <<tmh.end<<std::endl;
 
-			if(!fightingGuard && t.visitableObjects.size()) //call objects if they are visited
-			{
-				visitObjectOnTile(t, h);
-			}
+		if (!tryAttackingGuard(guardPos, h)) // If a creature guards the tile, block visit.
+			visitObjectOnTile(t, h);
 
-			tlog5 << "Movement end!\n";
-			return true;
-		}
+		tlog5 << "Movement end!\n";
+		return true;
 	}
 	else //instant move - teleportation
 	{
-		BOOST_FOREACH(CGObjectInstance* obj, t.blockingObjects)
-		{
-			if(obj->ID==Obj::HERO)
-			{
-				CGHeroInstance *dh = static_cast<CGHeroInstance *>(obj);
+		if(blockingVisit()) // e.g. hero on the other side of teleporter
+			return applyWithResult(TryMoveHero::BLOCKING_VISIT);
 
-				if( gameState()->getPlayerRelations(dh->tempOwner, h->tempOwner))
-				{
-					heroExchange(h->id, dh->id);
-					return true;
-				}
-				startBattleI(h, dh);
-				return true;
-			}
+		leaveTile();
+		applyWithResult(TryMoveHero::TELEPORTATION);
+
+		// visit town for town portal \ castle gates
+		// do not use generic visitObjectOnTile to avoid double-teleporting
+		// if this moveHero call was triggered by teleporter
+		if (!t.visitableObjects.empty())
+		{
+			if (CGTownInstance * town = dynamic_cast<CGTownInstance *>(t.visitableObjects.back()))
+				town->onHeroVisit(h);
 		}
-		tmh.result = TryMoveHero::TELEPORTATION;
-		getTilesInRange(tmh.fowRevealed,h->getSightCenter()+(tmh.end-tmh.start),h->getSightRadious(),h->tempOwner,1);
-		sendAndApply(&tmh);
+
 		return true;
 	}
 }
@@ -1808,9 +1807,7 @@ bool CGameHandler::teleportHero(si32 hid, si32 dstid, ui8 source, ui8 asker/* =
 			return false;
 	int3 pos = t->visitablePos();
 	pos += h->getVisitableOffset();
-	stopHeroVisitCastle(from->id, hid);
 	moveHero(hid,pos,1);
-	heroVisitCastle(dstid, hid);
 	return true;
 }
 
@@ -5484,19 +5481,13 @@ bool CGameHandler::castSpell(const CGHeroInstance *h, int spellID, const int3 &p
 				break;
 			}
 
-			//we need obtain guard pos before moving hero, otherwise we get nothing, because tile will be "unguarded" by hero
-			int3 guardPos = gs->guardingCreaturePosition(pos);
-
-			TryMoveHero tmh;
-			tmh.id = h->id;
-			tmh.movePoints = std::max<int>(0, h->movement - 300);
-			tmh.result = TryMoveHero::TELEPORTATION;
-			tmh.start = h->pos;
-			tmh.end = pos + h->getVisitableOffset();
-			getTilesInRange(tmh.fowRevealed, pos, h->getSightRadious(), h->tempOwner,1);
-			sendAndApply(&tmh);
-
-			tryAttackingGuard(guardPos, h);
+			if (moveHero(h->id, pos + h->getVisitableOffset(), true))
+			{
+				SetMovePoints smp;
+				smp.hid = h->id;
+				smp.val = std::max<ui32>(0, h->movement - 300);
+				sendAndApply(&smp);
+			}
 		}
 		break;
 	case FLY: //Fly
@@ -5550,10 +5541,7 @@ bool CGameHandler::castSpell(const CGHeroInstance *h, int spellID, const int3 &p
 				if (town->id != nearest)
 					COMPLAIN_RET("This hero can only teleport to nearest town!")
 			}
-			if (h->visitedTown)
-				stopHeroVisitCastle(h->visitedTown->id, h->id);
-			if (moveHero(h->id, town->visitablePos() + h->getVisitableOffset() ,1))
-				heroVisitCastle(town->id, h->id);
+			moveHero(h->id, town->visitablePos() + h->getVisitableOffset() ,1);
 		}
 		break;
 
@@ -5576,11 +5564,14 @@ bool CGameHandler::castSpell(const CGHeroInstance *h, int spellID, const int3 &p
 
 void CGameHandler::visitObjectOnTile(const TerrainTile &t, const CGHeroInstance * h)
 {
-	//to prevent self-visiting heroes on space press
-	if(t.visitableObjects.back() != h)
-		objectVisited(t.visitableObjects.back(), h);
-	else if(t.visitableObjects.size() > 1)
-		objectVisited(*(t.visitableObjects.end()-2),h);
+	if (!t.visitableObjects.empty())
+	{
+		//to prevent self-visiting heroes on space press
+		if(t.visitableObjects.back() != h)
+			objectVisited(t.visitableObjects.back(), h);
+		else if(t.visitableObjects.size() > 1)
+			objectVisited(*(t.visitableObjects.end()-2),h);
+	}
 }
 
 bool CGameHandler::tryAttackingGuard(const int3 &guardPos, const CGHeroInstance * h)

server/CVCMIServer.cpp

@@ -515,16 +515,28 @@ int main(int argc, char** argv)
 	{
 		io_service io_service;
 		CVCMIServer server;
-		while(!end2)
+
+		try
 		{
-			server.start();
+			while(!end2)
+			{
+				server.start();
+			}
+			io_service.run();
 		}
-		io_service.run();
-	} 
-	catch(boost::system::system_error &e) //for boost errors just log, not crash - probably client shut down connection
+		catch(boost::system::system_error &e) //for boost errors just log, not crash - probably client shut down connection
+		{
+			tlog1 << e.what() << std::endl;
+			end2 = true;
+		}HANDLE_EXCEPTION
+	}
+	catch(boost::system::system_error &e)
 	{
 		tlog1 << e.what() << std::endl;
-		end2 = true;
-	}HANDLE_EXCEPTION
+		//catch any startup errors (e.g. can't access port) errors
+		//and return non-zero status so client can detect error
+		throw;
+	}
+
   return 0;
 }