Collectible Card Game Headquarters

Sloth wrote:The most likely scenario is when a random trigger goes on the stack when the removal spell is cast (like Throne of Bone). I'm not sure this is enough to require some kind of check. I have to think a bit more about it.

My testing is limited to the Standard (RTR/M14/THS) environment so I did not expect such "when cast" triggers. In this case, it is all the more important to include such checks for the other removal APIs since only DamageDealAI has it originally.

The other reason why it might not be necessary is if the AI is not likely to have multiple removal spells that it can cast in a row to begin with. I suppose this is why only DamageDealAI had the check since burns are the cheapest removal and most likely to appear in multiples.

I did some checks with the "whenever cast blah" triggers and they do not seem to cause the AI to try another removal spell. The triggers are placed before the actual removal or responding spells so the top-stack will still be the relevant spell to be interrupted.

My next project would be fixing the pump spells and it appears to be much more complicated. I will be starting with the simple interrupts that pump toughness in response to burn/curse spells so the above issue about the AI wasting spells is going to come in.

PumpAiBase.isUsefulPumpKeyword() is a major cause of the AI wasting pump spells since it precedes most of the checks and the AI would grant keywords based on their value rather than their relevance to the boardstate.

The last part would be the use of P/T pumps to randomly add unblocked damage instead of saving them for interrupts or combat tricks and an issue with pumping a chum blocker that still dies and/or fail to kill the attacker anyway.

Good luck with that excessum. It would be awesome if you get that right!

First draft that includes the interrupts is up. Is there any reason why curse spells like Last Gasp are not included under ComputerUtil.predictThreatenedObjects()?

excessum wrote:First draft that includes the interrupts is up. Is there any reason why curse spells like Last Gasp are not included under ComputerUtil.predictThreatenedObjects()?

Is that the code that Regeneration uses to see if a card needs to regenerate? If so, since -X/-X cards generally force their way through Regeneration shields (except in the case of damage + lowering toughness > 0), there wasn't a point in doing it at the time of original coding. There's no reason it couldn't be added, but we'd just need to make sure that depending on what the AI is about to do to make sure they don't waste mana on useless activations.

friarsol wrote:Is that the code that Regeneration uses to see if a card needs to regenerate? If so, since -X/-X cards generally force their way through Regeneration shields (except in the case of damage + lowering toughness > 0), there wasn't a point in doing it at the time of original coding. There's no reason it couldn't be added, but we'd just need to make sure that depending on what the AI is about to do to make sure they don't waste mana on useless activations.

If I remember correctly it is also used somewhere else to check that the AI will not summon stuff that dies instantly in addition to the original hexproof/shroud PumpAI code. Anyway, ComputerUtil.predictThreatenedObjects() has blocks with separate checks for exile and Threaten effects too so it should not be too much trouble to code something up for curses.

Coding up something for -1/-1 counters (ApiType.PutCounter?) is going to be much harder since hexproof/shroud is the only surefire way to save the target without checking if it is affected by other until-end-of-turn effects.

Why are there so many different variations and methods for the checks on whether a Card can be destroyed? For creatures, Card.getShield(), ComputerUtil.canRegenerate() and Card.hasKeyword("Indestructible") should be sufficient but there are many methods all over the codebase with subsets of the above. For the other permanents, Card.hasKeyword("Indestructible") alone is enough.

I also have trouble understanding why Card.canBeDestroyed() is defined as "isInPlay() && (!hasKeyword("Indestructible") || (isCreature() && getNetDefense() <= 0))". Is it possible to replace or add the above checks for regeneration effects to canBeDestroyed and tidy up the code for all these checks?

excessum wrote:Why are there so many different variations and methods for the checks on whether a Card can be destroyed? For creatures, Card.getShield(), ComputerUtil.canRegenerate() and Card.hasKeyword("Indestructible") should be sufficient but there are many methods all over the codebase with subsets of the above. For the other permanents, Card.hasKeyword("Indestructible") alone is enough.

Your differentiation between Creatures and other Permanents is incorrect. Non-creatures may also regenerate, see Reknit.

Mainly this difference exists due to old code, and destruction having a few different paths that mean different things. In some instances it means the same exact thing as Magic defines it as. But in some cases it just means "should be sent to the graveyard" which is why toughness being 0 or less is also checked if the creature has Indestructible.

friarsol wrote:Your differentiation between Creatures and other Permanents is incorrect. Non-creatures may also regenerate, see Reknit.

Wow I did not even know such an effect exist... This simplifies matters then since there can be a universal single check.

friarsol wrote:Mainly this difference exists due to old code, and destruction having a few different paths that mean different things. In some instances it means the same exact thing as Magic defines it as. But in some cases it just means "should be sent to the graveyard" which is why toughness being 0 or less is also checked if the creature has Indestructible.

On second thought I think "(isCreature() && getNetDefense() <= 0)" is used for animated permanents who are currently not creatures now so it is still relevant because otherwise, anything with "getNetDefense() <= 0)" will be killed as a state-based effect regardless of regeneration or indestructible.

Tentatively, I propose to incorporate "Card.getShield()" and "ComputerUtil.canRegenerate()" with the "cannot be regenerated" checks into Card.canBeDestroyed(). Alternatively, I can design another method that does the above like say Card.canDie() or something.

excessum wrote:

friarsol wrote:Your differentiation between Creatures and other Permanents is incorrect. Non-creatures may also regenerate, see Reknit.

Wow I did not even know such an effect exist... This simplifies matters then since there can be a universal single check.

friarsol wrote:Mainly this difference exists due to old code, and destruction having a few different paths that mean different things. In some instances it means the same exact thing as Magic defines it as. But in some cases it just means "should be sent to the graveyard" which is why toughness being 0 or less is also checked if the creature has Indestructible.

On second thought I think "(isCreature() && getNetDefense() <= 0)" is used for animated permanents who are currently not creatures now so it is still relevant because otherwise, anything with "getNetDefense() <= 0)" will be killed as a state-based effect regardless of regeneration or indestructible.

Tentatively, I propose to incorporate "Card.getShield()" and "ComputerUtil.canRegenerate()" with the "cannot be regenerated" checks into Card.canBeDestroyed(). Alternatively, I can design another method that does the above like say Card.canDie() or something.

If you're exploring that part of code, maybe you could try to implement Pyramids

I am finally done with the proposed PumpAI patch. The check for "useful" keywords has been scrapped completely and replaced by more tangible benefits with appropriate cost functions, similar to the useRemoval logic. As usual, I will leave it here for a few days before committing in case I missed something or if someone has a better idea for the proposed changes.

I need some help with CharmAi. Apparently there are two independent calls to CharmAi.chooseOptionsAi(), once with the canPlayAI() and another with PlayerControllerAi.chooseModeForAbility(). If any probability-based logic is used in the choices (ie. PumpAiBase.shouldPumpCard() in Selesnya Charm and Boros Charm), there is a chance to fail the second CharmAi.chooseOptionsAi() call. This causes the spell to resolve with an empty choice which is obviously a bug.

Is there any way to save the choice from CharmAi.canPlayAI() since there is no other way to avoid such behavior?

Hi, excessum! We've got NPE everywhere.

Code: Select all

forge.ai.ability.PumpAiBase.pumpedCreature(Player, SpellAbility, Card, int, int, List<String>)
line 708:
Card pumped = c.isToken() || c.isFaceDown() ? CardFactory.copyStats(c, ai) : CardFactory.getCard(c.getPaperCard(), ai);


I've seen many instances of CardFactory.getCard(IPaperCard, Player) in your codes. I'm pretty sure CardFactory.getCard(c.getPaperCard(), ai) cannot work when the card is in alternate states (double-faced cards, for example). We have a method forge.game.card.CardFactory.copyCard(Card, boolean) to deal with this situation. Please be more careful.

swordshine wrote:Hi, excessum! We've got NPE everywhere.

Code: Select all

forge.ai.ability.PumpAiBase.pumpedCreature(Player, SpellAbility, Card, int, int, List<String>)
line 708:
Card pumped = c.isToken() || c.isFaceDown() ? CardFactory.copyStats(c, ai) : CardFactory.getCard(c.getPaperCard(), ai);


I've seen many instances of CardFactory.getCard(IPaperCard, Player) in your codes. I'm pretty sure CardFactory.getCard(c.getPaperCard(), ai) cannot work when the card is in alternate states (double-faced cards, for example). We have a method forge.game.card.CardFactory.copyCard(Card, boolean) to deal with this situation. Please be more careful.

I was following the code from CopyPermanentEffect.resolve() since it was the closest effect to what I required. What exactly is the difference in this case?

Considering that CardFactory.copyCard() is only referenced in CopySpellAbilityEffect.resolve(), I will probably never ever stumble on that example unless I was explicitly pointed there...

excessum wrote:I was following the code from CopyPermanentEffect.resolve() since it was the closest effect to what I required. What exactly is the difference in this case?

Considering that CardFactory.copyCard() is only referenced in CopySpellAbilityEffect.resolve(), I will probably never ever stumble on that example unless I was explicitly pointed there...

CopyPermanentEffect changes the CardCharacteristicName if that card is in the alternative state and changes back after copied.
I'm not quite familiar with AI codes, I hope Sloth could help to solve these bugs.