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.
fixed the bug where the ai "attacks/blocks" its own attackers during the combat phase.
Strangely, creatures only die in this bug when there are two or more suffering from
summoning sickness.
If somebody can double check this code change to make sure that it is appropriate.
My playtesting indicates a success but I don't have the variety of player decks others have.
with this change, currentmana is now what it could make and if anything is in the pool then add whats in the pool as potential mana., if it could potentially make mana then THIS is its currentpool. as getpotentialMana already handles the rest.
i originally added the manapool at the end of potential as my first attempt to stop this behavior, but this either/or if statement really got in the way of producing the result i wanted, which is, if
Ai has a 3 swamps, and a dark ritual, and it cast darkritual.
in hand Ai has a 5 black drop.
in OLD set up Ai would see it has 3 mana, then it would see it had 2 mana from the untapped swamps. it would pass the phase thinking that it could not cast the creature.
NOW ai looks for how much it has in mana, it see 3 swamps, it cast dark ritual, it now see what it can create on the next call to findcards, it see 2 swamps + 3 mana in pool, it cast the 5 drop creature.
mind you that this does not fix Ai mindlessly casting dark ritual as we have no current system to tell AI "hey dont cast that card now its useless to do so" like the fancy system to tell it what abilities it should use on cards :) *maybe someday*
2nd bug fix,
commented out a peice of code that compared if power and toughness would be cancelled out by a new counter. exsample:
1/1 is cancelled by -1/-1...this is actually extremely incorrect,
if you "put a 1/1 on a creature"
then you "put a -1/-1" on the same one. it does NOT remove the 1/1 as per MTG rules. the counters all all treated as NEW objects on a card. so a creature that had both those abilities used on it should have BOTH a 1/1 and a -1/-1 counter.
commenting out the section of code corrected this probelm.
modular creature, phantom creatures,sunburst and many many more will now correctly be able to use their counters even if they recieved "cancelling" Counters
last was just an improvement, as i got overly frustraited tonight while fighting with a deck that slaps many different types of counters on cards, i was having a very hard time telling what exactly i was about to put on a creature.
so what i did to correct this is create a much better menuText return on aacounter class.
PLEASE let me know if i missed a case where a counter is not showing correct text.
The check to give the award was lost in some of Jeck's reorg work (R1825), as ShopItem was deprecated & folded into GameStateShop. While at it, I took the liberty to delete ShopItem.h/cpp since they're not used.
The problem I found was specific to ManaProducer. I added an additional check when calling isReactingToClick(), if the cost has an extra cost, check if it can be paid. For this to work though, I had to change things around a little - there was a hack in the parsing code for ManaProducer abilities, where the cost wasn't being passed into the constructor. To compensate, the extra cost was being set during reactToClick, but this is too late, as isReactingToClick is called first. Now, where the hack occurs, after we construct the ManaProducer, if there's a cost, make sure we set the extra cost action immediately.
This seems to work well for the Vivid Creek card case: once the counters are tapped out, the menu system doesn't add the extra card abilities anymore, and you simply get the land tap without the menu popping up. I'm not seeing any adverse effects so far with other mana producer cards, but I'd appreciate a sanity check from other people here in case there's some other fallout I'm not seeing from this change.
I've patched the problem so that ActionLayer checks for a valid ID before doing anything, and does a no-op for IDs it doesn't know how to handle. However, this only fixes the problem during gameplay. It's quite possible that hitting the triangle button in other places in the app might equally cause a crash.
Issue: 544
fixed a Ai related bug, taught Ai not to mill itself to death basically. played a few matches which Ai was just destroying himself with a creature that allowed him to draw cards for each(whatever) in play. Ai will be a little more careful not to kill himself by Mill, also not to draw 30 cards in a turn when it clearly cant play them.
the tag "other " vs tag " other"
using "other " in target choosers will always work, however more often then not using " other" at the end of a line would produce weird effects, this was brought to my attention in a bug report by KF1, i moved all the " other" from outside target chooser into the actual target lines, as this always produces correct results in code, i imagine the tag at end of line was added before "other " was intruduced into targetchooser.
i will leave support for using " other" in code, however, i ask that we try not to use this code unless for some strange reason the targetchooser "other" doesnt take to the code.
stranger was that i noticed alot of mixed code, as in lord(other blahblah) 1/1 and this version with other tag at end, it was very inconsistent aside from causing weird weird results in game which i was actually able to confirm.
please update your primitive!!! this was ALOT of work.
correct syntax for "other "
lord(other blah)
all(other blah,blah,blah)
damage:1 target(other blah)
target(other *|somewhere)
notice its always first followed by a space. you only need one instence "other " in a targetchooser.
this also fixes the spamming of Abilities like Ai atempting to untap something more then once, or Ai uses regenerate ability multiple times on the same creature.
there will be no noticible difference in Ai game play except now it actually doesnt do the above mentioned bugs.
since the creature is going to die anyways, might as well send it in and see if it can kill something.
and yes i consider ai being THAT stupid a bug :P
- Fix a bug where AI would not check for the NULL status of some variables before calling functions on them. This would cause crashes that could seem random ingame. Possibly fixes issue 541
- fix for issue 539 . It is now possible to provide several possible paths in Res.txt, the game will use the first one that matches. If none of them works, it reverts to the oldschool "Res" folder
While debugging this, I noticed a separate issue: when changing profiles, we'd actually call refresh twice. Removed the spurious call, as reloading profiles doesn't need to concern itself with the image cache - that's already covered by the game options menu.
Also did some minor formatting / cleanup in the JGfx code for PSP - stubbed out a bunch of JLOG calls I had put in while debugging the PNG loading code.
