Splitting CardFactory

[zerker2000]

Nice!!! Would you be so kind as to commit them to svn? Also, if the plan is to make more than 10 of them, maybe CardFactory should be made into a seperate package?

[silly freak]

the main problem with packages is serialization. for example, the created decks are stored in a map, and that map is serialized and stored in a file. if we just move the deck class into a package, it will be incompatible with the serialized file
this prevents us from moving moving all the classes that use deck as well, because you can't import classes from the default package. I doubt that CardFactory is independent enough to be moved into a package...


btw, who's antechno777?

[silly freak]

good news, i managed to do something in that direction:
first, I moved the default package, which doesn't result in compile-time errors but fails when trying to load the decks.
after that , I created the Deck class in the default package again like that:

Code: Select all

public class Deck implements Serializable {
    private static final long serialVersionUID = -2188987217361601903L;
   
    private String            deckType;
   
    private boolean           isRegular;
    private boolean           isSealed;
    private boolean           isDraft;
   
    private ArrayList<String> main             = new ArrayList<String>();
    private ArrayList<String> sideboard        = new ArrayList<String>();
   
    //very important, do NOT change this
    private String            name             = "";
   
    private Object readResolve() throws ObjectStreamException {
        System.out.println("resolving obsolete Deck");
        forge.Deck d = new forge.Deck(deckType);
        d.setName(name);
        for(String s:main)
            d.addMain(s);
        for(String s:sideboard)
            d.addSideboard(s);
        return d;
    }
}


as you see, the class is Serializable, has the same serialVersionUID as the original Deck class, declares all the attributes, but has only one method: readResolve()
that method is used during deserialization to replace the read object with another. In this case, it replaces the Deck object with a forge.Deck object, and it seems to work!

i'm testing a little to find other classes that need similar treatment, but if you know of some, that would be great! so far, i have

• Deck
• QuestData_State
  I'm getting NullPointerExceptions with that. is it also with the version on SVN? are the quest files there up to date, or have broken the app myself?

[DennisBergkamp]

AWESOME!!!


Should split the keywords out to their own file too. And yeah, when you have the chance, separating the spells into Instants and Sorceries would rock. I don't think splitting by color would be necessary. We may need to end up with a Creatures1 and Creatures2 sort of idea after a while.

What was the trick?

I'm not exactly sure what we were doing wrong initially, but as I was debugging the stuff I tried last, it became clear that the SpellAbilities were added the other way around.
For instance, The Hive looked like:

SpellAbility[0] = "{5}, : Put a 1/1 colorless Insect artifact creature token with flying named Wasp onto the battlefield."
SpellAbility[1] = "The Hive"

Which is why nothing worked. So I just moved things around a bit, and after some trial and error, it actually worked

[DennisBergkamp]

Ah yes, I will commit all of these changes to SVN. By the way, are you still having trouble with ArrayList<Command>? If so, I have no objections changing them to CommandLists.

I'm not sure if this is a really big deal, but I also changed each 'if(cardName.equals("some card")' into 'else if(cardName.equals("some card")', combined with the CardFactory split, this *should* increase performance quite a bit when starting up MTGForge or when opening the Deck Editor.
I could even bring this is a step further, and do a "return card;" at the end of each if statement (which would make the "else if" change I just made pretty useless though I guess ), but the problem with that is that we currently add cycling abilities at the end of CardFactory.java and also CardFactory_Creatures.java.

[zerker2000]

Don't we have some spells who get their code from more than one if block? Also, would it be possible to keyword buyback(e.g. "String Spell.Buyback = "none"; ", "boolean buybackpayed", "if(!sa.Buyback.equals "none") StopSetNext(new InputPayBuyback(sa))", "setbuttonBoth; ... cancel:sa.buybackpayed = false; ok:"sa .setManaCost(sa.getManaCost() + " " + sa.Buyback); sa.buybackpaid=true; ", "if(!sorceCard.isPermanent) {if(buyack paid) moveToHand(sourceCard) else moveToGrave(sourceCard)}").

[Chris H.]

While looking through CardFactory.java I found 2 creatures that may have been overlooked in the move to CardFactory_Creatures.java.

Should we move Kinsbaile Borderguard and Lockjaw Snapper over?

[DennisBergkamp]

Ahh yes, that also explains why Kinsbaile Borderguard doesn't work anymore (someone reported it on the bug page).

[Chris H.]

I enjoy squashing the occasional bug.

I was scanning through the CardFactory.java file originally to find cards which can be converted from code to keyword. I found about two dozen of them.

It will keep me busy.

[Rob Cashwalker]

What effects were you looking at doing?

I'm going to touch up the spPumpTgt code to fit with the new tech, and then revise spLoseLife.

Another thing to try to work on is adapting some abilities to spells and vice-versa, like making abDraw[Tgt].

[Chris H.]

There are some spells which can be converted from code to keyword. The keywords were created some time ago. And not every existing card was converted over at the time. For example:

Ice Storm
Lay Waste
Sinkhole
Stone Rain
Terminate
Shatter
Brute Force
Wildsize
Might of Oaks
Vindicate

...

and others.

[Rob Cashwalker]

Oh.. right....

[DennisBergkamp]

Converting those should get rid of a lot of redundant lines of code

[Chris H.]

I spent several weeks prior to the SVN trying to complete the modification of the spRaiseDead keyword and got stuck. I meant at that time to ask for some guidance to get past where I was stuck.

We have had several new people contribute material. They needed some guidance and I did not want to interrupt there efforts by asking for help at that time.

Then Dennis set up the SVN and it took me several weeks to get up to speed. The SVN is great. It allows me to address the TODO list that I have. Granted, for the most part, my TODO list is fairly simple. The list is getting shorter.

I want to get back to keywords pretty soon. I should probably finish the modification of the spRaiseDead keyword first.

I had fun with the Disciple of Kangee and zerkers keyword. There are several land cards that need a similar treatment. It looks like we need some sort of a utility method(s) that could be reused when needed for these types of color changing cards.

I have spent some time recently trying to finish the Funeral Charm card. At some point I want to hear from you about your plans for charms. I am currently using a setText() (???) command to print a "Choose one\r\n". The text panel looks nice and is readable. The choice window only lists the three choices and does not include the Choose one\r\n" string.

For charms based on only keywords and not on code we could consider having a Charm keyword which would do the setText() command to print a "Choose one\r\n".

[Chris H.]

Converting those should get rid of a lot of redundant lines of code

Yeah. CardFactory is now only about 18### lines long.

With a little work and a substantial portion of that will be large sections that have been commented out.