- Minor refactor of "transforms" ability

- Updated changeling and transforms to only get creature types (as initially intended by the code), removed hardcoded list of "types to avoid"

The test suite passes

projects/mtg/include/Subtypes.h

@@ -47,6 +47,7 @@ public:
     bool isType(int type);
     bool isSubType(int type);
     int add(string value, int parentType);
+    const vector<string> getValuesById();
 };
 
 #endif

projects/mtg/src/AllAbilities.cpp

@@ -2401,39 +2401,33 @@ ATransformer::ATransformer(int id, MTGCardInstance * source, MTGCardInstance * t
     PopulateAbilityIndexVector(abilities, sabilities);
     PopulateColorIndexVector(colors, sabilities);
     
-    addNewColors = false;
+   
-    if(sabilities.find("newcolors") != string::npos)
-    addNewColors = true;
     //this subkeyword adds a color without removing the existing colors.
-    remove = false;
+    addNewColors = (sabilities.find("newcolors") != string::npos);
-    if (stypes == "removesubtypes")
+    remove = (stypes == "removesubtypes");
-        remove = true;
+    removeTypes = (stypes == "removetypes");
-    removeTypes = false;
+
-    if (stypes == "removetypes")
+    //Gains or loses all creature subtypes
-        removeTypes = true;
     if (stypes == "allsubtypes" || stypes == "removesubtypes")
     {
-        for (int i = Subtypes::LAST_TYPE + 1;; i++)
+        for (size_t i = 0; i <Subtypes::subtypesList->getValuesById().size(); ++i)
+        {
+            if (!Subtypes::subtypesList->isSubtypeOfType(i,Subtypes::TYPE_CREATURE))
+                continue;
+
+            //Erwan 2011/5/6 String comparison is expensive. Any way to do this in a cleaner way?
+            //I think this is releated to the fact that "Pestilence Rats" is a type for some reason, maybe we don't need that anymore
+            //TODO Remove the following block if possible
             {
                 string s = Subtypes::subtypesList->find(i);
-            {
+
-                if (s == "")
-                    break;
                 if (s.find(" ") != string::npos)
                     continue;
-                if (s == "Nothing" || s == "Swamp" || s == "Plains" || s == "Mountain" || s == "Forest"
-                        || s == "Island" || s == "Shrine" || s == "Basic" || s == "Colony" || s == "Desert"
-                        || s == "Dismiss" || s == "Equipment" || s == "Everglades" || s == "Grasslands" || s == "Lair"
-                        || s == "Level" || s == "Levelup" || s == "Mine" || s == "Oasis" || s == "World" || s == "Aura" || s == "Land"|| s == "Legendary" || s == "Token" || s == "Planeswalker")
-                {//dont add "nothing" or land type to this card.
             }
-                else
+
-                {
             types.push_back(i);
         }
     }
-        }
-    }
     else
     {
         PopulateSubtypesIndexVector(types, stypes);
@@ -2458,27 +2452,24 @@ ATransformer::ATransformer(int id, MTGCardInstance * source, MTGCardInstance * t
     {
         _target = (MTGCardInstance *) target;
     }
-        if (_target)
+
+    if (!_target)
     {
+        DebugTrace("ALL_ABILITIES: Target not set in ATransformer::addToGame\n");
+        return 0;
+    }
+        
     while (_target->next)
         _target = _target->next;
+
     for (int j = 0; j < Constants::MTG_NB_COLORS; j++)
     {
         if (_target->hasColor(j))
             oldcolors.push_back(j);
     }
-            for (int j = Subtypes::LAST_TYPE + 1;; j++)
+    for (size_t j = 0; j < _target->types.size(); ++j)
-            {
+        oldtypes.push_back( _target->types[j]);
-                string otypes = Subtypes::subtypesList->find(j);
+
-                if (otypes == "")
-                    break;
-                if (otypes.find(" ") != string::npos)
-                    continue;
-                if (_target->hasSubtype(j))
-                {
-                    oldtypes.push_back(j);
-                }
-            }
     list<int>::iterator it;
     for (it = colors.begin(); it != colors.end(); it++)
     {
@@ -2486,20 +2477,15 @@ ATransformer::ATransformer(int id, MTGCardInstance * source, MTGCardInstance * t
             _target->setColor(0, 1);
     }
 
-            for (it = types.begin(); it != types.end(); it++)
-            {
     if (removeTypes)
     {
         //remove the main types from a card, ie: hidden enchantment cycle.
-                    _target->removeType(0,1);
+        for (int i = 0; i < Subtypes::LAST_TYPE; ++ i)
-                    _target->removeType(1,1);
+            _target->removeType(i,1);
-                    _target->removeType(2,1);
-                    _target->removeType(3,1);
-                    _target->removeType(4,1);
-                    _target->removeType(5,1);
-                    _target->removeType(6,1);
-                    _target->removeType(7,1);
     }
+
+    for (it = types.begin(); it != types.end(); it++)
+    {
         if (remove)
         {
             _target->removeType(*it);
@@ -2532,10 +2518,6 @@ ATransformer::ATransformer(int id, MTGCardInstance * source, MTGCardInstance * t
         _target->basicAbilities.set(*it);
     }
 
-            for (it = oldcolors.begin(); it != oldcolors.end(); it++)
-            {
-            }
-
     if(newAbilityFound)
     {
         for (unsigned int k = 0 ; k < newAbilitiesList.size();k++)
@@ -2591,7 +2573,7 @@ ATransformer::ATransformer(int id, MTGCardInstance * source, MTGCardInstance * t
         _target->life = _target->toughness;
         delete val;
     }
-        }
+
     return MTGAbility::addToGame();
 }
     
@@ -2629,9 +2611,10 @@ int ATransformer::destroy()
         while (_target->next)
             _target = _target->next;
         list<int>::iterator it;
-        for (it = types.begin(); it != types.end(); it++)
+        
-        {
         if (!remove)
+        {
+            for (it = types.begin(); it != types.end(); it++)
             {
                     bool removing = true;
                     for(unsigned int k = 0;k < dontremove.size();k++)

projects/mtg/src/MTGAbility.cpp

@@ -2021,46 +2021,31 @@ MTGAbility * AbilityFactory::parseMagicLine(string s, int id, Spell * spell, MTG
         }            
         for(unsigned int j = 0;j < abilities.size();j++)
         {
-            if(abilities[j].find("setpower=") != string::npos)
+            vector<string> splitPower = parseBetween(abilities[j], "setpower=", ",", false);
+            if(splitPower.size())
             {
                 newpowerfound = true;
-                int powerstart = abilities[j].find("setpower=");
+                newpower = splitPower[1];
-                newpower = abilities[j].substr(powerstart + 9).c_str();
             }
-            if(abilities[j].find("settoughness=") != string::npos)
+            vector<string> splitToughness = parseBetween(abilities[j], "settoughness=", ",", false);
+            if(splitToughness.size())
             {
                 newtoughnessfound = true;
-                int toughnessstart = abilities[j].find("settoughness=");
+                newtoughness = splitToughness[1];
-                newtoughness = abilities[j].substr(toughnessstart + 13).c_str();
             }
-            if(abilities[j].find("newability[") != string::npos)
+            vector<string> splitAbilities = parseBetween(abilities[j], "newability[", "]");
+            if(splitAbilities.size())
             {
                 newAbilityFound = true;
-                size_t NewSkill = abilities[j].find("newability[");
+                newAbilitiesList.push_back(splitAbilities[1]);
-                size_t NewSkillEnd = abilities[j].find_last_of("]");
-                string newAbilities = abilities[j].substr(NewSkill + 11,NewSkillEnd - NewSkill - 11);
-                newAbilitiesList.push_back(newAbilities);
             }
         }
-        MTGAbility * a;
+
-        bool foreverEffect = false;
+        if (oneShot || forceUEOT || forceFOREVER)
-        if (forceFOREVER)
+            return NEW ATransformerInstant(id, card, target, stypes, sabilities,newpower,newpowerfound,newtoughness,newtoughnessfound,newAbilitiesList,newAbilityFound,forceFOREVER);
-        {
+        
-        foreverEffect = true;
+        return NEW ATransformer(id, card, target, stypes, sabilities,newpower,newpowerfound,newtoughness,newtoughnessfound,newAbilitiesList,newAbilityFound);
-        }
+
-        if (oneShot || forceUEOT)
-        {
-            a = NEW ATransformerInstant(id, card, target, stypes, sabilities,newpower,newpowerfound,newtoughness,newtoughnessfound,newAbilitiesList,newAbilityFound,foreverEffect);
-        }
-        else if(foreverEffect)
-        {
-        a = NEW ATransformerInstant(id, card, target, stypes, sabilities,newpower,newpowerfound,newtoughness,newtoughnessfound,newAbilitiesList,newAbilityFound,foreverEffect);
-        }
-        else
-        {
-            a = NEW ATransformer(id, card, target, stypes, sabilities,newpower,newpowerfound,newtoughness,newtoughnessfound,newAbilitiesList,newAbilityFound);
-        }
-        return a;
     }
     
     //Change Power/Toughness

projects/mtg/src/MTGCardInstance.cpp

@@ -171,26 +171,29 @@ void MTGCardInstance::initMTGCI()
     data = this; //an MTGCardInstance point to itself for data, allows to update it without killing the underlying database item
 
     if (basicAbilities[(int)Constants::CHANGELING])
-    {//if the card is a changeling.
+    {//if the card is a changeling, it gains all creature subtypes
-        for (int i = Subtypes::LAST_TYPE + 1;; i++)
+        for (size_t i = 0; i <Subtypes::subtypesList->getValuesById().size(); ++i)
+        {
+            if (hasSubtype(i))
+                continue;
+
+            if (!Subtypes::subtypesList->isSubtypeOfType(i,Subtypes::TYPE_CREATURE))
+                continue;
+
+            //Erwan 2011/5/6 String comparison is expensive. Any way to do this in a cleaner way?
+            //I think this is releated to the fact that "Pestilence Rats" is a type for some reason, maybe we don't need that anymore
+            //TODO Remove the following block if possible
             {
                 string s = Subtypes::subtypesList->find(i);
-            if (!hasSubtype(i))
+
-            {
-                if (s == "")
-                    break;
                 if (s.find(" ") != string::npos)
                     continue;
-                if (s == "Nothing" || s == "Swamp" || s == "Plains" || s == "Mountain" || s == "Forest" ||
+            }
-                    s == "Island" || s == "Shrine" || s == "Basic" || s == "Colony" || s == "Desert" ||
+
-                    s == "Dismiss" || s == "Equipment" || s == "Everglades" || s == "Grasslands" || s == "Lair" ||
-                    s == "Level" || s == "Levelup" || s == "Mine" || s == "Oasis" || s == "World" || s == "Aura"
-                    || s == "Land"|| s == "Legendary" || s == "Token" || s == "Planeswalker")
-                    continue;
             addType(i);
         }
     }
-    }
+
     int colored = 0;
 
     for (int i = Constants::MTG_COLOR_GREEN; i <= Constants::MTG_COLOR_WHITE; ++i)

projects/mtg/src/Subtypes.cpp

@@ -93,3 +93,8 @@ bool Subtypes::isSubType(int type)
 {
     return (!isSuperType(type) && !isType(type));
 }
+
+const vector<string> Subtypes::getValuesById()
+{
+    return valuesById;
+}