Page 1 of 1
checkStateEffects calls

Posted:
22 May 2013, 19:26
by Sloth
checkStateEffects should only be called when state based actions are check according to the rules. I removed two calls (in fillRequirements and playNoStack), because they were used in between resolving replacement effects (which is definitely incorrect).
For reference, here is the call hierarchy of checkStateEffects:
- call hierarchy of checkStateEffects | Open
- forge.card.mana.ManaPool.add(Iterable<Mana>)
forge.game.zone.MagicStack.add(SpellAbility)
forge.gui.match.controllers.CDock.concede()
forge.gui.GuiDisplayUtil.devSetupGameState()
forge.game.player.Player.doDraw()
forge.game.zone.MagicStack.finishResolving(SpellAbility, boolean)
forge.game.phase.PhaseHandler.handleBeginPhase()
forge.game.GameAction.handleLeylinesAndChancellors()
forge.game.player.Player.playLand(Card)
forge.card.cardfactory.CardFactoryCreatures.getCard_MasterOfTheWildHunt(...).MasterOfTheWildHuntAbility.resolve()
forge.card.ability.effects.EndTurnEffect.resolve(SpellAbility)
forge.game.zone.MagicStack.unfreezeStack()
At least doDraw is very suspicious. Thoughts?
Re: checkStateEffects calls

Posted:
22 May 2013, 20:01
by friarsol
Does MagicStack.unfreezeStack() even get called outside of finishResolving or MagicStack.add()?
I'm not sure about ManaPool.add() isn't that happening inside of an SA resolving too? Maybe it doesn't actually make it to that codepath.
Re: checkStateEffects calls

Posted:
03 Jun 2013, 09:24
by Max mtg
What do you think of removing all existing calls and place them only before players would get priority and on cleanup step? - that's exactly two places in PhaseHandler class
As I could understand from rule 704.3. there's no other place when state-based effects should be checked.
Re: checkStateEffects calls

Posted:
05 Jul 2013, 23:27
by Max mtg
removed a lot of calls to check state effects, kept only the ones before priority and cleanup phase
Re: checkStateEffects calls

Posted:
06 Jul 2013, 06:12
by Sloth
According to the rules, static abilities are checked all the time. Even during resolution of stuff.
This will break a ton of stuff Max. Please revert it.
EDIT: Try playing a land with Prismatic Omen and Valakut, the Molten Pinnacle (and at least 4 other lands) in play.
Re: checkStateEffects calls

Posted:
06 Jul 2013, 08:28
by Max mtg
Sloth, you are not correct now.
The test you've described does not work even in a reference revision of 22378 - so I didn't break it with change of amount of calls to check static abilities and state effects.
A good example for 'ton of broken stuff' would work in a previous revision and fail to provide an expected result in svn latest
PS: To improve Forge for your example a call to checkStaticAbilities() should be added after the card is added to new zone and before a trigger is run (that's around line 240 in forge.game.GameAction.changeZone(Zone, Zone, Card, Integer))
PS2: Still, Forge won't work correctly for Primeval Titan adding two mountains into play (with 4 mountains on the battlefield), because they are expected to ETB simultaneously - that had never been supported in Forge
Re: checkStateEffects calls

Posted:
06 Jul 2013, 10:16
by swordshine
Max mtg wrote:PS2: Still, Forge won't work correctly for Primeval Titan adding two mountains into play (with 4 mountains on the battlefield), because they are expected to ETB simultaneously - that had never been supported in Forge
I'm sure Scapeshift-Valakut combo was working one or two months ago, but in recent versions it failed again.
Re: checkStateEffects calls

Posted:
06 Jul 2013, 12:32
by friarsol
swordshine wrote:Max mtg wrote:PS2: Still, Forge won't work correctly for Primeval Titan adding two mountains into play (with 4 mountains on the battlefield), because they are expected to ETB simultaneously - that had never been supported in Forge
I'm sure Scapeshift-Valakut combo was working one or two months ago, but in recent versions it failed again.
It definitely was because I specifically tested and fixed it.
Re: checkStateEffects calls

Posted:
06 Jul 2013, 15:26
by Sloth
Here are some more scenarios that may have been broken (r22450):
1. Equip an untapped creature with Sword of the Paruns then cast Deadshot on it. Should deal 2 more damage.
2. Chaos Imps with a +1/+1 counter on it gets blocked by a creaturen with First Strike and Wither. Should loose trample.
3. You control Phyrexian Unlife and are at 2 life. Cast Glacial Ray spliced with another Glacial Ray on you. You should get 2 poison counters.
Re: checkStateEffects calls

Posted:
06 Jul 2013, 20:04
by Max mtg
1. can't reproduce. Two different creatures required - I can't understand how much damage should Deadshot deal - 1st target has P1/T1 second one has P2/T2. It deals P2 damage to second targeted creature, though as I understand the description P1 should be dealt.
Example: Grizzly Bears and Wall of Air. Wall of Air always receives 1 damage, though I expect it should take 2 damage.
2. restored with 22454
3. just the same thing
Re: checkStateEffects calls

Posted:
06 Jul 2013, 20:14
by Sloth
Max mtg wrote:1. can't reproduce. Two different creatures required - I can't understand how much damage should Deadshot deal - 1st target has P1/T1 second one has P2/T2. It deals P2 damage to second targeted creature, though as I understand the description P1 should be dealt.
Example: Grizzly Bears and Wall of Air. Wall of Air always receives 1 damage, though I expect it should take 2 damage.
Yes, the script of Deadshot was incorrect.
Re: checkStateEffects calls

Posted:
06 Jul 2013, 20:31
by Max mtg
Sloth wrote:Yes, the script of Deadshot was incorrect.
Deadshot needs even further fixing:
Grizzly Bears and Woodlot Crawler
The latter one has protection from green and cannot be dealt damage by Grizzly Bears.
The current script Deadshot works as if the Deadshot itself were dealing damage.
Meanwhile, I'll investigate where and how the Valacut - Scapeshift combo was broken
* rev 21917
Re: checkStateEffects calls

Posted:
06 Jul 2013, 20:57
by Sloth
Max mtg wrote:Deadshot needs even further fixing:
Grizzly Bears and Woodlot Crawler
The latter one has protection from green and cannot be dealt damage by Grizzly Bears.
The current script Deadshot works as if the Deadshot itself were dealing damage.
Fixed! Thanks Max.
Re: checkStateEffects calls

Posted:
06 Jul 2013, 23:32
by Max mtg
Combo restored by r22461
Please report other cases when the existing few calls to checkStaticAbilities don't handle game state properly