AI: fetch the land colour it is actually short of

[liamiak]

Land searches picked by list order. basicManaFixing chose the basic type the player had fewest of, then took list.get(0) from whatever survived that filter; getBestLandAI ended in Aggregates.random. Neither asked which colours were actually blocking anything.

Every fetchland comes through here — areAllBasics("Plains,Island") is true — and a "Plains" search matches every dual carrying the Plains type. Over 12 seeded AI-vs-AI games (deck 260613, three seeds) basicManaFixing fires 46 times, 44 with a real choice. One observed minType=Island decision offered:

Tundra(WU)  Underground Sea(UB)  Volcanic Island(UR)  Tropical Island(UG)
Raffine's Tower(WUB)  Ketria Triome(URG)  Breeding Pool(UG)  ...

All carry Island; all differ beside it. It took Tundra because Tundra was first.

The measure

ComputerUtilCard.getColorFixingValue(player, land) is the single number every caller ranks by: how many missing colour sources that land supplies across the player's hand and the activatable abilities on their permanents, plus depth in the colours they are thin on. Counting sources rather than colours is what credits a second Swamp towards BB, which a colour mask calls payable off a single one.

The parts are countMissingSources, countSourcesFixed and evaluateSpareSources.

Colour need is asked before the basic-type count, because that count cannot tell a colour that is missing from one that is merely uncommon: with three Islands and a hand wanting black it concluded it needed Plains — having none — and fetched a Plains-Island.

An "Any" source counted for nothing

Found while answering review here, and folded in because it is the same code path. getAvailableManaColors collected the raw Produced$ string, and every caller runs that through ColorSet.fromNames, which keeps only colour names — so Any was dropped entirely:

board, {W} in hand	before	after
3x City of Brass	no colours	WUBRG
3x Mana Confluence	no colours	WUBRG
3x Island	U	U

Checked against ComputerUtilMana.canPayManaCost as ground truth: before, the any-colour boards disagreed with it; after, every row agrees and the negatives stay negative. It now asks getProducibleColors, which resolves the colours and makes the set bounded, so it can also stop once every colour is present.

Testing

mvn -pl forge-gui-desktop -am test: 358 tests, 0 failures.

Seven tests, sized by mutation rather than by count: removing the depth term, its diminishing returns, the hand scan, the permanent-cost exclusion, the per-colour pip counting, the search narrowing, the tapped-source invariance or the any-colour fix each turns at least one of them red.

One caveat on the numbers above: the 46/44 counts describe the old behaviour and still stand, but the share of picks that change was measured against the first version of the metric and has not been re-run since the scoring changed.

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

[tool4ever]

might be some interesting ideas here but too messy

logic should be shared/consolidated around chooseBestLandToPlay
and if there's a difference needed for fetching vs. playing connect them with clear path

[liamiak]

Consolidated around chooseBestLandToPlay as you suggested — the colour logic is in one place now and every caller ranks by the same number.

ComputerUtilCost.getManaSourceCounts is the shared primitive: sources of each colour a player could produce, optionally counting one more card. ComputerUtilCard.getColorFixingValue is the one number lands are ranked by. chooseBestLandToPlay, basicManaFixing and getBestLandAI all use it, so the two hand-rolled colour scans in AiController are gone — it's +3/-39 there. Only that one method and one constant are public; the generic pickStandout/bestBy helpers you reacted to are deleted.

Fetching and playing differ in exactly two explicit things: fetching can be for another player, and the pool is a library rather than a hand.

Building it that way turned up three bugs in my own first version, all fixed in this push and covered by tests:

• reading the produced-mana string missed "any colour" lands — Mana Confluence scored below a basic Plains
• a colour mask couldn't tell one source of a colour from two, so BB looked payable off a single Swamp
• getAllSpellAbilities also handed back a permanent's own casting cost and the far face of an MDFC or Adventure, so a resolved Bonecrusher Giant made a Mountain look needed

Measured on a 40-permanent board: getColorFixingValue is 86us, so a land drop with eight candidates costs 0.69ms once per turn — and the shared scan is cheaper than the getAvailableManaColors call it partly replaces. A 12-game seeded mirror sim finished 6-6 with no exceptions.

Known limits: a multi-colour source counts once per colour though it makes one mana; COLOR_FIXING_WEIGHT is picked to match the existing scale rather than tuned against deck costs; and Command Tower measured 0, but only because the test harness has no commander for ColorIdentity to resolve against — it goes through canProduce, which handles Combo ColorIdentity, so it should be right in an actual game.

[liamiak]

Thank you for taking a look at it!

All five, in one push — they turned out to share a cause.

MagicColor.Color.values() is exactly WUBRGC in that order, so it replaces the hand-written array outright and its ordinal() indexes the counts. I moved shortfall onto the same enum too, so the ordering has one source of truth instead of two that happened to agree.

getProducibleManaColors is deleted. It had no callers left once counts replaced it, and reading getOrigProduced was the less accurate check — that's what made it miss "any colour" lands. getAvailableManaColors goes back to exactly what it was.

On canProduceColorMana — I went one level down and used what it's built on. canProduceSameManaTypeWith already walked the mana abilities collecting colours via CardUtil.canProduce, and handled ManaReflected; I extracted that as Card.getProducibleColors so both callers share it.

The two scans are one: the candidate can only add to what's already on the battlefield, so the second set of counts is a clone plus that one card. And basicManaFixing now scores each candidate once, keeping the best as it goes, instead of finding the maximum and filtering by recomputing it.

One thing worth flagging from doing this. Those canProduce calls are far more expensive when the ability has no activating player set — on a 40-permanent board the scan is 30us with one and 1191us without. So getProducibleColors fills it in, the way getMaxManaProduced already does. My first version set it unconditionally, which was wrong: ComputerUtilMana assigns a payer to mana abilities while it works out a payment, and overwriting that mid-simulation would have replaced state the simulation was relying on. It now only fills the field in when it's empty, so a payment in progress keeps its own. That's also the faster of the two, since abilities keep whatever was already set.

getColorFixingValue is 58us on that board now, against 86us before the review. 357 tests, 0 failures.

[tool4ever]

shouldn't this loop offer early exit in case colors is full?

[liamiak]

Added, and in the end in both places.

Within one card it is safe to break because canProduce(6, …) and getReflectableManaColors both draw from COLORS_AND_COLORLESS, so the set cannot exceed six. It earns very little there though — of 1,870 mana-producing cards in the pool, only Plaza of Heroes and White Lotus Hideout still have an ability left to walk once the set is full.

Across sources it is worth much more, but it needed a fix first. getAvailableManaColors was collecting the raw Produced$ string, so its set held Any and Combo ColorIdentity alongside W and there was no size at which it was full. Worse, since every caller runs it through ColorSet.fromNames, which keeps only colour names, an Any source was contributing nothing at all — three City of Brass read as no colours available, and canBePaidWithAvailable then disagreed with ComputerUtilMana.canPayManaCost about a plain {W}.

It now asks getProducibleColors, which resolves those and makes the set bounded, so the break there is both correct and fires on any five-colour board. Thanks for the nudge — I would not have looked at that method otherwise.

[tool4ever]

After further consideration I feel like complicating the logic this way isn't the right path forward since including all the available mana costs still says nothing about if AI would even want to pay them :/

Therefore it could be highly situational if building the mana base simply around missing colors doesn't lead to a better outcome.
Additionally this overlaps with my same argument in #11373 (though arguably a wrong result here isn't as bad as an untap-ramp into nothing) so a clean approach would benefit both.

These PR were still helpful for letting me think about the bigger picture, even if it's sometimes painful to untangle the AI code :P
I might try to cherry pick some of the cleanup done here later...

[liamiak]

Rebased onto current master, and pushed a change that I hope answers the objection rather than restating the PR.

You were right that counting what the board can produce says nothing about whether the AI would want to pay it. What the counting also cannot say is whether a land lets anything be cast this turn. With one Forest out and Doom Blade {1}{B} and Angel of Mercy {4}{W} in hand, a Swamp and a Plains each fix exactly one missing colour and score identically — but only the Swamp casts anything, and two lands is nowhere near five mana.

ComputerUtilCost.isPayableWith asks that. An untapped land entering play is one more mana of a colour, so it pays one pip up front the way convoke does and puts the remainder to ComputerUtilMana — nothing is moved and the answer is the engine's own. The source counting stays as the graded "how much closer" term; this is added alongside it, weighted at two colour-fixes rather than as a dominant term, since a mana base outlives the turn.

It also takes extra generic mana, because UntapAi.untapReachesASpell asks the same question about reusing a tapped source, and the TODO there asks for exactly the colour form this provides. So it is one helper for both, which I think is the shape you were asking for.

+210 lines over the previous state, four files. Confined to land decisions — chooseBestLandToPlay, getBestLandAI and basicManaFixing — and it never runs during ordinary priority evaluation. Measured end to end, seeded and in one JVM, at 9-15% of a fast game. Suite 368/0/6, checkstyle clean.

Worth being straight about the other half of that: I can measure what this costs but not what it wins. Seeded AI-vs-AI games measure noise at about the same magnitude, so whether better land choice is worth it is not something this harness can settle.

The cleanup you mentioned cherry-picking is still here and still independent of all of this.

[tool4ever]

My PoC: #11673

Further improvements later and in smaller steps.

[liamiak]

Re-tested on current master with #11673 in.

It rebases with one conflict, in the basic-selection loop we both touch, and the two compose — your shard division still runs, this PR only changes which lands reach the loop. One thing needs your call: #11673 reads UNPAID_COSTS for ai, and this PR splits that parameter into decider and owner. I used owner, since the land ends up with its owner, but they only differ when something fetches for another player — say if you'd rather it followed the decider.

On overlap, I had assumed #11673 covered the basic-land case and this PR only added non-basics. Measuring it says otherwise. canPlayAndPayForFace runs canPlaySa before canPayCost, so only spells that already have a legal target ever reach payManaCost and get recorded. With three Islands out and black cards in hand that have nothing to target, UNPAID_COSTS stays empty for the whole turn — I instrumented the write site to check.

That leaves the two mechanisms complementary rather than overlapping. Yours answers "I tried and could not pay"; this one answers "I can see what I am short of before I try". Added LandFetchColorNeedTest for the case: no Plains and no Swamps owned, so neither the existing count nor the shard bias can separate them and the tie falls to BASIC_LANDS order. It fetches a Plains on master with #11673, a Swamp with this.

Happy to still cut this down — the non-basic ranking and the land drop from hand are the parts #11673 does not reach, and I can drop the rest. Say which shape you want.

Suite 370/0/6, checkstyle clean.

[tool4ever]

I guess broadly speaking we'd have three main fetch factors to consider:
a) colorful mana base
b) missing color for something AI wanted to pay
c) color bias of available cards

I'd prefer some reasoning first for how these should be connected by default - is it more priority-like based on above order where only ties get attempted to resolve with another heuristic?
or should they be mixed together via some weighting, like I tried with a+b? 🤔

[liamiak]

Non-AI here: I think the answer is greedy-to hand and then balanced to deck, unless we have longer-term AI plans/needs that we lack:

1. Immediate — do we unlock something this turn?
2. Drawn — Do we get closer to a cost we've drawn we can't play yet on hand/board?
3. Potential draw — one of every colour the deck wants, then two, etc - maximize flexibility.

AI:
On your a/b/c: (b) is really two factors, split by timing — 1 and 2 above. That split is the part of this PR I'd most want to keep, because counting sources can't see it: with one Forest out and {1}{B} and {4}{W} in hand, a Swamp casts a spell now and a Plains doesn't, and both fix exactly one source. (a) and (c) I don't think are independent — (a) is the mechanism and (c) is the yardstick. Breadth is only worth having over colours the deck actually plays, which makes them one tier, not two.

Only 3 is a genuine tiebreak. 1 and 2 are commensurable (both count spells made payable), so I'd keep them as weights in one scale rather than a strict lexicographic override — otherwise a land unlocking one trivial spell beats a land fixing three colours.

For 3, AiDeckStatistics.maxPips is the yardstick already: GameStateEvaluator.evalManaBase caps a colour at min(counts[i], maxPips[i]). That cap is right but flat, so it's indifferent between the 2nd white source and the 1st; the falloff in this PR is breadth-first but uncapped. Falloff within the cap gives 3, and replaces this PR's ad-hoc term with your existing statistic. Reasoned, not measured — say the word and I'll build it and post numbers.