-
Notifications
You must be signed in to change notification settings - Fork 782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement "perpetually" mechanic #13016
base: master
Are you sure you want to change the base?
Conversation
…and few perpetually cards from another sets.
# Conflicts: # Mage.Sets/src/mage/sets/AlchemyInnistrad.java
This will be a lot to review, but thanks for writing out a clear description with references. |
That's PR contains many game engine changes to support new type of effects. Maybe it can be implemented more easy by standard rules and global static effect like If it's a more complex logic than that PR can be accepted (after reviews). |
I was brave enough to try implementing this digital mechanic and I hope that I didn't break anything else in process.
"Perpetually" is complex mechanic that isn't described in Comprehensive Rules, so my main source of information is MtGA experiments. And also this very useful third-party research.
According to that, there are core principles:
Summary of my pull-request:
PerpetuallyEffect
interface that helps any perpetual effect to keep its features (meld cards and commander workarounds) and to be recognisedBoostTargetPerpetuallyEffect
,GainAbilityTargetPerpetuallyEffect
,SetBasePowerToughnessTargetPerpetuallyEffect
)BoostSourcePerpetuallyEffect
,CardsInYourHandPerpetuallyGainEffect
,ChooseACardInYourHandItPerpetuallyGainsEffect
ContinuousEffects
: new methods for handling perpetual effects and map for storing perpetual rules that need to be coloredCardUtil
: colorizing perpetual rules text and their summationPlayerImpl
,CommanderReplacementEffect
andGameImpl
: optional removing perpetual effects from commander (arena update highlights)GameState
: I've added one condition toaddOtherAbility()
because of ability didn't change its controller and perpetually gained activated abilities attached to "stolen" card were not active (I hope it's not gamebreaking), also there's adventure and split cards workaroundCardImpl
: workaround for adventure and split cards that allows their abilities to trigger on stack and their perpetual abilities to be returnedCardView
and other client-focused changes: are made for colorizing perpetually affected P/TSpellCostIncreaseSourceEffect
(wrong Outcome)Also I wrote
PerpetuallyTest
where I tried to describe and validate main and troublesome interactions with perpetual effects. Most of the described things are tested here except of rules generation.Of course, there's bunch of example cards that helped me in general testing and writing unit-tests, but... I implemented more cards than had expected. I hope that perpetual effects structure review would have higher priority than 21 implemented cards which use its functionality. There're controversial cards, for example 'Davriel, Soul Broker' and lack of its emblem images.
The only thing where I've left TODO is "inactive check". There's no >3 players games in Arena so we don't know should perpetual effects be removed after their owner's leave or not. I hope to find answer in discussion.
Also fused cards aren't present in Arena, so perpetual effects don't apply on them in current version. This fact also needs discussion.