ManaPool: use Multiset for floatingMana#10824
Conversation
tool4ever
left a comment
tool4ever
left a comment
There was a problem hiding this comment.
Hmn, presenting me with another tricky refactor:
-
Personally I'm somewhat skeptical the code really becomes cleaner by introducing these additional concepts into it
-
But more importantly I'm also concerned this could degrade performance on a heavy workload class...
There seems to be way more filtering going on because you no longer store the color mapping - instead the HashMultiset will have lots of buckets based on the different Mana records
Sorry, but I'd prefer a second opinion here too before I can spend some time testing for regressions 🤔
That shouldn't be much of a problem, so in case of and right now, there shouldn't be too much filtering going on |
|
|
||
| if (manaList instanceof Multiset<Mana> manaSet) { | ||
| colors = manaSet.elementSet().stream().map(m -> MagicColor.Color.fromByte(m.getColor())).collect(Collectors.toSet()); | ||
| floatingMana.addAll(manaSet); |
There was a problem hiding this comment.
one reason why i don't have multiple lists per color, is that i can't use
Multisets.removeOccurrences on them.
|
I can try and check some more later this week but right now it looks like everyone (and their mother) wants me to review their stuff 😅 |
2 participants
Part of #10750
Store Mana in the ManaPool as
{Mana => 3}