Refactor batch events #11995
Refactor batch events #11995
Conversation
remove old code no longer used
|
This all makes sense to me, big fan of how much code was removed with this. looks good |
| @@ -81,7 +81,7 @@ public boolean checkTrigger(GameEvent event, Game game) { | |||
| if (!super.checkTrigger(event, game)) { | |||
| return false; | |||
| } | |||
| Player opponent = game.getPlayer(event.getPlayerId()); | |||
| Player opponent = game.getPlayer(event.getTargetId()); | |||
There was a problem hiding this comment.
It's hard to find such replacements from playerId to targetId due multi-classes inheritance. If you manually checked and replaced all related usages then all fine. If you fix it by failed tests then it's bad.
Fast usages search showing a missing fix like:
DAMAGED_BATCH_FOR_ONE_PLAYER -> OneOrMoreDealDamageTriggeredAbility -> DealCombatDamageControlledTriggeredAbility -> FelineSovereignTriggeredAbility:
There was a problem hiding this comment.
good find, I thought I got them all manually but apparently missed that one. multi inheritance certainly complicates that, fortunately since this batch event was recently introduced and not used that many places, it wasn't too bad
# Conflicts: # Mage/src/main/java/mage/game/events/LifeLostBatchEvent.java # Mage/src/main/java/mage/game/events/LifeLostEvent.java
Closes #11985.
The only changed functionality is the new
DAMAGED_BATCH_FOR_ALLevent, used in five places, to address existing bugs and TODOs that were directly in scope of the refactor. Closes #10773 .There's good test coverage on these batch events which is nice.