Issue 157: which classes do we need?

[Max mtg]

http://cardforge.org/bugz/view.php?id=157

I completely agree that using a single class for every possible card representation is a Door To Nothingness. So, let's figure out, which classes are needed for each purpose. At the moment, we have two methods of representing a card: an instance of Card and a String with name. The latter being used for deck and card pool management and the former for everyting else.

To my opinion three "levels of detail" for Card class are needed:
1. CardReference - contains only name, edition, picture number and foiled flag. This is useful to store cards owned by player, be it questmode cardpool or a deck. Picture number is good for distinguishing different copies of lands and promos from ordinary cards.
2. CardOutOfGame (or simply Card) - contains all the rules and abilities, but has no temporary effects on it. This class is useful for displaying cards as quest rewards or in deckbuilder GUI. Such cards instances do have enough properties to filter them by having some ability (infect for instance) or sort by cmc. Their PT and other properties are always nominal and no game-related effects may be placed on these cards. One instance of each card of this type per application is enough. [These cards may persist for the whole app runtime]
3. CardInGame - has a reference to CardOutOfGame (it's prototype), but this card can be placed to any zone in game, can have counters placed over it as well as other effects, can have changed P/T, color, types and so on. [These objects are disposed as soon as game ends]

[friarsol]

I'd prefer that 2 have just Oracle text not actual parsed Abilities. This would include Power, Toughness, Loyalty, CC, Card Types and any Card Text. I do have a script that would add the Card Text data for us (and without much extra work can handle some of the other stuff as well), but there has been some conversation regarding it back and forth so I haven't run it yet. This is important to speed up the Deck Editor load times, so the full Card Game Objects won't be generated.

[Braids]

Max mtg has a thinking cap on!

1. sounds great, but the name is too generic. this sounds more like an inventory item. InventoryCard? BasicCard? DeckListItem? CardNameAndPicture? we might want to represent some tokens this way as well. my tokens don't have art. {maybe i haven't configured my game correctly?} also, we need a way to get one of these and have it choose a random picture. i have visions of importing decklists, and not all formats identify the picture {or set}.
2. i agree with the good friar. this should have text attributes only. it might not even be a Java class -- it sounds more like database material {relational or otherwise}. or maybe an object-relational map. use Hibernate?
3. CardInGame has a great name, and the relationship to #2 makes sense. because we need text for the card panel in the upper right of the battlefield window. but we also need a relationship to #1 for the picture.
4. so we also need a disk based one to many relationship from canonical ASCII card name to picture identifiers {like a set+picture number}. this specifies which card names can have which pictures, so we can pick random pictures for #1 and perhaps verify the pictures for #2 and/or #3.

[Max mtg]

Max mtg has a thinking cap on!

What does this mean? English is not my native language - so the phrase above sounds unclear to me.

1. sounds great, but the name is too generic. this sounds more like an inventory item. InventoryCard? BasicCard? DeckListItem? CardNameAndPicture? we might want to represent some tokens this way as well. my tokens don't have art. {maybe i haven't configured my game correctly?} also, we need a way to get one of these and have it choose a random picture. i have visions of importing decklists, and not all formats identify the picture {or set}.

That is an inventory item. The reason I called it reference is because before that we had only cardnames as inventory storage unit, those were just strings. They only referred to a card, any card meeting the name criteria. Now it's time to eliminate randomness - I want such a record refer to one and only card, that is why extra fields were needed.

InventoryCard is a good idea. Others are not: BasicCard (class is not supposed to be an inheritance base and there won't be an AdvancedCard), DeckListItem (it would sound strange to give quest rewards with those or store them in cardpool), CardNameAndPicture (in fact they are, but there should be a name)
So, CardReference or CardInventory sound fine to me.

Tokens are never kept in inventory or deck, so we do not needed to support them at this level of abstraction... Do tokens belong to a set?

No random pictures! set+pictureIndex strictly define the appearance of any card, together with its name.
As for importing decklists - it's ok to have forge choose random picture for lands. Promo versions of cards should never get autoimported. In case edition is to be determined automatically, assume the latest one, when card was printed.

2. i agree with the good friar. this should have text attributes only. it might not even be a Java class -- it sounds more like database material {relational or otherwise}. or maybe an object-relational map. use Hibernate?

Despite of the method for storing that data, we'll anyway end up with a java class representing a storage unit. So, there's no escape from a class. If you want to have an OracleCard here, it's fine, but you'll still need a prototype with all abilities parsed for the next level of detalization - the one which is supposed to be read from %cardname%.txt

3. CardInGame has a great name, and the relationship to #2 makes sense. because we need text for the card panel in the upper right of the battlefield window. but we also need a relationship to #1 for the picture.

Yes, both references are necesary. Though you can always derive a reference to #2 (rules) from #1 (appearance).

4. so we also need a disk based one to many relationship from canonical ASCII card name to picture identifiers {like a set+picture number}. this specifies which card names can have which pictures, so we can pick random pictures for #1 and perhaps verify the pictures for #2 and/or #3.

I expect picture management to be achieved quite simply:
1. Have card images organized in a tree (root)\(set)\(cardname).jpg
2. In card description files (the %cardname%.txt ones) for each edition there should be a number indicating amount of copies diven edition has, alongside with urls to donwload these pictures from (gatherer/magiccards).

Once a card is queried by its name+set+index, there exists one or no requested picture. To avoid misses index should no more than amount_of_copies_of_given_card_in_set.
Once a card is queried by its name+set (index missing) - choose index 0 for everything except basic lands. Follow instructions in previous line.
If a card is queried by name only, set = card.mostRecentSet(), move on to previous line.

Why do you need disk based relations (cardname) => (set,picture)?

[friarsol]

2. i agree with the good friar. this should have text attributes only. it might not even be a Java class -- it sounds more like database material {relational or otherwise}. or maybe an object-relational map. use Hibernate?

Despite of the method for storing that data, we'll anyway end up with a java class representing a storage unit. So, there's no escape from a class. If you want to have an OracleCard here, it's fine, but you'll still need a prototype with all abilities parsed for the next level of detalization - the one which is supposed to be read from %cardname%.txt

I think the key point here is the Deck Editor doesn't need the Fleshed out abilities, only the text of the Abilities (for filtering/searching etc) so Oracle Text should be in this step and Fleshed Abilities should be in the one after this.

[Max mtg]

I think the key point here is the Deck Editor doesn't need the Fleshed out abilities, only the text of the Abilities (for filtering/searching etc) so Oracle Text should be in this step and Fleshed Abilities should be in the one after this.

I agree, the current deck editor is not versatile enough to deal with parsed abilities. And if parsing takes that much time, let's skip it.

[silly freak]

Parsing could be done lazily, but I think the oracle card is the better place, simply because it saves the game from parsing the ability for every card instance in every new game.

Although I'm not active on it right now, I have a similar structure in my game, with two levels. Actually, I do ability parsing at build time and store the parsed cards with functional ability objects in serialized form to improve startup time.

[Max mtg]

Parsing could be done lazily, but I think the oracle card is the better place, simply because it saves the game from parsing the ability for every card instance in every new game.

Although I'm not active on it right now, I have a similar structure in my game, with two levels. Actually, I do ability parsing at build time and store the parsed cards with functional ability objects in serialized form to improve startup time.

If I got your idea right, at first we process simple text-only oracle records for all cards (that should be fast as it does not include any parsing), and as soon as a battle begins that heavy entites of Card are being created by LazyCardFactory.
So how this collection of oracle card data should be stored?

[silly freak]

I'm not up to date with forge's ability code, but what I'm saying is basically this:

Laterna Magica does it like this
-cards are stored as separate files
-before building a release, the card parser creates the OracleCards from these files and uses Java serialization to store them. (I use a zip archive with individual zip entries for every card)
-at runtime, cards are lazily loaded: everytime a card is needed that was not yet loaded, it is read from the archive. no text analysis is needed for loading a card, and cards are loaded individually.

what I suggested for forge
I think there is no need to load all cards at once, as every card is independent. It might take some/much refactoring, but CardFactory would need to be able to load single cards.

-every time an oracle card is needed, read it from the file. the OracleCard then contains the static info, including both oracle card text and card script text.
-when the abilities of a card are first needed (i.e. in a game), the abilities of these cards are parsed from the already read ability text in OracleCard and stored.
-when an InGameCard is created, it copies all abilities from the oracle card, as abilities may be added/removed subsequently

this would mean that for the first game, only up to 120 card's abilities need to be parsed, not all, and starting the constructed deck editor needs the time to read all card files, but not to parse all abilities. still, every ability on every card is only parsed once.

EDIT: I hope I don't sound like a smart ass, as i haven't worked on forge or even my own program for a long time. I just see how forge's architecture grows into something I consider structured and extensible, and when I see an opportunity to help going into that direction, even if only by suggestions, I try to do so

[friarsol]

this would mean that for the first game, only up to 120 card's abilities need to be parsed, not all, and starting the constructed deck editor needs the time to read all card files, but not to parse all abilities. still, every ability on every card is only parsed once.

EDIT: I hope I don't sound like a smart ass, as i haven't worked on forge or even my own program for a long time. I just see how forge's architecture grows into something I consider structured and extensible, and when I see an opportunity to help going into that direction, even if only by suggestions, I try to do so

Not at all. This is exactly how I was picturing it occurring as well. I believe I stated as much the last time we had this conversation, but noone was actively working to make the improvements quite yet.

[Rob Cashwalker]

There should only need to be two classes of Card, one pure text, aka "printed" stats and one class which is created on the fly for each instance of the printed card needed.

The PrintedCard class just maintains the cardname.txt data, along with the Oracle text. One global, immutable List of these objects. No fancy data structures. Efficient searching and sorting would be nice, runtime DB or just faking it by wrapping a few TreeMaps built using different keys (name, color, type, keywords, etc..). Heap usage for those sorts of maps should be significantly less than the original map which included the entire Card object with code baggage.

The ActiveCard class is created by a Factory, on-demand based on parsing the PrintedCard object. This object has uniqueness and begins life only when the in-game library is being assembled. During assembly, SetInfo is assigned, along with picture selection, if multiples are available in the SetInfo. While in-game this object may move from zone-to-zone, can be cloned, can be blinked, etc, without affecting any other ActiveCards that just happen to share a name with each other.

One tricky part is Parsing a card object which isn't scripted... ahh, but here's a trick. Let's give all the hardcoded cards a special AbilityScript word that relates to their specific code, but takes it out of hard-coded CardFactories, in favor of an AbilityFactory_HardCode... or something.

[silly freak]

One tricky part is Parsing a card object which isn't scripted... ahh, but here's a trick. Let's give all the hardcoded cards a special AbilityScript word that relates to their specific code, but takes it out of hard-coded CardFactories, in favor of an AbilityFactory_HardCode... or something.

hmm... this sounds familiar^^

If the line is "text : Add to your mana pool.", the "text" handler tries to find an ability parser that can parse the line. Even if I can't represent an ability, I can make a class specifically for that ability and add a line handler that applies an arbitrary class to the card, like "apply laterna.magica.cards.DestroyAllCreatures".

[Braids]

Max mtg has a thinking cap on!

What does this mean? English is not my native language - so the phrase above sounds unclear to me.

oh, it's an idiomatic phrase. it means you have been thinking seriously about something and usually critically analyzing it.

1. sounds great, but the name is too generic. this sounds more like an inventory item. InventoryCard? BasicCard? DeckListItem? CardNameAndPicture? we might want to represent some tokens this way as well. my tokens don't have art. {maybe i haven't configured my game correctly?} also, we need a way to get one of these and have it choose a random picture. i have visions of importing decklists, and not all formats identify the picture {or set}.

That is an inventory item. The reason I called it reference is because before that we had only cardnames as inventory storage unit, those were just strings. They only referred to a card, any card meeting the name criteria. Now it's time to eliminate randomness - I want such a record refer to one and only card, that is why extra fields were needed.

InventoryCard is a good idea. Others are not: BasicCard (class is not supposed to be an inheritance base and there won't be an AdvancedCard), DeckListItem (it would sound strange to give quest rewards with those or store them in cardpool), CardNameAndPicture (in fact they are, but there should be a name)
So, CardReference or CardInventory sound fine to me.

CardReference sounds too much like a pointer or "passed by reference", which is a specific term in computer science. InventoryCard, then? not CardInventory, that denotes an entire inventory, instead of one card.

Tokens are never kept in inventory or deck, so we do not needed to support them at this level of abstraction... Do tokens belong to a set?

oh, you're right. yes, they can belong to a set, but it's much more important that the token's name, color(s), abilities, power, and toughness are correct. some tokens with the same name vary in these attributes.

No random pictures! set+pictureIndex strictly define the appearance of any card, together with its name.

your vote is duly noted, but there is no need to be so vehement.

As for importing decklists - it's ok to have forge choose random picture for lands. Promo versions of cards should never get autoimported. In case edition is to be determined automatically, assume the latest one, when card was printed.

i can live with that. when you say, "promo versions of cards", are you referring to {Magic Rewards'} textless spells?

2. i agree with the good friar. this should have text attributes only. it might not even be a Java class -- it sounds more like database material {relational or otherwise}. or maybe an object-relational map. use Hibernate?

Despite of the method for storing that data, we'll anyway end up with a java class representing a storage unit. So, there's no escape from a class. If you want to have an OracleCard here, it's fine, but you'll still need a prototype with all abilities parsed for the next level of detalization - the one which is supposed to be read from %cardname%.txt

yes, i suppose it has to be some Java class at some point, unless i want to use a raw HashMap to represent card text and attributes.

Why do you need disk based relations (cardname) => (set,picture)?

to validate possibly incorrectly imported deck lists, i think. but like you said, the relationship can be deduced from #3 to #1.

[Braids]

Parsing could be done lazily, but I think the oracle card is the better place, simply because it saves the game from parsing the ability for every card instance in every new game.

we presently have a LazyCardFactory, but it has only proven itself in unit testing. it will not yet work with the main Forge app, because it doesn't support iterating over every card.

Although I'm not active on it right now, I have a similar structure in my game, with two levels. Actually, I do ability parsing at build time and store the parsed cards with functional ability objects in serialized form to improve startup time.

very nice.

[Braids]

. . . The PrintedCard class just maintains the cardname.txt data, along with the Oracle text.

so you like #2. i think we would want Forge specific metadata in there, too, like AI compatibility. the deck generators should be able to work with this information without creating CardInGame (#3) instances.

The ActiveCard class is created by a Factory, on-demand based on parsing the PrintedCard object. This object has uniqueness and begins life only when the in-game library is being assembled. During assembly, SetInfo is assigned, along with picture selection, if multiples are available in the SetInfo. While in-game this object may move from zone-to-zone, can be cloned, can be blinked, etc, without affecting any other ActiveCards that just happen to share a name with each other.

what about a card inside a decklist, quest collection, shop inventory or sideboard? that's what #1 is about. i don't think we want to store all of #2 for each card in a deck.