Fix EntersBattlefieldOneOrMoreTriggeredAbility for ZONE_CHANGE_GROUP events making tokens controlled by different players #12592
Conversation
|
So, uh, why can't you rewrite |
|
Thought the effort was being contained to that issue. Rewritten. |
| return false; | ||
| } | ||
| break; | ||
| return enteringPermanents.anyMatch(permanent -> controller.hasOpponent(this.controllerId, game)); |
There was a problem hiding this comment.
this line looks wrong, it should be checking permanent.getControllerId(). Looks like no ability currently uses TargetController.OPPONENT though.
also, should add a default case to the switch that throws an error (you can imagine a dev trying to use TargetController.ANY and it will silently never trigger)
| return false; | ||
| } | ||
|
|
||
| ZoneChangeBatchEvent zEvent = (ZoneChangeBatchEvent) event; | ||
| Stream<Permanent> enteringPermanents = zEvent.getEvents().stream() |
There was a problem hiding this comment.
Do not use stream fields/params due code style. Use filter with all conditions inside.
| return false; | ||
| } | ||
| break; | ||
| return enteringPermanents.anyMatch(permanent -> permanent.getControllerId().equals(this.controllerId)); |
There was a problem hiding this comment.
Use getControllerId() and getTargetController() instead direct access due good code style.
There was a problem hiding this comment.
targetController is fine to access directly, it's internal to this class. Still need to update to getControllerId() though
| .filter(z -> z.getToZone() == Zone.BATTLEFIELD) | ||
| .filter(z -> filterPermanent.match(z.getTarget(), this.controllerId, this, game)) | ||
| .anyMatch(z -> { | ||
| UUID enteringPermanantControllerID = z.getTarget().getControllerId(); |
Fixes #12588
Will need the be reviewed when #11895 is implemented