Splitting CardFactory

[mtgrares]

I also asked huggybaby how to add another sub-forum like MTG Forge Decks so we could just discuss programming stuff. And I wanted to ask Dennis about how he stacked the lands. It works on the resizable screen but it doesn't work on the other non-resizable screen. If I can help out just ask me. I always use the non-resizable screen since I don't have a widescreen monitor.

I saw rob's post about how he and dennis are working on splitting up CardFactory. I don't mind looking at your code if you post it or if you send it to me (mtgrares yahoo com). Let me briefly try to tell you one way on how to split up CardFactory.

Code: Select all

interface MakeCard
{
  public Card getCard(Card fromCardFactory)
  {
    //cut and past from CardFactory here
  }
}

Then you build a HashMap like map.put("Wrath of God", new MakeCard_Wrath_of_God()), so CardFactory would look like this:

Code: Select all

class CardFactory
{
  private HashMap map = new HashMap();

  public CardFactory() {buildMap();}

  public Card getCard(String cardname, String player)
  {
    //build Card c from the top of CardFactory, which sets controller, owner and other stuff

    MakeCard m = map.get(cardname);
   return m.getCard(c);
  }
}

[Rob Cashwalker]

I'll let dennis respond with the last bit of code he had tried. But this is what I had sent him to start out with.

CardFactory: (just below the start of the getCard2() method, where all the cards get defined)

Code: Select all

Card ncard = CardFactory_Artifacts.getCard(card, cardName, owner);


CardFactory_Artifacts:

Code: Select all


class CardFactory_Artifacts
{
   public static Card getCard(final Card card, String cardName, String owner)
   {
       if(cardName.equals("AEther Vial"))
       {
          //final int[] converted = null;
          final Ability_Tap ability = new Ability_Tap(card, "0")
          {
            private static final long serialVersionUID = 1854859213307704018L;

            public boolean canPlay()
             {
                return card.getCounters(Counters.CHARGE) > 0;
             }
          
            public void resolve() {
               String player = card.getController();
               
               PlayerZone hand = AllZone.getZone(Constant.Zone.Hand, player);
               PlayerZone play = AllZone.getZone(Constant.Zone.Play, player);
               
               //converted[0] = card.getCounters(Counters.CHARGE);
               //System.out.println("converted: " + converted[0]);
               
               CardList list = new CardList(hand.getCards());
               list = list.filter(new CardListFilter()
               {
                  public boolean addCard(Card c) {
                     return CardUtil.getConvertedManaCost(c.getManaCost()) == card.getCounters(Counters.CHARGE) && c.isCreature();               
                  }
               });
               
               
               if (list.size()>0)
               {
                  if (player.equals(Constant.Player.Human))
                  {
                     Object o = AllZone.Display.getChoiceOptional("Pick creature to put into play", list.toArray());
                     if (o!=null)
                     {
                        Card c = (Card)o;
                        hand.remove(c);
                        play.add(c);
                     }
                  }
                  else
                  {
                     Card c = list.get(0);
                     if(AllZone.GameAction.isCardInZone(c, hand)) {
                        hand.remove(c);
                        play.add(c);
                     }
                  }
               }
            }
          };
          
          ability.setDescription("Tap: You may put a creature card with converted mana cost equal to the number of charge counters on Æther Vial from your hand into play.");
          ability.setStackDescription(card.getName() + " - put creature card with converted mana cost equal to the number of charge counters into play.");
          
          card.addSpellAbility(ability);
       }//*************** END ************ END **************************

       //*************** START *********** START **************************
       if(cardName.equals("The Hive"))
       {
         final SpellAbility ability = new Ability_Tap(card, "5")
         {
         private static final long serialVersionUID = -1091111822316858416L;

         public void resolve()
           {
             Card c = new Card();
             c.setName("C 1 1 Wasp");

             c.setOwner(card.getController());
             c.setController(card.getController());

             c.setManaCost("");
             c.setToken(true);

             c.addType("Artifact");
             c.addType("Creature");
             c.addType("Insect");

             c.setBaseAttack(1);
             c.setBaseDefense(1);
             c.addIntrinsicKeyword("Flying");

             PlayerZone play = AllZone.getZone(Constant.Zone.Play, card.getController());
             play.add(c);
           }//resolve()
         };
         ability.setDescription("5, tap: Put a 1/1 Insect artifact creature token with flying named Wasp into play.");
         ability.setStackDescription("The Hive - Put a 1/1 token with flying into play.");
         card.addSpellAbility(ability);
       }//*************** END ************ END **************************

       //*************** START *********** START **************************
       if(cardName.equals("That Which Was Taken"))
       {
         final SpellAbility ability = new Ability_Tap(card, "4")
         {
         private static final long serialVersionUID = -8996435083734446340L;
         public void resolve()
           {
             Card c = getTargetCard();

             if(AllZone.GameAction.isCardInPlay(c)  && CardFactoryUtil.canTarget(card, c) )
               c.addExtrinsicKeyword("Indestructible");
           }
           public boolean canPlayAI()
           {
             CardList creatures = getCreatures();

             for (int i = 0; i < creatures.size(); i++)
             {
               if (!creatures.get(i).getKeyword().contains("Indestructible"))
               {
               return true;
             }
             }

             return false;
           }
           public void chooseTargetAI()
           {
             //Card c = CardFactoryUtil.AI_getBestCreature(getCreatures());
             CardList a = getCreatures();
             CardListUtil.sortAttack(a);
             CardListUtil.sortFlying(a);

             Card c = null;

             for (int i = 0; i < a.size(); i++)
             {
               if (!a.get(i).getKeyword().contains("Indestructible"))
               {
               c = a.get(i);
               break;
             }
             }

             setTargetCard(c);
           }
           CardList getCreatures()
           {
             CardList list = new CardList();
             list.addAll(AllZone.Computer_Play.getCards());
             return list.getType("Creature");
           }
         };//SpellAbility

         Input target = new Input()
         {
         private static final long serialVersionUID = 137806881250205274L;
         public void showMessage()
           {
             AllZone.Display.showMessage("Select target permanent");
             ButtonUtil.enableOnlyCancel();
           }
           public void selectButtonCancel() {stop();}
           public void selectCard(Card c, PlayerZone zone)
           {
             if(zone.is(Constant.Zone.Play) && c != card)//cannot target self
             {
               ability.setTargetCard(c);
               stopSetNext(new Input_PayManaCost(ability));
             }
           }
         };//Input -- target

         ability.setBeforePayMana(target);
         ability.setDescription("4, tap: Tap a divinity counter on target permanent other than That Which Was Taken.");

         card.addSpellAbility(ability);
       }//*************** END ************ END **************************

       //*************** START *********** START **************************
       if(cardName.equals("Nevinyrral's Disk"))
       {
         SpellAbility summoningSpell = new Spell_Permanent(card)
         {
         private static final long serialVersionUID = -8859376851358601934L;

         public boolean canPlayAI()
           {
             boolean nevinyrralInPlay = false;

             CardList inPlay = new CardList();
             inPlay.addAll(AllZone.Computer_Play.getCards());
             for(int i=0; i<inPlay.size(); ++i)
             {
               if( inPlay.getCard(i).getName().equals("Nevinyrral's Disk"))
               {
                 nevinyrralInPlay = true;
               }
             }
             return ! nevinyrralInPlay && (0 < CardFactoryUtil.AI_getHumanCreature(card, false).size());
           }
         };
         card.clearSpellAbility();
         card.addSpellAbility(summoningSpell);

         card.setComesIntoPlay(new Command()
         {
         private static final long serialVersionUID = -2504426622672629123L;

         public void execute()
           {
             card.tap();
           }
         });
         final SpellAbility ability = new Ability_Tap(card, "1")
         {
         private static final long serialVersionUID = 4175577092552330100L;
         
         public void resolve()
           {
             CardList all = new CardList();
             all.addAll(AllZone.Human_Play.getCards());
             all.addAll(AllZone.Computer_Play.getCards());
             all = filter(all);

             for(int i = 0; i < all.size(); i++)
               AllZone.GameAction.destroy(all.get(i));
           }
           private CardList filter(CardList list)
           {
             return list.filter(new CardListFilter()
             {
               public boolean addCard(Card c)
               {
                 return c.isArtifact() || c.isCreature() || c.isEnchantment();
               }
             });
           }//filter()
           public boolean canPlayAI()
           {
             CardList human    = new CardList(AllZone.Human_Play.getCards());
             CardList computer = new CardList(AllZone.Computer_Play.getCards());

             human    = human.getType("Creature");
             computer = computer.getType("Creature");

             //the computer will at least destroy 2 more human creatures
             return computer.size() < human.size()-1 || AllZone.Computer_Life.getLife() < 7;
           }
         };//SpellAbility
         card.addSpellAbility(ability);
         ability.setDescription("1, tap: Destroy all artifacts, creatures, and enchantments.");
         ability.setStackDescription("Destroy all artifacts, creatures, and enchantments.");
       }//*************** END ************ END **************************

       //*************** START *********** START **************************
       else if(cardName.equals("Sensei's Divining Top"))
       {
         //ability2: Draw card, and put divining top on top of library
         final SpellAbility ability2 = new Ability_Tap(card, "0")
         {
         private static final long serialVersionUID = -2523015092351744208L;

         public void resolve()
           {
             String player = card.getController();
             String owner = card.getOwner();
            
             PlayerZone play =  AllZone.getZone(Constant.Zone.Play, player);
             PlayerZone lib =  AllZone.getZone(Constant.Zone.Library, owner);
                
             AllZone.GameAction.drawCard(player);
             play.remove(card);
             lib.add(card,0); //move divining top to top of library
             card.untap();
        
           }

           public boolean canPlayAI()
           {
             return false;
           }

           public boolean canPlay()
           {
              if (AllZone.getZone(card).is(Constant.Zone.Play))
                  return true;
               else
                  return false;
           }//canPlay()
         };//SpellAbility ability2

         ability2.setBeforePayMana(new Input()
         {
         private static final long serialVersionUID = -4773496833654414458L;
         @SuppressWarnings("unused") // check
         int check = -1;
            public void showMessage()
            {
                AllZone.Stack.push(ability2);
                stop();
            }//showMessage()
         });

        

         //ability (rearrange top 3 cards) :
         final SpellAbility ability1 = new Ability(card, "1")
         {
           public void resolve()
           {
              
              String player = card.getController();
              PlayerZone lib =  AllZone.getZone(Constant.Zone.Library, player);
              
              if (lib.size() < 3)
                 return;
              
              CardList topThree = new CardList();
              
              //show top 3 cards:
              topThree.add(lib.get(0));
              topThree.add(lib.get(1));
              topThree.add(lib.get(2));
              
              Object o = AllZone.Display.getChoiceOptional("Put on top: ", topThree.toArray());
              if(o != null)
               {
                 Card c1 = (Card)o;
                 topThree.remove(c1);
                 lib.remove(c1);
                 lib.add(c1,0);
               }
                
              o = AllZone.Display.getChoiceOptional("Put second from top: ", topThree.toArray());
              if(o != null)
               {
                 Card c2 = (Card)o;
                 topThree.remove(c2);
                 lib.remove(c2);
                 lib.add(c2,1);
               }
              o = AllZone.Display.getChoiceOptional("Put third from top: ", topThree.toArray());
              if(o != null)
               {
                 Card c3 = (Card)o;
                 topThree.remove(c3);
                 lib.remove(c3);
                 lib.add(c3,2);
               }
              
           }
           public boolean canPlayAI()
           {
              return false;
            
           }
           public boolean canPlay()
           {
             if (AllZone.getZone(card).is(Constant.Zone.Play))
                return true;
             else
                return false;
           }//canPlay()
         };//SpellAbility ability1


         ability1.setDescription("1: Look at the top three cards of your library, then put them back in any order.");
         ability1.setStackDescription("Sensei's Divining Top - rearrange top 3 cards");
         card.addSpellAbility(ability1);
         ability1.setBeforePayMana(new Input_PayManaCost(ability1));

         ability2.setDescription("tap: Draw a card, then put Sensei's Divining Top on top of its owner's library.");
         ability2.setStackDescription("Sensei's Divining Top - draw a card, then put back on owner's library");
         ability2.setBeforePayMana(new Input_NoCost_TapAbility((Ability_Tap) ability2));
         card.addSpellAbility(ability2);
        

       }
       //*************** END ************ END **************************

      return card;
   }
}


[mtgrares]

It looks good. You could return card inside of the if statement. I'm not sure it really matters either way.

Code: Select all

if(cardName.equals("AEther Vial"))
{
  //blah blah
  return card
}

And you just have to make sure that the card argument is initialized by CardFactory because CardFactory does some stuff at the beginning of getCard(), like convert some of the more complicated abilities in cards.txt into SpellAbility objects and other stuff like that.

Code: Select all

public static Card getCard(final Card card, String cardName, String owner)

[Rob Cashwalker]

Yep, we gave that consideration.... Again, Dennis needs to post what he has currently, since I sent him the above stuff, and then he worked on it for a few days.

[DennisBergkamp]

More for like an hour or two
I think I left CardFactory_Artifacts untouched. When I tried this, basically what happened is that the abilities work, but there's no text showing up for those abilities on the cards. Not sure what's going on exactly

In CardFactory - getCard2():

Code: Select all

final private Card getCard2(final String cardName, final String owner)
  {
    //o should be Card object
    Object o = map.get(cardName);
    if(o == null)
      throw new RuntimeException("CardFactory : getCard() invalid card name - " +cardName);

    Card temp;

    if (CardFactory_Artifacts.getCard(copyStats(o), cardName, owner).getSpellAbility().length > 0)
       temp = CardFactory_Artifacts.getCard(copyStats(o), cardName, owner);
    else
       temp = copyStats(o);
   
    final Card card = temp;
    card.setOwner(owner);
    card.setController(owner);
    //may have to change the spell
    //this is so permanents like creatures and artifacts have a "default" spell
    if(! card.isLand())
      card.addSpellAbility(new Spell_Permanent(card));


     //... etc.
   

[mtgrares]

When I tried this, basically what happened is that the abilities work, but there's no text showing up for those abilities on the cards. Not sure what's going on exactly.

In a way this is better than the reverse (the card text being right and the abilities not working). The "card detail" on the right uses Card.getText() - see CardDetailUtil.java - which uses SpellAbility.toString() which is set by SpellAbility.setDescription(String). I presume that the specific card that you are testing doesn't call SpellAbility.setDescription(String) in CardFactory_Artifacts.

And just in case you didn't know, SpellAbility.setStackDescription(String) is the message that is shown when the spell or ability is on the stack.

I hope that helps. And I don't read every post, so if you post a question and I don't answer it, feel free to send me a private message via this forum or e-mail me. I'm usually online twice a week (tues and friday).

[zerker2000]

Is it feasible to split off all "while(shouldKeyword)"s into CardFactoyKeywords.setKeywords(card), and all "shouldKeyword"s into CardFactoryKeywordUtil?

[Rob Cashwalker]

That's a good question. But I tend to think they would suffer the same error. Keyword scripted cards aren't significantly different in underlying implementation than the others. Something in the process of trying to branch the code broke the cards created in the branch.

Feel free to give it a shot though.

[zerker2000]

Why are all the "shouldKeyword" methods separate anyways? As far as I can see, almost all keywords written in the following form:

Code: Select all

    private final int hasKeyword(Card c, String keyword) {
        ArrayList<String> a = c.getKeyword();
        for (int i = 0; i < a.size(); i++)
        {
           if (a.get(i).toString().startsWith(keyword))
              return i;
        }
        return -1;
    }

If so, why is there a separate "should" method for each one? Shouldn't the while((shouldKeyword))'s just call the method for the particular string?

[DennisBergkamp]

This is a good question... I have no idea though, Rob might know the answer to this one.
But notice the shouldKeyword blocks are outside of the getCard2 method. Must be some reason for it.

[Rob Cashwalker]

I don't know either. But that's how Rares did it first, and I/we [all] just followed along.

It makes sense when you think of cards that may have a single ability twice but with different costs or numbers. I guess the code could be explicitly written out for each one, but we've already got enough nested if blocks.

While writing my dissertation on the enhanced damage spell I've been promising, I had another inspiration... don't worry, I'll post the damage stuff soon, I just want to actually test the code first.
I'm heading towards another vision... one where there's only one keyword handler, I shall call it "spTheMatrix" or "abTheMatrix". All actual code handlers are dynamically called functions, based on passing the parsed strings.
I dunno.

[zerker2000]

But notice the shouldKeyword blocks are outside of the getCard2 method. Must be some reason for it.

Erm no, I propose keeping a generic shouldKeyword block to replace all those that look exactly like it, and simply move the actual keyword strings inside getCard2. Also, and I'm sorry if I asked about this before, why do a lot of keyword blocks look like this:

Code: Select all

while(shouldKeyword(card) != -1)
    {
      int n = shouldKeyword(card);
      if(n != -1)
      {
        ...
      }
    }

instead of simply like this:

Code: Select all

while(shouldKeyword(card) != -1)
    {
      int n = shouldKeyword(card);
      ...
    }

or even this:

Code: Select all

int n;
n=shouldKeyword1(card)
while(n != -1)
    {
      ...
      n = shouldKeyword1(card);
    }
n=shouldKeyword2(card)

while(n != -1)
    {
      ...
      n = shouldKeyword2(card);
    }

[zerker2000]

one where there's only one keyword handler, I shall call it "spTheMatrix" or "abTheMatrix".

Isn't the "only one keyword handler" what I'm shooting for in "CardFactory_Keywords"? Also, for some reason I have a feeling keywords should be done using an enum:

Code: Select all

public enum Keyword {
  KEYWORD {void addTo(card c, String orig){.../*add the keyword*/}},
  ANOTHERKEYWORD{...},
  ...,
  FINALKEYWORD{...};
  abstract void addTo(card c, String orig);

}
...
//Map keyword names to enums, I think enums have a usable toString...
//Final result Map<String, Keyword> map
...
for(String keyword : card.getKeywords)
  map.get(keyword.spit(":")[0]).addTo(card, keyword);//persuming keywords are written as "Keyword:option/s"

[Rob Cashwalker]

The reason for the double-check of should___ is to handle cards with multiple instances of an effect. Each time through the loop, the first thing the code does is remove the first (remaining) instance of the keyword. The reason for this is because we don't want "spPumpTgt:+2/+2" to be printed in-game, we want the spellDescription to read "Target creature gets +2/+2 until end of turn."

I agree, zerker, that your example of hasKeyword(Card c, String keyword) makes perfect sense.

As far as your suggestion for "CardFactory_Keywords":
By that I infer your suggesting (on topic) of splitting CardFactory. This would be somewhat ideal. But initial experimentation didn't pan out, according to Dennis, and Rares didn't have any ideas. I think if anyone's going to make it work, you might.

In the meantime, if we can't physically split the code, then the next best thing is to LOGICALLY split the code.
Use a generic keyword handler that out-sources the logic to external helper classes and methods, each of which takes the keyword string and parses out its necessary information, and returns the few crucial parts of the keyword process.

In other words:
Every keyword we have now is basically the same, just the functions canPlayAI, chooseTargetAI, canPlay, and resolve are over-ridden. And the code for each is written out multiple times, even for two similar effects, but the differences are sufficient that the keywords must be separate, usually because of different AI.

So we have a canPlayAI method that we have to over-ride, but instead of writing the code here, we can just return the boolean result of a function call to a generic AI handler, which contains the long if-block, with the keyword specific AI decisions for each, and probably takes over the chooseTargetAI functionality too. This sort of handler design saves code space, because there would are a number of standard CardLists used in every canPlayAI that only have to be declared once.

The resolve method (in CardFactory) would be reduced to two lines:
A call to an external generic resolve, another long if-block.
A call to an external drawback handler, which is another long if-block. (this is already part of my enhanced damage stuff)

The exact implementation of storing and matching keyword strings isn't critical. I think it's easier to read explicitly named if-blocks. Even a Map or Enum still ends up being used in some sort of if-block before the actual functionality can be applied.

This is something we should hash out more on the board before implementing anything drastic.

[DennisBergkamp]

In the meantime, if we can't physically split the code, then the next best thing is to LOGICALLY split the code.

There HAS to be some way of physically splitting this thing. We're about 100 lines away from 40,000 in CardFactory my Eclipse rolls over and dies whenever I change anything in there. I'll also have another attempt at this.