* fixed possible corruption of pack sent by server when player request is rejected

* BattleAI will restore callback to its previous state, fixes freeze after battle #1104
* BattleAI won't lose turn when unable to correctly evaluate a spell
* VCAI will correctly recognize hero standing on town entrance
* War machines of defender will have correctly set side
* Faction 9 as neutral causes crashes, changing to -1 as used elsewhere in the code

AI/BattleAI/BattleAI.cpp

@@ -70,6 +70,10 @@ CBattleAI::CBattleAI(void)
 CBattleAI::~CBattleAI(void)
 {
 	print("destroyed");
+
+	//Restore previous state of CB - it may be shared with the main AI (like VCAI)
+	cb->waitTillRealize = wasWaitingForRealize;
+	cb->unlockGsWhenWaiting = wasUnlockingGs;
 }
 
 void CBattleAI::init( CBattleCallback * CB )
@@ -77,6 +81,9 @@ void CBattleAI::init( CBattleCallback * CB )
 	print("init called, saving ptr to IBattleCallback");
 	cbc = cb = CB;
 	playerID = CB->getPlayerID();; //TODO should be sth in callback
+
+	wasWaitingForRealize = cb->waitTillRealize;
+	wasUnlockingGs = CB->unlockGsWhenWaiting;
 	CB->waitTillRealize = true;
 	CB->unlockGsWhenWaiting = false;
 }
@@ -322,6 +329,14 @@ struct PotentialTargets
 
 		return *vstd::maxElementByFun(possibleAttacks, [](const AttackPossibility &ap) { return ap.damageDiff(); } );
 	}
+
+	int bestActionValue() const
+	{
+		if(possibleAttacks.empty())
+			return 0;
+
+		return bestAction().damageDiff();
+	}
 };
 
 BattleAction CBattleAI::activeStack( const CStack * stack )
@@ -335,6 +350,12 @@ BattleAction CBattleAI::activeStack( const CStack * stack )
 		if(cb->battleCanCastSpell())
 			attemptCastingSpell();
 
+		if(cb->battleGetStacks(CBattleInfoEssentials::ONLY_ENEMY).empty())
+		{
+			//We apparently won battle by casting spell, return defend... (accessing cb may cause trouble)
+			return BattleAction::makeDefend(stack);
+		}
+
 		ThreatMap threatsToUs(stack);
 		PotentialTargets targets(stack);
 
@@ -673,7 +694,7 @@ void CBattleAI::attemptCastingSpell()
 	BOOST_FOREACH(auto stack, cb->battleGetStacks())
 	{
 		PotentialTargets pt(stack);
-		valueOfStack[stack] = pt.bestAction().attackValue();
+		valueOfStack[stack] = pt.bestActionValue();
 	}
 
 	auto evaluateSpellcast = [&] (const PossibleSpellcast &ps) -> int
@@ -730,7 +751,7 @@ void CBattleAI::attemptCastingSpell()
 				state.bonusesOfStacks[swb.stack] = &swb;
 
 				PotentialTargets pt(swb.stack, state);
-				auto newValue = pt.bestAction().attackValue();
+				auto newValue = pt.bestActionValue();
 				auto oldValue = valueOfStack[swb.stack];
 				auto gain = newValue - oldValue;
 				if(swb.stack->owner != playerID) //enemy

AI/BattleAI/BattleAI.h

@@ -8,6 +8,9 @@ class CBattleAI : public CBattleGameInterface
 {
 	int side;
 	CBattleCallback *cb;
+	
+	//Previous setting of cb 
+	bool wasWaitingForRealize, wasUnlockingGs;
 
 	void print(const std::string &text) const;
 public:

AI/VCAI/VCAI.cpp

@@ -359,7 +359,12 @@ ui64 evaluateDanger(crint3 tile, const CGHeroInstance *visitor)
 
 	ui64 objectDanger = 0, guardDanger = 0;
 
-	if(const CGObjectInstance * dangerousObject = backOrNull(cb->getVisitableObjs(tile)))
+	auto visitableObjects = cb->getVisitableObjs(tile);
+	// in some scenarios hero happens to be "under" the object (eg town). Then we consider ONLY the hero.
+	if(vstd::contains_if(visitableObjects, objWithID<Obj::HERO>))
+		erase_if(visitableObjects, ! boost::bind(objWithID<Obj::HERO>, _1));
+
+	if(const CGObjectInstance * dangerousObject = backOrNull(visitableObjects))
 	{
 		objectDanger = evaluateDanger(dangerousObject); //unguarded objects can also be dangerous or unhandled
 		if (objectDanger)

lib/BattleState.cpp

@@ -513,7 +513,7 @@ BattleInfo * BattleInfo::setupBattle( int3 tile, int terrain, int battlefieldTyp
 		auto handleWarMachine= [&](int side, int artslot, int cretype, int hex)
 		{
 			if(heroes[side] && heroes[side]->getArt(artslot))
-				stacks.push_back(curB->generateNewStack(CStackBasicDescriptor(cretype, 1), true, 255, hex));
+				stacks.push_back(curB->generateNewStack(CStackBasicDescriptor(cretype, 1), !side, 255, hex));
 		};
 
 		handleWarMachine(0, 13, 146, 52); //ballista

lib/CModHandler.cpp

@@ -123,7 +123,7 @@ CCreature * CModHandler::loadCreature (const JsonNode &node)
 	cre->cost = Res::ResourceSet(node["cost"]);
 
 	cre->level = node["level"].Float();
-	cre->faction = 9; //neutral faction is 9 for now. Will be replaced by string -> id conversion
+	cre->faction = -1; //neutral faction is 9 for now. Will be replaced by string -> id conversion
 	//TODO: node["faction"].String() to id
 	cre->fightValue = node["fightValue"].Float();
 	cre->AIValue = node["aiValue"].Float();

server/NetPacksServer.cpp

@@ -14,7 +14,8 @@
 #define ERROR_AND_RETURN												\
 	do { if(c) {														\
 			SystemMessage temp_message("You are not allowed to perform this action!"); \
-			*c << &temp_message; \
+			boost::unique_lock<boost::mutex> lock(*c->wmx);				\
+			*c << &temp_message;										\
 		}																\
 		tlog1<<"Player is not allowed to perform this action!\n";		\
 		return false;} while(0)
@@ -22,7 +23,7 @@
 #define WRONG_PLAYER_MSG(expectedplayer) do {std::ostringstream oss;\
 			oss << "You were identified as player " << (int)gh->getPlayerAt(c) << " while expecting " << (int)expectedplayer;\
 			tlog1 << oss.str() << std::endl; \
-			if(c) { SystemMessage temp_message(oss.str()); *c << &temp_message; } } while(0)
+			if(c) { SystemMessage temp_message(oss.str()); boost::unique_lock<boost::mutex> lock(*c->wmx); *c << &temp_message; } } while(0)
 
 #define ERROR_IF_NOT_OWNS(id)	do{if(!PLAYER_OWNS(id)){WRONG_PLAYER_MSG(gh->getOwner(id)); ERROR_AND_RETURN; }}while(0)
 #define ERROR_IF_NOT(player)	do{if(player != gh->getPlayerAt(c)){WRONG_PLAYER_MSG(player); ERROR_AND_RETURN; }}while(0)