Card Development Questions

[Max mtg]

@Sloth, that was a single patch from RedDeckWins.

I would like to see a few more of his code contributions. Though I belive they won't hire to MS a person who writes bad code, a single card fixed is somewhat insufficient.

[Chris H.]

The Shaper Parasite morph trigger is broken.

From what I could debug, I found it was entering the first conditional (excerpted from SpellAbilityEffect) below, because tgt was not null. The desired behavior would be to return the defined player in ChooseGenericEffect.resolve() (line 44).

 
Thank you RedDeckWins.

[RedDeckWins]

So here is my first attempt at implementing a card. The UnlessCost method still leaves a lot to be desired, as it still can't handle a lot of cases, but I tweaked it such that Cyclone is now working. I checked to make sure Primordial Ooze was still working as well.

New Code in AbilityUtils.handleUnlessCost, last else if of the first conditional, line 1115

Code: Select all

        } else if( !StringUtils.isBlank(sa.getSVar(unlessCost)) || !StringUtils.isBlank(source.getSVar(unlessCost))) {
            // check for X costs (stored in SVars
            int xCost = calculateAmount(source, sa.getParam("UnlessCost").replace(" ", ""), sa);
            //Check for XColor
            ManaCostBeingPaid toPay = new ManaCostBeingPaid("0");
            byte xColor = MagicColor.fromName(sa.hasParam("UnlessXColor") ? sa.getParam("UnlessXColor") : "1");
            toPay.increaseShard(ManaCostShard.valueOf(xColor), xCost);
            unlessCost = toPay.toString();
        }


Card script:

Code: Select all

Name:Cyclone
ManaCost:2 G G
Types:Enchantment
T:Mode$ Phase | Phase$ Upkeep | ValidPlayer$ You | TriggerZones$ Battlefield | Execute$ TrigPutCounter | TriggerDescription$ At the beginning of your upkeep, put a wind counter on CARDNAME, then sacrifice CARDNAME unless you pay G for each wind counter on it. If you pay, CARDNAME deals damage equal to the number of wind counters on it to each creature and each player.
SVar:TrigPutCounter:AB$ PutCounter | Cost$ 0 | Defined$ Self | CounterType$ WIND | CounterNum$ 1 | SubAbility$ SacSelf
SVar:SacSelf:DB$ Sacrifice | Defined$ Card.Self | UnlessCost$ X | UnlessXColor$ G | UnlessPayer$ You | UnlessResolveSubs$ WhenPaid | SubAbility$ DBDamageAll | References$ X
SVar:DBDamageAll:DB$ DamageAll | NumDmg$ X |  ValidCards$ Creature | ValidPlayers$ Each | References$ X
SVar:X:Count$CardCounters.WIND
SVar:Picture:http://www.wizards.com/global/images/magic/general/cyclone.jpg
Oracle:At the beginning of your upkeep, put a wind counter on Cyclone, then sacrifice Cyclone unless you pay {G} for each wind counter on it. If you pay, Cyclone deals damage equal to the number of wind counters on it to each creature and each player.
SetInfo:CHR Rare
SetInfo:ARN Uncommon


[Max mtg]

New Code in AbilityUtils.handleUnlessCost, last else if of the first conditional, line 1115

Code: Select all

        } else if( !StringUtils.isBlank(sa.getSVar(unlessCost)) || !StringUtils.isBlank(source.getSVar(unlessCost))) {
            // check for X costs (stored in SVars
            int xCost = calculateAmount(source, sa.getParam("UnlessCost").replace(" ", ""), sa);
            //Check for XColor
            ManaCostBeingPaid toPay = new ManaCostBeingPaid("0");
            byte xColor = MagicColor.fromName(sa.hasParam("UnlessXColor") ? sa.getParam("UnlessXColor") : "1");
            toPay.increaseShard(ManaCostShard.valueOf(xColor), xCost);
            unlessCost = toPay.toString();
        }


You'll end up losing color restiction for X. Write unless cost as "0 X" - that will lead you past all the branches of the condition check.

See the ability declared at 1119 - you'll need to patch it: add XColor to its sVars and the very X. That'll make use of existing code that adjusts mana cost (CostPartMana:116)

[RedDeckWins]

See the ability declared at 1119 - you'll need to patch it: add XColor to its sVars and the very X. That'll make use of existing code that adjusts mana cost (CostPartMana:116)

That route does seem better. I'll give it a shot.

[RedDeckWins]

So, I've been trying to do this, and I don't think UnlessCost can ever hit CostPartMana.payHuman.

AbilityUtils.handleUnlessCost eventually uses GameActionUtil.payCostDuringAbilityResolve, which uses InputPayManaExecuteCommands for mana payments (never calls CostPart.payCost). I've set breakpoints, and at no point after hitting the UnlessCost method entry does it enter CostPartMana.payHuman. I think it is possible to change GameActionUtil to use costPart.payHuman instead of InputPayManaExecuteCommands, but I have no idea what other cards that would affect/break. It seems like a major change.

I am not sure I completely understand what you mean when you say:

You'll end up losing color restiction for X. Write unless cost as "0 X" - that will lead you past all the branches of the condition check.

If you are saying what I think you are saying, "0 X" (and "G X", "X G") all don't work in the existing implementation of handleUnlessCost either, so with the code above, we are no worse off.

EDIT:
Well, I went ahead and did it the better but more risky way. Definitely needs a review: https://dl.dropboxusercontent.com/u/129 ... e_v2.patch

[Max mtg]

Ah yes, that PayDuringAbilityResolve method uses its own routines to pay. Maybe it would be OK to adjust cost there right before InputPayManaExecuteCommands is invoked.

About change to payHuman - InputPayManaExecuteCommands can refund mana paid if cancelled. PayHuman does not, cause it's caller is supposed to do that thing. It might also have you pay multikicker cost again, if the source card had it.

"0 X" - I meant to say that if you typed just "X", you would get caught in an if-else branch at line 1113. In the change you've suggested a few posts ago you were analysing UnlessXColor, but converted the resulting manacost to string, that won't store color restriction for X as it gets back serialized into "X"... but why did I think that toString of the manaCOst you've declared would return just "X"? If it consists of simple shards that would get stored as "G G G" for instance!

I am sorry and it's my bad... your change was correct. =(

[swordshine]

What are the remaining issues to make Aven Mindcensor scriptable?

I think it's easy to modify the fetchlist in ChangeZone (and ChangeZoneAi) effect by granting the decider a "LimitLibrarySearch" keyword.
if (origin.contains(ZoneType.Library) && decider.hasKeyword("LimitSearchLibrary") && !sa.hasParam("NoLooking")) {
// Aven Mindcensor
fetchList.removeAll(player.getCardsIn(ZoneType.Library));
final int fetchNum = Math.min(player.getCardsIn(ZoneType.Library).size(), 4);
fetchList.addAll(player.getCardsIn(ZoneType.Library, fetchNum));
}

ChangeZoneAll effect might be a little annoying because we don't have a "Chooser" parameter.

[friarsol]

What are the remaining issues to make Aven Mindcensor scriptable?

I'd imagine that is all it would take.

Here's what my test scenarios would be:

Basic search (Demonic Tutor)
Keyworded search (Landcycling)
Searching Opponent's Library (Jester's Cap)
Searching multiple zones separately (Lobotomy)
Searching more than one zone contiguously (Arachnus Spinner)
Searching multiple libraries (Jace, Architect of Thought)
Multiple players searching own library (Natural Balance)

I did a quick search and the only ChangeZoneAll cards I noticed where Origin is Library are cards similar to Lobotomy. (Cranial Extraction, Eradicate)

I guess the main difference is determining the difference between "Searching" and just doing something to the library, like JTMS or Paradigm Shift.

[moomarc]

You would probably also have to exclude ChangeZone instances where a defined card is moving from the library - I can't think of any examples offhand, but I know cases like that exist where a Remembered card is moved after a dig or something.

[RedDeckWins]

4 cards have:
http://www.wizards.com.sixxs.org/ as there host for the image. Was this intentional?

[friarsol]

4 cards have:
http://www.wizards.com.sixxs.org/ as there host for the image. Was this intentional?

No, I think swordshine uses sixxs.org as a proxy, so sometimes it gets appended on some urls that he posts, and occasionally misses cleaning it up.

[swordshine]

4 cards have:
http://www.wizards.com.sixxs.org/ as there host for the image. Was this intentional?

No, I think swordshine uses sixxs.org as a proxy, so sometimes it gets appended on some urls that he posts, and occasionally misses cleaning it up.

I'll do the cleanup.

[moomarc]

Just wondering, can't Shadow of Doubt and Mindlock Orb (maybe Leonin Arbiter too) be scripted with a small change to LimitSearchLibrary to take an integer param. Aven Mindcensor would then give players the keyword LimitSearchLibrary_4 and the others listed would give LimitSearchLibrary_0.

Here's the relevant Gatherer rulings:
10/1/2005: If an effect says "You may search your library . . . If you do, shuffle your library," you can't choose to search since it's impossible, and you won't shuffle.
10/1/2005: If an effect says "Search your library . . . Then shuffle your library," the search effect fails, but you will have to shuffle.
10/1/2005: Since players can't search, players won't be able to find any cards in a library. The effect applies to all players and all libraries. If a spell or ability's effect has other parts that don't depend on searching for or finding cards, they will still work normally.

So we would just need to add in one more check at the shuffle, where if the move is optional and the player has LimitSearchLibrary_0 then don't shuffle. Does that sound right?

[swordshine]

Just wondering, can't Shadow of Doubt and Mindlock Orb (maybe Leonin Arbiter too) be scripted with a small change to LimitSearchLibrary to take an integer param. Aven Mindcensor would then give players the keyword LimitSearchLibrary_4 and the others listed would give LimitSearchLibrary_0.
...

Yeah, I think hundreds of cards need to be updated.