Failure/foul-magics
1
0
Fork 0
forked from External/mage
Code Pull requests Releases Packages Activity Actions

Refactor: private fields and performance tweaks (#9625)

Browse source 
1a. Make `costs`, `manaCosts`, and `manaCostsToPay` private in `AbilityImpl` with access through getters/setters 
1b. fix cost adjuster for imprinted cards affected by the above

2a. Lazy instantiation for rarely used `data` field in `TargetPointerImpl`

3a. Pre-allocate certain array sizes in `Modes` and `CostsImpl`

4a. Make `manaTemplate` private in `BasicManaEffect`, copy when passing outside the class 
4b. Don't copy `manaTemplate` in copy constructor since it doesn't change 
4c. Add comments explaining copy usage for `manaTemplate` 
4d. Remove redundant variable assignment and make fields final 

---------

Co-authored-by: xenohedron <xenohedron@users.noreply.github.com>
This commit is contained in:
Alex Vasile 2023-08-27 17:58:19 -04:00 • committed by GitHub
parent 53be4f384e
commit a2162ec3e7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
65 changed files with 262 additions and 196 deletions
Download patch file Download diff file Expand all files Collapse all files

83
Mage/src/main/java/mage/abilities/AbilityImpl.java
View file

@ -56,9 +56,9 @@ public abstract class AbilityImpl implements Ability {
protected AbilityType abilityType;
protected UUID controllerId;
protected UUID sourceId;
protected ManaCosts<ManaCost> manaCosts;
protected ManaCosts<ManaCost> manaCostsToPay;
protected Costs<Cost> costs;
private ManaCosts<ManaCost> manaCosts;
private ManaCosts<ManaCost> manaCostsToPay;
private Costs<Cost> costs;
protected Modes modes; // access to it by GetModes only (it can be overridden by some abilities)
protected Zone zone;
protected String name;
@ -253,14 +253,14 @@ public abstract class AbilityImpl implements Ability {
if (noMana) {
if (!this.getManaCostsToPay().getVariableCosts().isEmpty()) {
int xValue = this.getManaCostsToPay().getX();
this.getManaCostsToPay().clear();
this.clearManaCostsToPay();
VariableManaCost xCosts = new VariableManaCost(VariableCostType.ADDITIONAL);
// no x events - rules from Unbound Flourishing:
// - Spells with additional costs that include X won't be affected by Unbound Flourishing. X must be in the spell's mana cost.
xCosts.setAmount(xValue, xValue, false);
this.getManaCostsToPay().add(xCosts);
addManaCostsToPay(xCosts);
} else {
this.getManaCostsToPay().clear();
this.clearManaCostsToPay();
}
}
@ -379,7 +379,7 @@ public abstract class AbilityImpl implements Ability {
}
// this is a hack to prevent mana abilities with mana costs from causing endless loops - pay other costs first
if (this instanceof ActivatedManaAbilityImpl && !costs.pay(this, game, this, controllerId, noMana, null)) {
if (this instanceof ActivatedManaAbilityImpl && !getCosts().pay(this, game, this, controllerId, noMana, null)) {
logger.debug("activate mana ability failed - non mana costs");
return false;
}
@ -396,12 +396,12 @@ public abstract class AbilityImpl implements Ability {
}
//20100716 - 601.2f (noMana is not used here, because mana costs were cleared for this ability before adding additional costs and applying cost modification effects)
if (!manaCostsToPay.pay(this, game, this, activatorId, false, null)) {
if (!getManaCostsToPay().pay(this, game, this, activatorId, false, null)) {
return false; // cancel during mana payment
}
//20100716 - 601.2g
if (!costs.pay(this, game, this, activatorId, noMana, null)) {
if (!getCosts().pay(this, game, this, activatorId, noMana, null)) {
logger.debug("activate failed - non mana costs");
return false;
}
@ -509,13 +509,11 @@ public abstract class AbilityImpl implements Ability {
*/
protected String handleOtherXCosts(Game game, Player controller) {
StringBuilder announceString = new StringBuilder();
for (VariableCost variableCost : this.costs.getVariableCosts()) {
for (VariableCost variableCost : this.getCosts().getVariableCosts()) {
if (!(variableCost instanceof VariableManaCost) && !((Cost) variableCost).isPaid()) {
int xValue = variableCost.announceXValue(this, game);
Cost fixedCost = variableCost.getFixedCostsFromAnnouncedValue(xValue);
if (fixedCost != null) {
costs.add(fixedCost);
}
addCost(fixedCost);
// set the xcosts to paid
// no x events - rules from Unbound Flourishing:
// - Spells with additional costs that include X won't be affected by Unbound Flourishing. X must be in the spell's mana cost.
@ -534,7 +532,7 @@ public abstract class AbilityImpl implements Ability {
* life or the corresponding colored mana cost for each of those symbols.
*/
private void handlePhyrexianManaCosts(Game game, Player controller) {
Iterator<ManaCost> costIterator = manaCostsToPay.iterator();
Iterator<ManaCost> costIterator = getManaCostsToPay().iterator();
while (costIterator.hasNext()) {
ManaCost cost = costIterator.next();
@ -545,8 +543,8 @@ public abstract class AbilityImpl implements Ability {
if (payLifeCost.canPay(this, this, controller.getId(), game)
&& controller.chooseUse(Outcome.LoseLife, "Pay 2 life instead of " + cost.getText().replace("/P", "") + '?', this, game)) {
costIterator.remove();
costs.add(payLifeCost);
manaCostsToPay.incrPhyrexianPaid();
addCost(payLifeCost);
getManaCostsToPay().incrPhyrexianPaid();
}
}
}
@ -580,7 +578,7 @@ public abstract class AbilityImpl implements Ability {
// TODO: Handle announcing other variable costs here like: RemoveVariableCountersSourceCost
VariableManaCost variableManaCost = null;
for (ManaCost cost : manaCostsToPay) {
for (ManaCost cost : getManaCostsToPay()) {
if (cost instanceof VariableManaCost) {
if (variableManaCost == null) {
variableManaCost = (VariableManaCost) cost;
@ -625,8 +623,8 @@ public abstract class AbilityImpl implements Ability {
manaString.append('{').append(manaSymbol).append('}');
}
}
manaCostsToPay.add(new ManaCostsImpl<>(manaString.toString()));
manaCostsToPay.setX(xValue * xValueMultiplier, amountMana);
addManaCostsToPay(new ManaCostsImpl<>(manaString.toString()));
getManaCostsToPay().setX(xValue * xValueMultiplier, amountMana);
}
variableManaCost.setPaid();
}
@ -788,14 +786,14 @@ public abstract class AbilityImpl implements Ability {
public String getRule(boolean all) {
StringBuilder sbRule = threadLocalBuilder.get();
if (all || this.abilityType != AbilityType.SPELL) { // TODO: Why the override for non-spells?
if (!manaCosts.isEmpty()) {
sbRule.append(manaCosts.getText());
if (!getManaCosts().isEmpty()) {
sbRule.append(getManaCosts().getText());
}
if (!costs.isEmpty()) {
if (!getCosts().isEmpty()) {
if (sbRule.length() > 0) {
sbRule.append(", ");
}
sbRule.append(costs.getText());
sbRule.append(getCosts().getText());
}
if (sbRule.length() > 0) {
sbRule.append(": ");
@ -866,19 +864,46 @@ public abstract class AbilityImpl implements Ability {
} else {
// as single cost
if (cost instanceof ManaCost) {
this.addManaCost((ManaCost) cost);
addManaCost((ManaCost) cost);
} else {
this.costs.add(cost);
if (costs == null) {
costs = new CostsImpl<>();
}
costs.add(cost);
}
}
}
@Override
public void addManaCost(ManaCost cost) {
if (cost != null) {
this.manaCosts.add(cost);
this.manaCostsToPay.add(cost);
public void addManaCostsToPay(ManaCost manaCost) {
if (manaCost == null) {
return;
}
if (manaCostsToPay == null) {
manaCostsToPay = new ManaCostsImpl<>();
}
if (manaCost instanceof ManaCosts) {
manaCostsToPay.addAll((ManaCosts) manaCost);
} else {
manaCostsToPay.add(manaCost);
}
}
@Override
public void addManaCost(ManaCost manaCost) {
if (manaCost == null) {
return;
}
if (manaCosts == null) {
manaCosts = new ManaCostsImpl<>();
}
if (manaCostsToPay == null) {
manaCostsToPay = new ManaCostsImpl<>();
}
manaCosts.add(manaCost);
manaCostsToPay.add(manaCost);
}
@Override