From 7317e803db0ba111a0a5b0caa0abf7b55da9f6fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20W=2E=20Urba=C5=84czyk?= Date: Fri, 6 Apr 2012 15:02:15 +0000 Subject: [PATCH] Locking pim mutex in client pack handling method, instead of dozens playerint call-ins. GUI won't try updating in between gamestate change and call-ins about it. Should fix #912. Minor changes. --- CCallback.cpp | 5 +- client/CAdvmapInterface.cpp | 2 +- client/CAdvmapInterface.h | 2 +- client/CPlayerInterface.cpp | 218 +++++++++++++++++++----------------- client/CPlayerInterface.h | 8 +- client/Client.cpp | 1 + client/GUIClasses.cpp | 22 ++-- client/GUIClasses.h | 2 +- client/NetPacksClient.cpp | 5 +- lib/IGameEventsReceiver.h | 2 + lib/UnlockGuard.h | 24 ++++ 11 files changed, 165 insertions(+), 126 deletions(-) diff --git a/CCallback.cpp b/CCallback.cpp index d5ef7141e..e668815d8 100644 --- a/CCallback.cpp +++ b/CCallback.cpp @@ -226,10 +226,7 @@ void CBattleCallback::sendRequest(const CPack* request) if(waitTillRealize) { tlog5 << boost::format("We'll wait till request %d is answered.\n") % requestID; - unique_ptr unlocker; //optional, if flag set - if(unlockGsWhenWaiting) - unlocker = make_unique(getGsMutex()); - + auto gsUnlocker = vstd::makeUnlockSharedGuardIf(getGsMutex(), unlockGsWhenWaiting); cl->waitingRequest.waitWhileContains(requestID); } } diff --git a/client/CAdvmapInterface.cpp b/client/CAdvmapInterface.cpp index 2a5a6ed09..59cfbbb77 100644 --- a/client/CAdvmapInterface.cpp +++ b/client/CAdvmapInterface.cpp @@ -924,7 +924,7 @@ void CInfoBar::newDay(int Day) blitAnim(mode); } -void CInfoBar::showComp(CComponent * comp, int time) +void CInfoBar::showComp(const CComponent * comp, int time/*=5000*/) { if(comp->type != CComponent::hero) { diff --git a/client/CAdvmapInterface.h b/client/CAdvmapInterface.h index 09a3cb64d..d89b4fd15 100644 --- a/client/CAdvmapInterface.h +++ b/client/CAdvmapInterface.h @@ -159,7 +159,7 @@ public: CInfoBar(); ~CInfoBar(); void newDay(int Day); //start showing new day/week animation - void showComp(CComponent * comp, int time=5000); + void showComp(const CComponent * comp, int time=5000); void enemyTurn(ui8 color, double progress); void tick(); void showAll(SDL_Surface * to); // if specific==0 function draws info about selected hero/town diff --git a/client/CPlayerInterface.cpp b/client/CPlayerInterface.cpp index e1071a6e9..7ea62450c 100644 --- a/client/CPlayerInterface.cpp +++ b/client/CPlayerInterface.cpp @@ -56,6 +56,15 @@ * */ + +// The macro below is used to mark functions that are called by client when game state changes. +// They all assume that CPlayerInterface::pim mutex is locked. +#define EVENT_HANDLER_CALLED_BY_CLIENT + +// The macro marks functions that are run on a new thread by client. +// They do not own any mutexes intiially. +#define THREAD_CREATED_BY_CLIENT + using namespace boost::assign; using namespace CSDL_Ext; @@ -143,8 +152,8 @@ void CPlayerInterface::init(CCallback * CB) } void CPlayerInterface::yourTurn() { + EVENT_HANDLER_CALLED_BY_CLIENT; { - boost::unique_lock un(*pim); boost::unique_lock lock(eventsM); //block handling events until interface is ready LOCPLINT = this; @@ -219,10 +228,11 @@ STRONG_INLINE void delObjRect(const int & x, const int & y, const int & z, const } void CPlayerInterface::heroMoved(const TryMoveHero & details) { + EVENT_HANDLER_CALLED_BY_CLIENT; waitWhileDialog(); if(LOCPLINT != this) return; - boost::unique_lock un(*pim); + const CGHeroInstance * ho = cb->getHero(details.id); //object representing this hero int3 hp = details.start; @@ -362,7 +372,7 @@ void CPlayerInterface::heroMoved(const TryMoveHero & details) } void CPlayerInterface::heroKilled(const CGHeroInstance* hero) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; wanderingHeroes -= hero; if(vstd::contains(paths, hero)) paths.erase(hero); @@ -371,7 +381,7 @@ void CPlayerInterface::heroKilled(const CGHeroInstance* hero) } void CPlayerInterface::heroCreated(const CGHeroInstance * hero) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; wanderingHeroes.push_back(hero); adventureInt->heroList.updateHList(); } @@ -427,7 +437,7 @@ int3 CPlayerInterface::repairScreenPos(int3 pos) } void CPlayerInterface::heroPrimarySkillChanged(const CGHeroInstance * hero, int which, si64 val) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; if(which == 4) { if(CAltarWindow *ctw = dynamic_cast(GH.topInt())) @@ -439,6 +449,7 @@ void CPlayerInterface::heroPrimarySkillChanged(const CGHeroInstance * hero, int void CPlayerInterface::heroSecondarySkillChanged(const CGHeroInstance * hero, int which, int val) { + EVENT_HANDLER_CALLED_BY_CLIENT; CUniversityWindow* cuw = dynamic_cast(GH.topInt()); if(cuw) //university window is open { @@ -448,18 +459,18 @@ void CPlayerInterface::heroSecondarySkillChanged(const CGHeroInstance * hero, in void CPlayerInterface::heroManaPointsChanged(const CGHeroInstance * hero) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; updateInfo(hero); } void CPlayerInterface::heroMovePointsChanged(const CGHeroInstance * hero) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; if(makingTurn && hero->tempOwner == playerID) adventureInt->heroList.redraw(); } void CPlayerInterface::receivedResource(int type, int val) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; if(CMarketplaceWindow *mw = dynamic_cast(GH.topInt())) mw->resourceChanged(type, val); @@ -468,16 +479,16 @@ void CPlayerInterface::receivedResource(int type, int val) void CPlayerInterface::heroGotLevel(const CGHeroInstance *hero, int pskill, std::vector& skills, boost::function &callback) { + EVENT_HANDLER_CALLED_BY_CLIENT; waitWhileDialog(); CCS->soundh->playSound(soundBase::heroNewLevel); - boost::unique_lock un(*pim); CLevelWindow *lw = new CLevelWindow(hero,pskill,skills,callback); GH.pushInt(lw); } void CPlayerInterface::heroInGarrisonChange(const CGTownInstance *town) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; updateInfo(town); if(town->garrisonHero && vstd::contains(wanderingHeroes,town->garrisonHero)) //wandering hero moved to the garrison @@ -514,11 +525,11 @@ void CPlayerInterface::heroInGarrisonChange(const CGTownInstance *town) } void CPlayerInterface::heroVisitsTown(const CGHeroInstance* hero, const CGTownInstance * town) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(hero->tempOwner != playerID ) return; waitWhileDialog(); - boost::unique_lock un(*pim); openTownWindow(town); } void CPlayerInterface::garrisonChanged( const CGObjectInstance * obj, bool updateInfobox /*= true*/ ) @@ -546,7 +557,7 @@ void CPlayerInterface::garrisonChanged( const CGObjectInstance * obj, bool updat void CPlayerInterface::buildChanged(const CGTownInstance *town, int buildingID, int what) //what: 1 - built, 2 - demolished { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; switch (buildingID) { case 7: case 8: case 9: case 10: case 11: case 12: case 13: case 15: @@ -572,20 +583,20 @@ void CPlayerInterface::buildChanged(const CGTownInstance *town, int buildingID, void CPlayerInterface::battleStart(const CCreatureSet *army1, const CCreatureSet *army2, int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2, bool side) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } waitForAllDialogs(); - - boost::unique_lock un(*pim); GH.pushInt(battleInt); } void CPlayerInterface::battleStacksHealedRes(const std::vector > & healedStacks, bool lifeDrain, bool tentHeal, si32 lifeDrainFrom) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; @@ -635,16 +646,18 @@ void CPlayerInterface::battleStacksHealedRes(const std::vector un(*pim); + battleInt->newStack(stack); } void CPlayerInterface::battleObstaclesRemoved(const std::set & removedObstacles) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; @@ -667,23 +680,23 @@ void CPlayerInterface::battleObstaclesRemoved(const std::set & removedObst void CPlayerInterface::battleCatapultAttacked(const CatapultAttack & ca) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); battleInt->stackIsCatapulting(ca); } void CPlayerInterface::battleStacksRemoved(const BattleStacksRemoved & bsr) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); //fixme: this one caused deadlock for(std::set::const_iterator it = bsr.stackIDs.begin(); it != bsr.stackIDs.end(); ++it) //for each removed stack { battleInt->stackRemoved(*it); @@ -693,35 +706,35 @@ void CPlayerInterface::battleStacksRemoved(const BattleStacksRemoved & bsr) void CPlayerInterface::battleNewRound(int round) //called at the beginning of each turn, round=-1 is the tactic phase, round=0 is the first "normal" turn { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); battleInt->newRound(round); } void CPlayerInterface::actionStarted(const BattleAction* action) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); curAction = new BattleAction(*action); battleInt->startAction(action); } void CPlayerInterface::actionFinished(const BattleAction* action) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); delete curAction; curAction = NULL; battleInt->endAction(action); @@ -729,6 +742,7 @@ void CPlayerInterface::actionFinished(const BattleAction* action) BattleAction CPlayerInterface::activeStack(const CStack * stack) //called when it's turn of that stack { + THREAD_CREATED_BY_CLIENT; tlog5 << "Awaiting command for " << stack->nodeName() << std::endl; CBattleInterface *b = battleInt; @@ -759,61 +773,58 @@ BattleAction CPlayerInterface::activeStack(const CStack * stack) //called when i void CPlayerInterface::battleEnd(const BattleResult *br) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); battleInt->battleFinished(*br); } void CPlayerInterface::battleStackMoved(const CStack * stack, std::vector dest, int distance) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); battleInt->stackMoved(stack, dest, distance); } void CPlayerInterface::battleSpellCast( const BattleSpellCast *sc ) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); battleInt->spellCast(sc); } void CPlayerInterface::battleStacksEffectsSet( const SetStackEffect & sse ) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); battleInt->battleStacksEffectsSet(sse); } void CPlayerInterface::battleTriggerEffect (const BattleTriggerEffect & bte) { + EVENT_HANDLER_CALLED_BY_CLIENT; battleInt->battleTriggerEffect(bte); } void CPlayerInterface::battleStacksAttacked(const std::vector & bsa) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - - tlog5 << "CPlayerInterface::battleStackAttacked - locking..."; - boost::unique_lock un(*pim); - tlog5 << "done!\n"; - - + std::vector arg; for(std::vector::const_iterator i = bsa.begin(); i != bsa.end(); i++) { @@ -837,14 +848,12 @@ void CPlayerInterface::battleStacksAttacked(const std::vector un(*pim); - tlog5 << "done!\n"; assert(curAction); if(ba->lucky()) //lucky hit { @@ -902,22 +911,23 @@ void CPlayerInterface::battleAttack(const BattleAttack *ba) void CPlayerInterface::yourTacticPhase(int distance) { + THREAD_CREATED_BY_CLIENT; while(battleInt && battleInt->tacticsMode) boost::this_thread::sleep(boost::posix_time::millisec(1)); } -void CPlayerInterface::showComp(CComponent comp) +void CPlayerInterface::showComp(const CComponent &comp) { - waitWhileDialog();//Fix for mantis #98 - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; + waitWhileDialog(); //Fix for mantis #98 CCS->soundh->playSoundFromSet(CCS->soundh->pickupSounds); - adventureInt->infoBar.showComp(&comp,4000); } void CPlayerInterface::showInfoDialog(const std::string &text, const std::vector &components, int soundID) { + EVENT_HANDLER_CALLED_BY_CLIENT; std::vector intComps; for(int i=0;i & components, int soundID, bool delComps) { waitWhileDialog(); - boost::unique_lock un(*pim); stopMovement(); CInfoWindow *temp = CInfoWindow::create(text, playerID, &components); @@ -955,8 +964,8 @@ void CPlayerInterface::showYesNoDialog(const std::string &text, const std::vecto void CPlayerInterface::showBlockingDialog( const std::string &text, const std::vector &components, ui32 askID, int soundID, bool selection, bool cancel ) { + EVENT_HANDLER_CALLED_BY_CLIENT; waitWhileDialog(); - boost::unique_lock un(*pim); stopMovement(); CCS->soundh->playSound(static_cast(soundID)); @@ -994,7 +1003,7 @@ void CPlayerInterface::showBlockingDialog( const std::string &text, const std::v void CPlayerInterface::tileRevealed(const boost::unordered_set &pos) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; for(boost::unordered_set::const_iterator i=pos.begin(); i!=pos.end();i++) adventureInt->minimap.showTile(*i); if(pos.size()) @@ -1003,7 +1012,7 @@ void CPlayerInterface::tileRevealed(const boost::unordered_set void CPlayerInterface::tileHidden(const boost::unordered_set &pos) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; for(boost::unordered_set::const_iterator i=pos.begin(); i!=pos.end();i++) adventureInt->minimap.hideTile(*i); if(pos.size()) @@ -1058,7 +1067,7 @@ void CPlayerInterface::heroArtifactSetChanged(const CGHeroInstance*hero) void CPlayerInterface::availableCreaturesChanged( const CGDwelling *town ) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; if(const CGTownInstance * townObj = dynamic_cast(town)) { CFortScreen *fs = dynamic_cast(GH.topInt()); @@ -1082,8 +1091,10 @@ void CPlayerInterface::availableCreaturesChanged( const CGDwelling *town ) void CPlayerInterface::heroBonusChanged( const CGHeroInstance *hero, const Bonus &bonus, bool gain ) { - if(bonus.type == Bonus::NONE) return; - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; + if(bonus.type == Bonus::NONE) + return; + updateInfo(hero); if ((bonus.type == Bonus::FLYING_MOVEMENT || bonus.type == Bonus::WATER_WALKING) && !gain) { @@ -1138,11 +1149,13 @@ template void CPlayerInterface::serializeTempl( Handler &h, c void CPlayerInterface::serialize( COSer &h, const int version ) { + EVENT_HANDLER_CALLED_BY_CLIENT; serializeTempl(h,version); } void CPlayerInterface::serialize( CISer &h, const int version ) { + EVENT_HANDLER_CALLED_BY_CLIENT; serializeTempl(h,version); firstCall = -1; } @@ -1253,19 +1266,16 @@ bool CPlayerInterface::altPressed() const void CPlayerInterface::showGarrisonDialog( const CArmedInstance *up, const CGHeroInstance *down, bool removableUnits, boost::function &onEnd ) { + EVENT_HANDLER_CALLED_BY_CLIENT; + if(stillMoveHero.get() == DURING_MOVE && adventureInt->terrain.currentPath && adventureInt->terrain.currentPath->nodes.size() > 1) //to ignore calls on passing through garrisons - { //FIXME: why there is no currentPath at game start? + { onEnd(); return; } - waitWhileDialog(); - boost::unique_lock un(*pim); - while(dialogs.size()) - { - auto unlock = vstd::makeUnlockGuard(*pim); - SDL_Delay(20); - } + waitForAllDialogs(); + CGarrisonWindow *cgw = new CGarrisonWindow(up,down,removableUnits); cgw->quit->callback += onEnd; GH.pushInt(cgw); @@ -1310,20 +1320,21 @@ void CPlayerInterface::showArtifactAssemblyDialog (ui32 artifactID, ui32 assembl void CPlayerInterface::requestRealized( PackageApplied *pa ) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(stillMoveHero.get() == DURING_MOVE) stillMoveHero.setn(CONTINUE_MOVE); } void CPlayerInterface::heroExchangeStarted(si32 hero1, si32 hero2) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; GH.pushInt(new CExchangeWindow(hero1, hero2)); } void CPlayerInterface::objectPropertyChanged(const SetObjectProperty * sop) { + EVENT_HANDLER_CALLED_BY_CLIENT; //redraw minimap if owner changed - boost::unique_lock un(*pim); if(sop->what == ObjProperty::OWNER) { const CGObjectInstance * obj = cb->getObj(sop->id); @@ -1396,14 +1407,15 @@ const CGHeroInstance * CPlayerInterface::getWHero( int pos ) void CPlayerInterface::showRecruitmentDialog(const CGDwelling *dwelling, const CArmedInstance *dst, int level) { + EVENT_HANDLER_CALLED_BY_CLIENT; waitWhileDialog(); - boost::unique_lock un(*pim); CRecruitmentWindow *cr = new CRecruitmentWindow(dwelling, level, dst, boost::bind(&CCallback::recruitCreatures, cb, dwelling, _1, _2, -1)); GH.pushInt(cr); } -void CPlayerInterface::waitWhileDialog() +void CPlayerInterface::waitWhileDialog(bool unlockPim /*= true*/) { + auto unlock = vstd::makeUnlockGuardIf(*pim, unlockPim); boost::unique_lock un(showingDialog->mx); while(showingDialog->data) showingDialog->cond.wait(un); @@ -1411,7 +1423,7 @@ void CPlayerInterface::waitWhileDialog() void CPlayerInterface::showShipyardDialog(const IShipyard *obj) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; int state = obj->state(); std::vector cost; obj->getBoatCost(cost); @@ -1421,7 +1433,7 @@ void CPlayerInterface::showShipyardDialog(const IShipyard *obj) void CPlayerInterface::newObject( const CGObjectInstance * obj ) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; //we might have built a boat in shipyard in opened town screen if(obj->ID == 8 && LOCPLINT->castleInt @@ -1434,6 +1446,7 @@ void CPlayerInterface::newObject( const CGObjectInstance * obj ) void CPlayerInterface::centerView (int3 pos, int focusTime) { + EVENT_HANDLER_CALLED_BY_CLIENT; waitWhileDialog(); adventureInt->centerOn (pos); if(focusTime) @@ -1451,6 +1464,7 @@ void CPlayerInterface::centerView (int3 pos, int focusTime) void CPlayerInterface::objectRemoved( const CGObjectInstance *obj ) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(obj->ID == GameConstants::HEROI_TYPE && obj->tempOwner == playerID) { const CGHeroInstance *h = static_cast(obj); @@ -1965,6 +1979,7 @@ void CPlayerInterface::finishMovement( const TryMoveHero &details, const int3 &h void CPlayerInterface::gameOver(ui8 player, bool victory ) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) return; @@ -1993,23 +2008,23 @@ void CPlayerInterface::gameOver(ui8 player, bool victory ) else { if(!victory && cb->getPlayerStatus(playerID) == PlayerState::INGAME) //enemy has lost - { - std::string txt = CGI->generaltexth->allTexts[5]; //%s has been vanquished! - boost::algorithm::replace_first(txt, "%s", CGI->generaltexth->capColors[player]); - showInfoDialog(txt,std::vector(1, new CComponent(CComponent::flag, player, 0))); + { + std::string txt = CGI->generaltexth->allTexts[5]; //%s has been vanquished! + boost::algorithm::replace_first(txt, "%s", CGI->generaltexth->capColors[player]); + showInfoDialog(txt,std::vector(1, new CComponent(CComponent::flag, player, 0))); } } } void CPlayerInterface::playerBonusChanged( const Bonus &bonus, bool gain ) { - + EVENT_HANDLER_CALLED_BY_CLIENT; } void CPlayerInterface::showPuzzleMap() { + EVENT_HANDLER_CALLED_BY_CLIENT; waitWhileDialog(); - boost::unique_lock un(*pim); //TODO: interface should not know the real position of Grail... double ratio = 0; @@ -2020,6 +2035,7 @@ void CPlayerInterface::showPuzzleMap() void CPlayerInterface::advmapSpellCast(const CGHeroInstance * caster, int spellID) { + EVENT_HANDLER_CALLED_BY_CLIENT; if (spellID == Spells::FLY || spellID == Spells::WATER_WALK) { eraseCurrentPathOf(caster, false); @@ -2086,7 +2102,7 @@ void CPlayerInterface::acceptTurn() if(howManyPeople > 1) adventureInt->startTurn(); - boost::unique_lock un(*pim); + //boost::unique_lock un(*pim); //Select sound for day start int totalDays = cb->getDate(); @@ -2164,12 +2180,12 @@ void CPlayerInterface::updateInfo(const CGObjectInstance * specific) void CPlayerInterface::battleNewRoundFirst( int round ) { + EVENT_HANDLER_CALLED_BY_CLIENT; if(LOCPLINT != this) { //another local interface should do this return; } - boost::unique_lock un(*pim); battleInt->newRoundFirst(round); } @@ -2181,7 +2197,7 @@ void CPlayerInterface::stopMovement() void CPlayerInterface::showMarketWindow(const IMarket *market, const CGHeroInstance *visitor) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; if(market->o->ID == 2) //Altar { //EEMarketMode mode = market->availableModes().front(); @@ -2196,35 +2212,35 @@ void CPlayerInterface::showMarketWindow(const IMarket *market, const CGHeroInsta void CPlayerInterface::showUniversityWindow(const IMarket *market, const CGHeroInstance *visitor) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; CUniversityWindow *cuw = new CUniversityWindow(visitor, market); GH.pushInt(cuw); } void CPlayerInterface::showHillFortWindow(const CGObjectInstance *object, const CGHeroInstance *visitor) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; CHillFortWindow *chfw = new CHillFortWindow(visitor, object); GH.pushInt(chfw); } void CPlayerInterface::availableArtifactsChanged(const CGBlackMarket *bm /*= NULL*/) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; if(CMarketplaceWindow *cmw = dynamic_cast(GH.topInt())) cmw->artifactsChanged(false); } void CPlayerInterface::showTavernWindow(const CGObjectInstance *townOrTavern) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; CTavernWindow *tv = new CTavernWindow(townOrTavern); GH.pushInt(tv); } void CPlayerInterface::showThievesGuildWindow (const CGObjectInstance * obj) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; CThievesGuildWindow *tgw = new CThievesGuildWindow(obj); GH.pushInt(tgw); } @@ -2261,26 +2277,26 @@ void CPlayerInterface::sendCustomEvent( int code ) void CPlayerInterface::stackChagedCount(const StackLocation &location, const TQuantity &change, bool isAbsolute) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; garrisonChanged(location.army); } void CPlayerInterface::stackChangedType(const StackLocation &location, const CCreature &newType) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; garrisonChanged(location.army); } void CPlayerInterface::stacksErased(const StackLocation &location) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; garrisonChanged(location.army); } #define UPDATE_IF(LOC) static_cast(adventureInt->infoBar.curSel) == LOC.army.get() void CPlayerInterface::stacksSwapped(const StackLocation &loc1, const StackLocation &loc2) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; garrisonChanged(loc1.army, UPDATE_IF(loc1)); if(loc2.army != loc1.army) garrisonChanged(loc2.army, UPDATE_IF(loc2)); @@ -2288,14 +2304,14 @@ void CPlayerInterface::stacksSwapped(const StackLocation &loc1, const StackLocat void CPlayerInterface::newStackInserted(const StackLocation &location, const CStackInstance &stack) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; garrisonChanged(location.army); } void CPlayerInterface::stacksRebalanced(const StackLocation &src, const StackLocation &dst, TQuantity count) { - boost::unique_lock un(*pim); - //bool updateInfobox = true; + EVENT_HANDLER_CALLED_BY_CLIENT; + garrisonChanged(src.army, UPDATE_IF(src)); if(dst.army != src.army) garrisonChanged(dst.army, UPDATE_IF(dst)); @@ -2304,12 +2320,12 @@ void CPlayerInterface::stacksRebalanced(const StackLocation &src, const StackLoc void CPlayerInterface::artifactPut(const ArtifactLocation &al) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; } void CPlayerInterface::artifactRemoved(const ArtifactLocation &al) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; BOOST_FOREACH(IShowActivatable *isa, GH.listInt) { if(isa->type & IShowActivatable::WITH_ARTIFACTS) @@ -2321,7 +2337,7 @@ void CPlayerInterface::artifactRemoved(const ArtifactLocation &al) void CPlayerInterface::artifactMoved(const ArtifactLocation &src, const ArtifactLocation &dst) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; BOOST_FOREACH(IShowActivatable *isa, GH.listInt) { if(isa->type & IShowActivatable::WITH_ARTIFACTS) @@ -2333,7 +2349,7 @@ void CPlayerInterface::artifactMoved(const ArtifactLocation &src, const Artifact void CPlayerInterface::artifactAssembled(const ArtifactLocation &al) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; BOOST_FOREACH(IShowActivatable *isa, GH.listInt) { if(isa->type & IShowActivatable::WITH_ARTIFACTS) @@ -2345,7 +2361,7 @@ void CPlayerInterface::artifactAssembled(const ArtifactLocation &al) void CPlayerInterface::artifactDisassembled(const ArtifactLocation &al) { - boost::unique_lock un(*pim); + EVENT_HANDLER_CALLED_BY_CLIENT; BOOST_FOREACH(IShowActivatable *isa, GH.listInt) { if(isa->type & IShowActivatable::WITH_ARTIFACTS) @@ -2357,38 +2373,32 @@ void CPlayerInterface::artifactDisassembled(const ArtifactLocation &al) void CPlayerInterface::playerStartsTurn(ui8 player) { + EVENT_HANDLER_CALLED_BY_CLIENT; + if(!GH.listInt.size()) { - boost::unique_lock un(*pim); - if(!GH.listInt.size()) - { - GH.pushInt(adventureInt); - adventureInt->activateKeys(); - } - if(howManyPeople == 1) - { - GH.curInt = this; - adventureInt->startTurn(); - } + GH.pushInt(adventureInt); + adventureInt->activateKeys(); + } + if(howManyPeople == 1) + { + GH.curInt = this; + adventureInt->startTurn(); } if(player != playerID && this == LOCPLINT) { waitWhileDialog(); - boost::unique_lock un(*pim); adventureInt->aiTurnStarted(); } } -void CPlayerInterface::waitForAllDialogs() +void CPlayerInterface::waitForAllDialogs(bool unlockPim /*= true*/) { + while(dialogs.size()) { - boost::unique_lock un(*pim); - while(dialogs.size()) - { - auto unlock = vstd::makeUnlockGuard(*pim); - SDL_Delay(5); - } + auto unlock = vstd::makeUnlockGuardIf(*pim, unlockPim); + SDL_Delay(5); } - waitWhileDialog(); + waitWhileDialog(unlockPim); } CPlayerInterface::SpellbookLastSetting::SpellbookLastSetting() diff --git a/client/CPlayerInterface.h b/client/CPlayerInterface.h index 1b57d30c5..8e5d2c6a2 100644 --- a/client/CPlayerInterface.h +++ b/client/CPlayerInterface.h @@ -170,7 +170,8 @@ public: void objectPropertyChanged(const SetObjectProperty * sop) OVERRIDE; void objectRemoved(const CGObjectInstance *obj) OVERRIDE; void gameOver(ui8 player, bool victory) OVERRIDE; - void playerStartsTurn(ui8 player) OVERRIDE; + void playerStartsTurn(ui8 player) OVERRIDE; //called before yourTurn on active itnerface + void showComp(const CComponent &comp) OVERRIDE; //display component in the advmapint infobox void serialize(COSer &h, const int version) OVERRIDE; //saving void serialize(CISer &h, const int version) OVERRIDE; //loading @@ -199,13 +200,12 @@ public: void showArtifactAssemblyDialog(ui32 artifactID, ui32 assembleTo, bool assemble, CFunctionList onYes, CFunctionList onNo); void garrisonChanged(const CGObjectInstance * obj, bool updateInfobox = true); void heroKilled(const CGHeroInstance* hero); - void waitWhileDialog(); - void waitForAllDialogs(); + void waitWhileDialog(bool unlockPim = true); + void waitForAllDialogs(bool unlockPim = true); bool shiftPressed() const; //determines if shift key is pressed (left or right or both) bool ctrlPressed() const; //determines if ctrl key is pressed (left or right or both) bool altPressed() const; //determines if alt key is pressed (left or right or both) void redrawHeroWin(const CGHeroInstance * hero); - void showComp(CComponent comp); //TODO: comment me void openTownWindow(const CGTownInstance * town); //shows townscreen void openHeroWindow(const CGHeroInstance * hero); //shows hero window with given hero SDL_Surface * infoWin(const CGObjectInstance * specific); //specific=0 => draws info about selected town/hero diff --git a/client/Client.cpp b/client/Client.cpp index ab73d621e..ac6b50fac 100644 --- a/client/Client.cpp +++ b/client/Client.cpp @@ -488,6 +488,7 @@ void CClient::handlePack( CPack * pack ) CBaseForCLApply *apply = applier->apps[typeList.getTypeID(pack)]; //find the applier if(apply) { + boost::unique_lock guiLock(*LOCPLINT->pim); apply->applyOnClBefore(this,pack); tlog5 << "\tMade first apply on cl\n"; gs->apply(pack); diff --git a/client/GUIClasses.cpp b/client/GUIClasses.cpp index b216b382e..3cedf85e8 100644 --- a/client/GUIClasses.cpp +++ b/client/GUIClasses.cpp @@ -745,11 +745,15 @@ void CComponent::init(Etype Type, int Subtype, int Val) subtitle = CGI->generaltexth->capColors[Subtype]; break; } - img = NULL; - free = false; type = Type; subtype = Subtype; val = Val; + if(!img) + { + free = false; + if(type == Component::BUILDING) + setSurface(graphics->buildingPics[subtype],val); + } SDL_Surface * temp = this->getImg(); if(!temp) { @@ -759,21 +763,24 @@ void CComponent::init(Etype Type, int Subtype, int Val) pos.w = temp->w; pos.h = temp->h; } -CComponent::CComponent(Etype Type, int Subtype, int Val, SDL_Surface *sur, bool freeSur):img(sur),free(freeSur) +CComponent::CComponent(Etype Type, int Subtype, int Val, SDL_Surface *sur, bool freeSur) { + img = sur; + free = freeSur; init(Type,Subtype,Val); } CComponent::CComponent(const Component &c) { - if(c.id==5) + img = NULL; + if(c.id == Component::EXPERIENCE) init(experience,c.subtype,c.val); else if(c.id == Component::SPELL) init(spell,c.subtype,c.val); else init((Etype)c.id,c.subtype,c.val); - if(c.id==2 && c.when==-1) + if(c.id == Component::RESOURCE && c.when==-1) subtitle += CGI->generaltexth->allTexts[3].substr(2,CGI->generaltexth->allTexts[3].length()-2); } @@ -806,7 +813,7 @@ void CComponent::show(SDL_Surface * to) blitAt(getImg(),pos.x,pos.y,to); } -SDL_Surface * CComponent::getImg() +SDL_Surface * CComponent::getImg() const { if (img) return img; @@ -831,7 +838,8 @@ SDL_Surface * CComponent::getImg() case spell: return graphics->spellscr->ourImages[subtype].bitmap; case building: - return setSurface(graphics->buildingPics[subtype],val); + assert(0); //img should have been set + //return setSurface(graphics->buildingPics[subtype],val); case creature: return graphics->bigImgs[subtype]; case hero: diff --git a/client/GUIClasses.h b/client/GUIClasses.h index af7dca7d8..9ac164152 100644 --- a/client/GUIClasses.h +++ b/client/GUIClasses.h @@ -185,7 +185,7 @@ public: virtual ~CComponent(); //d-tor void clickRight(tribool down, bool previousState); //call-in - SDL_Surface * getImg(); + SDL_Surface * getImg() const; virtual void show(SDL_Surface * to); virtual void activate(); virtual void deactivate(); diff --git a/client/NetPacksClient.cpp b/client/NetPacksClient.cpp index 58c74769b..72ce72d54 100644 --- a/client/NetPacksClient.cpp +++ b/client/NetPacksClient.cpp @@ -771,10 +771,7 @@ void ShowInInfobox::applyCl(CClient *cl) { CComponent sc(c); text.toString(sc.description); - if(vstd::contains(cl->playerint, player) && cl->playerint[player]->human) - { - static_cast(cl->playerint[player])->showComp(sc); - } + INTERFACE_CALL_IF_PRESENT(player,showComp, sc); } diff --git a/lib/IGameEventsReceiver.h b/lib/IGameEventsReceiver.h index 863c2e453..2776b7499 100644 --- a/lib/IGameEventsReceiver.h +++ b/lib/IGameEventsReceiver.h @@ -33,6 +33,7 @@ class CCreatureSet; struct BattleAttack; struct SetStackEffect; struct BattleTriggerEffect; +class CComponent; class DLL_LINKAGE IBattleEventsReceiver @@ -116,4 +117,5 @@ public: virtual void playerBlocked(int reason){}; //reason: 0 - upcoming battle virtual void gameOver(ui8 player, bool victory){}; //player lost or won the game virtual void playerStartsTurn(ui8 player){}; + virtual void showComp(const CComponent &comp) {}; //display component in the advmapint infobox }; \ No newline at end of file diff --git a/lib/UnlockGuard.h b/lib/UnlockGuard.h index 9861a32d7..f19b3c0a5 100644 --- a/lib/UnlockGuard.h +++ b/lib/UnlockGuard.h @@ -50,6 +50,11 @@ namespace vstd unlock(*m); } + unlock_guard() + { + m = NULL; + } + unlock_guard(unlock_guard &&other) : m(other.m) { @@ -74,10 +79,29 @@ namespace vstd return unlock_guard >(m_); } template + unlock_guard > makeEmptyGuard(Mutex &) + { + return unlock_guard >(); + } + template + unlock_guard > makeUnlockGuardIf(Mutex &m_, bool shallUnlock) + { + return shallUnlock + ? makeUnlockGuard(m_) + : unlock_guard >(); + } + template unlock_guard > makeUnlockSharedGuard(Mutex &m_) { return unlock_guard >(m_); } + template + unlock_guard > makeUnlockSharedGuardIf(Mutex &m_, bool shallUnlock) + { + return shallUnlock + ? makeUnlockSharedGuard(m_) + : unlock_guard >(); + } typedef unlock_guard > unlock_shared_guard; }