Reworked cost adjuster logic for better support of X and cost modification effects:

Improves:
* refactor: split CostAdjuster logic in multiple parts - prepare X, prepare cost, increase cost, reduce cost;
* refactor: improved VariableManaCost to support min/max values, playable and AI calculations, test framework;
* refactor: improved EarlyTargetCost to support mana costs too (related to #13023);
* refactor: migrated some cards with CostAdjuster and X to EarlyTargetCost (Knollspine Invocation, etc - related to #13023);
* refactor: added shared code for "As an additional cost to cast this spell, discard X creature cards";
* refactor: added shared code for "X is the converted mana cost of the exiled card";
* tests: added dozens tests with cost adjusters;

Bug fixes:
* game: fixed that some cards with CostAdjuster ignore min/max limits for X (allow to choose any X, example: Scorched Earth, Open The Way);
* game: fixed that some cards ask to announce already defined X values (example: Bargaining Table);
* game: fixed that some cards with CostAdjuster do not support combo with other cost modification effects;
* game, gui: fixed missing game logs about predefined X values;
* game, gui: fixed wrong X icon for predefined X values;

Test framework:
* test framework: added X min/max check for wrong values;
* test framework: added X min/max info in miss X value announce;
* test framework: added check to find duplicated effect bugs (see assertNoDuplicatedEffects);

Cards:
* Open The Way - fixed that it allow to choose any X without limits (close #12810);
* Unbound Flourishing - improved combo support for activated abilities with predefined X mana costs like Bargaining Table;
This commit is contained in:
Oleg Agafonov 2025-04-08 22:39:10 +04:00
parent 13a832ae00
commit bae3089abb
100 changed files with 1519 additions and 449 deletions

View file

@ -291,6 +291,8 @@ public abstract class AbilityImpl implements Ability {
// fused or spliced spells contain multiple abilities (e.g. fused, left, right)
// optional costs and cost modification must be applied only to the first/main ability
// TODO: need tests with X announced costs, cost modification effects, CostAdjuster, early cost target, etc
// can be bugged due multiple calls (not all code parts below use isMainPartAbility)
boolean isMainPartAbility = !CardUtil.isFusedPartAbility(this, game);
/* 20220908 - 601.2b
@ -326,6 +328,16 @@ public abstract class AbilityImpl implements Ability {
return false;
}
// 20241022 - 601.2b
// Choose targets for costs that have to be chosen early
// Not yet included in 601.2b but this is where it will be
handleChooseCostTargets(game, controller);
// prepare dynamic costs (must be called before any x announce)
if (isMainPartAbility) {
adjustX(game);
}
// 20121001 - 601.2b
// If the spell has a variable cost that will be paid as it's being cast (such as an {X} in
// its mana cost; see rule 107.3), the player announces the value of that variable.
@ -402,7 +414,7 @@ public abstract class AbilityImpl implements Ability {
// Note: ActivatedAbility does include SpellAbility & PlayLandAbility, but those should be able to be canceled too.
boolean canCancel = this instanceof ActivatedAbility && controller.isHuman();
if (!getTargets().chooseTargets(outcome, this.controllerId, this, noMana, game, canCancel)) {
// was canceled during targer selection
// was canceled during target selection
return false;
}
}
@ -428,7 +440,7 @@ public abstract class AbilityImpl implements Ability {
//20101001 - 601.2e
if (isMainPartAbility) {
adjustCosts(game); // still needed for CostAdjuster objects (to handle some types of dynamic costs)
// adjustX already called before any announces
game.getContinuousEffects().costModification(this, game);
}
@ -726,6 +738,11 @@ public abstract class AbilityImpl implements Ability {
((EarlyTargetCost) cost).chooseTarget(game, this, controller);
}
}
for (ManaCost cost : getManaCostsToPay()) {
if (cost instanceof EarlyTargetCost && cost.getTargets().isEmpty()) {
((EarlyTargetCost) cost).chooseTarget(game, this, controller);
}
}
}
/**
@ -764,8 +781,15 @@ public abstract class AbilityImpl implements Ability {
if (!variableManaCost.isPaid()) { // should only happen for human players
int xValue;
if (!noMana || variableManaCost.getCostType().canUseAnnounceOnFreeCast()) {
xValue = controller.announceXMana(variableManaCost.getMinX(), variableManaCost.getMaxX(),
"Announce the value for " + variableManaCost.getText(), game, this);
if (variableManaCost.wasAnnounced()) {
// announce by rules
xValue = variableManaCost.getAmount();
} else {
// announce by player
xValue = controller.announceXMana(variableManaCost.getMinX(), variableManaCost.getMaxX(),
"Announce the value for " + variableManaCost.getText(), game, this);
}
int amountMana = xValue * variableManaCost.getXInstancesCount();
StringBuilder manaString = threadLocalBuilder.get();
if (variableManaCost.getFilter() == null || variableManaCost.getFilter().isGeneric()) {
@ -1072,6 +1096,60 @@ public abstract class AbilityImpl implements Ability {
}
}
@Override
public void setVariableCostsMinMax(int min, int max) {
// modify all values (mtg rules allow only one type of X, so min/max must be shared between all X instances)
// base cost
for (ManaCost cost : getManaCosts()) {
if (cost instanceof MinMaxVariableCost) {
MinMaxVariableCost minMaxCost = (MinMaxVariableCost) cost;
minMaxCost.setMinX(min);
minMaxCost.setMaxX(max);
}
}
// prepared cost
for (ManaCost cost : getManaCostsToPay()) {
if (cost instanceof MinMaxVariableCost) {
MinMaxVariableCost minMaxCost = (MinMaxVariableCost) cost;
minMaxCost.setMinX(min);
minMaxCost.setMaxX(max);
}
}
}
@Override
public void setVariableCostsValue(int xValue) {
// only mana cost supported
// base cost
boolean foundBaseCost = false;
for (ManaCost cost : getManaCosts()) {
if (cost instanceof VariableManaCost) {
foundBaseCost = true;
((VariableManaCost) cost).setMinX(xValue);
((VariableManaCost) cost).setMaxX(xValue);
((VariableManaCost) cost).setAmount(xValue, xValue, false);
}
}
// prepared cost
boolean foundPreparedCost = false;
for (ManaCost cost : getManaCostsToPay()) {
if (cost instanceof VariableManaCost) {
foundPreparedCost = true;
((VariableManaCost) cost).setMinX(xValue);
((VariableManaCost) cost).setMaxX(xValue);
((VariableManaCost) cost).setAmount(xValue, xValue, false);
}
}
if (!foundPreparedCost || !foundBaseCost) {
throw new IllegalArgumentException("Wrong code usage: auto-announced X values allowed in mana costs only");
}
}
@Override
public void addEffect(Effect effect) {
if (effect != null) {
@ -1676,17 +1754,6 @@ public abstract class AbilityImpl implements Ability {
}
}
/**
* Dynamic cost modification for ability.<br>
* Example: if it need stack related info (like real targets) then must
* check two states (game.inCheckPlayableState): <br>
* 1. In playable state it must check all possible use cases (e.g. allow to
* reduce on any available target and modes) <br>
* 2. In real cast state it must check current use case (e.g. real selected
* targets and modes)
*
* @param costAdjuster
*/
@Override
public AbilityImpl setCostAdjuster(CostAdjuster costAdjuster) {
this.costAdjuster = costAdjuster;
@ -1694,14 +1761,23 @@ public abstract class AbilityImpl implements Ability {
}
@Override
public CostAdjuster getCostAdjuster() {
return costAdjuster;
public void adjustX(Game game) {
if (costAdjuster != null) {
costAdjuster.prepareX(this, game);
}
}
@Override
public void adjustCosts(Game game) {
public void adjustCostsPrepare(Game game) {
if (costAdjuster != null) {
costAdjuster.adjustCosts(this, game);
costAdjuster.prepareCost(this, game);
}
}
@Override
public void adjustCostsModify(Game game, CostModificationType costModificationType) {
if (costAdjuster != null) {
costAdjuster.modifyCost(this, game, costModificationType);
}
}