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.