Refactor: Hidden keywords to StaticAbilityMode (Issue #3307)#9703
Refactor: Hidden keywords to StaticAbilityMode (Issue #3307)#9703calaespi wants to merge 20 commits intoCard-Forge:masterfrom
Conversation
|
You probably should check out the other StaticAbility classes, like using something like |
| S:Mode$ Continuous | Affected$ Creature.EnchantedBy | AddHiddenKeyword$ All creatures able to block CARDNAME do so. | Description$ All creatures able to block enchanted creature do so. | ||
| S:Mode$ Continuous | Affected$ Creature.EnchantedBy | AddStaticAbility$ SMustBlock | Description$ All creatures able to block enchanted creature do so. | ||
| SVar:SMustBlock:Mode$ MustBeBlockedByAll | ||
| Oracle:Enchant creature\nAll creatures able to block enchanted creature do so. |
There was a problem hiding this comment.
If MustBeBlockedByAll is done as a static, it should not be added to the enchanted creature anymore.
Notice how the Aura doesn't say: enchanted creature gains
There was a problem hiding this comment.
imo at least all MustBeBlockedBy variants should be handled together
but maybe remove it from this one so we can merge in smaller steps
and you have the static already as part of #4745 instead
|
I did the CountersRemain as extra MR: #9709 |
…ngesZone excludes Stack/Exile + Exiled). Initialize Localizer/Lang to prevent NPE during GameType init.
…tions to aid Outpost Siege simulations. Keep Issue4745Test disabled until full phase-driven harness is ready.
Harness de integración: Upkeep y TestAction para Outpost Siege (Issue Card-Forge#4745)
…ngesZone excludes Stack/Exile + Exiled). Initialize Localizer/Lang to prevent NPE during GameType init.
Asegurar ForgetOnMoved: unit test y init de localización
|
Merged master (incorporating StaticAbilityCountersRemain) and refactored CantGainControl, LethalDamageByPower, and BounceAtUntap to use dedicated StaticAbility classes as requested. Also removed the redundant RetainCounters mode. |
| // this is the amount of damage a creature needs to receive before it dies | ||
| public final int getLethal() { | ||
| if (hasKeyword("Lethal damage dealt to CARDNAME is determined by its power rather than its toughness.")) { | ||
| boolean lethalByPower = hasKeyword("Lethal damage dealt to CARDNAME is determined by its power rather than its toughness.") || StaticAbilityLethalDamageByPower.isLethalDamageByPower(this); |
There was a problem hiding this comment.
I'd prefer the new approach?
There was a problem hiding this comment.
Just to clarify: the current implementation already uses the cached flag approach (checking !lethalDamageByPower.isEmpty()) to avoid the expensive iteration. I assume this is the "new approach" you are referring to?
There was a problem hiding this comment.
👍 on that part
If the card gets updated, the keyword can be removed
Also remove the static mode that was added because of it
|
Refactored |
| return; | ||
| } | ||
|
|
||
| boolean cantGainControl = c.hasKeyword("Other players can't gain control of CARDNAME.") || StaticAbilityCantGainControl.cantGainControl(c); |
There was a problem hiding this comment.
this seems fine
but there is another place with the KW
remove checking for it from both
|
|
||
| // Simulate Detain effect | ||
| // Detain adds "CARDNAME can't attack or block." and "CARDNAME's activated abilities can't be activated." | ||
| bear.addIntrinsicKeyword("CARDNAME can't attack or block."); |
There was a problem hiding this comment.
shouldn't test for outdated KW
|
I have merged the latest changes from upstream/master and resolved the conflicts in AbilityActivated.java. |
There was a problem hiding this comment.
reverting too much now...
| @@ -0,0 +1,89 @@ | |||
| package forge.gamesimulationtests; | |||
There was a problem hiding this comment.
not sure what you're doing but you keep mixing your PR
this is from #9717
3 participants
Refactors hardcoded hidden keywords (MustBeBlocked, CantGainControl, etc.) to use type-safe StaticAbilityMode enum. Includes comprehensive tests in HiddenKeywordsMetaTest.java.