Copying a SpellAbility?

[Marek14]

How close are we to Twincast or storm?

[Rob Cashwalker]

Let me post the code so you can try this out yourself and figure out why it asks fro the wrong mana....

For spLoseLife
CardFactory:

Code: Select all

                card.clearSpellAbility();
                card.addSpellAbility(spLoseLife);
               
                String bbCost = card.getSVar("Buyback");
                if (!bbCost.equals(""))
                {
                   SpellAbility bbLoseLife = spLoseLife.copy();
                   bbLoseLife.setManaCost(CardUtil.addManaCosts(card.getManaCost(), bbCost));
                   bbLoseLife.setDescription("Buyback " + bbCost + "(You may pay an additional " + bbCost + " as you cast this spell. If you do, put this card into your hand as it resolves.)");
                   bbLoseLife.setIsBuyBackAbility(true);
                   
                   card.addSpellAbility(bbLoseLife);
                }


CardUtil:

Code: Select all

    static public String addManaCosts(String mc1, String mc2)
    {
       String tMC = new String("");
       
       Integer cl1, cl2, tCL;
       cl1 = Integer.valueOf(mc1.replaceAll("[WUBRGSX]", "").trim());
       cl2 = Integer.valueOf(mc2.replaceAll("[WUBRGSX]", "").trim());
       tCL = cl1 + cl2;
       
       mc1 = mc1.replace(cl1.toString(), "").trim();
       mc2 = mc2.replace(cl2.toString(), "").trim();
       
       tMC = tCL.toString() + " " + mc1 + " " + mc2;
       
       return tMC;
    }


SpellAbility:

Code: Select all

public abstract class SpellAbility implements Cloneable {
...
    public SpellAbility copy()
    {
       SpellAbility clone = null;
        try {
           clone = (SpellAbility)this.clone();
        } catch (CloneNotSupportedException e) {
           System.err.println(e);
        }
        return clone;
    }


cards.txt:

Code: Select all

Brush With Death
2 B
Sorcery
no text
spLoseLifeTgt:2:Drawback$YouGainLife/2:Target opponent loses 2 life and you gain 2 life.
SVar:Buyback:2 B B


[DennisBergkamp]

Ok, I'll give it a shot.

[DennisBergkamp]

I see what's going on, and just fixed it.
You correctly copied the spellAbility, set the new cost (4 B B B) and you set it to be a buyback Ability.
However, the new copied ability will still have the original "setBeforePayMana(CardFactoryUtil.input_targetPlayer(spLoseLife));" reference. And as you can see, input_targetPlayer will call Input_PayManaCost(spell), spell in this case will still be the old one: spLoseLife. Also, you might have noticed buyback didn't happen, this is because spLoseLife has isBuyBackAbility() set to false.

So all that's needed is a new setBeforePayMana():

Code: Select all

card.clearSpellAbility();
                card.addSpellAbility(spLoseLife);
               
                String bbCost = card.getSVar("Buyback");
                if (!bbCost.equals(""))
                {
                   SpellAbility bbLoseLife = spLoseLife.copy();
                   bbLoseLife.setManaCost(CardUtil.addManaCosts(card.getManaCost(), bbCost));
                   bbLoseLife.setDescription("Buyback " + bbCost + "(You may pay an additional " + bbCost + " as you cast this spell. If you do, put this card into your hand as it resolves.)");
                   bbLoseLife.setIsBuyBackAbility(true);
                   
                   if (Tgt[0] == true)
                       bbLoseLife.setBeforePayMana(CardFactoryUtil.input_targetPlayer(bbLoseLife));
                   
                   card.addSpellAbility(bbLoseLife);
                }

[Rob Cashwalker]

Bada bing, bada boom. I knew you'd get it.

[DennisBergkamp]

I just committed this to SVN, by the way.

[zerker2000]

A few things: Ertai's Meddling, Mirari, Twincast, Fork, Reiterate, Wild Ricochet, Izzet Guildmage, Cloven Casting, Sigil Tracer, Uyo, Silent Prophet, Mirror Sheen, Mischevious Quanar, Hive Mind, Ink-Treader Nephilim, Radiate, Pyromaster Acension; proper Storm, Replicate, and Epic handling. With most all of these requiring a functional "you may choose new targets for it", and a couple "it could target".
Also, why Svar? I think the keyword should be placed as "Buyback X", with cycling, first cloning the spell and then putting the result in a wrapper. Or better yet, get resolveCommands[] working, and add a "Command{execute{sa.controller.draw()}}" to those.

[Marek14]

You forgot conspire, gravestorm (Bitter Ordeal), and...

Chain Lightning
Chain of Acid
Chain of Plasma
Chain of Silence
Chain of Smog
Chain of Vapor
Chain Stasis
Odds//Ends
Rings of Brighthearth (maybe)

[zerker2000]

Yes. I think I interpreted conspire as "copy card", and simply forgot to add Chains. Bitter Ordeal is probably better off listed as a separate card, and Rings of Brightearth are possible if Mirari is. Also, Are there enough keywordable split spells to warrant coding recognition of "//"?

[Marek14]

Split cards have their own problems, the chief being their converted mana costs.

If something asks "is the CMC of this card greater/lesser/equal to X?" the answers is true if it holds for at least one half.
If something asks "what IS the CMC of this card?" (Augury Adept, for example), it gets two answers and (generally) adds them together.

Maybe the most plausible solution would be to hard-code split cards as exceptions to the functions which deal with mana costs.

Of course, to make it funnier, none of these is true on the stack where only one half of a split card truly exists.

[Rob Cashwalker]

Also, why Svar? I think the keyword should be placed as "Buyback X", with cycling, first cloning the spell and then putting the result in a wrapper. Or better yet, get resolveCommands[] working, and add a "Command{execute{sa.controller.draw()}}" to those.

The buyback version of the SpellAbility needs be dealt with in the same handler as the regular version. So I didn't want to bother with a complex parser, which I guess it really doesn't need... it could have been done almost as elegantly as with a direct keyword syntax.

[zerker2000]

More so: it could be coded in one place for All spells, removing lines of code from cards we already have(e.g. Capsize), instead of having to hardcode it everywhere you went. Though optimally, of course, it should be parsed in Input_PayManaCost in the first place.

[Rob Cashwalker]

Well, I agree, it should probably be handled separately.... As usual, I can provide a solution that closely resembles any original Forge code, but I'm not about to go rip into the guts like that....

The reason it needs to be in with each base effect handler, is that the target inputs need to be associated with the new copy, not the original as Dennis noticed. So any effect-specific differences need to be taken care of in similar manner.

[zerker2000]

Without ripping guts, you'll never find what might be festering inside them(and considering we have SVN for precise Time-turner capabilities, "oops I broke something" shouldn't be too much of a problem) . As for inputting, I think CardFactoryUtil.input_targetValid should be expanded and spread through all old code it should replace, and then adapted. It might even be wise to make a new "Input_for_ability" class, which has a sourceSA variable and uses That instead of the one statically defined in CardFactory, and whose clone would be thrown a reference to its new source.

[Rob Cashwalker]

Actually, you got me thinking there... I looked at the base Input class, to see if there was a constructor method of any kind. There isn't currently, but I'd like to know if and how one could be added....

Adding a dynamically set sourceSpellAbility is easy enough for me to work in.

However, the thought I had was to provide an overidden constructor method in the CardFactoryUtil Inputs. The only thing this constructor would have to do is "setSourceSA(spell)" where spell is the SpellAbility being passed to CardFactoryUtil's static Input methods.
Then wherever the other methods in the Inputs reference "spell", they're changed to "getSourceSA()"

Practical upshot of this will be that the buyback code can then come along, globally and do something along the lines of this: "originalSA.getBeforePayMana().setSourceSA(bbClone)" .