Splitting CardFactory

[Chris H.]

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.

`
We feel your pain Dennis, I find that using the revert command on CardFactory takes long enough for a coffee break.

[silly freak]

don't even think of doing the automatic eclipse formatter, save yourself from that pain

[Rob Cashwalker]

My boss and I were discussing some possible solutions. His recommendation was to turn each card into its own class file, each being an inherited card object... and groups of the classes can be easily packaged together. Which sounds like something right up silly freak's alley.
Now, his understanding of the architecture is nil. I explained that we're doing something similar. (really just subclassing the spellability objects instead of the whole card) But I'm thinking, we're probably not far off from that sort of implementation, and is something I recall Rares was talking about in v2.

[DennisBergkamp]

It's interesting, a friend of mine (who is a very good software architect) was thinking the same thing, using reflection and creating a separate class for each card.
Now, I'm not sure if that's the best way... first of all, it would obviously take a tremendous amount of work.
We also talked about the option of splitting up cards by type, but then there's the possibility of permanents having multiple types (Artifact Creature, Artifact Land, Creature Enchantment, etc.). But still I prefer the latter direction.

[nantuko84]

In MagicWars I use reflection to load card implementation from java class.
Actually it is easier for me as at the moment I have only 430 cards implemented that is less than yours, but even 430 is large number and it is not problem for me as I use packages depending on card color:
package mw.server.card.gold.ubr
so ubr package have only 8 cards than more less than 430

and my next step will be bringing set name to package name, smth like
mw.server.card.zen.green.

first of all, it would obviously take a tremendous amount of work.

your CardFactory has determinate structure: <if><card implementation><else if> etc. so I do believe it's easy to write parser that will cut them into java classes according to template. I'd say it will be entertaining task to do;) there are also generics at the beginning, I guess they also should be moved to separate classes (factories or decorators if you can combine them).

[zerker2000]

@Dennis: for me, it's more of my computer slowing down to a sleepy snail's pace .

@Rob: I would say that an optimal solution for AI methods would be a CardFactoryAI, which stores all (non-equivalent) AIresolve, canPlayAI, and chooseTargetAI methods, and only gets used during the actual game.

@Classes idea: I do think it's better to have a 40k line class than having 2000 20 line classes . Also, if AI methods get split off, it would be feasible to make a pseudocode card factory and run it through a generic action parser, and then convert all the "Card tok = new Card()"'s and "AllZone.Play.add(tok)"'s
into something more non-coder friendly:

Code: Select all

Make activated ability named ab1
Set ab1 mana cost to "1G"
Make action named ab1resolve
{
  Make card named tok
  Set tok name to "Insect"
  Set tok mana cost to "G"
  Set tok pt to 1/1
  Set tok image to "insect_1_1.jpg"
  Put tok in battlefield
}
Set ab1 resolve to ab1resolve
Set ab1 description to "1G: Put a 1/1 green Insect creature token onto the battlefield."
Add ab1 to card

Along these lines, you could of course have

Code: Select all

Make input named in
Set in buttons to none
Set in choose card to "target <insert spDestroy card filter here>"
Set card input to in

Or

Code: Select all

...
Make action choose
{
  Get in chosen card named choice
  If choice not in play
    Exit
  Get choice controller named cont
  Get maincard controller named cont2
  If cont not equal to cont2
    Exit
  Set maincard target to choice
  Make input named manain
  Set card mana cost to mcost
  Input mcost
}
Set in choose card to choose
...

This shouldn't be too hard to make either:
<code pending, wait for edit if not obvious, meanwhile comments please >
EDIT: Look a couple posts down

[Rob Cashwalker]

That sort of syntax is quite tempting... and not far from Rares' own vision.

10,000 classes doesn't seem too bad (in theory) since the code for any one card should be completely contained therein, especially if they're packaged separately by set or something. (not counting the possibility that cards show up in multiple sets...) Also consider that many cards can share a single class file that covers their effect... similar to how we do the keywords now. I'm told that Java can dynamically reference a class by using a string variable. So we could have for example, a single class that deals with "spPumpTgt".

[zerker2000]

I'm told that Java can dynamically reference a class by using a string variable. So we could have for example, a single class that deals with "spPumpTgt".

I think that would be

Code: Select all

java.lang.Class.forName("spPumpTgt").newInstance();

What would spPumpTgt extend though, SpellAbility?

[zerker2000]

Code: Select all

ArrayList<Object> all;
   HashMap<String, Integer> labels;   
   Integer index;
   public Card getNextCard(BufferedReader reader) throws IOException
   {
     labels.clear();
     final Card res = new Card();
     res.setName(reader.readLine());
     all.add(res);
     index++;
     labels.put("maincard", 0);
     parseCommand(reader, true).execute();
     return res;
   }

   Command parseCommand(BufferedReader reader, boolean linesleft) throws IOException
   {
     final String command = reader.readLine().trim();
     if(command.isEmpty() || command.equals ("}"))
     {
       linesleft = false;
       return Command.Blank;
     }
     final String[] pieces = command.split(" ");
     switch(pieces[0]){
       case "Exit": return null;
       case "{":
         boolean cont = true;
         final ArrayList<Command> todo;
         while(cont) todo.add(parseCommand(reader, cont));
         return new Command(){ public void execute()
         {
           for(Command c : todo)
           {
             if(c != null)
               c.execute();
             else
               break;
           }
         }};
      
      String label = command.split(" named ")[1];
      final int where = index;
       case "Make":
       labels.put(label, index);
       index++;
       return new Command(){ public void execute()
         {
           all.set(where, parseClass.get(command.substring(5).split(" named ")[0]).newInstance());
           //where parseClass is a Map<String, Class> containing values like <"activated ability", Ability.Class>
         }
       };
      
       final Object whose = labelGet(pieces[1]);
       final String how = unParseClass.get(whose.getClass()) + command.split(" to ")[0].substring(command.indexOf(" ", 4));
       //where unParseClass is a Map<Class, String> containing values like <Input.Class, "input">
       case "Set":
       final Object what = labelGet(command.split(" to ")[1]);
       return new Command(){ public void execute(){ parseSet.get(how).invoke(whose, what);}};
       //where parseSet is a Map<String, Method> containing values like <"card name", Card.setName>
      
       case "Get":
       labels.put(label, index);
       index++;
       return new Command(){ public void execute(){ all.set(where, parseGet.get(how).invoke(whose, what));}};
       //where parseGet is a Map<String, Method> containing values like <"spell or ability mana cost", SpellAbility.setManaCost>
      
       case "Input":
       final Object in = labelGet(pieces[1]);
       if(!(in instanceof Input)) throw new IllegalArgumentException("Error : " + command.split(" named ")[1] + " is not an Input");
       return new Command(){ public void execute(){ AllZone.InputControl.setInput((Input)in);}};

       case "Add":
       final SpellAbility sa = (SpellAbility)labelGet(pieces[1]);
       final Card card = (Card)labelGet(command.split(" to ")[1]);
       return new Command(){ public void execute(){ card.addSpellAbility(sa);}};//TODO: Counters?
      
       case "Put":
       final Card card_2 = (Card)labelGet(pieces[1]);
       final PlayerZone zone = parseZone.get(command.split(" in ")[1]);
       //where parseZone is a Map<String, PLayerZone> ... i.e. <"battlefield", AllZone.Play>
       return new Command(){ public void execute(){AllZone.GameAction.moveTo(zone, card_2);}};
      
       case "If":
       final Object arg1 = labelGet(pieces[1]);
       final boolean not = pieces[2].equals("not");
       final Command res = parseCommand(reader, linesleft);
       final Method todo_2 = parseIf.get(unParseClass.get(whose.getClass()) + command.substring(command.indexOf(" ", 3) + (not ? 0 : 4)));
       //where parseIf is a Map... <"input equals", Input.equals>, <"card in play", Card.inPlay>
       Class[] need = todo_2.getParameterTypes();
       final Object[] args = new Object[need.length];
       if(need.length == 1) args[1] = labelGet(pieces[pieces.length-1]);
       return new Command(){ public void execute(){ if(todo_2.invoke(arg1, args).equals(false) ^ not) res.execute();}};
     }
   }
   
   Object labelGet(String label){return all.get(labels.get(label));}

[zerker2000]

Err, comments please? Also, is there any way to get around switches being unable to use Strings without converting the code to a series of if statements?

[Rob Cashwalker]

I was thinking that you had blown my mind... and I was without any comment....
Then you brought it back down to earth, when you realized you can't switch Strings.
To be honest, if-blocks don't make much of a difference in readability. And a switch becomes basically the same as a series of if's when compiled.

Also, I'm thinking that the syntax example you're trying to work with is too free-form.
I'm thinking of something that combines the pseudo functions (from my spDamageTgt post) with a parser that doesn't care which order the pseudo functions are in the keyword string.

[zerker2000]

Code: Select all

enum name
{
EXIT, MAKE, SET, GET, INPUT, ADD, PUT, IF, PAROPEN;
public String toString(){return name().charAt(0) + name().toLowerCase().substring(1);}
}
HashMap<String, name> parseCommand = new HashMap<String, name>;
public void init()//call where appropriate
{
  parseCommand.put("{", name.PAROPEN);
  for (name n : name.values())
    if(!n.toString().equals("Paropen"))
      parseCommand.put(n.toString(), n);
}
...
switch (parseCommand.get(pieces[0]))
case EXIT:
...
case IF:

P.S. If you were blown away by what it would have been if Strings were switchable, would you like to write the parseFoo map initializer ?

[Rob Cashwalker]

Hmm... you're making this almost too easy.

[zerker2000]

The basic parsing part is. However, there's still the matter of going through all of CardFactory, converting it to this format, and mapping all conceivable variable types, get and set combinations, and If conditions: basically, lots of long tedious paperwork .

[Rob Cashwalker]

No, I see it as something that we should be able to make work concurrently with the existing stuff... just like the keywords are interpreted now. We just use a delimiter other than CRLF..