CardFactory.java

[Chris H.]

The following quote is fairly important and I decided to split it off into it's own forum topic.

In message:

Re: spPumpTgt - Targeted Pump Spells

The trick with multiple keywords, like spPumpTgt and Cycling is that the order of processing in CardFactory.java needs to be changed. The permanent ability keywords need to be first. Then the spell keywords need to be second. Then the "tack-on" keywords need to be processed last, possibly at the very end of CardFactory, allowing for "tack-ons" to be used for explicitly coded cards.

Cantrip isn't implemented like the others. It's checked upon resolution of any spell - if the card contains the keyword, then it just draws a card. Quite separately from any other card code.

The tricky part I've been trying to think about lately, is the Charms or Commands. Each individual ability of Chaos Charm for example, is something that we have keywords for. But we can't just do it like this:

Chaos Charm
R
Instant
Choose one - Destroy target Wall; or Chaos Charm deals 1 damage to target creature; or target creature gains haste until end of turn.
spDestroyTgt:Creature.Wall
spDamageC:1 (OK, we don't have creature-only damage yet, but it's easy enough)
spPumpTgt:Haste

Because each of the sp___ code blocks clears out the spell abilities of the card. (because all cards are assumed to be permanents, and given a default spell ability to be put into play).
So it would process spDestroyTgt (never mind the fact that the code needs the text formatted a certain way in order to extract the input dialog text) removing the default ability.
Then it would process the spDamageC ability, removing the spDestroyTgt abilty.
Then it would process the spPumpTgt ability, removing the spDamageC ability.
And that's just a rough example, because that's not the order of code blocks... spDamageC would be first, up by the spDamageCP and spDamageP blocks; then spDestroyTgt, and spPumpTgt would probably be inserted last when Dennis merges it.

I think the best way to handle this may be something like the following:

Code: Select all

public boolean hasSpKeyword()
{
  ArrayList k = card.getKeywords();
  for (i = 0; i < k.length; i++)
    if (k(i).startsWith("sp")
      return true;
  return false;
}


Code: Select all

if (hasSpKeyword)
{
   card.clearSpellAbility();

   if (shouldSpDamageCP)
   {
      ...
   }
   if (shouldSpDamageP)
   {
      ...
   }
   if (shouldSpDestroyTgt)
   {
      ...
   }
   if (shouldSpPumpTgt)
   {
      ...
   }
   ...
      etc
   ...
}


Then remove the clearSpellAbility() from each individual code block. Oh, and make sure each one has a spell.setDescription().

When you go to play the spell, the game will now present each spell option. This won't work for the Commands, which want two choices... but that's OK... for now.

[Chris H.]

I spent a little time looking at the new cards for Planechase. I found the spell Akroma's Vengeance and it is very close to the spell Jokulhaups. Initially, I was planning to do a copy + paste and then edit the second copy. I am still having problems figuring out how to set a new "private static final long serialVersionUID".

So I decided to edit the code for Jokulhaups so that it would cover both spells. Looking at the code I discovered that "||" represents OR in if statements. I guessed that "&" would represent AND.

The code changes worked fine in the test deck that I created. Except for the fact that Akroma's Vengeance has Cycling:3


Akroma's Vengeance
4 W W
Sorcery
Destroy all artifacts, creatures, and enchantments.
Cycling:3

Code: Select all

    //*************** START *********** START **************************
    if(cardName.equals("Jokulhaups") || cardName.equals("Akroma's Vengeance"))
    {
      final SpellAbility spell = new Spell(card)
      {
      private static final long serialVersionUID = -7384618531690849205L;

      public void resolve()
        {
          CardList all = new CardList();
          all.addAll(AllZone.Human_Play.getCards());
          all.addAll(AllZone.Computer_Play.getCards());

          for(int i = 0; i < all.size(); i++)
          {
            Card c = all.get(i);
           
            if(cardName.equals("Jokulhaups") & (c.isCreature() || c.isArtifact() || c.isLand()))
                AllZone.GameAction.destroyNoRegeneration(c);
             
            if(cardName.equals("Akroma's Vengeance") & (c.isCreature() || c.isArtifact() || c.isEnchantment()))
                  AllZone.GameAction.destroy(c);
          }
        }//resolve()
      };//SpellAbility
      card.clearSpellAbility();
      card.addSpellAbility(spell);
    }//*************** END ************ END **************************

[DennisBergkamp]

Ahh, awesome. I haven't started creating cards yet for the new set, but I think I'll try adding a few this weekend

By the way, the Cycling part probably won't work. I've had the same problem with Hush I think. So I had to add in the Cycling code separately, because a card.clearSpellAbility() gets called...

This should do the trick (check at the end) :

Code: Select all

    //*************** START *********** START **************************
    if(cardName.equals("Jokulhaups") || cardName.equals("Akroma's Vengeance"))
    {
      final SpellAbility spell = new Spell(card)
      {
      private static final long serialVersionUID = -7384618531690849205L;

      public void resolve()
        {
          CardList all = new CardList();
          all.addAll(AllZone.Human_Play.getCards());
          all.addAll(AllZone.Computer_Play.getCards());

          for(int i = 0; i < all.size(); i++)
          {
            Card c = all.get(i);
           
            if(cardName.equals("Jokulhaups") & (c.isCreature() || c.isArtifact() || c.isLand()))
                AllZone.GameAction.destroyNoRegeneration(c);
             
            if(cardName.equals("Akroma's Vengeance") & (c.isCreature() || c.isArtifact() || c.isEnchantment()))
                  AllZone.GameAction.destroy(c);
          }
        }//resolve()
      };//SpellAbility
      card.clearSpellAbility();
      card.addSpellAbility(spell);
      if (cardName.equals("Akroma's Vengeance")) //add cycling
               card.addSpellAbility(CardFactoryUtil.ability_cycle(card, "3"));
    }//*************** END ************ END **************************

By the way, for adding the serialVersionUID thing:
Eclipse will start complaining and give a warning whenever there's a serializable class that doesn't have a serialVersionUID. You'll see this through a yellow underline under the class declaration and a small yellow rectangle on the right hand side. Just hover over the yellow underline and select "Add generated serial version ID".

[Chris H.]

Wow, thank you.

I saw the "card.clearSpellAbility();" at the bottom and felt that it was probably removing the cycling keyword. I felt that there was a way to add it back in, just wasn't sure how to handle this situation.

I guess that I could search the list for cycle cards and I might find a few that can be added in this way. Some might require a change to cards.txt and I think that I can handle several keyword cards + cycle in a single section of code.

I was able to get the small yellow window to show up but I was unsure which of the choices should be made.

[mtgrares]

At the beginning of CardFactory for every card I called card.addSpellAbility(new Spell_Permanent()) to give the creatures in cards.txt a "default" spells, which summons a permanent such as an artifact, enchantment, land, planeswalker etc... So all instants and sorceries have to remove the permanent SpellAbility by calling card.clearSpellAbility().

Hopefully that explains things a little bit.

[Chris H.]

At the beginning of CardFactory for every card I called card.addSpellAbility(new Spell_Permanent()) to give the creatures in cards.txt a "default" spells, which summons a permanent such as an artifact, enchantment, land, planeswalker etc... So all instants and sorceries have to remove the permanent SpellAbility by calling card.clearSpellAbility().

Hopefully that explains things a little bit.

Thanks Rares.

That explains how in one of my tests a spell with Flashback ended up in play on the battlefield and in the graveyard/Flashback at the same time.



I would also like to comment on how editing the CardFactory.java file in Eclipse will cause my Macintosh to pause for a long time with a spinning beachball. And my computer is only a few months old and has a 3.06 GHz Intel Core 2 Duo.

The CardFactory.java file must be so large that Eclipse is having a hard time moving data around in memory.

At one time Rob and Dennis were considering splitting up the CardFactory.java file into smaller pieces. They ran into a few problems.

I wonder if Rares, Zeerker or Silly Freak have any ideas on how this could be handled. Splitting up the CardFactory.java file into smaller pieces could also include Rob's ideas that are quoted in the first message on this thread.

[Rob Cashwalker]

By the way, the Cycling part probably won't work. I've had the same problem with Hush I think. So I had to add in the Cycling code separately, because a card.clearSpellAbility() gets called...

This is what I was trying to point out in the quote:

The code that checks for cycling as a keyword needs to be moved to the end of either the keyword code block, or the end of CardFactory itself. Then it is added on AFTER any clearSpellAbility().

Same thing for Flashback and Unearth, I would guess.

The other point I made was that the keyword code block could start off with a check for the "sp" prefix added to spell keywords. If present, it clears the spellability BEFORE actually defining the execution code. (and the clearSpellAbility is removed from the code definition)
This would open up a simple way of choosing single mode charm spells.

[zerker2000]

And you cannot just have "default" code check for the card being a creature/land/artifact/enchantment/planeswalker/plane why?

[mtgrares]

And you cannot just have "default" code check for the card being a creature/land/artifact/enchantment/planeswalker/plane why?

That would work better. Different things makes sense at different times. I usually coded first and thought second. While this isn't a really great way to do things, at least I never got stuck anywhere. (And that also explains CardFactory, the world's longest method.)

p.s.
I did think long and hard about the Input class (which handles ALL of the mouse input) but it is very confusing to explain.

[zerker2000]

And you cannot just have "default" code check for the card being a creature/land/artifact/enchantment/planeswalker/plane why?

That would work better. Different things makes sense at different times. I usually coded first and thought second. While this isn't a really great way to do things, at least I never got stuck anywhere. (And that also explains CardFactory, the world's longest method.)

My point is that it should be a quick fix: edit the "default code" to include

Code: Select all

#buyback code#
boolean tofield=false;
boolean tograve=false;
String permanents = "Creature/Land/Artifact/Enchantment/Planeswalker/Plane"
for(String type: card.getTypes().split(" ");)
{
  if (permanents.contains(type)) tofield=true;
  if (type.equals("Instant")|| type.equals("Sorcery")) tograve=true;
}
if(tofield!^tograve) throw (#something heavy at developers#);
if(tofield) {#old code#}
if(tograve) {#old Instant / Sorcery casting code#}

, remove now unnecessary clearSpellAbilit-ies, spend about ten times that time debugging , and you're done.

p.s.
I did think long and hard about the Input class (which handles ALL of the mouse input) but it is very confusing to explain.

It seems OK to me... the whole targeting system could be tweaked to allow easier multi-target spells, but that is easy enough to do on a case-by-case basis (i.e. new Input[] example; example[0]= new Input{final target aquisition};<complicated but writeable Iterator code that defines example[next]'s recursively>) and otherwise, I haven't run into any other problems with Input.
EDIT: Btw something in eclipse died and I haven't had the time to reanimate it, so be sure to check all code I'll be posting in the near future for forge method name and trivial grammar errors.

[mtgrares]

It seems OK to me... the whole targeting system could be tweaked to allow easier multi-target spells.

Yeah, the current system presume that a card will only have one target, which works for many cards but definitely not all. This was an mistake that I have fixed in the 2.0 Input class. It has methods like addTargetCard(Card), countTargetCard() : int, and getTargetCard(int index) so many cards could be targeted easily instead of the "hacking" that is needed for cards like Hex, which can be done but they are messy and the computer can't play it.