magefree / magefree/mage

Devs: Java -> streams -> forEach can broke the game (lost priority bug)?

Open
#6,207 6 comments 0 reactions 0 assignees View on GitHub
bug refactoring
Dominant language
Java
Stars
2.4k
Forks
940
Avg merge
2d 12h
Merged PRs (30d)
160

Description

Example code from `Whirlwind Denial`:
``` java
game.getStack()
.stream()
.filter(Objects::nonNull)
.forEachOrdered(stackObject -> {
Cost cost = new GenericManaCost(4);
if (cost.canPay(source, source.getSourceId(), source.getControllerId(), game)
&& player.chooseUse(outcome, "Pay {4} to prevent " + stackObject.getIdName() + " from being countered?", source, game)
&& cost.pay(source, game, source.getSourceId(), source.getControllerId(), false)) {
return;
}
stackObject.counter(source.getSourceId(), game);
});
return true;
}
```

It uses a direct game objects list like Stack. If something can change it inside then it will be broken by ConcurrentModificationException (example with stack processing - if something add or remove from stack). It can be modified in the same code or by ApplyEffect, ProcessActions, game events and other “hidden” logic.

Recommends:
* if you need to do a game state changes to the same objects collection then collect it to the own list and make changes from it;
* some standard methods like getActivePermanents already returns a new list — so it’s safe for game state modifications.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.