At Z's request, fixed Issue 184, ExtraCost types with targets such as Siege-Gang Commander/Drowner of secrets combined with targeted abilities bug.

This one was a bit of a doozy to fix correctly, but the actual fix ended up being fairly simple - the upshot is that TargetAbility never checked for whether an extra cost needed setting prior doing a target selection.  While at it, I discovered and fixed another bug:  if you're in the middle of an extra cost choice (like sacrifice, for instance) and hit the next phase button, the game would let you proceed, and then hang in an endless loop.

While at it, did a little cleanup/refactoring around GameObserver's waitForExtraPayment - any time a bool has something that sounds like a verb, it probably deserves to be a function.  Now it is. (I needed to refactor it anyway, as I reused that code for the next phase hang.)

Note that after this fix, I had to patch two test cases (siege_gang_commander.txt & seismic_assault.txt) - since I've change the selection order (ie a target ability with a sacrifice cost requires the cost to be paid up front before picking the target), this means that tests involving targeting & sacrifices need to switch the order of the cards to pass.
This commit is contained in:
wrenczes@gmail.com
2010-12-06 06:49:36 +00:00
parent 4de3b4a3c0
commit ffd3b7b074
7 changed files with 53 additions and 32 deletions

View File

@@ -8,8 +8,8 @@ hand:129754
inplay:1250
[DO]
129884
1250
129754
1250
[ASSERT]
FIRSTMAIN
[PLAYER1]

View File

@@ -4,16 +4,16 @@
FIRSTMAIN
[PLAYER1]
inplay:130539,130378,129578
manapool:{2}{R}{R}
manapool:{R}{R}{R}{R}
[PLAYER2]
inplay:129586,129600
[DO]
130539
129586
129578
129586
130539
129600
130378
129600
[ASSERT]
FIRSTMAIN
[PLAYER1]

View File

@@ -30,6 +30,7 @@ class GameObserver{
int nbPlayers;
int untap(MTGCardInstance * card);
bool WaitForExtraPayment(MTGCardInstance* card);
public:
int currentPlayerId;
@@ -40,7 +41,7 @@ class GameObserver{
PhaseRing * phaseRing;
int cancelCurrentAction();
int currentGamePhase;
ExtraCosts * waitForExtraPayment;
ExtraCosts * mExtraPayment;
int oldGamePhase;
TargetChooser * targetChooser;
DuelLayers * mLayers;

View File

@@ -64,9 +64,9 @@ int ActionLayer::reactToTargetClick(ActionElement* ability, Targetable * card)
bool ActionLayer::CheckUserInput(JButton key)
{
GameObserver * g = GameObserver::GetInstance();
if (g->waitForExtraPayment && key == JGE_BTN_SEC)
if (g->mExtraPayment && key == JGE_BTN_SEC)
{
g->waitForExtraPayment = NULL;
g->mExtraPayment = NULL;
return 1;
}
if (menuObject)

View File

@@ -42,7 +42,7 @@ GameObserver::GameObserver(Player * _players[], int _nb_players)
currentGamePhase = -1;
targetChooser = NULL;
cardWaitingForTargets = NULL;
waitForExtraPayment = NULL;
mExtraPayment = NULL;
gameOver = NULL;
phaseRing = NULL;
replacementEffects = NEW ReplacementEffects();
@@ -201,6 +201,12 @@ void GameObserver::userRequestNextGamePhase()
if (getCurrentTargetChooser())
return;
// Wil 12/5/10: additional check, not quite understanding why TargetChooser doesn't seem active at this point.
// If we deem that an extra cost payment needs to be made, don't allow the next game phase to proceed.
// Here's what I find weird - if the extra cost is something like a sacrifice, doesn't that imply a TargetChooser?
if (WaitForExtraPayment(NULL))
return;
bool executeNextPhaseImmediately = true;
Phase * cPhaseOld = phaseRing->getCurrentPhase();
@@ -445,8 +451,8 @@ void GameObserver::Render()
mLayers->Render();
if (targetChooser || mLayers->actionLayer()->isWaitingForAnswer())
JRenderer::GetInstance()->DrawRect(0, 0, SCREEN_WIDTH, SCREEN_HEIGHT, ARGB(255,255,0,0));
if (waitForExtraPayment)
waitForExtraPayment->Render();
if (mExtraPayment)
mExtraPayment->Render();
for (int i = 0; i < nbPlayers; ++i)
{
players[i]->Render();
@@ -523,6 +529,26 @@ void GameObserver::stackObjectClicked(Interruptible * action)
}
}
bool GameObserver::WaitForExtraPayment(MTGCardInstance * card)
{
bool result = false;
if (mExtraPayment)
{
if (card)
{
mExtraPayment->tryToSetPayment(card);
}
if (mExtraPayment->isPaymentSet())
{
mLayers->actionLayer()->reactToClick(mExtraPayment->action, mExtraPayment->source);
mExtraPayment = NULL;
}
result = true;
}
return result;
}
int GameObserver::cardClick(MTGCardInstance * card, Targetable * object)
{
Player * clickedPlayer = NULL;
@@ -560,21 +586,10 @@ int GameObserver::cardClick(MTGCardInstance * card, Targetable * object)
return 1;
}
if (waitForExtraPayment)
{
if (card)
{
waitForExtraPayment->tryToSetPayment(card);
}
if (waitForExtraPayment->isPaymentSet())
{
mLayers->actionLayer()->reactToClick(waitForExtraPayment->action, waitForExtraPayment->source);
waitForExtraPayment = NULL;
}
if (WaitForExtraPayment(card))
return 1;
}
int reaction = 0;
int reaction = 0;
if (ORDER == combatStep)
{

View File

@@ -3036,7 +3036,6 @@ int ActivatedAbility::isReactingToClick(MTGCardInstance * card, ManaCost * mana)
int ActivatedAbility::reactToClick(MTGCardInstance * card)
{
// if (cost) cost->setExtraCostsAction(this, card);
if (!isReactingToClick(card))
return 0;
Player * player = game->currentlyActing();
@@ -3044,7 +3043,7 @@ int ActivatedAbility::reactToClick(MTGCardInstance * card)
{
if (!cost->isExtraPaymentSet())
{
game->waitForExtraPayment = cost->extraCosts;
game->mExtraPayment = cost->extraCosts;
return 0;
}
ManaCost * previousManaPool = NEW ManaCost(player->getManaPool());
@@ -3073,7 +3072,7 @@ int ActivatedAbility::reactToTargetClick(Targetable * object)
cost->setExtraCostsAction(this, (MTGCardInstance *) object);
if (!cost->isExtraPaymentSet())
{
game->waitForExtraPayment = cost->extraCosts;
game->mExtraPayment = cost->extraCosts;
return 0;
}
ManaCost * previousManaPool = NEW ManaCost(player->getManaPool());
@@ -3141,6 +3140,12 @@ int TargetAbility::reactToClick(MTGCardInstance * card)
{
if (isReactingToClick(card))
{
if (cost && !cost->isExtraPaymentSet())
{
game->mExtraPayment = cost->extraCosts;
return 0;
}
waitingForAnswer = 1;
game->mLayers->actionLayer()->setCurrentWaitingAction(this);
tc->initTargets();
@@ -3680,7 +3685,7 @@ int AManaProducer::reactToClick(MTGCardInstance * _card)
cost->setExtraCostsAction(this, _card);
if (!cost->isExtraPaymentSet())
{
GameObserver::GetInstance()->waitForExtraPayment = cost->extraCosts;
GameObserver::GetInstance()->mExtraPayment = cost->extraCosts;
return 0;
}
GameObserver::GetInstance()->currentlyActing()->getManaPool()->pay(cost);

View File

@@ -132,7 +132,7 @@ int MTGPutInPlayRule::reactToClick(MTGCardInstance * card)
else
{
cost->setExtraCostsAction(this, card);
game->waitForExtraPayment = cost->extraCosts;
game->mExtraPayment = cost->extraCosts;
return 0;
}
@@ -316,7 +316,7 @@ int MTGAlternativeCostRule::reactToClick(MTGCardInstance * card)
else
{
cost->alternative->setExtraCostsAction(this, card);
game->waitForExtraPayment = cost->alternative->extraCosts;
game->mExtraPayment = cost->alternative->extraCosts;
card->paymenttype = MTGAbility::ALTERNATIVE_COST;
return 0;
}
@@ -505,7 +505,7 @@ int MTGBuyBackRule::reactToClick(MTGCardInstance * card)
else
{
cost->BuyBack->setExtraCostsAction(this, card);
game->waitForExtraPayment = cost->BuyBack->extraCosts;
game->mExtraPayment = cost->BuyBack->extraCosts;
card->paymenttype = MTGAbility::BUYBACK_COST;
return 0;
}
@@ -689,7 +689,7 @@ int MTGFlashBackRule::reactToClick(MTGCardInstance * card)
else
{
cost->FlashBack->setExtraCostsAction(this, card);
game->waitForExtraPayment = cost->FlashBack->extraCosts;
game->mExtraPayment = cost->FlashBack->extraCosts;
card->paymenttype = MTGAbility::FLASHBACK_COST;
return 0;
}
@@ -873,7 +873,7 @@ int MTGRetraceRule::reactToClick(MTGCardInstance * card)
else
{
cost->Retrace->setExtraCostsAction(this, card);
game->waitForExtraPayment = cost->Retrace->extraCosts;
game->mExtraPayment = cost->Retrace->extraCosts;
card->paymenttype = MTGAbility::RETRACE_COST;;
return 0;
}