Fix for Flowstone Giant suicide

[Rob Cashwalker]

BTW, keep in mind with all that code above, not only is it untested.. it was copied from the original code, and then modified in the message text box... so there may be syntax errors.

[GandoTheBard]

Perhaps what is needed to avoid incorrect activations from your hand etc, is to not attach unnecessary abilities to cards not in appropriate areas. Cycling obviously should only trigger in the owner's hand. Dredge should only trigger in the graveyard. "manacost: do something" should only occur in the play area or in the graveyard (assuming the clauses are satisfied) etal.

[DennisBergkamp]

The parse works now (with the original syntax in cards.txt: PTPump U:+1/+0).
I also added in the hack in the canPlay() method, which fixes things, I can't play the ability from my hand anymore.

However, there's still trouble with the increment check, if I use:

Code: Select all

Input abCheck = new Input()
      {
        public void showMessage()
        {
          ability.getSourceCard().abilityUsed++;
          if (!CardFactoryUtil.canUseAbility(ability.getSourceCard().card))
          {
              AllZone.Display.showMessage("This ability can't be used.");
              ButtonUtil.enableOnlyCancel();
           }
        }
        public void selectButtonCancel() {stop();}
        }
      };

      ability.setBeforePayMana(abCheck);

I get a bunch of syntax errors, so I change it to:

Code: Select all

Input abCheck = new Input()
          {
            public void showMessage()
            {
              card.abilityUsed++;
              if (!CardFactoryUtil.canUseAbility(card))
              {
                  AllZone.Display.showMessage("This ability can't be used.");
                  ButtonUtil.enableOnlyCancel();
               }
             
            }
            public void selectButtonCancel() {stop();}
          };
         
          ability.setBeforePayMana(abCheck);

to make the compiler happy (but I'm not sure my logic is correct here, there shouldn't be a difference between "card" and "ability.getSourceCard()" though ?)
Anyway, it still gives me trouble because I just get "This ability can't be used." every time

[jpb]

Can we make Card.abilityUsed and card.abilityTurnUsed only available via getters and setters.

[Rob Cashwalker]

(but I'm not sure my logic is correct here, there shouldn't be a difference between "card" and "ability.getSourceCard()" though ?)
Anyway, it still gives me trouble because I just get "This ability can't be used." every time

I've got no ideas. I was hoping it was just a quick way to make a pseudo-"phase" stop. There may not be any way to actually prevent the human from doing this... SO, either we leave the increment in resolve(), and use the honor system with the human, or we leave turn-limited pump creatures out of the implementation.

[DennisBergkamp]

Hmm, I've looked into this some more. First of all, the reason I was able to play the ability of Azimaet Drake twice was because:

Code: Select all

return card.abilityUsed <= check.intValue();

Which should really be:

Code: Select all

return card.abilityUsed < check.intValue();

And I added a nasty hack in canUseAbility, at the end:

Code: Select all

    .... 
 
    SpellAbility sa;
    //this is a hack, check the stack to see if this card has an ability on the stack
    //if so, we can't use the ability: this is to prevent using a limited ability too many times
    for (int i=0; i<AllZone.Stack.size(); i++)
    {
       sa = AllZone.Stack.peek(i);
       if (sa.getSourceCard().equals(card))
             return false;
    }
   
    Integer check = (Integer) AbilityLimits[found][1];
    return card.abilityUsed < check.intValue();
  }//canUseAbility(Card card)

There's one small problem with this, you can't pump up your creature when that creature has an ability currently on the stack, you have to let that resolve first, then the creature can be pumped again. This limitation only affects creatures that have a limit on their pump abilities.

I've tested this with Azimaet Drake and also Vampire Bats, and it seems to work perfectly! I know the code's not pretty, but I think it's better than what we had before. So I think I'm planning on including this code for the next Beta version. What do you think, Rob?

[DennisBergkamp]

Can we make Card.abilityUsed and card.abilityTurnUsed only available via getters and setters.

Yeah, good point, I will make them private, and add public getters + setters.

[Rob Cashwalker]

There's one small problem with this, you can't pump up your creature when that creature has an ability currently on the stack, you have to let that resolve first, then the creature can be pumped again. This limitation only affects creatures that have a limit on their pump abilities.

I've tested this with Azimaet Drake and also Vampire Bats, and it seems to work perfectly! I know the code's not pretty, but I think it's better than what we had before. So I think I'm planning on including this code for the next Beta version. What do you think, Rob?

I think what you're trying to say is that if there are any other effects on the stack with the same source card, then this will not work. For example, a use-limited pump and a regenerate. If the regenerate is on the stack, you couldn't use the pump ability. And this would affect any card with the pump keywords, because the canPlay is executed for all cards. Maybe another function like "HasAbilityLimit(card)" which just looks in the list to see if it's a use-limited card, and then check the stack if it is.

I think this is a fine solution. It affects the computer and the human, with a single set of code, which is good.

Someone requested that I modify the KPump to allow keyword removal... but it might be more difficult than its worth.... Might be better off making a KLose keyword... or what's the opposite of pump? KSuck? ... Then again, there's a card that pumps itself and loses defender... so maybe its better to just deal with a minus sign in front of the keyword, so that it's all uniform.... Comments?

Add it as is for now... maybe don't release it until we work on this keyword removal part.

[DennisBergkamp]

Rob,

No currently it will only affect cards that have the ability limit, because:

Code: Select all

public static boolean canUseAbility(Card card)
  {
    int found = -1;

    //try to find card name in AbilityLimits[][]
    for(int i = 0; i < AbilityLimits.length; i++)
      if(AbilityLimits[i][0].equals(card.getName()))
        found = i;

    if(found == -1)
      return true;

    //card was found
    if(card.getAbilityTurnUsed() != AllZone.Phase.getTurn())
    {
      card.setAbilityTurnUsed(AllZone.Phase.getTurn());
      card.setAbilityUsed(0);
    }
    SpellAbility sa;
    //this is a hack, check the stack to see if this card has an ability on the stack
    //if so, we can't use the ability: this is to prevent using a limited ability too many times
    for (int i=0; i<AllZone.Stack.size(); i++)
    {
       sa = AllZone.Stack.peek(i);
       if (sa.getSourceCard().equals(card))
             return false;
    }
   
    Integer check = (Integer) AbilityLimits[found][1];
    return card.getAbilityUsed() < check.intValue();
  }//canUseAbility(Card card)


If it doesn't find a card with an ability limit, it will just return true, and it won't ever get to the part with the nasty hack:

Code: Select all

    if(found == -1)
      return true;

I'm not sure what you mean about keyword removal for KPump, is this for creatures that have the ability, to say, lose flying until end of turn?

[Rob Cashwalker]

Yes. Though I think a more prevalent ability has been to lose defender.... All this pump stuff sets precedent for keywording targeted pump, which should include the flexibility for anti-pump... or "target creature loses flying" and other such stuff. I see you've released it all now, so I'll try to work on it with the current source.

Edit...
On second thought, a little bit of searching hasn't shown up more than a couple cards where this interpretation will be necessary, except for the next step - targeted pump. So forget it.

Look at a few of the (very few) interesting targeted anti-pumps:
Downdraft looks promising... a very simple test subject. Ignoring the sac ability for now... although it looks easy, and it's an easy AI - duh, if human has more flyers than computer has, then shock them all!
Radjan Spirit becomes childs play, even a computer can do it!
Walking Sponge ... now there's the poster child of removing keywords!

The thing is, I swear I remember reading a card along the lines of "1W: CARDNAME gets +1/+2 and loses defender until EOT." ANd I can't find it now as an example. And I thought such abilities might have been more common.

Bah.. it's too late to be thinking this hard...

[DennisBergkamp]

It just looks like there's not that many:

Grozoth
Loyal Gyrfalcon
Tidewater Minion
Canopy Dragon
Phyrexian Splicer

to name a few more.

[Almost_Clever]

...The thing is, I swear I remember reading a card along the lines of "1W: CARDNAME gets +1/+2 and loses defender until EOT." ANd I can't find it now as an example. And I thought such abilities might have been more common.

Bah.. it's too late to be thinking this hard...

There are a few like Mobile Fort (http://beta.gatherer.wizards.com/Pages/ ... rseid=5550), Walking Wall (http://beta.gatherer.wizards.com/Pages/ ... rseid=2432), and Wall of Wonder (http://beta.gatherer.wizards.com/Pages/ ... seid=15797). Then there are a few other walls that can conditionally attack (Dark Maze, Mirror Wall, Slithering Shade, Wakestone Gargoyle) without the associated P/T adjustments. Prison Barricade can attack if its kicker cost was paid, and Vodalian War Machine requires tapping untapped merfolk to be able to attack (and change its P/T).

[Rob Cashwalker]

I think I figured out why it would allow the activation from the hand. The ability declaration I was using might be wrong.

This is what is there now.

Code: Select all

SpellAbility ability = new Ability_Activated(card, manaCost)


Which is different then Furnace Whelp:

Code: Select all

SpellAbility ability = new Ability(card, "R")


But I think they should be defined the same, and I recall using one of the other dragons as a model.

So I notice while I'm working on a creature-based targeted pump keyword, that looking at Giant Growth, it uses

Code: Select all

SpellAbility spell = new Spell(card)


And my model for creature-based targeted pump is Timberwatch Elf, which defines the ability like this:

Code: Select all

final Ability_Tap ability = new Ability_Tap(card)


The difference with Giant Growth and Timberwatch Elf (other than the whole number of elves thing) is that Giant Growth is played from the hand as a SPELL, and Timberwatch Elves is played as a Tap ability. I'm curious if changing the Pump spells to

Code: Select all

final Ability_Activated ability = new Ability_Activated(card, manacost)


would solve anything....

Now, this also highlights whether or not there's a need for this final keyword.... It makes some sense to me, like in the case of the variables, like manacost - once the value is set, that's it, no turning back. I kinda get it with these ad-hoc class creations... but when is it necessary, and when is it not?

So, recall that Forge mentioned that spells and abilities are very closely related in design and function, however they aren't interchangeable. As well, each of the Ability variants has two constructors, one which takes just a card (no mana cost) and another which takes a mana cost. So I was looking at that for a bit.

Code: Select all

public Ability_Activated(Card sourceCard)
    {
   this(sourceCard, "");
    }
    public Ability_Activated(Card sourceCard, String manaCost)
    {
   super(SpellAbility.Ability, sourceCard);
   setManaCost(manaCost);
    }   


Notice if no mana cost is desired, then don't specify "0", use "" (empty string).

Code: Select all

    public Ability_Hand(Card sourceCard)
    {
   this(sourceCard, "");
    }
    public Ability_Hand(Card sourceCard, String manaCost)
    {
   super(SpellAbility.Ability, sourceCard);
   setManaCost(manaCost);
    }


Same basic stuff right?

Code: Select all

    public Ability_Tap(Card sourceCard)
    {
   this(sourceCard, "0");
    }
    public Ability_Tap(Card sourceCard, String manaCost)
    {
   super(SpellAbility.Ability, sourceCard);
   setManaCost(manaCost);
    }


I remember a bug was posted about being requested to pay 0 mana.... I wonder if this was why.
Edit - Nevermind, the 0 mana was actually the pump cost. I guess we should throw a line into the parser if it encounters a "0" cost, to convert it to an empty string.

[DennisBergkamp]

Rob,

Interesting, I'll try to change the Ability type part, to see if that prevents playing it from hand.

Yes, unfortunately "" vs "0" doesn't solve the 0 mana "bug". It's present in a lot of cards now (Necropotence, a lot of those Fungi, Aluren, etc.).

[mtgrares]

I think I figured out why it would allow the activation from the hand. The ability declaration I was using might be wrong.

SpellAbility.canPlay() returns true if the ability can be played while the card is in the current zone. For example, the Card object for Elvish Piper holds two SpellAbility objects. One is the normal "summon creature" spell and the other is its activated ability. The SpellAbility object representing "summon creature" canPlay() returns true when the Card object is in hand, the SpellAbility for the activated ability returns true only when the Card object is in play. This prevents most abilities from being played from your hand, cycling is an exception since it is an ability that you play while the card is in your hand.

For some code lets see Ability_Hand which is used for abilities that you can play while the card is in your hand. Note that this class "implements java.io.Serializable" is old and can be removed.

Code: Select all

public boolean canPlay()
{
  PlayerZone zone = AllZone.getZone(getSourceCard());
  return zone.is(Constant.Zone.Hand, getSourceCard().getController());
}

Spell_Permanent is used for any permanent like creatures and enchantments.
Ability_Activated is used for activated abilities.
Ability_Tap is used for activated abilities that tap the card.
Ability is used for any other activated ability.

Note that all of these classes are subclasses of SpellAbility and basically just implement the appropriate canPlay() method. All of these classes are very short.