Fixed conditional mana bug #13540
Open
Fixed conditional mana bug #13540
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I was running AI sims with a deck with Plaza of Heroes and on turn 1 with just this land GetPlayables() thought it could cast sleep cursed faerie and would get stuck in an infinte loop trying to cast it. This happens because the conditional mana gets added to regular mana in addMana(List<ActivatedManaAbilityImpl> abilities, Game game). so I changed the logic to use addMana(Mana addMana) which handles conditional mana correctly.
JayDi85
reviewed
Apr 14, 2025
| if (existingMana.equalManaValue(newMana)) { | ||
| continue SkipAddMana; | ||
| } | ||
| Mana moreValuable = Mana.getMoreValuableMana(newMana, existingMana); |
Member
There was a problem hiding this comment.
There were a more complex logic -- ignore duplicated mana option, ignore useless mana option. New code will add any mana option to the list.
New simplified code must be a better solution cause it run mana optimization anyway, but it must be researched. Too much useless mana options (that can't be removed after optimization) can generate bad performance and memory overflow. So need additional research.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was running AI sims with a deck with Plaza of Heroes and on turn 1 with just this land GetPlayables() thought it could cast sleep cursed faerie and would get stuck in an infinte loop trying to cast it. This happens because the conditional mana gets added to regular mana in addMana(List abilities, Game game). so I changed the logic to use addMana(Mana addMana) which handles conditional mana correctly.
I ran all the tests on my local version from Feb 2025 and this change created 2 new fails in testCharmedPedant() and testDampingSphere(). This is because both these tests use Eldrazi temple which previously would only create 1 mana option in getManaOptions() (since the 2 conditional mana would become normal mana and make the 1 mana option redundent) now it creates 2 mana options: 2 condtional colorless or 1 normal colorless, which is the intended behavior.
There are other bugs like this that convert conditional mana to normal mana and seem to be fixable in the same way, if this gets approved and this is seen as an important thing to fix I can go through and change them.