Go-Shintai and AI

[Hanmac]

Go-Shintai of Hidden Cruelty AI paid the cost even if AI was the only player with destroyable creatures

The Part that makes AI pay this cost need more thinking.

[Agetian]

This is an interesting issue, I'd actually probably need some input on how to best approach fixing this.

The issue is basically that ImmediateTriggerAi calls DestroyAi#doTriggerAINoCost to test this for viability, and the latter doesn't bail despite the absence of preferred targets because it thinks the trigger is mandatory (mandatory is set to "true", so DestroyAi lines 358-359 are not tripped).

Is there any way to detect from inside that DestroyAi call that this is an ImmediateTrigger with a cost?
The "mandatory = true" parameter is passed from ImmediateTriggerAi#doTriggerAINoCost (at line 49). One of the ways to possibly approach this would be to pass "mandatory = false" if a cost is detected on that ImmediateTrigger (since I don't think any of the costs are forced to be paid). What do you think?

[Agetian]

Doing something like this in doTriggerAINoCost of ImmediateTriggerAi seems to do the job:

            if (!sa.getPayCosts().getCostParts().isEmpty()) {
                return aic.doTrigger(trigsa, false);
            }

Not sure what would be the best way to detect the presence of an (optional) cost.

[Hanmac]

for such trigger it is mostly "if you pay, when you do" stuff, so each cost needs to be paid for the trigger to happen

[Agetian]

That's true, but our problem is that the AI is overly eager to pay even if it doesn't have good targets :) The reason it happens is because it always considers these triggers mandatory, which screws up further logic checks since they ignore meaningful/reasonable limitations. Passing mandatory=false for the playability check seems to be a viable option in order to make the AI properly consider playability in a non-forced way (so that if it decides to pay, it'll do stuff).

[Hanmac]

yeah, mandatory=false should do the trick (for now)

[Agetian]

Cool, I'll submit that a bit later today then. What would be the best way to detect the presence of a cost? Is the check above fine in your opinion?

[tool4ever]

you might be able to work with isOptionalTrigger around L48
Edit: ^ actually maybe that is only set when the trigger actually resolves, so as not break forecasting with checkETBEffects manual parsing might still be needed

we need to be careful with the timing here though:
if the ImmediateTriggerAi is called with mandatory=true we can't refuse it yet, that's currently wrong

you only choose the target after paying

[tool4ever]

A few cards like The Restoration of Eiganjo are also written with UnlessCost$, they probably need to be rewritten normally for that to work

[Agetian]

Yeah, not entirely sure what the best approach would be here, tbh :)
Do you have an idea for a better fix, @tool4ever ?

[Agetian]

Btw, if the generic fix at the ImmediateTriggerAi entry point is suboptimal because it may break other cards/interactions when it comes to AI playability, maybe it may be worth marking specific cards with AI logic and then only treating them as non-mandatory at this point for the purpose of AI logic check if the logic is set? Not sure what would be more preferable (depends on how many cards are affected either way, I guess).

[Agetian]

I guess at least a part of the problem is that the current AI code path is written with the assumption that if the trigger code is called that way, it's assumed to have no cost (which is why doTriggerAINoCost is called), while in this case, this assumption is false (for the immediate trigger with a cost). But this is probably a pretty big rewrite for another day if this is to be completely split into two separate execution paths.

[tool4ever]

Yea, I don't think that's needed for now

What should be done imo:

1. always return true first call (with mandatory=true)
2. pass mandatory=false for the Sub checks

the 2. (=second call of ImmediateTriggerAi.doTrigger with mandatory=false) is only done when trigger is optional anyway, so that makes things easier for us

I hope that makes sense :)

[Agetian]

Yeah, I think that for now, we can simply check if ImmediateTriggerAi is on a subability, and if it is, then pass "false" for mandatory for the purpose of AI logic checks. What do you think?

[tool4ever]

Hmn, do you mean the other way around? For the card of this issue, sa will still be AbilityApiBased

Seems a bit complicated tbh, it's also not 100% guaranteed that it is Optional in that case (maybe scripter just did AB$ without Cost$ or OptionalDecider$ anyway)

[Agetian]

Hmm, then I'm not sure how to do this correctly ^^; Can you please help?

[tool4ever]

Sure, I'll propose a PR in a bit

[Agetian]

Ok, thanks a lot! 👍