[fixed]AI suicide attacks - weird AI attack decisions

[Aswan jaguar]

Describe the Bug:
In dev I get weird AI attack decisions and suicide attacks from AI.
Example: put in play for you a Bear Cub and give AI (following the exact order) an Abattoir Ghoul and any creature that will be killed and not kill Bear Cub (just to make it more clear) let's say Elvish Hexhunter or Stone-Throwing Devils, AI will attack only with the card with the bigger in game ID so only with Elvish Hexhunter or Stone-Throwing Devils. AI will decide correctly and attack only with Abattoir Ghoul if you put Abattoir Ghoul in play after Elvish Hexhunter or Stone-Throwing Devils so Abattoir Ghoul has the bigger in game ID.

This doesn't happen in release no matter which creature has the bigger ID AI will choose correctly and not suicide attack.

Which update are you using? (date, name)Which type? (duel, gauntlet, sealed deck)
dev based on commit 2bfae / duel

What exactly should be the correct behavior/interaction?


Are any other cards possibly affected by this bug?
-

[drool66]

Has to be something in 9c1c671 move ai_choose_attackers() into C

[Korath]

It seems likely. Bisection narrows it to something between 0748183f8 (which broke the build, thanks) and ed36b37a5.

[Aswan jaguar]

And with this report I finally found what was the pattern on Shandalar with AI weird attack suicide. On shandalar based on a commit at 26-12-2020. The exact opposite happens for shandalar. In the above example AI will attack only with the card with the lowest in game id.

[Korath]

I know why they're reversed, at least - the qsort comparator function in the Manalink version is just returning a bool. std::sort() expects a comparator that's equivalent to <, while qsort() expects one equivalent to <=>. The easy pattern if you don't care about overflow is rhs - lhs for a descending sort, which is what we want here.

[Korath]

I see what's causing this: ai_speculate_on_combat() stores its data sorted on card index, not on the order specified in attack_indices[], and so it disagrees with the data stored in ai_choose_attackers(). I have an ugly proof-of-concept fix in the Shandalar version - it builds the data twice - that needs some more thought.

Removing the qsort() call entirely should fix this if you want to release in the meantime, though it loses the improved alpha strikes and not letting creatures after the first sixteen to attack at all.

Two questions specific to the Manalink version:

• Did you verify no cards have EA_ATTACK_LEGALITY_ALL set in ct_all.csv before repurposing it? Shandalar removes it at runtime.
• When do you dispatch TRIGGER_ATTACKER_CHOSEN when the AI attacks, since it's not in ai_choose_attackers() anymore? Not that anything responds to it except Raging River.

[drool66]

Did you verify no cards have EA_ATTACK_LEGALITY_ALL set in ct_all.csv before repurposing it? Shandalar removes it at runtime.

I'm not sure exactly which one it is. The flag was previously "EA_SHANDALAR_RAREST" so I checked the fields labeled "Flags: Unknown (Shandalar)" - these fields are all filled with zeroes.

When do you dispatch TRIGGER_ATTACKER_CHOSEN when the AI attacks, since it's not in ai_choose_attackers() anymore? Not that anything responds to it except Raging River.

I grepped for it and saw nothing in c responded to it, so I didn't worry about it. I forgot about cards in exe

[EDIT]: EVENT_DECLARE_ATTACKERS was originally dispatched early in ai_choose_attackers(), but I see it's also dispatched in main_phase(). Any idea what's going on there? Should I put it back in ai_choose_attackers()?

[Aswan jaguar]

Your commit c885c7d ("Rebuild combat data", 2022-05-04) drool66 to solve this temporarily is not working the issue I posted is still there.

[Korath]

Current draft of the Shandalar version is in commit 9ea5597a3, branch wip/unassessed (mixed with other stuff that I did a week ago and haven't looked at since, hence the name).

Dunno on EVENT_DECLARE_ATTACKERS. main_phase() was clearly a huge mess even in the original, let alone what we're left with after reversing it. There's different codepaths for the AI, and for the human, and for the human during speculation, and there's gotos into the middles of all of them all over to deal with playing back recorded AI decisions.

[drool66]

Your commit c885c7d ("Rebuild combat data", 2022-05-04) drool66 to solve this temporarily is not working the issue I posted is still there.

Bummer - I was testing the wrong case. I can adapt 9ea5597a3; Aswan jaguar, what do you think, should I commit & we can test from there?

[Aswan jaguar]

I can adapt 9ea5597a3; Aswan jaguar, what do you think, should I commit & we can test from there?

Sure.

[drool66]

Ok; 9d91447
Tested working for the case at the top of the thread (Bear Cub, Abatoir Ghoul, Elvish Hexhunter)

[Korath]

The comparator function you're giving to qsort is still invalid as of that commit, and it would be well within its rights to crash instead of just giving you incorrect output (which it certainly will). man qsort, cppreference, some discussion and examples. Be aware the examples produce an ascending sort; you want a descending sort here.

[drool66]

The comparator function you're giving to qsort is still invalid

a0f936c

[Aswan jaguar]

I tested about this issue and is fixed now. I tested few other scenarios and I didn't find any other issues so far.