CardFactory Keywords

[friarsol]

So I'm thinking it might be better if we moved all of the keywords we have piled up in the last few months into their own class. Since the keywords have crept up to 5k I think it'd be easier to have them separate for Organizational reasons.

Our primary CardFactory would handle base card setup, run it through the keyword parser, then call the appropriate Factory for whatever card type we have. And all the other Factories wouldn't have to worry about parsing keywords at all. Just handling specific cards of their own type.

I was going to do this today, but if anyone is working in the CardFactory files I can wait before pulling this out.

[Chris H.]

So I'm thinking it might be better if we moved all of the keywords we have piled up in the last few months into their own class. Since the keywords have crept up to 5k I think it'd be easier to have them separate for Organizational reasons.

Our primary CardFactory would handle base card setup, run it through the keyword parser, then call the appropriate Factory for whatever card type we have. And all the other Factories wouldn't have to worry about parsing keywords at all. Just handling specific cards of their own type.

I was going to do this today, but if anyone is working in the CardFactory files I can wait before pulling this out.

`
Rob has mentioned something similar in the past. A keyword Factory would possibly allow cards like the broken Resuscitate spell to be re-written and this spell would finally work. I will hold off of any additional keyword work until this settles down as there are other areas for me to work on in the meantime.

[Rob Cashwalker]

I was just thinking about that too, though I would rather see the non-keyword code in the bottom half of CardFactory be moved into an appropriate class.

Either way, it's inevitable, that the amount of keyword code will eventually grow too.

edit:
Chris is speaking of the other solution - that keywords become parsed on their own entirely, on the fly. In many respects, this is the better long term solution, and if we're going to do it, we should do it soon, before the number of keyworded actions gets to be overwhelming.

It would be more accurate to call it a SpellAbilityFactory. Pass it the keyword line and the source card, and it returns a complete SpellAbility object to make that keyword work. It would take some substantial restructuring to do this, though most of the keyword-specific code can be copied. At the same rate, just about every keyword starts with the same few lines using the same variables, so there is some duped code that can be eliminated.

In the same sweeping change we could revise the keyword delimiter issue. Instead of expecting keyword parameters in a specific array element, we could use named parameters instead... ie:

Code: Select all

spPumpTgt:PTK$+1/+1/Flying:Drawback$YouGainLife/1:SpDesc$Target creature gets +1/+1 and gains flying until end of turn. You gain 1 life.

spDestroyTgt:Valid$Creature.nonBlack,Creature.nonArtifact:Regen$No


All elements after the first one are placed into a map with the parameter name as the key. Keyword specific code can then refer to the parameters they expect in any order necessary. Only reason to follow a particular order in cards.txt is for readability. This also opens the door to easily adding specific requirements to some keyword actions but not others... like pumping only an Elf creature.

[friarsol]

Chris is speaking of the other solution - that keywords become parsed on their own entirely, on the fly. In many respects, this is the better long term solution, and if we're going to do it, we should do it soon, before the number of keyworded actions gets to be overwhelming.

It would be more accurate to call it a SpellAbilityFactory. Pass it the keyword line and the source card, and it returns a complete SpellAbility object to make that keyword work. It would take some substantial restructuring to do this, though most of the keyword-specific code can be copied. At the same rate, just about every keyword starts with the same few lines using the same variables, so there is some duped code that can be eliminated.

Oh I like that way better than just migrating code out of CardFactory. While it would be good to clean up the keywords, if we can streamline our keywords at the same time we clean up CardFactory I think it's worth it.

The way Keyword parsing would look after that change may makes the migration a very temporary solution. So I guess I'll hold off on just migrating if we're going to do something else with the keywords.

Were you already in the process of looking into this Rob? If you aren't, I can poke around a bit instead of moving to the other features I was going to work on.

[Rob Cashwalker]

It's one of those architectural things that makes perfect sense to me in theory, but I'm not about to try to implement on my own.... Certainly not as long as the way we do it now is good enough.

I'm game to work on conversion once the basic structure is ready and some sample is operational.

Furthering the work on the alternative cost handling is important though, if that's the kind of stuff you were thinking of.