From edccbd480918e7ed28202454764c5e1a613087ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Sun, 15 Jul 2012 15:34:00 +0000 Subject: [PATCH] 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 | 13 ++-- AI/EmptyAI/CEmptyAI.h | 6 +- AI/VCAI/VCAI.cpp | 116 ++++++++++++++++++++++++++++-------- AI/VCAI/VCAI.h | 20 ++++--- CCallback.cpp | 20 +++++-- CCallback.h | 10 ++-- client/CAdvmapInterface.cpp | 7 ++- client/CCreatureWindow.cpp | 2 +- client/CCreatureWindow.h | 2 +- client/CPlayerInterface.cpp | 13 ++-- client/CPlayerInterface.h | 6 +- client/Client.cpp | 3 + client/GUIClasses.cpp | 2 +- client/GUIClasses.h | 2 +- client/NetPacksClient.cpp | 15 +++-- lib/CGameInterface.h | 6 +- lib/IGameEventsReceiver.h | 2 + lib/NetPacks.h | 16 +++-- server/CGameHandler.cpp | 49 ++++++++------- server/CGameHandler.h | 4 +- server/NetPacksServer.cpp | 9 ++- 21 files changed, 219 insertions(+), 104 deletions(-) diff --git a/AI/EmptyAI/CEmptyAI.cpp b/AI/EmptyAI/CEmptyAI.cpp index 390b50d8e..137156b58 100644 --- a/AI/EmptyAI/CEmptyAI.cpp +++ b/AI/EmptyAI/CEmptyAI.cpp @@ -12,14 +12,15 @@ void CEmptyAI::yourTurn() { cb->endTurn(); } -void CEmptyAI::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector &skills, boost::function &callback) + +void CEmptyAI::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector &skills, int queryID) { - callback(rand()%skills.size()); + cb->selectionMade(rand() % skills.size(), queryID); } -void CEmptyAI::commanderGotLevel(const CCommanderInstance * commander, std::vector skills, boost::function &callback) +void CEmptyAI::commanderGotLevel(const CCommanderInstance * commander, std::vector skills, int queryID) { - callback(0); + cb->selectionMade(rand() % skills.size(), queryID); } void CEmptyAI::showBlockingDialog(const std::string &text, const std::vector &components, ui32 askID, const int soundID, bool selection, bool cancel) @@ -27,7 +28,7 @@ void CEmptyAI::showBlockingDialog(const std::string &text, const std::vectorselectionMade(0, askID); } -void CEmptyAI::showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function &onEnd) +void CEmptyAI::showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID) { - onEnd(); + cb->selectionMade(0, queryID); } \ No newline at end of file diff --git a/AI/EmptyAI/CEmptyAI.h b/AI/EmptyAI/CEmptyAI.h index 2e830ee16..15e7106c4 100644 --- a/AI/EmptyAI/CEmptyAI.h +++ b/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 &skills, boost::function &callback) override; - void commanderGotLevel (const CCommanderInstance * commander, std::vector skills, boost::function &callback) override; + void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector &skills, int queryID) override; + void commanderGotLevel (const CCommanderInstance * commander, std::vector skills, int queryID) override; void showBlockingDialog(const std::string &text, const std::vector &components, ui32 askID, const int soundID, bool selection, bool cancel) override; - void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function &onEnd) override; + void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID) override; }; #define NAME "EmptyAI 0.1" diff --git a/AI/VCAI/VCAI.cpp b/AI/VCAI/VCAI.cpp index e5456b161..58d5ee9e2 100644 --- a/AI/VCAI/VCAI.cpp +++ b/AI/VCAI/VCAI.cpp @@ -733,7 +733,7 @@ void VCAI::requestRealized(PackageApplied *pa) if(pa->packType == typeList.getTypeID()) { - 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 &skills, boost::function &callback) +void VCAI::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector &skills, int queryID) { NET_EVENT_HANDLER; LOG_ENTRY; - status.addQuery(); - requestActionASAP(boost::bind(callback, 0)); + status.addQuery(queryID, boost::str(boost::format("Hero %s got level %d") % hero->name % hero->level)); + requestActionASAP([=]{ answerQuery(queryID, 0); }); } -void VCAI::commanderGotLevel (const CCommanderInstance * commander, std::vector skills, boost::function &callback) +void VCAI::commanderGotLevel (const CCommanderInstance * commander, std::vector skills, int queryID) { NET_EVENT_HANDLER; LOG_ENTRY; - status.addQuery(); - requestActionASAP(boost::bind(callback, 0)); + status.addQuery(queryID, boost::str(boost::format("Commander %s of %s got level %d") % commander->name % commander->armyObj->nodeName() % (int)commander->level)); + requestActionASAP([=]{ answerQuery(queryID, 0); }); } void VCAI::showBlockingDialog(const std::string &text, const std::vector &components, ui32 askID, const int soundID, bool selection, bool cancel) @@ -850,7 +850,8 @@ void VCAI::showBlockingDialog(const std::string &text, const std::vector 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::vectorselectionMade(sel, askID); + answerQuery(askID, sel); }); } -void VCAI::showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function &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 whatToDo) { +// static boost::mutex m; +// boost::unique_lock 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(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 lock(mx); - remainingQueries += val; - BNLOG("Changing count of queries by %d, to a total of %d", val % remainingQueries); - assert(remainingQueries >= 0); + if(ID == -1) + { + 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 lock(mx); + assert(vstd::contains(remainingQueries, ID)); -void AIStatus::removeQuery() -{ - addQueries(-1); + std::string description = remainingQueries[ID]; + remainingQueries.erase(ID); + cv.notify_all(); + BNLOG("Removing query %d - %s. Total queries count: %d", ID % description % remainingQueries.size()); } int AIStatus::getQueriesCount() { boost::unique_lock lock(mx); - return remainingQueries; + return remainingQueries.size(); } void AIStatus::startedTurn() @@ -2288,7 +2327,7 @@ void AIStatus::madeTurn() void AIStatus::waitTillFree() { boost::unique_lock 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 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 diff --git a/AI/VCAI/VCAI.h b/AI/VCAI/VCAI.h index 3a429a9b4..1375fb975 100644 --- a/AI/VCAI/VCAI.h +++ b/AI/VCAI/VCAI.h @@ -44,7 +44,9 @@ class AIStatus boost::condition_variable cv; BattleState battle; - int remainingQueries; + std::map remainingQueries; + std::map 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(); - void removeQuery(); + void addQuery(int ID, std::string description); + void removeQuery(int ID); 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 &skills, boost::function &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 skills, boost::function &callback) OVERRIDE; //TODO + + virtual void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector &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 skills, int queryID) OVERRIDE; //TODO virtual void showBlockingDialog(const std::string &text, const std::vector &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 &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 &h, const int version) OVERRIDE; //saving virtual void serialize(CISer &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 whatToDo); }; diff --git a/CCallback.cpp b/CCallback.cpp index eddbd140c..3a4a6e964 100644 --- a/CCallback.cpp +++ b/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 ) @@ -378,4 +388,4 @@ bool CBattleCallback::battleMakeTacticAction( BattleAction * action ) ma.ba = *action; sendRequest(&ma); return true; -} +} \ No newline at end of file diff --git a/CCallback.h b/CCallback.h index 2bf2c8b96..662154bcd 100644 --- a/CCallback.h +++ b/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,16 +83,15 @@ 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; public: int battleMakeAction(BattleAction* action) OVERRIDE;//for casting spells by hero - DO NOT use it for moving active stack bool battleMakeTacticAction(BattleAction * action) OVERRIDE; // performs tactic phase actions - + friend class CCallback; friend class CClient; }; @@ -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 diff --git a/client/CAdvmapInterface.cpp b/client/CAdvmapInterface.cpp index 72f9c7ea6..76b553e88 100644 --- a/client/CAdvmapInterface.cpp +++ b/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); diff --git a/client/CCreatureWindow.cpp b/client/CCreatureWindow.cpp index 8879d4c6f..ffa646b3b 100644 --- a/client/CCreatureWindow.cpp +++ b/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 &skills, const CCommanderInstance * Commander, boost::function &callback): +CCreatureWindow::CCreatureWindow (std::vector &skills, const CCommanderInstance * Commander, boost::function callback): CWindowObject(PLAYER_COLORED), type(COMMANDER_LEVEL_UP), commander (Commander), diff --git a/client/CCreatureWindow.h b/client/CCreatureWindow.h index ec7136e4b..b81c16706 100644 --- a/client/CCreatureWindow.h +++ b/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 Upg, boost::function Dsm, UpgradeInfo *ui); //full garrison window CCreatureWindow(const CCommanderInstance * commander); //commander window - CCreatureWindow(std::vector &skills, const CCommanderInstance * commander, boost::function &callback); + CCreatureWindow(std::vector &skills, const CCommanderInstance * commander, boost::function callback); CCreatureWindow(int Cid, int Type, int creatureCount); //c-tor void init(const CStackInstance *stack, const CBonusSystemNode *stackNode, const CGHeroInstance *heroOwner); diff --git a/client/CPlayerInterface.cpp b/client/CPlayerInterface.cpp index da5298ecc..7345b2954 100644 --- a/client/CPlayerInterface.cpp +++ b/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& skills, boost::function &callback) +void CPlayerInterface::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector& 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 skills, boost::function &callback) +void CPlayerInterface::commanderGotLevel (const CCommanderInstance * commander, std::vector 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 &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 { diff --git a/client/CPlayerInterface.h b/client/CPlayerInterface.h index 134dfe981..5dbfbb73b 100644 --- a/client/CPlayerInterface.h +++ b/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 &skills, boost::function &callback) OVERRIDE; - void commanderGotLevel (const CCommanderInstance * commander, std::vector skills, boost::function &callback) OVERRIDE; + void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector &skills, int queryID) OVERRIDE; + void commanderGotLevel (const CCommanderInstance * commander, std::vector 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 &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 &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; diff --git a/client/Client.cpp b/client/Client.cpp index d2b1a8a4f..d88fea1bf 100644 --- a/client/Client.cpp +++ b/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(request), requestID); + return requestID; } diff --git a/client/GUIClasses.cpp b/client/GUIClasses.cpp index b0ba17b3c..f1ab4eecb 100644 --- a/client/GUIClasses.cpp +++ b/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 &skills, boost::function &callback): +CLevelWindow::CLevelWindow(const CGHeroInstance *hero, int pskill, std::vector &skills, boost::function callback): CWindowObject(PLAYER_COLORED, "LVLUPBKG"), cb(callback) { diff --git a/client/GUIClasses.h b/client/GUIClasses.h index 520ff49de..e71b49178 100644 --- a/client/GUIClasses.h +++ b/client/GUIClasses.h @@ -481,7 +481,7 @@ class CLevelWindow : public CWindowObject void selectionChanged(unsigned to); public: - CLevelWindow(const CGHeroInstance *hero, int pskill, std::vector &skills, boost::function &callback); //c-tor + CLevelWindow(const CGHeroInstance *hero, int pskill, std::vector &skills, boost::function callback); //c-tor ~CLevelWindow(); //d-tor }; diff --git a/client/NetPacksClient.cpp b/client/NetPacksClient.cpp index f4d9cfef6..e95b24667 100644 --- a/client/NetPacksClient.cpp +++ b/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 callback = boost::function(boost::bind(&CCallback::selectionMade,cl->callbacks[h->tempOwner].get(),_1,id)); - cl->playerint[h->tempOwner]->heroGotLevel(const_cast(h),static_cast(primskill),skills, callback); + cl->playerint[h->tempOwner]->heroGotLevel(h, static_cast(primskill), skills, queryID); } } + 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(boost::bind(&CCallback::selectionMade,cl->callbacks[player].get(),_1,id)); - cl->playerint[player]->commanderGotLevel(commander, skills, callback); + cl->playerint[player]->commanderGotLevel(commander, skills, queryID); } } @@ -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 callback = boost::bind(&CCallback::selectionMade,cl->callbacks[h->getOwner()].get(),0,id); - cl->playerint[h->getOwner()]->showGarrisonDialog(obj,h,removableUnits,callback); + cl->playerint[h->getOwner()]->showGarrisonDialog(obj,h,removableUnits,queryID); } void BattleStart::applyCl( CClient *cl ) diff --git a/lib/CGameInterface.h b/lib/CGameInterface.h index c24129843..021374193 100644 --- a/lib/CGameInterface.h +++ b/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 &skills, boost::function &callback)=0; - virtual void commanderGotLevel (const CCommanderInstance * commander, std::vector skills, boost::function &callback)=0; + virtual void heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector &skills, int queryID)=0; + virtual void commanderGotLevel (const CCommanderInstance * commander, std::vector 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 &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 &onEnd) = 0; + virtual void showGarrisonDialog(const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, int queryID) = 0; virtual void serialize(COSer &h, const int version){}; //saving virtual void serialize(CISer &h, const int version){}; //loading virtual void finish(){}; //if for some reason we want to end diff --git a/lib/IGameEventsReceiver.h b/lib/IGameEventsReceiver.h index fee96ec58..6fa19fec6 100644 --- a/lib/IGameEventsReceiver.h +++ b/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 diff --git a/lib/NetPacks.h b/lib/NetPacks.h index 2dd36658a..3bff5e90f 100644 --- a/lib/NetPacks.h +++ b/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 void serialize(Handler &h, const int version) { h & result & packType & requestID & player; @@ -1156,7 +1162,7 @@ struct HeroLevelUp : public Query//2000 template 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 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 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 void serialize(Handler &h, const int version) { - h & id & objid & hid & removableUnits; + h & queryID & objid & hid & removableUnits; } }; diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 7201331c2..7f6f0302a 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -176,6 +176,7 @@ void PlayerStatuses::removeQuery(ui8 player, ui32 id) boost::unique_lock 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 &callback) +{ + boost::unique_lock 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 &callback ) { boost::unique_lock lock(gsm); - sel->id = QID; - callbacks[QID] = callback; - states.addQuery(player,QID); - QID++; + prepareNewQuery(sel, player, callback); sendAndApply(sel); } void CGameHandler::ask( Query * sel, ui8 player, const CFunctionList &callback ) { boost::unique_lock lock(gsm); - sel->id = QID; - callbacks[QID] = callback; - states.addQuery(player,QID); + prepareNewQuery(sel, player, callback); 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 lock(gsm); - gd.id = QID; - garrisonCallbacks[QID] = cb; - allowedExchanges[QID] = std::pair(upobj,hid); - states.addQuery(player,QID); - QID++; - gd.removableUnits = removableUnits; + prepareNewQuery(&gd, player); + + //register callback manually since we need to use query ID that's given in result of prepareNewQuery call + callbacks[gd.queryID] = [=](ui32 answer) + { + // Garrison callback calls the "original callback" and closes the exchange between objs. + cb(); + boost::unique_lock lockGsm(this->gsm); + allowedExchanges.erase(gd.queryID); + }; + + allowedExchanges[gd.queryID] = std::pair(upobj,hid); sendAndApply(&gd); } } diff --git a/server/CGameHandler.h b/server/CGameHandler.h index 2ce922825..c08dba2b2 100644 --- a/server/CGameHandler.h +++ b/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 > callbacks; //query id => callback function - for selection and yes/no dialogs - std::map > garrisonCallbacks; //query id => callback - for garrison dialogs std::map > 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 &callback); + void prepareNewQuery(Query * queryPack, ui8 player, const boost::function &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 &callback); void sendToAllClients(CPackForClient * info); void sendAndApply(CPackForClient * info); diff --git a/server/NetPacksServer.cpp b/server/NetPacksServer.cpp index 94e3e70b5..103b7ac9b 100644 --- a/server/NetPacksServer.cpp +++ b/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); }