Skip to content

Commit

Permalink
Trivial: Clean up some placement code. (#11821)
Browse files Browse the repository at this point in the history
No functional changes.

Cleans up code in AbstractPlaceDelegate:
  - Use player.getRulesAttachment() (also elsewhere)
  - Fix comments that were incorrectly wrapped before
  - Fix typos found by Intellij
  - Remove unnecessary extra wrapper functions
  - Remove a param that's always true
  • Loading branch information
asvitkine authored Jul 26, 2023
1 parent 9572dea commit 2aa26ae
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 99 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,11 @@ protected int getMaxUnitsToBePlacedFrom(

// Return collection of bid units which can placed in a land territory
@Override
protected Collection<Unit> getUnitsToBePlacedLand(
protected Collection<Unit> getUnitsToBePlaced(
final Territory to, final Collection<Unit> units, final GamePlayer player) {
if (to.isWater()) {
return super.getUnitsToBePlaced(to, units, player);
}
final Collection<Unit> unitsAtStartOfTurnInTo = unitsAtStartOfStepInTerritory(to);
final Collection<Unit> placeableUnits = new ArrayList<>();
// we add factories and constructions later
Expand All @@ -129,8 +132,8 @@ protected Collection<Unit> getUnitsToBePlacedLand(
final int maxUnits = howManyOfConstructionUnit(currentUnit, constructionsMap);
if (maxUnits > 0) {
// we are doing this because we could have multiple unitTypes with the same
// constructionType, so we have to be
// able to place the max placement by constructionType of each unitType
// constructionType, so we have to be able to place the max placement by constructionType
// of each unitType
if (skipUnit.contains(currentUnit)) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import games.strategy.engine.data.UnitType;
import games.strategy.engine.data.UnitTypeList;
import games.strategy.engine.data.properties.GameProperties;
import games.strategy.triplea.Constants;
import games.strategy.triplea.Properties;
import games.strategy.triplea.attachments.AbstractUserActionAttachment;
import games.strategy.triplea.attachments.ICondition;
Expand Down Expand Up @@ -996,8 +995,7 @@ public static Predicate<Territory> territoryIsPassableAndNotRestricted(final Gam
if (!Properties.getMovementByTerritoryRestricted(player.getData().getProperties())) {
return true;
}
final RulesAttachment ra =
(RulesAttachment) player.getAttachment(Constants.RULES_ATTACHMENT_NAME);
final RulesAttachment ra = player.getRulesAttachment();
if (ra == null || ra.getMovementRestrictionTerritories() == null) {
return true;
}
Expand Down Expand Up @@ -1047,9 +1045,7 @@ && territoryIsNeutralButNotWater().test(t)) {
return false;
}
if (Properties.getMovementByTerritoryRestricted(properties)) {
final RulesAttachment ra =
(RulesAttachment)
playerWhoOwnsAllTheUnitsMoving.getAttachment(Constants.RULES_ATTACHMENT_NAME);
final RulesAttachment ra = playerWhoOwnsAllTheUnitsMoving.getRulesAttachment();
if (ra != null && ra.getMovementRestrictionTerritories() != null) {
final String movementRestrictionType = ra.getMovementRestrictionType();
final Collection<Territory> listedTerritories =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ private Collection<Unit> getProductionUnits(
return productionUnits;
}
IntegerMap<UnitType> productionPerXTerritories = new IntegerMap<>();
final RulesAttachment ra =
(RulesAttachment) player.getAttachment(Constants.RULES_ATTACHMENT_NAME);
final RulesAttachment ra = player.getRulesAttachment();
// if they have no rules attachments, but are calling NoPU purchase, and have the game property
// isProductionPerValuedTerritoryRestricted, then they want 1 infantry for each territory with
// PU value > 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -644,8 +644,7 @@ private MoveValidationResult validateMovementRestrictedByTerritory(
if (!Properties.getMovementByTerritoryRestricted(data.getProperties())) {
return result;
}
final RulesAttachment ra =
(RulesAttachment) player.getAttachment(Constants.RULES_ATTACHMENT_NAME);
final RulesAttachment ra = player.getRulesAttachment();
if (ra == null || ra.getMovementRestrictionTerritories() == null) {
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import games.strategy.engine.data.GameSequence;
import games.strategy.engine.data.TerritoryEffect;
import games.strategy.engine.data.Unit;
import games.strategy.triplea.Constants;
import games.strategy.triplea.attachments.RulesAttachment;
import games.strategy.triplea.delegate.TerritoryEffectHelper;
import games.strategy.triplea.delegate.battle.BattleState;
Expand Down Expand Up @@ -172,18 +171,16 @@ private boolean isFirstTurnLimitedRoll(final GamePlayer player) {
}

private boolean isNegateDominatingFirstRoundAttack(final GamePlayer player) {
final RulesAttachment rulesAttachment =
(RulesAttachment) player.getAttachment(Constants.RULES_ATTACHMENT_NAME);
return rulesAttachment != null && rulesAttachment.getNegateDominatingFirstRoundAttack();
final RulesAttachment ra = player.getRulesAttachment();
return ra != null && ra.getNegateDominatingFirstRoundAttack();
}

private boolean isDominatingFirstRoundAttack(final GamePlayer player) {
if (player == null) {
return false;
}
final RulesAttachment rulesAttachment =
(RulesAttachment) player.getAttachment(Constants.RULES_ATTACHMENT_NAME);
return rulesAttachment != null && rulesAttachment.getDominatingFirstRoundAttack();
final RulesAttachment ra = player.getRulesAttachment();
return ra != null && ra.getDominatingFirstRoundAttack();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package games.strategy.triplea.delegate.power.calculator;

import static games.strategy.triplea.Constants.RULES_ATTACHMENT_NAME;
import static games.strategy.triplea.Constants.TERRITORYEFFECT_ATTACHMENT_NAME;
import static games.strategy.triplea.Constants.UNIT_ATTACHMENT_NAME;
import static games.strategy.triplea.delegate.battle.steps.MockGameData.givenGameData;
Expand Down Expand Up @@ -240,7 +239,7 @@ void calculatesValueWithFirstTurnLimited() throws GameParseException {
final GamePlayer attacker = mock(GamePlayer.class);
final RulesAttachment rulesAttachment = mock(RulesAttachment.class);
when(rulesAttachment.getDominatingFirstRoundAttack()).thenReturn(true);
when(attacker.getAttachment(RULES_ATTACHMENT_NAME)).thenReturn(rulesAttachment);
when(attacker.getRulesAttachment()).thenReturn(rulesAttachment);

final GameData gameData = givenGameData().withDiceSides(6).withRound(1, attacker).build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import games.strategy.engine.data.Territory;
import games.strategy.engine.data.Unit;
import games.strategy.engine.data.UnitType;
import games.strategy.triplea.Constants;
import games.strategy.triplea.Properties;
import games.strategy.triplea.UnitUtils;
import games.strategy.triplea.attachments.RulesAttachment;
Expand Down Expand Up @@ -221,8 +220,7 @@ private static int totalUnitNumberPurchased(final IntegerMap<ProductionRule> pur
}

private static boolean isUnlimitedProduction(final GamePlayer player) {
final RulesAttachment ra =
(RulesAttachment) player.getAttachment(Constants.RULES_ATTACHMENT_NAME);
final RulesAttachment ra = player.getRulesAttachment();
return ra != null && ra.getUnlimitedProduction();
}

Expand Down

0 comments on commit 2aa26ae

Please sign in to comment.