Rewritten many parts of query handling. Fixed several scenarios leading to a hang (including #1012). Purged boost::function from player interface (handy but impossible to serialize). VCAI will keep description for each unanswered query, so the further debugging will be easier.

AI/EmptyAI/CEmptyAI.cpp

@@ -12,14 +12,15 @@ void CEmptyAI::yourTurn()
 {
 	cb->endTurn();
 }
-void CEmptyAI::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> &callback)
+
+void CEmptyAI::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, int queryID)
 {
-	callback(rand()%skills.size());
+	cb->selectionMade(rand() % skills.size(), queryID);
 }
 
-void CEmptyAI::commanderGotLevel(const CCommanderInstance * commander, std::vector<ui32> skills, boost::function<void(ui32)> &callback)
+void CEmptyAI::commanderGotLevel(const CCommanderInstance * commander, std::vector<ui32> skills, int queryID)
 {
-	callback(0);
+	cb->selectionMade(rand() % skills.size(), queryID);
 }
 
 void CEmptyAI::showBlockingDialog(const std::string &text, const std::vector<Component> &components, ui32 askID, const int soundID, bool selection, bool cancel)
@@ -27,7 +28,7 @@ void CEmptyAI::showBlockingDialog(const std::string &text, const std::vector<Com
 	cb->selectionMade(0, askID);
 }
 
-void CEmptyAI::showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function<void()> &onEnd)
+void CEmptyAI::showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID)
 {
-	onEnd();
+	cb->selectionMade(0, queryID);
 }

AI/EmptyAI/CEmptyAI.h

@@ -12,10 +12,10 @@ class CEmptyAI : public CGlobalAI
 public:
 	void init(CCallback * CB) override;
 	void yourTurn() override;
-	void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> &callback) override; 
+	void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, int queryID) override; 
-	void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, boost::function<void(ui32)> &callback) override;
+	void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, int queryID) override;
 	void showBlockingDialog(const std::string &text, const std::vector<Component> &components, ui32 askID, const int soundID, bool selection, bool cancel) override; 
-	void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function<void()> &onEnd) override;
+	void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID) override;
 };
 
 #define NAME "EmptyAI 0.1"

AI/VCAI/VCAI.cpp

@@ -733,7 +733,7 @@ void VCAI::requestRealized(PackageApplied *pa)
 
 	if(pa->packType == typeList.getTypeID<QueryReply>())
 	{
-		status.removeQuery();
+		status.receivedAnswerConfirmation(pa->requestID, pa->result);
 	}
 }
 
@@ -829,20 +829,20 @@ void VCAI::yourTurn()
 	makingTurn = new boost::thread(&VCAI::makeTurn, this);
 }
 
-void VCAI::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> &callback)
+void VCAI::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, int queryID)
 {
 	NET_EVENT_HANDLER;
 	LOG_ENTRY;
-	status.addQuery();
+	status.addQuery(queryID, boost::str(boost::format("Hero %s got level %d") % hero->name % hero->level));
-	requestActionASAP(boost::bind(callback, 0));
+	requestActionASAP([=]{ answerQuery(queryID, 0); });
 }
 
-void VCAI::commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, boost::function<void(ui32)> &callback)
+void VCAI::commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, int queryID)
 {
 	NET_EVENT_HANDLER;
 	LOG_ENTRY;
-	status.addQuery();
+	status.addQuery(queryID, boost::str(boost::format("Commander %s of %s got level %d") % commander->name % commander->armyObj->nodeName() % (int)commander->level));
-	requestActionASAP(boost::bind(callback, 0));
+	requestActionASAP([=]{ answerQuery(queryID, 0); });
 }
 
 void VCAI::showBlockingDialog(const std::string &text, const std::vector<Component> &components, ui32 askID, const int soundID, bool selection, bool cancel)
@@ -850,7 +850,8 @@ void VCAI::showBlockingDialog(const std::string &text, const std::vector<Compone
 	NET_EVENT_HANDLER;
 	LOG_ENTRY;
 	int sel = 0;
-	status.addQuery();
+	status.addQuery(askID, boost::str(boost::format("Blocking dialog query with %d components - %s") 
+									  % components.size() % text));
 
 	if(selection) //select from multiple components -> take the last one (they're indexed [1-size])
 		sel = components.size();
@@ -860,21 +861,25 @@ void VCAI::showBlockingDialog(const std::string &text, const std::vector<Compone
 
 	requestActionASAP([=]()
 	{
-		cb->selectionMade(sel, askID);
+		answerQuery(askID, sel);
 	});
 }
 
-void VCAI::showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function<void()> &onEnd)
+void VCAI::showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID)
 {
 	NET_EVENT_HANDLER;
 	LOG_ENTRY;
-	status.addQuery();
+
+	std::string s1 = up ? up->nodeName() : "NONE";
+	std::string s2 = down ? down->nodeName() : "NONE";
+
+	status.addQuery(queryID, boost::str(boost::format("Garrison dialog with %s and %s") % s1 % s2));
 
 	//you can't request action from action-response thread
 	requestActionASAP([=]()
 	{
 		pickBestCreatures (down, up);
-		onEnd();
+		answerQuery(queryID, 0);
 	});
 }
 
@@ -2197,6 +2202,9 @@ void VCAI::finish()
 
 void VCAI::requestActionASAP(boost::function<void()> whatToDo)
 {
+// 	static boost::mutex m;
+// 	boost::unique_lock<boost::mutex> mylock(m);
+
 	boost::barrier b(2);
 	boost::thread newThread([&b,this,whatToDo]()
 	{
@@ -2221,10 +2229,32 @@ void VCAI::lostHero(HeroPtr h)
 	remove_if_present(reservedHeroesMap, h);
 }
 
+void VCAI::answerQuery(int queryID, int selection)
+{
+	BNLOG("I'll answer the query %d giving the choice %d", queryID % selection);
+	if(queryID != -1)
+	{
+		int requestID = cb->selectionMade(selection, queryID);
+	}
+	else
+	{
+		BNLOG("Since the query ID is %d, the answer won't be sent. This is not a real query!", queryID);
+		//do nothing
+	}
+}
+
+void VCAI::requestSent(const CPackForServer *pack, int requestID)
+{
+	//BNLOG("I have sent request of type %s", typeid(*pack).name());
+	if(auto reply = dynamic_cast<const QueryReply*>(pack))
+	{
+		status.attemptedAnsweringQuery(reply->qid, requestID);
+	}
+}
+
 AIStatus::AIStatus()
 {
 	battle = NO_BATTLE;
-	remainingQueries = 0;
 	havingTurn = false;
 }
 
@@ -2246,29 +2276,38 @@ BattleState AIStatus::getBattle()
 	return battle;
 }
 
-void AIStatus::addQueries(int val)
+void AIStatus::addQuery(int ID, std::string description)
 {
 	boost::unique_lock<boost::mutex> lock(mx);
-	remainingQueries += val;
+	if(ID == -1)
-	BNLOG("Changing count of queries by %d, to a total of %d", val % remainingQueries);
+	{
-	assert(remainingQueries >= 0);
+		BNLOG("The \"query\" has an id %d, it'll be ignored as non-query. Description: %s", ID % description);
+		return;
+	}
+
+	assert(!vstd::contains(remainingQueries, ID));
+	assert(ID >= 0);
+
+	remainingQueries[ID] = description;
 	cv.notify_all();
+	BNLOG("Adding query %d - %s. Total queries count: %d", ID % description % remainingQueries.size());
 }
 
-void AIStatus::addQuery()
+void AIStatus::removeQuery(int ID)
 {
-	addQueries(1);
+	boost::unique_lock<boost::mutex> lock(mx);
-}
+	assert(vstd::contains(remainingQueries, ID));
 
-void AIStatus::removeQuery()
+	std::string description = remainingQueries[ID];
-{
+	remainingQueries.erase(ID);
-	addQueries(-1);
+	cv.notify_all();
+	BNLOG("Removing query %d - %s. Total queries count: %d", ID % description % remainingQueries.size());
 }
 
 int AIStatus::getQueriesCount()
 {
 	boost::unique_lock<boost::mutex> lock(mx);
-	return remainingQueries;
+	return remainingQueries.size();
 }
 
 void AIStatus::startedTurn()
@@ -2288,7 +2327,7 @@ void AIStatus::madeTurn()
 void AIStatus::waitTillFree()
 {
 	boost::unique_lock<boost::mutex> lock(mx);
-	while(battle != NO_BATTLE || remainingQueries)
+	while(battle != NO_BATTLE || remainingQueries.size())
 		cv.wait(lock);
 }
 
@@ -2298,6 +2337,33 @@ bool AIStatus::haveTurn()
 	return havingTurn;
 }
 
+void AIStatus::attemptedAnsweringQuery(int queryID, int answerRequestID)
+{
+	boost::unique_lock<boost::mutex> lock(mx);
+	assert(vstd::contains(remainingQueries, queryID));
+	std::string description = remainingQueries[queryID];
+	BNLOG("Attempted answering query %d - %s. Request id=%d. Waiting for results...", queryID % description % answerRequestID);
+	requestToQueryID[answerRequestID] = queryID;
+}
+
+void AIStatus::receivedAnswerConfirmation(int answerRequestID, int result)
+{
+	assert(vstd::contains(requestToQueryID, answerRequestID));
+	int query = requestToQueryID[answerRequestID];
+	assert(vstd::contains(remainingQueries, query));
+	requestToQueryID.erase(answerRequestID);
+
+	if(result)
+	{
+		removeQuery(query);
+	}
+	else
+	{
+		tlog1 << "Something went really wrong, failed to answer query " << query << ": " << remainingQueries[query];
+		//TODO safely retry
+	}
+}
+
 int3 whereToExplore(HeroPtr h)
 {
 	//TODO it's stupid and ineffective, write sth better

AI/VCAI/VCAI.h

@@ -44,7 +44,9 @@ class AIStatus
 	boost::condition_variable cv;
 
 	BattleState battle;
-	int remainingQueries;
+	std::map<int, std::string> remainingQueries;
+	std::map<int, int> requestToQueryID; //IDs of answer-requests sent to server => query ids (so we can match answer confirmation from server to the query)
+
 	bool havingTurn;
 
 public:
@@ -52,14 +54,15 @@ public:
 	~AIStatus();
 	void setBattle(BattleState BS);
 	BattleState getBattle();
-	void addQueries(int val);
+	void addQuery(int ID, std::string description);
-	void addQuery();
+	void removeQuery(int ID);
-	void removeQuery();
 	int getQueriesCount();
 	void startedTurn();
 	void madeTurn();
 	void waitTillFree();
 	bool haveTurn();
+	void attemptedAnsweringQuery(int queryID, int answerRequestID);
+	void receivedAnswerConfirmation(int answerRequestID, int result);
 };
 
 enum EGoals
@@ -245,10 +248,11 @@ public:
 
 	virtual void init(CCallback * CB);
 	virtual void yourTurn();
-	virtual void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> &callback) OVERRIDE; //pskill is gained primary skill, interface has to choose one of given skills and call callback with selection id
+
-	virtual void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, boost::function<void(ui32)> &callback) OVERRIDE; //TODO
+	virtual void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, int queryID) OVERRIDE; //pskill is gained primary skill, interface has to choose one of given skills and call callback with selection id
+	virtual void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, int queryID) OVERRIDE; //TODO
 	virtual void showBlockingDialog(const std::string &text, const std::vector<Component> &components, ui32 askID, const int soundID, bool selection, bool cancel) OVERRIDE; //Show a dialog, player must take decision. If selection then he has to choose between one of given components, if cancel he is allowed to not choose. After making choice, CCallback::selectionMade should be called with number of selected component (1 - n) or 0 for cancel (if allowed) and askID.
-	virtual void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function<void()> &onEnd) OVERRIDE; //all stacks operations between these objects become allowed, interface has to call onEnd when done
+	virtual void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID) OVERRIDE; //all stacks operations between these objects become allowed, interface has to call onEnd when done
 	virtual void serialize(COSer<CSaveFile> &h, const int version) OVERRIDE; //saving
 	virtual void serialize(CISer<CLoadFile> &h, const int version) OVERRIDE; //loading
 	virtual void finish() OVERRIDE;
@@ -353,6 +357,8 @@ public:
 	TResources estimateIncome() const;
 	bool containsSavedRes(const TResources &cost) const;
 
+	void requestSent(const CPackForServer *pack, int requestID) OVERRIDE;
+	void answerQuery(int queryID, int selection);
 	//special function that can be called ONLY from game events handling thread and will send request ASAP
 	void requestActionASAP(boost::function<void()> whatToDo); 
 };

CCallback.cpp

@@ -55,12 +55,20 @@ bool CCallback::moveHero(const CGHeroInstance *h, int3 dst)
 	sendRequest(&pack);
 	return true;
 }
-void CCallback::selectionMade(int selection, int asker)
+
+int CCallback::selectionMade(int selection, int queryID)
 {
-	QueryReply pack(asker,selection);
+	if(queryID == -1)
+	{
+		tlog1 << "Cannot answer the query -1!\n";
+		return false;
+	}
+
+	QueryReply pack(queryID,selection);
 	pack.player = player;
-	sendRequest(&pack);
+	return sendRequest(&pack);
 }
+
 void CCallback::recruitCreatures(const CGObjectInstance *obj, ui32 ID, ui32 amount, si32 level/*=-1*/)
 {
 	if(player!=obj->tempOwner  &&  obj->ID != 106)
@@ -182,7 +190,7 @@ int CBattleCallback::battleMakeAction(BattleAction* action)
 	return 0;
 }
 
-void CBattleCallback::sendRequest(const CPack* request)
+int CBattleCallback::sendRequest(const CPack *request)
 {
 	int requestID = cl->sendRequest(request, player);
 	if(waitTillRealize)
@@ -191,6 +199,8 @@ void CBattleCallback::sendRequest(const CPack* request)
 		auto gsUnlocker = vstd::makeUnlockSharedGuardIf(getGsMutex(), unlockGsWhenWaiting);
 		cl->waitingRequest.waitWhileContains(requestID);
 	}
+
+	return requestID;
 }
 
 void CCallback::swapGarrisonHero( const CGTownInstance *town )

CCallback.h

@@ -56,7 +56,7 @@ public:
 
 	virtual void trade(const CGObjectInstance *market, int mode, int id1, int id2, int val1, const CGHeroInstance *hero = NULL)=0; //mode==0: sell val1 units of id1 resource for id2 resiurce
 
-	virtual void selectionMade(int selection, int asker) =0;
+	virtual int selectionMade(int selection, int queryID) =0;
 	virtual int swapCreatures(const CArmedInstance *s1, const CArmedInstance *s2, int p1, int p2)=0;//swaps creatures between two possibly different garrisons // TODO: AI-unsafe code - fix it!
 	virtual int mergeStacks(const CArmedInstance *s1, const CArmedInstance *s2, int p1, int p2)=0;//joins first stack to the second (creatures must be same type)
 	virtual int mergeOrSwapStacks(const CArmedInstance *s1, const CArmedInstance *s2, int p1, int p2) =0; //first goes to the second
@@ -83,9 +83,8 @@ class CBattleCallback : public IBattleCallback, public CBattleInfoCallback
 private:
 	CBattleCallback(CGameState *GS, int Player, CClient *C);
 
-
 protected:
-	void sendRequest(const CPack *request);
+	int sendRequest(const CPack *request); //returns requestID (that'll be matched to requestID in PackageApplied)
 	CClient *cl;
 	//virtual bool hasAccess(int playerId) const;
 
@@ -114,12 +113,13 @@ public:
 	virtual void calculatePaths(const CGHeroInstance *hero, CPathsInfo &out, int3 src = int3(-1,-1,-1), int movement = -1);
 	virtual void recalculatePaths(); //updates main, client pathfinder info (should be called when moving hero is over)
 
+
 	void unregisterMyInterface(); //stops delivering information about game events to that player's interface -> can be called ONLY after victory/loss
 
 //commands
 	bool moveHero(const CGHeroInstance *h, int3 dst); //dst must be free, neighbouring tile (this function can move hero only by one tile)
 	bool teleportHero(const CGHeroInstance *who, const CGTownInstance *where);
-	void selectionMade(int selection, int asker);
+	int selectionMade(int selection, int queryID);
 	int swapCreatures(const CArmedInstance *s1, const CArmedInstance *s2, int p1, int p2);
 	int mergeOrSwapStacks(const CArmedInstance *s1, const CArmedInstance *s2, int p1, int p2); //first goes to the second
 	int mergeStacks(const CArmedInstance *s1, const CArmedInstance *s2, int p1, int p2); //first goes to the second

client/CAdvmapInterface.cpp

@@ -983,7 +983,12 @@ void CAdvMapInt::select(const CArmedInstance *sel, bool centerView /*= true*/)
 	LOCPLINT->cb->setSelection(sel);
 	selection = sel;
 	if (LOCPLINT->battleInt == NULL && active & GENERAL)
-		CCS->musich->playMusic(CCS->musich->terrainMusics[LOCPLINT->cb->getTile(sel->visitablePos())->tertype], -1);
+	{
+		auto pos = sel->visitablePos();
+		auto tile = LOCPLINT->cb->getTile(pos);
+		if(tile)
+			CCS->musich->playMusic(CCS->musich->terrainMusics[tile->tertype], -1);
+	}
 	if(centerView)
 		centerOn(sel);
 

client/CCreatureWindow.cpp

@@ -147,7 +147,7 @@ CCreatureWindow::CCreatureWindow (const CCommanderInstance * Commander):
 	dismiss = new CAdventureMapButton("",CGI->generaltexth->zelp[445].second, cfl, 333, 148,"IVIEWCR2.DEF", SDLK_d);
 }
 
-CCreatureWindow::CCreatureWindow (std::vector<ui32> &skills, const CCommanderInstance * Commander, boost::function<void(ui32)> &callback):
+CCreatureWindow::CCreatureWindow (std::vector<ui32> &skills, const CCommanderInstance * Commander, boost::function<void(ui32)> callback):
     CWindowObject(PLAYER_COLORED),
     type(COMMANDER_LEVEL_UP),
 	commander (Commander),

client/CCreatureWindow.h

@@ -90,7 +90,7 @@ public:
 	CCreatureWindow (const CStackInstance &stack, int Type); //pop-up c-tor
 	CCreatureWindow(const CStackInstance &st, int Type, boost::function<void()> Upg, boost::function<void()> Dsm, UpgradeInfo *ui); //full garrison window
 	CCreatureWindow(const CCommanderInstance * commander); //commander window
-	CCreatureWindow(std::vector<ui32> &skills, const CCommanderInstance * commander, boost::function<void(ui32)> &callback); 
+	CCreatureWindow(std::vector<ui32> &skills, const CCommanderInstance * commander, boost::function<void(ui32)> callback); 
 	CCreatureWindow(int Cid, int Type, int creatureCount); //c-tor
 
 	void init(const CStackInstance *stack, const CBonusSystemNode *stackNode, const CGHeroInstance *heroOwner);

client/CPlayerInterface.cpp

@@ -470,22 +470,24 @@ void CPlayerInterface::receivedResource(int type, int val)
 	GH.totalRedraw();
 }
 
-void CPlayerInterface::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16>& skills, boost::function<void(ui32)> &callback)
+void CPlayerInterface::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16>& skills, int queryID)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 	waitWhileDialog();
 	CCS->soundh->playSound(soundBase::heroNewLevel);
 
-	CLevelWindow *lw = new CLevelWindow(hero,pskill,skills,callback);
+	CLevelWindow *lw = new CLevelWindow(hero,pskill,skills,
+										[=](ui32 selection){ cb->selectionMade(selection, queryID); });
 	GH.pushInt(lw);
 }
-void CPlayerInterface::commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, boost::function<void(ui32)> &callback)
+void CPlayerInterface::commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, int queryID)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
 	waitWhileDialog();
 	CCS->soundh->playSound(soundBase::heroNewLevel);
 
-	CCreatureWindow * cw = new CCreatureWindow(skills, commander, callback);
+	CCreatureWindow * cw = new CCreatureWindow(skills, commander, 
+												[=](ui32 selection){ cb->selectionMade(selection, queryID); });
 	GH.pushInt(cw);
 }
 void CPlayerInterface::heroInGarrisonChange(const CGTownInstance *town)
@@ -1279,9 +1281,10 @@ bool CPlayerInterface::altPressed() const
 	return SDL_GetKeyState(NULL)[SDLK_LALT]  ||  SDL_GetKeyState(NULL)[SDLK_RALT];
 }
 
-void CPlayerInterface::showGarrisonDialog( const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function<void()> &onEnd )
+void CPlayerInterface::showGarrisonDialog( const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
+	auto onEnd = [=]{ cb->selectionMade(0, queryID); };
 
 	if(stillMoveHero.get() == DURING_MOVE  && adventureInt->terrain.currentPath && adventureInt->terrain.currentPath->nodes.size() > 1) //to ignore calls on passing through garrisons
 	{

client/CPlayerInterface.h

@@ -140,8 +140,8 @@ public:
 	void artifactDisassembled(const ArtifactLocation &al);
 
 	void heroCreated(const CGHeroInstance* hero) OVERRIDE;
-	void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> &callback) OVERRIDE;
+	void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, int queryID) OVERRIDE;
-	void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, boost::function<void(ui32)> &callback) OVERRIDE;
+	void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, int queryID) OVERRIDE;
 	void heroInGarrisonChange(const CGTownInstance *town) OVERRIDE;
 	void heroMoved(const TryMoveHero & details) OVERRIDE;
 	void heroPrimarySkillChanged(const CGHeroInstance * hero, int which, si64 val) OVERRIDE;
@@ -154,7 +154,7 @@ public:
 	void showRecruitmentDialog(const CGDwelling *dwelling, const CArmedInstance *dst, int level) OVERRIDE;
 	void showShipyardDialog(const IShipyard *obj) OVERRIDE; //obj may be town or shipyard; 
 	void showBlockingDialog(const std::string &text, const std::vector<Component> &components, ui32 askID, int soundID, bool selection, bool cancel) OVERRIDE; //Show a dialog, player must take decision. If selection then he has to choose between one of given components, if cancel he is allowed to not choose. After making choice, CCallback::selectionMade should be called with number of selected component (1 - n) or 0 for cancel (if allowed) and askID.
-	void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function<void()> &onEnd) OVERRIDE;
+	void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID) OVERRIDE;
 	void showPuzzleMap() OVERRIDE;
 	void showMarketWindow(const IMarket *market, const CGHeroInstance *visitor) OVERRIDE;
 	void showUniversityWindow(const IMarket *market, const CGHeroInstance *visitor) OVERRIDE;

client/Client.cpp

@@ -642,6 +642,9 @@ int CClient::sendRequest(const CPack *request, int player)
 
 	waitingRequest.pushBack(requestID);
 	serv->sendPackToServer(*request, player, requestID);
+	if(vstd::contains(playerint, player))
+		playerint[player]->requestSent(dynamic_cast<const CPackForServer*>(request), requestID);
+
 	return requestID;
 }
 

client/GUIClasses.cpp

@@ -1619,7 +1619,7 @@ void CSplitWindow::sliderMoved(int to)
 	setAmount(rightMin + to, false);
 }
 
-CLevelWindow::CLevelWindow(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> &callback):
+CLevelWindow::CLevelWindow(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> callback):
     CWindowObject(PLAYER_COLORED, "LVLUPBKG"),
     cb(callback)
 {

client/GUIClasses.h

@@ -481,7 +481,7 @@ class CLevelWindow : public CWindowObject
 	void selectionChanged(unsigned to);
 public:
 
-	CLevelWindow(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> &callback); //c-tor
+	CLevelWindow(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> callback); //c-tor
 	~CLevelWindow(); //d-tor
 
 };

client/NetPacksClient.cpp

@@ -528,13 +528,14 @@ void SetObjectProperty::applyCl( CClient *cl )
 
 void HeroLevelUp::applyCl( CClient *cl )
 {
-	CGHeroInstance *h = GS(cl)->getHero(heroid);
+	const CGHeroInstance *h = cl->getHero(heroid);
+	//INTERFACE_CALL_IF_PRESENT(h->tempOwner, heroGotLevel, h, primskill, skills, id);
 	if(vstd::contains(cl->playerint,h->tempOwner))
 	{
-		boost::function<void(ui32)> callback = boost::function<void(ui32)>(boost::bind(&CCallback::selectionMade,cl->callbacks[h->tempOwner].get(),_1,id));
+		cl->playerint[h->tempOwner]->heroGotLevel(h, static_cast<int>(primskill), skills, queryID);
-		cl->playerint[h->tempOwner]->heroGotLevel(const_cast<const CGHeroInstance*>(h),static_cast<int>(primskill),skills, callback);
 	}
 }
+
 void CommanderLevelUp::applyCl( CClient *cl )
 {
 	CCommanderInstance * commander = GS(cl)->getHero(heroid)->commander;
@@ -542,8 +543,7 @@ void CommanderLevelUp::applyCl( CClient *cl )
 	ui8 player = commander->armyObj->tempOwner;
 	if (commander->armyObj && vstd::contains(cl->playerint, player)) //is it possible for Commander to exist beyond armed instance?
 	{
-		auto callback = boost::function<void(ui32)>(boost::bind(&CCallback::selectionMade,cl->callbacks[player].get(),_1,id));
+		cl->playerint[player]->commanderGotLevel(commander, skills, queryID);
-		cl->playerint[player]->commanderGotLevel(commander, skills, callback);
 	}
 }
 
@@ -553,7 +553,7 @@ void BlockingDialog::applyCl( CClient *cl )
 	text.toString(str);
 
 	if(vstd::contains(cl->playerint,player))
-		cl->playerint[player]->showBlockingDialog(str,components,id,(soundBase::soundID)soundID,selection(),cancel());
+		cl->playerint[player]->showBlockingDialog(str,components,queryID,(soundBase::soundID)soundID,selection(),cancel());
 	else
 		tlog2 << "We received YesNoDialog for not our player...\n";
 }
@@ -566,8 +566,7 @@ void GarrisonDialog::applyCl(CClient *cl)
 	if(!vstd::contains(cl->playerint,h->getOwner()))
 		return;
 
-	boost::function<void()> callback = boost::bind(&CCallback::selectionMade,cl->callbacks[h->getOwner()].get(),0,id);
+	cl->playerint[h->getOwner()]->showGarrisonDialog(obj,h,removableUnits,queryID);
-	cl->playerint[h->getOwner()]->showGarrisonDialog(obj,h,removableUnits,callback);
 }
 
 void BattleStart::applyCl( CClient *cl )

lib/CGameInterface.h

@@ -76,8 +76,8 @@ public:
 	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
-	virtual void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, boost::function<void(ui32)> &callback)=0;
+	virtual void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector<ui16> &skills, int queryID)=0;
-	virtual	void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, boost::function<void(ui32)> &callback)=0;
+	virtual	void commanderGotLevel (const CCommanderInstance * commander, std::vector<ui32> skills, int queryID)=0;
 
 	// Show a dialog, player must take decision. If selection then he has to choose between one of given components,
 	// if cancel he is allowed to not choose. After making choice, CCallback::selectionMade should be called
@@ -85,7 +85,7 @@ public:
 	virtual void showBlockingDialog(const std::string &text, const std::vector<Component> &components, ui32 askID, const int soundID, bool selection, bool cancel) = 0;
 
 	// all stacks operations between these objects become allowed, interface has to call onEnd when done
-	virtual void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function<void()> &onEnd) = 0;
+	virtual void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID) = 0;
 	virtual void serialize(COSer<CSaveFile> &h, const int version){}; //saving
 	virtual void serialize(CISer<CLoadFile> &h, const int version){}; //loading
 	virtual void finish(){}; //if for some reason we want to end

lib/IGameEventsReceiver.h

@@ -35,6 +35,7 @@ struct SetStackEffect;
 struct BattleTriggerEffect;
 class CComponent;
 struct CObstacleInstance;
+struct CPackForServer;
 
 class DLL_LINKAGE IBattleEventsReceiver
 {
@@ -112,6 +113,7 @@ public:
 	virtual void availableCreaturesChanged(const CGDwelling *town){};
 	virtual void heroBonusChanged(const CGHeroInstance *hero, const Bonus &bonus, bool gain){};//if gain hero received bonus, else he lost it
 	virtual void playerBonusChanged(const Bonus &bonus, bool gain){};//if gain hero received bonus, else he lost it
+	virtual void requestSent(const CPackForServer *pack, int requestID){};
 	virtual void requestRealized(PackageApplied *pa){};
 	virtual void heroExchangeStarted(si32 hero1, si32 hero2){};
 	virtual void objectPropertyChanged(const SetObjectProperty * sop){}; //eg. mine has been flagged

lib/NetPacks.h

@@ -81,7 +81,12 @@ struct CPackForServer : public CPack
 
 struct Query : public CPackForClient
 {
-	ui32 id;
+	ui32 queryID; // equals to -1 if it is not an actual query (and should not be answered)
+	
+	Query()
+	{
+		queryID = -1;
+	}
 };
 
 
@@ -201,6 +206,7 @@ struct PackageApplied : public CPackForClient //94
 	ui32 requestID; //an ID given by client to the request that was applied
 	ui8 player;
 
+
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & result & packType & requestID & player;
@@ -1156,7 +1162,7 @@ struct HeroLevelUp : public Query//2000
 	
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & id & heroid & primskill & level & skills;
+		h & queryID & heroid & primskill & level & skills;
 	}
 };
 
@@ -1174,7 +1180,7 @@ struct CommanderLevelUp : public Query
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & id & heroid & sl & skills;
+		h & queryID & heroid & sl & skills;
 	}
 };
 
@@ -1235,7 +1241,7 @@ struct BlockingDialog : public Query//2003
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & id & text & components & player & flags & soundID;
+		h & queryID & text & components & player & flags & soundID;
 	}
 };
 
@@ -1248,7 +1254,7 @@ struct GarrisonDialog : public Query//2004
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & id & objid & hid & removableUnits;
+		h & queryID & objid & hid & removableUnits;
 	}
 };
 

server/CGameHandler.cpp

@@ -176,6 +176,7 @@ void PlayerStatuses::removeQuery(ui8 player, ui32 id)
 	boost::unique_lock<boost::mutex> l(mx);
 	if(players.find(player) != players.end())
 	{
+		assert(vstd::contains(players[player].queries, id));
 		players[player].queries.erase(id);
 	}
 	else
@@ -2164,24 +2165,28 @@ void CGameHandler::heroExchange(si32 hero1, si32 hero2)
 	}
 }
 
+void CGameHandler::prepareNewQuery(Query * queryPack, ui8 player, const boost::function<void(ui32)> &callback)
+{
+	boost::unique_lock<boost::recursive_mutex> lock(gsm);
+	tlog4 << "Creating a query for player " << (int)player << " with ID=" << QID << std::endl;
+	callbacks[QID] = callback;
+	states.addQuery(player, QID);
+	queryPack->queryID = QID;
+	QID++;
+}
+
 void CGameHandler::applyAndAsk( Query * sel, ui8 player, boost::function<void(ui32)> &callback )
 {
 	boost::unique_lock<boost::recursive_mutex> lock(gsm);
-	sel->id = QID;
+	prepareNewQuery(sel, player, callback);
-	callbacks[QID] = callback;
-	states.addQuery(player,QID);
-	QID++; 
 	sendAndApply(sel);
 }
 
 void CGameHandler::ask( Query * sel, ui8 player, const CFunctionList<void(ui32)> &callback )
 {
 	boost::unique_lock<boost::recursive_mutex> lock(gsm);
-	sel->id = QID;
+	prepareNewQuery(sel, player, callback);
-	callbacks[QID] = callback;
-	states.addQuery(player,QID);
 	sendToAllClients(sel);
-	QID++; 
 }
 
 void CGameHandler::sendToAllClients( CPackForClient * info )
@@ -3207,16 +3212,9 @@ bool CGameHandler::queryReply(ui32 qid, ui32 answer, ui8 player)
 		if(callb)
 			callb(answer);
 	}
-	else if(vstd::contains(garrisonCallbacks,qid))
-	{
-		if(garrisonCallbacks[qid])
-			garrisonCallbacks[qid]();
-		garrisonCallbacks.erase(qid);
-		allowedExchanges.erase(qid);
-	}
 	else
 	{
-		tlog1 << "Unknown query reply...\n";
+		complain("Unknown query reply!");
 		return false;
 	}
 	return true;
@@ -4752,15 +4750,22 @@ void CGameHandler::showGarrisonDialog( int upobj, int hid, bool removableUnits,
 	GarrisonDialog gd;
 	gd.hid = hid;
 	gd.objid = upobj;
+	gd.removableUnits = removableUnits;
 
 	{
 		boost::unique_lock<boost::recursive_mutex> lock(gsm);
-		gd.id = QID;
+		prepareNewQuery(&gd, player);
-		garrisonCallbacks[QID] = cb;
+
-		allowedExchanges[QID] = std::pair<si32,si32>(upobj,hid);
+		//register callback manually since we need to use query ID that's given in result of prepareNewQuery call
-		states.addQuery(player,QID);
+		callbacks[gd.queryID] = [=](ui32 answer)
-		QID++; 
+		{ 
-		gd.removableUnits = removableUnits;
+			// Garrison callback calls the "original callback" and closes the exchange between objs.
+			cb();
+			boost::unique_lock<boost::recursive_mutex> lockGsm(this->gsm);
+			allowedExchanges.erase(gd.queryID);
+		};
+
+		allowedExchanges[gd.queryID] = std::pair<si32,si32>(upobj,hid);
 		sendAndApply(&gd);
 	}
 }

server/CGameHandler.h

@@ -94,8 +94,9 @@ public:
 	//queries stuff
 	boost::recursive_mutex gsm;
 	ui32 QID;
+
+	//TODO get rid of cfunctionlist (or similar) and use serialziable callback structure
 	std::map<ui32, CFunctionList<void(ui32)> > callbacks; //query id => callback function - for selection and yes/no dialogs
-	std::map<ui32, boost::function<void()> > garrisonCallbacks; //query id => callback - for garrison dialogs
 	std::map<ui32, std::pair<si32,si32> > allowedExchanges;
 
 	bool isBlockedByQueries(const CPack *pack, int packType, ui8 player); 
@@ -243,6 +244,7 @@ public:
 	void sendMessageToAll(const std::string &message);
 	void sendMessageTo(CConnection &c, const std::string &message);
 	void applyAndAsk(Query * sel, ui8 player, boost::function<void(ui32)> &callback);
+	void prepareNewQuery(Query * queryPack, ui8 player, const boost::function<void(ui32)> &callback = 0); //generates unique query id and writes it to the pack; blocks the player till query is answered (then callback is called)
 	void ask(Query * sel, ui8 player, const CFunctionList<void(ui32)> &callback);
 	void sendToAllClients(CPackForClient * info);
 	void sendAndApply(CPackForClient * info);

server/NetPacksServer.cpp

@@ -226,7 +226,14 @@ bool BuildBoat::applyGh( CGameHandler *gh )
 
 bool QueryReply::applyGh( CGameHandler *gh )
 {
-	ERROR_IF_NOT(player);
+	auto playerToConnection = gh->connections.find(player);
+	if(playerToConnection == gh->connections.end())
+		COMPLAIN_AND_RETURN("No such player!");
+	if(playerToConnection->second != c)
+		COMPLAIN_AND_RETURN("Message came from wrong connection!");
+	if(qid == -1)
+		COMPLAIN_AND_RETURN("Cannot answer the query with id -1!");
+
 	assert(vstd::contains(gh->states.players, player));
 	return gh->queryReply(qid, answer, player);
 }