ai bug fixes, minor refactor of phaseaction/phaseactionmulti/upcost/upcostmulti, counter cost was safed wrong, sorry!, and fixed parsing of upcost..it was chopping the last bracket of the cost when used for things like echo, it required us to add {0} to those cost...that is no longer required.

projects/mtg/include/MTGAbility.h

@@ -520,6 +520,9 @@ public:
     int TapAll(TargetChooser * tc);
     int UntapAll(TargetChooser * tc);
     void addAbilities(int _id, Spell * spell);
+    MTGAbility * parseUpkeepAbility(string s = "", MTGCardInstance * card = NULL, Spell * spell = NULL, int restrictions = 0, int id = -1);
+    MTGAbility * parsePhaseActionAbility(string s = "", MTGCardInstance * card = NULL, Spell * spell = NULL,MTGCardInstance * target = NULL, int restrictions = 0, int id = -1);
+
 };
 
 

projects/mtg/src/AIPlayer.cpp

@@ -665,7 +665,6 @@ int AIAction::getEfficiency()
     if (s->has(ability))
         return 0;
     MTGAbility * a = AbilityFactory::getCoreAbility(ability);
-
     if (!a)
     {
         DebugTrace("FATAL: Ability is NULL in AIAction::getEfficiency()");
@@ -941,40 +940,29 @@ int AIAction::getEfficiency()
         }
         break;
     }
-
-    case MTGAbility::UPCOST:
-    {
-        //hello, Ai pay your upcost please :P, this entices Ai into paying upcost, the conditional isAi() is required strangely ai is able to pay upcost during YOUR upkeep.
-        if (g->getCurrentGamePhase() == Constants::MTG_PHASE_UPKEEP && g->currentPlayer == p && p == a->source->controller())
-        {
-            efficiency = 100;
-        }
-        break;
-    }
-
     case MTGAbility::FOREACH:
     case MTGAbility::MANA_PRODUCER://only way to hit this condition is nested manaabilities, ai skips manaproducers by defualt when finding an ability to use.
     {
         MTGCardInstance * _target = (MTGCardInstance *) (a->target);
         MTGAbility * a = AbilityFactory::getCoreAbility(ability);
-
+        AManaProducer * amp = dynamic_cast<AManaProducer*>(a);
         efficiency = 0;
         //trying to encourage Ai to use his foreach manaproducers in first main
-        if (a->getCost() && a->getCost()->getConvertedCost() && a->aType == MTGAbility::MANA_PRODUCER && (g->getCurrentGamePhase() == Constants::MTG_PHASE_FIRSTMAIN
+        if (amp && amp->output && amp->output->getConvertedCost() && (g->getCurrentGamePhase() == Constants::MTG_PHASE_FIRSTMAIN
                 || g->getCurrentGamePhase() == Constants::MTG_PHASE_SECONDMAIN) && _target->controller()->game->hand->nb_cards > 0)
         {
             efficiency = 0;
             for (int i = Constants::MTG_NB_COLORS - 1; i > 0; i--)
             {
                 if ((p->game->hand->hasColor(i) || p->game->hand->hasColor(0))
-                        && (dynamic_cast<AManaProducer*> (a)->output->hasColor(i)))
+                        && amp->output->hasColor(i))
                 {
                     
                     efficiency = 100;
                 }
             }
 
-            if (a->getCost() && a->getCost()->getConvertedCost() && p->game->hand->hasX())
+            if (amp->getCost() && amp->getCost()->getConvertedCost() && p->game->hand->hasX())
                 efficiency = 100;
 
         }
@@ -1196,6 +1184,14 @@ int AIAction::getEfficiency()
         }
         break;
     }
+    if(AUpkeep * auk = dynamic_cast<AUpkeep *>(ability))
+    {
+        //hello, Ai pay your upcost please :P, this entices Ai into paying upcost, the conditional isAi() is required strangely ai is able to pay upcost during YOUR upkeep.
+        if (auk && g->getCurrentGamePhase() == Constants::MTG_PHASE_UPKEEP && g->currentPlayer == p && p == a->source->controller())
+        {
+            efficiency = 100;
+        }
+    }
     if (AAMover * aam = dynamic_cast<AAMover *>(a))
     {
         MTGGameZone * z = aam->destinationZone(target);

projects/mtg/src/ExtraCost.cpp

@@ -621,16 +621,17 @@ int CounterCost::isPaymentSet()
 int CounterCost::canPay()
 {
     // if target needs to be chosen, then move on.
-    if(!target)
-        return 0;
     if (tc)
         return 1;
     if (counter->nb >= 0)
         return 1; //add counters always possible
     // otherwise, move on only if target has enough counters
-    Counter * targetCounter = target->counters->hasCounter(counter->name.c_str(), counter->power, counter->toughness);
+    if(target)
-    if (targetCounter && targetCounter->nb >= -counter->nb)
+    {
-        return 1;
+        Counter * targetCounter = target->counters->hasCounter(counter->name.c_str(), counter->power, counter->toughness);
+        if (targetCounter && targetCounter->nb >= -counter->nb)
+            return 1;
+    }
     return 0;
 }
 

projects/mtg/src/MTGAbility.cpp

@@ -977,6 +977,7 @@ MTGAbility * AbilityFactory::parseMagicLine(string s, int id, Spell * spell, MTG
             string cond = sWithoutTc.substr(ifKeywords[i].length(),ifKeywords[i].length() + sWithoutTc.find(" then ")-6);
             string s1 = s.substr(s.find(" then ")+6);
             MTGAbility * a1 = parseMagicLine(s1, id, spell, card);
+            if(!a1) return NULL;
             MTGAbility * a = NEW IfThenAbility(id, a1, card,checkIf[i],cond);
             a->canBeInterrupted = false;
             a->oneShot = true;
@@ -1022,89 +1023,14 @@ MTGAbility * AbilityFactory::parseMagicLine(string s, int id, Spell * spell, MTG
     found = s.find("upcostmulti");
     if (found != string::npos)
     {
-        bool Cumulative = false;
+        return AbilityFactory::parseUpkeepAbility(s,card,spell,restrictions,id);
-        size_t cumulative = s.find("cumulativeupcost");
-        if(cumulative != string::npos)
-            Cumulative = true;
-        size_t start = s.find("[");
-        size_t end = s.find("]", start);
-        string s1 = s.substr(start + 1, end - start - 1);
-        size_t seperator = s1.find(";");
-        int phase = Constants::MTG_PHASE_UPKEEP;
-        int once = 0;
-        if (seperator != string::npos)
-        {
-            for (int i = 0; i < Constants::NB_MTG_PHASES; i++)
-            {
-                if (s1.find("next") != string::npos)
-                    once = 1;
-                string phaseStr = Constants::MTGPhaseCodeNames[i];
-                if (s1.find(phaseStr) != string::npos)
-                {
-                    phase = PhaseRing::phaseStrToInt(phaseStr);
-                    break;
-                }
-            }
-            s1 = s1.substr(0, seperator - 1);
-        }
-        ManaCost * cost = ManaCost::parseManaCost(s1);
-
-        if (!cost)
-        {
-            DebugTrace("MTGABILITY: Parsing Error: " << s);
-            return NULL;
-        }
-
-        string sAbility = s.substr(end + 1);
-        MTGAbility * a = parseMagicLine(sAbility, id, spell, card);
-
-        if (!a)
-        {
-            DebugTrace("MTGABILITY: Parsing Error: " << s);
-            delete (cost);
-            return NULL;
-        }
-
-        return NEW AUpkeep(id, card, a, cost, restrictions, phase, once,Cumulative);
     }
-    
     //Phase based actions
     found = s.find("phaseactionmulti");
     if (found != string::npos)
     {
-        vector<string> splitActions = parseBetween(s, "[", "]");
+        return parsePhaseActionAbility(s,card,spell,target,restrictions,id);
-        if (!splitActions.size())
-        {
-            DebugTrace("MTGABILITY:Parsing Error " << s);
-            return NULL;
-        }
-        string s1 = splitActions[1];
-        int phase = Constants::MTG_PHASE_UPKEEP;
-        for (int i = 0; i < Constants::NB_MTG_PHASES; i++)
-        {
-            string phaseStr = Constants::MTGPhaseCodeNames[i];
-            if (s1.find(phaseStr) != string::npos)
-            {
-                phase = PhaseRing::phaseStrToInt(phaseStr);
-                break;
-            }
-        }
-        
-        bool opponentturn = (s1.find("my") == string::npos);
-        bool myturn = (s1.find("opponent") == string::npos);
-
-        bool sourceinPlay = (s1.find("sourceinplay") != string::npos);
-        bool next = (s1.find("next") == string::npos); //Why is this one the opposite of the two others? That's completely inconsistent
-        bool once = (s1.find("once") != string::npos);
-
-        MTGCardInstance * _target = NULL;
-        if (spell)
-            _target = spell->getNextCardTarget();
-        if(!_target)
-            _target = target;
-          return NEW APhaseActionGeneric(id, card,_target, splitActions[2], restrictions, phase,sourceinPlay,next,myturn,opponentturn,once);
     }
-    
     //Multiple abilities for ONE cost
     found = s.find("&&");
     if (found != string::npos)
@@ -1441,97 +1367,17 @@ MTGAbility * AbilityFactory::parseMagicLine(string s, int id, Spell * spell, MTG
    }
 
     //Phase based actions  
-    //TODO: This is 100% the same code as phaseActionMulti, can't these 2 be merged somehow?   
     found = s.find("phaseaction");
     if (found != string::npos)
     {
-        vector<string> splitActions = parseBetween(s, "[", "]");
+        return parsePhaseActionAbility(s,card,spell,target,restrictions,id);
-        if (!splitActions.size())
-        {
-            DebugTrace("MTGABILITY:Parsing Error " << s);
-            return NULL;
-        }
-        string s1 = splitActions[1];
-        int phase = Constants::MTG_PHASE_UPKEEP;
-        for (int i = 0; i < Constants::NB_MTG_PHASES; i++)
-        {
-            string phaseStr = Constants::MTGPhaseCodeNames[i];
-            if (s1.find(phaseStr) != string::npos)
-            {
-                phase = PhaseRing::phaseStrToInt(phaseStr);
-                break;
-            }
-        }
-
-        bool opponentturn = (s1.find("my") == string::npos);
-        bool myturn = (s1.find("opponent") == string::npos);
-
-        bool sourceinPlay = (s1.find("sourceinplay") != string::npos);
-        bool next = (s1.find("next") == string::npos); //Why is this one the opposite of the two others? That's completely inconsistent
-        bool once = (s1.find("once") != string::npos);
-
-        MTGCardInstance * _target = NULL;
-        if (spell)
-            _target = spell->getNextCardTarget();
-        if(!_target)
-            _target = target;
-          return NEW APhaseActionGeneric(id, card,_target, trim(splitActions[2]), restrictions, phase,sourceinPlay,next,myturn,opponentturn,once);
     }
-
     //Upkeep Cost
-    //TODO: This is 100% the same code as upkeepCostMulti, can't these 2 be merged somehow?
-    //also see phaseActionMulti
     found = s.find("upcost");
     if (found != string::npos)
     {
-    bool Cumulative = false;
+       return AbilityFactory::parseUpkeepAbility(s,card,spell,restrictions,id);
-    size_t cumulative = s.find("cumulativeupcost");
-    if(cumulative != string::npos)
-    Cumulative = true;
-        size_t start = s.find("[");
-        size_t end = s.find("]", start);
-        string s1 = s.substr(start + 1, end - start - 1);
-        size_t seperator = s1.find(";");
-        int phase = Constants::MTG_PHASE_UPKEEP;
-        int once = 0;
-        if (seperator != string::npos)
-        {
-            for (int i = 0; i < Constants::NB_MTG_PHASES; i++)
-            {
-                if (s1.find("next") != string::npos)
-                    once = 1;
-
-                string phaseStr = Constants::MTGPhaseCodeNames[i];
-                if (s1.find(phaseStr) != string::npos)
-                {
-                    phase = PhaseRing::phaseStrToInt(phaseStr);
-                    break;
-                }
-
-            }
-            s1 = s1.substr(0, seperator - 1);
-        }
-        ManaCost * cost = ManaCost::parseManaCost(s1);
-
-        if (!cost)
-        {
-            DebugTrace("MTGABILITY: Parsing Error: " << s);
-            return NULL;
-        }
-
-        string sAbility = s.substr(end + 1);
-        MTGAbility * a = parseMagicLine(sAbility, id, spell, card);
-
-        if (!a)
-        {
-            DebugTrace("MTGABILITY: Parsing Error: " << s);
-            delete (cost);
-            return NULL;
-        }
-
-        return NEW AUpkeep(id, card, a, cost, restrictions, phase, once,Cumulative);
     }
-
     //Cycling
     found = s.find("cycling");
     if (found != string::npos)
@@ -2455,6 +2301,90 @@ MTGAbility * AbilityFactory::parseMagicLine(string s, int id, Spell * spell, MTG
     return NULL;
 }
 
+MTGAbility * AbilityFactory::parseUpkeepAbility(string s,MTGCardInstance * card,Spell * spell,int restrictions,int id)
+{
+    MTGAbility * a1 = NULL;
+    bool Cumulative = false;
+    size_t cumulative = s.find("cumulativeupcost");
+    if(cumulative != string::npos)
+        Cumulative = true;
+    size_t start = s.find("[");
+    size_t end = s.find("]", start);
+    string s1 = s.substr(start + 1, end - start - 1);
+    size_t seperator = s1.find(";");
+    int phase = Constants::MTG_PHASE_UPKEEP;
+    int once = 0;
+    if (seperator != string::npos)
+    {
+        for (int i = 0; i < Constants::NB_MTG_PHASES; i++)
+        {
+            if (s1.find("next") != string::npos)
+                once = 1;
+
+            string phaseStr = Constants::MTGPhaseCodeNames[i];
+            if (s1.find(phaseStr) != string::npos)
+            {
+                phase = PhaseRing::phaseStrToInt(phaseStr);
+                break;
+            }
+
+        }
+        s1 = s1.substr(0, seperator);
+    }
+    ManaCost * cost = ManaCost::parseManaCost(s1);
+
+    if (!cost)
+    {
+        DebugTrace("MTGABILITY: Parsing Error: " << s);
+        return NULL;
+    }
+
+    string sAbility = s.substr(end + 1);
+    MTGAbility * a = parseMagicLine(sAbility, id, spell, card);
+
+    if (!a)
+    {
+        DebugTrace("MTGABILITY: Parsing Error: " << s);
+        delete (cost);
+        return NULL;
+    }
+    return  NEW AUpkeep(id, card, a, cost, restrictions, phase, once,Cumulative);;
+}
+
+MTGAbility * AbilityFactory::parsePhaseActionAbility(string s,MTGCardInstance * card,Spell * spell,MTGCardInstance * target, int restrictions,int id)
+{
+        vector<string> splitActions = parseBetween(s, "[", "]");
+        if (!splitActions.size())
+        {
+            DebugTrace("MTGABILITY:Parsing Error " << s);
+            return NULL;
+        }
+        string s1 = splitActions[1];
+        int phase = Constants::MTG_PHASE_UPKEEP;
+        for (int i = 0; i < Constants::NB_MTG_PHASES; i++)
+        {
+            string phaseStr = Constants::MTGPhaseCodeNames[i];
+            if (s1.find(phaseStr) != string::npos)
+            {
+                phase = PhaseRing::phaseStrToInt(phaseStr);
+                break;
+            }
+        }
+
+        bool opponentturn = (s1.find("my") == string::npos);
+        bool myturn = (s1.find("opponent") == string::npos);
+        bool sourceinPlay = (s1.find("sourceinplay") != string::npos);
+        bool next = (s1.find("next") == string::npos); //Why is this one the opposite of the two others? That's completely inconsistent
+        bool once = (s1.find("once") != string::npos);
+
+        MTGCardInstance * _target = NULL;
+        if (spell)
+            _target = spell->getNextCardTarget();
+        if(!_target)
+            _target = target;
+          return NEW APhaseActionGeneric(id, card,_target, trim(splitActions[2]), restrictions, phase,sourceinPlay,next,myturn,opponentturn,once);
+}
+
 //Tells the AI if the ability should target itself or an ennemy
 int AbilityFactory::abilityEfficiency(MTGAbility * a, Player * p, int mode, TargetChooser * tc,Targetable * target)
 {