Implement prepare mechanic (cast spell face while prepared)#15599
Implement prepare mechanic (cast spell face while prepared)#15599wakame1367 wants to merge 2 commits into
Conversation
Complete the "prepare" mechanic (Secrets of Strixhaven), previously a
marker-only stub ("TODO: Implement properly"). The prepared designation
machinery from magefree#14846 was already present; this fills in the missing core
behavior: while a permanent is prepared, its controller may cast a copy of
its prepare spell (CR 722.4), following that spell's timing, and doing so
unprepares the permanent. The permanent stays on the battlefield.
Core (Mage):
- SpellAbilityType: add PREPARE_SPELL.
- PrepareSpellAbility (new): the prepare spell's SpellAbility. It lives in the
battlefield zone so it is offered on the prepared permanent, only activates
while that permanent is prepared, and carries the prepare spell's timing
(sorcery/instant) so sorcery-typed prepare spells are sorcery-speed only.
- PrepareSpellCard: hold the PrepareSpellAbility and, on cast, create a copy of
itself (the blueprint is never moved, so it can be reused) via game.copyCard,
cast that copy through ZonesHandler.cast, and unprepare the source permanent.
- PrepareCard: getSpellCard() now returns the concrete PrepareSpellCard and the
copy constructor copies the blueprint; implementations still add effects and
targets to getSpellCard().getSpellAbility().
- GameImpl.loadCards: register the prepare spell card (kept out of normal zones)
so its copy can be created when cast.
- PlayerImpl.getPlayableFromObjectAll: offer the prepare spell's ability from a
prepared permanent on the battlefield.
Cards (Mage.Sets):
- Emeritus of Abundance, Emeritus of Ideation and Vastlands Scavenger are now
functional and removed from the Secrets of Strixhaven unfinished list.
- VastlandsScavenger: add the missing EntersPreparedAbility (it had only the
comment, so the creature never actually entered prepared).
Tests: add PrepareCardsTest (5 tests). Regression: OmenCardsTest and
AdventureCardsTest (neighboring CardWithSpellOption casting path) still pass.
Part of magefree#14329.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| * Tests for the "prepare" mechanic: while a permanent is prepared, its controller may cast a copy of | ||
| * its prepare spell (following that spell's timing), and doing so unprepares the permanent. | ||
| * | ||
| * @author TheElk801 |
| this.addAbility(DeathtouchAbility.getInstance()); | ||
|
|
||
| // This creature enters prepared. | ||
| this.addAbility(new EntersPreparedAbility()); |
There was a problem hiding this comment.
Good catch. Surprised to see this change but it indeed looks like we missed this on the initial card implementation.
| */ | ||
| public final class SecretsOfStrixhaven extends ExpansionSet { | ||
|
|
||
| private static final List<String> unfinished = Arrays.asList("Abigale, Poet Laureate", "Adventurous Eater", "Blazing Firesinger", "Campus Composer", "Cheerful Osteomancer", "Decorum Dissertation", "Echocasting Symposium", "Elite Interceptor", "Emeritus of Abundance", "Emeritus of Conflict", "Emeritus of Ideation", "Emeritus of Truce", "Emeritus of Woe", "Encouraging Aviator", "Germination Practicum", "Goblin Glasswright", "Grave Researcher", "Harmonized Trio", "Honorbound Page", "Improvisation Capstone", "Infirmary Healer", "Jadzi, Steward of Fate", "Joined Researchers", "Kirol, History Buff", "Landscape Painter", "Leech Collector", "Lluwen, Exchange Student", "Maelstrom Artisan", "Pigment Wrangler", "Quill-Blade Laureate", "Restoration Seminar", "Sanar, Unfinished Genius", "Scathing Shadelock", "Scheming Silvertongue", "Skycoach Conductor", "Spellbook Seeker", "Spiritcall Enthusiast", "Strife Scholar", "Studious First-Year", "Tam, Observant Sequencer", "Vastlands Scavenger"); |
There was a problem hiding this comment.
Honestly, if the mechanic is truly ready, all of these should be given a quick sanity check and we remove this variable and the functionality to omit showing them in game. The working assumption right now is that the Prepare mechanic isn't implemented, but once it is, those card implementations should just be good to go once it is correctly put in place.
| * prepare spell can only be cast at sorcery speed. Casting it creates and casts a copy of the prepare | ||
| * spell (see {@link PrepareSpellCard#cast}). | ||
| * | ||
| * @author TheElk801 |
There was a problem hiding this comment.
Again. This might seem minor and I'm honestly not here to argue for or against AI usage in general, but if you are going to use it, be sure to review what it's generated thoroughly before putting the onus on others to review it too.
In principle, falsely attributing credit to someone else isn't particularly cool.
Broadly, if minor things like this have escaped human review, the assumption (and admittedly, it could be flawed) is that there's larger problems that have not been scrutinized adequately too.
Grath
left a comment
There was a problem hiding this comment.
This will need significant revision to implement the actual rules and mechanisms of Prepare, not whatever incorrect information you got (presumably via an AI, given the Claude co-authorship) about how prepare works.
|
|
||
| @Override | ||
| public boolean cast(Game game, Zone fromZone, SpellAbility ability, UUID controllerId) { | ||
| // CR 722.4b To cast a copy of a prepare spell, its controller creates a copy of the prepare |
There was a problem hiding this comment.
Your AI hallucinated something else that is not the rules text of Prepare; the actual relevant rules are:
722.3c As a permanent with a prepare spell gains the prepared designation or phases in prepared, its controller creates a copy of that object in exile, except that copy has only the characteristics of that permanent’s prepare spell, ignoring other exceptions to the copying process that apply to that permanent. Those characteristics become the copy’s normal characteristics. This copy remains in exile for as long as the prepared permanent remains on the battlefield and has the prepared designation. This is an exception to rule 704.5e. For as long as the copy remains in exile, the prepared permanent’s controller may cast the copy. That permanent loses the prepared designation at the time the spell becomes cast (see rule 601.2i).
| spellCard.setOwnerId(ownerId); | ||
| addCardToState(spellCard); | ||
| } else if (card instanceof PrepareCard) { | ||
| // the prepare spell is never in a normal zone; it is a blueprint used to create the |
There was a problem hiding this comment.
722.3c, it's actually just a non-card object which exists in the Exile zone which you may cast following all the normal rules for casting a card from exile so this should not be necessary (but adding it as an exception to "non-card things in exile immediately cease to exist" from 704.5e will be necessary.)
PrepareSpellAbility and PrepareCardsTest are new in this PR and were incorrectly attributed to TheElk801 (copied from neighboring file headers). Credit belongs to the PR author. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
An important test to add, once 722.3 is implemented correctly, because it will cover a lot of the worst corner cases:
|
|
Thanks @muz and @Grath for the quick and careful review. All three points are fair.
To be transparent: I do use AI assistance for this work. The thing that failed here was my own review gate, and I'm fixing that on my side as well. Will re-request review once tests are green. |
|
Closing in favor of #15601 — nandmp's implementation follows the CR 722.3c exile-copy model correctly and is more complete than this one (including client-side rendering). Thanks @muz and @Grath for the review that made the right architecture clear. @Grath — that copy-effect corner case is a great one; I'll pass it along to #15601. The Vastlands Scavenger |
The card text says "This creature enters prepared." but the ability was never added, so the permanent entered without the prepared designation. Uses the existing EntersPreparedAbility. Surfaced during review of #15599. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What this does
Completes the prepare mechanic (Secrets of Strixhaven) so a prepared permanent's controller can actually cast a copy of its prepare spell.
The battlefield already had the prepared designation machinery from #14846 (
Permanent#isPrepared/setPrepared,EntersPreparedAbility,BecomePreparedSource/TargetEffect, CR 722.3a), and thePrepareCard/PrepareSpellCardblueprint classes existed asTODO: Implement properlystubs. This PR fills in the missing core behavior: while a permanent is prepared, its controller may cast a copy of the prepare spell (CR 722.4), following that spell's timing, and doing so unprepares the permanent (the permanent stays on the battlefield).Design
A prepare spell is never cast directly and its blueprint card is never moved between zones — it is a template used to produce the copy that is actually cast, the same idiom used elsewhere in the engine (e.g. Isochron Scepter casting an imprinted copy).
SpellAbilityType.PREPARE_SPELL— new type for the prepare spell's ability.PrepareSpellAbility(new) — the prepare spell'sSpellAbility. Its zone isBATTLEFIELDso it is offered on the prepared permanent;canActivateonly returns true while that permanent is prepared; and it carries the prepare spell'sTimingRule(instant vs. sorcery), so a sorcery-typed prepare spell is sorcery-speed only.PrepareSpellCard#cast— on cast it creates a copy of itself withgame.copyCard, casts the copy viaZonesHandler.cast(marking itsetCopy(true, ...)), then sets the source permanent unprepared (CR 722.4a). The blueprint is left untouched so it can be reused if the permanent becomes prepared again.PrepareCard—getSpellCard()now returns the concretePrepareSpellCard, and the copy constructor copies the blueprint. Card implementations still add effects/targets togetSpellCard().getSpellAbility().Two minimal hooks make the blueprint reachable without disturbing normal zone handling:
GameImpl#loadCards— registers the prepare spell card in game state (like the existingCardWithSpellOptionbranch) so its copy can be created on cast. It is otherwise kept out of the normal zones.PlayerImpl#getPlayableFromObjectAll— offers the prepare spell's ability from a prepared permanent on the battlefield (characteristics come from the prepare spell card, not the permanent).Both hooks sit right next to the analogous adventure/omen (
CardWithSpellOption) handling, so the change is additive and touches nothing on the common casting path.Cards
The three Secrets of Strixhaven prepare cards are now functional and removed from the set's
unfinishedlist:EntersPreparedAbility(it only had the comment, so the creature never actually entered prepared).Tests
New
PrepareCardsTest(5 tests, all passing) covering:Existing
OmenCardsTestandAdventureCardsTest(which exercise the neighboringCardWithSpellOptioncasting path touched by the hooks) still pass.Part of the Secrets of Strixhaven implementation tracking issue #14329.