Thursday, January 22, 2009

SpellAbility Improvements - Part 2

Ideally SpellAbility could also be used for upkeep triggered abilities like Juzam Djinn (deal 1 damage to you during your upkeep) and damage triggered abilities like Thieving Magpie (draw one card when you deal damage). This would require more methods to be added to SpellAbility.

In order to implement upkeep triggered abilities like Juzam Djinn, SpellAbility could have the method isPhaseTrigger() which returns true or false if the ability should trigger during the current phase. SpellAbility.doPhaseTrigger() would add the SpellAbility object onto the stack and SpellAbility.resolve() could implement the effect of the trigger, such as dealing 1 damage. isPhaseTrigger() could work for any phase but it would be most commonly use during the upkeep phase. (I know up upkeep is technically a step and not a phase but I refer to it as a phase because it is just easier.)

Damage triggered abilities like Thieving Magpie require a little bit more work since damage triggered abilities can trigger when a creature damages another creature or damages a player. damageCard(Card c, int damage) and damagePlayer(String player, int damage) could be added to SpellAbility. And to further complicate things, some cards only trigger on combat damage, so corresponding combat damage methods would also have to be added.

In case you didn't know or forgot, "A triggered ability begins with the word “when,” “whenever,” or “at.”", 404.1 of Magic's comprehensive rules.

"The first 90 percent of the code accounts for the first 90 percent of the development time. The remaining 10 percent of the code accounts for the other 90 percent of the development time." --Tom Cargill


Gando the Wandering Fool said...

It seems to me it would be more orderly to separate phase specific triggers from everything else.

so that you have UpkeepSpellTriggers
and then have members of that for specific types of triggers. (my 2.5cents anyway)

Forge said...

Obviously I am stretching SpellAbility. It would be great if this one object could handle all the wierd abilities like upkeep and combat damage. So in a sense I am forcing more "stuff" into SpellAbility because the rules are written that way. I could put all of the abilities that trigger during combat in one place, like in CombatTriggerAbility instead of forcing them into SpellAbility.

Forge said...

(This is a cross post, but it's my blog, ha.)

Generalizing the code is one of the most important attributes when programming Magic or anything else. You want the code to be as general as possible while being able to handle all of the situations that might occur. This entails that you understand the problem backwards, forwards, and sideways. Understanding the problem well enough before you start to code anything is one of the challenges of programming.

In my mind I see costs as being
mana tap
mana sacrifice

and my goal is to try to handle all of those with the same code, NOT with specialized code. I may use specialized code to handle some ability, but I won't like it, lol.

Silly Freak said...

have own methods for different types of triggered abilities seems to me much like specialized code. I would do some sort of trigger class, and whenever an event happens, every SpellAbility can check if it matches the trigger.
Think of it this way: object oriented programming is very much for modularizing the code so that you have independent parts. having separate methods for every trigger would make this impossible, because the other code would have to do special things with your SpellAbility

Anonymous said...

Forge - use an event system. It'll be much easier. Then you can write a generic triggered ability class that registers for specific events (with certain conditions).

Forge said...

I like the idea of a real event system. I've used alot of observers, but I think events would really help out.

Anonymous said...

I wrote a reply, but it was getting long, so I moved it to the forum pages:

Unknown said...

In the forums, I posted a suggestion that these sort of effects be coded as command objects. Then anywhere in the normal course of the game, when the state-based effects or the upkeep effects are run, instead of running the hard-coded code in GameActionUtil, the appropriate command object for the card is executed. Like right now, there exists a command object defined in the Card object that is called when the card comes into play. Well, just define a command object for the upkeep effect, and call it from the upkeep engine.

Anonymous said...

Rob - what about cards that don't care about the upkeep step? Should every card care about every possible thing (even if it defaults to a nop)?

Anonymous said...

Is there missing the "fast effects before blockers" step intentional? I can't activate Mishra's Factory or create a token from Imperious Perfect. It's very annoing :(. Also game always let me start first and in addition gives me an extra card (in real MtG starting player shouldn't draw).

Unknown said...

Incantus -
if(card.upKeepEffect != null) should take care of it.

Anonymous said...

How about this:

Each card with a triggered ability creates an ability that registers for a particular event. When that event happens, the dispatch system automatically calls into all the cards that registered for that event. Then you don't have to iterate through each card for every possible event. Can you imagine doing that for every phase (Upkeep, Draw, Combat, etc), everytime you generate mana (Manabarbs), a card taps (Orcish Mine), everytime damage is dealt (Auntie's Snitch), everytime a card enters or leaves a zone (lots of cards), etc. You'll have a mess.

Unknown said...

Granted that's how it should work out. But of course we're limited in how we can "respond" to events.... I just figure the simplest way is to iterate through all cards in play, which wouldn't most of the time is a handful of cards.

Forge said...

"Is there missing the "fast effects before blockers" step intentional?"

Yes, MTG Forge has a limited number of phases, as more cards were added the "missing phase" problem grew bigger.

Anonymous said...