From f8368d01c4859a6615a8395f679c878a7272f1d7 Mon Sep 17 00:00:00 2001 From: "wrenczes@gmail.com" Date: Fri, 10 Dec 2010 08:37:41 +0000 Subject: [PATCH] Found & fixed two memory leaks: - computeActions would leak a ManaCost. This was fairly minor. - AIPlayer::SelectAbility had a major leak. Basically, there's some code that pulls a random number for an efficiency check - if the action's efficiency value was below that random number, the action pointer was cleared, and none of the actions that were to be discarded would be deleted out of the rankings map. I've switched out the rankings container to not contain action pointers, but real action objects, so regardless what kind of logic is implemented, the map will properly clear out its objects upon destruction. --- projects/mtg/include/AIPlayer.h | 73 +++++++++++++++++++++------------ projects/mtg/src/AIPlayer.cpp | 41 +++++++----------- 2 files changed, 61 insertions(+), 53 deletions(-) diff --git a/projects/mtg/include/AIPlayer.h b/projects/mtg/include/AIPlayer.h index a508823dd..89e12d16f 100644 --- a/projects/mtg/include/AIPlayer.h +++ b/projects/mtg/include/AIPlayer.h @@ -21,39 +21,58 @@ using std::queue; class AIStats; -class AIAction{ +class AIAction +{ protected: - int efficiency; - static int currentId; + int efficiency; + static int currentId; public: - MTGAbility * ability; - NestedAbility * nability; - Player * player; - int id; - bool checked; - MTGCardInstance * click; - MTGCardInstance * target; // TODO Improve - AIAction(MTGAbility * a, MTGCardInstance * c, MTGCardInstance * t = NULL):ability(a),click(c),target(t){player = NULL; efficiency = -1; id = currentId++;}; - AIAction(MTGCardInstance * c, MTGCardInstance * t = NULL):click(c),target(t){player = NULL; ability = NULL; efficiency = -1; id = currentId++;}; - AIAction(Player * p):player(p){ability = NULL; target = NULL; click = NULL; efficiency = -1;}; - int getEfficiency(); - int Act(); - + MTGAbility * ability; + NestedAbility * nability; + Player * player; + int id; + bool checked; + MTGCardInstance * click; + MTGCardInstance * target; // TODO Improve + AIAction(MTGAbility * a, MTGCardInstance * c, MTGCardInstance * t = NULL) + : ability(a),click(c),target(t), player(NULL), efficiency(-1) + { + id = currentId++; + }; + + AIAction(MTGCardInstance * c, MTGCardInstance * t = NULL) + : click(c),target(t), player(NULL), ability(NULL), efficiency(-1) + { + id = currentId++; + }; + + AIAction(Player * p) + : player(p), ability(NULL), target(NULL), click(NULL), efficiency(-1) + { + }; + + int getEfficiency(); + int Act(); }; -class CmpAbilities { // compares Abilities efficiency - public: - bool operator()(AIAction * a1, AIAction * a2) const { - int e1 = 0; - e1 = a1->getEfficiency(); - int e2 = 0; - e2 = a2->getEfficiency(); - if (e1 == e2) return a1->id < a2->id; - return (e1 > e2); - } +// compares Abilities efficiency +class CmpAbilities +{ +public: + bool operator()(const AIAction& a1, const AIAction& a2) const + { + AIAction* a1Ptr = const_cast(&a1); + AIAction* a2Ptr = const_cast(&a2); + int e1 = a1Ptr->getEfficiency(); + int e2 = a2Ptr->getEfficiency(); + if (e1 == e2) return a1Ptr->id < a2Ptr->id; + return (e1 > e2); + } }; +typedef std::map RankingContainer; + class AIPlayer: public Player{ protected: //Variables used by Test suite @@ -92,7 +111,7 @@ public: int isAI(){return 1;}; int canHandleCost(MTGAbility * ability); int selectAbility(); - int createAbilityTargets(MTGAbility * a, MTGCardInstance * c, map * ranking); + int createAbilityTargets(MTGAbility * a, MTGCardInstance * c, RankingContainer& ranking); int useAbility(); virtual int getEfficiency(AIAction * action); diff --git a/projects/mtg/src/AIPlayer.cpp b/projects/mtg/src/AIPlayer.cpp index e2ec8f0e3..9e4948c79 100644 --- a/projects/mtg/src/AIPlayer.cpp +++ b/projects/mtg/src/AIPlayer.cpp @@ -576,12 +576,12 @@ int AIAction::getEfficiency() return efficiency; } -int AIPlayer::createAbilityTargets(MTGAbility * a, MTGCardInstance * c, map * ranking) +int AIPlayer::createAbilityTargets(MTGAbility * a, MTGCardInstance * c, RankingContainer& ranking) { if (!a->tc) { - AIAction * as = NEW AIAction(a, c, NULL); - (*ranking)[as] = 1; + AIAction aiAction(a, c, NULL); + ranking[aiAction] = 1; return 1; } GameObserver * g = GameObserver::GetInstance(); @@ -598,8 +598,8 @@ int AIPlayer::createAbilityTargets(MTGAbility * a, MTGCardInstance * c, maptc->canTarget(t)) { - AIAction * as = NEW AIAction(a, c, t); - (*ranking)[as] = 1; + AIAction aiAction(a, c, t); + ranking[aiAction] = 1; } } } @@ -621,7 +621,7 @@ int AIPlayer::selectAbility() return 0; } findingAbility = true;//im looking now safely! - map ranking; + RankingContainer ranking; list::iterator it; GameObserver * g = GameObserver::GetInstance(); //This loop is extrmely inefficient. TODO: optimize! @@ -636,9 +636,10 @@ int AIPlayer::selectAbility() { MTGCardInstance * card = game->inPlay->cards[j]; if (a->isReactingToClick(card, totalPotentialMana)) - { //This test is to avod the huge call to getPotentialManaCost after that + { //This test is to avoid the huge call to getPotentialManaCost after that ManaCost * pMana = getPotentialMana(card); - if (a->isReactingToClick(card, pMana)) createAbilityTargets(a, card, &ranking); + if (a->isReactingToClick(card, pMana)) + createAbilityTargets(a, card, ranking); delete (pMana); } } @@ -647,30 +648,17 @@ int AIPlayer::selectAbility() if (ranking.size()) { - AIAction * a = ranking.begin()->first; + AIAction action = ranking.begin()->first; int chance = 1; if (!forceBestAbilityUse) chance = 1 + WRand() % 100; - if (getEfficiency(a) < chance) - { - a = NULL; - } - else + if (action.getEfficiency() >= chance) { if (!clickstream.size()) { DebugTrace("AIPlayer:Using Activated ability"); - if (tapLandsForMana(a->ability->cost, a->click)) - clickstream.push(a); + if (tapLandsForMana(action.ability->cost, action.click)) + clickstream.push(NEW AIAction(action)); } - else - { - a = NULL; - } - } - map::iterator it2; - for (it2 = ranking.begin(); it2 != ranking.end(); it2++) - { - if (a && a != it2->first) delete (it2->first); } } findingAbility = false;//ok to start looking again. @@ -1224,7 +1212,8 @@ int AIPlayerBaka::computeActions() } } - if (potential) delete (currentMana); + if (currentMana != NULL) + delete (currentMana); if (nextCardToPlay) { if (potential)