checkStateEffects calls

[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?

[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.

[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.

[Max mtg]

removed a lot of calls to check state effects, kept only the ones before priority and cleanup phase

[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.

[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

[swordshine]

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.

[friarsol]

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.

[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.

[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

[Sloth]

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.

[Max mtg]

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

[Sloth]

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.

[Max mtg]

Combo restored by r22461

Please report other cases when the existing few calls to checkStaticAbilities don't handle game state properly