forge icon indicating copy to clipboard operation
forge copied to clipboard

Fix alternative costs from static abilities

Open tchataway opened this issue 1 year ago • 2 comments

Currently, when a static ability grants an alternative cost to spells (for example, Hunting Velociraptor giving Dinosaur spells "prowl", and Henzie "Toolbox" Torre giving mana value 4 and greater creature spells "blitz"), the alternative costs only apply when casting from the Hand. In the case of Toolbox, the mana value is also ignored, allowing any creature spell to be cast from the hand with blitz.

This change allows alternative costs granted from static abilities to function properly. The main change in behaviour here is that the alternative cost can be paid whenever it is supposed to be able to be paid. E.g. if you can cast creature spells from the top of your library, you can now cast them using alternative costs granted by static abilities. Basically, if you could cast it normally, and there aren't other restrictions for the alternative cost (e.g. "flashback" and "escape" only casting from the graveyard), then you are able to cast it using that cost (relevant rules here being 601.2, 601.2b, and 601.2f-h).

Overview

  • Move "alternative costs from static ability" logic from inside getAlternativeCosts to new function getAlternativeCostsGrantedByStaticAbilities function which is called before alternative costs are retrieved for card
  • Update "alternative costs from static ability" logic to check all alternative cost keywords, not just blitz
  • Move cmcGE4 restriction from Toolbox's static ability's AddKeyword parameter to its Affected parameter
  • Change Hunting Velociraptor's static ability's AffectedZone parameter from Hand to Stack

Testing

  • No unit tests (I can add some if necessary)
  • Local testing of both Toolbox and Hunting Velociraptor, ensuring
    • Restrictions are applied
    • Cost reductions are applied

tchataway avatar Feb 15 '24 15:02 tchataway

@tool4ever remember me to recreate the Issue about May Play again

in short summary, we need to split the MayPlay logic between multiple effects that may or may not be combined:

  • Effects that alter the Cost like Rooftop Storm, that should be use a Static instead of a Keyword like Alternative Cost:0
  • Effects that allow you to cast/play stuff from zones but without setting a cost, like Augur of Autumn
  • Effects that combine both, but for these, the other effects can't be used too like Bolas's Citadel
  • The Goal also was to reduce the use of Continuous for LKI stuff

i think my first attempt to split was 3+ years ago: https://github.com/Card-Forge/forge/compare/mayPlayAlternativeCost

Hanmac avatar Feb 16 '24 16:02 Hanmac

i think in the current state, the best thing would be if we are doing it in a three step process:

  • get Base Abilities, (including Modal)
  • get possible Stack Keyword from them (this needs to apply possible Modal States for LKI)
  • and then apply MayPlay stuff (this should notice that it can't apply MayPlay WithoutCost stuff on Stack Keywords)

Hanmac avatar Mar 04 '24 10:03 Hanmac

@tool4ever to fix the problems addressed there, we probably still need to refactor MayPlay stuff

Hanmac avatar Apr 18 '24 08:04 Hanmac

This PR has not been updated in a while nad has been marked on stale. Stale PRs will be auto closed

github-actions[bot] avatar Jun 02 '24 09:06 github-actions[bot]

Gonna close this for now since the logic got shuffled around and improved a decent amount since this started But feel free to revisit this if your time allows

tool4ever avatar Jun 03 '24 13:06 tool4ever