Ability_Cost discussion

[Rob Cashwalker]

Are you also responsible for adding the new Ability_Cost? I see it's been merged into the abPump code, and I question if the AI can even use it. If it can't then what will it take to make it work?

Ability_Cost wasn't changed very much from what was in place. I pulled out some very similar code from abPump and abDamage, so I could add in the Sacrifice subkeyword in one centralized location.

Eventually, Ability_Cost would be used for any cost on a card (Life Payment, removal of counters, etc). Instead of using ability.getBeforePayMana or ability.getAfterPayMana, and be limited to two input types, we could fill out an Ability_Cost object and go through in a consistent order to paying costs. Skipping any that don't apply.
Example: Choose targets, tap this permanent, pay mana, sacrifice this permanent, sacrifice other permanents, remove counters, pay life.
Once all the costs have been successfully paid, add the ability to the stack.

I'm not sure what you are questioning? The AI can use these abilities in the same way they could before. I just tested Nantuko Shade/Mox Jet deck for the AI and he overran me in 3 turns.

For the Sacrifice subkeyword, I turned the AI Playing to false so we could make sure everything was working with the keyword first. I haven't heard any complaints of the new Class, so we can look into implementing decision making for sacrificing.

Do you want to start up a discussion on a different thread on when the AI would use sacrifice effects?

Taking your example, the human is presented with the cost payment using the setAfterPayMana command. SpellAbility has a get/setBeforePayManaAI, but not a get/setAfterPayManaAI. Currently, the BeforePayManaAI is only used to handle surprise, surprise, bouncing Islands as an alternative cost.

Ideally, we would have a single point of cost handling, both mana and alternative costs.

[DennisBergkamp]

Yes, currently all of the "AI cost handling" is done either in the resolve(), or the chooseTargetAI() methods.
A lot of cost handling for the human player is also still done in resolve(), very bad.

[friarsol]

Rob,

I see why you think it wouldn't work now. The sacrifice part probably wouldn't happen due to the restrictions you referred to, even if the CanPlayAI was enabled. I hadn't gotten that far in the code to realize that stuff was missing. It probably could have been "forced" in chooseTargetAI() but that would have felt weird.

It seems we are on the same page with centralizing the pay cost functions.

We would have a pay cost (human) function and a pay cost (AI) function with the Ability sent in. We could send in Ability_Cost and the Ability, and go through each of the costs in order as I mentioned in my example.

I did have plans to come back to sacrifice, but decided to straighten up the mana pool stuff first.

I may end up forking my code locally so I could work on this, since it looks to be a longer term project. I think if it would be a big win if we could get this working as basically any ability would have to come through this bottleneck, and we wouldn't have the problems where costs are paid on resolution. It may also create an opportunity for the AI to better evaluate abilities to activate.

[Rob Cashwalker]

Well, the individual card AI needs to care.. sort of. Like if the cost can't be paid... like discarding a card with an empty hand.

There is already a little bit of sac-specific AI code - choosing a target for the pump. You specifically remove the source card if it would need to be sacrificed. This may quickly become messy as more possible costs are added.

My keywords don't use chooseTargetAI, because canPlayAI makes sure that it has a target in mind before returning. But it's not the right time (rules-wise) to pay costs.

It couldn't hurt to set this up in an AfterPayManaAI (added in similar fashion to BeforePayManaAI) function for now, until some universal cost payment system can be worked out.

[friarsol]

Alright so I'm going on vacation for a few weeks at the end of this week, but I think I have some functional code in place that bypasses the whole before/after pay mana thing.

I've only implemented this for the abDamage and abPump keywords so far.

Basically when you are about to pay for your costs, if an Ability_Cost has been created for that Spell_Ability it will create a Cost_Payment object and cycle through all of the costs in a specific order for the player. It does this by requesting Input from the player of the specific cost type, and then when that is filled, it checks for the next type needed. Once all the costs are paid, the ability is placed on the stack.

The AI code is mostly working for this as it was mostly just ported over from what is around previously, with some added conditionals. However, when the abilities get used needs to be improved. Especially for pump abilities, it would be great if these would mostly occur in response to spells, or after blockers are declared. To either deal a little extra damage, or survive combat.

I've written the code so it can be enhanced by other costs over time, just by adding a some code in a centralized location.

Cards that use the Cost_Payment function will have costs paid in this order currently:
1. Remove spell from hand (currently unused, for spells)
2. Targeting. (Only creatures/players for now since only abDamage and abPump are using it)
3. Tap cost. In essence, Cost_Payment will make Ability_Tap unnecessary. It will take some time for the transition to occur.
4. Pay mana. Since the card is tapped prior to paying mana, we don't have to worry about lands trying to pay for their own ability.
5. Sacrifice cost. Sacrifice should probably always be last as a payment to ease undoability of abilities.

[Rob Cashwalker]

I am trying to do a Destroy Target ability. Using abPump as a bit of a guide.

I'm wondering why Ability_Cost should be responsible for checking "Tgt" in the keyword name? Sure it's not too bad an idea to generalize, I guess. But I question why Ability_Cost. The costs should only be parsed following the space.

[friarsol]

I am trying to do a Destroy Target ability. Using abPump as a bit of a guide.

I'm wondering why Ability_Cost should be responsible for checking "Tgt" in the keyword name? Sure it's not too bad an idea to generalize, I guess. But I question why Ability_Cost. The costs should only be parsed following the space.

I don't know why it would matter where the parsing starts. abPump is the ability name. Targeting is arguably a requirement for the ability. It's not exactly a "cost" per se, but it is something that needs to happen before the ability hits the stack.

We could pull it out separately if you want, but since Cost was overriding how player would pay for costs, it made sense to just roll the target changes into it as well, since it would just be one check if additional costs were necessary. If they were, Cost_Payment would handle all of it.

I'm definitely not married to Target needing to happen there. But I'd prefer it be separated from each ability so we can reuse the code as much as possible, and move over the prep code I had for minimum/maximum targets.

[Rob Cashwalker]

Previously, we had a setBeforePayMana(Input). This would usually be used for the target choosing input. Each keyword should only use the appropriate type of input.
DestroyTgt wouldn't use the C or CP notation, because it wouldn't just affect creatures, and definitely not players.... It needs CardFactoryUtil.input_targetValid.

With the possible revision to the keywords using a SpellAbilityFactory, and named arguments, I see no reason why there can't be something more dedicated to generalizing target choosing. But it definitely should be separate from paying costs.

[friarsol]

Of course the targeting doesn't exist yet for other permanent types, since there was no need for it with the other ability keywords.

But fair enough, I can pull it out of the costs portion and make a specific targeting class. It'll make it easier to handle up to and multiple targeting for cards like Arc Lightning or Plow Under.

I'll take a look tonight and see if I can pull it together so you can get to work on the destroy ability.

[Rob Cashwalker]

I've got the overall structure for it done, the Ability_cost pre-empting the setBeforePayMana is the only hold-up to testing.

I think SpellAbility itself should have methods like "doesTarget" and "canTargetPlayer", etc. It could then have a specific method for setting the target Input class.

So GameAction.playSpellAbility would first check for a targeting Input, execute if not null, then check for the Ability_Cost, execute if not null, then finally execute the ability itself. (better not be null)

[friarsol]

Alright I uploaded the changes to pull targeting out of Ability_Cost. Hopefully that is what you were thinking of when you were talking about the changes.

We can just fill out Input classes in Target_Selection similar to the ones already there. Although reusing isValidCard for abilities like this may end up being better for our long-term goals.

[Rob Cashwalker]

Thanks... I'll check out what you did and retrofit as necessary. Perfecting the cost interpretation scheme is a huge benefit that will provide for many cards to come.

BTW, any ideas on applying it to spells? (As an additional cost, Sac-Creature)

Yes... I was realizing as I wrote it, that while input_TargetCreaturePlayer (or whatever it is) is fine when it's literally any creature or player.... But ultimately, what does input_TargetValid (or whatever) do? It could just as easily enable "Player" as an option along with any isValidCard option.

[friarsol]

Thanks... I'll check out what you did and retrofit as necessary. Perfecting the cost interpretation scheme is a huge benefit that will provide for many cards to come.

BTW, any ideas on applying it to spells? (As an additional cost, Sac-Creature)

Yes... I was realizing as I wrote it, that while input_TargetCreaturePlayer (or whatever it is) is fine when it's literally any creature or player.... But ultimately, what does input_TargetValid (or whatever) do? It could just as easily enable "Player" as an option along with any isValidCard option.

I'll move trying to get this working with spells on top of my to-do list. It would be nice if Harrow was handled in the same way as abilities so I didn't have to remember to tap the land for mana before it gets sacrificed.

[Sloth]

I'll move trying to get this working with spells on top of my to-do list. It would be nice if Harrow was handled in the same way as abilities so I didn't have to remember to tap the land for mana before it gets sacrificed.

That would be a big step towards user-friendlyness! Forge needs more of it.

[Rob Cashwalker]

friarsol,

I've added "TgtV" and some support functionality for it. However I'm not sure how to adapt CardFactoryUtil.input_targetValid like targetPlayer, etc. Could you pickup from where I left off in r2056?