Fix for issue 647 (Evil Presence cast on a swamp removes mana ability)

projects/mtg/bin/Res/test/_tests.txt

@@ -260,6 +260,8 @@ erg_raiders_i157.txt
 evil_presence.txt
 evil_presence2.txt
 evil_presence3.txt
+evil_presence_i647.txt
+evil_presence_i647_2.txt
 explore.txt
 Faceless_Butcher.txt
 fangren_pathcutter.txt

projects/mtg/bin/Res/test/evil_presence_i647.txt

@@ -0,0 +1,21 @@
+#Testing Evil Presence on a swamp...it should have no visible effect
+# Bug: it removes the swamp's ability
+#see http://code.google.com/p/wagic/issues/detail?id=647
+[INIT]
+FIRSTMAIN
+[PLAYER1]
+hand:Evil Presence
+inplay:Swamp
+manapool:{B}
+[PLAYER2]
+[DO]
+Evil Presence
+Swamp
+Swamp
+[ASSERT]
+FIRSTMAIN
+[PLAYER1]
+inplay:Swamp,Evil Presence
+manapool:{B}
+[PLAYER2]
+[END]

projects/mtg/bin/Res/test/evil_presence_i647_2.txt

@@ -0,0 +1,33 @@
+#Testing Evil Presence on a swamp...it should have no visible effect
+# Bug: it removes the swamp's ability
+#In thistest we make sure we don't get a crash during the next turn, when abilities are added/removed from different sources
+#see http://code.google.com/p/wagic/issues/detail?id=647
+[INIT]
+FIRSTMAIN
+[PLAYER1]
+hand:Evil Presence, Disenchant
+inplay:Swamp,Forest,Plains
+manapool:{B}
+[PLAYER2]
+[DO]
+Evil Presence
+Swamp
+eot
+eot
+next
+#upkeep
+next
+#draw
+next
+#main
+Forest
+Plains
+Disenchant
+Evil Presence
+[ASSERT]
+FIRSTMAIN
+[PLAYER1]
+inplay:Swamp,Forest,Plains
+graveyard:Evil Presence,disenchant
+[PLAYER2]
+[END]

projects/mtg/include/AllAbilities.h

@@ -2160,6 +2160,7 @@ class ALord: public ListMaintainerAbility, public NestedAbility
 public:
     int includeSelf;
     map<Damageable *, MTGAbility *> abilities;
+    vector<MTGAbility *> bermudaTriangle; //Hack: Lost abilities that need to be properly removed if they reappear
 
     ALord(int _id, MTGCardInstance * card, TargetChooser * _tc, int _includeSelf, MTGAbility * a) :
         ListMaintainerAbility(_id, card), NestedAbility(a)
@@ -2170,7 +2171,19 @@ public:
             if(ability->aType == MTGAbility::STANDARD_PREVENT)
             aType = MTGAbility::STANDARD_PREVENT;
         }
-        
+     
+    void Update(float dt)
+    {
+        ListMaintainerAbility::Update(dt);
+
+        //This is a hack to avoid some abilities "combing back from the dead" because of the loseAbility keyword :/
+        for (int i = (int)(bermudaTriangle.size()) - 1 ; i >= 0 ; --i)
+        {
+            if (game->removeObserver(bermudaTriangle[i]))
+                bermudaTriangle.erase(bermudaTriangle.begin()+i);
+        }
+    }
+
     int canBeInList(Player *p)
     {
         if (tc->canTarget(p)) return 1;
@@ -2237,8 +2250,9 @@ public:
     int removed(MTGCardInstance * card)
     {
         if (abilities.find(card) != abilities.end())
-        {
-            game->removeObserver(abilities[card]);
+        { 
+            if (!game->removeObserver(abilities[card]))
+                bermudaTriangle.push_back(abilities[card]);
             abilities.erase(card);
         }
         return 1;

projects/mtg/include/GameObserver.h

@@ -76,7 +76,7 @@ class GameObserver{
   void Affinity();
   void eventOccured();
   void addObserver(MTGAbility * observer);
-  void removeObserver(ActionElement * observer);
+  bool removeObserver(ActionElement * observer);
   void startGame(Rules * rules);
   void untapPhase();
   int isInPlay(MTGCardInstance *  card);

projects/mtg/include/MTGCardInstance.h

@@ -58,6 +58,7 @@ public:
     bool wasDealtDamage;
     bool damageToOpponent;
     bool damageToController;
+    bool mPropertiesChangedSinceLastUpdate;
     int reduxamount;
     int flanked;
     int regenerateTokens;

projects/mtg/src/CardPrimitive.cpp

@@ -274,7 +274,7 @@ void CardPrimitive::setName(const string& value)
     std::transform(lcname.begin(), lcname.end(), lcname.begin(), ::tolower);
     //This is a bug fix for plague rats and the "foreach ability"
     //Right now we add names as types, so that they get recognized
-    if (lcname.at(value.length() - 1) == 's')
+    if (value.length() && lcname.at(value.length() - 1) == 's')
         Subtypes::subtypesList->find(lcname);
 }
 

projects/mtg/src/GameObserver.cpp

@@ -303,14 +303,15 @@ void GameObserver::addObserver(MTGAbility * observer)
     mLayers->actionLayer()->Add(observer);
 }
 
-void GameObserver::removeObserver(ActionElement * observer)
+//Returns true if the Ability was correctly removed from the game, false otherwise
+//Main (valid) reason of returning false is an attempt at removing an Ability that has already been removed
+bool GameObserver::removeObserver(ActionElement * observer)
 {
-    if (observer)
-        mLayers->actionLayer()->moveToGarbage(observer);
+    if (!observer)
+        return false;
+
+    return mLayers->actionLayer()->moveToGarbage(observer);
 
-    else
-    {
-    } //TODO log error
 }
 
 GameObserver::~GameObserver()
@@ -375,6 +376,8 @@ void GameObserver::gameStateBasedEffects()
         {
             MTGCardInstance * card = zone->cards[j];
             card->afterDamage();
+            card->mPropertiesChangedSinceLastUpdate = false;
+
             ///////////////////////////////////////////////////////
             //Remove auras that don't have a valid target anymore//
             ///////////////////////////////////////////////////////

projects/mtg/src/MTGAbility.cpp

@@ -3767,7 +3767,7 @@ void ListMaintainerAbility::updateTargets()
     for (map<MTGCardInstance *, bool>::iterator it = cards.begin(); it != cards.end(); ++it)
     {
         MTGCardInstance * card = (*it).first;
-        if (!canBeInList(card))
+        if (!canBeInList(card) || card->mPropertiesChangedSinceLastUpdate)
         {
             temp[card] = true;
         }
@@ -3794,11 +3794,12 @@ void ListMaintainerAbility::updateTargets()
             {
                 for (int j = 0; j < zone->nb_cards; j++)
                 {
-                    if (canBeInList(zone->cards[j]))
+                     MTGCardInstance * card = zone->cards[j];
+                    if (canBeInList(card))
                     {
-                        if (cards.find(zone->cards[j]) == cards.end())
+                        if (cards.find(card) == cards.end())
                         {
-                            temp[zone->cards[j]] = true;
+                            temp[card] = true;
                         }
                     }
                 }

projects/mtg/src/MTGCardInstance.cpp

@@ -145,6 +145,7 @@ void MTGCardInstance::initMTGCI()
     wasDealtDamage = false;
     suspended = false;
     castMethod = Constants::NOT_CAST;
+    mPropertiesChangedSinceLastUpdate = false;
 
 
     for (int i = 0; i < ManaCost::MANA_PAID_WITH_RETRACE +1; i++)
@@ -233,6 +234,16 @@ void MTGCardInstance::addType(int type)
 {
     bool before = hasType(type);
     CardPrimitive::addType(type);
+
+    if (!before)
+        mPropertiesChangedSinceLastUpdate = true;
+
+    // If the card name is not set, set it to the type.
+    //This is a hack for "transform", used in combination with "losesubtypes" on Basic Lands
+    //See removeType below
+    if (!name.length())
+        setName(Subtypes::subtypesList->find(type));
+
     WEvent * e = NEW WEventCardChangeType(this, type, before, true);
     GameObserver * go = GameObserver::GetInstance();
     if (go)
@@ -267,6 +278,16 @@ int MTGCardInstance::removeType(int id, int removeAll)
     bool before = hasType(id);
     int result = CardPrimitive::removeType(id, removeAll);
     bool after = hasType(id);
+    if (before != after)
+    {
+        mPropertiesChangedSinceLastUpdate = true;
+        // Basic lands have the same name as their subtypes, and TargetChoosers don't make a distinction between name and type,
+        // so if we remove a subtype "Forest", we also need to remove its name.
+        //This means the card might lose its name, but usually when we force remove a type, we add another one just after that.
+        //see "AddType" above which force sets a name if necessary
+        if (name.compare(Subtypes::subtypesList->find(id)) == 0)
+            setName("");
+    }
     WEvent * e = NEW WEventCardChangeType(this, id, before, after);
     GameObserver * go = GameObserver::GetInstance();
     if (go)