Card AI (Improvements) Requests

[excessum]

Yeah I was trying to find some kind of an optimal solution to this but was yet unable to devise one - either the AI "underpumps" (as in "refrains from pumping at all while attacking with e.g. pumpable 0/1s", or it "overpumps" (and sometimes gets to spam useless abilities). For now the latter is kind of partly limited by the fact that the AI tries to reserve mana for a future spell, so at least it hopefully won't spam all its mana when it needs some, but obviously some kind of a better solution will be necessary here (such that the AI can tell the difference between actually useful, e.g. power-buffing, pumps that will allow it to deal more damage, and the "non-stacking" ability pumps (first strike, unblockable etc.) that only make sense if the creature doesn't have this ability on it yet. Sadly, I haven't found a way to implement that yet. If anybody knows how to make this better, please assist.
- Agetian

The repeated pumping bug (at least for AEtherling) was introduced by your addition in the line I quoted and I was expecting you to fix or revert it. The original intent for that revision was to allow the AI to pump its attack when unblocked and introducing a bug while doing that is kind of self-defeating...

The entire point of my original revamp for PumpAi was to give strict scenarios for the AI's use of pumps. This is done with a limited implementation of game-state re-rolling to verify that the pumps actually do something so the AI does "know" what pumps are useful. I never did encounter instances of the AI spamming "non-stacking" ability pumps even before r27158 so unless someone provides a specific example there is nothing that can be done.

[Agetian]

The point is, it was already buggy, albeit in a different way, before the line was introduced. If the AI is supposed to know which pumps are useful without any additional checks, it does a rather poor job at actually deciding to pump. Namely, it attacks with creatures and then does not pump them with effects that it knows(?) are useful. Yes, the change introduced in my line does introduce a different issue, and I'll take another look at what can bedone to both fix it and not introduce the old one, but like I currently said, I don't have an insightful idea yet and sadly it doesn't look like you have a better idea (other than just reverting) either. I can revert it, and i will most likely have to if i can't find a better thing to do, but my point is that right now the pump ai is dumb both ways, and we are trying to choose which way is less obviously stupid.

EDIT: I committed r27368 which is aimed at fixing the problem with the stacking pumps while still preserving the functionality that allows the AI to decide to pump creatures with attack-increasing pumps. Can you please tell me how I can reliably reproduce the AEtherling bug to confirm that it's fixed or see it in action and work some more on fixing it?

- Agetian

[excessum]

The point is, it was already buggy, albeit in a different way, before the line was introduced. If the AI is supposed to know which pumps are useful without any additional checks, it does a rather poor job at actually deciding to pump. Namely, it attacks with creatures and then does not pump them with effects that it knows(?) are useful. Yes, the change introduced in my line does introduce a different issue, and I'll take another look at what can bedone to both fix it and not introduce the old one, but like I currently said, I don't have an insightful idea yet and sadly it doesn't look like you have a better idea (other than just reverting) either. I can revert it, and i will most likely have to if i can't find a better thing to do, but my point is that right now the pump ai is dumb both ways, and we are trying to choose which way is less obviously stupid.

- Agetian

You do realise that attacking and pumping are independent AI decisions right? Choices to attack are made by AiAttackController without consulting PumpAi at all. Obviously this is sub-optimal hence the "create and buff attackers" block in PumpAiBase which forcibly pumps creature before declare attackers or else the AI will happily ignore them. If the AI chooses to attack with "useless" creatures like Killer Bees, the fault is in AiAttackController, probably another gotcha which assumes the AI will always pump creatures. I need other concrete examples of the pump logic failing to even know what might not be working as expected.

[Agetian]

You do realise that attacking and pumping are independent AI decisions right? Choices to attack are made by AiAttackController without consulting PumpAi at all. Obviously this is sub-optimal hence the "create and buff attackers" block in PumpAiBase which forcibly pumps creature before declare attackers or else the AI will happily ignore them. If the AI chooses to attack with "useless" creatures like Killer Bees, the fault is in AiAttackController, probably another gotcha which assumes the AI will always pump creatures. I need other concrete examples of the pump logic failing to even know what might not be working as expected.

Yes I do, and that actually makes it all the more difficult to find a more optimal solution. The issue is - it assumes correctly that it can attack and do damage via pumping (it makes sense). It makes no sense not to attack (in context) and it makes no sense not to pump after attacking (in context), so the decision to attack is legitimate, but after that the pump logic, being the separate decision and all, does not play ball with that decision to attack.
The easiest way to reproduce the pump logic failing is with this simple test case (you'd have to comment out line 588 in PumpAiBase.java in order to get the old behavior back):

Code: Select all

AICardsInPlay=Killer Bees;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest
AICardsInHand=Forest


Start a brand new game. Human should have no cards in play. Get to the AI's turn and watch it continuously whack the player for a whopping 0 damage with Killer Bees when it can do 20+ for lethal.
You can replace the pumped creature and the lands with any appropriate combination (Frozen Shade and Swamps, etc.). Works the same for all creatures and in all similar circumstances.

P.S. Tried to commit a somewhat better (albeit still suboptimal) fix for that, at least temporarily until a better, more "coupled" solution can be devised, if any.

P.P.S. Can you please provide me with a test case for AEtherling? I can't get it to spam-pump useless abilities (even with the old faulty logic), I want to see the situation where it happens (or happened) so that I can ascertain that it no longer happens.

- Agetian

[excessum]

Yes I do, and that actually makes it all the more difficult to find a more optimal solution. The issue is - it assumes correctly that it can attack and do damage via pumping (it makes sense). It makes no sense not to attack (in context) and it makes no sense not to pump after attacking (in context), so the decision to attack is legitimate, but after that the pump logic, being the separate decision and all, does not play ball with that decision to attack.
The easiest way to reproduce the pump logic failing is with this simple test case (you'd have to comment out line 588 in PumpAiBase.java in order to get the old behavior back):

Code: Select all

AICardsInPlay=Killer Bees;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest;Forest
AICardsInHand=Forest


Start a brand new game. Human should have no cards in play. Get to the AI's turn and watch it continuously whack the player for a whopping 0 damage with Killer Bees when it can do 20+ for lethal.
You can replace the pumped creature and the lands with any appropriate combination (Frozen Shade and Swamps, etc.). Works the same for all creatures and in all similar circumstances.

P.S. Tried to commit a somewhat better (albeit still suboptimal) fix for that, at least temporarily until a better, more "coupled" solution can be devised, if any.

- Agetian

Without a global decision manager, the AI may attack and find itself with no mana left to pump due to pumping something else, casting instants, etc. As an example, based on your logic, the AI can attack with multiple Killer Bees but it will probably pump the first one with all of its mana and then lose some or all of them to blockers it should have been able to kill due to "expected" pumps. Besides, it is generally a bad idea to code something assuming that some other distinct module will follow suit.

This is still the Killer Bees example which is due to AiAttackController and not PumpAi. Besides, the AI will not know that it can activate 20 times in a row since each activation is an independent call to canPlayAi(). Both you and lujo claimed that there were other examples of the AI stacking keyword pumps, pumping non-attacker, etc. which would be a bug if it can be reproduced and I am still waiting for that.

Your commit fixes the AEtherling example (attached) so no worries there. There is probably a better check that "sa.hasParam" but I honestly cannot think of one.

[Agetian]

Yes, I understand that in order to make things like these really "smarter" we need a better AI model (Minimax, Monte-Carlo, etc.) that decides based on an evaluation of a combination of actions together, and until we get that coded (I'm really hoping Krazy's idea with traversable game states will work out), *something* is got to be suboptimal - the very idea of independently deciding each single minute step of the game without coordinating it with everything else seems to be asking for trouble in a game like MTG, to be honest. You're right, there are gotchas with this modification as well (e.g. the AI attacking with two pumpables and only pumping one of them), but then again, it's worse without this modification (because it will attack with two pumpables and not pump either of them), so as a temporary "middle ground" solution I believe it should work. The other alternative would be to just make the AI not attack with pumpables at all and use them on the defense, which sort of defeats their purpose to a large extent as potentially strong aggressive creatures, personally I don't like this solution either.

Hopefully if AEtherling condition is fixed, then other similar conditions (where the AI would repeatedly pump non-stacking things) would be fixed as well, but if not, I'd happily look into test cases as well (and if I come across anything, I'll provide a test case too). I haven't seen the AI pumping non-attackers lately (but I recall seeing that happen a while ago), if it ever happens in the latest revisions I'll try to build a test case based on the battlefield position.

- Agetian

[excessum]

Yes, I understand that in order to make things like these really "smarter" we need a better AI model (Minimax, Monte-Carlo, etc.) that decides based on an evaluation of a combination of actions together, and until we get that coded (I'm really hoping Krazy's idea with traversable game states will work out), *something* is got to be suboptimal - the very idea of independently deciding each single minute step of the game without coordinating it with everything else seems to be asking for trouble in a game like MTG, to be honest. You're right, there are gotchas with this modification as well (e.g. the AI attacking with two pumpables and only pumping one of them), but then again, it's worse without this modification (because it will attack with two pumpables and not pump either of them), so as a temporary "middle ground" solution I believe it should work. The other alternative would be to just make the AI not attack with pumpables at all and use them on the defense, which sort of defeats their purpose to a large extent as potentially strong aggressive creatures, personally I don't like this solution either.

Hopefully if AEtherling condition is fixed, then other similar conditions (where the AI would repeatedly pump non-stacking things) would be fixed as well, but if not, I'd happily look into test cases as well (and if I come across anything, I'll provide a test case too). I haven't seen the AI pumping non-attackers lately (but I recall seeing that happen a while ago), if it ever happens in the latest revisions I'll try to build a test case based on the battlefield position.

- Agetian

You are still missing the point... The AI attacking with 0 power creatures is due to faulty logic in AiAttackController and not PumpAi. The current logic (ie. assume every creature with att/def pumps WILL be pumped) is dangerous as it falsely assumes that the AI can AND will pump every creature. It should not be attacking at all since it should base its evaluation on the current state and not make assumptions on future actions. The alternative should be to activate the pump and then declare attackers as 1/2s, however sub-optimal it might be. The idea of attacking with 0/1s and then choosing to pump based on the opponent's response is fine for humans but the AI will never get this without serious hack work.

I believe the bugs reported about pumping useless creatures should be fixed with my latest commit. It should be due to external effects like Pacifism failing to be copied when considering the pump effect and/or not checking if the pump target is tapped and cannot attack/block.

[Agetian]

Right, I get this, but what I'm saying is that it's a classic catch-22 situation for the current AI model, there is no optimal solution among everything you said or I said thus far. Like you said, according to the principles of this AI model, it shouldn't theoretically even attack in this scenario, which is going to render the AI unable to use these pump creatures effectively (people will say: stupid). Since it does base it on this assumption, though it may as well be dangerous (and it wasn't me who coded it that way in case you're wondering), it basically has two options later: either "go ahead and do it" (pump anyway - which is the current way it does it; there *will* be cases where people will say: stupid - but at least in many cases the action of attacking+pumping will appear justified and will actually damage the opponent) or "back away and not do it" (which results in the behavior that we've seen before - it attacks with 0/1, according to the faulty hacky AI attacker logic, and then doesn't do anything because the pump AI can't predict the outcome of combined pump play (people will say: stupid).

- Agetian

[Sloth]

You are still missing the point... The AI attacking with 0 power creatures is due to faulty logic in AiAttackController and not PumpAi. The current logic (ie. assume every creature with att/def pumps WILL be pumped) is dangerous as it falsely assumes that the AI can AND will pump every creature. It should not be attacking at all since it should base its evaluation on the current state and not make assumptions on future actions.

The AI will assume that the pump ability will be activated once, when it can pay the cost of one activation. I think that is pretty save and only fails in corner cases when multiple pump creatures are out.

[excessum]

Right, I get this, but what I'm saying is that it's a classic catch-22 situation for the current AI model, there is no optimal solution among everything you said or I said thus far. Like you said, according to the principles of this AI model, it shouldn't theoretically even attack in this scenario, which is going to render the AI unable to use these pump creatures effectively (people will say: stupid). Since it does base it on this assumption, though it may as well be dangerous (and it wasn't me who coded it that way in case you're wondering), it basically has two options later: either "go ahead and do it" (pump anyway - which is the current way it does it; there *will* be cases where people will say: stupid - but at least in many cases the action of attacking+pumping will appear justified and will actually damage the opponent) or "back away and not do it" (which results in the behavior that we've seen before - it attacks with 0/1, according to the faulty hacky AI attacker logic, and then doesn't do anything because the pump AI can't predict the outcome of combined pump play (people will say: stupid).

- Agetian

Yeah I know that it has always been like this. I only took offence when you said that this should be the expected behaviour. There is no indication anywhere that the AI must or will activate that ability and even if a comment states this explicitly, it is still a bad practice. I spent a lot of time working with the AI and such gotchas are extremely annoying. How on earth am I supposed to know that ComputerUtilCombat.damageIfUnblocked() will automatically trigger activated abilities? Without well-defined parameters (ie. game-state file), it is a real pain to debug such bugs since you never know where the unexpected behaviour is coming from.

The AI will assume that the pump ability will be activated once, when it can pay the cost of one activation. I think that is pretty save and only fails in corner cases when multiple pump creatures are out.

My gripe is the assumption that it will be activated at all. There is no link between this requirement/assumption and the actual decision to activate ie. (if A then do B when A is to be decided in the future). If there is a way to force the AI to activate said ability once decided, like a delayed playChosenSpellAbility(), I will have no issues with this (silly, yes, but perfectly logical). The other failure would be the AI running out of mana due to instants or activating other abilities for whatever reasons.

[Agetian]

Well, the difference in the interpretation of the "expected behavior" is huge for us programmers and for the players who just play Forge and who have no idea about AI models, game states, decision-making details, and other stuff that we are theorizing about at the moment. From the programmer's perspective - yes, you're right, the AI makes no such obligatory contract that there is going to be an expected connection between the two actions, which is the result of all the corner cases and gotchas that we're discussing at such great length here. I can also relate to your statement that certain AI functions have unexpected side effects and tracing through them can be a pain. However, as of yet, it's the best we have (like I said, in the "ideal world" with "ideal Forge", we would have a more appropriate game state-based AI with a much stronger basis such as e.g. Monte-Carlo, and we would also have a "perfect" understanding of how, given this better AI model, we should make it possible for the AI to evaluate different states such that it would always know which battlefield position is better and which is worse and what to do best in each particular case; we don't have any of that yet, but, you know, it's just the "dream future scenario").

From the end user's perspective, when the player sees the AI play Frozen Shade and then attack with it next turn with Swamps open, he expects the AI to do something about it (pump, try to deal damage). When he sees that the AI does nothing and just passes with the creature still being 0/1 (possibly also losing the said creature to a blocker, but maybe not), the player says "a stupid AI is stupid" and he could care less about all our theories here, even though it may sound offensive to the programmers who are trying to improve the current simple AI model to the best of their abilities. Then this player will come here and will report it as an issue (and, from the player's perspective, rightly so). The same situation happens when the player sees the AI play Frozen Shade and have Swamps open and then just sit back and do nothing about it, no matter what's on the battlefield. It can be useless explaining to the said player that the AI can't (or, in "academic theory", even shouldn't given the current AI model) add the two and two together and predict that a pump+attack is valid. The user will still say "a stupid AI is stupid", you know, because it's a bad play no matter how you look at it (you essentially end up with a partly dead creature that might potentially have been powerful). Same goes for when it attacks, then casts some expensive instant spell and thus loses the ability to pump (which, from the point of an ordinary player, "was probably the AI's original intent", even though technically it wasn't). Because we don't have an advanced predictive AI in Forge yet, what we can do is we can minimize the amount of corner cases and "weird" situations in which the AI will obviously misplay in ways that are obvious blunders, but the issue is that it will still misplay at least in some cases because MTG is a complex game that is not normally played with individual decisions in mind (which is essentially what our AI does at large) - at least that's what I'm trying to go for in my commits that are geared towards improving AI, but I make no false claims or promises here and I welcome any improvements to my modifications which are, by no means, "optimal" (as in "the AI will never misplay with these changes in mind").

So yep, sorry if my comment was offensive or anything - it wasn't meant to be, I was just merely analyzing the situation from the position of an ordinary player because that perspective is, after all, the most important one once it gets to the feedback we're getting in this particular thread.

Anyhow, I guess it's enough pure theory for me - I think it's safe to assume we understand where we're at at the moment, and I'm sure that each of us will, given enough time and desire to do so, try to improve the situation to the best of our ability until, perhaps, the better days come and there is finally a true better "thinking" AI support in Forge.

- Agetian

[excessum]

Lol I think we are going off in tangents already... I am speaking purely from a coder's perspective when I talk about "expected" behaviour. What you have been describing is a higher level concept, say... strategic AI layer or something similar. ComputerUtilCombat.damageIfUnblocked() should tell me exactly what it says, the damage if said creature is unblocked given static/triggered abilities from the fixed board state. I do not wish to know if I can activate its +1/+1, or if I can convoke out something like Gather Courage, or if the opponent can sacrifice his creature to trigger Dictate of Erebos. All these ifs and maybes should be relegated on a higher level function that invokes damageIfUnblocked() so that any developer who looks at the code need not have to guess what other surprises are hidden within the function. The sub-function ComputerUtilCombat.predictPowerBonusOfAttacker() actually has a withoutAbilities boolean that does exactly what I want, it's just that damageIfUnblocked() automatically (wrongly) assumes that it should use abilities by default.

From the above standpoint, AiAttackController's behaviour is an anomaly and I have already repeated my examples endlessly. I have no idea why you insist that this should be the expected behaviour (attack with Frozen Shade or Killer Bees before pump even when the "become attacker for whatever reason" block is specifically there already...) since it hinges on the AI independently choosing to pump said creatures. If you compromise on the coding standard to squeeze in the "gamer/player's" perspective, it just wreaks havoc for everyone. If some-day someone revamps the AI decision system, like say delegate activated ability decisions to the game-state estimation system, this behaviour will break completely again and it is then up to the new designer to figure out what the hell went wrong.

I have been thinking of how to build an AI strategy controller and will probably make a separate post in the near future. In the end, my stand is that functions should do exactly what their name and description says and no more. If optional parameters like kicker/alternate costs and activated abilities are involved, either inherit/composite the function/class or rewrite it with the necessary flags and arguments.

[Agetian]

Lol, this is indeed funny - we're certainly speaking about the same thing (even though from somewhat different points of view) and I even openly state that I agree with your points, yet you insist that I don't understand what you're saying and that you don't understand what I'm saying. Well, as long as there is a de facto understanding (which there is), I'm good with that. As for why I insist that pumping a creature with open lands after it has already been decided to make an attack into an empty opposing battlefield should be the expected behavior for a game that plays Magic: the Gathering - I have provided all my reasons above, and I believe they are clear enough, so I won't go into any further details. Yes, it is the player's perspective, not the coder's perspective. How it works from a coder's perspective is dictated by the design choices that are made and that are to be made, that's different and I don't argue with that - the cleaner the code base, the easier it can be maintained and expanded. Also, do not misunderstand me, I do not think that horrible design decisions should be made as an excuse in order to stay true to the player's perspective, but I also do not believe that purely academic design decision theory, a valid thing in and of itself, should become an excuse for horrible artificial intelligence blunders that lead to incremental losses of tempo, card advantage, material, etc. Ideally, once again, the AI should make decisions that make sense in context (the thing I originally meant) while being well organized internally (the thing that you rightly brought forward as a concern).

Speaking of which, yes, your idea of making a higher level of abstraction that will provide a layer for "strategic decisions" is indeed both practically relevant and interesting and it looks like it can essentially provide an option to make more complex decisions rather than one-shot individual decisions while making the code better organized, which, in its turn, can prove to be the solution for both your concerns and for mine. Once again, in theory it's all nice and dandy, however, it is, at the moment, just that - a theory. I, for one, know that I have neither enough practical experience nor, sadly, enough time at the moment to reorganize the code in this fashion. However, if some day you (or somebody else, for that matter) feel that you can and are ready and willing to refactor the code in a way that will both provide a higher level of abstraction for strategic play *and* stay true to the "one function does one thing" approach, I think it will be of great benefit to everybody working on Forge and also, as a consequence, to people playing Forge (who definitely expect different things than coders). If a project like that ever kicks off, I'll make sure to try and contribute as much as I can in any way that I can be effective with, including testing and some code reorganization according to the standards and principles that you suggest. Good luck!

- Agetian

[excessum]

As for why I insist that pumping a creature with open lands after it has already been decided to make an attack into an empty opposing battlefield should be the expected behavior
- Agetian

I think it all boils down to the above statement. My disagreement is with "after it has already been decided to make an attack" only, your addition to use mana to repeatedly pump attack is perfectly fine with me. In the end it looks like it was just a whole lot of miscommunication .

The offending chunk in AiAttackController that wants to attack with 0/1s by assuming that it will pump them is indeed using ComputerUtilCombat.damageIfUnblocked(). Switching the withoutAbilities flag to true will give the "intuitive" behaviour that I wanted. The drawback, as I mentioned wayyyyyyyy back was that the AI may not choose to make the first +1 pump and make the 0/1 into an eligible attacker if it feels that the opponent's life is too high (10% rounded at 20 life if unblocked).

[Agetian]

As for why I insist that pumping a creature with open lands after it has already been decided to make an attack into an empty opposing battlefield should be the expected behavior
- Agetian

I think it all boils down to the above statement. My disagreement is with "after it has already been decided to make an attack" only, your addition to use mana to repeatedly pump attack is perfectly fine with me. In the end it looks like it was just a whole lot of miscommunication .

The offending chunk in AiAttackController that wants to attack with 0/1s by assuming that it will pump them is indeed using ComputerUtilCombat.damageIfUnblocked(). Switching the withoutAbilities flag to true will give the "intuitive" behaviour that I wanted. The drawback, as I mentioned wayyyyyyyy back was that the AI may not choose to make the first +1 pump and make the 0/1 into an eligible attacker if it feels that the opponent's life is too high (10% rounded at 20 life if unblocked).

Ah, yes, indeed, that makes sense and I agree with your statement here. And yep, of course the initial attack itself doesn't have to be guaranteed (and may depend on a variety of strategic considerations that may happen in the higher, "strategic" layer of AI), but in the case that it does take place (and even whether it takes place or not, at large, given the current game situation) it should, so to speak, "make sense".

- Agetian