From c018e3dcc241d8c56c4628733406d567fbe511d0 Mon Sep 17 00:00:00 2001 From: zethfoxster Date: Mon, 4 Jul 2016 15:13:35 -0400 Subject: [PATCH] fixed a ton of memleaks, narrowed down the memleaks to just the computeNewCost function. still working on a fix for this. --- projects/mtg/src/AllAbilities.cpp | 2 + projects/mtg/src/Damage.cpp | 2 +- projects/mtg/src/MTGCardInstance.cpp | 30 +++---- projects/mtg/src/MTGRules.cpp | 116 ++++++++++++++------------- projects/mtg/src/ManaCost.cpp | 26 +++--- 5 files changed, 95 insertions(+), 81 deletions(-) diff --git a/projects/mtg/src/AllAbilities.cpp b/projects/mtg/src/AllAbilities.cpp index a5b673f3a..bb3c3f39d 100644 --- a/projects/mtg/src/AllAbilities.cpp +++ b/projects/mtg/src/AllAbilities.cpp @@ -1319,6 +1319,7 @@ int AACopier::resolve() MTGCard* clone = MTGCollection()->getCardById(_target->copiedID); MTGCardInstance * myClone = NEW MTGCardInstance(clone, source->controller()->game); source->copy(myClone); + SAFE_DELETE(myClone); source->isACopier = true; source->copiedID = _target->getMTGId(); source->modifiedbAbi = _target->modifiedbAbi; @@ -6133,6 +6134,7 @@ int AProduceMana::produce() { AManaProducer *amp = NEW AManaProducer(game, game->mLayers->actionLayer()->getMaxId(), source, source->controller(), ManaCost::parseManaCost(ManaDescription,NULL,source), NULL, 0,"",false); amp->resolve(); + SAFE_DELETE(amp);//once you call resolve() on a ability, you can safely delete it. } return 1; } diff --git a/projects/mtg/src/Damage.cpp b/projects/mtg/src/Damage.cpp index 9177b442d..b5a94633c 100644 --- a/projects/mtg/src/Damage.cpp +++ b/projects/mtg/src/Damage.cpp @@ -251,7 +251,7 @@ int Damage::resolve() target->lifeLostThisTurn += damage; if ( typeOfDamage == 1 && target == source->controller()->opponent() )//add vector prowledtypes. { - vector values = MTGAllCards::getCreatureValuesById(); + vector values = MTGAllCards::getCreatureValuesById();//getting a weird crash here. rarely. for (size_t i = 0; i < values.size(); ++i) { if ( source->hasSubtype( values[i] ) && find(source->controller()->prowledTypes.begin(), source->controller()->prowledTypes.end(), values[i])==source->controller()->prowledTypes.end() ) diff --git a/projects/mtg/src/MTGCardInstance.cpp b/projects/mtg/src/MTGCardInstance.cpp index 1c89311f5..a96ade840 100644 --- a/projects/mtg/src/MTGCardInstance.cpp +++ b/projects/mtg/src/MTGCardInstance.cpp @@ -962,20 +962,22 @@ JQuadPtr MTGCardInstance::getIcon() ManaCost * MTGCardInstance::computeNewCost(MTGCardInstance * card,ManaCost * newCost, ManaCost * refCost, bool noTrinisphere) { + //this function introduces a nasty memleak, it also cant be safe_deleted at the locations if its not used as it leads to a wild nullpointer crash. if(!card) return NULL; - + ManaCost * cardUnchangedCost = NEW ManaCost(); + cardUnchangedCost->copy(newCost); if(card->getIncreasedManaCost()->getConvertedCost()) - newCost->add(card->getIncreasedManaCost()); + cardUnchangedCost->add(card->getIncreasedManaCost()); if(card->getReducedManaCost()->getConvertedCost()) - newCost->remove(card->getReducedManaCost()); + cardUnchangedCost->remove(card->getReducedManaCost()); if(refCost->extraCosts) - newCost->extraCosts = refCost->extraCosts; + cardUnchangedCost->extraCosts = refCost->extraCosts; //affinity int color = 0; string type = ""; ManaCost * original = NEW ManaCost(); - original->copy(newCost); + original->copy(cardUnchangedCost); int reducem = 0; bool resetCost = false; @@ -987,7 +989,7 @@ ManaCost * MTGCardInstance::computeNewCost(MTGCardInstance * card,ManaCost * new if(!resetCost) { resetCost = true; - newCost->copy(original); + cardUnchangedCost->copy(original); } TargetChooserFactory tf(observer); TargetChooser * tcn = tf.createTargetChooser(newAff->tcString,card,NULL); @@ -1006,7 +1008,7 @@ ManaCost * MTGCardInstance::computeNewCost(MTGCardInstance * card,ManaCost * new SAFE_DELETE(tcn); ManaCost * removingCost = ManaCost::parseManaCost(newAff->manaString); for(int j = 0; j < reducem; j++) - newCost->remove(removingCost); + cardUnchangedCost->remove(removingCost); SAFE_DELETE(removingCost); } }//end2 @@ -1035,7 +1037,7 @@ ManaCost * MTGCardInstance::computeNewCost(MTGCardInstance * card,ManaCost * new color = 1; type = "creature"; } - newCost->copy(original); + cardUnchangedCost->copy(original); int reduce = 0; if(card->has(Constants::AFFINITYGREENCREATURES)) { @@ -1047,8 +1049,8 @@ ManaCost * MTGCardInstance::computeNewCost(MTGCardInstance * card,ManaCost * new else reduce = card->controller()->game->battlefield->countByType(type); for(int i = 0; i < reduce;i++) - if(newCost->getCost(color) > 0) - newCost->remove(color,1); + if(cardUnchangedCost->getCost(color) > 0) + cardUnchangedCost->remove(color,1); }//end3 if(!noTrinisphere) @@ -1056,9 +1058,9 @@ ManaCost * MTGCardInstance::computeNewCost(MTGCardInstance * card,ManaCost * new //trinisphere... now how to implement kicker recomputation if(card->has(Constants::TRINISPHERE)) { - for(int jj = newCost->getConvertedCost(); jj < 3; jj++) + for(int jj = cardUnchangedCost->getConvertedCost(); jj < 3; jj++) { - newCost->add(Constants::MTG_COLOR_ARTIFACT, 1); + cardUnchangedCost->add(Constants::MTG_COLOR_ARTIFACT, 1); card->countTrini++; } } @@ -1066,7 +1068,7 @@ ManaCost * MTGCardInstance::computeNewCost(MTGCardInstance * card,ManaCost * new { if(card->countTrini) { - newCost->remove(Constants::MTG_COLOR_ARTIFACT, card->countTrini); + cardUnchangedCost->remove(Constants::MTG_COLOR_ARTIFACT, card->countTrini); card->countTrini=0; } } @@ -1074,7 +1076,7 @@ ManaCost * MTGCardInstance::computeNewCost(MTGCardInstance * card,ManaCost * new SAFE_DELETE(original); - return newCost; + return cardUnchangedCost; } MTGCardInstance * MTGCardInstance::getNextPartner() diff --git a/projects/mtg/src/MTGRules.cpp b/projects/mtg/src/MTGRules.cpp index 6d61dc687..99269961b 100644 --- a/projects/mtg/src/MTGRules.cpp +++ b/projects/mtg/src/MTGRules.cpp @@ -709,7 +709,7 @@ int MTGAlternativeCostRule::isReactingToClick(MTGCardInstance * card, ManaCost * return 0;//overload has its own rule if(!card->getManaCost()->getAlternative()) return 0; - ManaCost * alternateCost = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getAlternative()),card->getManaCost()->getAlternative()); + ManaCost * alternateCost = card->computeNewCost(card,card->model->data->getManaCost()->getAlternative(),card->getManaCost()->getAlternative()); if(alternateCost->extraCosts) for(unsigned int i = 0; i < alternateCost->extraCosts->costs.size();i++) { @@ -773,7 +773,7 @@ int MTGAlternativeCostRule::reactToClick(MTGCardInstance * card) if ( !isReactingToClick(card)) return 0; - ManaCost * alternateCost = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getAlternative()),card->getManaCost()->getAlternative()); + ManaCost * alternateCost = card->computeNewCost(card,card->model->data->getManaCost()->getAlternative(),card->getManaCost()->getAlternative()); card->paymenttype = MTGAbility::ALTERNATIVE_COST; if(alternateCost->extraCosts) for(unsigned int i = 0; i < alternateCost->extraCosts->costs.size();i++) @@ -942,7 +942,7 @@ int MTGBuyBackRule::isReactingToClick(MTGCardInstance * card, ManaCost * mana) return 0; if(!card->getManaCost()->getBuyback()) return 0; - ManaCost * buybackCost = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getBuyback()),card->getManaCost()->getBuyback()); + ManaCost * buybackCost = card->computeNewCost(card,card->model->data->getManaCost()->getBuyback(),card->getManaCost()->getBuyback()); if(buybackCost->extraCosts) for(unsigned int i = 0; i < buybackCost->extraCosts->costs.size();i++) { @@ -956,7 +956,7 @@ int MTGBuyBackRule::reactToClick(MTGCardInstance * card) if (!isReactingToClick(card)) return 0; - ManaCost * buybackCost = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getBuyback()),card->getManaCost()->getBuyback()); + ManaCost * buybackCost = card->computeNewCost(card,card->model->data->getManaCost()->getBuyback(),card->getManaCost()->getBuyback()); if(buybackCost->extraCosts) for(unsigned int i = 0; i < buybackCost->extraCosts->costs.size();i++) { @@ -996,7 +996,7 @@ int MTGFlashBackRule::isReactingToClick(MTGCardInstance * card, ManaCost * mana) return 0; if(!card->getManaCost()->getFlashback()) return 0; - ManaCost * flashbackCost = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getFlashback()),card->getManaCost()->getFlashback()); + ManaCost * flashbackCost = card->computeNewCost(card,card->model->data->getManaCost()->getFlashback(),card->getManaCost()->getFlashback()); if(flashbackCost->extraCosts) for(unsigned int i = 0; i < flashbackCost->extraCosts->costs.size();i++) { @@ -1007,7 +1007,7 @@ int MTGFlashBackRule::isReactingToClick(MTGCardInstance * card, ManaCost * mana) int MTGFlashBackRule::reactToClick(MTGCardInstance * card) { - ManaCost * flashbackCost = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getFlashback()),card->getManaCost()->getFlashback()); + ManaCost * flashbackCost = card->computeNewCost(card,card->model->data->getManaCost()->getFlashback(),card->getManaCost()->getFlashback()); if(flashbackCost->extraCosts) for(unsigned int i = 0; i < flashbackCost->extraCosts->costs.size();i++) { @@ -1049,17 +1049,17 @@ int MTGRetraceRule::isReactingToClick(MTGCardInstance * card, ManaCost * mana) Player * player = game->currentlyActing(); if(!card->getManaCost()->getRetrace()) return 0; - ManaCost * retraceCost = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getRetrace()),card->getManaCost()->getRetrace()); + if (!player->game->graveyard->hasCard(card)) + { + return 0; + } + auto retraceCost = card->computeNewCost(card, card->model->data->getManaCost()->getRetrace(), card->getManaCost()->getRetrace()); if(retraceCost->extraCosts) for(unsigned int i = 0; i < retraceCost->extraCosts->costs.size();i++) { retraceCost->extraCosts->costs[i]->setSource(card); - } - - if (!player->game->graveyard->hasCard(card)) - return 0; - - return MTGAlternativeCostRule::isReactingToClick( card, mana, retraceCost ); + } + return MTGAlternativeCostRule::isReactingToClick( card, mana, retraceCost); } @@ -1068,7 +1068,7 @@ int MTGRetraceRule::reactToClick(MTGCardInstance * card) if (!isReactingToClick(card)) return 0; - ManaCost * retraceCost = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getRetrace()),card->getManaCost()->getRetrace()); + ManaCost * retraceCost = card->computeNewCost(card,card->model->data->getManaCost()->getRetrace(),card->getManaCost()->getRetrace()); if(retraceCost->extraCosts) for(unsigned int i = 0; i < retraceCost->extraCosts->costs.size();i++) { @@ -1245,7 +1245,7 @@ int MTGMorphCostRule::isReactingToClick(MTGCardInstance * card, ManaCost *) if (card->controller()->game->playRestrictions->canPutIntoZone(card, card->controller()->game->stack) == PlayRestriction::CANT_PLAY) return 0; ManaCost * playerMana = player->getManaPool(); - ManaCost * morph = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getMorph()),card->getManaCost()->getMorph()); + ManaCost * morph = card->computeNewCost(card,card->model->data->getManaCost()->getMorph(),card->getManaCost()->getMorph()); if(morph->extraCosts) for(unsigned int i = 0; i < morph->extraCosts->costs.size();i++) { @@ -1274,7 +1274,7 @@ int MTGMorphCostRule::reactToClick(MTGCardInstance * card) Player * player = game->currentlyActing(); ManaCost * cost = card->getManaCost(); ManaCost * playerMana = player->getManaPool(); - ManaCost * morph = card->computeNewCost(card,NEW ManaCost(card->model->data->getManaCost()->getMorph()),card->getManaCost()->getMorph()); + ManaCost * morph = card->computeNewCost(card,card->model->data->getManaCost()->getMorph(),card->getManaCost()->getMorph()); if(morph->extraCosts) for(unsigned int i = 0; i < morph->extraCosts->costs.size();i++) { @@ -1364,23 +1364,26 @@ MTGAlternativeCostRule(observer, _id) int MTGPayZeroRule::isReactingToClick(MTGCardInstance * card, ManaCost * mana) { - if(!card->has(Constants::PAYZERO)) - return 0; - Player * player = game->currentlyActing(); - ManaCost * cost = NEW ManaCost(ManaCost::parseManaCost("{0}",NULL,NULL)); - ManaCost * newCost = card->computeNewCost(card,cost,cost); - if(newCost->extraCosts) - for(unsigned int i = 0; i < newCost->extraCosts->costs.size();i++) - { - newCost->extraCosts->costs[i]->setSource(card); - } - - if(card->isLand()) - return 0; - if (!player->game->graveyard->hasCard(card) && !player->game->exile->hasCard(card) && !player->game->hand->hasCard(card)) - return 0; - if ((!card->has(Constants::CANPLAYFROMGRAVEYARD) && player->game->graveyard->hasCard(card))||(!card->has(Constants::CANPLAYFROMEXILE) && player->game->exile->hasCard(card))) - return 0; + if (!card->has(Constants::PAYZERO)) + return 0; + Player * player = game->currentlyActing(); + if (card->isLand() || (!player->game->graveyard->hasCard(card) && !player->game->exile->hasCard(card) && !player->game->hand->hasCard(card))) + { + //only allowed to pay zero for cards in library??? above is "if you dont have it in hand, grave, or exile" + return 0; + } + if ((!card->has(Constants::CANPLAYFROMGRAVEYARD) && player->game->graveyard->hasCard(card)) || (!card->has(Constants::CANPLAYFROMEXILE) && player->game->exile->hasCard(card))) + { + return 0; + } + ManaCost * cost = NEW ManaCost(ManaCost::parseManaCost("{0}", NULL, NULL)); + ManaCost * newCost = card->computeNewCost(card, cost, cost); + SAFE_DELETE(cost); + if (newCost->extraCosts) + for (unsigned int i = 0; i < newCost->extraCosts->costs.size(); i++) + { + newCost->extraCosts->costs[i]->setSource(card); + } if(card->has(Constants::CANPLAYFROMGRAVEYARD)) CustomName = "Zero Cast From Graveyard"; else if(card->has(Constants::CANPLAYFROMEXILE)) @@ -1430,22 +1433,26 @@ int MTGOverloadRule::isReactingToClick(MTGCardInstance * card, ManaCost * mana) { if (card->alias != 11000) return 0; - Player * player = game->currentlyActing(); - ManaCost * cost = NEW ManaCost(card->model->data->getManaCost()->getAlternative()); - ManaCost * newCost = card->computeNewCost(card,cost,cost); + if (card->isLand()) + { + return 0; + } + Player * player = card->controller(); + if (!player->game->graveyard->hasCard(card) && !player->game->exile->hasCard(card) && !player->game->hand->hasCard(card)) + { + return 0; + } + if ((!card->has(Constants::CANPLAYFROMGRAVEYARD) && player->game->graveyard->hasCard(card)) || (!card->has(Constants::CANPLAYFROMEXILE) && player->game->exile->hasCard(card))) + { + return 0; + } + ManaCost * newCost = card->computeNewCost(card, card->model->data->getManaCost()->getAlternative(), card->getManaCost()->getAlternative()); if(newCost->extraCosts) for(unsigned int i = 0; i < newCost->extraCosts->costs.size();i++) { newCost->extraCosts->costs[i]->setSource(card); } - if (card->isLand()) - return 0; - if (!player->game->graveyard->hasCard(card) && !player->game->exile->hasCard(card) && !player->game->hand->hasCard(card)) - return 0; - if ((!card->has(Constants::CANPLAYFROMGRAVEYARD) && player->game->graveyard->hasCard(card))||(!card->has(Constants::CANPLAYFROMEXILE) && player->game->exile->hasCard(card))) - return 0; - return MTGAlternativeCostRule::isReactingToClick(card, mana, newCost); } @@ -1455,7 +1462,7 @@ int MTGOverloadRule::reactToClick(MTGCardInstance * card) return 0; ManaCost * cost = NEW ManaCost(card->model->data->getManaCost()->getAlternative()); - ManaCost * newCost = card->computeNewCost(card,cost,cost); + ManaCost * newCost = card->computeNewCost(card, card->model->data->getManaCost()->getAlternative(), card->getManaCost()->getAlternative()); if(newCost->extraCosts) for(unsigned int i = 0; i < newCost->extraCosts->costs.size();i++) { @@ -1489,23 +1496,24 @@ int MTGBestowRule::isReactingToClick(MTGCardInstance * card, ManaCost * mana) { if (!card->model) return 0; - //Player * player = game->currentlyActing(); if (!card->model->data->getManaCost()->getBestow()) return 0; if (card->isInPlay(game)) return 0; - ManaCost * cost = NEW ManaCost(card->model->data->getManaCost()->getBestow()); - ManaCost * newCost = card->computeNewCost(card, cost, cost); + if (card->isLand()) + { + return 0; + } + if (!card->controller()->inPlay()->hasType("creature") && !card->controller()->opponent()->inPlay()->hasType("creature")) + { + return 0; + } + ManaCost * newCost = card->computeNewCost(card, card->model->data->getManaCost()->getBestow(), card->getManaCost()->getBestow()); if (newCost->extraCosts) for (unsigned int i = 0; i < newCost->extraCosts->costs.size(); i++) { newCost->extraCosts->costs[i]->setSource(card); } - SAFE_DELETE(cost); - if (card->isLand()) - return 0; - if (!card->controller()->inPlay()->hasType("creature") && !card->controller()->opponent()->inPlay()->hasType("creature")) - return 0; return MTGAlternativeCostRule::isReactingToClick(card, mana, newCost); } @@ -1515,8 +1523,7 @@ int MTGBestowRule::reactToClick(MTGCardInstance * card) return 0; //this new method below in all alternative cost type causes a memleak, however, you cant safedelete the cost here as it cause a crash //TODO::::we need to get to the source of this leak and fix it. - ManaCost * cost = NEW ManaCost(card->model->data->getManaCost()->getBestow()); - ManaCost * newCost = card->computeNewCost(card, cost, cost); + ManaCost * newCost = card->computeNewCost(card, card->model->data->getManaCost()->getBestow(), card->getManaCost()->getBestow()); if (newCost->extraCosts) for (unsigned int i = 0; i < newCost->extraCosts->costs.size(); i++) @@ -2895,6 +2902,7 @@ int MTGPersistRule::receiveEvent(WEvent * event) } AAMover *putinplay = NEW AAMover(game, game->mLayers->actionLayer()->getMaxId(), copy, copy,"ownerbattlefield",code,NULL,undying,persist); putinplay->oneShot = true; + game->mLayers->actionLayer()->garbage.push_back(putinplay); putinplay->fireAbility(); return 1; } diff --git a/projects/mtg/src/ManaCost.cpp b/projects/mtg/src/ManaCost.cpp index de9991d65..bdde06e50 100644 --- a/projects/mtg/src/ManaCost.cpp +++ b/projects/mtg/src/ManaCost.cpp @@ -389,11 +389,10 @@ ManaCost::ManaCost(ManaCost * manaCost) { cost[i] = manaCost->getCost(i); } - hybrids = manaCost->hybrids; - - kicker = NEW ManaCost( manaCost->kicker ); - if(kicker) - kicker->isMulti = manaCost->isMulti; + hybrids = manaCost->hybrids; + kicker = NEW ManaCost(manaCost->kicker); + if (kicker) + kicker->isMulti = manaCost->isMulti; Retrace = NEW ManaCost( manaCost->Retrace ); BuyBack = NEW ManaCost( manaCost->BuyBack ); alternative = NEW ManaCost( manaCost->alternative ); @@ -401,8 +400,11 @@ ManaCost::ManaCost(ManaCost * manaCost) morph = NEW ManaCost( manaCost->morph ); suspend = NEW ManaCost( manaCost->suspend ); Bestow = NEW ManaCost(manaCost->Bestow); - - extraCosts = manaCost->extraCosts ? manaCost->extraCosts->clone() : NULL; + extraCosts = NULL; + if (manaCost->extraCosts) + { + extraCosts = manaCost->extraCosts->clone(); + } manaUsedToCast = NULL; xColor = manaCost->xColor; } @@ -430,8 +432,12 @@ ManaCost::ManaCost(const ManaCost& manaCost) morph = NEW ManaCost( manaCost.morph ); suspend = NEW ManaCost( manaCost.suspend ); Bestow = NEW ManaCost(manaCost.Bestow); + extraCosts = NULL; + if (manaCost.extraCosts) + { + extraCosts = manaCost.extraCosts->clone(); + } - extraCosts = manaCost.extraCosts ? manaCost.extraCosts->clone() : NULL; manaUsedToCast = NULL; xColor = manaCost.xColor; } @@ -480,7 +486,6 @@ void ManaCost::x() { if (cost.size() <= (size_t)Constants::NB_Colors) { - DebugTrace("Seems ManaCost was not properly initialized"); return; } @@ -491,7 +496,6 @@ int ManaCost::hasX() { if (cost.size() <= (size_t)Constants::NB_Colors) { - DebugTrace("Seems ManaCost was not properly initialized"); return 0; } if (xColor > 0) @@ -504,7 +508,6 @@ void ManaCost::specificX(int color) { if (cost.size() <= (size_t)Constants::NB_Colors) { - DebugTrace("Seems ManaCost was not properly initialized"); return; } xColor = color; @@ -515,7 +518,6 @@ int ManaCost::hasSpecificX() { if (cost.size() <= (size_t)Constants::NB_Colors) { - DebugTrace("Seems ManaCost was not properly initialized"); return 0; } if(xColor > 0)