Added action logging during attack/block of the AI player. To do that, I wrapped some of the direct access from the player to the action layer into the game observer.

First version where I managed to finish a normal game while undoing several actions until the end. There are still some problems in direct damage spells and interruption management. I added several assert in the code to catch them.

JGE/src/JFileSystem.cpp

@@ -183,11 +183,14 @@ bool JFileSystem::DirExists(const string& strDirname)
 
 bool JFileSystem::FileExists(const string& strFilename)
 { 
-    izfstream temp;
+    bool result = false;
-    bool result = openForRead(temp, strFilename);
+    if(strFilename != "")
-    if (temp)
+    {
-        temp.close();
+        izfstream temp;
-
+        result = openForRead(temp, strFilename);
+        if (temp)
+            temp.close();
+    }
     return result;
 }
 

projects/mtg/include/GameObserver.h

@@ -26,7 +26,11 @@ class GameObserver{
  protected:
   MTGCardInstance * cardWaitingForTargets;
   queue<WEvent *> eventsQueue;
+  // used when we're running to log actions
   list<string> actionsList;
+  // used when we're loading to know what to load
+  list<string> loadingList;
+  list<string>::iterator loadingite;
 
   int untap(MTGCardInstance * card);
   bool WaitForExtraPayment(MTGCardInstance* card);
@@ -34,9 +38,7 @@ class GameObserver{
   void cleanup();
   string startupGameSerialized;
   bool parseLine(const string& s);
-  void logAction(const string& s) {
+  void logAction(const string& s);
-      actionsList.push_back(s);
-  };
   bool processActions(bool undo);
   friend ostream& operator<<(ostream&, GameObserver&);
   bool load(const string& s, bool undo);
@@ -65,6 +67,8 @@ class GameObserver{
   TargetChooser * getCurrentTargetChooser();
   void stackObjectClicked(Interruptible * action);
 
+  int cardClick(MTGCardInstance * card, MTGAbility *ability);
+  int cardClick(MTGCardInstance * card, int abilityType);
   int cardClick(MTGCardInstance * card,Targetable * _object = NULL );
   int getCurrentGamePhase();
   const char * getCurrentGamePhaseName();
@@ -106,7 +110,7 @@ class GameObserver{
   void logAction(int playerId, const string& s="") {
       logAction(players[playerId], s);
   };
-  void logAction(MTGCardInstance* card, MTGGameZone* zone = NULL);
+  void logAction(MTGCardInstance* card, MTGGameZone* zone, size_t index, int result);
   bool undo();
   bool isLoading(){ return mLoading; };
 };

projects/mtg/src/AIPlayer.cpp

@@ -53,7 +53,7 @@ int AIAction::Act()
     }
     if (ability)
     {
-        g->mLayers->actionLayer()->reactToClick(ability, click);
+        g->cardClick(click, ability);
         if (target && !mAbilityTargets.size())
         {
             g->cardClick(target);

projects/mtg/src/AIPlayerBaka.cpp

@@ -1985,10 +1985,9 @@ int AIPlayerBaka::chooseAttackers()
         cd.init();
         cd.setType("creature");
         MTGCardInstance * card = NULL;
-        MTGAbility * a = observer->mLayers->actionLayer()->getAbility(MTGAbility::MTG_ATTACK_RULE);
         while ((card = cd.nextmatch(game->inPlay, card)))
         {
-            observer->mLayers->actionLayer()->reactToClick(a, card);
+            observer->cardClick(card, MTGAbility::MTG_ATTACK_RULE);
         }
     }
     return 1;
@@ -2033,12 +2032,11 @@ int AIPlayerBaka::chooseBlockers()
     cd.setType("Creature");
     cd.unsecureSetTapped(-1);
     card = NULL;
-    MTGAbility * a = observer->mLayers->actionLayer()->getAbility(MTGAbility::MTG_BLOCK_RULE);
 
     // We first try to block the major threats, those that are marked in the Top 3 of our stats
     while ((card = cd.nextmatch(game->inPlay, card)))
     {
-        observer->mLayers->actionLayer()->reactToClick(a, card);
+        observer->cardClick(card, MTGAbility::MTG_BLOCK_RULE);
         int set = 0;
         while (!set)
         {
@@ -2062,7 +2060,7 @@ int AIPlayerBaka::chooseBlockers()
                 }
                 else
                 {
-                    observer->mLayers->actionLayer()->reactToClick(a, card);
+                    observer->cardClick(card, MTGAbility::MTG_BLOCK_RULE);
                 }
             }
         }
@@ -2077,7 +2075,7 @@ int AIPlayerBaka::chooseBlockers()
         {
             while (card->defenser)
             {
-                observer->mLayers->actionLayer()->reactToClick(a, card);
+                observer->cardClick(card, MTGAbility::MTG_BLOCK_RULE);
             }
         }
     }
@@ -2088,7 +2086,7 @@ int AIPlayerBaka::chooseBlockers()
     {
         if (!card->defenser)
         {
-            observer->mLayers->actionLayer()->reactToClick(a, card);
+            observer->cardClick(card, MTGAbility::MTG_BLOCK_RULE);
             int set = 0;
             while (!set)
             {
@@ -2102,7 +2100,7 @@ int AIPlayerBaka::chooseBlockers()
                     if (opponentsToughness[attacker] <= 0 || (card->toughness <= attacker->power && opponentForce * 2 < life
                             && !canFirstStrikeKill(card, attacker)) || attacker->nbOpponents() > 1)
                     {
-                        observer->mLayers->actionLayer()->reactToClick(a, card);
+                        observer->cardClick(card, MTGAbility::MTG_BLOCK_RULE);
                     }
                     else
                     {

projects/mtg/src/GameObserver.cpp

@@ -245,6 +245,8 @@ void GameObserver::nextCombatStep()
 
 void GameObserver::userRequestNextGamePhase()
 {
+    stringstream stream;
+    stream << "next " << currentGamePhase;
     if(getCurrentTargetChooser() && getCurrentTargetChooser()->maxtargets == 1000)
     {
         getCurrentTargetChooser()->done = true;
@@ -278,7 +280,9 @@ void GameObserver::userRequestNextGamePhase()
     {
        nextGamePhase(); 
     }
-    logAction(currentPlayer, "next");
+
+    stream << " " << currentGamePhase ;
+    logAction(currentPlayer, stream.str());
 }
 
 int GameObserver::forceShuffleLibraries()
@@ -406,7 +410,7 @@ void GameObserver::Update(float dt)
     Player * player = currentPlayer;
     if (Constants::MTG_PHASE_COMBATBLOCKERS == currentGamePhase && BLOCKERS == combatStep)
     {
-            player = player->opponent();
+        player = player->opponent();
     }
     if(getCurrentTargetChooser() && getCurrentTargetChooser()->Owner && player != getCurrentTargetChooser()->Owner)
     {
@@ -945,121 +949,174 @@ bool GameObserver::WaitForExtraPayment(MTGCardInstance * card)
     return result;
 }
 
+int GameObserver::cardClick(MTGCardInstance * card, MTGAbility *ability)
+{
+    MTGGameZone* zone = card->currentZone;
+    size_t index = card->currentZone->getIndex(card);
+    int result = ability->reactToClick(card);
+    logAction(card, zone, index, result);
+    return result;
+}
+
+int GameObserver::cardClick(MTGCardInstance * card, int abilityType)
+{
+    int result = 0;
+    MTGAbility * a = mLayers->actionLayer()->getAbility(abilityType);
+
+    if(a)
+    {
+        result = cardClick(card, a);
+    }
+
+    return result;
+}
+
 int GameObserver::cardClick(MTGCardInstance * card, Targetable * object)
 {
     Player * clickedPlayer = NULL;
+    int toReturn;
+    MTGGameZone* zone;
+    size_t index;
+    MTGCardInstance* backup;
+
     if (!card) {
     	clickedPlayer = ((Player *) object);
         logAction(clickedPlayer);
     } else {
-        logAction(card);
+        backup = card;
+        zone = card->currentZone;
+        index = zone->getIndex(card);
     }
-    if (targetChooser)
+
-    {
+    do {
-        int result;
+        if (targetChooser)
-        if (card)
         {
-            if (card == cardWaitingForTargets)
+            int result;
+            if (card)
             {
-                int _result = targetChooser->ForceTargetListReady();
+                if (card == cardWaitingForTargets)
-                if(targetChooser->targetMin && int(targetChooser->targets.size()) < targetChooser->maxtargets)
-                    _result = 0;
-                if (_result)
                 {
-                    result = TARGET_OK_FULL;
+                    int _result = targetChooser->ForceTargetListReady();
+                    if(targetChooser->targetMin && int(targetChooser->targets.size()) < targetChooser->maxtargets)
+                        _result = 0;
+                    if (_result)
+                    {
+                        result = TARGET_OK_FULL;
+                    }
+                    else
+                    {
+                        result = targetChooser->targetsReadyCheck();
+                    }
                 }
                 else
                 {
-                    result = targetChooser->targetsReadyCheck();
+                    result = targetChooser->toggleTarget(card);
+                    WEvent * e = NEW WEventTarget(card,cardWaitingForTargets);
+                    receiveEvent(e);
                 }
             }
             else
             {
-                result = targetChooser->toggleTarget(card);
+                result = targetChooser->toggleTarget(clickedPlayer);
-                WEvent * e = NEW WEventTarget(card,cardWaitingForTargets);
+            }
-                receiveEvent(e);
+            if (result == TARGET_OK_FULL)
+                card = cardWaitingForTargets;
+            else {
+                toReturn = 1;
+                break;
+            }
+        }
+
+        if (WaitForExtraPayment(card)) {
+            toReturn = 1;
+            break;
+        }
+
+        int reaction = 0;
+
+        if (ORDER == combatStep)
+        {
+            //TODO it is possible at this point that card is NULL. if so, what do we return since card->defenser would result in a crash?
+            card->defenser->raiseBlockerRankOrder(card);
+            toReturn = 1;
+            break;
+        }
+
+        if (card)
+        {
+            //card played as normal, alternative cost, buyback, flashback, retrace.
+
+            //the variable "paymenttype = int" only serves one purpose, to tell this bug fix what menu item you clicked on...
+            // all alternative cost or play methods suffered from the fix because if the card contained "target="
+            // it would automatically force the play method to putinplayrule...even charge you the original mana cost.
+
+            /* Fix for Issue http://code.google.com/p/wagic/issues/detail?id=270
+             put into play is hopefully the only ability causing that kind of trouble
+             If the same kind of issue occurs with other abilities, let's think of a cleaner solution
+             */
+            if (targetChooser)
+            {
+                MTGAbility * a = mLayers->actionLayer()->getAbility(card->paymenttype);
+                toReturn = a->reactToClick(card);
+                break;
+            }
+
+            reaction = mLayers->actionLayer()->isReactingToClick(card);
+            if (reaction == -1) {
+                toReturn = mLayers->actionLayer()->reactToClick(card);
+                break;
             }
         }
         else
-        {
+        {//this handles abilities on a menu...not just when card is being played
-            result = targetChooser->toggleTarget(clickedPlayer);
+            reaction = mLayers->actionLayer()->isReactingToTargetClick(object);
-        }
+            if (reaction == -1) {
-        if (result == TARGET_OK_FULL)
+                toReturn = mLayers->actionLayer()->reactToTargetClick(object);
-            card = cardWaitingForTargets;
+                break;
-        else
+            }
-            return 1;
-    }
-
-    if (WaitForExtraPayment(card))
-        return 1;
-
-    int reaction = 0;
-
-    if (ORDER == combatStep)
-    {
-        //TODO it is possible at this point that card is NULL. if so, what do we return since card->defenser would result in a crash?
-        card->defenser->raiseBlockerRankOrder(card);
-        return 1;
-    }
-
-    if (card)
-    {
-        //card played as normal, alternative cost, buyback, flashback, retrace.
-
-        //the variable "paymenttype = int" only serves one purpose, to tell this bug fix what menu item you clicked on...
-        // all alternative cost or play methods suffered from the fix because if the card contained "target=" 
-        // it would automatically force the play method to putinplayrule...even charge you the original mana cost.
-
-        /* Fix for Issue http://code.google.com/p/wagic/issues/detail?id=270
-         put into play is hopefully the only ability causing that kind of trouble
-         If the same kind of issue occurs with other abilities, let's think of a cleaner solution
-         */
-        if (targetChooser)
-        {
-            MTGAbility * a = mLayers->actionLayer()->getAbility(card->paymenttype);
-            return a->reactToClick(card);
         }
 
-        reaction = mLayers->actionLayer()->isReactingToClick(card);
+        if (!card) {
-        if (reaction == -1)
+            toReturn = 0;
-            return mLayers->actionLayer()->reactToClick(card);
+            break;
-    }
-    else
-    {//this handles abilities on a menu...not just when card is being played
-        reaction = mLayers->actionLayer()->isReactingToTargetClick(object);
-        if (reaction == -1)
-            return mLayers->actionLayer()->reactToTargetClick(object);
-    }
-
-    if (!card)
-        return 0;
-
-    //Current player's hand
-    if (currentPlayer->game->hand->hasCard(card) && currentGamePhase == Constants::MTG_PHASE_CLEANUP
-		&& currentPlayer->game->hand->nb_cards > currentPlayer->handsize && currentPlayer->nomaxhandsize == false)
-    {
-        WEvent * e = NEW WEventCardDiscard(currentPlayer->game->hand->cards[0]);
-        receiveEvent(e);
-        currentPlayer->game->putInGraveyard(card);
-    }
-    else if (reaction)
-    {
-        if (reaction == 1)
-        {
-            return mLayers->actionLayer()->reactToClick(card);
         }
-        else
+
+        //Current player's hand
+        if (currentPlayer->game->hand->hasCard(card) && currentGamePhase == Constants::MTG_PHASE_CLEANUP
+                    && currentPlayer->game->hand->nb_cards > currentPlayer->handsize && currentPlayer->nomaxhandsize == false)
         {
-            mLayers->actionLayer()->setMenuObject(object);
+            WEvent * e = NEW WEventCardDiscard(currentPlayer->game->hand->cards[0]);
-            return 1;
+            receiveEvent(e);
+            currentPlayer->game->putInGraveyard(card);
         }
-    }
+        else if (reaction)
-    else if (card->isTapped() && card->controller() == currentPlayer)
+        {
-    {
+            if (reaction == 1)
-        return untap(card);
+            {
+                toReturn = mLayers->actionLayer()->reactToClick(card);
+                break;
+            }
+            else
+            {
+                mLayers->actionLayer()->setMenuObject(object);
+                toReturn = 1;
+                break;
+            }
+        }
+        else if (card->isTapped() && card->controller() == currentPlayer)
+        {
+            toReturn = untap(card);
+            break;
+        }
+    } while(0);
+
+    if (clickedPlayer) {
+        logAction(clickedPlayer);
+    } else {
+        logAction(backup, zone, index, toReturn);
     }
 
-    return 0;
+    return toReturn;
 }
 
 int GameObserver::untap(MTGCardInstance * card)
@@ -1337,22 +1394,21 @@ bool GameObserver::processActions(bool undo)
 {
     bool result = false;
 
-    list<string> copyList = actionsList;
+    loadingList = actionsList;
     actionsList.clear();
 
     mLoading = true;
-    list<string>::iterator ite;
     float counter = 0.0f;
 
     // To handle undo, we'll remove the last P1 action and all P2 actions after.
-    if(undo && copyList.size()) {
+    if(undo && loadingList.size()) {
-        while(copyList.back().find("p2") != string::npos)
+        while(loadingList.back().find("p2") != string::npos)
-            copyList.pop_back();
+            loadingList.pop_back();
-        copyList.pop_back();
+        loadingList.pop_back();
     }
-    for(ite = copyList.begin(); ite != copyList.end(); ite++)
+    for(loadingite = loadingList.begin(); loadingite != loadingList.end(); loadingite++)
     {
-        string s = *ite;
+        string s = *loadingite;
         Player* p = players[1];
         if (s.find("p1") != string::npos)
             p = players[0];
@@ -1370,7 +1426,8 @@ bool GameObserver::processActions(bool undo)
         if(zone) {
             size_t begin = s.find("[")+1;
             size_t size = s.find("]")-begin;
-            int index = atoi(s.substr(begin, size).c_str());
+            size_t index = atoi(s.substr(begin, size).c_str());
+            assert(index < zone->cards.size());
             cardClick(zone->cards[index], zone->cards[index]);
         } else if (s.find("yes") != string::npos) {
             mLayers->stackLayer()->setIsInterrupting(p);
@@ -1389,12 +1446,12 @@ bool GameObserver::processActions(bool undo)
             assert(0);
         }
 
-        // let's fake an update
+        for (int i = 0; i<10; i++)
-        Update(counter);
+        {
-        counter += 1.000f;
+            // let's fake an update
-        // or two
+            Update(counter);
-        Update(counter);
+            counter += 1.000f;
-        counter += 1.000f;
+        }
     }
 
     mLoading = false;
@@ -1414,14 +1471,26 @@ void GameObserver::logAction(Player* player, const string& s) {
             logAction("p2");
 }
 
-void GameObserver::logAction(MTGCardInstance* card, MTGGameZone* zone) {
+void GameObserver::logAction(MTGCardInstance* card, MTGGameZone* zone, size_t index, int result) {
     stringstream stream;
     if(zone == NULL) zone = card->currentZone;
     stream << "p" << ((card->controller()==players[0])?"1.":"2.")
-           << zone->getName()<< "[" << zone->getIndex(card) << "]";
+           << zone->getName()<< "[" << index << "] "
+           << result << card->getLCName();
     logAction(stream.str());
 }
 
+void GameObserver::logAction(const string& s)
+{
+    if(mLoading)
+    {
+        string toCheck = *loadingite;
+        if(toCheck != s)
+                assert(0);
+    }
+    actionsList.push_back(s);
+};
+
 bool GameObserver::undo()
 {
     stringstream stream;