Rain of Riches does not cascade

[Alex-Vasile]

[[Rain of Riches]]

[github-actions[bot]]

Rain of Riches - (Gatherer) (Scryfall) (EDHREC)

{3}{R}{R} Enchantment When Rain of Riches enters the battlefield, create two Treasure tokens. The first spell you cast each turn that mana from a Treasure was spent to cast has cascade. (When you cast the spell, exile cards from the top of your library until you exile a nonland card that costs less. You may cast it without paying its mana cost. Put the exiled cards on the bottom of your library in a random order.)

[alexander-novo]

Hey has there been any update on this or is it something I could look into?

[alexander-novo]

Okay the problem with this card is that (I believe) static abilities are currently not checked between mana costs being paid for a spell and that spell being cast (and triggers being checked).

With the way static abilities currently work, we can either check the spell being cast before mana costs are being paid (which is currently what is happening), so it will never think there is treasure being spent, or we can check the spell after it has been cast (and priority attempts to move forward), which is long after the cascade ability would have checked its trigger (so adding cascade at this point does nothing).

I guess there needs to be another check for static abilities to make this card work properly. Here are some candidates:

1) After the player has finished casting the spell (including paying all costs), but before watchers and triggers are updated by the SPELL_CAST event https://github.com/magefree/mage/blob/f60b48400a58336510e7c8c89161e458f25508b8/Mage/src/main/java/mage/players/PlayerImpl.java#L1212-L1216

2) In between updating watchers and checking for triggers for all events https://github.com/magefree/mage/blob/7c49366f105b42ddf8e95d311d44478fc39ed22d/Mage/src/main/java/mage/game/GameState.java#L812-L816

I think rules-wise, the second option makes more sense and would future-proof this issue more, as static abilities often use watchers to decide what things to apply to, and it makes sense that every time we update watchers we would also want to update the things listening to them. In this case, Rain of Riches is easy to fix - simply update the watcher to look at the SPELL_CAST event, after mana costs are paid, and then the static ability would be checked before the cascade trigger is checked on the same game event. However, I also think the second option would have a larger game performance implication.

The first option would fix this immediate issue, but it would be a bit awkward. I think the rain of riches watcher would need to be rewritten to pay attention to the MANA_PAID event and reuse code from ManaPaidSourceWatcher.

@Alex-Vasile do you think you could give me your opinion on this?

[BlackWyvern]

Pinging this to confirm still bugged as of version 1.4.52-V6-beta6 (build: 2023-11-10 14:03)

[alexander-novo]

There has been a fix for a while however it’s not clear how correct it is

[xenohedron]

@JayDi85 - the proposed fix in #9785 resolves this using option 1 above. It seems acceptable to me; we need to support this rules interaction somehow. Please let us know if you have a strong opinion on it.

[Zerrisx]

Checking back in on this open issue. While not a dev, my take as a QA'r would be that we should do the awkward but acceptable fix now to make the card work. If we still have concerns, we can open up a new ticket to document why the solution wasn't ideal, when it might cause problems, and give anybody who might want to implement/refactor the more rules-accurate solution in the future a starting point and a link back to this discussion. I think that as long as we believe the fix will work and won't break anything that isn't already broken, then the current state (card doesn't work) is worse than the proposed state (card works, but fix is hacky).