Splitting CardFactory

[zerker2000]

We just use a delimiter other than CRLF..

You mean using Java.util.Scanner instead of java.io.BufferedReader? As for working concurrently, I don't see why we'd use something meant to simplify/split cardFactory merely to complicate standard keywording. For full keyword functionality, you can simply use JavaCompiler...

[Rob Cashwalker]

OK, I'm guessing those classes would be the "right way" to work with string parsing? Scanning for tokens and such? Don't let me interfere then, if there's a better, grokable (Mark Rosewater's term for someone "getting it") method to the madness.
I was perfectly happy to just parse it the old fashioned way, with a change from position-sensitive parameters to a "param=value:param=value:..." sort of method.

I was merely recognizing that cards.txt has a CRLF-delimited format, that's well-established and works out for all of our needs. Whereas I presume a more scripted-form would require multiple "lines" of "code", which the current cards.txt architecture would choke. Hence, Rares and I established using ":" and "/" to delimit parameters. (not like we discussed it, his first keyword was Pump, so it was natural to use the colon to separate the cost from the numbers, and the slash for the pump numbers. We just kept using them again and again, until I added the "$" to make the Count and Drawback feel more function-like)
If we define a delimiter such that our script fits on one line, then the "ultimate keyword" can coexist with all working code until all the card code has been replaced with "ultimate scripts", and CardFactory shrinks to JUST the "ultimate interpreter".
Meanwhile, complex cards keep getting added in pure code, simple cards keep being added with either simple keywords or ultimate keywords as necessary. And we never feel like we're starting from zero.

[zerker2000]

OK, I'm guessing those classes would be the "right way" to work with string parsing? Scanning for tokens and such?

I am merely pointing out that the code uses BufferedReader to read the file, and if non-CRLF delimiters are to be used, Scanner would be a more reasonable choice, as its delimiter can be defined manually.

Don't let me interfere then, if there's a better, grokable (Mark Rosewater's term for someone "getting it") method to the madness.

Yes, I have read Stranger in a Strange Land. No, I do not think the script is measurably better than card factory in its current incarnation, but the drinkability Is increased noticeably. The main point for using it is the ability to split the code up any way you choose.

And we never feel like we're starting from zero.

Who proposed starting from zero? What I propose is the improvement of the old. Given some time, I will probably convert CardFactory into the simplified script anyways, all I am asking for are suggestions that seem obvious to a more experienced coder.

[Rob Cashwalker]

No, I do not think the script is measurably better than card factory in its current incarnation, but the drinkability Is increased noticeably.

Tastes great.
Less filling.

Tastes great!
Less filling!

TASTES GREAT!
LESS FILLING!

OK.. some of us might not get the joke... Miller Light commercial from the eighties.
How it applies, is either the card.txt looks elegant, or the CardFactory is smaller... probably can't have it both ways.

Who proposed starting from zero? What I propose is the improvement of the old. Given some time, I will probably convert CardFactory into the simplified script anyways, all I am asking for are suggestions that seem obvious to a more experienced coder.

You implied it:

However, there's still the matter of going through all of CardFactory, converting it to this format

Doing a total conversion would be starting from zero... basically. Converting here and there, like we've been doing with the keywords lately, is a more sound approach.

My suggestions don't mean much if I'm ignorant of any better way than what I've been coming up with, which Dennis is already calling "complex". If my ideas are complex, then a completely free-form system isn't going to be much simpler. Both of our designs make perfect sense, to US. We need to make sure that whatever we come up with is simple enough for THEM.

Why do I have Pink Floyd in my head now?

[zerker2000]

I didn't mean literally reading every line of cardFactory. I mean finding a statement/very small block of code, converting it into a regexp, adding any new values to matrixInit, running a replaceAll, and repeating until you run out of "pure" java code. It really shouldn't be very hard or too time consuming, I will (naturally)be the only one responsible for doing it, and the process can be pretty much stretched indefinitely(after a replaceAll, run a search for "^&[^;]+^&", which should show you all cards that have been fully converted, add those to the file used by forge, remove them from the cardFactory files, and you now hopefully have a smaller working file and no moral obligation continue work immediately). The final result would be a more readable cardFactory which can be split into two files at about any %n%n(new card) with low probability of bugs (editted from "no possibility" ).

[DennisBergkamp]

Well, "complex"... relative to our current keywords, they definitely are.
The reason I didn't add any cards with them (yet) is because I still had a lot of other things to wrap up, and I even forgot some of them (Life Burst). I figured adding your keyword into the next version would be cool, so Chris and Sloth (and possibly others) could test and then submit a bunch of them.

Right now I'm in favor of any kind of (logical) split of CardFactory... whether it's 2 pieces or 10 pieces, it all sounds good to me I've actually been getting crashes in Eclipse when working with CardFactory. This is not that strange I guess, it has surpassed 40,000 lines now (40,455!).

[thiagohac]

How about http://en.wikipedia.org/wiki/Decorator_pattern ?

[mtgrares]

The decorator pattern would be a good way to create a card's effect, SpellAbility.resolve().

Code: Select all

SpellAbilityList list = new SpellAbilityList();

list.add (new DamageTargetPlayer(4));
list.add (new DrawCard(1));

//then to resolve
list.resolve();

Obviously there are many details to hammer out, such as how does the effect know which player to damage? Code reuse is good and yet very hard to do.

[DennisBergkamp]

After some debugging and messing around with the code, I actually got Rob's original way working. I've split up CardFactory into CardFactory_Lands, CardFactory_Auras, CardFactory_Equipment and CardFactory_Planeswalkers. Everything works, and my local CardFactory.java is 31700 lines now instead of 42000

I will probably also take out the creature cards and create a CardFactory_Creatures.java, which would probably be around 15,000 - 20,000 lines of code (we have quite a few creatures after all).

[mtgrares]

Sounds very good. I suggest dividing up CardFactory by color but who knows.

[Chris H.]

After some debugging and messing around with the code, I actually got Rob's original way working. I've split up CardFactory into CardFactory_Lands, CardFactory_Auras, CardFactory_Equipment and CardFactory_Planeswalkers. Everything works, and my local CardFactory.java is 31700 lines now instead of 42000

I will probably also take out the creature cards and create a CardFactory_Creatures.java, which would probably be around 15,000 - 20,000 lines of code (we have quite a few creatures after all).

Awesome work, Dennis.

I had a CardFactory crash a couple of days ago. The bins became corrupted and the source was questionable. Thankfully I have developed a habit of zipping my project folder on a regular basis.

Oh, and I assume that you now have CardFactory_Instants and CardFactory_Sorceries files.

Once again, nice work. We all will be better off.

[DennisBergkamp]

No, I don't have Instants and Sorceries yet, that will require a bunch more work.

[Chris H.]

No, I don't have Instants and Sorceries yet, that will require a bunch more work.

`
That is OK. I'm not that sure that this would make a big difference at this time. Separating the creatures from the rest of CardFactory may be enough to keep everyone happy.

[DennisBergkamp]

Pfew, alright. So I think I'm done with all of the splitting, and this is what I have so far:

CardFactory.java - 16647 lines
CardFactory_Auras.java - 4500 lines
CardFactory_Creatures.java - 15133 lines
CardFactory_Equipment.java - 1630 lines
CardFactory_Lands.java - 1867 lines
CardFactory_Planeswalkers.java - 1848 lines

I've thought about splitting by color. The reason I didn't is because it would be quite a lot of work to split everything.
Auras, Equipment and Planeswalkers were very easy to divide... creatures took a lot more work, but mainly because it was so many of them.

[Rob Cashwalker]

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?