Rework Aetherspouts to use non-targeting permanent selection#14616
Rework Aetherspouts to use non-targeting permanent selection#14616matoro wants to merge 1 commit intomagefree:masterfrom
Conversation
My understanding of this is: the reason why TargetCard does not support the battlefield as a zone is because cards don't exist on the battlefield, permanents do. So we must instead choose permanents rather than cards. This will change the interface to selecting target permanents on the battlefield for ordering, rather than the card selection popup. It also skips asking about tokens, but this could be changed. Fixes magefree#14351
xenohedron
left a comment
xenohedron
left a comment
There was a problem hiding this comment.
this looks quite overimplemented but it's just one card, you added test coverage, and I agree with the fundamental problem and solution, so good enough I guess
| @@ -0,0 +1,81 @@ | |||
| package org.mage.test.cards.single.mic; | |||
There was a problem hiding this comment.
nit, original printing m15
| permanentsToTop.add(permanent); | ||
| continue; | ||
| } | ||
| if (player.chooseUse(outcome, "Put " + permanent.getLogName() + " to the top? (else it goes to bottom)", source, game)) { |
There was a problem hiding this comment.
There are existing PutOnTopOrBottomLibraryTargetEffect with support of any amount and any object type. It also support puts by owner. It's require only one thing - sorted targets list in player order (I recommend to modify PutOnLibraryTargetEffect to support players order due paper rules anyway).
So final logic for current card:
- use standard PutOnLibraryTargetEffect;
- collect target as all attacking permanents;
- it's will process all objects inside effect, no need in custom code
There was a problem hiding this comment.
It's require little modification to start cycle by getPlayersInRange, collect cards list and call choose dialog to process it.
There was a problem hiding this comment.
There are existing
PutOnTopOrBottomLibraryTargetEffectwith support of any amount and any object type. It also support puts by owner. It's require only one thing - sorted targets list in player order (I recommend to modifyPutOnLibraryTargetEffectto support players order due paper rules anyway).So final logic for current card:
* use standard PutOnLibraryTargetEffect; * collect target as all attacking permanents; * it's will process all objects inside effect, no need in custom code
PutOnTopOrBottomLibraryTargetEffect doesn't seem to support multiple targets in the sense the Aetherspouts needs, because it only prompts the player once. Aetherspouts needs to prompt each player for each card where they want it to go.
We could use PutOnLibraryTargetEffect, but it barely saves us any code, as most of the logic here is looping to split the cards into one list for the top and one list for the bottom, and PutOnLibraryTargetEffect only supports sending all cards to top or all cards to bottom.
3 participants
My understanding of this is: the reason why TargetCard does not support the battlefield as a zone is because cards don't exist on the battlefield, permanents do. So we must instead choose permanents rather than cards. This will change the interface to selecting target permanents on the battlefield for ordering, rather than the card selection popup. It also skips asking about tokens, but this could be changed.
Fixes #14351