Page 1 of 6

Restricted use mana

PostPosted: 10 May 2012, 13:02
by FabioFLX
I just have added a "UseOnlyFor" parameter in the Mana ability so Forge can trace mana coming from spells like Grand Architect.

To make this usable I'm editing ManaPool class so it has a new floatingRestrictedTotals array which works in a way similar to snow mana.

It seems this way can produce some result at least.
By the way, do you have notes to share with me about this topic?

Thanks,
FabioFLX

Re: Restricted use mana

PostPosted: 10 May 2012, 14:21
by ArsenalNut
I've have been thinking about how to do Mana restrictions for a while. I was going to start working on it when I could devote a good chunk of time to it. I had made a high level plan of attack but I haven't had time to work the details. Here's what I was planning.

I was thinking to add a "Restrictions$" parameter to the Mana AF that would be used to filter the use of the mana similar to the various "Valid" parameters. I was looking to add a "restrictions" property to the Mana class. The AbilityMana would set the restrictions property when the Mana objects are created. The Manapool functions would need to be modified to compare any restriction on a Mana object versus what the mana cost being paid. forge.game.player.ComputerUtil.payManaCost would need to be updated so that the AI takes into account the mana restrictions when making the list of its available mana sources. Also, forge.control.input.InputPayManaCostUtil.activateManaAbility would need to be updated to check the restrictions of the mana ability so that the human player cannot activate a mana ability that doesn't meet the restrictions.

Re: Restricted use mana

PostPosted: 10 May 2012, 14:34
by friarsol
I'd say there's a few places changes need to occur

AbilityFactoryMana
Anywhere human pays Mana Costs (CostPayment, a few others)
Anywhere computer pays Mana Costs (I can't remember exactly where this is)
Mana
ManaPool (probably want to use restrictive mana first if you can)

I always pictured the parameters being RestrictValid and RestrictSpellAbility, where RestrictValid would use the getValidCards structure, to filter out invalid cards and RestrictSpellAbility would work on the type of SA, just like it does for Spells.

For Example, Grand Architect would be something like:

AB$ Mana | Cost$ tapX<1/Creature.blue> | Amount$ 2 | RestrictValid$ Artifact | RestrictSpellAbility$ Spell,Activated

Mishra's Workshop could just be:

AB$ Mana | Cost$ T | Amount$ 3 | RestrictValid$ Artifact | RestrictSpellAbility$ Spell


Sloth, do we have a getValidSpellAbility() somewhere? It might be handy for things like Spell.Flashback or Triggered.CumulativeUpkeep, for some of these potential cards.

Re: Restricted use mana

PostPosted: 10 May 2012, 14:58
by ArsenalNut
friarsol wrote:Anywhere human pays Mana Costs (CostPayment, a few others)
adding restrictions checks in forge.control.input.InputPayManaCostUtil.activateManaAbility would cover this

friarsol wrote:Anywhere computer pays Mana Costs (I can't remember exactly where this is)
forge.game.player.ComputerUtil.payManaCost is where this is controlled.

friarsol wrote:I always pictured the parameters being RestrictValid and RestrictSpellAbility, where RestrictValid would use the getValidCards structure, to filter out invalid cards and RestrictSpellAbility would work on the type of SA, just like it does for Spells.

For Example, Grand Architect would be something like:

AB$ Mana | Cost$ tapX<1/Creature.blue> | Amount$ 2 | RestrictValid$ Artifact | RestrictSpellAbility$ Spell,Activated

I had originally been thinking the restriction would look like

RestrictValid$ Spell.Artifact

but breaking it up into RestrictValid and RestrictSpellAbility may make it easier to to do the filtering.

Re: Restricted use mana

PostPosted: 10 May 2012, 15:18
by friarsol
Thanks for the clarification Arsenal. I don't have the codebase here, so wasn't able to see exactly where it was.

Re: Restricted use mana

PostPosted: 10 May 2012, 15:40
by Sloth
friarsol wrote:Sloth, do we have a getValidSpellAbility() somewhere? It might be handy for things like Spell.Flashback or Triggered.CumulativeUpkeep, for some of these potential cards.
No, something like this doesn't exist as far as i know.

Re: Restricted use mana

PostPosted: 10 May 2012, 15:54
by FabioFLX
After so many tries I think the easiest solution is to have a second ManaPool which can handle the restrictions.

Well, maybe it's not an elegant solution, but checking this pool before the standard pool could made the trick.
What do you think?

Also, adding new parameters to the Mana ability is quite easy.

Re: Restricted use mana

PostPosted: 10 May 2012, 16:04
by friarsol
FabioFLX wrote:After so many tries I think the easiest solution is to have a second ManaPool which can handle the restrictions.

Well, maybe it's not an elegant solution, but checking this pool before the standard pool could made the trick.
What do you think?

Also, adding new parameters to the Mana ability is quite easy.
I think that's a bad idea. We used to have a second mana pool for snow mana and I removed it a year or so ago, because it was clunky and hard to display. What's wrong with just having restrictions attached to Mana objects? And checking if they are non-null like Arsenal suggested?

Re: Restricted use mana

PostPosted: 10 May 2012, 16:41
by FabioFLX
friarsol wrote:
FabioFLX wrote:After so many tries I think the easiest solution is to have a second ManaPool which can handle the restrictions.

Well, maybe it's not an elegant solution, but checking this pool before the standard pool could made the trick.
What do you think?

Also, adding new parameters to the Mana ability is quite easy.
I think that's a bad idea. We used to have a second mana pool for snow mana and I removed it a year or so ago, because it was clunky and hard to display. What's wrong with just having restrictions attached to Mana objects? And checking if they are non-null like Arsenal suggested?
I was trying your kind of solution in the first approach to the problem.
I added "isRestricted" just like there is an "isSnow", but while it seemed to simplify the approch, I realized this would force me to change the whole ManaPool logic.

Knowing this, I think creating a brand new class will handle the problem at first, and then we could merge the old one into the new logic to have a single class for both purposes, all working with the new logic.

Re: Restricted use mana

PostPosted: 10 May 2012, 18:12
by ArsenalNut
FabioFLX wrote:
I was trying your kind of solution in the first approach to the problem.
I added "isRestricted" just like there is an "isSnow", but while it seemed to simplify the approch, I realized this would force me to change the whole ManaPool logic.

Knowing this, I think creating a brand new class will handle the problem at first, and then we could merge the old one into the new logic to have a single class for both purposes, all working with the new logic.
I don't think there is quick solution. ManaPool is going to take the a lot of work to handle mana restrictions. Fundamentally, the mana pool needs to know if it is allowed use a given mana object to pay for a cost or not. I am not seeing how adding another class is going to fix that problem without breaking the current mana pool paradigm.

Re: Restricted use mana

PostPosted: 11 May 2012, 08:15
by FabioFLX
ArsenalNut wrote:I don't think there is quick solution. ManaPool is going to take the a lot of work to handle mana restrictions. Fundamentally, the mana pool needs to know if it is allowed use a given mana object to pay for a cost or not. I am not seeing how adding another class is going to fix that problem without breaking the current mana pool paradigm.
Well, maybe I have explayned my idea not enough.
I think it is NOT a quick solution for the problem too, but meanwhile I think it is quite easy to create a new manapool class to handle just that kind of restricted mana.

Of course I know this is a ugly patch but once I complete the new ManaPool I think it will be easy to implement the old one and replace it at all, moving whole Forge to just the new mana pool paradigm.

--

After some hour of coding I think actual ManaPool is a bit out-of-standard, having different arrays for the same data, and methods to made them equivalent. For example, there is a int[] and a ArrayList<Mana> for the same data, with some code using the first and some code the other.
To me, it could be really better to remove the int[] parts at all and move all the ManaPool to just the ArrayList of Mana.

While this will reduce a bit the speed of mana checking during a game, it will allow we to add the "restriction use" just to the Mana class, with the ManaPool able to handle the new info in a easier way.

Re: Restricted use mana

PostPosted: 11 May 2012, 12:32
by ArsenalNut
FabioFLX wrote:
ArsenalNut wrote:I don't think there is quick solution. ManaPool is going to take the a lot of work to handle mana restrictions. Fundamentally, the mana pool needs to know if it is allowed use a given mana object to pay for a cost or not. I am not seeing how adding another class is going to fix that problem without breaking the current mana pool paradigm.
Well, maybe I have explayned my idea not enough.
I think it is NOT a quick solution for the problem too, but meanwhile I think it is quite easy to create a new manapool class to handle just that kind of restricted mana.

Of course I know this is a ugly patch but once I complete the new ManaPool I think it will be easy to implement the old one and replace it at all, moving whole Forge to just the new mana pool paradigm.

--

After some hour of coding I think actual ManaPool is a bit out-of-standard, having different arrays for the same data, and methods to made them equivalent. For example, there is a int[] and a ArrayList<Mana> for the same data, with some code using the first and some code the other.
To me, it could be really better to remove the int[] parts at all and move all the ManaPool to just the ArrayList of Mana.

While this will reduce a bit the speed of mana checking during a game, it will allow we to add the "restriction use" just to the Mana class, with the ManaPool able to handle the new info in a easier way.
Ok now I understand what you're trying to do. I do not necessarily agree that the ManaPool needs to be completely rewritten to add restricted mana but there many ways to solve a problem.

Rewriting the ManaPool class is a pretty big undertaking that has a potential to effect the other developers. I think your development should be done as a feature branch to minimize impact on the Trunk until your changes are complete and tested. I already created a " mana_restrictions" branch that you can use.

Re: Restricted use mana

PostPosted: 11 May 2012, 15:29
by Max mtg
I agree that manapool should be completelly remade.
May I also advice the person who is going to work on that epic quest to use my classes of ManaCost, ManaAtom, ManaShard from forge.card package.

I see a core problem in the actual costs mechanics that receives string as 'cost' values - while such a string may mena not just mana but any other manipulations. There should be a tree-like structure which describes actions that should be taken to pay the cost.

Re: Restricted use mana

PostPosted: 13 May 2012, 13:02
by FabioFLX
Well, I must admit that after another couple of experiments I understood that - for the moment - the ManaPool does not need to be absolutely rewritten.
Instead, I achieved the functionality with some minor code inside the Mana class and a very little modification at the ManaPool class.
I haven't tested the code enough to say it's ready, but finally it allows cards like Grand Architect to work as expected.

Thanks for the branch. How can I share the code with other programmers without impacting on the actual code in the svn?

Thank you,
Fabio

Re: Restricted use mana

PostPosted: 13 May 2012, 13:42
by Sloth
FabioFLX wrote:Well, I must admit that after another couple of experiments I understood that - for the moment - the ManaPool does not need to be absolutely rewritten.
Instead, I achieved the functionality with some minor code inside the Mana class and a very little modification at the ManaPool class.
I haven't tested the code enough to say it's ready, but finally it allows cards like Grand Architect to work as expected.

Thanks for the branch. How can I share the code with other programmers without impacting on the actual code in the svn?

Thank you,
Fabio
Here is a little info: http://www.slightlymagic.net/wiki/Forge ... e#Branches