Delayed Triggered Abilities

[Hellfish]

r6958 introduces delayed triggered abilities and I would appreciate a few more sets of eyes on the code.

The main thing would be moving "storage" of defined cards over to the Card object in order to support Abomination-likes. I think most, if not all, delayed triggers are phase triggers so there's no worry about the delayed trigger overwriting the defined's of the original.

[slapshot5]

I'm working on converting Time Elemental to this system. During testing, I received this error which may be related to your changes:

Code: Select all

An error has occured. You can copy/paste this message or save it to a file.
Please report this, plus what you tried to do, to:
   http://www.slightlymagic.net/forum/viewforum.php?f=26
If you don't want to register an account, you can mail it directly to
   mtgerror@yahoo.com


null


Version:
Forge -- official beta: $Date: 2011-01-06 10:34:48 -0600 (Thu, 06 Jan 2011) $, SVN revision: $Revision: 4891 $

OS: Mac OS X Version: 10.6.6 Architecture: x86_64

Java Version: 1.6.0_22 Vendor: Apple Inc.

Detailed error trace:
java.lang.NullPointerException
   at java.util.Hashtable.containsKey(Hashtable.java:307)
   at forge.Card.getSVar(Card.java:583)
   at forge.CombatUtil.predictPowerBonusOfBlocker(CombatUtil.java:701)
   at forge.CombatUtil.canDestroyAttacker(CombatUtil.java:783)
   at forge.ComputerUtil_Attack2.shouldAttack(ComputerUtil_Attack2.java:486)
   at forge.ComputerUtil_Attack2.getAttackers(ComputerUtil_Attack2.java:430)
   at forge.ComputerUtil.getAttackers(ComputerUtil.java:802)
   at forge.ComputerAI_General.declare_attackers(ComputerAI_General.java:194)
   at forge.ComputerAI_Input.think(ComputerAI_Input.java:52)
   at forge.ComputerAI_Input.showMessage(ComputerAI_Input.java:31)
   at forge.GuiInput.setInput(GuiInput.java:27)
   at forge.GuiInput.update(GuiInput.java:21)
   at java.util.Observable.notifyObservers(Observable.java:142)
   at java.util.Observable.notifyObservers(Observable.java:98)
   at forge.MyObservable.updateObservers(MyObservable.java:9)
   at forge.Phase.nextPhase(Phase.java:379)
   at forge.GuiDisplay4$25.actionPerformed(GuiDisplay4.java:740)
   at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:2028)
   at javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2351)
   at javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:387)
   at javax.swing.DefaultButtonModel.setPressed(DefaultButtonModel.java:242)
   at javax.swing.plaf.basic.BasicButtonListener$Actions.actionPerformed(BasicButtonListener.java:287)
   at javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1639)
   at javax.swing.JComponent.processKeyBinding(JComponent.java:2851)
   at javax.swing.JComponent.processKeyBindings(JComponent.java:2886)
   at javax.swing.JComponent.processKeyEvent(JComponent.java:2814)
   at java.awt.Component.processEvent(Component.java:6129)
   at java.awt.Container.processEvent(Container.java:2085)
   at java.awt.Component.dispatchEventImpl(Component.java:4714)
   at java.awt.Container.dispatchEventImpl(Container.java:2143)
   at java.awt.Component.dispatchEvent(Component.java:4544)
   at java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1850)
   at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:712)
   at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:990)
   at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:855)
   at java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:676)
   at java.awt.Component.dispatchEventImpl(Component.java:4586)
   at java.awt.Container.dispatchEventImpl(Container.java:2143)
   at java.awt.Window.dispatchEventImpl(Window.java:2478)
   at java.awt.Component.dispatchEvent(Component.java:4544)
   at java.awt.EventQueue.dispatchEvent(EventQueue.java:635)
   at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:296)
   at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:211)
   at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:201)
   at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:196)
   at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:188)
   at java.awt.EventDispatchThread.run(EventDispatchThread.java:122)


CombatUtil.java:701 is:

Code: Select all

String ability = defender.getSVar(trigParams.get("Execute"));

My Time Elemental code is:

Code: Select all

Name:Time Elemental
ManaCost:2 U
Types:Creature Elemental
Text:no text
PT:0/2
A:AB$ ChangeZone | Cost$ 2 U U T | ValidTgts$ Permanent.unenchanted | TgtPrompt$ Select target unenchanted permanent | Origin$ Battlefield | Destination$ Hand | SpellDescription$ Return target permanent that isn't enchanted to its owner's hand.
T:Mode$ Attacks | ValidCard$ Card.Self | DelayedTrigger$ DelayedTrig | TriggerDescription$ When CARDNAME attacks or blocks, at end of combat, sacrifice it and it deals 5 damage to you.
T:Mode$ Blocks | ValidCard$ Card.Self | DelayedTrigger$ DelayedTrig | Secondary$ True | TriggerDescription$ When CARDNAME attacks or blocks, at end of combat, sacrifice it and it deals 5 damage to you.
SVar:DelayedTrig:Mode$ Phase | Phase$ EndCombat | ValidPlayer$ Each | Execute$ TrigSac | TriggerDescription$ At end of combat, sacrifice CARDNAME and CARDNAME deals 5 damage to you.
SVar:TrigSac:AB$Sacrifice | Cost$ 0 | Defined$ Self | SubAbility$ SVar=DBDamage
SVar:DBDamage:DB$DealDamage | NumDmg$ 5 | Defined$ You
SVar:RemAIDeck:True
SVar:Rarity:Rare
SVar:Picture:http://www.wizards.com/global/images/magic/general/time_elemental.jpg
SetInfo:5ED|Rare|http://magiccards.info/scans/en/5e/129.jpg
SetInfo:4ED|Rare|http://magiccards.info/scans/en/4e/108.jpg
SetInfo:LEG|Rare|http://magiccards.info/scans/en/lg/81.jpg
End


-slapshot5

[Hellfish]

Ah, I wasn't aware the combat code was using the trigger system at that level. I'll fix it up.
EDIT: Fixed.
EDIT2: Hah, double-fixed. Thanks, Sloth!

[Sloth]

r6958 introduces delayed triggered abilities and I would appreciate a few more sets of eyes on the code.

The main thing would be moving "storage" of defined cards over to the Card object in order to support Abomination-likes. I think most, if not all, delayed triggers are phase triggers so there's no worry about the delayed trigger overwriting the defined's of the original.

Storing the trigger on the afected card would also make it easier to make the AI realize what's going on with a card.

EDIT: Fixed.
EDIT2: Hah, double-fixed. Thanks, Sloth!

I didn't even read the post by slapshot. Just noticed it myself. There should be an error proof checking for HashMap entries. Something like: keyEquals(key,entry) that returns false if the key does not exist.

[Zirbert]

Is Cleanup a valid phase for delayed triggers? I expected it to be, based on the discussion over in the Issue 113 thread, but I can't get this attempt at Thawing Glaciers (I thought just for a change I'd try scripting a card somebody might want to play with) to bounce:

Code: Select all

Name:Thawing Glaciers
ManaCost:no cost
Types:Land
Text:no text
K:CARDNAME enters the battlefield tapped.
A:AB$ChangeZone | Cost$ 1 T | Origin$ Library | Destination$ Battlefield | Tapped$ True | ChangeType$ Land.Basic | ChangeNum$ 1 | DelayedTrigger$ DelayedTrig | SpellDescription$ Search your library for a basic land card, put that card onto the battlefield tapped, then shuffle your library. Return CARDNAME to its owner's hand at the beginning of the next cleanup step.
SVar:DelayedTrig:Mode$ Phase | Phase$ Cleanup | ValidPlayer$ Each | Execute$ TrigBounce | TriggerDescription$ Return CARDNAME to its owner's hand at the beginning of the next cleanup step.
SVar:TrigBounce:AB$ChangeZone | Cost$ 0 | Origin$ Battlefield | Destination$ Hand | SpellDescription$ Return CARDNAME to its owner's hand.
SVar:Rarity:Rare
SVar:Picture:http://www.wizards.com/global/images/magic/general/thawing_glaciers.jpg
SetInfo:ALL|Rare|http://magiccards.info/scans/en/ai/189.jpg
End

But then again, changing the DelayedTrig phase to Upkeep doesn't do it either.

As always - what am I missing here?

[Sloth]

Is Cleanup a valid phase for delayed triggers? I expected it to be, based on the discussion over in the Issue 113 thread, but I can't get this attempt at Thawing Glaciers (I thought just for a change I'd try scripting a card somebody might want to play with) to bounce:

But then again, changing the DelayedTrig phase to Upkeep doesn't do it either.

As always - what am I missing here?

The Delayed Trigger parameter is only available for Triggers at the moment (starting with T:Mode$ ). Remember your card, since the implementation should be just like you did it.

[Hellfish]

Oop,my fault for being unclear. Sloth is correct. Delayed Triggers on abilities and spells are in the cards(Hah!), though.

[friarsol]

The Delayed Trigger parameter is only available for Triggers at the moment (starting with T:Mode$ ). Remember your card, since the implementation should be just like you did it.

Can we pump Triggers yet? If we could, then a pumped Delayed Trigger here might do the trick.

[Hellfish]

Currently we can't pump triggers and that will probably work in a slightly different way, anyway. I'm pondering if it(delayed triggers from abilities) should be handled similar to subability's (a short if-clause at the end of each AF's resolve method). I don't know how to best handle passing the target(s) of the ability to the trigger,though.

EDIT: Thawing Glacier's work with the above though.

[slapshot5]

r7449

Possible bug in delayed triggers:
If the source of the delayed trigger is not on the battlefield when the delayed portion goes on the stack, the delayed trigger does not resolve properly

repro:
block a comp creature with power >= 4 with a Thicket Basilisk

notice:
1. the trigger goes on the stack
1a. Thicket Basilisk is destroyed
1b. advance to end of combat
2. the delayed part goes on the stack
3. the attacker is not destroyed

repeating with blocking a creature with power < 4 results in the attacker being destroyed at EndCombat as expected.

-slapshot5

[Hellfish]

Fixed, somewhere between the initial trigger and the delayed trigger, the Thicket Basilisk Card object was copied which made it forget it's triggering objects. The copy function now copies those as well.

[slapshot5]

Fixed, somewhere between the initial trigger and the delayed trigger, the Thicket Basilisk Card object was copied which made it forget it's triggering objects. The copy function now copies those as well.

It gets copied when it's moved to the graveyard, which explains why it was fine when not destroyed. Thanks for the fix.

-slapshot5