fix Release to the Wind, small rework/clean for edge cases #12226
Conversation
| } | ||
| } | ||
| return ActivationStatus.getFalse(); | ||
| } | ||
|
|
||
| private ActivationStatus canActivateUsingForetell(SpellAbility spellAbility, UUID playerId, Game game) { | ||
| return spellAbility.copy().setSpellAbilityCastMode(SpellAbilityCastMode.FORETELL).canActivate(playerId, game); |
There was a problem hiding this comment.
Do you understand the problem this is solving?
There was a problem hiding this comment.
Yes, I understand -- you introduced new workaround with injecting global identifier in abilities, it doesn't work with game engine and that's second workaround required to support it.
There was a problem hiding this comment.
small rework/clean for edge cases
It's not small rework. There are multiple changes with many workaround/hacks all around game engine.
Most important problem: it also uses objects modifications on playable calc -- it's smells very bad and do not contains any comments about it. Must be more safer way to do same things in read only.
Please, split it by smaller steps/PRs.
- Fixed card/implementation;
- Broken use cases/tests;
- Migrate all old cards/abilities/code to new version -- xmage must use same code/logic for all.
Current PR can't be merged without each workaround reviewed and checked.
Susucre
force-pushed
the
fix-release-to-the-wind
branch
from
ad52436
to
8dd3f4a
Compare
Susucre
force-pushed
the
fix-release-to-the-wind
branch
from
8dd3f4a
to
7c6cf00
Compare
Fix #12224
This is part of the general work to consolidate "you may play/you may cast" effects, add unit tests and fix edge cases.
Changes are along several axes:
MayCastTargetCardEffecteffect.MayCastTargetCardEffectnow supports:MageIdentifier.WithoutPayingManaCostAlternateCasthas been introduced to support the last bullet point (and not affect other alternate ways to cast the card from exile. e.g. [[Misthollow Griffin]])AdventureSpellAbilityImpl::canActivatewas not correct in edge cases. See the two TODO comments on weird choice in the Kess test and the AdventureCards test, the adventure cast was sometimes wrongly enabled to be activated, and it is now fixed.Plot(resp.Foretell) are now calling thecanActivatemethod of theSpellAbilitythey want to cast, usingSpellAbilityCastMode.PLOT(resp. the newly addedSpellAbilityCastMode.FORETELL). This allow bypassing the "from your hand" check inAdventureSpellAbilityImpl::canActivate, as this way of casting does not want to actually check if the card can be cast from hand or with an activator.