Fire Covenant - First attempt at contributing a card

[JeffHoogland]

Howdy Folks,

Started using XMage recently and I have some coding background (although not much with java to date) so I'd like to contribute where I can. My first crack is trying to code the card Fire Covenant which is missing (that I'd like to play ).

Current code is a bit of a hack/paste job from cards with similar effects. If anyone would be kind enough to explain what looks wrong and why I'd be grateful. Also - are there docs/a wiki somewhere detailing all of the internal mage.abilities and such the cards call?

Pastebin Link: http://paste.debian.net/114708/

Code: Select all

package mage.sets.iceage;

import java.util.UUID;
import mage.abilities.costs.common.PayVariableLifeCost;
import mage.abilities.dynamicvalue.DynamicValue;
import mage.abilities.dynamicvalue.common.GetXValue;
import mage.abilities.dynamicvalue.common.SignInversionDynamicValue;
import mage.abilities.effects.common.continious.BoostAllEffect;
import mage.abilities.effects.common.DamageMultiEffect;
import mage.cards.CardImpl;
import mage.constants.CardType;
import mage.constants.Duration;
import mage.constants.Rarity;
import mage.filter.common.FilterCreaturePermanent;

/**
 *
 * @author NA
 */


public class FireCovenant extends CardImpl {

    public FireCovenant(UUID ownerId) {
        super(ownerId, 145, "Fire Covenant", Rarity.RARE, new CardType[]{CardType.INSTANT}, "{1}{B}{R}");
        this.expansionSetCode = "ICE";

        this.color.setRed(true);
        this.color.setBlack(true);

        // As an additional cost to cast Fire Covenant, pay X life.
        this.getSpellAbility().addCost(new PayVariableLifeCost(true));
        // Deals X damage divided as we choose between any number of target creatures
        DynamicValue xValue = new SignInversionDynamicValue(new GetXValue());
        this.getSpellAbility().addTarget(new TargetCreaturePermanent());
        this.getSpellAbility().addEffect(new DamageMultiEffect(xValue));
    }

    public FireCovenant(final FireCovenant card) {
        super(card);
    }

    @Override
    public FireCovenant copy() {
        return new FireCovenant(this);
    }
}

[LevelX]

You need some posts in this forum to post such things.
Not many, 4 or 5 posts I guess.

But a better way to share the code is to make a XMage fork in github and send pull requests. So I can look at the code and comment if something is not ok and you can update it.
If ok I can include the code directly to XMage.

[JeffHoogland]

One post.

[JeffHoogland]

Two post.

[JeffHoogland]

Blue post.

[JeffHoogland]

Okie dokie, added the code to the top level post. I didn't want to screw around with GIT as I can't really have a conversation there about what is wrong and why.

~Jeff

[LevelX]

Code:
You should not use the SignInversionDynamicValue() because you don't need a negative value like for Toxic Deluge. Rest looks good. Did you test it?

Two more things:
1) Did you use the gen-card.pl script to create the card?
If not you should and take a look at this side:
https://github.com/magefree/mage/wiki/Developer-Getting-Started#perl

2) We use a web application called "Card Tracker" to track which cards are implemented and which are missing in XMage.
http://ct-magefree.rhcloud.com/
You can mark implemented cards as implemented, also you can reserve cards to implement so other developers know you are working on the implementation.
But the best thing about it is that its very useful to search for cards with similar effects already implemented if you implement new cards.
If you go on to implement cards, I'll create a profile for you and send you the access info by private forum mail.

[troyready]

Jeff, FWIW github adds on some pretty nice commenting/collab tools on top of raw git push/pulls/comments (e.g. inline discussion on code segements). I'd also recommend trying it by opening a pull request and adding comments on your code there.