Let the AI count converge before it decides whether to cast

[liamiak]

Rebased onto master. #11515 took the PermanentAi piece, so that is out of this branch — what
remains is the prediction and the paths the loop cannot reach.

Count$Converge reads 0 while deciding. Nothing has been spent yet and getCastSA() is null, so
Bring to Light searched for a 0-drop and never cast. getPayingColors() now asks the activating
player's controller what it expects to spend: PlayerController answers 0, so humans are unchanged,
and PlayerControllerAi forwards to what AiController worked out. Nothing is parked on the spell.

Storing it. The prediction lives on AiController, keyed by spell identity, reset at priority
next to AiCache.clear() — the same species of state as HELD_MANA_SOURCES_FOR_NEXT_SPELL four
lines up. canPlayAndPayFor snapshots and restores it, so a nested evaluation cannot lose the outer
spell's answer. Keeping the announced X alongside the colours also settles the recompute you
flagged: the same spell reaches setMaxXValue once from the prediction and again from its own API
logic, and each walk solves the payment at every X. Chamber Sentry off five lands, 12 payment solves
→ 6; off nine lands, with a five-colour early exit, 20 → 6.

Sweep the Skies is the case the loop does not reach. TokenAmount$ Y reads Count$Converge off
a different SVar than X announces, so neither the per-API override nor PermanentAi ever sized it —
one thopter on master, five here.

The four Count$Converge overrides are gone. Each worked the amount out with calculateAmount
and then overrode it, because calculateAmount returned zero before anything had been paid. With
the prediction the engine's own expression is right, and getConvergeCount has no callers left.

Then it tutored for the worst card. chooseCardToHiddenOriginChangeZone treats
Destination$ Exile as dumping junk, so on your own library it took getWorstAI — Bring to Light
picked a Runeclaw Bear over a Serra Angel. Exile is a staging step before DB$ Play. Eight
unflagged cards share this. Scoped by calling the chooser across every destination × origin ×
own-or-opponent: one cell of thirty moves.

Engineered Explosives stays flagged — its counters select which mana value the wipe hits, so most
colours is usually the worst live choice.

355 tests, 0 failures.

🤖 Implemented with the assistance of Claude Code (Opus 5).

[tool4ever]

don't like having to create extra fake Mana

what are the alternatives?

[liamiak]

You're right — I missed this one on the first pass. Working it now, expect another push.

[liamiak]

Pushed. Went with predicting the colours rather than the payment: a byte predictedPayingColors on SpellAbility that getPayingColors() ORs in, set and cleared on the same lines that already borrow and return castSA.

payingMana is a multiset with provenance and the prediction is only a set of colours, so the Mana objects were right about colour and invented the rest — one entry per colour, each sourced from the spell itself, which is what Count$Adamant, Count$TotalManaSpent and Count$EachSpentToCast read.

On the redundancy point: getSunburst() had no callers left once getConvergeCount was rewritten — it was ColorSet.fromMask(sunburstMap).countColors() over the same field getColorsPaid() returns — so I removed it. Happy to put it back if you'd rather keep the accessor.

To be straight about the scope: the mask is only set for hasConverge() cards, and none of the 31 in the pool use ManaColorsPaid or ManaSpent, so Count$Converge is the only reader that can observe a non-zero value today. Outside the AI's own evaluation getPayingColors() is unchanged.

[tool4ever]

Mhn, I'll think about it though a lot of work has previously gone into removing AI only stuff from the main rules engines...

[tool4ever]

don't mind a more generic solution but then you need to replace the specific variant above
otherwise we end up with duplicated logic that also wastes runtime
though the ones with X will probably make things tricky 🤔

[liamiak]

Agreed, and that's done as of the push after your review — getConvergeCount is now just ColorSet.fromMask(getConvergeColors(sa, ai)).countColors(), so there's one payment simulation rather than two. Same result either way: getSunburst() was already ColorSet.fromMask(sunburstMap).countColors() and getColorsPaid() returns that same mask.

The X ones are in that push too — X gets announced before the colours are measured, since on those cards X is what buys them.

[tool4ever]

well the code duplication is gone but the method is still used and calculates again for no reason?

[liamiak]

You're right, and it goes further than that — the method can go entirely.

All four callers had the same shape: work out the amount with calculateAmount, then override it for Count$Converge. That override exists because calculateAmount returned zero before anything had been paid, which is the problem this branch set out to fix. With the prediction in place the engine's own expression gives the right number, so the override was running a full payment simulation to reach a value the line above it already held.

I checked rather than assumed, since it would be easy to see the right number coming from resolution instead of evaluation. Instrumenting the Count$Converge branch in AbilityUtils and tagging each call with its caller: 21 of them arrive via ChangeZoneAi.hiddenOriginCanPlayAI — the AI deciding whether to cast — with castSA set and the count correct.

PermanentAi's converge loop was already replaced by setXForBestConverge earlier on this branch, so removing these four leaves getConvergeCount with no callers and it goes too. That commit is +1/-20.

Verified on the paths I could reach. Painful Truths on the DrawAi path and Unified Front on TokenAi both get cast and sized correctly with the override gone — three cards and three life, four tokens. I added the first as a regression test. DamageAllAi I could not exercise: its checkApiLogic is never reached for Radiant Flames, the only DamageAll converge card in the pool. It is the same one-line pattern as the other two so I would not expect it to differ, but it is untested here rather than verified.

339 tests, 0 failures.

[liamiak]

Pushed two commits.

reuse the converge colours the X search already found — the recompute you spotted. setXForBestConverge now returns the mask it settled on instead of the caller working it out again: 13 → 12 payment simulations casting Chamber Sentry off five lands.

offer the converge prediction through PlayerController instead — an alternative for your point about AI-only state in the rules engine. Not meant to land alongside the field version; take whichever you prefer and I'll drop the other.

It parks nothing on the spell. getPayingColors() asks the activating player's controller what it means to spend while nothing has been spent; PlayerController answers 0 so humans are unchanged, and PlayerControllerAi forwards to what AiController worked out. One seam still serves Converge, Count$ManaColorsPaid and both ManaSpent checks, and there's no mask left to go stale.

Scoped twice — canPlayAndPayFor hands back whatever the outer spell was expecting, so a nested evaluation can't lose the outer prediction, and chooseSpellAbilityToPlay resets it next to AiCache.clear() and predictedCombat. HELD_MANA_SOURCES_FOR_NEXT_SPELL four lines up is the same species of state: good for one spell, kept AI side, reset at priority.

The honest trade: the field version is +17/−21 across three files, this is +60/−17 across six, and the prediction becomes controller-scoped rather than spell-scoped, so a runWithController swap loses it. Both cases degrade to 0, which is the answer before any of this.

[tool4ever]

my recompute was still meant for something else but I'll try to explain if I can also figure out good prediction storing to revisit it

I think for now I'll just cherry pick the fix in PermanentAi while keeping the early loop exit around

[liamiak]

Rebased — #11515 is in, so the PermanentAi loop is out of this branch.

I re-ran the tests against plain master to see what the rest is still worth: Chamber Sentry and
Skyrider Elf now pass without any of this, so those two tests stopped proving anything and I have
stopped claiming them. Only Bring to Light still fails. I added one more for Sweep the Skies, whose
TokenAmount$ Y reads Count$Converge off a different SVar than X announces — one thopter on
master, five with the prediction.

On prediction storing: I had a go while this was waiting. It sits on AiController keyed by spell
identity, reset at priority next to AiCache.clear(), and canPlayAndPayFor snapshots and restores
it so a nested evaluation cannot clobber the outer spell's answer. Keeping the announced X with the
colours is also what settles your recompute — Chamber Sentry off five lands went 12 payment solves
to 6, and off nine lands, with a five-colour early exit, 20 to 6.

No rush from me. If you would rather land your own version of the storing, say so and I will drop
this to just the Sweep the Skies and tutor pieces.