Two cards with {cost}, Sacrifice CARDNAME: Add {mana} to yo

[Sloth]

I found 2 cards that can be added to cards.txt:

Code: Select all

Implements of Sacrifice
2
Artifact
no text
1, tap, Sacrifice CARDNAME: Add W W to your mana pool.
1, tap, Sacrifice CARDNAME: Add U U to your mana pool.
1, tap, Sacrifice CARDNAME: Add B B to your mana pool.
1, tap, Sacrifice CARDNAME: Add R R to your mana pool.
1, tap, Sacrifice CARDNAME: Add G G to your mana pool.

Kaleidostone
2
Artifact
no text
When CARDNAME enters the battlefield, draw a card.
5, tap, Sacrifice CARDNAME: Add W U B R G to your mana pool.


in card-pictures.txt:

Code: Select all

implements_of_sacrifice.jpg      http://www.wizards.com/global/images/magic/general/implements_of_sacrifice.jpg
kaleidostone.jpg      http://www.wizards.com/global/images/magic/general/kaleidostone.jpg


Thanks for adding Grave-Shell Scarab Chris!

PS: Little Fix: Fire-Field Ogre has an Unearth cost of U B R not 1 U B R

[Chris H.]

Thank you for your submissions Sloth. Unfortunately, the Implements of Sacrifice card displays the same cosmetic problem as the Wild Cantor card.

Cards that have more than one copy of this keyword bring up a Choose dialog where we choose one of these spell abilities from a list. "CARDNAME" is displayed in the listing and is not being replaced automatically with the card's name.

I found several lines of code in the Ability_Mana.java class and I discovered that I could add several lines of code which would give us several versions of this keyword. We could use this version where there is only one spell ability:

Code: Select all

{cost}, Sacrifice CARDNAME: Add {mana} to your mana pool.

`
and then use one of the two versions below for cards that have more than one spell ability:

Code: Select all

{cost}, Sacrifice this card: Add {mana} to your mana pool.
{cost}, Sacrifice this creature: Add {mana} to your mana pool.

`
This would prevent the term "CARDNAME" from appearing in the list of choices. "this card" and "this creature" might be less unsightly for our user-base.

[Rob Cashwalker]

Just guessing, but try modifying SpellAbility.getSpellDescription() to replace CARDNAME, just like Card.getText().

[Chris H.]

Just guessing, but try modifying SpellAbility.getSpellDescription() to replace CARDNAME, just like Card.getText().

`
Thank you, Rob.

I will take a look at this section of code and will see if I can figure this out.

[Chris H.]

I examined the code for Card.getText() and the code for SpellAbility.setDescription() and discovered that they both include a line which replaces "CARDNAME" with .getName().

I think that the option panel that asks us to "Choose" from a list of spell abilities is displaying these from a list of keywords. The code which replaces "CARDNAME" with .getName() does not change the actual keyword but the spell description text?

[Rob Cashwalker]

That's odd.

The idea is that the "keyword" representation of an ability should stay the same, while the outward appearance is that CARDNAME is replaced on the fly.

The GUI for the list may be called in different ways, so the ManaAbility may populate the list differently, I guess.

[DennisBergkamp]

Yeah, exactly... I've played around with this a bit and couldn't really find a way to do a string replace.

[Chris H.]

This gets even odder. I replaced all of the CARDNAME with the actual card's name for these cards in cards.txt. I then found and modified two sections of code in Abiility_Mana.isSacrifice()

Code: Select all

    public boolean isSacrifice()
    {
       return orig.contains("Sacrifice CARDNAME: Add ") ||
             orig.contains("Sacrifice " + sourceCard.getName() + " : Add ");
    }

`
and in Ability_Mana.mana()

Code: Select all

       else if ((orig.contains("Sacrifice this creature: Add ") ||
             orig.contains("Sacrifice CARDNAME: Add ") ||
             orig.contains("Sacrifice " + sourceCard.getName() + ": Add "))
             
             && orig.contains(" to your mana pool."))
       {
          String m = orig.split(": Add ")[1].split(" to ")[0];
          return m;
       }

`
and ran a test deck. The cards in question still display CARDNAME in the choose window. I may have made a simple mistake. I will take a second look at this later. But I am starting to feel that it is time to move on.

[Rob Cashwalker]

It starts in GameAction.playCard:

Code: Select all

} else {
            SpellAbility[] choices = canPlaySpellAbility(c.getSpellAbility());
            SpellAbility sa;
            /*
             System.out.println(choices.length);
             for(int i = 0; i < choices.length; i++)
                 System.out.println(choices[i]);
            */
            if(choices.length == 0) return;
            else if(choices.length == 1) sa = choices[0];
            else {
               sa = AllZone.Display.getChoiceOptional("Choose", choices);


Display.getChoiceOptional:

Code: Select all

public <T> T getChoiceOptional(String message, T... choices);


Wow... um... huh?

Searching for "getChoiceOptional" I find the real meat of it in GuiDisplay2 and GuiDisplay3:

Code: Select all

    public <T> T getChoiceOptional(String message, T... choices) {
        if(choices == null || choices.length == 0) return null;
        List<T> choice = getChoices(message, 0, 1, choices);

...
    public <T> List<T> getChoices(String message, int min, int max, T... choices) {
        ListChooser<T> c = new ListChooser<T>(message, min, max, choices);
        final JList list = c.getJList();
        list.addListSelectionListener(new ListSelectionListener() {
            public void valueChanged(ListSelectionEvent ev) {
                if(list.getSelectedValue() instanceof Card) {
                    setCard((Card) list.getSelectedValue());
                }
            }
        });
        c.show();
        return c.getSelectedValues();


ListChooser:

Code: Select all

   public ListChooser(String title, String message, int minChoices, int maxChoices, List<T> list) {
        this.title = title;
        this.minChoices = minChoices;
        this.maxChoices = maxChoices;
        this.list = unmodifiableList(list);
        jList = new JList(new ChooserListModel());
        ok = new CloseAction(OK_OPTION, "OK");
        ok.setEnabled(minChoices == 0);
        cancel = new CloseAction(CANCEL_OPTION, "Cancel");
       
        Object[] options;
        if(minChoices == 0) options = new Object[] {new JButton(ok), new JButton(cancel)};
        else options = new Object[] {new JButton(ok)};
       
        p = new JOptionPane(new Object[] {message, new JScrollPane(jList)}, QUESTION_MESSAGE, DEFAULT_OPTION,
                null, options, options[0]);


Notice, nowhere along the way here, did any of this code attempt to extract the spell description. So internally, the list chooser must try to use ".toString()".
SpellAbilityList has a toString method, as does SpellAbility itself. But since Ability_Mana extends SpellAbility, the Ability_Mana.toString overrides it:

Code: Select all

    public String toString() {
        return orig;


orig is used throughout Ability_Mana looking for "CARDNAME" to key off the different effects. So it hasn't received any replacement.

BTW, SpellAbility.toString is bad as well:

Code: Select all

    public String toString() {
       if (description.contains("CARDNAME"))
          description = description.replace("CARDNAME", this.getSourceCard().getName());
        return description;


It modifies the underlying description. So if some effect wants to examine the actual description for CARDNAME later, it can't. "getDescription" should be separate from "toString", though in "getDescription" sometimes you might want to see "CARDNAME" and sometimes you don't....

[Chris H.]

Thank you for looking into this situation.

Wow... um... huh?

`

Most of the code that you point out is over my level of knowledge and comprehension. I guess with a worst case scenario I could convert that one keyword to use "this creature" or "this card".

[silly freak]

Code: Select all

public <T> T getChoiceOptional(String message, T... choices);

Wow... um... huh?

the "<T>" is a generic declaration. essentially, it means that the compiler replaces the type when calling. "T..." is a "vararg" array. for example, you could call it like

Code: Select all

String choice = getChoiceOptional("Title", "a", "b");


The compiler translates this into

Code: Select all

String choice = getChoiceOptional("Title", new String[] {"a", "b"});


and also knows that <T> is, for this single call, <String>

that's the magic

[Rob Cashwalker]

So I'm correct in guessing that the chooser is using toString (a semi-standard method most objects should implement) to generate the list? Therefore changing the Ability_Mana.toString to include the replacement should do the trick?