From 095bc2ab5c802a1a3ecf1d5206955945929bffbf Mon Sep 17 00:00:00 2001 From: liamiak Date: Sun, 2 Aug 2026 21:24:32 -0600 Subject: [PATCH 1/2] Let the AI use Goblin Welder The ability targets the artifact that gets sacrificed and the graveyard card that replaces it, so the two have to be read as one trade. PumpAi read neither: with no AILogic it fell through to the generic pump targeting, which takes the most expensive artifact on the battlefield regardless of who owns it. Measured on a board of Goblin Welder, Chromatic Star and Sol Ring, with a Wurmcoil Engine in our own graveyard, it sacrificed the Sol Ring. Given only a worse card in the yard it still welded, trading a Sol Ring for an Ornithopter. And with the opponent holding the biggest artifact it targeted theirs, sacrificing their Wurmcoil Engine to hand them the Blightsteel Colossus out of their own graveyard. AILogic$ Weld picks the pair together. Ours: something already marked expendable through the existing SacCost preference, otherwise the cheapest artifact we control, swapped for the best card in our graveyard. An opponent's: their best board artifact for the worst card in their graveyard, which turns the gift above into removal. Either way the card coming back has to cost more than the one going away - the test ZoneExchangeAi already uses for a swap across zones, and half of what ChangeZoneAi.doSacAndReturnFromGraveLogic uses for sacrifice-and-reanimate. Timing goes through the shared AtOppEOT rule rather than the mid-combat window the generic pump phase check happened to allow. Both branches sit behind the new AILogic, which only this card carries, so nothing else that goes through PumpAi changes. Co-Authored-By: Claude Opus 5 --- .../main/java/forge/ai/ability/PumpAi.java | 58 +++++++++++++++++++ forge-gui/res/cardsfolder/g/goblin_welder.txt | 5 +- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/ability/PumpAi.java b/forge-ai/src/main/java/forge/ai/ability/PumpAi.java index 9645a9017173..699d834b304d 100644 --- a/forge-ai/src/main/java/forge/ai/ability/PumpAi.java +++ b/forge-ai/src/main/java/forge/ai/ability/PumpAi.java @@ -80,6 +80,9 @@ protected boolean checkPhaseRestrictions(final Player ai, final SpellAbility sa, if (ph.getPhase().isAfter(PhaseType.COMBAT_FIRST_STRIKE_DAMAGE) || !ph.inCombat()) { return false; } + } else if (logic.equals("Weld")) { + // nothing is gained by welding early, so hold it to the last moment before our turn + return super.checkPhaseRestrictions(ai, sa, ph, "AtOppEOT"); } return super.checkPhaseRestrictions(ai, sa, ph); } @@ -233,6 +236,8 @@ protected AiAbilityDecision checkApiLogic(Player ai, SpellAbility sa) { } else if (aiLogic.startsWith("Donate")) { // Donate step 1 - try to target an opponent, preferably one who does not have a donate target yet return SpecialCardAi.Donate.considerTargetingOpponent(ai, sa); + } else if ("Weld".equals(aiLogic)) { + return doWeldLogic(ai, sa); } else if (aiLogic.equals("InfernoOfTheStarMounts")) { int numRedMana = ComputerUtilMana.determineLeftoverMana(new SpellAbility.EmptySa(source), ai, "R", false); int currentPower = source.getNetPower(); @@ -345,6 +350,55 @@ protected AiAbilityDecision checkApiLogic(Player ai, SpellAbility sa) { return new AiAbilityDecision(0, AiPlayDecision.TargetingFailed); } + /** + * Goblin Welder: the artifact targeted here is sacrificed and the artifact card targeted by the + * sub-ability replaces it, both belonging to the same player. Worth doing when that trade is an + * upgrade for us, or a downgrade for an opponent. + */ + private AiAbilityDecision doWeldLogic(final Player ai, final SpellAbility sa) { + final SpellAbility returnSa = sa.getSubAbility(); + if (returnSa == null) { + return new AiAbilityDecision(0, AiPlayDecision.CantPlayAi); + } + + sa.resetTargets(); + final CardCollection targetable = CardLists.getTargetableCards(ai.getGame().getCardsIn(ZoneType.Battlefield), sa); + + final List welders = Lists.newArrayList(ai); + welders.addAll(ai.getOpponents()); + for (final Player p : welders) { + final boolean ours = p == ai; + final CardCollection board = CardLists.filter(targetable, CardPredicates.isController(p)); + final Card sacrificed = ours ? weldSacrifice(ai, sa, board) + : ComputerUtilCard.getBestRemovalTargetAI(ai, board); + if (sacrificed == null) { + continue; + } + + // once the parent target is set the sub-ability can choose what comes back, so what is + // weighed here is what will actually be targeted + sa.getTargets().add(sacrificed); + if (pumpMandatoryTarget(ai, returnSa)) { + final Card returned = returnSa.getTargetCard(); + // upgrading ourselves, or leaving an opponent with the lesser artifact + if (ours ? returned.getCMC() > sacrificed.getCMC() : sacrificed.getCMC() > returned.getCMC()) { + return new AiAbilityDecision(100, AiPlayDecision.WillPlay); + } + } + sa.resetTargets(); + returnSa.resetTargets(); + } + + return new AiAbilityDecision(0, AiPlayDecision.TargetingFailed); + } + + private static Card weldSacrifice(final Player ai, final SpellAbility sa, final CardCollection board) { + // anything already marked as expendable goes first, since mana value alone cannot tell a + // Sol Ring apart from a Chromatic Star + final Card expendable = ComputerUtil.getCardPreference(ai, sa.getHostCard(), "SacCost", board, sa); + return expendable != null ? expendable : ComputerUtilCard.getWorstAI(board); + } + private boolean pumpTgtAI(final Player ai, final SpellAbility sa, final int defense, final int attack, final boolean mandatory, boolean immediately) { final List keywords = sa.hasParam("KW") ? Arrays.asList(sa.getParam("KW").split(" & ")) @@ -419,6 +473,10 @@ private boolean pumpTgtAI(final Player ai, final SpellAbility sa, final int defe return true; } return false; + } else if (sa.getParam("AILogic").equals("Weld")) { + // the parent has already settled whose artifact is going away, and the graveyard it + // has to come out of: best from our own, cheapest from theirs + return pumpMandatoryTarget(ai, sa); } else if (sa.getParam("AILogic").equals("SameName")) { return doSameNameLogic(ai, sa); } else if (sa.getParam("AILogic").equals("SacOneEach")) { diff --git a/forge-gui/res/cardsfolder/g/goblin_welder.txt b/forge-gui/res/cardsfolder/g/goblin_welder.txt index 04954a4e0f1e..99d42f01c9a4 100644 --- a/forge-gui/res/cardsfolder/g/goblin_welder.txt +++ b/forge-gui/res/cardsfolder/g/goblin_welder.txt @@ -2,8 +2,8 @@ Name:Goblin Welder ManaCost:R Types:Creature Goblin Artificer PT:1/1 -A:AB$ Pump | Cost$ T | ValidTgts$ Artifact | TgtPrompt$ Select target artifact a player controls | RememberObjects$ ThisTargetedCard | SubAbility$ DBTargetYard | StackDescription$ If both targets are still legal as this ability resolves, {p:TargetedController} simultaneously sacrifices {c:ThisTargetedCard} | SpellDescription$ Choose target artifact a player controls and target artifact card in that player's graveyard. If both targets are still legal as this ability resolves, that player simultaneously sacrifices the artifact and returns the artifact card to the battlefield. -SVar:DBTargetYard:DB$ Pump | ValidTgts$ Artifact | TargetsWithDefinedController$ ParentTargetedController | TgtPrompt$ Select target artifact card in that player's graveyard | TgtZone$ Graveyard | PumpZone$ Graveyard | ImprintCards$ ThisTargetedCard | StackDescription$ and returns {c:ThisTargetedCard} to the battlefield. | SubAbility$ DBBranch +A:AB$ Pump | Cost$ T | ValidTgts$ Artifact | AILogic$ Weld | TgtPrompt$ Select target artifact a player controls | RememberObjects$ ThisTargetedCard | SubAbility$ DBTargetYard | StackDescription$ If both targets are still legal as this ability resolves, {p:TargetedController} simultaneously sacrifices {c:ThisTargetedCard} | SpellDescription$ Choose target artifact a player controls and target artifact card in that player's graveyard. If both targets are still legal as this ability resolves, that player simultaneously sacrifices the artifact and returns the artifact card to the battlefield. +SVar:DBTargetYard:DB$ Pump | ValidTgts$ Artifact | AILogic$ Weld | TargetsWithDefinedController$ ParentTargetedController | TgtPrompt$ Select target artifact card in that player's graveyard | TgtZone$ Graveyard | PumpZone$ Graveyard | ImprintCards$ ThisTargetedCard | StackDescription$ and returns {c:ThisTargetedCard} to the battlefield. | SubAbility$ DBBranch SVar:DBBranch:DB$ Branch | BranchConditionSVar$ TargetCheck | BranchConditionSVarCompare$ GE2 | TrueSubAbility$ DBSacrifice | FalseSubAbility$ DBCleanup SVar:DBSacrifice:DB$ SacrificeAll | ValidCards$ Card.IsRemembered | SubAbility$ DBReturn SVar:DBReturn:DB$ ChangeZone | Defined$ Imprinted | Origin$ Graveyard | Destination$ Battlefield | SubAbility$ DBCleanup @@ -11,6 +11,5 @@ SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True | ClearImprinted$ True SVar:TargetCheck:SVar$CheckRemem/Plus.CheckImprint SVar:CheckRemem:Remembered$Valid Artifact.sharesControllerWith Imprinted SVar:CheckImprint:Imprinted$Valid Artifact.sharesControllerWith Remembered -AI:RemoveDeck:All AI:RemoveDeck:Random Oracle:{T}: Choose target artifact a player controls and target artifact card in that player's graveyard. If both targets are still legal as this ability resolves, that player simultaneously sacrifices the artifact and returns the artifact card to the battlefield. From 4171132e0d227cfe6672133ebb3298a659327386 Mon Sep 17 00:00:00 2001 From: liamiak Date: Sun, 2 Aug 2026 21:24:32 -0600 Subject: [PATCH 2/2] Add a regression test for the Goblin Welder trade Plays until the AI welds of its own accord at the opponent's end step, and checks what it chose: our own worst artifact swapped up into the best one in our graveyard, an opponent's artifact traded down for the junk in theirs, and nothing at all when the swap gains nothing or the graveyard is worse. Drop this commit if you would rather not carry the test. Co-Authored-By: Claude Opus 5 --- .../forge/ai/ability/GoblinWelderAiTest.java | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 forge-gui-desktop/src/test/java/forge/ai/ability/GoblinWelderAiTest.java diff --git a/forge-gui-desktop/src/test/java/forge/ai/ability/GoblinWelderAiTest.java b/forge-gui-desktop/src/test/java/forge/ai/ability/GoblinWelderAiTest.java new file mode 100644 index 000000000000..0b8464dd7f3e --- /dev/null +++ b/forge-gui-desktop/src/test/java/forge/ai/ability/GoblinWelderAiTest.java @@ -0,0 +1,123 @@ +package forge.ai.ability; + +import org.testng.annotations.Test; + +import forge.ai.AITest; +import forge.game.Game; +import forge.game.card.Card; +import forge.game.phase.PhaseType; +import forge.game.player.Player; +import forge.game.zone.ZoneType; + +import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.assertTrue; + +/** + * Goblin Welder targets the artifact that gets sacrificed and the graveyard card that replaces it, + * so the AI has to read both halves as one trade before it picks either one. + */ +public class GoblinWelderAiTest extends AITest { + + private Card welder(Player ai) { + Card welder = addCard("Goblin Welder", ai); + welder.setSickness(false); + return welder; + } + + private void endOfOpponentsTurn(Game game, Player opp) { + // Chromatic Star draws a card on the way out, and decking ends the game before we can look + for (Player p : game.getPlayers()) { + fillLibrary(p, 10); + } + game.getPhaseHandler().devModeSet(PhaseType.END_OF_TURN, opp); + game.getAction().checkStateEffects(true); + gameLoopUntilNextPhase(game); + } + + @Test + public void weldsOurWorstArtifactIntoOurBest() { + Game game = initAndCreateGame(); + Player ai = game.getPlayers().get(1); + Player opp = game.getPlayers().get(0); + + welder(ai); + addCard("Chromatic Star", ai); + addCard("Sol Ring", ai); + addCardToZone("Wurmcoil Engine", ai, ZoneType.Graveyard); + + endOfOpponentsTurn(game, opp); + + assertEquals(1, countCardsWithName(game, "Wurmcoil Engine")); + assertEquals(1, countCardsWithName(game, "Chromatic Star", ZoneType.Graveyard)); + assertEquals("kept the artifact worth keeping", 1, countCardsWithName(game, "Sol Ring")); + } + + @Test + public void leavesTheOpponentNoBetterOffThanTheyWere() { + Game game = initAndCreateGame(); + Player ai = game.getPlayers().get(1); + Player opp = game.getPlayers().get(0); + + welder(ai); + addCard("Chromatic Star", ai); + addCardToZone("Ornithopter", ai, ZoneType.Graveyard); + // welding for the opponent here would trade their Wurmcoil up into a Blightsteel Colossus + addCard("Wurmcoil Engine", opp); + addCardToZone("Blightsteel Colossus", opp, ZoneType.Graveyard); + + endOfOpponentsTurn(game, opp); + + assertEquals(0, countCardsWithName(game, "Blightsteel Colossus")); + assertEquals(1, countCardsWithName(game, "Wurmcoil Engine")); + assertEquals("nothing of ours was spent either", 1, countCardsWithName(game, "Chromatic Star")); + } + + @Test + public void tradesTheOpponentsArtifactDownForTheirOwnJunk() { + Game game = initAndCreateGame(); + Player ai = game.getPlayers().get(1); + Player opp = game.getPlayers().get(0); + + welder(ai); + addCard("Wurmcoil Engine", opp); + addCardToZone("Ornithopter", opp, ZoneType.Graveyard); + + endOfOpponentsTurn(game, opp); + + assertEquals(1, countCardsWithName(game, "Wurmcoil Engine", ZoneType.Graveyard)); + assertTrue("the Ornithopter came back instead", countCardsWithName(game, "Ornithopter") == 1); + } + + @Test + public void declinesASwapWorthNothing() { + Game game = initAndCreateGame(); + Player ai = game.getPlayers().get(1); + Player opp = game.getPlayers().get(0); + + welder(ai); + addCard("Sol Ring", ai); + // same mana value, so the AI cannot tell these apart and has nothing to gain by swapping + addCardToZone("Chromatic Star", ai, ZoneType.Graveyard); + + endOfOpponentsTurn(game, opp); + + assertEquals(1, countCardsWithName(game, "Sol Ring")); + assertEquals(0, countCardsWithName(game, "Chromatic Star")); + } + + @Test + public void leavesItAloneWithNothingToGain() { + Game game = initAndCreateGame(); + Player ai = game.getPlayers().get(1); + Player opp = game.getPlayers().get(0); + + welder(ai); + addCard("Sol Ring", ai); + addCardToZone("Ornithopter", ai, ZoneType.Graveyard); + + endOfOpponentsTurn(game, opp); + + assertEquals(1, countCardsWithName(game, "Sol Ring")); + assertEquals(0, countCardsWithName(game, "Ornithopter")); + } +}