Collectible Card Game Headquarters

ArsenalNut wrote:I am confused. The ManaPool already keeps an ArrayList of Mana objects i.e. floatingMana. ManaPool is the only class that creates Mana objects and all of the methods in Manapool add and remove Mana objects without creating new ones. It's not clear to me how your objects are losing their restrictions.

It would really help if you could share out the code base you're using. Is there something preventing you from posting a patch or checking it into a branch?

Yew, ManaPool has an ArrayList of Mana but the program often uses a String representation of the cost.
What I'm saying is that this should be avoided in favor of a Mana ArrayList for every mana recipient.

In a specific case, when A.I. uses ComputerUtil class to test for its SpellAbility's to be castable checking its mana cost, it substracts the Mana from the player.manapool instance.
This is ok but, when the test is over, ComputerUtil rollbacks this mana recreating the pool, and so each Mana loses any previous restricion.

I'd like to check my code into a branch but I still haven't exactly understood how to

Max mtg wrote:I agree. Moreover, ManaCost (which tracks the payment) should hold the mana items spent to pay it - so that they could be released into manapool intact.

The exact Mana objects are stored with the ability that the player is trying to pay for. The refund methods move the objects from the list held in the spellAbility back to the floating mana array without making any copies. We can argue about which object the list of mana paid should be associated but the list does currently exist.

FabioFLX wrote:At the moment, I'm using a copy() method I added to the ManaPool class that, in my opinion, could simplify handling of rollbacks.

I believe this is the most likely source of your bug. None of the existing methods in the ManaPool ever made copies so the original Mana objects were left intact and just added and removed to various ArrayLists.

FabioFLX wrote:I'd like to check my code into a branch but I still haven't exactly understood how to

How about making a patch? Do you know how to do that?

ArsenalNut wrote:I believe this is the most likely source of your bug. None of the existing methods in the ManaPool ever made copies so the original Mana objects were left intact and just added and removed to various ArrayLists.

No ArsenalNut, this is not where the bug comes from, indeed I added the copy() method to avoid the loss of restrictions bug.

Anyway I'm rewriting my code because it is my first try in developing for Forge and so it is all but standard... A patch with my code as-it-is-now may result in making things worse than now for following modifications.

New code is really pretty and it avoids rubbish in touched classes.
By the way it's time to stop developing for today. I think a patch with working restrictions to be ready for tomorrow.

FabioFLX wrote:

ArsenalNut wrote:I believe this is the most likely source of your bug. None of the existing methods in the ManaPool ever made copies so the original Mana objects were left intact and just added and removed to various ArrayLists.

No ArsenalNut, this is not where the bug comes from, indeed I added the copy() method to avoid the loss of restrictions bug.

I found the culprit in ManaPool that is probably deleting your mana restrictions. There is function that creates an array list of new mana object from a parent object based on the amount in the parent object.

After looking at what ManaPool was doing, I decided it would be more efficient if the Mana object switched paradigms from being able to represent any amount of a given color to representing a single piece of mana in a given color so something like Black Lotus would now produce three mana objects rather than one with an amount of three. Changing to this paradigm, allows the logic in ManaPool to be much simpler and straight forward because you don't have to keep track of how much of the mana was used. I also means no additional Mana objects are created and I could reduce the reliance of string based operations. I have most ManaPool rewritten to use this paradigm. I have one function left to address then I'll check my code into the branch

I checked my updated Mana and ManaPool code into the branch. I have mana restrictions implemented but there is an issue with multikicker. I makes my head hurt to look at the multikicker code but from what I can tell there is no way to tell what source card the multikicker is associated with when the multikicker is being paid. This makes it impossible to tell whether or not a restricted mana can be used to pay a multikicker cost. Any thoughts on how to fix this?

ArsenalNut wrote:I checked my updated Mana and ManaPool code into the branch. I have mana restrictions implemented but there is an issue with multikicker. I makes my head hurt to look at the multikicker code but from what I can tell there is no way to tell what source card the multikicker is associated with when the multikicker is being paid. This makes it impossible to tell whether or not a restricted mana can be used to pay a multikicker cost. Any thoughts on how to fix this?

Multikicker needs to be completely rewritten, so don't bother with it. Just make it not payable with this mana (which should be correct for 90% of these cards anyway).

Just to throw a small spanner in the works, do the restrictions interact correctly with Doubling Cube (and possibly other mana copying, not too sure which cards to check though). Here's the 12/1/2004 ruling:

Any restrictions on the mana in your mana pool aren't copied. For example, if you have with no restrictions on it in your mana pool and that can be used only to cast artifact spells, you'll end up with , that can be used only to cast artifact spells, and that can be used for anything.

moomarc wrote:Just to throw a small spanner in the works, do the restrictions interact correctly with Doubling Cube (and possibly other mana copying, not too sure which cards to check though). Here's the 12/1/2004 ruling:

Any restrictions on the mana in your mana pool aren't copied. For example, if you have with no restrictions on it in your mana pool and that can be used only to cast artifact spells, you'll end up with , that can be used only to cast artifact spells, and that can be used for anything.

The way that Doubling Cube is scripted, it uses its own mana abilities to "copy" the amounts in the mana pool so the new mana has no restrictions.

A couple of question about doubling cube though
1) Shouldn't it double the amount of colorless mana as well? It currently does not.
2) Strictly speaking, is the use of mana ability correct i.e. Doubling Cube is not really a mana ability? It seems like there is a potential issue with TapsForMana triggers that happens to be avoided because all the currently implemented TapsForMana triggers involve lands being tapped.

I checked in a version that allows mana to make spells paid with it uncounterable. The version includes a working Cavern of Souls.

I am not completely happy with my solution though. I couldn't see a clean way to add the keyword "CARDNAME can't be countered" to a card and then remove the keyword when the card left the stack. I ended up adding a property to the Card class that ManaPool could set as appropriate. It would be cleaner if I could check the mana used to pay for a spell when its on the stack but all the array lists with this information get reset when a spellAbility is put on the stack. Is there reason why these array lists need to be cleared when the spell is put on the stack? Does it break anything if this step is done when the spell is removed from the stack?

ArsenalNut wrote:It would be cleaner if I could check the mana used to pay for a spell when its on the stack but all the array lists with this information get reset when a spellAbility is put on the stack. Is there reason why these array lists need to be cleared when the spell is put on the stack? Does it break anything if this step is done when the spell is removed from the stack?

You can probably move this information over to StackInstance, so it's available when the SpellAbility is retransformed for checking if it can be countered?

friarsol wrote:

ArsenalNut wrote:It would be cleaner if I could check the mana used to pay for a spell when its on the stack but all the array lists with this information get reset when a spellAbility is put on the stack. Is there reason why these array lists need to be cleared when the spell is put on the stack? Does it break anything if this step is done when the spell is removed from the stack?

You can probably move this information over to StackInstance, so it's available when the SpellAbility is retransformed for checking if it can be countered?

Good thought but the payingMana array list gets cleared before StackInstance gets created. From what I see, the payment history gets cleared almost immediately after a cost is completely paid. I am trying to figure what the design reason is for this behavior. Any ideas?

ArsenalNut wrote:Good thought but the payingMana array list gets cleared before StackInstance gets created. From what I see, the payment history gets cleared almost immediately after a cost is completely paid. I am trying to figure what the design reason is for this behavior. Any ideas?

Not at my codebase, but I'd guess it's leftover from before StackInstances even existed.

What happens if you switch these lines in SpellAbilityRequirements all the way at the end of the file

AllZone.getHumanPlayer().getManaPool().clearPay(this.ability, false);
AllZone.getStack().addAndUnfreeze(this.ability);

And inside the StackInstance Creation, grab the Payment list?

friarsol wrote:

ArsenalNut wrote:Good thought but the payingMana array list gets cleared before StackInstance gets created. From what I see, the payment history gets cleared almost immediately after a cost is completely paid. I am trying to figure what the design reason is for this behavior. Any ideas?

Not at my codebase, but I'd guess it's leftover from before StackInstances even existed.

What happens if you switch these lines in SpellAbilityRequirements all the way at the end of the file

AllZone.getHumanPlayer().getManaPool().clearPay(this.ability, false);
AllZone.getStack().addAndUnfreeze(this.ability);

And inside the StackInstance Creation, grab the Payment list?

No luck with switching the lines. StackInstance still gets created after clearPay is called when I run through with the debugger if the stack is not frozen.

For now my implementation works, it's just not as clean as it could be. I am going to work on play testing and creating all the restricted mana cards. I'll take another crack at it after that.

ArsenalNut wrote:

friarsol wrote:

ArsenalNut wrote:Good thought but the payingMana array list gets cleared before StackInstance gets created. From what I see, the payment history gets cleared almost immediately after a cost is completely paid. I am trying to figure what the design reason is for this behavior. Any ideas?

Not at my codebase, but I'd guess it's leftover from before StackInstances even existed.

What happens if you switch these lines in SpellAbilityRequirements all the way at the end of the file

AllZone.getHumanPlayer().getManaPool().clearPay(this.ability, false);
AllZone.getStack().addAndUnfreeze(this.ability);

And inside the StackInstance Creation, grab the Payment list?

No luck with switching the lines. StackInstance still gets created after clearPay is called when I run through with the debugger if the stack is not frozen.

For now my implementation works, it's just not as clean as it could be. I am going to work on play testing and creating all the restricted mana cards. I'll take another crack at it after that.

So, you too noticed that the arrays in the ManaPool was redundant, and that the most of the problems about the mana restrictions came from the use of two different storages for the same information.
A simple "for" cycle for cards that have an Amount$ of produced mana greater that 1 should make the trick.

I had restrictions working without problems some days ago, but Saturday was my birthday and I had no time to work on the code since last friday. Should I discard my work or should it be usefull to post it anyway in some branch?