Copying a SpellAbility?

[Rob Cashwalker]

So, I thought I had a good idea for implementing Buyback. Just make a copy of the spellability built by the keyword code and then change the parameters for isBuyBackAbility, change the description and then add it to the card.

But what I got was a card (Brush with Death) with two Buybacks.

I'm realizing that's because the ability object wasn't copied by value, it was copied by reference. So when I modified my copy, it really modified the original.

How could this be accomplished? Or is this why Rares wrote the SpellAbility twice in the code for other spells with Buyback?

[DennisBergkamp]

I've tried doing this myself as well, and ran into the same problems you did.
As far as I know, there's no way to copy SpellAbilities.

[Rob Cashwalker]

OK. This will be solvable when we make an ability factory. Not about to jump on that just for Buyback....

[silly freak]

maybe some sort of decorator would work... a new SpellAbility that delegates its work to the original one, plus a change to the zone the card is put into? i'm just brainstorming, it's likely not that easy, is it?

[nantuko84]

Hi!
hope I may help you here
it is possible to copy any spell or ability in MagicWars and here how it was implemented there:

Code: Select all

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

and here how I used it for Trap cards in Zendikar:

Code: Select all

public class SpellTrapFactory {

   public static void addTrap(Card card, SpellAbility spell, String trapCost, AbilityCanPlay canPlay) {
      SpellAbility trap = spell.getCopy();
      trap.setManaCost(trapCost);
      trap.setDescription("Trap: pay {" + trapCost + "} rather than " + card + "'s mana cost.");
      trap.setStackDescription("[!!!Trap] " + spell.getStackDescription());
      spell.setStackDescription("[No trap] " + spell.getStackDescription());
      if (spell instanceof GroovySpell) {
         ((GroovySpell)trap).setAbilityCanPlay(canPlay);
      }
      card.addSpellAbility(trap);
   }
}

and for kicker:

Code: Select all

public static void addKicker(final GameManager game, final Card card, SpellAbility spell, final String costWithKicker) {
      
      SpellAbility kickedSA = spell.getCopy();
      kickedSA.setKickerWasPayed(true);
      kickedSA.setManaCost(costWithKicker);
      kickedSA.setDescription("Play with kicker: " + costWithKicker);
      kickedSA.setStackDescription("[Kicked]" + spell.getStackDescription());
      card.addSpellAbility(kickedSA);
      spell.setStackDescription("[Without kicker] " + spell.getStackDescription());
}


the idea is pretty easy
hope it helps

p.s. better PM me if u have any question regarding this

regards

[zerker2000]

So basically we need SpellAbility to "implimnent cloneable" somehow, right?

[nantuko84]

So basically we need SpellAbility to "implimnent cloneable" somehow, right?

somehow? no
every Object contains clone() method
so I've written you all the code from SpellAbility you need, just copy it

regards

[Rob Cashwalker]

What's the likelyhood that it throws the exception? Does it have to be a really old Java to not support it? Then wouldn't a lot of other stuff we're using also cause exceptions on the oldest running java environments?

Reading up in the java documentation, it says that some member fields may need to become cloneable as well, but all primitives are automatically copied. Did you have to do anything else in particular? (Since the SpellAbility architecture is common the our projects)

[zerker2000]

Err, wouldn't SpellAbility.clone() execute sourceCard.clone(), which would execute sourceCard.spellAbility.clone(), which would cause a stack overflow?

[Rob Cashwalker]

If I'm understanding any of this at all, the sourceCard wasn't a copy of the card in the first place, it was a reference to the original card.

If the Card class isn't cloneable, then I'm guessing that the sourceCard continues to be just as much of a reference as it was in the first place, so it shouldn't cascade like that. As would any of the other non-primitive objects.

Arrays of various sorts are cloneable by default, according to the docs, but if we wrap any arrays inside a custom class (like CardList) then the custom class needs to become cloneable as well.

[zerker2000]

Okay... so sourceCard is going to be the original card, and therefore it is not true that sa.getSorceCard().getSpellAbility().contains(sa)? Is there anything wrong with this making me uncomfortable?

[Rob Cashwalker]

????

If sa.getSourceCard().getSpellAbility().contains(sa) was true in a non-cloned SpellAbility, then it should remain true for the clone of SpellAbility. After cloning, the new spellAbility isn't magically added to the sourceCard, it needs to be added by code in (CardFactory). After which, the card may have a number of SpellAbilities. One is an original, and another may be a clone with slightly different parameters, such as cost or buyback.....

[zerker2000]

Actually, forget my previous post, I checked the rules and it does make perfect sense for Rings of Brightearth not to add the copied ability back to the card. And sa.getSourceCard().getSpellAbility().contains(sa) might be false when(if) sa is phrased "{1}, {Q], CARDNAME loses this ability: Tap target creature an opponent controls and it gains this ability.". Actually, that would be fun indeed(to copy with Quicksilver Elemental).

[Rob Cashwalker]

I successfully made SpellAbility Cloneable last night. (OK, actually early this morning) Then I wrote a buyback skeleton, and tested it.
While I now have (seemingly) two distinct SpellAbilities, and both options show up when I try to cast the spell, the mana cost for both of them is the same. The code does a setManaCost, with a string, which is the result of combining the original cost, and the buyback cost using a method I added to CardUtil. The addManaCosts method takes the colorless numeric costs and adds them, then concatenates the colored mana.

[DennisBergkamp]

Very nice, this should facilitate the process of adding some extra buyback cards (and I'm sure it could be used for quite a few other things).