Store LKI about what a permanent was attacking/blocking; fix Gomazoa#14899
Store LKI about what a permanent was attacking/blocking; fix Gomazoa#14899xenohedron wants to merge 2 commits into
Conversation
|
[[Gomazoa]] |
|
Gomazoa - (Gatherer) (Scryfall) (EDHREC)
|
| creature.setAttacking(false); | ||
| creature.setBlocking(0); | ||
| creature.setAttacking(null); | ||
| creature.clearBlocking(); |
There was a problem hiding this comment.
it's better to use same naming for attack-block like:
- setAttacking
- setBlocking
- clearAttacking
- clearBlocking
- and other
| creature.setBlocking(0); | ||
| creature.setAttacking(null); | ||
| creature.clearBlocking(); | ||
| creature.clearBandedCards(); |
There was a problem hiding this comment.
Miss getBlockers().stream() code in other places with block clear?
If miss then it must be replaced by other clear code logic due mor and mor's compare implementation (it's will keep blinked/remove permanentns forever in the list and cause error logs in some use cases, e.g. copies).
|
|
||
| @Override | ||
| public void removeBlocking(UUID attackerId, Game game) { | ||
| this.blockingSet.remove(new MageObjectReference(attackerId, game)); |
There was a problem hiding this comment.
See above comments about clear code -- make sure it's called for existing attacker. Maybe some runtime check can help like:
if (this.blockingSet.stream().anyMatch(ref -> Objects.equals(ref.getSourceId(), attackerId))) {
throw new IllegalArgumentException("Wrong code usage: trying to clear non existing blocking object");
}2 participants
Was investigating one of the items in #13774 and turns out [[Gomazoa]] has a unique sort of effect. I added tests for a variety of edge cases. Although I tried various approaches with new/existing watchers, no easy combination of watched events was going to work in all my test cases. My conclusion is that the info needed to be stored in the LKI of the permanent, and after implementing that functionality it works nicely.
Previously, a permanent only stored a boolean for whether it was attacking. Now, it stores a MageObjectReference referring to what it's attacking (not just UUID, because e.g. planeswalker could be blinked). Null for not attacking, so testing for null reproduces existing behavior. Added a line to update the reference in the edge case of reselecting defender. This reference isn't actually used yet by any card I know of but since I was adding the functionality for blockers I thought it was good to make it consistent.
A permanent also stores an integer for the number of creatures it's blocking, with 0 for not blocking. This is used in some weird ways to check if further blocks are possible and didn't seem to be updated on every usage I would need, and I didn't want to change existing behavior. So I added a separate set of MageObjectReference to refer to the things it's blocking. This needed a special check for when creatures are explicitly removed from combat. It still holds references to permanents that have left the battlefield, which is what we need for LKI usage. It's not for the purpose of counting, that's handled by existing watchers.
The existing combat code is pretty messy and could likely be cleaned up with some investigation, but I kept these adjustments minimal here. All tests pass.