Player Hidden Keywords related to SearchLibrary#10219
Player Hidden Keywords related to SearchLibrary#10219kojotak wants to merge 17 commits intoCard-Forge:masterfrom
Conversation
496b3a5 to
98f3002
Compare
| } | ||
| return targetPlayer == null || !targetPlayer.equals(sa.getActivatingPlayer()) | ||
| || !hasKeyword("Spells and abilities you control can't cause you to search your library."); | ||
| || !StaticAbilityCantSearchLibrary.cantCauseToSearchLibrary(this); |
There was a problem hiding this comment.
this should just use the same method above so it can be more dynamic
There was a problem hiding this comment.
Do you mean:
- remove the
cantCauseToSearchLibrary()method completely - add check for
CantCauseToSearchLibrarymode intocantSearchLibrary()?
There was a problem hiding this comment.
it's just a slightly more specific variant, not a different mode
we usually have one entry point, so scripts can just use params for their needs
There was a problem hiding this comment.
Removed the unnecessary mode in f7b33c7 but this may break something else, because the if is no longer specific to Ashiok only.
I was thinking about an alternative: add ValidActivatingPlayer$ Opponent for Ashiok card, move the targetPlayer vs activatingPlayer check into new cantCauseToSearchLibrary() method.
| Types:Legendary Planeswalker Ashiok | ||
| Loyalty:5 | ||
| S:Mode$ Continuous | Affected$ Opponent | AddKeyword$ Spells and abilities you control can't cause you to search your library. | Description$ Spells and abilities your opponents control can't cause their controller to search their library. | ||
| S:Mode$ CantSearchLibrary | ValidPlayer$ Opponent | Description$ Spells and abilities your opponents control can't cause their controller to search their library. |
There was a problem hiding this comment.
We need to add ValidCause to check for Spells and Abilities and check if they are controlled by the same player as the one that gets their library searched
…k by ValidCause$ SpellAbility.OppCtrl
| Types:Legendary Planeswalker Ashiok | ||
| Loyalty:5 | ||
| S:Mode$ CantSearchLibrary | ValidPlayer$ Opponent | Description$ Spells and abilities your opponents control can't cause their controller to search their library. | ||
| S:Mode$ CantSearchLibrary | ValidPlayer$ Opponent | ValidCause$ SpellAbility.OppCtrl | Description$ Spells and abilities your opponents control can't cause their controller to search their library. |
There was a problem hiding this comment.
not enough, needs to be the same Player,
checkout Params like "BlockerRelative" for similar stuff
There was a problem hiding this comment.
Oh, multi player situation, right🤔?
P1 owns Ashiok, P2 causes to P3 to search his library. This should work, since P2 != P3. P2 can not search its own library. But P2 can force P1 to search his library 🙈.
There was a problem hiding this comment.
I've tested this a little bit and concluded, that there should be just 2 conditions necessary. ValidPlayer and ValidCause, but not this "simple" one (with 1 parameter - the cause), but "relative-like" (with 2 parameters - cause and context - given card causing the search effect, instead of the default context card - source of static abbility) - I've called it ValidPlayerCauseRelative - similar to BlockerRelative.
…er) check by ValidCause$ SpellAbility.OppCtrl" This reverts commit 1778105.
…TheirLibrary using param similar to "BlockerRelative"
| } | ||
| return targetPlayer == null || !targetPlayer.equals(sa.getActivatingPlayer()) | ||
| || !hasKeyword("Spells and abilities you control can't cause you to search your library."); | ||
| return !StaticAbilitySearchLibrary.cantSearchLibrary(this, sa); |
There was a problem hiding this comment.
I'm a bit skeptical about the usefulness of these tests in general
(you're not even using targetPlayer any longer)
There was a problem hiding this comment.
I agree. These tests help me to understand and experiment with Forge engine and give me some (false?) confidence. I can easily remove them if the rest is OK.
There was a problem hiding this comment.
Let's try to break it down:
Ashiok needs the following information
a) SA controller
b) Searcher
c) Searched
then the condition has to check a = b = c = Opponent (all the same player)
I have a hard time seeing that this refactor has this equivalently covered yet
An attempt to refactor 3 hidden player keywords related to "search library" from #4760 :
Tested using
Stoneforge Mystic,Mindlock Orb,Ashiok, Dream Render,Aven Mindcensor. Example of applying library search limit of 4 cards: