forge icon indicating copy to clipboard operation
forge copied to clipboard

CountersPutAI: shouldPumpCard is too restrictive for instant speed PutCounter

Open Agetian opened this issue 2 years ago • 5 comments

Currently, the AI only goes by shouldPumpCard when deciding whether to put a +1/+1 counter on its own creature or not. This is suboptimal because: (a) shouldPumpCard is truly designed for combat trick pumps that last until the end of turn, while PutCounter is a lasting effect that gives a more permanent bonus to the creature; (b) shouldPumpCard mostly evaluates combat tricks in the AI's own turn during the pre-combat phases or various blocking scenarios in the opponent's turn, thus, the AI would fail to activate the PutCounter ability at other times even when it's clearly beneficial.

Example case: the AI has a creature on board and I have a Raging Goblin in play. The AI casts Subtle Strike. It'll pretty much never choose to put a counter on its own creature because shouldPumpCard fails.

Expected behavior: the AI should pretty much always buff the creature with a counter unless it's useless or unless it can't receive counters (of this type or at all).

Now, currently I wrote it in such a way that shouldPumpCard is still checked, but if it fails, for instant speed stuff the AI goes by less restrictive isUselessCreature to determine playability. This makes Subtle Strike work correctly, but I don't know if it's too lax and if it should be generalized. Thus, the questions are:

(a) Should we completely replace shouldPumpCard with a different check (isUselessCreature + maybe something else)? (b) If we keep shouldPumpCard, should anything be changed about the instant speed test? Is it too lax, and is there a better option?

I understand that the intention to use shouldPumpCard was to try to teach the AI to use a card like this one as a form of a combat trick, but unfortunately, I don't think the AI really holds on to this particular card for a possible combat trick (due to the presence of the other option in AF Charm), so it sort of loses its meaning... Maybe in that case, the best approach would be to just flag this card (and other possible similar cards) with the AI logic (it's already flagged with AILogic$ Good, but I don't think that's hooked up into PutCounterAi) so that the AI knows not to wait specifically for shouldPumpCard to succeed?

Agetian avatar Apr 27 '22 04:04 Agetian

I need to think about it a bit more

But it seems we should either remove the AILogic$ Good from the example or teach it the bonus choice doesn't cost more because currently it only works when the minimum choice amount is > 1

I don't like this kind of fake AI hints without any support in the code, also might mislead other scripters when they use it as base for a new card

tool4ever avatar Apr 27 '22 21:04 tool4ever

But it seems we should either remove the AILogic$ Good from the example or teach it the bonus choice doesn't cost more because currently it only works when the minimum choice amount is > 1

I don't like this kind of fake AI hints without any support in the code, also might mislead other scripters when they use it as base for a new card

Yeah, indeed. In my opinion, AILogic$ Good can hooked up to this alternative check in PutCounterAi so that the AI knows that this option is OK to use as a generally good option (or maybe it can be renamed to AILogic$ Always for consistency). One thing for sure is that it shouldn't stay on the card without any code support.

Agetian avatar Apr 28 '22 04:04 Agetian

My ideas:

  • go with b) but split it into two sequential for-loop because a target found by shouldPumpCard will probably have a bigger game impact
  • in the first loop you could also add getKilledByTargeting so it can be used as a cheap kill spell on opps creatures
  • for the same reason, can you check if a call to getSafeTargets is needed on AI list somewhere above?
  • in the second loop make the timing so it's only used at the last possible moment for immediate benefit (at this point it's most likely not needed as combat trick in that turn, so not before DECLARE_BLOCKERS)

tool4ever avatar Apr 28 '22 08:04 tool4ever

Cool, thanks for the ideas! :) Sounds like a good plan, I'll see what I can work out soon ;)

Agetian avatar Apr 28 '22 15:04 Agetian

AI should treat Shield counter as possible protection against Damage/Destruction effects (unless it is already indestructible)

Hanmac avatar Apr 29 '22 11:04 Hanmac

@Agetian friendly ping if you still want to finish this? :)

tool4ever avatar Feb 25 '23 11:02 tool4ever

Eh, to be honest I'm not sure what the best approach would be here :/ I tried a couple times but each approach seemed to have a serious drawback which broke more than it fixed. I could probably use some help here, or maybe this can be converted into an issue if no one's up to the job at the moment :)

Agetian avatar Feb 25 '23 12:02 Agetian

Sure, I'll try to get those ideas I mentioned above working here

tool4ever avatar Feb 25 '23 12:02 tool4ever

Alrighty, sounds good! Thanks for help! :)

Agetian avatar Feb 25 '23 13:02 Agetian

This was indeed quite tricky and while I tried to be more generic some of my hopes seemed to be out of reach. But the charm case should behave properly now.

Towards a more general handling for such scenarios after some deliberation I've added another part to the condition: This will give it at least 1-2 turns with combat worth a chance to use the Counter for a surprise effect. That's a bit more like humans would usually deal with it (accepting it has to be played suboptimally rather than sitting around dead).

tool4ever avatar Feb 26 '23 17:02 tool4ever

I like this solution! :+1: Thanks for looking into it! Yeah, a comprehensive solution is indeed quite difficult to figure out ^^;

Agetian avatar Feb 26 '23 18:02 Agetian